Netdev List
 help / color / mirror / Atom feed
* [PATCH v3 0/4] IPQ5018: Add and enable GEPHY RX and TX clocks
@ 2026-06-05 12:41 George Moussalem via B4 Relay
  2026-06-05 12:41 ` [PATCH v3 1/4] dt-bindings: net: ethernet-phy: increase max clock count to two George Moussalem via B4 Relay
                   ` (3 more replies)
  0 siblings, 4 replies; 10+ messages in thread
From: George Moussalem via B4 Relay @ 2026-06-05 12:41 UTC (permalink / raw)
  To: Andrew Lunn, Heiner Kallweit, Russell King, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Florian Fainelli,
	Bjorn Andersson, Konrad Dybcio
  Cc: netdev, devicetree, linux-kernel, Konrad Dybcio, linux-arm-msm,
	George Moussalem, Conor Dooley, Dmitry Baryshkov

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>



^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v3 1/4] dt-bindings: net: ethernet-phy: increase max clock count to two
  2026-06-05 12:41 [PATCH v3 0/4] IPQ5018: Add and enable GEPHY RX and TX clocks George Moussalem via B4 Relay
@ 2026-06-05 12:41 ` George Moussalem via B4 Relay
  2026-06-05 15:59   ` Rob Herring
  2026-06-05 12:41 ` [PATCH v3 2/4] dt-bindings: net: qca,ar803x: Add clocks for IPQ5018 PHY George Moussalem via B4 Relay
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 10+ messages in thread
From: George Moussalem via B4 Relay @ 2026-06-05 12:41 UTC (permalink / raw)
  To: Andrew Lunn, Heiner Kallweit, Russell King, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Florian Fainelli,
	Bjorn Andersson, Konrad Dybcio
  Cc: netdev, devicetree, linux-kernel, Konrad Dybcio, linux-arm-msm,
	George Moussalem

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



^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v3 2/4] dt-bindings: net: qca,ar803x: Add clocks for IPQ5018 PHY
  2026-06-05 12:41 [PATCH v3 0/4] IPQ5018: Add and enable GEPHY RX and TX clocks George Moussalem via B4 Relay
  2026-06-05 12:41 ` [PATCH v3 1/4] dt-bindings: net: ethernet-phy: increase max clock count to two George Moussalem via B4 Relay
@ 2026-06-05 12:41 ` George Moussalem via B4 Relay
  2026-06-05 12:41 ` [PATCH v3 3/4] arm64: qcom: ipq5018: Add GEPHY RX and TX clocks George Moussalem via B4 Relay
  2026-06-05 12:41 ` [PATCH v3 4/4] net: phy: at803x: add RX and TX clock management for IPQ5018 PHY George Moussalem via B4 Relay
  3 siblings, 0 replies; 10+ messages in thread
From: George Moussalem via B4 Relay @ 2026-06-05 12:41 UTC (permalink / raw)
  To: Andrew Lunn, Heiner Kallweit, Russell King, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Florian Fainelli,
	Bjorn Andersson, Konrad Dybcio
  Cc: netdev, devicetree, linux-kernel, Konrad Dybcio, linux-arm-msm,
	George Moussalem, Conor Dooley

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



^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v3 3/4] arm64: qcom: ipq5018: Add GEPHY RX and TX clocks
  2026-06-05 12:41 [PATCH v3 0/4] IPQ5018: Add and enable GEPHY RX and TX clocks George Moussalem via B4 Relay
  2026-06-05 12:41 ` [PATCH v3 1/4] dt-bindings: net: ethernet-phy: increase max clock count to two George Moussalem via B4 Relay
  2026-06-05 12:41 ` [PATCH v3 2/4] dt-bindings: net: qca,ar803x: Add clocks for IPQ5018 PHY George Moussalem via B4 Relay
@ 2026-06-05 12:41 ` George Moussalem via B4 Relay
  2026-06-05 12:41 ` [PATCH v3 4/4] net: phy: at803x: add RX and TX clock management for IPQ5018 PHY George Moussalem via B4 Relay
  3 siblings, 0 replies; 10+ messages in thread
From: George Moussalem via B4 Relay @ 2026-06-05 12:41 UTC (permalink / raw)
  To: Andrew Lunn, Heiner Kallweit, Russell King, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Florian Fainelli,
	Bjorn Andersson, Konrad Dybcio
  Cc: netdev, devicetree, linux-kernel, Konrad Dybcio, linux-arm-msm,
	George Moussalem, Dmitry Baryshkov

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



^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v3 4/4] net: phy: at803x: add RX and TX clock management for IPQ5018 PHY
  2026-06-05 12:41 [PATCH v3 0/4] IPQ5018: Add and enable GEPHY RX and TX clocks George Moussalem via B4 Relay
                   ` (2 preceding siblings ...)
  2026-06-05 12:41 ` [PATCH v3 3/4] arm64: qcom: ipq5018: Add GEPHY RX and TX clocks George Moussalem via B4 Relay
@ 2026-06-05 12:41 ` George Moussalem via B4 Relay
  2026-06-05 14:23   ` Andrew Lunn
  3 siblings, 1 reply; 10+ messages in thread
From: George Moussalem via B4 Relay @ 2026-06-05 12:41 UTC (permalink / raw)
  To: Andrew Lunn, Heiner Kallweit, Russell King, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Florian Fainelli,
	Bjorn Andersson, Konrad Dybcio
  Cc: netdev, devicetree, linux-kernel, Konrad Dybcio, linux-arm-msm,
	George Moussalem

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



^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH v3 4/4] net: phy: at803x: add RX and TX clock management for IPQ5018 PHY
  2026-06-05 12:41 ` [PATCH v3 4/4] net: phy: at803x: add RX and TX clock management for IPQ5018 PHY George Moussalem via B4 Relay
@ 2026-06-05 14:23   ` Andrew Lunn
  2026-06-05 14:45     ` George Moussalem
  0 siblings, 1 reply; 10+ messages in thread
From: Andrew Lunn @ 2026-06-05 14:23 UTC (permalink / raw)
  To: george.moussalem
  Cc: Heiner Kallweit, Russell King, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Florian Fainelli, Bjorn Andersson, Konrad Dybcio,
	netdev, devicetree, linux-kernel, Konrad Dybcio, linux-arm-msm

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v3 4/4] net: phy: at803x: add RX and TX clock management for IPQ5018 PHY
  2026-06-05 14:23   ` Andrew Lunn
@ 2026-06-05 14:45     ` George Moussalem
  2026-06-05 15:01       ` Andrew Lunn
  0 siblings, 1 reply; 10+ messages in thread
From: George Moussalem @ 2026-06-05 14:45 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: Heiner Kallweit, Russell King, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Florian Fainelli, Bjorn Andersson, Konrad Dybcio,
	netdev, devicetree, linux-kernel, Konrad Dybcio, linux-arm-msm

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v3 4/4] net: phy: at803x: add RX and TX clock management for IPQ5018 PHY
  2026-06-05 14:45     ` George Moussalem
@ 2026-06-05 15:01       ` Andrew Lunn
  2026-06-05 16:51         ` George Moussalem
  0 siblings, 1 reply; 10+ messages in thread
From: Andrew Lunn @ 2026-06-05 15:01 UTC (permalink / raw)
  To: George Moussalem
  Cc: Heiner Kallweit, Russell King, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Florian Fainelli, Bjorn Andersson, Konrad Dybcio,
	netdev, devicetree, linux-kernel, Konrad Dybcio, linux-arm-msm

> 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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v3 1/4] dt-bindings: net: ethernet-phy: increase max clock count to two
  2026-06-05 12:41 ` [PATCH v3 1/4] dt-bindings: net: ethernet-phy: increase max clock count to two George Moussalem via B4 Relay
@ 2026-06-05 15:59   ` Rob Herring
  0 siblings, 0 replies; 10+ messages in thread
From: Rob Herring @ 2026-06-05 15:59 UTC (permalink / raw)
  To: George Moussalem
  Cc: Andrew Lunn, Heiner Kallweit, Russell King, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, Krzysztof Kozlowski,
	Conor Dooley, Florian Fainelli, Bjorn Andersson, Konrad Dybcio,
	netdev, devicetree, linux-kernel, Konrad Dybcio, linux-arm-msm

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>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v3 4/4] net: phy: at803x: add RX and TX clock management for IPQ5018 PHY
  2026-06-05 15:01       ` Andrew Lunn
@ 2026-06-05 16:51         ` George Moussalem
  0 siblings, 0 replies; 10+ messages in thread
From: George Moussalem @ 2026-06-05 16:51 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: Heiner Kallweit, Russell King, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Florian Fainelli, Bjorn Andersson, Konrad Dybcio,
	netdev, devicetree, linux-kernel, Konrad Dybcio, linux-arm-msm

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2026-06-05 16:51 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-06-05 12:41 [PATCH v3 0/4] IPQ5018: Add and enable GEPHY RX and TX clocks George Moussalem via B4 Relay
2026-06-05 12:41 ` [PATCH v3 1/4] dt-bindings: net: ethernet-phy: increase max clock count to two George Moussalem via B4 Relay
2026-06-05 15:59   ` Rob Herring
2026-06-05 12:41 ` [PATCH v3 2/4] dt-bindings: net: qca,ar803x: Add clocks for IPQ5018 PHY George Moussalem via B4 Relay
2026-06-05 12:41 ` [PATCH v3 3/4] arm64: qcom: ipq5018: Add GEPHY RX and TX clocks George Moussalem via B4 Relay
2026-06-05 12:41 ` [PATCH v3 4/4] net: phy: at803x: add RX and TX clock management for IPQ5018 PHY George Moussalem via B4 Relay
2026-06-05 14:23   ` Andrew Lunn
2026-06-05 14:45     ` George Moussalem
2026-06-05 15:01       ` Andrew Lunn
2026-06-05 16:51         ` George Moussalem

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox