Subject: Re: [PATCH net-next v4 13/12] net: lan865x: optional hardware reset

From: Ramón Nordin Rodriguez <[email-masked]>
Date: Mon, 29 Apr 2024 12:38:05 +0200

> > This commit optionally enables a hardware reset of the lan8650/1
> > mac-phy. These chips have a software reset that is discourage from use
> > in the manual since it only resets the internal phy.
> The software reset done by the current driver is not only resetting the 
> internal PHY, it resets the entire MAC-PHY including the integrated PHY.
> The reset bit of the Clause 22 basic control register only will reset 
> the internal PHY alone. But oa_tc6_sw_reset_macphy() function is writing 
> software reset bit in the Reset Control and Status register which resets 
> the entire MAC-PHY including the internal PHY.

All right, I did not dig deep enough obviously.

> The above note is given in the lan8650 datasheet to let the user to know 
> that clause 22 software reset will reset only internal PHY but I don't 
> think they mean it for the MAC-PHY software reset done from Reset 
> Control and Status register.

Could still be relevant to implement the .soft_reset with -EOPNOTSUPP as
Andrew has suggested in the phy driver.

> 
> So in my opinion, I don't see the need of external pin reset as the 
> existing oa_tc6_sw_reset_macphy() function does the software reset of 
> the entire MAC-PHY.

I agree with your assesment that this invalidates the problem I was
aiming at solving.

Additionally I figured out why my setup did not work without the HW
reset, I had missed a pull resistor in the schematic that held the IC in
reset.

To me it seems more feature complete to have a driver option for the
physical capabilities of the chip, but if it doesn't actually solve a
problem it might just be bloat.

> 
> Still if you see a need to have this external pin reset as an optional 
> function then it may be needed for all the vendor specific MAC drivers. 
> In that case, reset-gpios parameter value alone can be taken from the 
> chip specific device tree and the remaining code for operating the reset 
> gpio can be moved to oa_tc6.c and the function name can be 
> oa_tc6_hw_reset_macphy().
> 

If the consensus is to keep a HW reset I do like this suggestion.

I won't push for this to be included, if it is, I'm happy to address the
feedback of patch 13.

R