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

[PATCH 0/3] 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: Splits the IPQ5018 definitions to its own bindings schema 
           to accommodate the multi-clock requirement.
- Patch 2: Appends the missing RX/TX clock definitions into the IPQ5018
           device tree before driver modification to avoid the driver
	   failing to probe.
- Patch 3: Updates the Qualcomm AT803x PHY driver framework to safely
	   acquire, enable, and gate these clocks upon link state
	   changes for runtime power optimization.

Signed-off-by: George Moussalem <george.moussalem@outlook.com>
---
George Moussalem (3):
      dt-bindings: net: qca,ipq5018: Split IPQ5018 PHY bindings from ar803x
      arm64: qcom: ipq5018: Add GEPHY RX and TX clocks
      net: phy: qca,at803x: add RX and TX clock management for IPQ5018 PHY

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

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

[PATCH 1/3] dt-bindings: net: qca,ipq5018: Split IPQ5018 PHY bindings from ar803x

[George Moussalem via B4 Relay]

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

Move the definition for the Qualcomm IPQ5018 Internal PHY out of the
qca,ar803x.yaml binding file and isolate them in a dedicated
qca,ipq5018.yaml file. This is necessary due to the restriction of max
one clock in ethernet-phy.yaml which ar803x references as further
testing revealed that the RX and TX clocks of the IPQ5018 PHY need to
be explicitly enabled.

Signed-off-by: George Moussalem <george.moussalem@outlook.com>
---
 .../devicetree/bindings/net/qca,ar803x.yaml        | 43 -------------
 .../devicetree/bindings/net/qca,ipq5018.yaml       | 75 ++++++++++++++++++++++
 2 files changed, 75 insertions(+), 43 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/qca,ar803x.yaml b/Documentation/devicetree/bindings/net/qca,ar803x.yaml
index 7ae5110e7aa2..3acd09f0da86 100644
--- a/Documentation/devicetree/bindings/net/qca,ar803x.yaml
+++ b/Documentation/devicetree/bindings/net/qca,ar803x.yaml
@@ -16,37 +16,8 @@ description: |
 
 allOf:
   - $ref: ethernet-phy.yaml#
-  - if:
-      properties:
-        compatible:
-          contains:
-            enum:
-              - ethernet-phy-id004d.d0c0
-
-    then:
-      properties:
-        reg:
-          const: 7  # This PHY is always at MDIO address 7 in the IPQ5018 SoC
-
-        resets:
-          items:
-            - description:
-                GE PHY MISC reset which triggers a reset across MDC, DSP, RX, and TX lines.
-
-        qcom,dac-preset-short-cable:
-          description:
-            Set if this phy is connected to another phy to adjust the values for
-            MDAC and EDAC to adjust amplitude, bias current settings, and error
-            detection and correction algorithm to accommodate for short cable length.
-            If not set, DAC values are not modified and it is assumed the MDI output pins
-            of this PHY are directly connected to an RJ45 connector.
-          type: boolean
 
 properties:
-  compatible:
-    enum:
-      - ethernet-phy-id004d.d0c0
-
   qca,clk-out-frequency:
     description: Clock output frequency in Hertz.
     $ref: /schemas/types.yaml#/definitions/uint32
@@ -161,17 +132,3 @@ examples:
             };
         };
     };
-  - |
-    #include <dt-bindings/reset/qcom,gcc-ipq5018.h>
-
-    mdio {
-        #address-cells = <1>;
-        #size-cells = <0>;
-
-        ge_phy: ethernet-phy@7 {
-            compatible = "ethernet-phy-id004d.d0c0";
-            reg = <7>;
-
-            resets = <&gcc GCC_GEPHY_MISC_ARES>;
-        };
-    };
diff --git a/Documentation/devicetree/bindings/net/qca,ipq5018.yaml b/Documentation/devicetree/bindings/net/qca,ipq5018.yaml
new file mode 100644
index 000000000000..7cdc96f6d389
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/qca,ipq5018.yaml
@@ -0,0 +1,75 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/net/qca,ipq5018.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Qualcomm Atheros IPQ5018 Internal PHY
+
+maintainers:
+  - George Moussalem <george.moussalem@outlook.com>
+
+description: |
+  The IPQ5018 PHY is part of the IPQ5018 SoC and is always wired to GMAC0.
+  The PHY supports 10/100/1000 mbps link modes, CDT, auto-negotiation and
+  802.3az EEE.
+
+properties:
+  compatible:
+    const: ethernet-phy-id004d.d0c0
+
+  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:
+          GE PHY MISC reset which triggers a reset across MDC, DSP, RX, and TX lines.
+
+  qcom,dac-preset-short-cable:
+    description: |
+      Set if this phy is connected to another phy to adjust the values for
+      MDAC and EDAC to adjust amplitude, bias current settings, and error
+      detection and correction algorithm to accommodate for short cable length.
+      If not set, DAC values are not modified and it is assumed the MDI output pins
+      of this PHY are directly connected to an RJ45 connector.
+    type: boolean
+
+required:
+  - compatible
+  - reg
+  - clocks
+  - clock-names
+  - resets
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/clock/qcom,gcc-ipq5018.h>
+    #include <dt-bindings/reset/qcom,gcc-ipq5018.h>
+
+    mdio {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        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 2/3] 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")
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 3/3] net: phy: qca,at803x: add RX and TX clock management for IPQ5018 PHY

[George Moussalem via B4 Relay]

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

Acquire and enable the RX and TX clocks for the IPQ5018 PHY. These
clocks are required for the PHY's datapath to function correctly.
In addition, 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 | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

diff --git a/drivers/net/phy/qcom/at803x.c b/drivers/net/phy/qcom/at803x.c
index 63726cf98cd4..b7361a14220d 100644
--- a/drivers/net/phy/qcom/at803x.c
+++ b/drivers/net/phy/qcom/at803x.c
@@ -19,6 +19,7 @@
 #include <linux/regulator/consumer.h>
 #include <linux/of.h>
 #include <linux/phylink.h>
+#include <linux/clk.h>
 #include <linux/reset.h>
 #include <linux/phy_port.h>
 #include <dt-bindings/net/qca-ar803x.h>
@@ -176,6 +177,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 +1065,16 @@ 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;
+
+	if (phydev->link) {
+		clk_enable(priv->rx_clk);
+		clk_enable(priv->tx_clk);
+	} else {
+		clk_disable(priv->rx_clk);
+		clk_disable(priv->tx_clk);
+	}
+
 	/*
 	 * 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 +1097,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_enabled(dev, "rx");
+	if (IS_ERR(priv->rx_clk))
+		return dev_err_probe(dev, PTR_ERR(priv->rx_clk),
+				     "failed to get and enable RX clock\n");
+
+	priv->tx_clk = devm_clk_get_enabled(dev, "tx");
+	if (IS_ERR(priv->tx_clk))
+		return dev_err_probe(dev, PTR_ERR(priv->tx_clk),
+				     "failed to get and enable 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 1/3] dt-bindings: net: qca,ipq5018: Split IPQ5018 PHY bindings from ar803x

[Rob Herring (Arm)]

On Mon, 01 Jun 2026 17:19:08 +0400, George Moussalem wrote:
> Move the definition for the Qualcomm IPQ5018 Internal PHY out of the
> qca,ar803x.yaml binding file and isolate them in a dedicated
> qca,ipq5018.yaml file. This is necessary due to the restriction of max
> one clock in ethernet-phy.yaml which ar803x references as further
> testing revealed that the RX and TX clocks of the IPQ5018 PHY need to
> be explicitly enabled.
> 
> Signed-off-by: George Moussalem <george.moussalem@outlook.com>
> ---
>  .../devicetree/bindings/net/qca,ar803x.yaml        | 43 -------------
>  .../devicetree/bindings/net/qca,ipq5018.yaml       | 75 ++++++++++++++++++++++
>  2 files changed, 75 insertions(+), 43 deletions(-)
> 

My bot found errors running 'make dt_binding_check' on your patch:

yamllint warnings/errors:

dtschema/dtc warnings/errors:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/net/qca,ipq5018.example.dtb: ethernet-phy@7 (ethernet-phy-id004d.d0c0): clocks: [[4294967295, 36], [4294967295, 37]] is too long
	from schema $id: http://devicetree.org/schemas/net/ethernet-phy.yaml

doc reference errors (make refcheckdocs):

See https://patchwork.kernel.org/project/devicetree/patch/20260601-ipq5018-gephy-clocks-v1-1-2df8287712c3@outlook.com

The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.

Re: [PATCH 1/3] dt-bindings: net: qca,ipq5018: Split IPQ5018 PHY bindings from ar803x

[Rob Herring]

On Mon, Jun 01, 2026 at 12:45:25PM -0500, Rob Herring (Arm) wrote:
> 
> On Mon, 01 Jun 2026 17:19:08 +0400, George Moussalem wrote:
> > Move the definition for the Qualcomm IPQ5018 Internal PHY out of the
> > qca,ar803x.yaml binding file and isolate them in a dedicated
> > qca,ipq5018.yaml file. This is necessary due to the restriction of max
> > one clock in ethernet-phy.yaml which ar803x references as further
> > testing revealed that the RX and TX clocks of the IPQ5018 PHY need to
> > be explicitly enabled.

Just splitting does not solve this problem as you can see with the 
error. You will need to drop this constraint from ethernet-phy.yaml and 
add it (1 clock) to every specific phy binding (that uses 'clocks').

> > 
> > Signed-off-by: George Moussalem <george.moussalem@outlook.com>
> > ---
> >  .../devicetree/bindings/net/qca,ar803x.yaml        | 43 -------------
> >  .../devicetree/bindings/net/qca,ipq5018.yaml       | 75 ++++++++++++++++++++++
> >  2 files changed, 75 insertions(+), 43 deletions(-)
> > 
> 
> My bot found errors running 'make dt_binding_check' on your patch:
> 
> yamllint warnings/errors:
> 
> dtschema/dtc warnings/errors:
> /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/net/qca,ipq5018.example.dtb: ethernet-phy@7 (ethernet-phy-id004d.d0c0): clocks: [[4294967295, 36], [4294967295, 37]] is too long
> 	from schema $id: http://devicetree.org/schemas/net/ethernet-phy.yaml
> 
> doc reference errors (make refcheckdocs):
> 
> See https://patchwork.kernel.org/project/devicetree/patch/20260601-ipq5018-gephy-clocks-v1-1-2df8287712c3@outlook.com
> 
> The base for the series is generally the latest rc1. A different dependency
> should be noted in *this* patch.
> 
> If you already ran 'make dt_binding_check' and didn't see the above
> error(s), then make sure 'yamllint' is installed and dt-schema is up to
> date:
> 
> pip3 install dtschema --upgrade
> 
> Please check and re-submit after running the above command yourself. Note
> that DT_SCHEMA_FILES can be set to your schema file to speed up checking
> your schema. However, it must be unset to test all examples with your schema.
> 
> 

Re: [PATCH 1/3] dt-bindings: net: qca,ipq5018: Split IPQ5018 PHY bindings from ar803x

[George Moussalem]

On 6/2/26 00:49, Rob Herring wrote:
> On Mon, Jun 01, 2026 at 12:45:25PM -0500, Rob Herring (Arm) wrote:
>>
>> On Mon, 01 Jun 2026 17:19:08 +0400, George Moussalem wrote:
>>> Move the definition for the Qualcomm IPQ5018 Internal PHY out of the
>>> qca,ar803x.yaml binding file and isolate them in a dedicated
>>> qca,ipq5018.yaml file. This is necessary due to the restriction of max
>>> one clock in ethernet-phy.yaml which ar803x references as further
>>> testing revealed that the RX and TX clocks of the IPQ5018 PHY need to
>>> be explicitly enabled.
> 
> Just splitting does not solve this problem as you can see with the 
> error. You will need to drop this constraint from ethernet-phy.yaml and 
> add it (1 clock) to every specific phy binding (that uses 'clocks').

Thanks for the advise, Rob. I've made changes accordingly and sent out
v2:
https://lore.kernel.org/netdev/20260602-ipq5018-gephy-clocks-v2-0-65a1f1d881f3@outlook.com/T/#t

Mind you that you'll likely still see an error coming from running 'make
dt_binding_check':

  DTC [C] Documentation/devicetree/bindings/net/renesas,ether.example.dtb
/home/george/src/linux-next/out/Documentation/devicetree/bindings/net/renesas,ether.example.dtb:
ethernet-phy@1 (ethernet-phy-id0022.1537): compatible:
['ethernet-phy-id0022.1537', 'ethernet-phy-ieee802.3-c22'] is too long
	from schema $id: http://devicetree.org/schemas/net/micrel.yaml

This was already there and I checked against master.

> 
>>>
>>> Signed-off-by: George Moussalem <george.moussalem@outlook.com>
>>> ---
>>>  .../devicetree/bindings/net/qca,ar803x.yaml        | 43 -------------
>>>  .../devicetree/bindings/net/qca,ipq5018.yaml       | 75 ++++++++++++++++++++++
>>>  2 files changed, 75 insertions(+), 43 deletions(-)
>>>
>>
>> My bot found errors running 'make dt_binding_check' on your patch:
>>
>> yamllint warnings/errors:
>>
>> dtschema/dtc warnings/errors:
>> /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/net/qca,ipq5018.example.dtb: ethernet-phy@7 (ethernet-phy-id004d.d0c0): clocks: [[4294967295, 36], [4294967295, 37]] is too long
>> 	from schema $id: http://devicetree.org/schemas/net/ethernet-phy.yaml
>>
>> doc reference errors (make refcheckdocs):
>>
>> See https://patchwork.kernel.org/project/devicetree/patch/20260601-ipq5018-gephy-clocks-v1-1-2df8287712c3@outlook.com
>>
>> The base for the series is generally the latest rc1. A different dependency
>> should be noted in *this* patch.
>>
>> If you already ran 'make dt_binding_check' and didn't see the above
>> error(s), then make sure 'yamllint' is installed and dt-schema is up to
>> date:
>>
>> pip3 install dtschema --upgrade
>>
>> Please check and re-submit after running the above command yourself. Note
>> that DT_SCHEMA_FILES can be set to your schema file to speed up checking
>> your schema. However, it must be unset to test all examples with your schema.
>>
>>

Best regards,
George

Re: [PATCH 1/3] dt-bindings: net: qca,ipq5018: Split IPQ5018 PHY bindings from ar803x

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Schema Validation Bypass Failure and Binding Incompleteness
- [High] Broken Schema Validation for AR803x PHYs due to Removed Selection Criteria
--

commit 48327af0b67eba3af6e8fa934ae07a6c6cb94d54
Author: George Moussalem <george.moussalem@outlook.com>

dt-bindings: net: qca,ipq5018: Split IPQ5018 PHY bindings from ar803x

This commit moves the Qualcomm IPQ5018 Internal PHY definition into a dedicated
file. It isolates the binding from qca,ar803x.yaml in an attempt to support
both RX and TX clocks, as the core ethernet-phy.yaml restricts clocks to one.

> diff --git a/Documentation/devicetree/bindings/net/qca,ar803x.yaml b/Documentation/devicetree/bindings/net/qca,ar803x.yaml
> index 7ae5110e7aa2c..3acd09f0da863 100644
> --- a/Documentation/devicetree/bindings/net/qca,ar803x.yaml
> +++ b/Documentation/devicetree/bindings/net/qca,ar803x.yaml

[ ... ]

>  properties:
> -  compatible:
> -    enum:
> -      - ethernet-phy-id004d.d0c0
> -
>    qca,clk-out-frequency:
>      description: Clock output frequency in Hertz.
>      $ref: /schemas/types.yaml#/definitions/uint32

[Severity: High]
With the compatible property removed, how will dt-schema match this binding
to device nodes?

Because device tree schema validation relies on the compatible property
to automatically generate selection conditions, removing it leaves the schema
with no way to match device nodes.

This causes qca,ar803x.yaml to be silently skipped during validation,
breaking validation for properties like qca,clk-out-frequency for all
downstream AR803x PHY configurations.

[ ... ]

> diff --git a/Documentation/devicetree/bindings/net/qca,ipq5018.yaml b/Documentation/devicetree/bindings/net/qca,ipq5018.yaml
> new file mode 100644
> index 0000000000000..7cdc96f6d389f
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/qca,ipq5018.yaml

[ ... ]

> +properties:
> +  compatible:
> +    const: ethernet-phy-id004d.d0c0
> +
> +  reg:
> +    const: 7  # This PHY is always at MDIO address 7 in the IPQ5018 SoC
> +
> +  clocks:
> +    items:
> +      - description: RX clock
> +      - description: TX clock

[Severity: High]
The commit message mentions splitting this file to bypass the 1-clock
restriction in ethernet-phy.yaml, but does this actually prevent the core
schema from applying?

Since the device tree node is named ethernet-phy@7 (as seen in the examples),
dt-schema will still automatically apply the core ethernet-phy.yaml schema
due to its select logic on the node name. Validation will therefore still
fail for having two clocks.

[ ... ]

> +required:
> +  - compatible
> +  - reg
> +  - clocks
> +  - clock-names
> +  - resets
> +
> +additionalProperties: false

[Severity: High]
Does this block standard Ethernet PHY properties?

Because additionalProperties is set to false without a reference to the
core ethernet-phy.yaml schema, any standard PHY properties (such as
max-speed or interrupts) added to a device tree will be incorrectly
rejected.

The standard approach is to modify ethernet-phy.yaml to allow multiple
clocks, include the $ref to it here, and use unevaluatedProperties: false.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260601-ipq5018-gephy-clocks-v1-0-2df8287712c3@outlook.com?part=1

Re: [PATCH 2/3] arm64: qcom: ipq5018: Add GEPHY RX and TX clocks

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Critical] DeviceTree ABI breakage due to requiring newly introduced clock properties on an existing PHY.
- [High] Unbalanced clock enable/disable calls in link state change notifications lead to enable count underflows and kernel warnings.
--

commit 6e2b942b1bf7f7a0247513729082526ff6e92c6c
Author: George Moussalem <george.moussalem@outlook.com>

arm64: qcom: ipq5018: Add GEPHY RX and TX clocks

This commit adds rx and tx clocks for the IPQ5018 GEPHY in the device tree
to enable the datapath. It specifies the clock handles and their corresponding
names for the ethernet phy node.

> diff --git a/arch/arm64/boot/dts/qcom/ipq5018.dtsi b/arch/arm64/boot/dts/qcom/ipq5018.dtsi
> index 6f8004a22a1ff..60c27a6f2b10c 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";

[Severity: Critical]
Can requiring these clock properties cause a device tree backward compatibility
regression?

Looking at ipq5018_probe() in drivers/net/phy/qcom/at803x.c:

	priv->rx_clk = devm_clk_get_enabled(dev, "rx");
	if (IS_ERR(priv->rx_clk))
		return dev_err_probe(dev, PTR_ERR(priv->rx_clk),
				     "failed to get and enable RX clock\n");

The driver uses devm_clk_get_enabled() to acquire the new rx and tx clocks,
meaning these properties are strictly mandatory for the probe to succeed.

Since this phy was already supported in the kernel, existing device trees
do not contain these newly introduced clock properties.

When booting the new kernel with an older device tree, devm_clk_get_enabled()
will return -ENOENT, causing ipq5018_probe() to abort.

Should the driver gracefully handle missing clocks on older device trees by
using devm_clk_get_optional_enabled()?

[Severity: High]
Will gating these clocks based on link state result in an enable count underflow?

Looking at ipq5018_link_change_notify() in drivers/net/phy/qcom/at803x.c:

	if (phydev->link) {
		clk_enable(priv->rx_clk);
		clk_enable(priv->tx_clk);
	} else {
		clk_disable(priv->rx_clk);
		clk_disable(priv->tx_clk);
	}

Since phydev->link can be false in multiple consecutive states (like PHY_NOLINK
and PHY_HALTED), won't clk_disable() be called multiple times in a row without a
matching clk_enable(), causing the clock enable count to underflow and triggering
a kernel warning?

Also, transitioning to link up increments the count to 2. If the device is
unbound while the link is up, devres cleanup only reduces the count by 1.

Could this lead to a clock reference leak?

>  				resets = <&gcc GCC_GEPHY_MISC_ARES>;
>  			};
>  		};

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260601-ipq5018-gephy-clocks-v1-0-2df8287712c3@outlook.com?part=2

Re: [PATCH 3/3] net: phy: qca,at803x: add RX and TX clock management for IPQ5018 PHY

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Unbalanced clock reference counting causing CCF underflow warnings
--

commit 05a5fab6d0a0c9ba1c5e411c7e94bc9ff8040850
Author: George Moussalem <george.moussalem@outlook.com>

net: phy: qca,at803x: add RX and TX clock management for IPQ5018 PHY

This patch acquires and enables the RX and TX clocks for the IPQ5018
PHY datapath. It also gates these clocks upon link state changes to
improve power management.

> diff --git a/drivers/net/phy/qcom/at803x.c b/drivers/net/phy/qcom/at803x.c
> index 63726cf98cd42..b7361a14220d8 100644
> --- a/drivers/net/phy/qcom/at803x.c
> +++ b/drivers/net/phy/qcom/at803x.c

[ ... ]

> @@ -1062,6 +1065,16 @@ 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;
> +
> +	if (phydev->link) {
> +		clk_enable(priv->rx_clk);
> +		clk_enable(priv->tx_clk);
> +	} else {
> +		clk_disable(priv->rx_clk);
> +		clk_disable(priv->tx_clk);
> +	}
> +

[Severity: High]
Will this lead to unbalanced clock reference counts and Common Clock
Framework (CCF) underflow warnings?

Because ipq5018_link_change_notify() is invoked on every PHY state
transition, it can be called multiple times consecutively while
phydev->link remains false (for example, transitioning from PHY_NOLINK
to PHY_HALTED). This would result in multiple calls to clk_disable()
without corresponding clk_enable() calls.

>  	/*
>  	 * 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 +1097,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_enabled(dev, "rx");
> +	if (IS_ERR(priv->rx_clk))
> +		return dev_err_probe(dev, PTR_ERR(priv->rx_clk),
> +				     "failed to get and enable RX clock\n");
> +

[Severity: High]
If the driver unbinds while the link is down, won't the unconditional
cleanup registered by devm_clk_get_enabled() trigger another CCF
underflow warning?

The initial devm_clk_get_enabled() sets the enable count to 1. If the
link goes down, the count drops to 0. When devres cleans up on unbind,
it will attempt to disable the clock again. Should state tracking be
added, or should this use devm_clk_get() instead if the clock state
is going to be managed dynamically?

> +	priv->tx_clk = devm_clk_get_enabled(dev, "tx");
> +	if (IS_ERR(priv->tx_clk))
> +		return dev_err_probe(dev, PTR_ERR(priv->tx_clk),
> +				     "failed to get and enable TX clock\n");
> +

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260601-ipq5018-gephy-clocks-v1-0-2df8287712c3@outlook.com?part=3