Hi Felipe,
> This commit fixes the way gadget's pullup method (wrapped at
> usb_gadget_connect/disconnect) is called in the udc-core.
>
> The composite driver allows correct driver registration, even when it
> calls the usb_gadget_disconnect method (composite driver is defered
> for further user space configuration - please look into the
> description of usb_composite_probe at composite.c - line: 1621)
>
> One such example is the CCG (Configurable Composite Gadget) driver (at
> drivers/staging/ccg), which after its registration has no usb
> descriptor (i.e. idProduct, idVendor etc.) and functions registered.
> Those are configured after writing to /sys/module/g_ccg/parameters/
> or /sys/class/ccg_usb/ccg0/.
>
> Unfortunately, the code at 'usb_gadget_probe_driver' method (some
> code omitted):
>
> if (udc_is_newstyle(udc)) {
> bind(udc->gadget);
> usb_gadget_udc_start(udc->gadget, driver);
> usb_gadget_connect(udc->gadget);
> }
>
> Explicitly calls the usb_gadget_connect method for this driver. It
> looks that the udc-core enables pullup for a driver, which has no
> functions and no descriptor filled (those values are feed from
> userspace).
>
> The solution (at least until the udc-core is reworked) is to add
> atomic variable, which helps in balancing the number of called
> usb_gadget_connect/ disconnect functions.
>
> Signed-off-by: Lukasz Majewski <l.majewski@xxxxxxxxxxx>
> Signed-off-by: Kyungmin Park <kyungmin.park@xxxxxxxxxxx>
> ---
> drivers/usb/gadget/udc-core.c | 17 ++++++++++++++++-
> include/linux/usb/gadget.h | 13 +++++++++++--
> 2 files changed, 27 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/usb/gadget/udc-core.c
> b/drivers/usb/gadget/udc-core.c index e5e44f8..a26517e 100644
> --- a/drivers/usb/gadget/udc-core.c
> +++ b/drivers/usb/gadget/udc-core.c
> @@ -349,7 +349,22 @@ found:
> }
> usb_gadget_connect(udc->gadget);
> } else {
> -
> + /*
> + * This is a hack for "old style" gadgets:
> + *
> + * The udc_start for "old style" gadgets performs
> implicitly all
> + * operations done by usb_gadget_connect(but not
> calling it).
> + * Therefore non composite gadgets (like rndis)
> works even with
> + * wrong connect_count value ("old style" gadgets
> don't call
> + * usb_gadget_connect/disconnect).
> + *
> + * On the other hand the CCG (Configurable Composite
> Gadget)
> + * requires this incrementation since it calls
> + * usb_gadget_disconnect on its probe (it is
> allowed) and hence
> + * the proper balance is needed when the
> usb_gadget_connect(i.e.
> + * pullup) is called, when triggered from userspace
> event.
> + */
> + atomic_inc(&udc->gadget->connect_count);
> ret = usb_gadget_start(udc->gadget, driver, bind);
> if (ret)
> goto err1;
> diff --git a/include/linux/usb/gadget.h b/include/linux/usb/gadget.h
> index 9517466..0801d83 100644
> --- a/include/linux/usb/gadget.h
> +++ b/include/linux/usb/gadget.h
> @@ -534,6 +534,7 @@ struct usb_gadget {
> unsigned b_hnp_enable:1;
> unsigned a_hnp_support:1;
> unsigned a_alt_hnp_support:1;
> + atomic_t connect_count;
> const char *name;
> struct device dev;
> };
> @@ -739,7 +740,11 @@ static inline int usb_gadget_connect(struct
> usb_gadget *gadget) {
> if (!gadget->ops->pullup)
> return -EOPNOTSUPP;
> - return gadget->ops->pullup(gadget, 1);
> +
> + if (atomic_inc_return(&gadget->connect_count) == 1)
> + return gadget->ops->pullup(gadget, 1);
> +
> + return 0;
> }
>
> /**
> @@ -761,7 +766,11 @@ static inline int usb_gadget_disconnect(struct
> usb_gadget *gadget) {
> if (!gadget->ops->pullup)
> return -EOPNOTSUPP;
> - return gadget->ops->pullup(gadget, 0);
> +
> + if (atomic_dec_and_test(&gadget->connect_count))
> + return gadget->ops->pullup(gadget, 0);
> +
> + return 0;
> }
>
>
Any thoughts about solving this problem?
--
Best regards,
Lukasz Majewski
Samsung Poland R&D Center | Linux Platform Group
--
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
