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

Ramón Nordin Rodriguez
2024-04-29T10:38:09Z

> > 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