[PATCH] phy: zynqmp: register PHY provider after probe initialization

[PATCH] phy: zynqmp: register PHY provider after probe initialization

[Radhey Shyam Pandey]

Register the OF PHY provider only after saved_regs allocation and
runtime PM setup complete successfully. Publishing the provider
mid-probe allowed concurrent consumers to obtain a phy via xpsgtr_xlate()
while probe could still fail, leaving them with references into driver
state that devres would free on the error path.

Reported-by: Sashiko <sashiko-bot@kernel.org>
Link: https://lore.kernel.org/all/20260720155732.9DA581F000E9@smtp.kernel.org
Fixes: b3db66f62446 ("phy: xilinx: add runtime PM support")
Fixes: 5af9b304bc60 ("phy: xilinx: phy-zynqmp: Fix SGMII linkup failure on resume")
Signed-off-by: Radhey Shyam Pandey <radhey.shyam.pandey@amd.com>
---
 drivers/phy/xilinx/phy-zynqmp.c | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/drivers/phy/xilinx/phy-zynqmp.c b/drivers/phy/xilinx/phy-zynqmp.c
index 240626b55475..8930cbe24b5a 100644
--- a/drivers/phy/xilinx/phy-zynqmp.c
+++ b/drivers/phy/xilinx/phy-zynqmp.c
@@ -1060,13 +1060,6 @@ static int xpsgtr_probe(struct platform_device *pdev)
 					    xpsgtr_status_read);
 	}
 
-	/* Register the PHY provider. */
-	provider = devm_of_phy_provider_register(&pdev->dev, xpsgtr_xlate);
-	if (IS_ERR(provider)) {
-		dev_err(&pdev->dev, "registering provider failed\n");
-		return PTR_ERR(provider);
-	}
-
 	gtr_dev->saved_regs = devm_kmalloc(gtr_dev->dev,
 					   sizeof(save_reg_address),
 					   GFP_KERNEL);
@@ -1082,6 +1075,14 @@ static int xpsgtr_probe(struct platform_device *pdev)
 		return ret;
 	}
 
+	provider = devm_of_phy_provider_register(&pdev->dev, xpsgtr_xlate);
+	if (IS_ERR(provider)) {
+		dev_err(&pdev->dev, "registering provider failed\n");
+		pm_runtime_put(gtr_dev->dev);
+		pm_runtime_disable(gtr_dev->dev);
+		return PTR_ERR(provider);
+	}
+
 	return 0;
 }
 

base-commit: ea2bff00da89d7767d677bb68470130ba96f4928
-- 
2.44.4

Re: [PATCH] phy: zynqmp: register PHY provider after probe initialization

[Vinod Koul]

On 07-08-26, 21:55, Radhey Shyam Pandey wrote:
> Register the OF PHY provider only after saved_regs allocation and
> runtime PM setup complete successfully. Publishing the provider
> mid-probe allowed concurrent consumers to obtain a phy via xpsgtr_xlate()
> while probe could still fail, leaving them with references into driver
> state that devres would free on the error path.

That fails for me!

> 
> Reported-by: Sashiko <sashiko-bot@kernel.org>
> Link: https://lore.kernel.org/all/20260720155732.9DA581F000E9@smtp.kernel.org
> Fixes: b3db66f62446 ("phy: xilinx: add runtime PM support")
> Fixes: 5af9b304bc60 ("phy: xilinx: phy-zynqmp: Fix SGMII linkup failure on resume")
> Signed-off-by: Radhey Shyam Pandey <radhey.shyam.pandey@amd.com>
> ---
>  drivers/phy/xilinx/phy-zynqmp.c | 15 ++++++++-------
>  1 file changed, 8 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/phy/xilinx/phy-zynqmp.c b/drivers/phy/xilinx/phy-zynqmp.c
> index 240626b55475..8930cbe24b5a 100644
> --- a/drivers/phy/xilinx/phy-zynqmp.c
> +++ b/drivers/phy/xilinx/phy-zynqmp.c
> @@ -1060,13 +1060,6 @@ static int xpsgtr_probe(struct platform_device *pdev)
>  					    xpsgtr_status_read);
>  	}
>  
> -	/* Register the PHY provider. */
> -	provider = devm_of_phy_provider_register(&pdev->dev, xpsgtr_xlate);
> -	if (IS_ERR(provider)) {
> -		dev_err(&pdev->dev, "registering provider failed\n");
> -		return PTR_ERR(provider);
> -	}
> -
>  	gtr_dev->saved_regs = devm_kmalloc(gtr_dev->dev,
>  					   sizeof(save_reg_address),
>  					   GFP_KERNEL);
> @@ -1082,6 +1075,14 @@ static int xpsgtr_probe(struct platform_device *pdev)
>  		return ret;
>  	}
>  
> +	provider = devm_of_phy_provider_register(&pdev->dev, xpsgtr_xlate);
> +	if (IS_ERR(provider)) {
> +		dev_err(&pdev->dev, "registering provider failed\n");
> +		pm_runtime_put(gtr_dev->dev);
> +		pm_runtime_disable(gtr_dev->dev);
> +		return PTR_ERR(provider);
> +	}
> +
>  	return 0;
>  }
>  
> 
> base-commit: ea2bff00da89d7767d677bb68470130ba96f4928

and this does not exist for me!

-- 
~Vinod

Re: [PATCH] phy: zynqmp: register PHY provider after probe initialization

[Pandey, Radhey Shyam]

On 8/11/2026 4:49 PM, Vinod Koul wrote:
> On 07-08-26, 21:55, Radhey Shyam Pandey wrote:
>> Register the OF PHY provider only after saved_regs allocation and
>> runtime PM setup complete successfully. Publishing the provider
>> mid-probe allowed concurrent consumers to obtain a phy via xpsgtr_xlate()
>> while probe could still fail, leaving them with references into driver
>> state that devres would free on the error path.
> 
> That fails for me!
> 
Thanks for the review. I assume you mean build failure? If yes, I will
rebase it on the latest linux-phy next branch and send out v2.

Also to add - this is a theoretical race identified through sashiko and
not something hit in practice. fw_devlink does enforce the ordering, but
since it can be disabled, it seemed better to make the driver itself
robust rather than depend on that.

>>
>> Reported-by: Sashiko <sashiko-bot@kernel.org>
>> Link: https://lore.kernel.org/all/20260720155732.9DA581F000E9@smtp.kernel.org
>> Fixes: b3db66f62446 ("phy: xilinx: add runtime PM support")
>> Fixes: 5af9b304bc60 ("phy: xilinx: phy-zynqmp: Fix SGMII linkup failure on resume")
>> Signed-off-by: Radhey Shyam Pandey <radhey.shyam.pandey@amd.com>
>> ---
>>   drivers/phy/xilinx/phy-zynqmp.c | 15 ++++++++-------
>>   1 file changed, 8 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/phy/xilinx/phy-zynqmp.c b/drivers/phy/xilinx/phy-zynqmp.c
>> index 240626b55475..8930cbe24b5a 100644
>> --- a/drivers/phy/xilinx/phy-zynqmp.c
>> +++ b/drivers/phy/xilinx/phy-zynqmp.c
>> @@ -1060,13 +1060,6 @@ static int xpsgtr_probe(struct platform_device *pdev)
>>   					    xpsgtr_status_read);
>>   	}
>>   
>> -	/* Register the PHY provider. */
>> -	provider = devm_of_phy_provider_register(&pdev->dev, xpsgtr_xlate);
>> -	if (IS_ERR(provider)) {
>> -		dev_err(&pdev->dev, "registering provider failed\n");
>> -		return PTR_ERR(provider);
>> -	}
>> -
>>   	gtr_dev->saved_regs = devm_kmalloc(gtr_dev->dev,
>>   					   sizeof(save_reg_address),
>>   					   GFP_KERNEL);
>> @@ -1082,6 +1075,14 @@ static int xpsgtr_probe(struct platform_device *pdev)
>>   		return ret;
>>   	}
>>   
>> +	provider = devm_of_phy_provider_register(&pdev->dev, xpsgtr_xlate);
>> +	if (IS_ERR(provider)) {
>> +		dev_err(&pdev->dev, "registering provider failed\n");
>> +		pm_runtime_put(gtr_dev->dev);
>> +		pm_runtime_disable(gtr_dev->dev);
>> +		return PTR_ERR(provider);
>> +	}
>> +
>>   	return 0;
>>   }
>>   
>>
>> base-commit: ea2bff00da89d7767d677bb68470130ba96f4928
> 
> and this does not exist for me!

oh i see , this was on linux-next, will fix it in v2.

Thanks,
Radhey