Messages in this thread | | | From | John Ernberg <> | Subject | Re: [PATCH net v3 2/2] net: fec: Suspend the PHY on probe | Date | Wed, 20 Mar 2024 15:25:54 +0000 |
| |
Hi Russel,
On 3/19/24 09:51, Russell King (Oracle) wrote: > On Tue, Mar 19, 2024 at 08:37:44AM +0000, John Ernberg wrote: >> There is also a case where the phy driver module is not automatically >> loaded, in cases where request_module() fails, either due to the >> userspace helper feature being compiled out or other reasons, and the >> module is loaded manually later. I suspect for reasons like these the >> genphy probe happens so late. My solution here doesn't cover non-loaded >> modules either, but this could maybe be covered by moving phy_suspend() >> to phy_probe(). Unless there is an even more clever way to go about it >> which I can't see from inexperience. > > Note that in the case where the PHY driver module is loaded late, > phy_probe() won't be called for the PHY until that happens. > > I would say if one wants a platform to behave with minimal power > consumption, that is something that has to be done across the > software stack, and that includes the boot firmware. So, if one > wants the PHY to be in a low power state at boot time, then > firmware needs to ensure that happens. > > Trying to shoe-horn that into the kernel isn't going to work > because we get to decide what to do with the PHY way too late > (due to PHY drivers being modular and on the rootfs.) >
What we really want is the PHY to be suspended on suspend to RAM regardless of us having had an initial link up or not.
This worked prior to 4c0d2e96ba05 ("net: phy: consider that suspend2ram may cut off PHY power") which was added in Linux 5.11, and 557d5dc83f68 ("net: fec: use mac-managed PHY PM") which was added in Linux 5.12.
Since FEC requires mac_managed_pm the generic PM suspend-resume paths are not taken. The resume sequencing with generic PM has been broken with the FEC since generic PM of the mdio bus was added, as the FEC will do phy_start() (via FEC resume) and then generic PM runs phy_init_hw() via mdio bus resume (previously: less damaging phy_resume()) due to how the FEC IP block works.
Some background context to our usecase which might have been lost is that our system bring the link up based on outside input and to conserve power we suspend regularly, and this is the only situation where we care about the power consumption. Since we cannot decide if link shall be up ourselves we can go through numerous suspend cycles before the first link up. We could in theory work around it in userspace by doing "ip link set <eth> up && ip link set <eth> down", but it wasn't required before.
Thanks! // John Ernberg | |