Re: [PATCH v5 3/3] usb: udc: add usb_udc_activation_handler
On Tue, Feb 03, 2015 at 12:04:08PM +0200, Alexander Shishkin wrote:
> Peter Chen writes:
>
> > Currently, connect gadget is unconditional after binding,
> > but some function drivers may want to connect gadget on the fly.
> > With this API, the function driver can disconnect gadget during
> > the initialization, and connect gadget when it wants.
> >
> > During this API, the deactivation count will be update, and it
> > will try to connect or disconnect gadget. It can be used to
> > enable functions for gadget when necessary.
> >
> > Signed-off-by: Peter Chen
> > Acked-by: Alan Stern
> > ---
> > drivers/usb/gadget/udc/udc-core.c | 35 ++-
> > include/linux/usb/gadget.h| 5 +
> > 2 files changed, 39 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/usb/gadget/udc/udc-core.c
> > b/drivers/usb/gadget/udc/udc-core.c
> > index 9396a86..a757334 100644
> > --- a/drivers/usb/gadget/udc/udc-core.c
> > +++ b/drivers/usb/gadget/udc/udc-core.c
> > @@ -37,6 +37,7 @@
> > * @list - for use by the udc class driver
> > * @vbus - for udcs who care about vbus status, this value is real vbus
> > status;
> > * for udcs who do not care about vbus status, this value is always true
> > + * @deactivations - the deactivation count to connect or disconnect gadget
> > *
> > * This represents the internal data structure which is used by the
> > UDC-class
> > * to hold information about udc driver and gadget together.
> > @@ -47,6 +48,7 @@ struct usb_udc {
> > struct device dev;
> > struct list_headlist;
> > boolvbus;
> > + int deactivations;
> > };
> >
> > static struct class *udc_class;
> > @@ -168,13 +170,44 @@ EXPORT_SYMBOL_GPL(usb_gadget_set_state);
> >
> > static void usb_udc_connect_control(struct usb_udc *udc)
> > {
> > - if (udc->vbus)
> > + if (udc->vbus && !udc->deactivations)
> > usb_gadget_connect(udc->gadget);
> > else
> > usb_gadget_disconnect(udc->gadget);
> > }
> >
> > /**
> > + * usb_udc_activation_handler - updates udc's deactivation count and
> > + * try to connect or disconnect
> > + *
> > + * @gadget: The gadget which the function is at
> > + * @active: the function needs to be active or not
> > + *
> > + * The composite core or function driver calls it when it
> > + * wants to activate or deactivate function.
> > + */
> > +void usb_udc_activation_handler(struct usb_gadget *gadget, bool active)
> > +{
> > + struct usb_udc *udc = usb_gadget_find_udc(gadget);
> > +
>
> if (!udc)
> return;
>
> can make this function slightly easier on the eyes.
Will change
>
> > + mutex_lock(&udc_lock);
> > + if (udc) {
> > + if (active) {
> > + if (udc->deactivations == 0) {
> > + WARN_ON(1);
> > + return;
>
> This returns with the udc_lock locked.
oops, will change
> Also, you probably want WARN_ON_ONCE() like so:
>
> if (WARN_ON_ONCE(udc->deactivations)) ...
>
Will change
> Also, I don't think you want udc_lock here at all, it looks like its (at
> least intended) use is to serialize accesses to udc_list. You could use
> an atomic if you want to protect against concurrent (de-)activations,
> but then you would still need to synchronize
> usb_gadget_connect()/usb_gadget_disconnect() that result from these
> activations. I assume that normally these will serialize on gadget
> driver's spinlock in pullup(), so it should be all right.
>
I would like udc-core can make sure deactivations count change
and usb_gadget_connect/usb_gadget_disconnect calling be serial.
--
Best Regards,
Peter Chen
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 3/3] usb: udc: add usb_udc_activation_handler
Peter Chen writes:
> Currently, connect gadget is unconditional after binding,
> but some function drivers may want to connect gadget on the fly.
> With this API, the function driver can disconnect gadget during
> the initialization, and connect gadget when it wants.
>
> During this API, the deactivation count will be update, and it
> will try to connect or disconnect gadget. It can be used to
> enable functions for gadget when necessary.
>
> Signed-off-by: Peter Chen
> Acked-by: Alan Stern
> ---
> drivers/usb/gadget/udc/udc-core.c | 35 ++-
> include/linux/usb/gadget.h| 5 +
> 2 files changed, 39 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/usb/gadget/udc/udc-core.c
> b/drivers/usb/gadget/udc/udc-core.c
> index 9396a86..a757334 100644
> --- a/drivers/usb/gadget/udc/udc-core.c
> +++ b/drivers/usb/gadget/udc/udc-core.c
> @@ -37,6 +37,7 @@
> * @list - for use by the udc class driver
> * @vbus - for udcs who care about vbus status, this value is real vbus
> status;
> * for udcs who do not care about vbus status, this value is always true
> + * @deactivations - the deactivation count to connect or disconnect gadget
> *
> * This represents the internal data structure which is used by the UDC-class
> * to hold information about udc driver and gadget together.
> @@ -47,6 +48,7 @@ struct usb_udc {
> struct device dev;
> struct list_headlist;
> boolvbus;
> + int deactivations;
> };
>
> static struct class *udc_class;
> @@ -168,13 +170,44 @@ EXPORT_SYMBOL_GPL(usb_gadget_set_state);
>
> static void usb_udc_connect_control(struct usb_udc *udc)
> {
> - if (udc->vbus)
> + if (udc->vbus && !udc->deactivations)
> usb_gadget_connect(udc->gadget);
> else
> usb_gadget_disconnect(udc->gadget);
> }
>
> /**
> + * usb_udc_activation_handler - updates udc's deactivation count and
> + * try to connect or disconnect
> + *
> + * @gadget: The gadget which the function is at
> + * @active: the function needs to be active or not
> + *
> + * The composite core or function driver calls it when it
> + * wants to activate or deactivate function.
> + */
> +void usb_udc_activation_handler(struct usb_gadget *gadget, bool active)
> +{
> + struct usb_udc *udc = usb_gadget_find_udc(gadget);
> +
if (!udc)
return;
can make this function slightly easier on the eyes.
> + mutex_lock(&udc_lock);
> + if (udc) {
> + if (active) {
> + if (udc->deactivations == 0) {
> + WARN_ON(1);
> + return;
This returns with the udc_lock locked.
Also, you probably want WARN_ON_ONCE() like so:
if (WARN_ON_ONCE(udc->deactivations)) ...
Also, I don't think you want udc_lock here at all, it looks like its (at
least intended) use is to serialize accesses to udc_list. You could use
an atomic if you want to protect against concurrent (de-)activations,
but then you would still need to synchronize
usb_gadget_connect()/usb_gadget_disconnect() that result from these
activations. I assume that normally these will serialize on gadget
driver's spinlock in pullup(), so it should be all right.
Regards,
--
Alex
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v5 3/3] usb: udc: add usb_udc_activation_handler
Currently, connect gadget is unconditional after binding,
but some function drivers may want to connect gadget on the fly.
With this API, the function driver can disconnect gadget during
the initialization, and connect gadget when it wants.
During this API, the deactivation count will be update, and it
will try to connect or disconnect gadget. It can be used to
enable functions for gadget when necessary.
Signed-off-by: Peter Chen
Acked-by: Alan Stern
---
drivers/usb/gadget/udc/udc-core.c | 35 ++-
include/linux/usb/gadget.h| 5 +
2 files changed, 39 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/gadget/udc/udc-core.c
b/drivers/usb/gadget/udc/udc-core.c
index 9396a86..a757334 100644
--- a/drivers/usb/gadget/udc/udc-core.c
+++ b/drivers/usb/gadget/udc/udc-core.c
@@ -37,6 +37,7 @@
* @list - for use by the udc class driver
* @vbus - for udcs who care about vbus status, this value is real vbus status;
* for udcs who do not care about vbus status, this value is always true
+ * @deactivations - the deactivation count to connect or disconnect gadget
*
* This represents the internal data structure which is used by the UDC-class
* to hold information about udc driver and gadget together.
@@ -47,6 +48,7 @@ struct usb_udc {
struct device dev;
struct list_headlist;
boolvbus;
+ int deactivations;
};
static struct class *udc_class;
@@ -168,13 +170,44 @@ EXPORT_SYMBOL_GPL(usb_gadget_set_state);
static void usb_udc_connect_control(struct usb_udc *udc)
{
- if (udc->vbus)
+ if (udc->vbus && !udc->deactivations)
usb_gadget_connect(udc->gadget);
else
usb_gadget_disconnect(udc->gadget);
}
/**
+ * usb_udc_activation_handler - updates udc's deactivation count and
+ * try to connect or disconnect
+ *
+ * @gadget: The gadget which the function is at
+ * @active: the function needs to be active or not
+ *
+ * The composite core or function driver calls it when it
+ * wants to activate or deactivate function.
+ */
+void usb_udc_activation_handler(struct usb_gadget *gadget, bool active)
+{
+ struct usb_udc *udc = usb_gadget_find_udc(gadget);
+
+ mutex_lock(&udc_lock);
+ if (udc) {
+ if (active) {
+ if (udc->deactivations == 0) {
+ WARN_ON(1);
+ return;
+ }
+ udc->deactivations--;
+ } else {
+ udc->deactivations++;
+ }
+ usb_udc_connect_control(udc);
+ }
+ mutex_unlock(&udc_lock);
+}
+EXPORT_SYMBOL_GPL(usb_udc_activation_handler);
+
+/**
* usb_udc_vbus_handler - updates the udc core vbus status, and try to
* connect or disconnect gadget
* @gadget: The gadget which vbus change occurs
diff --git a/include/linux/usb/gadget.h b/include/linux/usb/gadget.h
index 2be007a..4d1adea 100644
--- a/include/linux/usb/gadget.h
+++ b/include/linux/usb/gadget.h
@@ -1034,6 +1034,11 @@ extern void usb_udc_vbus_handler(struct usb_gadget
*gadget, bool status);
/*-*/
+/* utility to activate or deactive function */
+extern void usb_udc_activation_handler(struct usb_gadget *gadget, bool active);
+
+/*-*/
+
/* utility wrapping a simple endpoint selection policy */
extern struct usb_ep *usb_ep_autoconfig(struct usb_gadget *,
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
