* [PATCH] usb: gadget: fsl: check vbus presence on probe @ 2014-04-24 8:54 Paul Fertser 2014-04-30 16:06 ` Felipe Balbi 0 siblings, 1 reply; 11+ messages in thread From: Paul Fertser @ 2014-04-24 8:54 UTC (permalink / raw) To: Li Yang, Felipe Balbi; +Cc: linux-usb, linux-kernel, Paul Fertser 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 <fercerpav@gmail.com> --- 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 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH] usb: gadget: fsl: check vbus presence on probe 2014-04-24 8:54 [PATCH] usb: gadget: fsl: check vbus presence on probe Paul Fertser @ 2014-04-30 16:06 ` Felipe Balbi 2014-04-30 19:27 ` Paul Fertser 0 siblings, 1 reply; 11+ messages in thread From: Felipe Balbi @ 2014-04-30 16:06 UTC (permalink / raw) To: Paul Fertser; +Cc: Li Yang, Felipe Balbi, linux-usb, linux-kernel [-- Attachment #1: Type: text/plain, Size: 1076 bytes --] 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 <fercerpav@gmail.com> > --- > 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 [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 819 bytes --] ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] usb: gadget: fsl: check vbus presence on probe 2014-04-30 16:06 ` Felipe Balbi @ 2014-04-30 19:27 ` Paul Fertser 2014-04-30 20:12 ` Felipe Balbi 0 siblings, 1 reply; 11+ messages in thread From: Paul Fertser @ 2014-04-30 19:27 UTC (permalink / raw) To: Felipe Balbi; +Cc: Li Yang, linux-usb, linux-kernel 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:fercerpav@gmail.com ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] usb: gadget: fsl: check vbus presence on probe 2014-04-30 19:27 ` Paul Fertser @ 2014-04-30 20:12 ` Felipe Balbi 2014-05-01 14:27 ` suresh.gupta 0 siblings, 1 reply; 11+ messages in thread From: Felipe Balbi @ 2014-04-30 20:12 UTC (permalink / raw) To: Paul Fertser; +Cc: Felipe Balbi, Li Yang, linux-usb, linux-kernel, suresh.gupta [-- Attachment #1: Type: text/plain, Size: 1608 bytes --] +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:fercerpav@gmail.com -- balbi [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 819 bytes --] ^ permalink raw reply [flat|nested] 11+ messages in thread
* RE: [PATCH] usb: gadget: fsl: check vbus presence on probe 2014-04-30 20:12 ` Felipe Balbi @ 2014-05-01 14:27 ` suresh.gupta 2014-05-08 14:30 ` suresh.gupta 0 siblings, 1 reply; 11+ messages in thread From: suresh.gupta @ 2014-05-01 14:27 UTC (permalink / raw) To: balbi, Paul Fertser; +Cc: LeoLi, linux-usb, linux-kernel Give me some time (actually some days), I will try this and update you. > -----Original Message----- > From: Felipe Balbi [mailto:balbi@ti.com] > Sent: Thursday, May 01, 2014 1:42 AM > To: Paul Fertser > Cc: Felipe Balbi; Li Yang-Leo-R58472; linux-usb@vger.kernel.org; linux- > kernel@vger.kernel.org; Gupta Suresh-B42813 > 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_di > > fferences#-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:fercerpav@gmail.com > > -- > balbi ^ permalink raw reply [flat|nested] 11+ messages in thread
* RE: [PATCH] usb: gadget: fsl: check vbus presence on probe 2014-05-01 14:27 ` suresh.gupta @ 2014-05-08 14:30 ` suresh.gupta 2014-05-08 15:38 ` Paul Fertser 0 siblings, 1 reply; 11+ messages in thread From: suresh.gupta @ 2014-05-08 14:30 UTC (permalink / raw) To: suresh.gupta, balbi, Paul Fertser; +Cc: LeoLi, linux-usb, linux-kernel Hi, As per my limited knowledge, the purpose of OTGSC_STS_B_SESSION_VALID bit is to tell either VBUS is above the B session valid threshold and which comes only Host is attached. And Host might be attach after system bootup or after driver initialization. So putting this check in probe will not help much. > -----Original Message----- > From: linux-usb-owner@vger.kernel.org [mailto:linux-usb- > owner@vger.kernel.org] On Behalf Of suresh.gupta@freescale.com > Sent: Thursday, May 01, 2014 7:58 PM > To: balbi@ti.com; Paul Fertser > Cc: Li Yang-Leo-R58472; linux-usb@vger.kernel.org; linux- > kernel@vger.kernel.org > Subject: RE: [PATCH] usb: gadget: fsl: check vbus presence on probe > > Give me some time (actually some days), I will try this and update you. > > > -----Original Message----- > > From: Felipe Balbi [mailto:balbi@ti.com] > > Sent: Thursday, May 01, 2014 1:42 AM > > To: Paul Fertser > > Cc: Felipe Balbi; Li Yang-Leo-R58472; linux-usb@vger.kernel.org; > > linux- kernel@vger.kernel.org; Gupta Suresh-B42813 > > 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_ > > > di > > > fferences#-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:fercerpav@gmail.com > > > > -- > > balbi > -- > To unsubscribe from this list: send the line "unsubscribe linux-usb" in > the body of a message to majordomo@vger.kernel.org More majordomo info at > http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] usb: gadget: fsl: check vbus presence on probe 2014-05-08 14:30 ` suresh.gupta @ 2014-05-08 15:38 ` Paul Fertser 2014-05-08 18:42 ` suresh.gupta 0 siblings, 1 reply; 11+ messages in thread From: Paul Fertser @ 2014-05-08 15:38 UTC (permalink / raw) To: suresh.gupta; +Cc: balbi, LeoLi, linux-usb, linux-kernel Hi, On Thu, May 08, 2014 at 02:30:39PM +0000, suresh.gupta@freescale.com wrote: > As per my limited knowledge, the purpose of > OTGSC_STS_B_SESSION_VALID bit is to tell either VBUS is above the B > session valid threshold and which comes only Host is attached. Yes, that matches the datasheet I've read. > And Host might be attach after system bootup or after driver > initialization. So putting this check in probe will not help much. If the host is attached after the driver was initialised, the interrupt will trigger and the driver will get notified that VBUS appeared and everything will go smooth, at least that's how it should work (I do not have any board handy to real-life check that, but AFAICT that's the intent of the current code). I actually do not have any iMX demoboards at all, I've only got some custom-designed i.MX25 boards where I can't control VBUS, it's permanently pulled up. But I was fixing the problem that was clearly, 100% reproducibly happening when VBUS was applied before the interrupt was configured. So what exactly do you mean here? Do you mean this check I've added doesn't fix the bug? Or do you mean this bug should be fixed somehow else? Or do you mean there's no bug in the first place and my board doesn't work because of something else? -- Be free, use free (http://www.gnu.org/philosophy/free-sw.html) software! mailto:fercerpav@gmail.com ^ permalink raw reply [flat|nested] 11+ messages in thread
* RE: [PATCH] usb: gadget: fsl: check vbus presence on probe 2014-05-08 15:38 ` Paul Fertser @ 2014-05-08 18:42 ` suresh.gupta 2014-05-08 20:14 ` Paul Fertser 0 siblings, 1 reply; 11+ messages in thread From: suresh.gupta @ 2014-05-08 18:42 UTC (permalink / raw) To: Paul Fertser, balbi; +Cc: LeoLi, linux-usb, linux-kernel > -----Original Message----- > From: Paul Fertser [mailto:fercerpav@gmail.com] > Sent: Thursday, May 08, 2014 9:09 PM > To: Gupta Suresh-B42813 > Cc: balbi@ti.com; Li Yang-Leo-R58472; linux-usb@vger.kernel.org; linux- > kernel@vger.kernel.org > Subject: Re: [PATCH] usb: gadget: fsl: check vbus presence on probe > > Hi, > > On Thu, May 08, 2014 at 02:30:39PM +0000, suresh.gupta@freescale.com > wrote: > > As per my limited knowledge, the purpose of OTGSC_STS_B_SESSION_VALID > > bit is to tell either VBUS is above the B session valid threshold and > > which comes only Host is attached. > > Yes, that matches the datasheet I've read. > > > And Host might be attach after system bootup or after driver > > initialization. So putting this check in probe will not help much. > > If the host is attached after the driver was initialised, the interrupt > will trigger and the driver will get notified that VBUS appeared and > everything will go smooth, at least that's how it should work (I do not > have any board handy to real-life check that, but AFAICT that's the > intent of the current code). If is go through the code flow starting from usb_composite_probe the sequence is usb_composite_probe->usb_gadget_probe_driver->udc_bind_to_driver-> udc_bind_to_driver->usb_gadget_udc_start->fsl_udc_start->usbcmd=RUN udc_bind_to_driver->usb_gadget_connect->fsl_pullup The function fsl_pullup make usbcmd=STOP if my fix is not there and if controller is stopped we do not get any interrupt. > > I actually do not have any iMX demoboards at all, I've only got some > custom-designed i.MX25 boards where I can't control VBUS, it's > permanently pulled up. > > But I was fixing the problem that was clearly, 100% reproducibly > happening when VBUS was applied before the interrupt was configured. So Wait a minute, are you using OTG. If your VBUS is permanently pulled up, that means you are only Gadget(I might miss something) then why you use OTG mode. > what exactly do you mean here? Do you mean this check I've added doesn't > fix the bug? Or do you mean this bug should be fixed somehow else? Or do What expertly my concern is, probe is not proper place to check VBUS valid. I think we should wait to hear what Balbi has to say. > you mean there's no bug in the first place and my board doesn't work > because of something else? > > -- > Be free, use free (http://www.gnu.org/philosophy/free-sw.html) software! > mailto:fercerpav@gmail.com ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] usb: gadget: fsl: check vbus presence on probe 2014-05-08 18:42 ` suresh.gupta @ 2014-05-08 20:14 ` Paul Fertser 2014-05-09 9:07 ` suresh.gupta 0 siblings, 1 reply; 11+ messages in thread From: Paul Fertser @ 2014-05-08 20:14 UTC (permalink / raw) To: suresh.gupta; +Cc: balbi, LeoLi, linux-usb, linux-kernel On Thu, May 08, 2014 at 06:42:53PM +0000, suresh.gupta@freescale.com wrote: > > > And Host might be attach after system bootup or after driver > > > initialization. So putting this check in probe will not help much. > > > > If the host is attached after the driver was initialised, the interrupt > > will trigger and the driver will get notified that VBUS appeared and > > everything will go smooth, at least that's how it should work (I do not > > have any board handy to real-life check that, but AFAICT that's the > > intent of the current code). > > If is go through the code flow starting from usb_composite_probe the sequence is > usb_composite_probe->usb_gadget_probe_driver->udc_bind_to_driver-> > udc_bind_to_driver->usb_gadget_udc_start->fsl_udc_start->usbcmd=RUN > udc_bind_to_driver->usb_gadget_connect->fsl_pullup > > The function fsl_pullup make usbcmd=STOP if my fix is not there and if controller > is stopped we do not get any interrupt. Are you really sure we can't get async VBUS state change notifications until controller has USB_CMD_RUN_STOP bit set (and the same bit actually controls internal 1.5k dataline pullup)? If yes, I guess it means we need to check VBUS state _every_ time we set that bit to sync the vbus_active variable with the actual hardware state (unless an external OTG PHY is used and VBUS pad state is irrelevant)? > > I actually do not have any iMX demoboards at all, I've only got some > > custom-designed i.MX25 boards where I can't control VBUS, it's > > permanently pulled up. > > > > But I was fixing the problem that was clearly, 100% reproducibly > > happening when VBUS was applied before the interrupt was configured. So > > Wait a minute, are you using OTG. If your VBUS is permanently pulled up, that > means you are only Gadget(I might miss something) then why you use OTG mode. I'm using FSL_USB2_DR_DEVICE mode (FSL_USB2_PHY_UTMI phy_mode), so the OTG controller should always work in the device mode only. > > what exactly do you mean here? Do you mean this check I've added doesn't > > fix the bug? Or do you mean this bug should be fixed somehow else? Or do > > What expertly my concern is, probe is not proper place to check VBUS valid. > I think we should wait to hear what Balbi has to say. Yes, I hope he can understand both of us :) -- Be free, use free (http://www.gnu.org/philosophy/free-sw.html) software! mailto:fercerpav@gmail.com ^ permalink raw reply [flat|nested] 11+ messages in thread
* RE: [PATCH] usb: gadget: fsl: check vbus presence on probe 2014-05-08 20:14 ` Paul Fertser @ 2014-05-09 9:07 ` suresh.gupta 2014-05-09 11:18 ` Paul Fertser 0 siblings, 1 reply; 11+ messages in thread From: suresh.gupta @ 2014-05-09 9:07 UTC (permalink / raw) To: Paul Fertser, balbi; +Cc: LeoLi, linux-usb, linux-kernel > -----Original Message----- > From: Paul Fertser [mailto:fercerpav@gmail.com] > Sent: Friday, May 09, 2014 1:44 AM > To: Gupta Suresh-B42813 > Cc: balbi@ti.com; Li Yang-Leo-R58472; linux-usb@vger.kernel.org; linux- > kernel@vger.kernel.org > Subject: Re: [PATCH] usb: gadget: fsl: check vbus presence on probe > > On Thu, May 08, 2014 at 06:42:53PM +0000, suresh.gupta@freescale.com > wrote: > > > > And Host might be attach after system bootup or after driver > > > > initialization. So putting this check in probe will not help much. > > > > > > If the host is attached after the driver was initialised, the > > > interrupt will trigger and the driver will get notified that VBUS > > > appeared and everything will go smooth, at least that's how it > > > should work (I do not have any board handy to real-life check that, > > > but AFAICT that's the intent of the current code). > > > > If is go through the code flow starting from usb_composite_probe the > > sequence is > > usb_composite_probe->usb_gadget_probe_driver->udc_bind_to_driver-> > > udc_bind_to_driver->usb_gadget_udc_start->fsl_udc_start->usbcmd=RUN > > udc_bind_to_driver->usb_gadget_connect->fsl_pullup > > > > The function fsl_pullup make usbcmd=STOP if my fix is not there and if > > controller is stopped we do not get any interrupt. > > Are you really sure we can't get async VBUS state change notifications > until controller has USB_CMD_RUN_STOP bit set (and the same bit actually > controls internal 1.5k dataline pullup)? If yes, I guess it means we need > to check VBUS state _every_ time we set that bit to sync the vbus_active > variable with the actual hardware state (unless an external OTG PHY is > used and VBUS pad state is irrelevant)? There will be no interrupts if USB_CMD_RUN_STOP is not set but status get change. I doubt, my previous patch (252455c40316009cc791f00338ee2e367d2d2739) make any difference in your device working. Can you please remove my patch and check either your device work or not. We are using two different configs(yours OTG and mine gadget only) that why I did not face the issue. I remember Balbi suggest me to check hardware to initialize vbus_active. But I am not sure, the proper place to set vbus_active that why I did not implement it. Balbi can help us to decide which is the proper place to set vbus_active :). Thanks > > > > I actually do not have any iMX demoboards at all, I've only got some > > > custom-designed i.MX25 boards where I can't control VBUS, it's > > > permanently pulled up. > > > > > > But I was fixing the problem that was clearly, 100% reproducibly > > > happening when VBUS was applied before the interrupt was configured. > > > So > > > > Wait a minute, are you using OTG. If your VBUS is permanently pulled > > up, that means you are only Gadget(I might miss something) then why you > use OTG mode. > > I'm using FSL_USB2_DR_DEVICE mode (FSL_USB2_PHY_UTMI phy_mode), so the > OTG controller should always work in the device mode only. > > > > what exactly do you mean here? Do you mean this check I've added > > > doesn't fix the bug? Or do you mean this bug should be fixed somehow > > > else? Or do > > > > What expertly my concern is, probe is not proper place to check VBUS > valid. > > I think we should wait to hear what Balbi has to say. > > Yes, I hope he can understand both of us :) > > -- > Be free, use free (http://www.gnu.org/philosophy/free-sw.html) software! > mailto:fercerpav@gmail.com ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] usb: gadget: fsl: check vbus presence on probe 2014-05-09 9:07 ` suresh.gupta @ 2014-05-09 11:18 ` Paul Fertser 0 siblings, 0 replies; 11+ messages in thread From: Paul Fertser @ 2014-05-09 11:18 UTC (permalink / raw) To: suresh.gupta; +Cc: balbi, LeoLi, linux-usb, linux-kernel Hi, On Fri, May 09, 2014 at 09:07:00AM +0000, suresh.gupta@freescale.com wrote: > > Are you really sure we can't get async VBUS state change notifications > > until controller has USB_CMD_RUN_STOP bit set (and the same bit actually > > controls internal 1.5k dataline pullup)? If yes, I guess it means we need > > to check VBUS state _every_ time we set that bit to sync the vbus_active > > variable with the actual hardware state (unless an external OTG PHY is > > used and VBUS pad state is irrelevant)? > > There will be no interrupts if USB_CMD_RUN_STOP is not set but > status get change. I doubt, my previous patch > (252455c40316009cc791f00338ee2e367d2d2739) make any difference in > your device working. Can you please remove my patch and check > either your device work or not. I wasn't using your patch actually as I'm running 3.10 + OpenWrt patches, not current kernel HEAD. Will there be no interrupts without CMD_RUN_STOP as well even if an external PHY/transceiver is used? How do you expect fsl_vbus_session(..., 1) to ever get called then? > We are using two different configs(yours OTG and mine gadget only) > that why I did not face the issue. How do you tell I'm using OTG config and what does it mean given I'm using FSL_USB2_DR_DEVICE? And I'm not using any external OTG transceiver. And if you're using "gadget-only" config (what does it actually mean?) what and when was actually setting vbus_active=1 for you? And BTW, I looked through all the places USB_CMD_RUN_STOP is set and it looks like a real mess. E.g. why do you call dr_controller_run unconditionally (without external transceiver) from fsl_udc_start() and thus set CMD_RUN_STOP in there when afaict the gadget framework expects you to activate the pull up only after fsl_pullup (via usb_gadget_connect) is called? What and when is supposed to initialise interrupts when an external transceiver is used? Was "OTG mode" (which I'm not using afaict) tested ever at all? How can it work given the current code? If it's not possible to track VBUS state changes in "stop" mode why aren't you checking it explicitly every time after entering "run" mode? So far the current fsl_udc_core.c code looks like a broken omap_udc.c copycat to me. Don't you think everything that deals with external PHY should be removed first, then all the init/deinit/start/stop carefully reviewed, restructured, reordered according to the gadget framework requirements, common parts factored out, tested on different hardware, then external transceiver support reintroduced in a clean and obvious manner? Given the history of your patches I see on patchwork, no, you do not think so. I admit I'm not nearly an expert in Linux UDC/gadget framework but your responses so far are confusing me to no end. Balbi, as a stop-gap measure I will try to test and send a v2 patch that would sense VBUS at the end of dr_controller_run(). I should also probably send another one marking the driver BROKEN in Kconfig :/ -- Be free, use free (http://www.gnu.org/philosophy/free-sw.html) software! mailto:fercerpav@gmail.com ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2014-05-09 11:18 UTC | newest] Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2014-04-24 8:54 [PATCH] usb: gadget: fsl: check vbus presence on probe Paul Fertser 2014-04-30 16:06 ` Felipe Balbi 2014-04-30 19:27 ` Paul Fertser 2014-04-30 20:12 ` Felipe Balbi 2014-05-01 14:27 ` suresh.gupta 2014-05-08 14:30 ` suresh.gupta 2014-05-08 15:38 ` Paul Fertser 2014-05-08 18:42 ` suresh.gupta 2014-05-08 20:14 ` Paul Fertser 2014-05-09 9:07 ` suresh.gupta 2014-05-09 11:18 ` Paul Fertser
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).