* [PATCH] phy: zynqmp: register PHY provider after probe initialization
@ 2026-08-07 16:25 Radhey Shyam Pandey
2026-08-07 16:31 ` sashiko-bot
0 siblings, 1 reply; 2+ messages in thread
From: Radhey Shyam Pandey @ 2026-08-07 16:25 UTC (permalink / raw)
To: tomi.valkeinen, vkoul, neil.armstrong, michal.simek
Cc: linux-kernel, linux-phy, linux-arm-kernel, git,
Radhey Shyam Pandey, Sashiko
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
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH] phy: zynqmp: register PHY provider after probe initialization
2026-08-07 16:25 [PATCH] phy: zynqmp: register PHY provider after probe initialization Radhey Shyam Pandey
@ 2026-08-07 16:31 ` sashiko-bot
0 siblings, 0 replies; 2+ messages in thread
From: sashiko-bot @ 2026-08-07 16:31 UTC (permalink / raw)
To: Radhey Shyam Pandey; +Cc: neil.armstrong, olteanv, linux-phy, vkoul
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Using asynchronous pm_runtime_put() immediately before pm_runtime_disable() on the new error path cancels the pending suspend, leaving the hardware in an active power state.
--
--- Patch [1]: [PATCH] phy: zynqmp: register PHY provider after probe initialization ---
commit d73f55f7c5982fd671a5ed442cce7f38caa3d78a
Author: Radhey Shyam Pandey <radhey.shyam.pandey@amd.com>
phy: zynqmp: register PHY provider after probe initialization
This commit moves the registration of the OF PHY provider to the end of
probe, after saved_regs allocation and runtime PM are successfully setup.
This prevents concurrent consumers from obtaining references to driver
state that could be freed if the probe failed later.
Link: https://lore.kernel.org/all/20260720155732.9DA581F000E9@smtp.kernel.org
> diff --git a/drivers/phy/xilinx/phy-zynqmp.c b/drivers/phy/xilinx/phy-zynqmp.c
> --- a/drivers/phy/xilinx/phy-zynqmp.c
> +++ b/drivers/phy/xilinx/phy-zynqmp.c
[ ... ]
> @@ -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);
[Severity: Medium]
Does this error path leave the device in an active power state?
Using pm_runtime_put() queues an asynchronous idle notification. Calling
pm_runtime_disable() immediately afterward cancels pending async suspend
work, leaving the hardware active.
Would it be better to use pm_runtime_put_sync() here to ensure the device
is synchronously suspended before disabling runtime PM?
> + return PTR_ERR(provider);
> + }
> +
> return 0;
> }
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260807162510.3681572-1-radhey.shyam.pandey@amd.com?part=1
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2026-08-07 16:31 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-07 16:25 [PATCH] phy: zynqmp: register PHY provider after probe initialization Radhey Shyam Pandey
2026-08-07 16:31 ` sashiko-bot
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox