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

[George Moussalem]

On 6/5/26 22:14, Andrew Lunn wrote:
>>>> 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?

I've given it some more thought, I think it's best we just use
devm_clk_get_enabled on both clocks during probe. For devices that don't
use the PHY, we can disable it in the device tree and we don't have to
store the clocks in the priv. It makes it much simpler. Would you agree
with that approach?

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

Best regards,
George