[PATCH v2 0/3] usb: musb: omap2430: handle PHY setup failures across PM

[PATCH v2 0/3] usb: musb: omap2430: handle PHY setup failures across PM

[Pengpeng Hou]

The OMAP2430 glue has three unchecked PHY setup sites: initial setup,
runtime resume and system resume. This v2 resends them as one threaded
series, as Greg requested, and closes the shared reference-ownership gap
before adding the remaining error propagation.

Patch 1 records initialization and power references independently and
uses common helpers in init, PM and exit. Patch 2 stops runtime resume
and mailbox work when resuming fails. Patch 3 reports normal system-resume
failure while ending the temporary I2C/SPI PHY deferral, allowing a later
runtime resume to retry.

The existing phy_suspended flag controls system-sleep ordering, not
resource ownership. Keeping those roles separate avoids double-release
on another suspend or platform exit after a failed resume. A release
failure retains its ownership flag rather than silently discarding it.

The three original setup sites share this lifecycle machinery. They are
not three independent proofs of hardware faults, and this series does
not address errors hidden inside individual PHY providers.

The issues were found by our static-analysis tool.

Changes since v1:
- resend with all three members replying directly to this cover
- share reference tracking across init, runtime PM, system PM and exit
- account for a cleanup operation itself failing
- stop mailbox access when runtime resume fails
- retain needs_resume until early resume succeeds and end system PHY
  deferral after normal resume even when setup fails

Previous series:
https://lore.kernel.org/all/20260906034149.85550-1-hppiscas@163.com/

Pengpeng Hou (3):
  usb: musb: omap2430: track PHY references and check initial setup
  usb: musb: omap2430: stop runtime resume after PHY setup failure
  usb: musb: omap2430: report system-resume PHY setup failures

 drivers/usb/musb/omap2430.c | 123 ++++++++++++++++++++++++++++++++++++++------
 1 file changed, 107 insertions(+), 16 deletions(-)

base-commit: f0100363d8c374bd8e9ea7c9ba02744f0b802ca4

[PATCH v2 1/3] usb: musb: omap2430: track PHY references and check initial setup

[Pengpeng Hou]

omap2430_musb_init() ignores generic PHY initialization and power-on
errors before accessing the controller. The same PHY is later acquired
and released by runtime PM, system PM and platform exit.

Track initialization and power references separately, and use shared
helpers at all these acquisition and release sites. A failed power-on
releases its initialization reference when possible; a failed release
retains the corresponding ownership flag so a later cleanup does not
double-release or lose track of it. Suspend failures attempt to restore
the PHY before returning the original error.

Abort initial setup before accessing the interface registers on failure.
musb_init_controller() skips platform exit when platform init fails, so
initial setup must unwind its own references. Platform exit uses the same
ownership-aware release path when entered after later setup failures.
Keep the PHY pointer in the parent glue, which owns the managed PHY
handle, so parent removal can retry a release left by a failed child
initialization or exit even after the child clears musb->phy.

Keep phy_suspended as the system-sleep ordering flag: it defers access to
I2C/SPI PHYs until their normal resume phase. It is not a substitute for
tracking the two generic PHY reference counts.

The issue was found by our static-analysis tool.

Fixes: 3e3101d57c50 ("usb: musb: omap2430: use the new generic PHY framework")
Assisted-by: gpt 5
Signed-off-by: Pengpeng Hou <hppiscas@163.com>
---
 drivers/usb/musb/omap2430.c | 105 +++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 93 insertions(+), 12 deletions(-)

diff --git a/drivers/usb/musb/omap2430.c b/drivers/usb/musb/omap2430.c
index 6e749faac33c..df807ee9deec 100644
--- a/drivers/usb/musb/omap2430.c
+++ b/drivers/usb/musb/omap2430.c
@@ -34,14 +34,71 @@ struct omap2430_glue {
 	enum musb_vbus_id_status status;
 	struct work_struct	omap_musb_mailbox_work;
 	struct device		*control_otghs;
+	struct phy		*phy;
 	unsigned int		is_runtime_suspended:1;
 	unsigned int		needs_resume:1;
+	/* Defer PHY access until the normal system-resume phase. */
 	unsigned int		phy_suspended:1;
+	unsigned int		phy_initialized:1;
+	unsigned int		phy_powered:1;
 };
 #define glue_to_musb(g)		platform_get_drvdata(g->musb)
 
 static struct omap2430_glue	*_glue;
 
+static int omap2430_phy_power_on(struct omap2430_glue *glue)
+{
+	int ret;
+
+	if (!glue->phy)
+		return 0;
+
+	if (!glue->phy_initialized) {
+		ret = phy_init(glue->phy);
+		if (ret)
+			return ret;
+		glue->phy_initialized = 1;
+	}
+
+	if (glue->phy_powered)
+		return 0;
+
+	ret = phy_power_on(glue->phy);
+	if (ret) {
+		/* Keep ownership if phy_exit() itself cannot release the reference. */
+		if (!phy_exit(glue->phy))
+			glue->phy_initialized = 0;
+		return ret;
+	}
+	glue->phy_powered = 1;
+
+	return 0;
+}
+
+static int omap2430_phy_power_off(struct omap2430_glue *glue)
+{
+	int ret;
+
+	if (!glue->phy)
+		return 0;
+
+	if (glue->phy_powered) {
+		ret = phy_power_off(glue->phy);
+		if (ret)
+			return ret;
+		glue->phy_powered = 0;
+	}
+
+	if (glue->phy_initialized) {
+		ret = phy_exit(glue->phy);
+		if (ret)
+			return ret;
+		glue->phy_initialized = 0;
+	}
+
+	return 0;
+}
+
 static inline void omap2430_low_level_exit(struct musb *musb)
 {
 	u32 l;
@@ -192,6 +249,7 @@ static int omap2430_musb_init(struct musb *musb)
 	struct device *dev = musb->controller;
 	struct musb_hdrc_platform_data *plat = dev_get_platdata(dev);
 	struct omap_musb_board_data *data = plat->board_data;
+	struct omap2430_glue *glue = dev_get_drvdata(dev->parent);
 
 	/* We require some kind of external transceiver, hooked
 	 * up through ULPI.  TWL4030-family PMICs include one,
@@ -222,8 +280,10 @@ static int omap2430_musb_init(struct musb *musb)
 		return PTR_ERR(musb->phy);
 	}
 	musb->isr = omap2430_musb_interrupt;
-	phy_init(musb->phy);
-	phy_power_on(musb->phy);
+	glue->phy = musb->phy;
+	status = omap2430_phy_power_on(glue);
+	if (status)
+		return status;
 
 	l = musb_readl(musb->mregs, OTG_INTERFSEL);
 
@@ -272,10 +332,12 @@ static int omap2430_musb_exit(struct musb *musb)
 {
 	struct device *dev = musb->controller;
 	struct omap2430_glue *glue = dev_get_drvdata(dev->parent);
+	int ret;
 
 	omap2430_low_level_exit(musb);
-	phy_power_off(musb->phy);
-	phy_exit(musb->phy);
+	ret = omap2430_phy_power_off(glue);
+	if (ret)
+		dev_err(dev, "failed to shut down PHY: %d\n", ret);
 	musb->phy = NULL;
 	cancel_work_sync(&glue->omap_musb_mailbox_work);
 
@@ -471,9 +533,13 @@ static int omap2430_probe(struct platform_device *pdev)
 static void omap2430_remove(struct platform_device *pdev)
 {
 	struct omap2430_glue *glue = platform_get_drvdata(pdev);
+	int ret;
 
 	platform_device_unregister(glue->musb);
 	pm_runtime_disable(glue->dev);
+	ret = omap2430_phy_power_off(glue);
+	if (ret)
+		dev_err(&pdev->dev, "failed to release PHY references: %d\n", ret);
 	if (!IS_ERR(glue->control_otghs))
 		put_device(glue->control_otghs);
 }
@@ -484,6 +550,7 @@ static int omap2430_runtime_suspend(struct device *dev)
 {
 	struct omap2430_glue		*glue = dev_get_drvdata(dev);
 	struct musb			*musb = glue_to_musb(glue);
+	int ret;
 
 	if (!musb)
 		return 0;
@@ -494,8 +561,13 @@ static int omap2430_runtime_suspend(struct device *dev)
 	omap2430_low_level_exit(musb);
 
 	if (!glue->phy_suspended) {
-		phy_power_off(musb->phy);
-		phy_exit(musb->phy);
+		ret = omap2430_phy_power_off(glue);
+		if (ret) {
+			if (omap2430_phy_power_on(glue))
+				dev_err(dev, "failed to restore PHY after suspend error\n");
+			omap2430_low_level_init(musb);
+			return ret;
+		}
 	}
 
 	glue->is_runtime_suspended = 1;
@@ -512,8 +584,7 @@ static int omap2430_runtime_resume(struct device *dev)
 		return 0;
 
 	if (!glue->phy_suspended) {
-		phy_init(musb->phy);
-		phy_power_on(musb->phy);
+		omap2430_phy_power_on(glue);
 	}
 
 	omap2430_low_level_init(musb);
@@ -533,9 +604,17 @@ static int omap2430_suspend(struct device *dev)
 {
 	struct omap2430_glue *glue = dev_get_drvdata(dev);
 	struct musb *musb = glue_to_musb(glue);
+	int ret;
 
-	phy_power_off(musb->phy);
-	phy_exit(musb->phy);
+	if (!musb)
+		return 0;
+
+	ret = omap2430_phy_power_off(glue);
+	if (ret) {
+		if (omap2430_phy_power_on(glue))
+			dev_err(dev, "failed to restore PHY after suspend error\n");
+		return ret;
+	}
 	glue->phy_suspended = 1;
 
 	return 0;
@@ -571,8 +650,10 @@ static int omap2430_resume(struct device *dev)
 	struct omap2430_glue *glue = dev_get_drvdata(dev);
 	struct musb *musb = glue_to_musb(glue);
 
-	phy_init(musb->phy);
-	phy_power_on(musb->phy);
+	if (!musb)
+		return 0;
+
+	omap2430_phy_power_on(glue);
 	glue->phy_suspended = 0;
 
 	return 0;

base-commit: f0100363d8c374bd8e9ea7c9ba02744f0b802ca4
-- 
2.50.1 (Apple Git-155)

[PATCH v2 2/3] usb: musb: omap2430: stop runtime resume after PHY setup failure

[Pengpeng Hou]

omap2430_runtime_resume() continues restoring the glue registers and
clears is_runtime_suspended even when generic PHY setup fails.

Return the setup error before accessing those registers or publishing
the resumed state. Also stop mailbox processing if its runtime-PM get
fails, rather than treating an unsuccessful resume as access permission.
pm_runtime_resume_and_get() balances its usage reference on failure.

Keep needs_resume set until the early system-resume operation succeeds.
The normal system-resume phase remains responsible for I2C/SPI PHYs.

The issue was found by our static-analysis tool.

Fixes: 10ac7e7757f5 ("usb: musb: omap2430: Add support for idling phy when musb is idle")
Assisted-by: gpt 5
Signed-off-by: Pengpeng Hou <hppiscas@163.com>
---
 drivers/usb/musb/omap2430.c | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/drivers/usb/musb/omap2430.c b/drivers/usb/musb/omap2430.c
index df807ee9deec..382d6cd4a9e6 100644
--- a/drivers/usb/musb/omap2430.c
+++ b/drivers/usb/musb/omap2430.c
@@ -150,7 +150,9 @@ static void omap_musb_set_mailbox(struct omap2430_glue *glue)
 	struct musb *musb = glue_to_musb(glue);
 	int error;
 
-	pm_runtime_get_sync(musb->controller);
+	error = pm_runtime_resume_and_get(musb->controller);
+	if (error < 0)
+		return;
 
 	dev_dbg(musb->controller, "VBUS %s, devctl %02x\n",
 		usb_otg_state_string(musb->xceiv->otg->state),
@@ -579,12 +581,15 @@ static int omap2430_runtime_resume(struct device *dev)
 {
 	struct omap2430_glue		*glue = dev_get_drvdata(dev);
 	struct musb			*musb = glue_to_musb(glue);
+	int ret;
 
 	if (!musb)
 		return 0;
 
 	if (!glue->phy_suspended) {
-		omap2430_phy_power_on(glue);
+		ret = omap2430_phy_power_on(glue);
+		if (ret)
+			return ret;
 	}
 
 	omap2430_low_level_init(musb);
@@ -636,13 +641,16 @@ static int omap2430_suspend_late(struct device *dev)
 static int omap2430_resume_early(struct device *dev)
 {
 	struct omap2430_glue *glue = dev_get_drvdata(dev);
+	int ret;
 
 	if (!glue->needs_resume)
 		return 0;
 
-	glue->needs_resume = 0;
+	ret = omap2430_runtime_resume(dev);
+	if (!ret)
+		glue->needs_resume = 0;
 
-	return omap2430_runtime_resume(dev);
+	return ret;
 }
 
 static int omap2430_resume(struct device *dev)

base-commit: f0100363d8c374bd8e9ea7c9ba02744f0b802ca4
-- 
2.50.1 (Apple Git-155)

[PATCH v2 3/3] usb: musb: omap2430: report system-resume PHY setup failures

[Pengpeng Hou]

omap2430_resume() ignores PHY setup failures and always reports success.

Return the result of the ownership-aware PHY setup helper. End the
system-sleep deferral even on failure: leaving phy_suspended set would
make later runtime resumes skip the PHY permanently. The independent
reference flags still record which resources need cleanup or retry.

A later suspend or platform exit therefore releases only references
actually held, while a runtime resume can retry setup without adding a
second initialization reference left over from a failed cleanup.

The issue was found by our static-analysis tool.

Fixes: 68d9f95d6fd5 ("usb: musb: Fix suspend and resume issues for PHYs on I2C and SPI")
Assisted-by: gpt 5
Signed-off-by: Pengpeng Hou <hppiscas@163.com>
---
 drivers/usb/musb/omap2430.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/musb/omap2430.c b/drivers/usb/musb/omap2430.c
index 382d6cd4a9e6..951e60906f2c 100644
--- a/drivers/usb/musb/omap2430.c
+++ b/drivers/usb/musb/omap2430.c
@@ -657,14 +657,16 @@ static int omap2430_resume(struct device *dev)
 {
 	struct omap2430_glue *glue = dev_get_drvdata(dev);
 	struct musb *musb = glue_to_musb(glue);
+	int ret;
 
 	if (!musb)
 		return 0;
 
-	omap2430_phy_power_on(glue);
+	ret = omap2430_phy_power_on(glue);
+	/* End system-sleep deferral so a later runtime resume can retry. */
 	glue->phy_suspended = 0;
 
-	return 0;
+	return ret;
 }
 
 static const struct dev_pm_ops omap2430_pm_ops = {

base-commit: f0100363d8c374bd8e9ea7c9ba02744f0b802ca4
-- 
2.50.1 (Apple Git-155)