Messages in this thread | | | Date | Sat, 11 Dec 2021 15:57:54 +0100 | From | Francesco Dolcini <> | Subject | Re: [PATCH net-next] net: phy: perform a PHY reset on resume |
| |
Hello,
On Sat, Dec 11, 2021 at 02:15:54PM +0000, Russell King (Oracle) wrote: > I don't particularly like this - this impacts everyone who is using > phylib at this point, whereas no reset was happening if the reset was > already deasserted here.
I understand your concern, but I do not believe that this can create any issue. The code should be able to handle the situation in which the PHY is getting out of reset at that time.
> In the opening remarks to this series, it is stated: > > If a hardware-design is able to control power to the Ethernet PHY and > relying on software to do a reset, the PHY does no longer work after > resuming from suspend, given the PHY does need a hardware-reset. > > This requirement is conditional on the hardware design, it isn't a > universal requirement and won't apply everywhere. I think it needs to > be described in firmware that this action is required. That said... > > Please check the datasheet on the PHY regarding application of power and > reset. You may find that the PHY datasheet requires the reset to be held > active from power up until the clock input is stable - this could mean > you need some other arrangement to assert the PHY reset signal after > re-applying power sooner than would happen by the proposed point in the > kernel.
I checked before sending this patch, the phy is a KSZ9131 [1] and according to the power sequence timing the reset should be toggled after the power-up. No requirement on the clock or other signals.
The HW design is quite simple, we have a regulator controlling the PHY power, and a GPIO controlling the reset. On suspend we remove the power (FEC driver), on resume after enabling the power the PHY require a reset, but nobody is doing it.
The issue here is that the phy regulator is handled by the FEC driver, while the RESET_N GPIO can be controlled by both the FEC driver or the phylib. The initial proposal was to handle this into the FEC driver, but it was not considered a good idea, therefore this new proposal.
One more comment about that, I do not believe that asserting the reset in the suspend path is a good idea, in the general situation in which the PHY is powered in suspend the power-consumption is likely to be higher if the device is in reset compared to software power-down using the BMCR register.
> universal requirement and won't apply everywhere. I think it needs to > be described in firmware that this action is required. That said... Are you thinking at a DTS property? Not sure to understand how do you envision this. At the moment the regulator is not handled by the phylib, and the property should be something like reset-on-resume, I guess ...
Francesco
[1] https://ww1.microchip.com/downloads/en/DeviceDoc/00002841C.pdf
| |