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

[George Moussalem]

On 6/5/26 19:01, Andrew Lunn wrote:
>> Link state detection works without these clocks though which
>> explains the approach to enabling/disabling them upon link state changes.
> 
> It would be good to add that to the commit message, since it is not
> what i would expect. However, ...

Sure, I'll add it to the commit message.

> 
>> 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. I also went for link_change_notify in the
scenario that the PHY is wired up but the port isn't used (cable
unplugged) to avoid these clocks running unnecessarily. Thoughts?

The if checks were added based on earlier comments in v2 to ensure the
enable count isn't incremented more than once.

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

Kindly advise on best direction.

> 
>     Andrew

Thanks for the guidance as always,
George