* [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).