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

[Jakub Kicinski <kuba@kernel.org>]

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.