2014-04-24 09:56:56

by Paul Fertser

[permalink] [raw]
Subject: [PATCH] usb: gadget: fsl: check vbus presence on probe

If VBUS is already present during the driver initialisation, the
corresponding IRQ never fires, so there is no way the gadget can get
enumerated.

This patch is real-life tested on an i.MX25 board with VBUS constantly
hooked up.

Signed-off-by: Paul Fertser <[email protected]>
---
drivers/usb/gadget/fsl_udc_core.c | 5 +++++
1 file changed, 5 insertions(+)

diff --git a/drivers/usb/gadget/fsl_udc_core.c b/drivers/usb/gadget/fsl_udc_core.c
index a2f26cd..b4b1516 100644
--- a/drivers/usb/gadget/fsl_udc_core.c
+++ b/drivers/usb/gadget/fsl_udc_core.c
@@ -2501,6 +2501,11 @@ static int __init fsl_udc_probe(struct platform_device *pdev)
if (ret)
goto err_del_udc;

+ /* Now let it settle a bit and sense VBUS */
+ msleep_interruptible(1);
+ if (fsl_readl(&dr_regs->otgsc) & OTGSC_STS_B_SESSION_VALID)
+ udc_controller->vbus_active = 1;
+
create_proc_file();
return 0;

--
1.7.10


2014-04-30 16:06:36

by Felipe Balbi

[permalink] [raw]
Subject: Re: [PATCH] usb: gadget: fsl: check vbus presence on probe

On Thu, Apr 24, 2014 at 12:54:13PM +0400, Paul Fertser wrote:
> If VBUS is already present during the driver initialisation, the

s/initialisation/initialization

> corresponding IRQ never fires, so there is no way the gadget can get
> enumerated.
>
> This patch is real-life tested on an i.MX25 board with VBUS constantly
> hooked up.
>
> Signed-off-by: Paul Fertser <[email protected]>
> ---
> drivers/usb/gadget/fsl_udc_core.c | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/drivers/usb/gadget/fsl_udc_core.c b/drivers/usb/gadget/fsl_udc_core.c
> index a2f26cd..b4b1516 100644
> --- a/drivers/usb/gadget/fsl_udc_core.c
> +++ b/drivers/usb/gadget/fsl_udc_core.c
> @@ -2501,6 +2501,11 @@ static int __init fsl_udc_probe(struct platform_device *pdev)
> if (ret)
> goto err_del_udc;
>
> + /* Now let it settle a bit and sense VBUS */
> + msleep_interruptible(1);
> + if (fsl_readl(&dr_regs->otgsc) & OTGSC_STS_B_SESSION_VALID)
> + udc_controller->vbus_active = 1;

good fix, should this go to stable ?

--
balbi


Attachments:
(No filename) (1.02 kB)
signature.asc (819.00 B)
Digital signature
Download all attachments

2014-04-30 19:27:54

by Paul Fertser

[permalink] [raw]
Subject: Re: [PATCH] usb: gadget: fsl: check vbus presence on probe

Hello,

Thank you for the review.

On Wed, Apr 30, 2014 at 11:06:25AM -0500, Felipe Balbi wrote:
> On Thu, Apr 24, 2014 at 12:54:13PM +0400, Paul Fertser wrote:
> > If VBUS is already present during the driver initialisation, the
>
> s/initialisation/initialization

If I'm understanding [1] properly, both spelling variants are correct.

> > + /* Now let it settle a bit and sense VBUS */
> > + msleep_interruptible(1);
> > + if (fsl_readl(&dr_regs->otgsc) & OTGSC_STS_B_SESSION_VALID)
> > + udc_controller->vbus_active = 1;
>
> good fix, should this go to stable ?

I was reluctant to Cc stable because I'm not sure which versions need
to be fixed, as I've seen an earlier version (3.0 something or a bit
before that probably) working without this (no idea why); also I'm not
sure about that msleep as it's deduced solely from experimenting with
a particular i.MX model, I hoped someone from Freescale would comment
on it. I've seen your review [2] on a related patch and my fix should
have been obvious to Suresh (and trivial to test on different boards),
yet he didn't implement it, that worries me a bit.

[1] https://en.wikipedia.org/wiki/American_and_British_English_spelling_differences#-ise.2C_-ize_.28-isation.2C_-ization.29
[2] https://patchwork.kernel.org/patch/3822321/
--
Be free, use free (http://www.gnu.org/philosophy/free-sw.html) software!
mailto:[email protected]

2014-04-30 20:12:13

by Felipe Balbi

[permalink] [raw]
Subject: Re: [PATCH] usb: gadget: fsl: check vbus presence on probe

+Suresh

(top-posting, yeah yeah :-)

On Wed, Apr 30, 2014 at 11:27:45PM +0400, Paul Fertser wrote:
> Hello,
>
> Thank you for the review.
>
> On Wed, Apr 30, 2014 at 11:06:25AM -0500, Felipe Balbi wrote:
> > On Thu, Apr 24, 2014 at 12:54:13PM +0400, Paul Fertser wrote:
> > > If VBUS is already present during the driver initialisation, the
> >
> > s/initialisation/initialization
>
> If I'm understanding [1] properly, both spelling variants are correct.
>
> > > + /* Now let it settle a bit and sense VBUS */
> > > + msleep_interruptible(1);
> > > + if (fsl_readl(&dr_regs->otgsc) & OTGSC_STS_B_SESSION_VALID)
> > > + udc_controller->vbus_active = 1;
> >
> > good fix, should this go to stable ?
>
> I was reluctant to Cc stable because I'm not sure which versions need
> to be fixed, as I've seen an earlier version (3.0 something or a bit
> before that probably) working without this (no idea why); also I'm not
> sure about that msleep as it's deduced solely from experimenting with
> a particular i.MX model, I hoped someone from Freescale would comment
> on it. I've seen your review [2] on a related patch and my fix should
> have been obvious to Suresh (and trivial to test on different boards),
> yet he didn't implement it, that worries me a bit.
>
> [1] https://en.wikipedia.org/wiki/American_and_British_English_spelling_differences#-ise.2C_-ize_.28-isation.2C_-ization.29
> [2] https://patchwork.kernel.org/patch/3822321/
> --
> Be free, use free (http://www.gnu.org/philosophy/free-sw.html) software!
> mailto:[email protected]

--
balbi


Attachments:
(No filename) (1.53 kB)
signature.asc (819.00 B)
Digital signature
Download all attachments