Re: [PATCH] phy: zynqmp: register PHY provider after probe initialization
"Pandey, Radhey Shyam" <[email protected]>
| Newsgroups | org.infradead.lists.linux-phy,org.infradead.lists.linux-arm-kernel,org.kernel.vger.linux-kernel |
|---|---|
| Message-ID | <[email protected]> |
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 <[email protected]> >> Link: https://lore.kernel.org/all/[email protected] >> 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 <[email protected]> >> --- >> 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 [email protected] https://lists.infradead.org/mailman/listinfo/linux-phy