Linux-PHY Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [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