Re: [PATCH v3 4/4] net: phy: at803x: add RX and TX clock management for IPQ5018 PHY

[Andrew Lunn]

> >> This PHY is integrated into the IPQ5018 SoC, connected to the first GMAC
> >> (GMAC0) and probed upon boot. However, this PHY is not used on all
> >> boards because an external PHY or switch can be wired to the SoC's
> >> second GMAC instead (through a PCS). So from a power management
> >> perspective, it would be better if we can disable the clocks if there's
> >> no link detected.
> > 
> > Humm, is link the correct criteria? If the PHY is not used,
> > .config_aneg should not be called. Why not have the probe method get
> > the optional clocks, but leave them off. When .config_aneg is called
> > for the first time, enable the clocks?
> 
> Will check if config_aneg is called and test accordingly.
> 
> ip link set eth0 up/down and cable (un)plug do trigger
> link_change_notify, and based on the link state the RX/TX clocks are
> turned off/on properly.

You are talking about something else here. You say the device is not
used. If it is not used, .config_aneg should not be called.

This is a second use case, the device is used, and you want to limit
the power it consumes, when there is no link. Do you have any numbers?
How much power is actually saved?

> Could you explain why to use _optional variants of devm_clk_get as these
> clocks are required? There are currently no users upstream.

This last bit is important. We don't want to cause regressions on
existing systems by adding in requirements which were not there in the
past. But if you say there are currently no users, that is fine.

	Andrew