* [PATCH] chipidea: Fix issue in reconnecing gadget without insmod/rmmod @ 2017-04-11 15:16 Niranjan Dighe 2017-04-11 17:17 ` kbuild test robot ` (2 more replies) 0 siblings, 3 replies; 6+ messages in thread From: Niranjan Dighe @ 2017-04-11 15:16 UTC (permalink / raw) To: Peter.Chen, gregkh, linux-usb, linux-kernel Currently usb_gadget_connect() is called only through gadget registration via composite_driver_probe(). As a result, after a disconnection, if the role transitions to host and back to gadget, the gadget is not recognized by the host anymore. This is a typical scenario with an iAP device in the following usecase - conn iAP dev -> send roleswitch command -> roleswitch ourself to gadget -> disconnect iAP device -> roleswitch back to host to enumerate the device again -> reconnect device -> resend the roleswitch command -> roleswitch ourself to gadget -> ISSUE. To workaround this, do the following - 1. Restart OTG FSM on SLi interrupt and on switching role to gadget so that device transitions to B_IDLE state from B_PERIPHERAL state. A transition from B_IDLE to B_PERIPHERAL is needed to enable interrups and restore the correct state of the chipidea controller so that communication with host is possible 2. usb_gadget_connect() after roleswitch to gadget so that gadget->ops->pullup() is called and D+ line is asserted. This causes host to "see" the device and enumeration can happen. Signed-off-by: Niranjan Dighe <niranjan.dighe@gmail.com> --- drivers/usb/chipidea/debug.c | 10 +++++++++- drivers/usb/chipidea/udc.c | 3 +++ 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/drivers/usb/chipidea/debug.c b/drivers/usb/chipidea/debug.c index a4f7db2..66c1485 100644 --- a/drivers/usb/chipidea/debug.c +++ b/drivers/usb/chipidea/debug.c @@ -16,7 +16,7 @@ #include "udc.h" #include "bits.h" #include "otg.h" - +extern void ci_hdrc_otg_fsm_restart(struct ci_hdrc *); /** * ci_device_show: prints information about device capabilities and status */ @@ -325,6 +325,14 @@ static ssize_t ci_role_write(struct file *file, const char __user *ubuf, ci_role_stop(ci); ret = ci_role_start(ci, role); enable_irq(ci->irq); + + /* REVISIT - Avoid repeated FSM restart*/ + + if (role == CI_ROLE_GADGET) { + ci_hdrc_otg_fsm_restart(ci); + usb_gadget_connect(&ci->gadget); + } + pm_runtime_put_sync(ci->dev); return ret ? ret : count; diff --git a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c index a0accd5..1e3b827 100644 --- a/drivers/usb/chipidea/udc.c +++ b/drivers/usb/chipidea/udc.c @@ -29,6 +29,8 @@ #include "otg.h" #include "otg_fsm.h" +extern void ci_hdrc_otg_fsm_restart(struct ci_hdrc *); + /* control endpoint description */ static const struct usb_endpoint_descriptor ctrl_endpt_out_desc = { @@ -1881,6 +1883,7 @@ static irqreturn_t udc_irq(struct ci_hdrc *ci) ci->driver->suspend(&ci->gadget); usb_gadget_set_state(&ci->gadget, USB_STATE_SUSPENDED); + ci_hdrc_otg_fsm_restart(ci); spin_lock(&ci->lock); } } -- 2.1.4 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] chipidea: Fix issue in reconnecing gadget without insmod/rmmod 2017-04-11 15:16 [PATCH] chipidea: Fix issue in reconnecing gadget without insmod/rmmod Niranjan Dighe @ 2017-04-11 17:17 ` kbuild test robot 2017-04-11 18:22 ` kbuild test robot 2017-04-12 11:51 ` Peter Chen 2 siblings, 0 replies; 6+ messages in thread From: kbuild test robot @ 2017-04-11 17:17 UTC (permalink / raw) To: Niranjan Dighe; +Cc: kbuild-all, Peter.Chen, gregkh, linux-usb, linux-kernel [-- Attachment #1: Type: text/plain, Size: 971 bytes --] Hi Niranjan, [auto build test ERROR on usb/usb-testing] [also build test ERROR on v4.11-rc6] [cannot apply to next-20170411] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] url: https://github.com/0day-ci/linux/commits/Niranjan-Dighe/chipidea-Fix-issue-in-reconnecing-gadget-without-insmod-rmmod/20170411-235845 base: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git usb-testing config: x86_64-acpi-redef (attached as .config) compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901 reproduce: # save the attached .config to linux build tree make ARCH=x86_64 All errors (new ones prefixed by >>): drivers/built-in.o: In function `ci_role_write': >> debug.c:(.text+0x56ca41): undefined reference to `ci_hdrc_otg_fsm_restart' --- 0-DAY kernel test infrastructure Open Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation [-- Attachment #2: .config.gz --] [-- Type: application/gzip, Size: 29540 bytes --] ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] chipidea: Fix issue in reconnecing gadget without insmod/rmmod 2017-04-11 15:16 [PATCH] chipidea: Fix issue in reconnecing gadget without insmod/rmmod Niranjan Dighe 2017-04-11 17:17 ` kbuild test robot @ 2017-04-11 18:22 ` kbuild test robot 2017-04-12 11:51 ` Peter Chen 2 siblings, 0 replies; 6+ messages in thread From: kbuild test robot @ 2017-04-11 18:22 UTC (permalink / raw) To: Niranjan Dighe; +Cc: kbuild-all, Peter.Chen, gregkh, linux-usb, linux-kernel [-- Attachment #1: Type: text/plain, Size: 930 bytes --] Hi Niranjan, [auto build test ERROR on usb/usb-testing] [also build test ERROR on v4.11-rc6] [cannot apply to next-20170411] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] url: https://github.com/0day-ci/linux/commits/Niranjan-Dighe/chipidea-Fix-issue-in-reconnecing-gadget-without-insmod-rmmod/20170411-235845 base: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git usb-testing config: i386-randconfig-x0-04111920 (attached as .config) compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901 reproduce: # save the attached .config to linux build tree make ARCH=i386 All errors (new ones prefixed by >>): >> ERROR: "ci_hdrc_otg_fsm_restart" [drivers/usb/chipidea/ci_hdrc.ko] undefined! --- 0-DAY kernel test infrastructure Open Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation [-- Attachment #2: .config.gz --] [-- Type: application/gzip, Size: 25535 bytes --] ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] chipidea: Fix issue in reconnecing gadget without insmod/rmmod 2017-04-11 15:16 [PATCH] chipidea: Fix issue in reconnecing gadget without insmod/rmmod Niranjan Dighe 2017-04-11 17:17 ` kbuild test robot 2017-04-11 18:22 ` kbuild test robot @ 2017-04-12 11:51 ` Peter Chen 2017-04-12 12:48 ` Niranjan Dighe 2 siblings, 1 reply; 6+ messages in thread From: Peter Chen @ 2017-04-12 11:51 UTC (permalink / raw) To: Niranjan Dighe; +Cc: Peter.Chen, gregkh, linux-usb, linux-kernel On Tue, Apr 11, 2017 at 08:46:24PM +0530, Niranjan Dighe wrote: > Currently usb_gadget_connect() is called only through gadget > registration via composite_driver_probe(). As a result, after a > disconnection, if the role transitions to host and back to gadget, > the gadget is not recognized by the host anymore. > This is a typical scenario with an iAP device in the following > usecase - > conn iAP dev -> send roleswitch command -> roleswitch ourself > to gadget -> disconnect iAP device -> roleswitch back to host to > enumerate the device again -> reconnect device -> resend the > roleswitch command -> roleswitch ourself to gadget -> ISSUE. > > To workaround this, do the following - > > 1. Restart OTG FSM on SLi interrupt and on switching role to gadget > so that device transitions to B_IDLE state from B_PERIPHERAL state. > A transition from B_IDLE to B_PERIPHERAL is needed to enable > interrups and restore the correct state of the chipidea controller > so that communication with host is possible > > 2. usb_gadget_connect() after roleswitch to gadget so that > gadget->ops->pullup() is called and D+ line is asserted. This > causes host to "see" the device and enumeration can happen. > > Signed-off-by: Niranjan Dighe <niranjan.dighe@gmail.com> > --- > drivers/usb/chipidea/debug.c | 10 +++++++++- > drivers/usb/chipidea/udc.c | 3 +++ > 2 files changed, 12 insertions(+), 1 deletion(-) > > diff --git a/drivers/usb/chipidea/debug.c b/drivers/usb/chipidea/debug.c > index a4f7db2..66c1485 100644 > --- a/drivers/usb/chipidea/debug.c > +++ b/drivers/usb/chipidea/debug.c > @@ -16,7 +16,7 @@ > #include "udc.h" > #include "bits.h" > #include "otg.h" > - > +extern void ci_hdrc_otg_fsm_restart(struct ci_hdrc *); > /** > * ci_device_show: prints information about device capabilities and status > */ > @@ -325,6 +325,14 @@ static ssize_t ci_role_write(struct file *file, const char __user *ubuf, > ci_role_stop(ci); > ret = ci_role_start(ci, role); > enable_irq(ci->irq); > + > + /* REVISIT - Avoid repeated FSM restart*/ > + > + if (role == CI_ROLE_GADGET) { > + ci_hdrc_otg_fsm_restart(ci); > + usb_gadget_connect(&ci->gadget); > + } > + > pm_runtime_put_sync(ci->dev); > > return ret ? ret : count; > diff --git a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c > index a0accd5..1e3b827 100644 > --- a/drivers/usb/chipidea/udc.c > +++ b/drivers/usb/chipidea/udc.c > @@ -29,6 +29,8 @@ > #include "otg.h" > #include "otg_fsm.h" > > +extern void ci_hdrc_otg_fsm_restart(struct ci_hdrc *); > + > /* control endpoint description */ > static const struct usb_endpoint_descriptor > ctrl_endpt_out_desc = { > @@ -1881,6 +1883,7 @@ static irqreturn_t udc_irq(struct ci_hdrc *ci) > ci->driver->suspend(&ci->gadget); > usb_gadget_set_state(&ci->gadget, > USB_STATE_SUSPENDED); > + ci_hdrc_otg_fsm_restart(ci); > spin_lock(&ci->lock); > } > } > -- Hi Niranjan, When working with iAP device, there are two role-switch methods - Through OTG FSM, and using sysfs entries under /sys/bus/platform/devices/ci_hdrc.0/inputs but you may need to patch code to keep vbus always on for A-device, it is not compliance with OTG spec. - Using role interface under debugfs (I move it under sysfs for v4.12). But you need to patch the code and let the A-device switch back to host after iAP is disconnected. You don't need to use above two methods together, I suggest using the 2nd method since OTG FSM is hard to maintain due to no mandatory use case for it. -- Best Regards, Peter Chen ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] chipidea: Fix issue in reconnecing gadget without insmod/rmmod 2017-04-12 11:51 ` Peter Chen @ 2017-04-12 12:48 ` Niranjan Dighe 2017-04-14 9:38 ` Peter Chen 0 siblings, 1 reply; 6+ messages in thread From: Niranjan Dighe @ 2017-04-12 12:48 UTC (permalink / raw) To: Peter Chen; +Cc: Peter.Chen, Greg Kroah-Hartman, linux-usb, linux-kernel On Wed, Apr 12, 2017 at 5:21 PM, Peter Chen <hzpeterchen@gmail.com> wrote: > On Tue, Apr 11, 2017 at 08:46:24PM +0530, Niranjan Dighe wrote: >> Currently usb_gadget_connect() is called only through gadget >> registration via composite_driver_probe(). As a result, after a >> disconnection, if the role transitions to host and back to gadget, >> the gadget is not recognized by the host anymore. >> This is a typical scenario with an iAP device in the following >> usecase - >> conn iAP dev -> send roleswitch command -> roleswitch ourself >> to gadget -> disconnect iAP device -> roleswitch back to host to >> enumerate the device again -> reconnect device -> resend the >> roleswitch command -> roleswitch ourself to gadget -> ISSUE. >> >> To workaround this, do the following - >> >> 1. Restart OTG FSM on SLi interrupt and on switching role to gadget >> so that device transitions to B_IDLE state from B_PERIPHERAL state. >> A transition from B_IDLE to B_PERIPHERAL is needed to enable >> interrups and restore the correct state of the chipidea controller >> so that communication with host is possible >> >> 2. usb_gadget_connect() after roleswitch to gadget so that >> gadget->ops->pullup() is called and D+ line is asserted. This >> causes host to "see" the device and enumeration can happen. >> >> Signed-off-by: Niranjan Dighe <niranjan.dighe@gmail.com> >> --- >> drivers/usb/chipidea/debug.c | 10 +++++++++- >> drivers/usb/chipidea/udc.c | 3 +++ >> 2 files changed, 12 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/usb/chipidea/debug.c b/drivers/usb/chipidea/debug.c >> index a4f7db2..66c1485 100644 >> --- a/drivers/usb/chipidea/debug.c >> +++ b/drivers/usb/chipidea/debug.c >> @@ -16,7 +16,7 @@ >> #include "udc.h" >> #include "bits.h" >> #include "otg.h" >> - >> +extern void ci_hdrc_otg_fsm_restart(struct ci_hdrc *); >> /** >> * ci_device_show: prints information about device capabilities and status >> */ >> @@ -325,6 +325,14 @@ static ssize_t ci_role_write(struct file *file, const char __user *ubuf, >> ci_role_stop(ci); >> ret = ci_role_start(ci, role); >> enable_irq(ci->irq); >> + >> + /* REVISIT - Avoid repeated FSM restart*/ >> + >> + if (role == CI_ROLE_GADGET) { >> + ci_hdrc_otg_fsm_restart(ci); >> + usb_gadget_connect(&ci->gadget); >> + } >> + >> pm_runtime_put_sync(ci->dev); >> >> return ret ? ret : count; >> diff --git a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c >> index a0accd5..1e3b827 100644 >> --- a/drivers/usb/chipidea/udc.c >> +++ b/drivers/usb/chipidea/udc.c >> @@ -29,6 +29,8 @@ >> #include "otg.h" >> #include "otg_fsm.h" >> >> +extern void ci_hdrc_otg_fsm_restart(struct ci_hdrc *); >> + >> /* control endpoint description */ >> static const struct usb_endpoint_descriptor >> ctrl_endpt_out_desc = { >> @@ -1881,6 +1883,7 @@ static irqreturn_t udc_irq(struct ci_hdrc *ci) >> ci->driver->suspend(&ci->gadget); >> usb_gadget_set_state(&ci->gadget, >> USB_STATE_SUSPENDED); >> + ci_hdrc_otg_fsm_restart(ci); >> spin_lock(&ci->lock); >> } >> } >> -- > > Hi Niranjan, > > When working with iAP device, there are two role-switch methods > - Through OTG FSM, and using sysfs entries under > /sys/bus/platform/devices/ci_hdrc.0/inputs > but you may need to patch code to keep vbus always on for A-device, > it is not compliance with OTG spec. > - Using role interface under debugfs (I move it under sysfs for v4.12). > But you need to patch the code and let the A-device switch back to > host after iAP is disconnected. > > You don't need to use above two methods together, I suggest using the > 2nd method since OTG FSM is hard to maintain due to no mandatory use > case for it. > > -- > > Best Regards, > Peter Chen Thank you Peter for your response. Yes, I will try to switch the role on disconnection of iAP device to Host. If I understand correctly I have to do something like this - ci_role_stop(ci); //Gadget role stop ret = ci_role_start(ci, role); //Host role start correct? Regards, Niranjan ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] chipidea: Fix issue in reconnecing gadget without insmod/rmmod 2017-04-12 12:48 ` Niranjan Dighe @ 2017-04-14 9:38 ` Peter Chen 0 siblings, 0 replies; 6+ messages in thread From: Peter Chen @ 2017-04-14 9:38 UTC (permalink / raw) To: Niranjan Dighe; +Cc: Peter.Chen, Greg Kroah-Hartman, linux-usb, linux-kernel On Wed, Apr 12, 2017 at 06:18:20PM +0530, Niranjan Dighe wrote: > > Hi Niranjan, > > > > When working with iAP device, there are two role-switch methods > > - Through OTG FSM, and using sysfs entries under > > /sys/bus/platform/devices/ci_hdrc.0/inputs > > but you may need to patch code to keep vbus always on for A-device, > > it is not compliance with OTG spec. > > - Using role interface under debugfs (I move it under sysfs for v4.12). > > But you need to patch the code and let the A-device switch back to > > host after iAP is disconnected. > > > > You don't need to use above two methods together, I suggest using the > > 2nd method since OTG FSM is hard to maintain due to no mandatory use > > case for it. > > > > -- > > > > Best Regards, > > Peter Chen > > Thank you Peter for your response. > > Yes, I will try to switch the role on disconnection of iAP device to Host. If > I understand correctly I have to do something like this - > > ci_role_stop(ci); //Gadget role stop > ret = ci_role_start(ci, role); //Host role start > Applying below patch [1] first. - When your app detects the board needs to switch role to gadget, do echo gadget > /sys/bus/platform/devices/ci_hdrc.0/role - When your app detects the iAP devices is disconnected from the board, do echo host > /sys/bus/platform/devices/ci_hdrc.0/role [1] https://git.kernel.org/pub/scm/linux/kernel/git/peter.chen/usb.git/commit/?h=ci-for-usb-next -- Best Regards, Peter Chen ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2017-04-14 9:39 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2017-04-11 15:16 [PATCH] chipidea: Fix issue in reconnecing gadget without insmod/rmmod Niranjan Dighe 2017-04-11 17:17 ` kbuild test robot 2017-04-11 18:22 ` kbuild test robot 2017-04-12 11:51 ` Peter Chen 2017-04-12 12:48 ` Niranjan Dighe 2017-04-14 9:38 ` Peter Chen
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).