[PATCH v3 0/4] IPQ5018: Add and enable GEPHY RX and TX clocks

[PATCH v3 0/4] IPQ5018: Add and enable GEPHY RX and TX clocks

[George Moussalem via B4 Relay]

Greetings,

This patch series addresses a missing hardware description issue for the
Qualcomm IPQ5018 Internal Ethernet PHY, where the data paths fail to
function correctly unless their dedicated RX and TX clocks are
explicitly enabled.

Further testing revealed that leaving these clocks unmanaged by the
kernel, they were inadvertently left enabled by the bootloader / QSDK
platform, which masked the issue. Testing a fresh network configuration
path exposed that the data link fails to work without explicit software
gating.

To correctly introduce the required multi-clock properties, the IPQ5018
binding definition must first be split away from the shared
qca,ar803x.yaml schema. This isolation is required because ar803x
references the generic ethernet-phy.yaml, which enforces a strict
single-clock limit constraint. 

- Patch 1: Moves the clocks property and its restriction out of the
	   generic ethernet-phy.yaml schema to individual bindings files
	   that need it to allow for PHYs that require multiple clocks.
- Patch 2: Add clocks property to qca,ar803x.yaml for the IPQ5018 PHY.
- Patch 3: Appends the missing RX/TX clock definitions into the IPQ5018
           device tree before driver modification to avoid the driver
	   failing to probe.
- Patch 4: Updates the Qualcomm AT803x PHY driver framework to acquire,
	   enable, and gate these clocks upon link state changes for
	   runtime power optimization.

Signed-off-by: George Moussalem <george.moussalem@outlook.com>
---
Changes in v3:
- Revert the change to move the clocks property out of the generic
  ethernet-phy.yaml schema and simple increase the maxItems limit to 2.
- Listed clocks, clock-names, and resets as required properties in the
  schema.
- Acquire the RX and TX clock during probe without enabling them. Then
  enable/disable the clocks in the link state change callback to ensure
  they are only active when needed with checks to avoid redundant
  enable/disable calls.
- Re-ran make dt_binding_check on all schemas in net folder without new
  issues.
- Link to v2: https://lore.kernel.org/r/20260602-ipq5018-gephy-clocks-v2-0-65a1f1d881f3@outlook.com

Changes in v2:
- Added patch 1 to move the clocks property and its restriction out of
  the generic ethernet-phy.yaml schema to individual bindings files that
  need it to allow for PHYs that require multiple clocks.
- Reverted splitting out IPQ5018 from the shared qca,ar803x.yaml schema
  and simply added the clocks and clock-names properties to the
  definition of the IPQ5018 PHY.
- Corrected / updated commit title of patch 4 (qca,at803x -> at803x)
- Link to v1: https://lore.kernel.org/r/20260601-ipq5018-gephy-clocks-v1-0-2df8287712c3@outlook.com

---
George Moussalem (4):
      dt-bindings: net: ethernet-phy: increase max clock count to two
      dt-bindings: net: qca,ar803x: Add clocks for IPQ5018 PHY
      arm64: qcom: ipq5018: Add GEPHY RX and TX clocks
      net: phy: at803x: add RX and TX clock management for IPQ5018 PHY

 .../devicetree/bindings/net/ethernet-phy.yaml      |  9 +++--
 .../devicetree/bindings/net/qca,ar803x.yaml        | 19 ++++++++++
 arch/arm64/boot/dts/qcom/ipq5018.dtsi              |  3 ++
 drivers/net/phy/qcom/at803x.c                      | 43 ++++++++++++++++++++++
 4 files changed, 71 insertions(+), 3 deletions(-)
---
base-commit: 7da7f07112610a520567421dd2ffcb51beaefbcc
change-id: 20260601-ipq5018-gephy-clocks-e8a2440178a0

Best regards,
-- 
George Moussalem <george.moussalem@outlook.com>

[PATCH v3 1/4] dt-bindings: net: ethernet-phy: increase max clock count to two

[George Moussalem via B4 Relay]

From: George Moussalem <george.moussalem@outlook.com>

The clocks property has a restriction to maximum one.
Yet, some PHYs may require more than 1 clock such as the IPQ5018 PHY
which requires two clocks for RX and TX. As such, increase maxItems to
two.

Signed-off-by: George Moussalem <george.moussalem@outlook.com>
---
Commit 350b7a258f20 introduced the clocks property with a restriction to
maximum 1 to the main ethernet-phy.yaml binding for Realtek to add an
optional external clock source. This is restrictive to all PHY bindings,
as some PHYs may require more than 1 clock such as the IPQ5018 PHY which
requires 2 clocks (for RX and TX).
---
 Documentation/devicetree/bindings/net/ethernet-phy.yaml | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/ethernet-phy.yaml b/Documentation/devicetree/bindings/net/ethernet-phy.yaml
index 21a1a63506f0..c3ebb3af8b52 100644
--- a/Documentation/devicetree/bindings/net/ethernet-phy.yaml
+++ b/Documentation/devicetree/bindings/net/ethernet-phy.yaml
@@ -106,10 +106,13 @@ properties:
       by software.
 
   clocks:
-    maxItems: 1
+    minItems: 1
+    maxItems: 2
     description:
-      External clock connected to the PHY. If not specified it is assumed
-      that the PHY uses a fixed crystal or an internal oscillator.
+      External clock connected to the PHY or RX and TX clocks that the PHY
+      requires to enable explicitly. If not specified it is assumed
+      that the PHY uses a fixed crystal or an internal oscillator or that the
+      RX/TX clocks are hardware enabled by default.
 
   enet-phy-lane-swap:
     $ref: /schemas/types.yaml#/definitions/flag

-- 
2.53.0

Re: [PATCH v3 1/4] dt-bindings: net: ethernet-phy: increase max clock count to two

[Rob Herring]

On Fri, Jun 05, 2026 at 04:41:26PM +0400, George Moussalem wrote:
> The clocks property has a restriction to maximum one.
> Yet, some PHYs may require more than 1 clock such as the IPQ5018 PHY
> which requires two clocks for RX and TX. As such, increase maxItems to
> two.
> 
> Signed-off-by: George Moussalem <george.moussalem@outlook.com>
> ---
> Commit 350b7a258f20 introduced the clocks property with a restriction to
> maximum 1 to the main ethernet-phy.yaml binding for Realtek to add an
> optional external clock source. This is restrictive to all PHY bindings,
> as some PHYs may require more than 1 clock such as the IPQ5018 PHY which
> requires 2 clocks (for RX and TX).
> ---
>  Documentation/devicetree/bindings/net/ethernet-phy.yaml | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)

Reviewed-by: Rob Herring (Arm) <robh@kernel.org>

[PATCH v3 2/4] dt-bindings: net: qca,ar803x: Add clocks for IPQ5018 PHY

[George Moussalem via B4 Relay]

From: George Moussalem <george.moussalem@outlook.com>

Further testing revealed that the RX and TX clocks of the IPQ5018 PHY
need to be explicitly enabled. As such, add the required clocks to the
schema.

Acked-by: Conor Dooley <conor.dooley@microchip.com>
Signed-off-by: George Moussalem <george.moussalem@outlook.com>
---
 Documentation/devicetree/bindings/net/qca,ar803x.yaml | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/Documentation/devicetree/bindings/net/qca,ar803x.yaml b/Documentation/devicetree/bindings/net/qca,ar803x.yaml
index 7ae5110e7aa2..53f648c4135f 100644
--- a/Documentation/devicetree/bindings/net/qca,ar803x.yaml
+++ b/Documentation/devicetree/bindings/net/qca,ar803x.yaml
@@ -28,6 +28,16 @@ allOf:
         reg:
           const: 7  # This PHY is always at MDIO address 7 in the IPQ5018 SoC
 
+        clocks:
+          items:
+            - description: RX clock
+            - description: TX clock
+
+        clock-names:
+          items:
+            - const: rx
+            - const: tx
+
         resets:
           items:
             - description:
@@ -42,6 +52,11 @@ allOf:
             of this PHY are directly connected to an RJ45 connector.
           type: boolean
 
+      required:
+        - clocks
+        - clock-names
+        - resets
+
 properties:
   compatible:
     enum:
@@ -162,6 +177,7 @@ examples:
         };
     };
   - |
+    #include <dt-bindings/clock/qcom,gcc-ipq5018.h>
     #include <dt-bindings/reset/qcom,gcc-ipq5018.h>
 
     mdio {
@@ -172,6 +188,9 @@ examples:
             compatible = "ethernet-phy-id004d.d0c0";
             reg = <7>;
 
+            clocks = <&gcc GCC_GEPHY_RX_CLK>,
+                     <&gcc GCC_GEPHY_TX_CLK>;
+            clock-names = "rx", "tx";
             resets = <&gcc GCC_GEPHY_MISC_ARES>;
         };
     };

-- 
2.53.0

[PATCH v3 3/4] arm64: qcom: ipq5018: Add GEPHY RX and TX clocks

[George Moussalem via B4 Relay]

From: George Moussalem <george.moussalem@outlook.com>

Add RX and TX clocks for the IPQ5018 GEPHY to enable the datapath.

Fixes: f5f2b835e316 ("arm64: dts: qcom: ipq5018: Add GE PHY to internal mdio bus")
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
Signed-off-by: George Moussalem <george.moussalem@outlook.com>
---
 arch/arm64/boot/dts/qcom/ipq5018.dtsi | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/ipq5018.dtsi b/arch/arm64/boot/dts/qcom/ipq5018.dtsi
index 6f8004a22a1f..60c27a6f2b10 100644
--- a/arch/arm64/boot/dts/qcom/ipq5018.dtsi
+++ b/arch/arm64/boot/dts/qcom/ipq5018.dtsi
@@ -229,6 +229,9 @@ ge_phy: ethernet-phy@7 {
 				compatible = "ethernet-phy-id004d.d0c0";
 				reg = <7>;
 
+				clocks = <&gcc GCC_GEPHY_RX_CLK>,
+					 <&gcc GCC_GEPHY_TX_CLK>;
+				clock-names = "rx", "tx";
 				resets = <&gcc GCC_GEPHY_MISC_ARES>;
 			};
 		};

-- 
2.53.0

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

[George Moussalem via B4 Relay]

From: George Moussalem <george.moussalem@outlook.com>

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.

Fixes: d46502279a11 ("net: phy: qcom: at803x: Add Qualcomm IPQ5018 Internal PHY support")
Signed-off-by: George Moussalem <george.moussalem@outlook.com>
---
 drivers/net/phy/qcom/at803x.c | 43 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 43 insertions(+)

diff --git a/drivers/net/phy/qcom/at803x.c b/drivers/net/phy/qcom/at803x.c
index 63726cf98cd4..99bc710531a4 100644
--- a/drivers/net/phy/qcom/at803x.c
+++ b/drivers/net/phy/qcom/at803x.c
@@ -19,6 +19,8 @@
 #include <linux/regulator/consumer.h>
 #include <linux/of.h>
 #include <linux/phylink.h>
+#include <linux/clk.h>
+#include <linux/clk-provider.h>
 #include <linux/reset.h>
 #include <linux/phy_port.h>
 #include <dt-bindings/net/qca-ar803x.h>
@@ -176,6 +178,8 @@ struct at803x_context {
 };
 
 struct ipq5018_priv {
+	struct clk *rx_clk;
+	struct clk *tx_clk;
 	struct reset_control *rst;
 	bool set_short_cable_dac;
 };
@@ -1062,6 +1066,35 @@ static int ipq5018_config_init(struct phy_device *phydev)
 
 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)) {
+			ret = clk_prepare_enable(priv->rx_clk);
+			if (ret) {
+				dev_err(&phydev->mdio.dev,
+					"failed to enable RX clock\n");
+				goto reset_fifo;
+			}
+		}
+
+		if (!__clk_is_enabled(priv->tx_clk)) {
+			ret = clk_prepare_enable(priv->tx_clk);
+			if (ret) {
+				dev_err(&phydev->mdio.dev,
+					"failed to enable TX clock\n");
+				clk_disable_unprepare(priv->rx_clk);
+			}
+		}
+	} else {
+		if (__clk_is_enabled(priv->rx_clk))
+			clk_disable_unprepare(priv->rx_clk);
+		if (__clk_is_enabled(priv->tx_clk))
+			clk_disable_unprepare(priv->tx_clk);
+	}
+
+reset_fifo:
 	/*
 	 * Reset the FIFO buffer upon link disconnects to clear any residual data
 	 * which may cause issues with the FIFO which it cannot recover from.
@@ -1084,6 +1117,16 @@ static int ipq5018_probe(struct phy_device *phydev)
 	priv->set_short_cable_dac = of_property_read_bool(dev->of_node,
 							  "qcom,dac-preset-short-cable");
 
+	priv->rx_clk = devm_clk_get(dev, "rx");
+	if (IS_ERR(priv->rx_clk))
+		return dev_err_probe(dev, PTR_ERR(priv->rx_clk),
+				     "failed to get RX clock\n");
+
+	priv->tx_clk = devm_clk_get(dev, "tx");
+	if (IS_ERR(priv->tx_clk))
+		return dev_err_probe(dev, PTR_ERR(priv->tx_clk),
+				     "failed to get TX clock\n");
+
 	priv->rst = devm_reset_control_array_get_exclusive(dev);
 	if (IS_ERR(priv->rst))
 		return dev_err_probe(dev, PTR_ERR(priv->rst),

-- 
2.53.0

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

[Andrew Lunn]

On Fri, Jun 05, 2026 at 04:41:29PM +0400, George Moussalem via B4 Relay wrote:
> From: George Moussalem <george.moussalem@outlook.com>
> 
> 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?

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.

     Andrew

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@outlook.com>
>>
>> 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

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

[Andrew Lunn]

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

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

    Andrew

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

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