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

[George Moussalem]

On 6/5/26 18:23, Andrew Lunn wrote:
> On Fri, Jun 05, 2026 at 04:41:29PM +0400, George Moussalem via B4 Relay wrote:
>> From: George Moussalem <george.moussalem@xxxxxxxxxxx>
>>
>> Acquire and manage the RX and TX clocks for the IPQ5018 PHY.
>> These clocks are required for the PHY's datapath to function correctly.
>> Gate the clocks upon link state changes for improved power management.
> 
>>  static void ipq5018_link_change_notify(struct phy_device *phydev)
>>  {
>> +	struct ipq5018_priv *priv = phydev->priv;
>> +	int ret;
>> +
>> +	if (phydev->link) {
>> +		if (!__clk_is_enabled(priv->rx_clk)) {
> 
> Using __ methods is usually a bad sign.
> 
> The logical also seems a bit odd. In order to get link, you need to Rx
> and Tx. Or is this device able to perform autoneg, send link pulses,
> without these clocks?

No, RX and TX are critical and need to be enabled for any ethernet data
to pass. Link state detection works without these clocks though which
explains the approach to enabling/disabling them upon link state changes.

> 
> Maybe when we have a better understanding of the requirements, we can
> find a better way to use the CCF without needing to go to its insides.

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.

> 
>      Andrew

George