[PATCH V2 0/2] net: phy: dp83822: Add optional external PHY clock

[PATCH V2 0/2] net: phy: dp83822: Add optional external PHY clock

[Stefan Wahren]

This small series implement support for external PHY clock for the
dp83822 driver.

Changes in V2:
- Make readability changes a separate patch as suggested by Andrew
- Make clk a local pointer as suggested by Andrew

Stefan Wahren (2):
  net: phy: dp83822: Improve readability in dp8382x_probe
  net: phy: dp83822: Add optional external PHY clock

 drivers/net/phy/dp83822.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

-- 
2.43.0

[PATCH V2 1/2] net: phy: dp83822: Improve readability in dp8382x_probe

[Stefan Wahren]

Introduce a local pointer for device so devm_kzalloc() fit into
a single line. Also this makes following changes easier to read.

Signed-off-by: Stefan Wahren <wahrenst@gmx.net>
---
 drivers/net/phy/dp83822.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/phy/dp83822.c b/drivers/net/phy/dp83822.c
index c012dfab3171..d8c5b5cd1bc0 100644
--- a/drivers/net/phy/dp83822.c
+++ b/drivers/net/phy/dp83822.c
@@ -984,10 +984,10 @@ static int dp83822_attach_mdi_port(struct phy_device *phydev,
 
 static int dp8382x_probe(struct phy_device *phydev)
 {
+	struct device *dev = &phydev->mdio.dev;
 	struct dp83822_private *dp83822;
 
-	dp83822 = devm_kzalloc(&phydev->mdio.dev, sizeof(*dp83822),
-			       GFP_KERNEL);
+	dp83822 = devm_kzalloc(dev, sizeof(*dp83822), GFP_KERNEL);
 	if (!dp83822)
 		return -ENOMEM;
 
-- 
2.43.0

[PATCH V2 2/2] net: phy: dp83822: Add optional external PHY clock

[Stefan Wahren]

In some cases, the PHY can use an external ref clock source instead of a
crystal.

Add an optional clock in the PHY node to make sure that the clock source
is enabled, if specified, before probing.

Signed-off-by: Stefan Wahren <wahrenst@gmx.net>
---
 drivers/net/phy/dp83822.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/net/phy/dp83822.c b/drivers/net/phy/dp83822.c
index d8c5b5cd1bc0..6fc86be9d593 100644
--- a/drivers/net/phy/dp83822.c
+++ b/drivers/net/phy/dp83822.c
@@ -4,6 +4,7 @@
  * Copyright (C) 2017 Texas Instruments Inc.
  */
 
+#include <linux/clk.h>
 #include <linux/ethtool.h>
 #include <linux/etherdevice.h>
 #include <linux/kernel.h>
@@ -986,11 +987,18 @@ static int dp8382x_probe(struct phy_device *phydev)
 {
 	struct device *dev = &phydev->mdio.dev;
 	struct dp83822_private *dp83822;
+	struct clk *clk;
 
 	dp83822 = devm_kzalloc(dev, sizeof(*dp83822), GFP_KERNEL);
 	if (!dp83822)
 		return -ENOMEM;
 
+	clk = devm_clk_get_optional_enabled(dev, NULL);
+	if (IS_ERR(clk)) {
+		return dev_err_probe(dev, PTR_ERR(clk),
+				     "Failed to request ref clock\n");
+	}
+
 	dp83822->tx_amplitude_100base_tx_index = -1;
 	dp83822->mac_termination_index = -1;
 	phydev->priv = dp83822;
-- 
2.43.0

Re: [PATCH V2 1/2] net: phy: dp83822: Improve readability in dp8382x_probe

[Andrew Lunn]

On Thu, May 28, 2026 at 08:46:41PM +0200, Stefan Wahren wrote:
> Introduce a local pointer for device so devm_kzalloc() fit into
> a single line. Also this makes following changes easier to read.
> 
> Signed-off-by: Stefan Wahren <wahrenst@gmx.net>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

Re: [PATCH V2 2/2] net: phy: dp83822: Add optional external PHY clock

[Andrew Lunn]

On Thu, May 28, 2026 at 08:46:42PM +0200, Stefan Wahren wrote:
> In some cases, the PHY can use an external ref clock source instead of a
> crystal.
> 
> Add an optional clock in the PHY node to make sure that the clock source
> is enabled, if specified, before probing.
> 
> Signed-off-by: Stefan Wahren <wahrenst@gmx.net>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

Re: [PATCH V2 2/2] net: phy: dp83822: Add optional external PHY clock

[Jakub Kicinski]

On Thu, 28 May 2026 20:46:42 +0200 Stefan Wahren wrote:
> In some cases, the PHY can use an external ref clock source instead of a
> crystal.
> 
> Add an optional clock in the PHY node to make sure that the clock source
> is enabled, if specified, before probing.


> diff --git a/drivers/net/phy/dp83822.c b/drivers/net/phy/dp83822.c
> index d8c5b5cd1bc0..6fc86be9d593 100644
> --- a/drivers/net/phy/dp83822.c
> +++ b/drivers/net/phy/dp83822.c
> @@ -4,6 +4,7 @@
>   * Copyright (C) 2017 Texas Instruments Inc.
>   */
>  
> +#include <linux/clk.h>
>  #include <linux/ethtool.h>
>  #include <linux/etherdevice.h>
>  #include <linux/kernel.h>
> @@ -986,11 +987,18 @@ static int dp8382x_probe(struct phy_device *phydev)
>  {
>  	struct device *dev = &phydev->mdio.dev;
>  	struct dp83822_private *dp83822;
> +	struct clk *clk;
>  
>  	dp83822 = devm_kzalloc(dev, sizeof(*dp83822), GFP_KERNEL);
>  	if (!dp83822)
>  		return -ENOMEM;
>  
> +	clk = devm_clk_get_optional_enabled(dev, NULL);
> +	if (IS_ERR(clk)) {
> +		return dev_err_probe(dev, PTR_ERR(clk),
> +				     "Failed to request ref clock\n");
> +	}

nit: unnecessary parenthesis

The AI says:

  Does this initialization sequence violate the DP83822 power-on
  requirements? The PHY framework deasserts the hardware reset line before
  invoking the probe callback. By enabling the external clock here, 
  the clock starts after the hardware reset is already deasserted.

  The datasheet requires the reset signal to be asserted for at least 
  50 ms after power and clocks are stable. Without performing a subsequent
  hardware reset here, could the PHY be left in an undefined state or
  lead to initialization failures?

Did it really read the datasheet or is this a hallucination?

FWIW it also says:

  Should this clock pointer be saved in the driver's private data
  structure (struct dp83822_private) instead of a local variable?

  Without storing it, the dp83822_suspend callback cannot disable the clock
  when Wake-on-LAN is disabled, which could prevent the clock provider from
  entering low-power states.