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

["Pandey, Radhey Shyam" <radheys@amd.com>]

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

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy