Re: [PATCH V3] usb: musb: Fix unstable init of OTG_INTERFSEL.

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Am 17.12.2013 18:22, schrieb David Cohen:
On Tue, Dec 17, 2013 at 05:48:33PM +0100, anaumann@xxxxxxxxxxxxxx wrote:
From: Andreas Naumann <anaumann@xxxxxxxxxxxxxx>

This is a hard to reproduce problem which leads to non-functional
USB-OTG port in 0.1%-1% of all boots. Tracked it down to commit
e25bec160158abe86c276d7d206264afc3646281, which introduces save/restore
of OTG_INTERFSEL over suspend.
Since the resume function is also called early in driver init, it uses a
non-initialized value (which is 0 and a non-supported setting in DM37xx
for INTERFSEL). Shortly after the correct value is set. Apparently this
works most time, but not always.

Fix it by not writing the value on runtime resume if it has not been
initialized yet.

Signed-off-by: Andreas Naumann <anaumann@xxxxxxxxxxxxxx>
---
Even though I find the implementation a bit awkward this should fix
the issue without breaking anything else. Hope everyone is happy
with this.

  drivers/usb/musb/omap2430.c | 6 +++++-
  1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/musb/omap2430.c b/drivers/usb/musb/omap2430.c
index 4315d35..fbe2c08 100644
--- a/drivers/usb/musb/omap2430.c
+++ b/drivers/usb/musb/omap2430.c
@@ -48,6 +48,7 @@ struct omap2430_glue {
  	enum omap_musb_vbus_id_status status;
  	struct work_struct	omap_musb_mailbox_work;
  	struct device		*control_otghs;
+	u8 	initialized;
  };
  #define glue_to_musb(g)		platform_get_drvdata(g->musb)

@@ -383,6 +384,7 @@ static int omap2430_musb_init(struct musb *musb)
  	}

  	musb_writel(musb->mregs, OTG_INTERFSEL, l);
+	glue->initialized = 1;

  	pr_debug("HS USB OTG: revision 0x%x, sysconfig 0x%02x, "
  			"sysstatus 0x%x, intrfsel 0x%x, simenable  0x%x\n",
@@ -509,6 +511,7 @@ static int omap2430_probe(struct platform_device *pdev)
  	glue->dev			= &pdev->dev;
  	glue->musb			= musb;
  	glue->status			= OMAP_MUSB_UNKNOWN;
+	glue->initialized	= 0;

You don't need to do this. 'glue' was already allocated with kzalloc().

ok



  	if (np) {
  		pdata = devm_kzalloc(&pdev->dev, sizeof(*pdata), GFP_KERNEL);
@@ -646,7 +649,8 @@ static int omap2430_runtime_resume(struct device *dev)

  	if (musb) {
  		omap2430_low_level_init(musb);
-		musb_writel(musb->mregs, OTG_INTERFSEL,
+		if(glue->initialized)

Are you sure this is thread safe?
If you're sending this patch it means runtime_resume can be called
before omap2430_must_init(), but how about at the same time?
You defined 'initialized' as u8 type, then read/write operations won't
be atomic in ARM.

You're right, wasnt thinking of that. Shall I use atomic_t and helpers?


Br, David Cohen

+			musb_writel(musb->mregs, OTG_INTERFSEL,
  				musb->context.otg_interfsel);

  		usb_phy_set_suspend(musb->xceiv, 0);
--
1.8.4.1

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux