[PATCH 1/2] leds: usb: Add LED trigger for USB gadget activity

[PATCH 1/2] leds: usb: Add LED trigger for USB gadget activity

[Michal Sojka]

With this patch, USB gadget activity can be signaled by blinking a LED.

Since there is no generic code where to put the trigger for all USB
controllers, each USB controller needs to call the trigger individually.
This patch adds the call only for the musb controller where I can test
it.

Signed-off-by: Michal Sojka <sojka@merica.cz>
---
 drivers/leds/trigger/Kconfig             |  8 ++++++
 drivers/leds/trigger/Makefile            |  1 +
 drivers/leds/trigger/ledtrig-usbgadget.c | 45 ++++++++++++++++++++++++++++++++
 drivers/usb/musb/musb_gadget.c           |  6 +++--
 include/linux/leds.h                     |  6 +++++
 5 files changed, 64 insertions(+), 2 deletions(-)
 create mode 100644 drivers/leds/trigger/ledtrig-usbgadget.c

diff --git a/drivers/leds/trigger/Kconfig b/drivers/leds/trigger/Kconfig
index 49794b4..9562963 100644
--- a/drivers/leds/trigger/Kconfig
+++ b/drivers/leds/trigger/Kconfig
@@ -41,6 +41,14 @@ config LEDS_TRIGGER_IDE_DISK
 	  This allows LEDs to be controlled by IDE disk activity.
 	  If unsure, say Y.
 
+config LEDS_TRIGGER_USBGADGET
+	bool "LED USB Gadget Trigger"
+	depends on (USB_MUSB_GADGET || USB_MUSB_DUAL_ROLE)
+	depends on LEDS_TRIGGERS
+	help
+	  This allows LEDs to be controlled by USB gadget activity.
+	  If unsure, say Y.
+
 config LEDS_TRIGGER_HEARTBEAT
 	tristate "LED Heartbeat Trigger"
 	depends on LEDS_TRIGGERS
diff --git a/drivers/leds/trigger/Makefile b/drivers/leds/trigger/Makefile
index 1abf48d..45917c0 100644
--- a/drivers/leds/trigger/Makefile
+++ b/drivers/leds/trigger/Makefile
@@ -8,3 +8,4 @@ obj-$(CONFIG_LEDS_TRIGGER_CPU)		+= ledtrig-cpu.o
 obj-$(CONFIG_LEDS_TRIGGER_DEFAULT_ON)	+= ledtrig-default-on.o
 obj-$(CONFIG_LEDS_TRIGGER_TRANSIENT)	+= ledtrig-transient.o
 obj-$(CONFIG_LEDS_TRIGGER_CAMERA)	+= ledtrig-camera.o
+obj-$(CONFIG_LEDS_TRIGGER_USBGADGET)	+= ledtrig-usbgadget.o
diff --git a/drivers/leds/trigger/ledtrig-usbgadget.c b/drivers/leds/trigger/ledtrig-usbgadget.c
new file mode 100644
index 0000000..1eb90da
--- /dev/null
+++ b/drivers/leds/trigger/ledtrig-usbgadget.c
@@ -0,0 +1,45 @@
+/*
+ * LED Trigger for USB Gadget Activity
+ *
+ * Copyright 2014 Michal Sojka <sojka@merica.cz>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ */
+
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/init.h>
+#include <linux/leds.h>
+
+#define BLINK_DELAY 30
+
+DEFINE_LED_TRIGGER(ledtrig_usbgadget);
+static unsigned long usbgadget_blink_delay = BLINK_DELAY;
+
+void ledtrig_usbgadget_activity(void)
+{
+	led_trigger_blink_oneshot(ledtrig_usbgadget,
+				  &usbgadget_blink_delay, &usbgadget_blink_delay, 0);
+}
+EXPORT_SYMBOL(ledtrig_usbgadget_activity);
+
+static int __init ledtrig_usbgadget_init(void)
+{
+	led_trigger_register_simple("usb-gadget", &ledtrig_usbgadget);
+	return 0;
+}
+
+static void __exit ledtrig_usbgadget_exit(void)
+{
+	led_trigger_unregister_simple(ledtrig_usbgadget);
+}
+
+module_init(ledtrig_usbgadget_init);
+module_exit(ledtrig_usbgadget_exit);
+
+MODULE_AUTHOR("Michal Sojka <sojka@merica.cz>");
+MODULE_DESCRIPTION("LED Trigger for USB Gadget Activity");
+MODULE_LICENSE("GPL");
diff --git a/drivers/usb/musb/musb_gadget.c b/drivers/usb/musb/musb_gadget.c
index d4aa779..98f8b24 100644
--- a/drivers/usb/musb/musb_gadget.c
+++ b/drivers/usb/musb/musb_gadget.c
@@ -42,6 +42,7 @@
 #include <linux/delay.h>
 #include <linux/dma-mapping.h>
 #include <linux/slab.h>
+#include <linux/leds.h>
 
 #include "musb_core.h"
 
@@ -167,11 +168,12 @@ __acquires(ep->musb->lock)
 	if (!dma_mapping_error(&musb->g.dev, request->dma))
 		unmap_dma_buffer(req, musb);
 
-	if (request->status == 0)
+	if (request->status == 0) {
 		dev_dbg(musb->controller, "%s done request %p,  %d/%d\n",
 				ep->end_point.name, request,
 				req->request.actual, req->request.length);
-	else
+		ledtrig_usbgadget_activity();
+	} else
 		dev_dbg(musb->controller, "%s request %p, %d/%d fault %d\n",
 				ep->end_point.name, request,
 				req->request.actual, req->request.length,
diff --git a/include/linux/leds.h b/include/linux/leds.h
index 0287ab2..5d9668e 100644
--- a/include/linux/leds.h
+++ b/include/linux/leds.h
@@ -218,6 +218,12 @@ extern void ledtrig_ide_activity(void);
 static inline void ledtrig_ide_activity(void) {}
 #endif
 
+#ifdef CONFIG_LEDS_TRIGGER_USBGADGET
+extern void ledtrig_usbgadget_activity(void);
+#else
+static inline void ledtrig_usbgadget_activity(void) {}
+#endif
+
 #if defined(CONFIG_LEDS_TRIGGER_CAMERA) || defined(CONFIG_LEDS_TRIGGER_CAMERA_MODULE)
 extern void ledtrig_flash_ctrl(bool on);
 extern void ledtrig_torch_ctrl(bool on);
-- 
2.1.0

[PATCH 2/2] leds: usb: Add LED trigger for USB host activity

[Michal Sojka]

With this patch, USB host activity can be signaled by blinking a LED.

This should work with all host controllers. Tested only with musb.

Signed-off-by: Michal Sojka <sojka@merica.cz>
---
 drivers/leds/trigger/Kconfig           |  8 ++++++
 drivers/leds/trigger/Makefile          |  1 +
 drivers/leds/trigger/ledtrig-usbhost.c | 45 ++++++++++++++++++++++++++++++++++
 drivers/usb/core/hcd.c                 |  3 +++
 include/linux/leds.h                   |  6 +++++
 5 files changed, 63 insertions(+)
 create mode 100644 drivers/leds/trigger/ledtrig-usbhost.c

diff --git a/drivers/leds/trigger/Kconfig b/drivers/leds/trigger/Kconfig
index 9562963..528d736 100644
--- a/drivers/leds/trigger/Kconfig
+++ b/drivers/leds/trigger/Kconfig
@@ -49,6 +49,14 @@ config LEDS_TRIGGER_USBGADGET
 	  This allows LEDs to be controlled by USB gadget activity.
 	  If unsure, say Y.
 
+config LEDS_TRIGGER_USBHOST
+	bool "LED USB Host Trigger"
+	depends on USB
+	depends on LEDS_TRIGGERS
+	help
+	  This allows LEDs to be controlled by USB host activity.
+	  If unsure, say Y.
+
 config LEDS_TRIGGER_HEARTBEAT
 	tristate "LED Heartbeat Trigger"
 	depends on LEDS_TRIGGERS
diff --git a/drivers/leds/trigger/Makefile b/drivers/leds/trigger/Makefile
index 45917c0..9e9ca19 100644
--- a/drivers/leds/trigger/Makefile
+++ b/drivers/leds/trigger/Makefile
@@ -9,3 +9,4 @@ obj-$(CONFIG_LEDS_TRIGGER_DEFAULT_ON)	+= ledtrig-default-on.o
 obj-$(CONFIG_LEDS_TRIGGER_TRANSIENT)	+= ledtrig-transient.o
 obj-$(CONFIG_LEDS_TRIGGER_CAMERA)	+= ledtrig-camera.o
 obj-$(CONFIG_LEDS_TRIGGER_USBGADGET)	+= ledtrig-usbgadget.o
+obj-$(CONFIG_LEDS_TRIGGER_USBHOST)	+= ledtrig-usbhost.o
diff --git a/drivers/leds/trigger/ledtrig-usbhost.c b/drivers/leds/trigger/ledtrig-usbhost.c
new file mode 100644
index 0000000..0a12f67
--- /dev/null
+++ b/drivers/leds/trigger/ledtrig-usbhost.c
@@ -0,0 +1,45 @@
+/*
+ * LED Trigger for USB Host Activity
+ *
+ * Copyright 2014 Michal Sojka <sojka@merica.cz>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ */
+
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/init.h>
+#include <linux/leds.h>
+
+#define BLINK_DELAY 30
+
+DEFINE_LED_TRIGGER(ledtrig_usbhost);
+static unsigned long usbhost_blink_delay = BLINK_DELAY;
+
+void ledtrig_usbhost_activity(void)
+{
+	led_trigger_blink_oneshot(ledtrig_usbhost,
+				  &usbhost_blink_delay, &usbhost_blink_delay, 0);
+}
+EXPORT_SYMBOL(ledtrig_usbhost_activity);
+
+static int __init ledtrig_usbhost_init(void)
+{
+	led_trigger_register_simple("usb-host", &ledtrig_usbhost);
+	return 0;
+}
+
+static void __exit ledtrig_usbhost_exit(void)
+{
+	led_trigger_unregister_simple(ledtrig_usbhost);
+}
+
+module_init(ledtrig_usbhost_init);
+module_exit(ledtrig_usbhost_exit);
+
+MODULE_AUTHOR("Michal Sojka <sojka@merica.cz>");
+MODULE_DESCRIPTION("LED Trigger for USB Host Activity");
+MODULE_LICENSE("GPL");
diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
index 9c4e292..bcfea14 100644
--- a/drivers/usb/core/hcd.c
+++ b/drivers/usb/core/hcd.c
@@ -41,6 +41,7 @@
 #include <linux/workqueue.h>
 #include <linux/pm_runtime.h>
 #include <linux/types.h>
+#include <linux/leds.h>
 
 #include <linux/usb.h>
 #include <linux/usb/hcd.h>
@@ -1664,6 +1665,8 @@ static void __usb_hcd_giveback_urb(struct urb *urb)
 	usbmon_urb_complete(&hcd->self, urb, status);
 	usb_anchor_suspend_wakeups(anchor);
 	usb_unanchor_urb(urb);
+	if (status == 0)
+		ledtrig_usbhost_activity();
 
 	/* pass ownership to the completion handler */
 	urb->status = status;
diff --git a/include/linux/leds.h b/include/linux/leds.h
index 5d9668e..5653495 100644
--- a/include/linux/leds.h
+++ b/include/linux/leds.h
@@ -224,6 +224,12 @@ extern void ledtrig_usbgadget_activity(void);
 static inline void ledtrig_usbgadget_activity(void) {}
 #endif
 
+#ifdef CONFIG_LEDS_TRIGGER_USBHOST
+extern void ledtrig_usbhost_activity(void);
+#else
+static inline void ledtrig_usbhost_activity(void) {}
+#endif
+
 #if defined(CONFIG_LEDS_TRIGGER_CAMERA) || defined(CONFIG_LEDS_TRIGGER_CAMERA_MODULE)
 extern void ledtrig_flash_ctrl(bool on);
 extern void ledtrig_torch_ctrl(bool on);
-- 
2.1.0

Re: [PATCH 1/2] leds: usb: Add LED trigger for USB gadget activity

[Bryan Wu]

On Fri, Aug 22, 2014 at 4:53 AM, Michal Sojka <sojka-Knnw/vAvyUalVyrhU4qvOw@public.gmane.org> wrote:
> With this patch, USB gadget activity can be signaled by blinking a LED.
>
> Since there is no generic code where to put the trigger for all USB
> controllers, each USB controller needs to call the trigger individually.
> This patch adds the call only for the musb controller where I can test
> it.
>

Generally I think one led trigger for both USB host and USB gadget
activity is good enough. We don't need 2 same led trigger here.

And probably you can just put this code in drivers/usb subsystem,
since this driver is quite simple to add to USB subsystem.

Thanks,
-Bryan

> Signed-off-by: Michal Sojka <sojka-Knnw/vAvyUalVyrhU4qvOw@public.gmane.org>
> ---
>  drivers/leds/trigger/Kconfig             |  8 ++++++
>  drivers/leds/trigger/Makefile            |  1 +
>  drivers/leds/trigger/ledtrig-usbgadget.c | 45 ++++++++++++++++++++++++++++++++
>  drivers/usb/musb/musb_gadget.c           |  6 +++--
>  include/linux/leds.h                     |  6 +++++
>  5 files changed, 64 insertions(+), 2 deletions(-)
>  create mode 100644 drivers/leds/trigger/ledtrig-usbgadget.c
>
> diff --git a/drivers/leds/trigger/Kconfig b/drivers/leds/trigger/Kconfig
> index 49794b4..9562963 100644
> --- a/drivers/leds/trigger/Kconfig
> +++ b/drivers/leds/trigger/Kconfig
> @@ -41,6 +41,14 @@ config LEDS_TRIGGER_IDE_DISK
>           This allows LEDs to be controlled by IDE disk activity.
>           If unsure, say Y.
>
> +config LEDS_TRIGGER_USBGADGET
> +       bool "LED USB Gadget Trigger"
> +       depends on (USB_MUSB_GADGET || USB_MUSB_DUAL_ROLE)
> +       depends on LEDS_TRIGGERS
> +       help
> +         This allows LEDs to be controlled by USB gadget activity.
> +         If unsure, say Y.
> +
>  config LEDS_TRIGGER_HEARTBEAT
>         tristate "LED Heartbeat Trigger"
>         depends on LEDS_TRIGGERS
> diff --git a/drivers/leds/trigger/Makefile b/drivers/leds/trigger/Makefile
> index 1abf48d..45917c0 100644
> --- a/drivers/leds/trigger/Makefile
> +++ b/drivers/leds/trigger/Makefile
> @@ -8,3 +8,4 @@ obj-$(CONFIG_LEDS_TRIGGER_CPU)          += ledtrig-cpu.o
>  obj-$(CONFIG_LEDS_TRIGGER_DEFAULT_ON)  += ledtrig-default-on.o
>  obj-$(CONFIG_LEDS_TRIGGER_TRANSIENT)   += ledtrig-transient.o
>  obj-$(CONFIG_LEDS_TRIGGER_CAMERA)      += ledtrig-camera.o
> +obj-$(CONFIG_LEDS_TRIGGER_USBGADGET)   += ledtrig-usbgadget.o
> diff --git a/drivers/leds/trigger/ledtrig-usbgadget.c b/drivers/leds/trigger/ledtrig-usbgadget.c
> new file mode 100644
> index 0000000..1eb90da
> --- /dev/null
> +++ b/drivers/leds/trigger/ledtrig-usbgadget.c
> @@ -0,0 +1,45 @@
> +/*
> + * LED Trigger for USB Gadget Activity
> + *
> + * Copyright 2014 Michal Sojka <sojka-Knnw/vAvyUalVyrhU4qvOw@public.gmane.org>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + */
> +
> +#include <linux/module.h>
> +#include <linux/kernel.h>
> +#include <linux/init.h>
> +#include <linux/leds.h>
> +
> +#define BLINK_DELAY 30
> +
> +DEFINE_LED_TRIGGER(ledtrig_usbgadget);
> +static unsigned long usbgadget_blink_delay = BLINK_DELAY;
> +
> +void ledtrig_usbgadget_activity(void)
> +{
> +       led_trigger_blink_oneshot(ledtrig_usbgadget,
> +                                 &usbgadget_blink_delay, &usbgadget_blink_delay, 0);
> +}
> +EXPORT_SYMBOL(ledtrig_usbgadget_activity);
> +
> +static int __init ledtrig_usbgadget_init(void)
> +{
> +       led_trigger_register_simple("usb-gadget", &ledtrig_usbgadget);
> +       return 0;
> +}
> +
> +static void __exit ledtrig_usbgadget_exit(void)
> +{
> +       led_trigger_unregister_simple(ledtrig_usbgadget);
> +}
> +
> +module_init(ledtrig_usbgadget_init);
> +module_exit(ledtrig_usbgadget_exit);
> +
> +MODULE_AUTHOR("Michal Sojka <sojka-Knnw/vAvyUalVyrhU4qvOw@public.gmane.org>");
> +MODULE_DESCRIPTION("LED Trigger for USB Gadget Activity");
> +MODULE_LICENSE("GPL");
> diff --git a/drivers/usb/musb/musb_gadget.c b/drivers/usb/musb/musb_gadget.c
> index d4aa779..98f8b24 100644
> --- a/drivers/usb/musb/musb_gadget.c
> +++ b/drivers/usb/musb/musb_gadget.c
> @@ -42,6 +42,7 @@
>  #include <linux/delay.h>
>  #include <linux/dma-mapping.h>
>  #include <linux/slab.h>
> +#include <linux/leds.h>
>
>  #include "musb_core.h"
>
> @@ -167,11 +168,12 @@ __acquires(ep->musb->lock)
>         if (!dma_mapping_error(&musb->g.dev, request->dma))
>                 unmap_dma_buffer(req, musb);
>
> -       if (request->status == 0)
> +       if (request->status == 0) {
>                 dev_dbg(musb->controller, "%s done request %p,  %d/%d\n",
>                                 ep->end_point.name, request,
>                                 req->request.actual, req->request.length);
> -       else
> +               ledtrig_usbgadget_activity();
> +       } else
>                 dev_dbg(musb->controller, "%s request %p, %d/%d fault %d\n",
>                                 ep->end_point.name, request,
>                                 req->request.actual, req->request.length,
> diff --git a/include/linux/leds.h b/include/linux/leds.h
> index 0287ab2..5d9668e 100644
> --- a/include/linux/leds.h
> +++ b/include/linux/leds.h
> @@ -218,6 +218,12 @@ extern void ledtrig_ide_activity(void);
>  static inline void ledtrig_ide_activity(void) {}
>  #endif
>
> +#ifdef CONFIG_LEDS_TRIGGER_USBGADGET
> +extern void ledtrig_usbgadget_activity(void);
> +#else
> +static inline void ledtrig_usbgadget_activity(void) {}
> +#endif
> +
>  #if defined(CONFIG_LEDS_TRIGGER_CAMERA) || defined(CONFIG_LEDS_TRIGGER_CAMERA_MODULE)
>  extern void ledtrig_flash_ctrl(bool on);
>  extern void ledtrig_torch_ctrl(bool on);
> --
> 2.1.0
>
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] leds: usb: Add LED trigger for USB gadget activity

[Greg Kroah-Hartman]

On Fri, Aug 22, 2014 at 10:39:03AM -0700, Bryan Wu wrote:
> On Fri, Aug 22, 2014 at 4:53 AM, Michal Sojka <sojka@merica.cz> wrote:
> > With this patch, USB gadget activity can be signaled by blinking a LED.
> >
> > Since there is no generic code where to put the trigger for all USB
> > controllers, each USB controller needs to call the trigger individually.
> > This patch adds the call only for the musb controller where I can test
> > it.
> >
> 
> Generally I think one led trigger for both USB host and USB gadget
> activity is good enough. We don't need 2 same led trigger here.

What about systems that have both running at the same time?  Don't you
want individual control?

> And probably you can just put this code in drivers/usb subsystem,
> since this driver is quite simple to add to USB subsystem.

I have no objection to that, if the LED people don't mind it.

thanks,

greg k-h

Re: [PATCH 1/2] leds: usb: Add LED trigger for USB gadget activity

[Bryan Wu]

On Fri, Aug 22, 2014 at 2:42 PM, Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
> On Fri, Aug 22, 2014 at 10:39:03AM -0700, Bryan Wu wrote:
>> On Fri, Aug 22, 2014 at 4:53 AM, Michal Sojka <sojka@merica.cz> wrote:
>> > With this patch, USB gadget activity can be signaled by blinking a LED.
>> >
>> > Since there is no generic code where to put the trigger for all USB
>> > controllers, each USB controller needs to call the trigger individually.
>> > This patch adds the call only for the musb controller where I can test
>> > it.
>> >
>>
>> Generally I think one led trigger for both USB host and USB gadget
>> activity is good enough. We don't need 2 same led trigger here.
>
> What about systems that have both running at the same time?  Don't you
> want individual control?
>

Actually I wanted to say we don't need 2 same driver for USB host and
USB gadget but one driver which has 2 led triggers like
usb_host_ledtrig and usb_gadget_ledtrig.

I think drivers/net/can/led.c is a good example to start.

>> And probably you can just put this code in drivers/usb subsystem,
>> since this driver is quite simple to add to USB subsystem.
>
> I have no objection to that, if the LED people don't mind it.
>

Because logically it's only used by USB subsystem and it can be a core
component of USB, also drivers/net/can/led.c is a good example.

Thanks,
-Bryan

Re: [PATCH 1/2] leds: usb: Add LED trigger for USB gadget activity

[Michal Sojka]

On Fri, Aug 22 2014, Greg Kroah-Hartman wrote:
> On Fri, Aug 22, 2014 at 10:39:03AM -0700, Bryan Wu wrote:
>> On Fri, Aug 22, 2014 at 4:53 AM, Michal Sojka <sojka@merica.cz> wrote:
>> > With this patch, USB gadget activity can be signaled by blinking a LED.
>> >
>> > Since there is no generic code where to put the trigger for all USB
>> > controllers, each USB controller needs to call the trigger individually.
>> > This patch adds the call only for the musb controller where I can test
>> > it.
>> >
>> 
>> Generally I think one led trigger for both USB host and USB gadget
>> activity is good enough. We don't need 2 same led trigger here.
>
> What about systems that have both running at the same time?  Don't you
> want individual control?

Yes, I have a device with two USB connectors (host, device) and a LED
next to each. From the LEDs it should be clear, which connector is being
used.

Thanks,
-Michal

Re: [PATCH 1/2] leds: usb: Add LED trigger for USB gadget activity

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 4624 bytes --]

Hi,

On Fri, Aug 22, 2014 at 01:53:12PM +0200, Michal Sojka wrote:
> With this patch, USB gadget activity can be signaled by blinking a LED.
> 
> Since there is no generic code where to put the trigger for all USB
> controllers, each USB controller needs to call the trigger individually.
> This patch adds the call only for the musb controller where I can test
> it.
> 
> Signed-off-by: Michal Sojka <sojka@merica.cz>
> ---
>  drivers/leds/trigger/Kconfig             |  8 ++++++
>  drivers/leds/trigger/Makefile            |  1 +
>  drivers/leds/trigger/ledtrig-usbgadget.c | 45 ++++++++++++++++++++++++++++++++
>  drivers/usb/musb/musb_gadget.c           |  6 +++--
>  include/linux/leds.h                     |  6 +++++
>  5 files changed, 64 insertions(+), 2 deletions(-)
>  create mode 100644 drivers/leds/trigger/ledtrig-usbgadget.c
> 
> diff --git a/drivers/leds/trigger/Kconfig b/drivers/leds/trigger/Kconfig
> index 49794b4..9562963 100644
> --- a/drivers/leds/trigger/Kconfig
> +++ b/drivers/leds/trigger/Kconfig
> @@ -41,6 +41,14 @@ config LEDS_TRIGGER_IDE_DISK
>  	  This allows LEDs to be controlled by IDE disk activity.
>  	  If unsure, say Y.
>  
> +config LEDS_TRIGGER_USBGADGET
> +	bool "LED USB Gadget Trigger"
> +	depends on (USB_MUSB_GADGET || USB_MUSB_DUAL_ROLE)
> +	depends on LEDS_TRIGGERS
> +	help
> +	  This allows LEDs to be controlled by USB gadget activity.
> +	  If unsure, say Y.
> +
>  config LEDS_TRIGGER_HEARTBEAT
>  	tristate "LED Heartbeat Trigger"
>  	depends on LEDS_TRIGGERS
> diff --git a/drivers/leds/trigger/Makefile b/drivers/leds/trigger/Makefile
> index 1abf48d..45917c0 100644
> --- a/drivers/leds/trigger/Makefile
> +++ b/drivers/leds/trigger/Makefile
> @@ -8,3 +8,4 @@ obj-$(CONFIG_LEDS_TRIGGER_CPU)		+= ledtrig-cpu.o
>  obj-$(CONFIG_LEDS_TRIGGER_DEFAULT_ON)	+= ledtrig-default-on.o
>  obj-$(CONFIG_LEDS_TRIGGER_TRANSIENT)	+= ledtrig-transient.o
>  obj-$(CONFIG_LEDS_TRIGGER_CAMERA)	+= ledtrig-camera.o
> +obj-$(CONFIG_LEDS_TRIGGER_USBGADGET)	+= ledtrig-usbgadget.o
> diff --git a/drivers/leds/trigger/ledtrig-usbgadget.c b/drivers/leds/trigger/ledtrig-usbgadget.c
> new file mode 100644
> index 0000000..1eb90da
> --- /dev/null
> +++ b/drivers/leds/trigger/ledtrig-usbgadget.c
> @@ -0,0 +1,45 @@
> +/*
> + * LED Trigger for USB Gadget Activity
> + *
> + * Copyright 2014 Michal Sojka <sojka@merica.cz>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + */
> +
> +#include <linux/module.h>
> +#include <linux/kernel.h>
> +#include <linux/init.h>
> +#include <linux/leds.h>
> +
> +#define BLINK_DELAY 30
> +
> +DEFINE_LED_TRIGGER(ledtrig_usbgadget);
> +static unsigned long usbgadget_blink_delay = BLINK_DELAY;
> +
> +void ledtrig_usbgadget_activity(void)
> +{
> +	led_trigger_blink_oneshot(ledtrig_usbgadget,
> +				  &usbgadget_blink_delay, &usbgadget_blink_delay, 0);
> +}
> +EXPORT_SYMBOL(ledtrig_usbgadget_activity);
> +
> +static int __init ledtrig_usbgadget_init(void)
> +{
> +	led_trigger_register_simple("usb-gadget", &ledtrig_usbgadget);
> +	return 0;
> +}
> +
> +static void __exit ledtrig_usbgadget_exit(void)
> +{
> +	led_trigger_unregister_simple(ledtrig_usbgadget);
> +}
> +
> +module_init(ledtrig_usbgadget_init);
> +module_exit(ledtrig_usbgadget_exit);
> +
> +MODULE_AUTHOR("Michal Sojka <sojka@merica.cz>");
> +MODULE_DESCRIPTION("LED Trigger for USB Gadget Activity");
> +MODULE_LICENSE("GPL");
> diff --git a/drivers/usb/musb/musb_gadget.c b/drivers/usb/musb/musb_gadget.c
> index d4aa779..98f8b24 100644
> --- a/drivers/usb/musb/musb_gadget.c
> +++ b/drivers/usb/musb/musb_gadget.c
> @@ -42,6 +42,7 @@
>  #include <linux/delay.h>
>  #include <linux/dma-mapping.h>
>  #include <linux/slab.h>
> +#include <linux/leds.h>
>  
>  #include "musb_core.h"
>  
> @@ -167,11 +168,12 @@ __acquires(ep->musb->lock)
>  	if (!dma_mapping_error(&musb->g.dev, request->dma))
>  		unmap_dma_buffer(req, musb);
>  
> -	if (request->status == 0)
> +	if (request->status == 0) {
>  		dev_dbg(musb->controller, "%s done request %p,  %d/%d\n",
>  				ep->end_point.name, request,
>  				req->request.actual, req->request.length);
> -	else
> +		ledtrig_usbgadget_activity();

looks like this should, somehow, be done at udc-core.c although you'd
need some refactoring to make that happen. It shouldn't be too difficult
to have a generic usb_gadget_giveback_request()

cheers

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH 1/2] leds: usb: Add LED trigger for USB gadget activity

[Michal Sojka]

Hi Felipe,

On Fri, Aug 22 2014, Felipe Balbi wrote:
> Hi,
>
> On Fri, Aug 22, 2014 at 01:53:12PM +0200, Michal Sojka wrote:
>> With this patch, USB gadget activity can be signaled by blinking a LED.
>> 
>> Since there is no generic code where to put the trigger for all USB
>> controllers, each USB controller needs to call the trigger individually.
>> This patch adds the call only for the musb controller where I can test
>> it.
>> 
>> Signed-off-by: Michal Sojka <sojka@merica.cz>
>> ---
>>  drivers/leds/trigger/Kconfig             |  8 ++++++
>>  drivers/leds/trigger/Makefile            |  1 +
>>  drivers/leds/trigger/ledtrig-usbgadget.c | 45 ++++++++++++++++++++++++++++++++
>>  drivers/usb/musb/musb_gadget.c           |  6 +++--
>>  include/linux/leds.h                     |  6 +++++
>>  5 files changed, 64 insertions(+), 2 deletions(-)
>>  create mode 100644 drivers/leds/trigger/ledtrig-usbgadget.c
>> 
>> diff --git a/drivers/leds/trigger/Kconfig b/drivers/leds/trigger/Kconfig
>> index 49794b4..9562963 100644
>> --- a/drivers/leds/trigger/Kconfig
>> +++ b/drivers/leds/trigger/Kconfig
>> @@ -41,6 +41,14 @@ config LEDS_TRIGGER_IDE_DISK
>>  	  This allows LEDs to be controlled by IDE disk activity.
>>  	  If unsure, say Y.
>>  
>> +config LEDS_TRIGGER_USBGADGET
>> +	bool "LED USB Gadget Trigger"
>> +	depends on (USB_MUSB_GADGET || USB_MUSB_DUAL_ROLE)
>> +	depends on LEDS_TRIGGERS
>> +	help
>> +	  This allows LEDs to be controlled by USB gadget activity.
>> +	  If unsure, say Y.
>> +
>>  config LEDS_TRIGGER_HEARTBEAT
>>  	tristate "LED Heartbeat Trigger"
>>  	depends on LEDS_TRIGGERS
>> diff --git a/drivers/leds/trigger/Makefile b/drivers/leds/trigger/Makefile
>> index 1abf48d..45917c0 100644
>> --- a/drivers/leds/trigger/Makefile
>> +++ b/drivers/leds/trigger/Makefile
>> @@ -8,3 +8,4 @@ obj-$(CONFIG_LEDS_TRIGGER_CPU)		+= ledtrig-cpu.o
>>  obj-$(CONFIG_LEDS_TRIGGER_DEFAULT_ON)	+= ledtrig-default-on.o
>>  obj-$(CONFIG_LEDS_TRIGGER_TRANSIENT)	+= ledtrig-transient.o
>>  obj-$(CONFIG_LEDS_TRIGGER_CAMERA)	+= ledtrig-camera.o
>> +obj-$(CONFIG_LEDS_TRIGGER_USBGADGET)	+= ledtrig-usbgadget.o
>> diff --git a/drivers/leds/trigger/ledtrig-usbgadget.c b/drivers/leds/trigger/ledtrig-usbgadget.c
>> new file mode 100644
>> index 0000000..1eb90da
>> --- /dev/null
>> +++ b/drivers/leds/trigger/ledtrig-usbgadget.c
>> @@ -0,0 +1,45 @@
>> +/*
>> + * LED Trigger for USB Gadget Activity
>> + *
>> + * Copyright 2014 Michal Sojka <sojka@merica.cz>
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License version 2 as
>> + * published by the Free Software Foundation.
>> + *
>> + */
>> +
>> +#include <linux/module.h>
>> +#include <linux/kernel.h>
>> +#include <linux/init.h>
>> +#include <linux/leds.h>
>> +
>> +#define BLINK_DELAY 30
>> +
>> +DEFINE_LED_TRIGGER(ledtrig_usbgadget);
>> +static unsigned long usbgadget_blink_delay = BLINK_DELAY;
>> +
>> +void ledtrig_usbgadget_activity(void)
>> +{
>> +	led_trigger_blink_oneshot(ledtrig_usbgadget,
>> +				  &usbgadget_blink_delay, &usbgadget_blink_delay, 0);
>> +}
>> +EXPORT_SYMBOL(ledtrig_usbgadget_activity);
>> +
>> +static int __init ledtrig_usbgadget_init(void)
>> +{
>> +	led_trigger_register_simple("usb-gadget", &ledtrig_usbgadget);
>> +	return 0;
>> +}
>> +
>> +static void __exit ledtrig_usbgadget_exit(void)
>> +{
>> +	led_trigger_unregister_simple(ledtrig_usbgadget);
>> +}
>> +
>> +module_init(ledtrig_usbgadget_init);
>> +module_exit(ledtrig_usbgadget_exit);
>> +
>> +MODULE_AUTHOR("Michal Sojka <sojka@merica.cz>");
>> +MODULE_DESCRIPTION("LED Trigger for USB Gadget Activity");
>> +MODULE_LICENSE("GPL");
>> diff --git a/drivers/usb/musb/musb_gadget.c b/drivers/usb/musb/musb_gadget.c
>> index d4aa779..98f8b24 100644
>> --- a/drivers/usb/musb/musb_gadget.c
>> +++ b/drivers/usb/musb/musb_gadget.c
>> @@ -42,6 +42,7 @@
>>  #include <linux/delay.h>
>>  #include <linux/dma-mapping.h>
>>  #include <linux/slab.h>
>> +#include <linux/leds.h>
>>  
>>  #include "musb_core.h"
>>  
>> @@ -167,11 +168,12 @@ __acquires(ep->musb->lock)
>>  	if (!dma_mapping_error(&musb->g.dev, request->dma))
>>  		unmap_dma_buffer(req, musb);
>>  
>> -	if (request->status == 0)
>> +	if (request->status == 0) {
>>  		dev_dbg(musb->controller, "%s done request %p,  %d/%d\n",
>>  				ep->end_point.name, request,
>>  				req->request.actual, req->request.length);
>> -	else
>> +		ledtrig_usbgadget_activity();
>
> looks like this should, somehow, be done at udc-core.c although you'd
> need some refactoring to make that happen. It shouldn't be too difficult
> to have a generic usb_gadget_giveback_request()

I'm sending a second version, where it is moved to udc directory. I'll
look at refactoring next week.

Thanks,
-Michal

[PATCH v2 0/3] LED triggers for USB host and device

[Michal Sojka]

This adds LED triggers for USB host and device.

Changes from v1:
- Moved from drivers/leds/ to drivers/usb/
- Improved Kconfig help
- Linked with other modules rather than being standalone modules

Michal Sojka (3):
  usb: Add missing #include
  usb: Add LED trigger for USB host activity
  usb: Add LED trigger for USB gadget activity

 drivers/usb/core/Kconfig        |  9 +++++++++
 drivers/usb/core/Makefile       |  1 +
 drivers/usb/core/hcd.c          |  2 ++
 drivers/usb/core/led.c          | 38 ++++++++++++++++++++++++++++++++++++++
 drivers/usb/gadget/Kconfig      | 12 ++++++++++++
 drivers/usb/gadget/udc/Makefile |  5 ++++-
 drivers/usb/gadget/udc/led.c    | 38 ++++++++++++++++++++++++++++++++++++++
 drivers/usb/musb/musb_gadget.c  |  5 +++--
 include/linux/usb/gadget.h      | 10 ++++++++++
 include/linux/usb/hcd.h         |  7 +++++++
 10 files changed, 124 insertions(+), 3 deletions(-)
 create mode 100644 drivers/usb/core/led.c
 create mode 100644 drivers/usb/gadget/udc/led.c

-- 
2.1.0

[PATCH v2 1/3] usb: Add missing #include

[Michal Sojka]

linux/usb/hcd.h needs the definition of struct usb_bus.

Signed-off-by: Michal Sojka <sojka@merica.cz>
---
 include/linux/usb/hcd.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/include/linux/usb/hcd.h b/include/linux/usb/hcd.h
index 485cd5e..b43f0fe 100644
--- a/include/linux/usb/hcd.h
+++ b/include/linux/usb/hcd.h
@@ -23,6 +23,7 @@
 
 #include <linux/rwsem.h>
 #include <linux/interrupt.h>
+#include <linux/usb.h>
 
 #define MAX_TOPO_LEVEL		6
 
-- 
2.1.0

[PATCH v2 2/3] usb: Add LED trigger for USB host activity

[Michal Sojka]

With this patch, USB host activity can be signaled by blinking a LED.

This should work with all host controllers. Tested only with musb.

Signed-off-by: Michal Sojka <sojka@merica.cz>
---
 drivers/usb/core/Kconfig  |  9 +++++++++
 drivers/usb/core/Makefile |  1 +
 drivers/usb/core/hcd.c    |  2 ++
 drivers/usb/core/led.c    | 38 ++++++++++++++++++++++++++++++++++++++
 include/linux/usb/hcd.h   |  6 ++++++
 5 files changed, 56 insertions(+)
 create mode 100644 drivers/usb/core/led.c

diff --git a/drivers/usb/core/Kconfig b/drivers/usb/core/Kconfig
index 1060657..8295f65 100644
--- a/drivers/usb/core/Kconfig
+++ b/drivers/usb/core/Kconfig
@@ -90,3 +90,12 @@ config USB_OTG_FSM
 	  Implements OTG Finite State Machine as specified in On-The-Go
 	  and Embedded Host Supplement to the USB Revision 2.0 Specification.
 
+config USB_HOST_LED
+	bool "USB Host LED Trigger"
+	depends on LEDS_CLASS
+	select LEDS_TRIGGERS
+	help
+	  This option adds a LED trigger for USB host controllers.
+
+	  Say Y here if you are working on a system with led-class supported
+	  LEDs and you want to use them as USB host activity indicators.
diff --git a/drivers/usb/core/Makefile b/drivers/usb/core/Makefile
index 2f6f932..324c8c9 100644
--- a/drivers/usb/core/Makefile
+++ b/drivers/usb/core/Makefile
@@ -9,5 +9,6 @@ usbcore-y += port.o
 
 usbcore-$(CONFIG_PCI)		+= hcd-pci.o
 usbcore-$(CONFIG_ACPI)		+= usb-acpi.o
+usbcore-$(CONFIG_USB_HOST_LED)	+= led.o
 
 obj-$(CONFIG_USB)		+= usbcore.o
diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
index 487abcf..46d9f3a 100644
--- a/drivers/usb/core/hcd.c
+++ b/drivers/usb/core/hcd.c
@@ -1664,6 +1664,8 @@ static void __usb_hcd_giveback_urb(struct urb *urb)
 	usbmon_urb_complete(&hcd->self, urb, status);
 	usb_anchor_suspend_wakeups(anchor);
 	usb_unanchor_urb(urb);
+	if (status == 0)
+		usb_hcd_led_activity();
 
 	/* pass ownership to the completion handler */
 	urb->status = status;
diff --git a/drivers/usb/core/led.c b/drivers/usb/core/led.c
new file mode 100644
index 0000000..49ff76c
--- /dev/null
+++ b/drivers/usb/core/led.c
@@ -0,0 +1,38 @@
+/*
+ * LED Trigger for USB Host Activity
+ *
+ * Copyright 2014 Michal Sojka <sojka@merica.cz>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ */
+
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/init.h>
+#include <linux/leds.h>
+#include <linux/usb/hcd.h>
+
+#define BLINK_DELAY 30
+
+DEFINE_LED_TRIGGER(ledtrig_usb_hcd);
+static unsigned long usb_hcd_blink_delay = BLINK_DELAY;
+
+void usb_hcd_led_activity(void)
+{
+	led_trigger_blink_oneshot(ledtrig_usb_hcd,
+				  &usb_hcd_blink_delay, &usb_hcd_blink_delay, 0);
+}
+
+int __init ledtrig_usb_hcd_init(void)
+{
+	led_trigger_register_simple("usb-host", &ledtrig_usb_hcd);
+	return 0;
+}
+
+void __exit ledtrig_usb_hcd_exit(void)
+{
+	led_trigger_unregister_simple(ledtrig_usb_hcd);
+}
diff --git a/include/linux/usb/hcd.h b/include/linux/usb/hcd.h
index b43f0fe..eb5fa0f 100644
--- a/include/linux/usb/hcd.h
+++ b/include/linux/usb/hcd.h
@@ -700,6 +700,12 @@ extern struct rw_semaphore ehci_cf_port_reset_rwsem;
 #define USB_EHCI_LOADED		2
 extern unsigned long usb_hcds_loaded;
 
+#ifdef CONFIG_USB_HOST_LED
+extern void usb_hcd_led_activity(void);
+#else
+static inline void usb_hcd_led_activity(void) {}
+#endif
+
 #endif /* __KERNEL__ */
 
 #endif /* __USB_CORE_HCD_H */
-- 
2.1.0

Re: [PATCH v2 2/3] usb: Add LED trigger for USB host activity

[Bryan Wu]

On Fri, Aug 22, 2014 at 5:08 PM, Michal Sojka <sojka@merica.cz> wrote:
> With this patch, USB host activity can be signaled by blinking a LED.
>
> This should work with all host controllers. Tested only with musb.
>
> Signed-off-by: Michal Sojka <sojka@merica.cz>
> ---
>  drivers/usb/core/Kconfig  |  9 +++++++++
>  drivers/usb/core/Makefile |  1 +
>  drivers/usb/core/hcd.c    |  2 ++
>  drivers/usb/core/led.c    | 38 ++++++++++++++++++++++++++++++++++++++
>  include/linux/usb/hcd.h   |  6 ++++++
>  5 files changed, 56 insertions(+)
>  create mode 100644 drivers/usb/core/led.c
>
> diff --git a/drivers/usb/core/Kconfig b/drivers/usb/core/Kconfig
> index 1060657..8295f65 100644
> --- a/drivers/usb/core/Kconfig
> +++ b/drivers/usb/core/Kconfig
> @@ -90,3 +90,12 @@ config USB_OTG_FSM
>           Implements OTG Finite State Machine as specified in On-The-Go
>           and Embedded Host Supplement to the USB Revision 2.0 Specification.
>
> +config USB_HOST_LED
> +       bool "USB Host LED Trigger"
> +       depends on LEDS_CLASS
> +       select LEDS_TRIGGERS
> +       help
> +         This option adds a LED trigger for USB host controllers.
> +
> +         Say Y here if you are working on a system with led-class supported
> +         LEDs and you want to use them as USB host activity indicators.
> diff --git a/drivers/usb/core/Makefile b/drivers/usb/core/Makefile
> index 2f6f932..324c8c9 100644
> --- a/drivers/usb/core/Makefile
> +++ b/drivers/usb/core/Makefile
> @@ -9,5 +9,6 @@ usbcore-y += port.o
>
>  usbcore-$(CONFIG_PCI)          += hcd-pci.o
>  usbcore-$(CONFIG_ACPI)         += usb-acpi.o
> +usbcore-$(CONFIG_USB_HOST_LED) += led.o
>
>  obj-$(CONFIG_USB)              += usbcore.o
> diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
> index 487abcf..46d9f3a 100644
> --- a/drivers/usb/core/hcd.c
> +++ b/drivers/usb/core/hcd.c
> @@ -1664,6 +1664,8 @@ static void __usb_hcd_giveback_urb(struct urb *urb)
>         usbmon_urb_complete(&hcd->self, urb, status);
>         usb_anchor_suspend_wakeups(anchor);
>         usb_unanchor_urb(urb);
> +       if (status == 0)
> +               usb_hcd_led_activity();
>
>         /* pass ownership to the completion handler */
>         urb->status = status;
> diff --git a/drivers/usb/core/led.c b/drivers/usb/core/led.c
> new file mode 100644
> index 0000000..49ff76c
> --- /dev/null
> +++ b/drivers/usb/core/led.c
> @@ -0,0 +1,38 @@
> +/*
> + * LED Trigger for USB Host Activity
> + *
> + * Copyright 2014 Michal Sojka <sojka@merica.cz>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + */
> +
> +#include <linux/module.h>
> +#include <linux/kernel.h>
> +#include <linux/init.h>
> +#include <linux/leds.h>
> +#include <linux/usb/hcd.h>
> +
> +#define BLINK_DELAY 30
> +
> +DEFINE_LED_TRIGGER(ledtrig_usb_hcd);

Add one more trigger named ledtrig_usb_gadget

> +static unsigned long usb_hcd_blink_delay = BLINK_DELAY;
> +
> +void usb_hcd_led_activity(void)

Give an input parameter like emum usb_led_event.
USB_LED_EVENT_HOST = 0
USB_LED_EVENT_GADGET = 1


> +{

Add case for USB_LED_EVENT_HOST:
> +       led_trigger_blink_oneshot(ledtrig_usb_hcd,
> +                                 &usb_hcd_blink_delay, &usb_hcd_blink_delay, 0);

Add case for USB_LED_EVENT_GADGET:
 led_trigger_blink_oneshot(ledtrig_usb_gadget,
                             &usb_gadget_blink_delay,
&usb_gadget_blink_delay, 0);

> +}
> +
> +int __init ledtrig_usb_hcd_init(void)
> +{
> +       led_trigger_register_simple("usb-host", &ledtrig_usb_hcd);
register one more trigger for gadget.

> +       return 0;
> +}
> +
> +void __exit ledtrig_usb_hcd_exit(void)
> +{
> +       led_trigger_unregister_simple(ledtrig_usb_hcd);
unregister one more trigger for gadget.

> +}
> diff --git a/include/linux/usb/hcd.h b/include/linux/usb/hcd.h
> index b43f0fe..eb5fa0f 100644
> --- a/include/linux/usb/hcd.h
> +++ b/include/linux/usb/hcd.h
> @@ -700,6 +700,12 @@ extern struct rw_semaphore ehci_cf_port_reset_rwsem;
>  #define USB_EHCI_LOADED                2
>  extern unsigned long usb_hcds_loaded;
>
> +#ifdef CONFIG_USB_HOST_LED
> +extern void usb_hcd_led_activity(void);

User can call usb_led_activity(USB_LED_EVENT_HOST);
or usb_led_activity(USB_LED_EVENT_GADGET);

> +#else
> +static inline void usb_hcd_led_activity(void) {}
> +#endif
> +
>  #endif /* __KERNEL__ */
>
>  #endif /* __USB_CORE_HCD_H */
> --
> 2.1.0
>

Re: [PATCH v2 2/3] usb: Add LED trigger for USB host activity

[Michal Sojka]

Hi Bryan,

thanks for the review. See some comments below.

On Sat, Aug 23 2014, Bryan Wu wrote:
> On Fri, Aug 22, 2014 at 5:08 PM, Michal Sojka <sojka@merica.cz> wrote:
>> With this patch, USB host activity can be signaled by blinking a LED.
>>
>> This should work with all host controllers. Tested only with musb.
>>
>> Signed-off-by: Michal Sojka <sojka@merica.cz>
>> ---
>>  drivers/usb/core/Kconfig  |  9 +++++++++
>>  drivers/usb/core/Makefile |  1 +
>>  drivers/usb/core/hcd.c    |  2 ++
>>  drivers/usb/core/led.c    | 38 ++++++++++++++++++++++++++++++++++++++
>>  include/linux/usb/hcd.h   |  6 ++++++
>>  5 files changed, 56 insertions(+)
>>  create mode 100644 drivers/usb/core/led.c
>>
>> diff --git a/drivers/usb/core/Kconfig b/drivers/usb/core/Kconfig
>> index 1060657..8295f65 100644
>> --- a/drivers/usb/core/Kconfig
>> +++ b/drivers/usb/core/Kconfig
>> @@ -90,3 +90,12 @@ config USB_OTG_FSM
>>           Implements OTG Finite State Machine as specified in On-The-Go
>>           and Embedded Host Supplement to the USB Revision 2.0 Specification.
>>
>> +config USB_HOST_LED
>> +       bool "USB Host LED Trigger"
>> +       depends on LEDS_CLASS
>> +       select LEDS_TRIGGERS
>> +       help
>> +         This option adds a LED trigger for USB host controllers.
>> +
>> +         Say Y here if you are working on a system with led-class supported
>> +         LEDs and you want to use them as USB host activity indicators.
>> diff --git a/drivers/usb/core/Makefile b/drivers/usb/core/Makefile
>> index 2f6f932..324c8c9 100644
>> --- a/drivers/usb/core/Makefile
>> +++ b/drivers/usb/core/Makefile
>> @@ -9,5 +9,6 @@ usbcore-y += port.o
>>
>>  usbcore-$(CONFIG_PCI)          += hcd-pci.o
>>  usbcore-$(CONFIG_ACPI)         += usb-acpi.o
>> +usbcore-$(CONFIG_USB_HOST_LED) += led.o
>>
>>  obj-$(CONFIG_USB)              += usbcore.o
>> diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
>> index 487abcf..46d9f3a 100644
>> --- a/drivers/usb/core/hcd.c
>> +++ b/drivers/usb/core/hcd.c
>> @@ -1664,6 +1664,8 @@ static void __usb_hcd_giveback_urb(struct urb *urb)
>>         usbmon_urb_complete(&hcd->self, urb, status);
>>         usb_anchor_suspend_wakeups(anchor);
>>         usb_unanchor_urb(urb);
>> +       if (status == 0)
>> +               usb_hcd_led_activity();
>>
>>         /* pass ownership to the completion handler */
>>         urb->status = status;
>> diff --git a/drivers/usb/core/led.c b/drivers/usb/core/led.c
>> new file mode 100644
>> index 0000000..49ff76c
>> --- /dev/null
>> +++ b/drivers/usb/core/led.c
>> @@ -0,0 +1,38 @@
>> +/*
>> + * LED Trigger for USB Host Activity
>> + *
>> + * Copyright 2014 Michal Sojka <sojka@merica.cz>
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License version 2 as
>> + * published by the Free Software Foundation.
>> + *
>> + */
>> +
>> +#include <linux/module.h>
>> +#include <linux/kernel.h>
>> +#include <linux/init.h>
>> +#include <linux/leds.h>
>> +#include <linux/usb/hcd.h>
>> +
>> +#define BLINK_DELAY 30
>> +
>> +DEFINE_LED_TRIGGER(ledtrig_usb_hcd);
>
> Add one more trigger named ledtrig_usb_gadget
>
>> +static unsigned long usb_hcd_blink_delay = BLINK_DELAY;
>> +
>> +void usb_hcd_led_activity(void)
>
> Give an input parameter like emum usb_led_event.
> USB_LED_EVENT_HOST = 0
> USB_LED_EVENT_GADGET = 1
>
>
>> +{
>
> Add case for USB_LED_EVENT_HOST:
>> +       led_trigger_blink_oneshot(ledtrig_usb_hcd,
>> +                                 &usb_hcd_blink_delay, &usb_hcd_blink_delay, 0);
>
> Add case for USB_LED_EVENT_GADGET:
>  led_trigger_blink_oneshot(ledtrig_usb_gadget,
>                              &usb_gadget_blink_delay,
> &usb_gadget_blink_delay, 0);
>
>> +}
>> +
>> +int __init ledtrig_usb_hcd_init(void)
>> +{
>> +       led_trigger_register_simple("usb-host", &ledtrig_usb_hcd);
> register one more trigger for gadget.

This way, the code will be full of #ifdefs. Is the following really what
you want? If you want to have it without #ifdefs, then I don't think it
is a good idea to offer users the usb-gadget trigger on systems without
usb gadget support.

#define BLINK_DELAY 30

static unsigned long usb_blink_delay = BLINK_DELAY;

enum usb_led_event {
	USB_LED_EVENT_HOST = 0,
	USB_LED_EVENT_GADGET = 1,
};

#ifdef CONFIG_USB_GADGET_LED
DEFINE_LED_TRIGGER(ledtrig_usbgadget);
#endif
#ifdef CONFIG_USB_HOST_LED
DEFINE_LED_TRIGGER(ledtrig_usb_hcd);
#endif

void usb_led_activity(enum usb_led_event ev)
{
	struct led_trigger *trig;

	switch (ev) {
#ifdef CONFIG_USB_GADGET_LED
	case USB_LED_EVENT_GADGET: trig = ledtrig_usb_gadget; break;
#endif
#ifdef CONFIG_USB_HOST_LED
	case USB_LED_EVENT_HOST:   trig = ledtrig_usb_hcd; break;
#endif
	default:;
	}
	led_trigger_blink_oneshot(trig, &usb_blink_delay, &usb_blink_delay, 0);
}
EXPORT_SYMBOL(usb_led_activity);


int __init ledtrig_usb_init(void)
{
#ifdef CONFIG_USB_GADGET_LED
	led_trigger_register_simple("usb-gadget", &ledtrig_usbgadget);
#endif
#ifdef CONFIG_USB_HOST_LED
	led_trigger_register_simple("usb-host", &ledtrig_usb_hcd);
#endif
	return 0;
}

void __exit ledtrig_usb_exit(void)
{
#ifdef CONFIG_USB_GADGET_LED
	led_trigger_unregister_simple(ledtrig_usbgadget);
#endif
#ifdef CONFIG_USB_HOST_LED
	led_trigger_unregister_simple(ledtrig_usb_hcd);
#endif
}


-Michal

Re: [PATCH v2 2/3] usb: Add LED trigger for USB host activity

[Bryan Wu]

On Sat, Aug 23, 2014 at 2:52 AM, Michal Sojka <sojka-Knnw/vAvyUalVyrhU4qvOw@public.gmane.org> wrote:
> Hi Bryan,
>
> thanks for the review. See some comments below.
>
> On Sat, Aug 23 2014, Bryan Wu wrote:
>> On Fri, Aug 22, 2014 at 5:08 PM, Michal Sojka <sojka-Knnw/vAvyUalVyrhU4qvOw@public.gmane.org> wrote:
>>> With this patch, USB host activity can be signaled by blinking a LED.
>>>
>>> This should work with all host controllers. Tested only with musb.
>>>
>>> Signed-off-by: Michal Sojka <sojka-Knnw/vAvyUalVyrhU4qvOw@public.gmane.org>
>>> ---
>>>  drivers/usb/core/Kconfig  |  9 +++++++++
>>>  drivers/usb/core/Makefile |  1 +
>>>  drivers/usb/core/hcd.c    |  2 ++
>>>  drivers/usb/core/led.c    | 38 ++++++++++++++++++++++++++++++++++++++
>>>  include/linux/usb/hcd.h   |  6 ++++++
>>>  5 files changed, 56 insertions(+)
>>>  create mode 100644 drivers/usb/core/led.c
>>>
>>> diff --git a/drivers/usb/core/Kconfig b/drivers/usb/core/Kconfig
>>> index 1060657..8295f65 100644
>>> --- a/drivers/usb/core/Kconfig
>>> +++ b/drivers/usb/core/Kconfig
>>> @@ -90,3 +90,12 @@ config USB_OTG_FSM
>>>           Implements OTG Finite State Machine as specified in On-The-Go
>>>           and Embedded Host Supplement to the USB Revision 2.0 Specification.
>>>
>>> +config USB_HOST_LED
>>> +       bool "USB Host LED Trigger"
>>> +       depends on LEDS_CLASS
>>> +       select LEDS_TRIGGERS
>>> +       help
>>> +         This option adds a LED trigger for USB host controllers.
>>> +
>>> +         Say Y here if you are working on a system with led-class supported
>>> +         LEDs and you want to use them as USB host activity indicators.
>>> diff --git a/drivers/usb/core/Makefile b/drivers/usb/core/Makefile
>>> index 2f6f932..324c8c9 100644
>>> --- a/drivers/usb/core/Makefile
>>> +++ b/drivers/usb/core/Makefile
>>> @@ -9,5 +9,6 @@ usbcore-y += port.o
>>>
>>>  usbcore-$(CONFIG_PCI)          += hcd-pci.o
>>>  usbcore-$(CONFIG_ACPI)         += usb-acpi.o
>>> +usbcore-$(CONFIG_USB_HOST_LED) += led.o
>>>
>>>  obj-$(CONFIG_USB)              += usbcore.o
>>> diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
>>> index 487abcf..46d9f3a 100644
>>> --- a/drivers/usb/core/hcd.c
>>> +++ b/drivers/usb/core/hcd.c
>>> @@ -1664,6 +1664,8 @@ static void __usb_hcd_giveback_urb(struct urb *urb)
>>>         usbmon_urb_complete(&hcd->self, urb, status);
>>>         usb_anchor_suspend_wakeups(anchor);
>>>         usb_unanchor_urb(urb);
>>> +       if (status == 0)
>>> +               usb_hcd_led_activity();
>>>
>>>         /* pass ownership to the completion handler */
>>>         urb->status = status;
>>> diff --git a/drivers/usb/core/led.c b/drivers/usb/core/led.c
>>> new file mode 100644
>>> index 0000000..49ff76c
>>> --- /dev/null
>>> +++ b/drivers/usb/core/led.c
>>> @@ -0,0 +1,38 @@
>>> +/*
>>> + * LED Trigger for USB Host Activity
>>> + *
>>> + * Copyright 2014 Michal Sojka <sojka-Knnw/vAvyUalVyrhU4qvOw@public.gmane.org>
>>> + *
>>> + * This program is free software; you can redistribute it and/or modify
>>> + * it under the terms of the GNU General Public License version 2 as
>>> + * published by the Free Software Foundation.
>>> + *
>>> + */
>>> +
>>> +#include <linux/module.h>
>>> +#include <linux/kernel.h>
>>> +#include <linux/init.h>
>>> +#include <linux/leds.h>
>>> +#include <linux/usb/hcd.h>
>>> +
>>> +#define BLINK_DELAY 30
>>> +
>>> +DEFINE_LED_TRIGGER(ledtrig_usb_hcd);
>>
>> Add one more trigger named ledtrig_usb_gadget
>>
>>> +static unsigned long usb_hcd_blink_delay = BLINK_DELAY;
>>> +
>>> +void usb_hcd_led_activity(void)
>>
>> Give an input parameter like emum usb_led_event.
>> USB_LED_EVENT_HOST = 0
>> USB_LED_EVENT_GADGET = 1
>>
>>
>>> +{
>>
>> Add case for USB_LED_EVENT_HOST:
>>> +       led_trigger_blink_oneshot(ledtrig_usb_hcd,
>>> +                                 &usb_hcd_blink_delay, &usb_hcd_blink_delay, 0);
>>
>> Add case for USB_LED_EVENT_GADGET:
>>  led_trigger_blink_oneshot(ledtrig_usb_gadget,
>>                              &usb_gadget_blink_delay,
>> &usb_gadget_blink_delay, 0);
>>
>>> +}
>>> +
>>> +int __init ledtrig_usb_hcd_init(void)
>>> +{
>>> +       led_trigger_register_simple("usb-host", &ledtrig_usb_hcd);
>> register one more trigger for gadget.
>
> This way, the code will be full of #ifdefs. Is the following really what
> you want? If you want to have it without #ifdefs, then I don't think it
> is a good idea to offer users the usb-gadget trigger on systems without
> usb gadget support.
>

Why do we need #ifdef here?
We can always define the 2 triggers for both USB host and gadget and
provide API like usb_led_activity().

If USB gadget stack is disabled in kernel, there is no users of this
usb_led_activity(, USB_LED_EVENT_GADGET). We don't need to change
anything in our driver but just waste one trigger instance.

Thanks,
-Bryan

> #define BLINK_DELAY 30
>
> static unsigned long usb_blink_delay = BLINK_DELAY;
>
> enum usb_led_event {
>         USB_LED_EVENT_HOST = 0,
>         USB_LED_EVENT_GADGET = 1,
> };
>
> #ifdef CONFIG_USB_GADGET_LED
> DEFINE_LED_TRIGGER(ledtrig_usbgadget);
> #endif
> #ifdef CONFIG_USB_HOST_LED
> DEFINE_LED_TRIGGER(ledtrig_usb_hcd);
> #endif
>
> void usb_led_activity(enum usb_led_event ev)
> {
>         struct led_trigger *trig;
>
>         switch (ev) {
> #ifdef CONFIG_USB_GADGET_LED
>         case USB_LED_EVENT_GADGET: trig = ledtrig_usb_gadget; break;
> #endif
> #ifdef CONFIG_USB_HOST_LED
>         case USB_LED_EVENT_HOST:   trig = ledtrig_usb_hcd; break;
> #endif
>         default:;
>         }
>         led_trigger_blink_oneshot(trig, &usb_blink_delay, &usb_blink_delay, 0);
> }
> EXPORT_SYMBOL(usb_led_activity);
>
>
> int __init ledtrig_usb_init(void)
> {
> #ifdef CONFIG_USB_GADGET_LED
>         led_trigger_register_simple("usb-gadget", &ledtrig_usbgadget);
> #endif
> #ifdef CONFIG_USB_HOST_LED
>         led_trigger_register_simple("usb-host", &ledtrig_usb_hcd);
> #endif
>         return 0;
> }
>
> void __exit ledtrig_usb_exit(void)
> {
> #ifdef CONFIG_USB_GADGET_LED
>         led_trigger_unregister_simple(ledtrig_usbgadget);
> #endif
> #ifdef CONFIG_USB_HOST_LED
>         led_trigger_unregister_simple(ledtrig_usb_hcd);
> #endif
> }
>
>
> -Michal
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v3 0/2] LED triggers for USB host and device

[Michal Sojka]

This adds LED triggers for USB host and device. First patch refactors
UDC drivers as requested by Felipe Balbi, second patch adds the LED
triggers.

Changes from v2:
- host/gadget triggers merged to a single file in usb/common/
- udc drivers refactored so that LED trigger works for all of them

Changes from v1:
- Moved from drivers/leds/ to drivers/usb/
- Improved Kconfig help
- Linked with other modules rather than being standalone modules

Michal Sojka (2):
  usb: gadget: Refactor request completion
  usb: Add LED triggers for USB activity

 drivers/usb/Kconfig                     |  11 +++
 drivers/usb/chipidea/udc.c              |   6 +-
 drivers/usb/common/Makefile             |   5 +-
 drivers/usb/common/common.c             | 144 ++++++++++++++++++++++++++++++++
 drivers/usb/common/led.c                |  59 +++++++++++++
 drivers/usb/common/usb-common.c         | 144 --------------------------------
 drivers/usb/core/hcd.c                  |   2 +
 drivers/usb/dwc2/gadget.c               |   6 +-
 drivers/usb/dwc3/gadget.c               |   2 +-
 drivers/usb/gadget/udc/amd5536udc.c     |   2 +-
 drivers/usb/gadget/udc/at91_udc.c       |   2 +-
 drivers/usb/gadget/udc/atmel_usba_udc.c |   4 +-
 drivers/usb/gadget/udc/bcm63xx_udc.c    |   2 +-
 drivers/usb/gadget/udc/dummy_hcd.c      |  10 +--
 drivers/usb/gadget/udc/fotg210-udc.c    |   2 +-
 drivers/usb/gadget/udc/fsl_qe_udc.c     |   6 +-
 drivers/usb/gadget/udc/fsl_udc_core.c   |   6 +-
 drivers/usb/gadget/udc/fusb300_udc.c    |   2 +-
 drivers/usb/gadget/udc/goku_udc.c       |   2 +-
 drivers/usb/gadget/udc/gr_udc.c         |   2 +-
 drivers/usb/gadget/udc/lpc32xx_udc.c    |   2 +-
 drivers/usb/gadget/udc/m66592-udc.c     |   2 +-
 drivers/usb/gadget/udc/mv_u3d_core.c    |   8 +-
 drivers/usb/gadget/udc/mv_udc_core.c    |   8 +-
 drivers/usb/gadget/udc/net2272.c        |   2 +-
 drivers/usb/gadget/udc/net2280.c        |   2 +-
 drivers/usb/gadget/udc/omap_udc.c       |   2 +-
 drivers/usb/gadget/udc/pch_udc.c        |   2 +-
 drivers/usb/gadget/udc/pxa25x_udc.c     |   2 +-
 drivers/usb/gadget/udc/pxa27x_udc.c     |   2 +-
 drivers/usb/gadget/udc/r8a66597-udc.c   |   2 +-
 drivers/usb/gadget/udc/s3c-hsudc.c      |   3 +-
 drivers/usb/gadget/udc/s3c2410_udc.c    |   2 +-
 drivers/usb/gadget/udc/udc-core.c       |  13 +++
 drivers/usb/musb/musb_gadget.c          |   2 +-
 drivers/usb/renesas_usbhs/mod_gadget.c  |   2 +-
 include/linux/usb.h                     |  12 +++
 include/linux/usb/gadget.h              |   8 ++
 38 files changed, 294 insertions(+), 201 deletions(-)
 create mode 100644 drivers/usb/common/common.c
 create mode 100644 drivers/usb/common/led.c
 delete mode 100644 drivers/usb/common/usb-common.c

-- 
2.1.0

[PATCH v3 1/2] usb: gadget: Refactor request completion

[Michal Sojka]

All USB peripheral controller drivers called completion routines
directly. This patch moves the completion call from drivers to
usb_gadget_giveback_request(), in order to have a place where common
functionality can be added.

All places in drivers/usb/ matching "[-.]complete(" were replaced with a
call to usb_gadget_giveback_request(). This was compile-tested with all
ARM drivers enabled and runtime-tested for musb.

Signed-off-by: Michal Sojka <sojka@merica.cz>
---
 drivers/usb/chipidea/udc.c              |  6 +++---
 drivers/usb/dwc2/gadget.c               |  6 +++---
 drivers/usb/dwc3/gadget.c               |  2 +-
 drivers/usb/gadget/udc/amd5536udc.c     |  2 +-
 drivers/usb/gadget/udc/at91_udc.c       |  2 +-
 drivers/usb/gadget/udc/atmel_usba_udc.c |  4 ++--
 drivers/usb/gadget/udc/bcm63xx_udc.c    |  2 +-
 drivers/usb/gadget/udc/dummy_hcd.c      | 10 +++++-----
 drivers/usb/gadget/udc/fotg210-udc.c    |  2 +-
 drivers/usb/gadget/udc/fsl_qe_udc.c     |  6 +-----
 drivers/usb/gadget/udc/fsl_udc_core.c   |  6 ++----
 drivers/usb/gadget/udc/fusb300_udc.c    |  2 +-
 drivers/usb/gadget/udc/goku_udc.c       |  2 +-
 drivers/usb/gadget/udc/gr_udc.c         |  2 +-
 drivers/usb/gadget/udc/lpc32xx_udc.c    |  2 +-
 drivers/usb/gadget/udc/m66592-udc.c     |  2 +-
 drivers/usb/gadget/udc/mv_u3d_core.c    |  8 ++------
 drivers/usb/gadget/udc/mv_udc_core.c    |  8 ++------
 drivers/usb/gadget/udc/net2272.c        |  2 +-
 drivers/usb/gadget/udc/net2280.c        |  2 +-
 drivers/usb/gadget/udc/omap_udc.c       |  2 +-
 drivers/usb/gadget/udc/pch_udc.c        |  2 +-
 drivers/usb/gadget/udc/pxa25x_udc.c     |  2 +-
 drivers/usb/gadget/udc/pxa27x_udc.c     |  2 +-
 drivers/usb/gadget/udc/r8a66597-udc.c   |  2 +-
 drivers/usb/gadget/udc/s3c-hsudc.c      |  3 +--
 drivers/usb/gadget/udc/s3c2410_udc.c    |  2 +-
 drivers/usb/gadget/udc/udc-core.c       |  9 +++++++++
 drivers/usb/musb/musb_gadget.c          |  2 +-
 drivers/usb/renesas_usbhs/mod_gadget.c  |  2 +-
 include/linux/usb/gadget.h              |  8 ++++++++
 31 files changed, 58 insertions(+), 56 deletions(-)

diff --git a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c
index b8125aa..0444d3f 100644
--- a/drivers/usb/chipidea/udc.c
+++ b/drivers/usb/chipidea/udc.c
@@ -627,7 +627,7 @@ __acquires(hwep->lock)
 
 		if (hwreq->req.complete != NULL) {
 			spin_unlock(hwep->lock);
-			hwreq->req.complete(&hwep->ep, &hwreq->req);
+			usb_gadget_giveback_request(&hwep->ep, &hwreq->req);
 			spin_lock(hwep->lock);
 		}
 	}
@@ -922,7 +922,7 @@ __acquires(hwep->lock)
 			if ((hwep->type == USB_ENDPOINT_XFER_CONTROL) &&
 					hwreq->req.length)
 				hweptemp = hwep->ci->ep0in;
-			hwreq->req.complete(&hweptemp->ep, &hwreq->req);
+			usb_gadget_giveback_request(&hweptemp->ep, &hwreq->req);
 			spin_lock(hwep->lock);
 		}
 	}
@@ -1347,7 +1347,7 @@ static int ep_dequeue(struct usb_ep *ep, struct usb_request *req)
 
 	if (hwreq->req.complete != NULL) {
 		spin_unlock(hwep->lock);
-		hwreq->req.complete(&hwep->ep, &hwreq->req);
+		usb_gadget_giveback_request(&hwep->ep, &hwreq->req);
 		spin_lock(hwep->lock);
 	}
 
diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
index 0ba9c33..5a524a6 100644
--- a/drivers/usb/dwc2/gadget.c
+++ b/drivers/usb/dwc2/gadget.c
@@ -987,8 +987,8 @@ static int s3c_hsotg_process_req_feature(struct s3c_hsotg *hsotg,
 					hs_req = ep->req;
 					ep->req = NULL;
 					list_del_init(&hs_req->queue);
-					hs_req->req.complete(&ep->ep,
-							     &hs_req->req);
+					usb_gadget_giveback_request(&ep->ep,
+								    &hs_req->req);
 				}
 
 				/* If we have pending request, then start it */
@@ -1245,7 +1245,7 @@ static void s3c_hsotg_complete_request(struct s3c_hsotg *hsotg,
 
 	if (hs_req->req.complete) {
 		spin_unlock(&hsotg->lock);
-		hs_req->req.complete(&hs_ep->ep, &hs_req->req);
+		usb_gadget_giveback_request(&hs_ep->ep, &hs_req->req);
 		spin_lock(&hsotg->lock);
 	}
 
diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index 349cacc..b4b7a6b 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -268,7 +268,7 @@ void dwc3_gadget_giveback(struct dwc3_ep *dep, struct dwc3_request *req,
 			req->request.length, status);
 
 	spin_unlock(&dwc->lock);
-	req->request.complete(&dep->endpoint, &req->request);
+	usb_gadget_giveback_request(&dep->endpoint, &req->request);
 	spin_lock(&dwc->lock);
 }
 
diff --git a/drivers/usb/gadget/udc/amd5536udc.c b/drivers/usb/gadget/udc/amd5536udc.c
index 41b062e..3b9d138 100644
--- a/drivers/usb/gadget/udc/amd5536udc.c
+++ b/drivers/usb/gadget/udc/amd5536udc.c
@@ -841,7 +841,7 @@ __acquires(ep->dev->lock)
 		&req->req, req->req.length, ep->ep.name, sts);
 
 	spin_unlock(&dev->lock);
-	req->req.complete(&ep->ep, &req->req);
+	usb_gadget_giveback_request(&ep->ep, &req->req);
 	spin_lock(&dev->lock);
 	ep->halted = halted;
 }
diff --git a/drivers/usb/gadget/udc/at91_udc.c b/drivers/usb/gadget/udc/at91_udc.c
index cfd18bc..9968f53 100644
--- a/drivers/usb/gadget/udc/at91_udc.c
+++ b/drivers/usb/gadget/udc/at91_udc.c
@@ -267,7 +267,7 @@ static void done(struct at91_ep *ep, struct at91_request *req, int status)
 
 	ep->stopped = 1;
 	spin_unlock(&udc->lock);
-	req->req.complete(&ep->ep, &req->req);
+	usb_gadget_giveback_request(&ep->ep, &req->req);
 	spin_lock(&udc->lock);
 	ep->stopped = stopped;
 
diff --git a/drivers/usb/gadget/udc/atmel_usba_udc.c b/drivers/usb/gadget/udc/atmel_usba_udc.c
index 906e65f..e96032f 100644
--- a/drivers/usb/gadget/udc/atmel_usba_udc.c
+++ b/drivers/usb/gadget/udc/atmel_usba_udc.c
@@ -463,7 +463,7 @@ static void receive_data(struct usba_ep *ep)
 			list_del_init(&req->queue);
 			usba_ep_writel(ep, CTL_DIS, USBA_RX_BK_RDY);
 			spin_unlock(&udc->lock);
-			req->req.complete(&ep->ep, &req->req);
+			usb_gadget_giveback_request(&ep->ep, &req->req);
 			spin_lock(&udc->lock);
 		}
 
@@ -495,7 +495,7 @@ request_complete(struct usba_ep *ep, struct usba_request *req, int status)
 		ep->ep.name, req, req->req.status, req->req.actual);
 
 	spin_unlock(&udc->lock);
-	req->req.complete(&ep->ep, &req->req);
+	usb_gadget_giveback_request(&ep->ep, &req->req);
 	spin_lock(&udc->lock);
 }
 
diff --git a/drivers/usb/gadget/udc/bcm63xx_udc.c b/drivers/usb/gadget/udc/bcm63xx_udc.c
index e969eb8..2235b88 100644
--- a/drivers/usb/gadget/udc/bcm63xx_udc.c
+++ b/drivers/usb/gadget/udc/bcm63xx_udc.c
@@ -1088,7 +1088,7 @@ static int bcm63xx_ep_disable(struct usb_ep *ep)
 			breq->req.status = -ESHUTDOWN;
 
 			spin_unlock_irqrestore(&udc->lock, flags);
-			breq->req.complete(&iudma->bep->ep, &breq->req);
+			usb_gadget_giveback_request(&iudma->bep->ep, &breq->req);
 			spin_lock_irqsave(&udc->lock, flags);
 		}
 	}
diff --git a/drivers/usb/gadget/udc/dummy_hcd.c b/drivers/usb/gadget/udc/dummy_hcd.c
index 2b54955..81dc595 100644
--- a/drivers/usb/gadget/udc/dummy_hcd.c
+++ b/drivers/usb/gadget/udc/dummy_hcd.c
@@ -258,7 +258,7 @@ static void nuke(struct dummy *dum, struct dummy_ep *ep)
 		req->req.status = -ESHUTDOWN;
 
 		spin_unlock(&dum->lock);
-		req->req.complete(&ep->ep, &req->req);
+		usb_gadget_giveback_request(&ep->ep, &req->req);
 		spin_lock(&dum->lock);
 	}
 }
@@ -658,7 +658,7 @@ static int dummy_queue(struct usb_ep *_ep, struct usb_request *_req,
 		spin_unlock(&dum->lock);
 		_req->actual = _req->length;
 		_req->status = 0;
-		_req->complete(_ep, _req);
+		usb_gadget_giveback_request(_ep, _req);
 		spin_lock(&dum->lock);
 	}  else
 		list_add_tail(&req->queue, &ep->queue);
@@ -702,7 +702,7 @@ static int dummy_dequeue(struct usb_ep *_ep, struct usb_request *_req)
 		dev_dbg(udc_dev(dum),
 				"dequeued req %p from %s, len %d buf %p\n",
 				req, _ep->name, _req->length, _req->buf);
-		_req->complete(_ep, _req);
+		usb_gadget_giveback_request(_ep, _req);
 	}
 	local_irq_restore(flags);
 	return retval;
@@ -1385,7 +1385,7 @@ top:
 			list_del_init(&req->queue);
 
 			spin_unlock(&dum->lock);
-			req->req.complete(&ep->ep, &req->req);
+			usb_gadget_giveback_request(&ep->ep, &req->req);
 			spin_lock(&dum->lock);
 
 			/* requests might have been unlinked... */
@@ -1761,7 +1761,7 @@ restart:
 						req);
 
 				spin_unlock(&dum->lock);
-				req->req.complete(&ep->ep, &req->req);
+				usb_gadget_giveback_request(&ep->ep, &req->req);
 				spin_lock(&dum->lock);
 				ep->already_seen = 0;
 				goto restart;
diff --git a/drivers/usb/gadget/udc/fotg210-udc.c b/drivers/usb/gadget/udc/fotg210-udc.c
index e143d69..1d31592 100644
--- a/drivers/usb/gadget/udc/fotg210-udc.c
+++ b/drivers/usb/gadget/udc/fotg210-udc.c
@@ -70,7 +70,7 @@ static void fotg210_done(struct fotg210_ep *ep, struct fotg210_request *req,
 		req->req.status = status;
 
 	spin_unlock(&ep->fotg210->lock);
-	req->req.complete(&ep->ep, &req->req);
+	usb_gadget_giveback_request(&ep->ep, &req->req);
 	spin_lock(&ep->fotg210->lock);
 
 	if (ep->epnum) {
diff --git a/drivers/usb/gadget/udc/fsl_qe_udc.c b/drivers/usb/gadget/udc/fsl_qe_udc.c
index 7324308..dd18ea3 100644
--- a/drivers/usb/gadget/udc/fsl_qe_udc.c
+++ b/drivers/usb/gadget/udc/fsl_qe_udc.c
@@ -118,10 +118,7 @@ static void done(struct qe_ep *ep, struct qe_req *req, int status)
 	ep->stopped = 1;
 	spin_unlock(&udc->lock);
 
-	/* this complete() should a func implemented by gadget layer,
-	 * eg fsg->bulk_in_complete() */
-	if (req->req.complete)
-		req->req.complete(&ep->ep, &req->req);
+	usb_gadget_giveback_request(&ep->ep, &req->req);
 
 	spin_lock(&udc->lock);
 
@@ -2728,4 +2725,3 @@ module_platform_driver(udc_driver);
 MODULE_DESCRIPTION(DRIVER_DESC);
 MODULE_AUTHOR(DRIVER_AUTHOR);
 MODULE_LICENSE("GPL");
-
diff --git a/drivers/usb/gadget/udc/fsl_udc_core.c b/drivers/usb/gadget/udc/fsl_udc_core.c
index 75b23ea..c362079 100644
--- a/drivers/usb/gadget/udc/fsl_udc_core.c
+++ b/drivers/usb/gadget/udc/fsl_udc_core.c
@@ -197,10 +197,8 @@ __acquires(ep->udc->lock)
 	ep->stopped = 1;
 
 	spin_unlock(&ep->udc->lock);
-	/* complete() is from gadget layer,
-	 * eg fsg->bulk_in_complete() */
-	if (req->req.complete)
-		req->req.complete(&ep->ep, &req->req);
+
+	usb_gadget_giveback_request(&ep->ep, &req->req);
 
 	spin_lock(&ep->udc->lock);
 	ep->stopped = stopped;
diff --git a/drivers/usb/gadget/udc/fusb300_udc.c b/drivers/usb/gadget/udc/fusb300_udc.c
index d40255f..d52687d 100644
--- a/drivers/usb/gadget/udc/fusb300_udc.c
+++ b/drivers/usb/gadget/udc/fusb300_udc.c
@@ -876,7 +876,7 @@ static void done(struct fusb300_ep *ep, struct fusb300_request *req,
 		req->req.status = status;
 
 	spin_unlock(&ep->fusb300->lock);
-	req->req.complete(&ep->ep, &req->req);
+	usb_gadget_giveback_request(&ep->ep, &req->req);
 	spin_lock(&ep->fusb300->lock);
 
 	if (ep->epnum) {
diff --git a/drivers/usb/gadget/udc/goku_udc.c b/drivers/usb/gadget/udc/goku_udc.c
index 6c85839..bf9c5ef 100644
--- a/drivers/usb/gadget/udc/goku_udc.c
+++ b/drivers/usb/gadget/udc/goku_udc.c
@@ -320,7 +320,7 @@ done(struct goku_ep *ep, struct goku_request *req, int status)
 	/* don't modify queue heads during completion callback */
 	ep->stopped = 1;
 	spin_unlock(&dev->lock);
-	req->req.complete(&ep->ep, &req->req);
+	usb_gadget_giveback_request(&ep->ep, &req->req);
 	spin_lock(&dev->lock);
 	ep->stopped = stopped;
 }
diff --git a/drivers/usb/gadget/udc/gr_udc.c b/drivers/usb/gadget/udc/gr_udc.c
index 08df5c4..924533b 100644
--- a/drivers/usb/gadget/udc/gr_udc.c
+++ b/drivers/usb/gadget/udc/gr_udc.c
@@ -339,7 +339,7 @@ static void gr_finish_request(struct gr_ep *ep, struct gr_request *req,
 	} else if (req->req.complete) {
 		spin_unlock(&dev->lock);
 
-		req->req.complete(&ep->ep, &req->req);
+		usb_gadget_giveback_request(&ep->ep, &req->req);
 
 		spin_lock(&dev->lock);
 	}
diff --git a/drivers/usb/gadget/udc/lpc32xx_udc.c b/drivers/usb/gadget/udc/lpc32xx_udc.c
index 1629ad7..feab0ba 100644
--- a/drivers/usb/gadget/udc/lpc32xx_udc.c
+++ b/drivers/usb/gadget/udc/lpc32xx_udc.c
@@ -1479,7 +1479,7 @@ static void done(struct lpc32xx_ep *ep, struct lpc32xx_request *req, int status)
 
 	ep->req_pending = 0;
 	spin_unlock(&udc->lock);
-	req->req.complete(&ep->ep, &req->req);
+	usb_gadget_giveback_request(&ep->ep, &req->req);
 	spin_lock(&udc->lock);
 }
 
diff --git a/drivers/usb/gadget/udc/m66592-udc.c b/drivers/usb/gadget/udc/m66592-udc.c
index de88d33..8985656 100644
--- a/drivers/usb/gadget/udc/m66592-udc.c
+++ b/drivers/usb/gadget/udc/m66592-udc.c
@@ -729,7 +729,7 @@ __acquires(m66592->lock)
 		restart = 1;
 
 	spin_unlock(&ep->m66592->lock);
-	req->req.complete(&ep->ep, &req->req);
+	usb_gadget_giveback_request(&ep->ep, &req->req);
 	spin_lock(&ep->m66592->lock);
 
 	if (restart) {
diff --git a/drivers/usb/gadget/udc/mv_u3d_core.c b/drivers/usb/gadget/udc/mv_u3d_core.c
index 1624871..046a1f8 100644
--- a/drivers/usb/gadget/udc/mv_u3d_core.c
+++ b/drivers/usb/gadget/udc/mv_u3d_core.c
@@ -222,12 +222,8 @@ void mv_u3d_done(struct mv_u3d_ep *ep, struct mv_u3d_req *req, int status)
 	}
 
 	spin_unlock(&ep->u3d->lock);
-	/*
-	 * complete() is from gadget layer,
-	 * eg fsg->bulk_in_complete()
-	 */
-	if (req->req.complete)
-		req->req.complete(&ep->ep, &req->req);
+
+	usb_gadget_giveback_request(&ep->ep, &req->req);
 
 	spin_lock(&ep->u3d->lock);
 }
diff --git a/drivers/usb/gadget/udc/mv_udc_core.c b/drivers/usb/gadget/udc/mv_udc_core.c
index 040fb16..3c5db80 100644
--- a/drivers/usb/gadget/udc/mv_udc_core.c
+++ b/drivers/usb/gadget/udc/mv_udc_core.c
@@ -248,12 +248,8 @@ static void done(struct mv_ep *ep, struct mv_req *req, int status)
 	ep->stopped = 1;
 
 	spin_unlock(&ep->udc->lock);
-	/*
-	 * complete() is from gadget layer,
-	 * eg fsg->bulk_in_complete()
-	 */
-	if (req->req.complete)
-		req->req.complete(&ep->ep, &req->req);
+
+	usb_gadget_giveback_request(&ep->ep, &req->req);
 
 	spin_lock(&ep->udc->lock);
 	ep->stopped = stopped;
diff --git a/drivers/usb/gadget/udc/net2272.c b/drivers/usb/gadget/udc/net2272.c
index 059cfe5..84d7162 100644
--- a/drivers/usb/gadget/udc/net2272.c
+++ b/drivers/usb/gadget/udc/net2272.c
@@ -394,7 +394,7 @@ net2272_done(struct net2272_ep *ep, struct net2272_request *req, int status)
 	/* don't modify queue heads during completion callback */
 	ep->stopped = 1;
 	spin_unlock(&dev->lock);
-	req->req.complete(&ep->ep, &req->req);
+	usb_gadget_giveback_request(&ep->ep, &req->req);
 	spin_lock(&dev->lock);
 	ep->stopped = stopped;
 }
diff --git a/drivers/usb/gadget/udc/net2280.c b/drivers/usb/gadget/udc/net2280.c
index f4eac11..2cda166 100644
--- a/drivers/usb/gadget/udc/net2280.c
+++ b/drivers/usb/gadget/udc/net2280.c
@@ -928,7 +928,7 @@ done(struct net2280_ep *ep, struct net2280_request *req, int status)
 	/* don't modify queue heads during completion callback */
 	ep->stopped = 1;
 	spin_unlock(&dev->lock);
-	req->req.complete(&ep->ep, &req->req);
+	usb_gadget_giveback_request(&ep->ep, &req->req);
 	spin_lock(&dev->lock);
 	ep->stopped = stopped;
 }
diff --git a/drivers/usb/gadget/udc/omap_udc.c b/drivers/usb/gadget/udc/omap_udc.c
index e731373..dcdfea4 100644
--- a/drivers/usb/gadget/udc/omap_udc.c
+++ b/drivers/usb/gadget/udc/omap_udc.c
@@ -315,7 +315,7 @@ done(struct omap_ep *ep, struct omap_req *req, int status)
 	/* don't modify queue heads during completion callback */
 	ep->stopped = 1;
 	spin_unlock(&ep->udc->lock);
-	req->req.complete(&ep->ep, &req->req);
+	usb_gadget_giveback_request(&ep->ep, &req->req);
 	spin_lock(&ep->udc->lock);
 	ep->stopped = stopped;
 }
diff --git a/drivers/usb/gadget/udc/pch_udc.c b/drivers/usb/gadget/udc/pch_udc.c
index eb8c3be..0b69b32 100644
--- a/drivers/usb/gadget/udc/pch_udc.c
+++ b/drivers/usb/gadget/udc/pch_udc.c
@@ -1486,7 +1486,7 @@ static void complete_req(struct pch_udc_ep *ep, struct pch_udc_request *req,
 	spin_unlock(&dev->lock);
 	if (!ep->in)
 		pch_udc_ep_clear_rrdy(ep);
-	req->req.complete(&ep->ep, &req->req);
+	usb_gadget_giveback_request(&ep->ep, &req->req);
 	spin_lock(&dev->lock);
 	ep->halted = halted;
 }
diff --git a/drivers/usb/gadget/udc/pxa25x_udc.c b/drivers/usb/gadget/udc/pxa25x_udc.c
index 251e4d5..42f7eeb 100644
--- a/drivers/usb/gadget/udc/pxa25x_udc.c
+++ b/drivers/usb/gadget/udc/pxa25x_udc.c
@@ -347,7 +347,7 @@ static void done(struct pxa25x_ep *ep, struct pxa25x_request *req, int status)
 
 	/* don't modify queue heads during completion callback */
 	ep->stopped = 1;
-	req->req.complete(&ep->ep, &req->req);
+	usb_gadget_giveback_request(&ep->ep, &req->req);
 	ep->stopped = stopped;
 }
 
diff --git a/drivers/usb/gadget/udc/pxa27x_udc.c b/drivers/usb/gadget/udc/pxa27x_udc.c
index 597d39f..4868369 100644
--- a/drivers/usb/gadget/udc/pxa27x_udc.c
+++ b/drivers/usb/gadget/udc/pxa27x_udc.c
@@ -758,7 +758,7 @@ static void req_done(struct pxa_ep *ep, struct pxa27x_request *req, int status,
 	if (pflags)
 		spin_unlock_irqrestore(&ep->lock, *pflags);
 	local_irq_save(flags);
-	req->req.complete(&req->udc_usb_ep->usb_ep, &req->req);
+	usb_gadget_giveback_request(&req->udc_usb_ep->usb_ep, &req->req);
 	local_irq_restore(flags);
 	if (pflags)
 		spin_lock_irqsave(&ep->lock, *pflags);
diff --git a/drivers/usb/gadget/udc/r8a66597-udc.c b/drivers/usb/gadget/udc/r8a66597-udc.c
index 4600842..0ec09a4 100644
--- a/drivers/usb/gadget/udc/r8a66597-udc.c
+++ b/drivers/usb/gadget/udc/r8a66597-udc.c
@@ -925,7 +925,7 @@ __acquires(r8a66597->lock)
 		sudmac_free_channel(ep->r8a66597, ep, req);
 
 	spin_unlock(&ep->r8a66597->lock);
-	req->req.complete(&ep->ep, &req->req);
+	usb_gadget_giveback_request(&ep->ep, &req->req);
 	spin_lock(&ep->r8a66597->lock);
 
 	if (restart) {
diff --git a/drivers/usb/gadget/udc/s3c-hsudc.c b/drivers/usb/gadget/udc/s3c-hsudc.c
index 10c6a12..dfbf557 100644
--- a/drivers/usb/gadget/udc/s3c-hsudc.c
+++ b/drivers/usb/gadget/udc/s3c-hsudc.c
@@ -258,8 +258,7 @@ static void s3c_hsudc_complete_request(struct s3c_hsudc_ep *hsep,
 
 	hsep->stopped = 1;
 	spin_unlock(&hsudc->lock);
-	if (hsreq->req.complete != NULL)
-		hsreq->req.complete(&hsep->ep, &hsreq->req);
+	usb_gadget_giveback_request(&hsep->ep, &hsreq->req);
 	spin_lock(&hsudc->lock);
 	hsep->stopped = stopped;
 }
diff --git a/drivers/usb/gadget/udc/s3c2410_udc.c b/drivers/usb/gadget/udc/s3c2410_udc.c
index 357b58e..ff423d1 100644
--- a/drivers/usb/gadget/udc/s3c2410_udc.c
+++ b/drivers/usb/gadget/udc/s3c2410_udc.c
@@ -272,7 +272,7 @@ static void s3c2410_udc_done(struct s3c2410_ep *ep,
 		status = req->req.status;
 
 	ep->halted = 1;
-	req->req.complete(&ep->ep, &req->req);
+	usb_gadget_giveback_request(&ep->ep, &req->req);
 	ep->halted = halted;
 }
 
diff --git a/drivers/usb/gadget/udc/udc-core.c b/drivers/usb/gadget/udc/udc-core.c
index b0d9817..2eb0ae4 100644
--- a/drivers/usb/gadget/udc/udc-core.c
+++ b/drivers/usb/gadget/udc/udc-core.c
@@ -102,6 +102,15 @@ void usb_gadget_unmap_request(struct usb_gadget *gadget,
 }
 EXPORT_SYMBOL_GPL(usb_gadget_unmap_request);
 
+void usb_gadget_giveback_request(struct usb_ep *ep,
+		struct usb_request *req)
+{
+	/* complete() is from gadget layer,
+	 * eg fsg->bulk_in_complete() */
+	if (req->complete)
+		req->complete(ep, req);
+}
+EXPORT_SYMBOL_GPL(usb_gadget_giveback_request);
 #endif	/* CONFIG_HAS_DMA */
 
 /* ------------------------------------------------------------------------- */
diff --git a/drivers/usb/musb/musb_gadget.c b/drivers/usb/musb/musb_gadget.c
index d4aa779..24c8c02 100644
--- a/drivers/usb/musb/musb_gadget.c
+++ b/drivers/usb/musb/musb_gadget.c
@@ -176,7 +176,7 @@ __acquires(ep->musb->lock)
 				ep->end_point.name, request,
 				req->request.actual, req->request.length,
 				request->status);
-	req->request.complete(&req->ep->end_point, &req->request);
+	usb_gadget_giveback_request(&req->ep->end_point, &req->request);
 	spin_lock(&musb->lock);
 	ep->busy = busy;
 }
diff --git a/drivers/usb/renesas_usbhs/mod_gadget.c b/drivers/usb/renesas_usbhs/mod_gadget.c
index 04e6505..2d17c10 100644
--- a/drivers/usb/renesas_usbhs/mod_gadget.c
+++ b/drivers/usb/renesas_usbhs/mod_gadget.c
@@ -129,7 +129,7 @@ static void usbhsg_queue_pop(struct usbhsg_uep *uep,
 	dev_dbg(dev, "pipe %d : queue pop\n", usbhs_pipe_number(pipe));
 
 	ureq->req.status = status;
-	ureq->req.complete(&uep->ep, &ureq->req);
+	usb_gadget_giveback_request(&uep->ep, &ureq->req);
 }
 
 static void usbhsg_queue_done(struct usbhs_priv *priv, struct usbhs_pkt *pkt)
diff --git a/include/linux/usb/gadget.h b/include/linux/usb/gadget.h
index c3a6185..f795e95 100644
--- a/include/linux/usb/gadget.h
+++ b/include/linux/usb/gadget.h
@@ -1013,6 +1013,14 @@ extern void usb_gadget_set_state(struct usb_gadget *gadget,
 
 /*-------------------------------------------------------------------------*/
 
+/* utility to give requests back to the gadget layer */
+
+extern void usb_gadget_giveback_request(struct usb_ep *ep,
+		struct usb_request *req);
+
+
+/*-------------------------------------------------------------------------*/
+
 /* utility wrapping a simple endpoint selection policy */
 
 extern struct usb_ep *usb_ep_autoconfig(struct usb_gadget *,
-- 
2.1.0

Re: [PATCH v3 1/2] usb: gadget: Refactor request completion

[Michal Sojka]

On Wed, Aug 27 2014, Michal Sojka wrote:
> All USB peripheral controller drivers called completion routines
> directly. This patch moves the completion call from drivers to
> usb_gadget_giveback_request(), in order to have a place where common
> functionality can be added.
>
> All places in drivers/usb/ matching "[-.]complete(" were replaced with a
> call to usb_gadget_giveback_request(). This was compile-tested with all
> ARM drivers enabled and runtime-tested for musb.

[...]

> diff --git a/drivers/usb/gadget/udc/udc-core.c b/drivers/usb/gadget/udc/udc-core.c
> index b0d9817..2eb0ae4 100644
> --- a/drivers/usb/gadget/udc/udc-core.c
> +++ b/drivers/usb/gadget/udc/udc-core.c
> @@ -102,6 +102,15 @@ void usb_gadget_unmap_request(struct usb_gadget *gadget,
>  }
>  EXPORT_SYMBOL_GPL(usb_gadget_unmap_request);
>  
> +void usb_gadget_giveback_request(struct usb_ep *ep,
> +		struct usb_request *req)
> +{
> +	/* complete() is from gadget layer,
> +	 * eg fsg->bulk_in_complete() */
> +	if (req->complete)
> +		req->complete(ep, req);
> +}
> +EXPORT_SYMBOL_GPL(usb_gadget_giveback_request);
>  #endif	/* CONFIG_HAS_DMA */

#endif should be before usb_gadget_giveback_request(). I'll fix this in
v4.

-Michal

Re: [PATCH v3 1/2] usb: gadget: Refactor request completion

[Alan Stern]

On Wed, 27 Aug 2014, Michal Sojka wrote:

> All USB peripheral controller drivers called completion routines
> directly. This patch moves the completion call from drivers to
> usb_gadget_giveback_request(), in order to have a place where common
> functionality can be added.
> 
> All places in drivers/usb/ matching "[-.]complete(" were replaced with a
> call to usb_gadget_giveback_request(). This was compile-tested with all
> ARM drivers enabled and runtime-tested for musb.


> --- a/drivers/usb/gadget/udc/udc-core.c
> +++ b/drivers/usb/gadget/udc/udc-core.c
> @@ -102,6 +102,15 @@ void usb_gadget_unmap_request(struct usb_gadget *gadget,
>  }
>  EXPORT_SYMBOL_GPL(usb_gadget_unmap_request);
>  
> +void usb_gadget_giveback_request(struct usb_ep *ep,
> +		struct usb_request *req)
> +{
> +	/* complete() is from gadget layer,
> +	 * eg fsg->bulk_in_complete() */

Wrong format for a multi-line comment.  It should look like this:

	/*
	 * Blah, blah, blah
	 * blah, blah, blah
	 */

Also, the reference to fsg is obsolete; the File-backed Storage Gadget 
is no longer in the kernel.  You might as well leave the comment out 
entirely.

> +	if (req->complete)

This test shouldn't be here.  The UDC drivers don't do it, but they
probably do check the req->complete isn't NULL when the request is
submitted.  In any case, if req->complete is NULL then we want to know
about it because it is a bug.  It should not be covered up silently.

> +		req->complete(ep, req);
> +}
> +EXPORT_SYMBOL_GPL(usb_gadget_giveback_request);
>  #endif	/* CONFIG_HAS_DMA */

Alan Stern

Re: [PATCH v3 1/2] usb: gadget: Refactor request completion

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 20506 bytes --]

On Wed, Aug 27, 2014 at 03:03:44PM +0200, Michal Sojka wrote:
> All USB peripheral controller drivers called completion routines
> directly. This patch moves the completion call from drivers to
> usb_gadget_giveback_request(), in order to have a place where common
> functionality can be added.
> 
> All places in drivers/usb/ matching "[-.]complete(" were replaced with a
> call to usb_gadget_giveback_request(). This was compile-tested with all
> ARM drivers enabled and runtime-tested for musb.
> 
> Signed-off-by: Michal Sojka <sojka-Knnw/vAvyUalVyrhU4qvOw@public.gmane.org>
> ---
>  drivers/usb/chipidea/udc.c              |  6 +++---
>  drivers/usb/dwc2/gadget.c               |  6 +++---
>  drivers/usb/dwc3/gadget.c               |  2 +-
>  drivers/usb/gadget/udc/amd5536udc.c     |  2 +-
>  drivers/usb/gadget/udc/at91_udc.c       |  2 +-
>  drivers/usb/gadget/udc/atmel_usba_udc.c |  4 ++--
>  drivers/usb/gadget/udc/bcm63xx_udc.c    |  2 +-
>  drivers/usb/gadget/udc/dummy_hcd.c      | 10 +++++-----
>  drivers/usb/gadget/udc/fotg210-udc.c    |  2 +-
>  drivers/usb/gadget/udc/fsl_qe_udc.c     |  6 +-----
>  drivers/usb/gadget/udc/fsl_udc_core.c   |  6 ++----
>  drivers/usb/gadget/udc/fusb300_udc.c    |  2 +-
>  drivers/usb/gadget/udc/goku_udc.c       |  2 +-
>  drivers/usb/gadget/udc/gr_udc.c         |  2 +-
>  drivers/usb/gadget/udc/lpc32xx_udc.c    |  2 +-
>  drivers/usb/gadget/udc/m66592-udc.c     |  2 +-
>  drivers/usb/gadget/udc/mv_u3d_core.c    |  8 ++------
>  drivers/usb/gadget/udc/mv_udc_core.c    |  8 ++------
>  drivers/usb/gadget/udc/net2272.c        |  2 +-
>  drivers/usb/gadget/udc/net2280.c        |  2 +-
>  drivers/usb/gadget/udc/omap_udc.c       |  2 +-
>  drivers/usb/gadget/udc/pch_udc.c        |  2 +-
>  drivers/usb/gadget/udc/pxa25x_udc.c     |  2 +-
>  drivers/usb/gadget/udc/pxa27x_udc.c     |  2 +-
>  drivers/usb/gadget/udc/r8a66597-udc.c   |  2 +-
>  drivers/usb/gadget/udc/s3c-hsudc.c      |  3 +--
>  drivers/usb/gadget/udc/s3c2410_udc.c    |  2 +-
>  drivers/usb/gadget/udc/udc-core.c       |  9 +++++++++
>  drivers/usb/musb/musb_gadget.c          |  2 +-
>  drivers/usb/renesas_usbhs/mod_gadget.c  |  2 +-
>  include/linux/usb/gadget.h              |  8 ++++++++
>  31 files changed, 58 insertions(+), 56 deletions(-)
> 
> diff --git a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c
> index b8125aa..0444d3f 100644
> --- a/drivers/usb/chipidea/udc.c
> +++ b/drivers/usb/chipidea/udc.c
> @@ -627,7 +627,7 @@ __acquires(hwep->lock)
>  
>  		if (hwreq->req.complete != NULL) {
>  			spin_unlock(hwep->lock);
> -			hwreq->req.complete(&hwep->ep, &hwreq->req);
> +			usb_gadget_giveback_request(&hwep->ep, &hwreq->req);
>  			spin_lock(hwep->lock);
>  		}
>  	}
> @@ -922,7 +922,7 @@ __acquires(hwep->lock)
>  			if ((hwep->type == USB_ENDPOINT_XFER_CONTROL) &&
>  					hwreq->req.length)
>  				hweptemp = hwep->ci->ep0in;
> -			hwreq->req.complete(&hweptemp->ep, &hwreq->req);
> +			usb_gadget_giveback_request(&hweptemp->ep, &hwreq->req);
>  			spin_lock(hwep->lock);
>  		}
>  	}
> @@ -1347,7 +1347,7 @@ static int ep_dequeue(struct usb_ep *ep, struct usb_request *req)
>  
>  	if (hwreq->req.complete != NULL) {
>  		spin_unlock(hwep->lock);
> -		hwreq->req.complete(&hwep->ep, &hwreq->req);
> +		usb_gadget_giveback_request(&hwep->ep, &hwreq->req);
>  		spin_lock(hwep->lock);
>  	}
>  
> diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
> index 0ba9c33..5a524a6 100644
> --- a/drivers/usb/dwc2/gadget.c
> +++ b/drivers/usb/dwc2/gadget.c
> @@ -987,8 +987,8 @@ static int s3c_hsotg_process_req_feature(struct s3c_hsotg *hsotg,
>  					hs_req = ep->req;
>  					ep->req = NULL;
>  					list_del_init(&hs_req->queue);
> -					hs_req->req.complete(&ep->ep,
> -							     &hs_req->req);
> +					usb_gadget_giveback_request(&ep->ep,
> +								    &hs_req->req);
>  				}
>  
>  				/* If we have pending request, then start it */
> @@ -1245,7 +1245,7 @@ static void s3c_hsotg_complete_request(struct s3c_hsotg *hsotg,
>  
>  	if (hs_req->req.complete) {
>  		spin_unlock(&hsotg->lock);
> -		hs_req->req.complete(&hs_ep->ep, &hs_req->req);
> +		usb_gadget_giveback_request(&hs_ep->ep, &hs_req->req);
>  		spin_lock(&hsotg->lock);
>  	}
>  
> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
> index 349cacc..b4b7a6b 100644
> --- a/drivers/usb/dwc3/gadget.c
> +++ b/drivers/usb/dwc3/gadget.c
> @@ -268,7 +268,7 @@ void dwc3_gadget_giveback(struct dwc3_ep *dep, struct dwc3_request *req,
>  			req->request.length, status);
>  
>  	spin_unlock(&dwc->lock);
> -	req->request.complete(&dep->endpoint, &req->request);
> +	usb_gadget_giveback_request(&dep->endpoint, &req->request);
>  	spin_lock(&dwc->lock);
>  }
>  
> diff --git a/drivers/usb/gadget/udc/amd5536udc.c b/drivers/usb/gadget/udc/amd5536udc.c
> index 41b062e..3b9d138 100644
> --- a/drivers/usb/gadget/udc/amd5536udc.c
> +++ b/drivers/usb/gadget/udc/amd5536udc.c
> @@ -841,7 +841,7 @@ __acquires(ep->dev->lock)
>  		&req->req, req->req.length, ep->ep.name, sts);
>  
>  	spin_unlock(&dev->lock);
> -	req->req.complete(&ep->ep, &req->req);
> +	usb_gadget_giveback_request(&ep->ep, &req->req);
>  	spin_lock(&dev->lock);
>  	ep->halted = halted;
>  }
> diff --git a/drivers/usb/gadget/udc/at91_udc.c b/drivers/usb/gadget/udc/at91_udc.c
> index cfd18bc..9968f53 100644
> --- a/drivers/usb/gadget/udc/at91_udc.c
> +++ b/drivers/usb/gadget/udc/at91_udc.c
> @@ -267,7 +267,7 @@ static void done(struct at91_ep *ep, struct at91_request *req, int status)
>  
>  	ep->stopped = 1;
>  	spin_unlock(&udc->lock);
> -	req->req.complete(&ep->ep, &req->req);
> +	usb_gadget_giveback_request(&ep->ep, &req->req);
>  	spin_lock(&udc->lock);
>  	ep->stopped = stopped;
>  
> diff --git a/drivers/usb/gadget/udc/atmel_usba_udc.c b/drivers/usb/gadget/udc/atmel_usba_udc.c
> index 906e65f..e96032f 100644
> --- a/drivers/usb/gadget/udc/atmel_usba_udc.c
> +++ b/drivers/usb/gadget/udc/atmel_usba_udc.c
> @@ -463,7 +463,7 @@ static void receive_data(struct usba_ep *ep)
>  			list_del_init(&req->queue);
>  			usba_ep_writel(ep, CTL_DIS, USBA_RX_BK_RDY);
>  			spin_unlock(&udc->lock);
> -			req->req.complete(&ep->ep, &req->req);
> +			usb_gadget_giveback_request(&ep->ep, &req->req);
>  			spin_lock(&udc->lock);
>  		}
>  
> @@ -495,7 +495,7 @@ request_complete(struct usba_ep *ep, struct usba_request *req, int status)
>  		ep->ep.name, req, req->req.status, req->req.actual);
>  
>  	spin_unlock(&udc->lock);
> -	req->req.complete(&ep->ep, &req->req);
> +	usb_gadget_giveback_request(&ep->ep, &req->req);
>  	spin_lock(&udc->lock);
>  }
>  
> diff --git a/drivers/usb/gadget/udc/bcm63xx_udc.c b/drivers/usb/gadget/udc/bcm63xx_udc.c
> index e969eb8..2235b88 100644
> --- a/drivers/usb/gadget/udc/bcm63xx_udc.c
> +++ b/drivers/usb/gadget/udc/bcm63xx_udc.c
> @@ -1088,7 +1088,7 @@ static int bcm63xx_ep_disable(struct usb_ep *ep)
>  			breq->req.status = -ESHUTDOWN;
>  
>  			spin_unlock_irqrestore(&udc->lock, flags);
> -			breq->req.complete(&iudma->bep->ep, &breq->req);
> +			usb_gadget_giveback_request(&iudma->bep->ep, &breq->req);
>  			spin_lock_irqsave(&udc->lock, flags);
>  		}
>  	}
> diff --git a/drivers/usb/gadget/udc/dummy_hcd.c b/drivers/usb/gadget/udc/dummy_hcd.c
> index 2b54955..81dc595 100644
> --- a/drivers/usb/gadget/udc/dummy_hcd.c
> +++ b/drivers/usb/gadget/udc/dummy_hcd.c
> @@ -258,7 +258,7 @@ static void nuke(struct dummy *dum, struct dummy_ep *ep)
>  		req->req.status = -ESHUTDOWN;
>  
>  		spin_unlock(&dum->lock);
> -		req->req.complete(&ep->ep, &req->req);
> +		usb_gadget_giveback_request(&ep->ep, &req->req);
>  		spin_lock(&dum->lock);
>  	}
>  }
> @@ -658,7 +658,7 @@ static int dummy_queue(struct usb_ep *_ep, struct usb_request *_req,
>  		spin_unlock(&dum->lock);
>  		_req->actual = _req->length;
>  		_req->status = 0;
> -		_req->complete(_ep, _req);
> +		usb_gadget_giveback_request(_ep, _req);
>  		spin_lock(&dum->lock);
>  	}  else
>  		list_add_tail(&req->queue, &ep->queue);
> @@ -702,7 +702,7 @@ static int dummy_dequeue(struct usb_ep *_ep, struct usb_request *_req)
>  		dev_dbg(udc_dev(dum),
>  				"dequeued req %p from %s, len %d buf %p\n",
>  				req, _ep->name, _req->length, _req->buf);
> -		_req->complete(_ep, _req);
> +		usb_gadget_giveback_request(_ep, _req);
>  	}
>  	local_irq_restore(flags);
>  	return retval;
> @@ -1385,7 +1385,7 @@ top:
>  			list_del_init(&req->queue);
>  
>  			spin_unlock(&dum->lock);
> -			req->req.complete(&ep->ep, &req->req);
> +			usb_gadget_giveback_request(&ep->ep, &req->req);
>  			spin_lock(&dum->lock);
>  
>  			/* requests might have been unlinked... */
> @@ -1761,7 +1761,7 @@ restart:
>  						req);
>  
>  				spin_unlock(&dum->lock);
> -				req->req.complete(&ep->ep, &req->req);
> +				usb_gadget_giveback_request(&ep->ep, &req->req);
>  				spin_lock(&dum->lock);
>  				ep->already_seen = 0;
>  				goto restart;
> diff --git a/drivers/usb/gadget/udc/fotg210-udc.c b/drivers/usb/gadget/udc/fotg210-udc.c
> index e143d69..1d31592 100644
> --- a/drivers/usb/gadget/udc/fotg210-udc.c
> +++ b/drivers/usb/gadget/udc/fotg210-udc.c
> @@ -70,7 +70,7 @@ static void fotg210_done(struct fotg210_ep *ep, struct fotg210_request *req,
>  		req->req.status = status;
>  
>  	spin_unlock(&ep->fotg210->lock);
> -	req->req.complete(&ep->ep, &req->req);
> +	usb_gadget_giveback_request(&ep->ep, &req->req);
>  	spin_lock(&ep->fotg210->lock);
>  
>  	if (ep->epnum) {
> diff --git a/drivers/usb/gadget/udc/fsl_qe_udc.c b/drivers/usb/gadget/udc/fsl_qe_udc.c
> index 7324308..dd18ea3 100644
> --- a/drivers/usb/gadget/udc/fsl_qe_udc.c
> +++ b/drivers/usb/gadget/udc/fsl_qe_udc.c
> @@ -118,10 +118,7 @@ static void done(struct qe_ep *ep, struct qe_req *req, int status)
>  	ep->stopped = 1;
>  	spin_unlock(&udc->lock);
>  
> -	/* this complete() should a func implemented by gadget layer,
> -	 * eg fsg->bulk_in_complete() */
> -	if (req->req.complete)
> -		req->req.complete(&ep->ep, &req->req);
> +	usb_gadget_giveback_request(&ep->ep, &req->req);
>  
>  	spin_lock(&udc->lock);
>  
> @@ -2728,4 +2725,3 @@ module_platform_driver(udc_driver);
>  MODULE_DESCRIPTION(DRIVER_DESC);
>  MODULE_AUTHOR(DRIVER_AUTHOR);
>  MODULE_LICENSE("GPL");
> -
> diff --git a/drivers/usb/gadget/udc/fsl_udc_core.c b/drivers/usb/gadget/udc/fsl_udc_core.c
> index 75b23ea..c362079 100644
> --- a/drivers/usb/gadget/udc/fsl_udc_core.c
> +++ b/drivers/usb/gadget/udc/fsl_udc_core.c
> @@ -197,10 +197,8 @@ __acquires(ep->udc->lock)
>  	ep->stopped = 1;
>  
>  	spin_unlock(&ep->udc->lock);
> -	/* complete() is from gadget layer,
> -	 * eg fsg->bulk_in_complete() */
> -	if (req->req.complete)
> -		req->req.complete(&ep->ep, &req->req);
> +
> +	usb_gadget_giveback_request(&ep->ep, &req->req);
>  
>  	spin_lock(&ep->udc->lock);
>  	ep->stopped = stopped;
> diff --git a/drivers/usb/gadget/udc/fusb300_udc.c b/drivers/usb/gadget/udc/fusb300_udc.c
> index d40255f..d52687d 100644
> --- a/drivers/usb/gadget/udc/fusb300_udc.c
> +++ b/drivers/usb/gadget/udc/fusb300_udc.c
> @@ -876,7 +876,7 @@ static void done(struct fusb300_ep *ep, struct fusb300_request *req,
>  		req->req.status = status;
>  
>  	spin_unlock(&ep->fusb300->lock);
> -	req->req.complete(&ep->ep, &req->req);
> +	usb_gadget_giveback_request(&ep->ep, &req->req);
>  	spin_lock(&ep->fusb300->lock);
>  
>  	if (ep->epnum) {
> diff --git a/drivers/usb/gadget/udc/goku_udc.c b/drivers/usb/gadget/udc/goku_udc.c
> index 6c85839..bf9c5ef 100644
> --- a/drivers/usb/gadget/udc/goku_udc.c
> +++ b/drivers/usb/gadget/udc/goku_udc.c
> @@ -320,7 +320,7 @@ done(struct goku_ep *ep, struct goku_request *req, int status)
>  	/* don't modify queue heads during completion callback */
>  	ep->stopped = 1;
>  	spin_unlock(&dev->lock);
> -	req->req.complete(&ep->ep, &req->req);
> +	usb_gadget_giveback_request(&ep->ep, &req->req);
>  	spin_lock(&dev->lock);
>  	ep->stopped = stopped;
>  }
> diff --git a/drivers/usb/gadget/udc/gr_udc.c b/drivers/usb/gadget/udc/gr_udc.c
> index 08df5c4..924533b 100644
> --- a/drivers/usb/gadget/udc/gr_udc.c
> +++ b/drivers/usb/gadget/udc/gr_udc.c
> @@ -339,7 +339,7 @@ static void gr_finish_request(struct gr_ep *ep, struct gr_request *req,
>  	} else if (req->req.complete) {
>  		spin_unlock(&dev->lock);
>  
> -		req->req.complete(&ep->ep, &req->req);
> +		usb_gadget_giveback_request(&ep->ep, &req->req);
>  
>  		spin_lock(&dev->lock);
>  	}
> diff --git a/drivers/usb/gadget/udc/lpc32xx_udc.c b/drivers/usb/gadget/udc/lpc32xx_udc.c
> index 1629ad7..feab0ba 100644
> --- a/drivers/usb/gadget/udc/lpc32xx_udc.c
> +++ b/drivers/usb/gadget/udc/lpc32xx_udc.c
> @@ -1479,7 +1479,7 @@ static void done(struct lpc32xx_ep *ep, struct lpc32xx_request *req, int status)
>  
>  	ep->req_pending = 0;
>  	spin_unlock(&udc->lock);
> -	req->req.complete(&ep->ep, &req->req);
> +	usb_gadget_giveback_request(&ep->ep, &req->req);
>  	spin_lock(&udc->lock);
>  }
>  
> diff --git a/drivers/usb/gadget/udc/m66592-udc.c b/drivers/usb/gadget/udc/m66592-udc.c
> index de88d33..8985656 100644
> --- a/drivers/usb/gadget/udc/m66592-udc.c
> +++ b/drivers/usb/gadget/udc/m66592-udc.c
> @@ -729,7 +729,7 @@ __acquires(m66592->lock)
>  		restart = 1;
>  
>  	spin_unlock(&ep->m66592->lock);
> -	req->req.complete(&ep->ep, &req->req);
> +	usb_gadget_giveback_request(&ep->ep, &req->req);
>  	spin_lock(&ep->m66592->lock);
>  
>  	if (restart) {
> diff --git a/drivers/usb/gadget/udc/mv_u3d_core.c b/drivers/usb/gadget/udc/mv_u3d_core.c
> index 1624871..046a1f8 100644
> --- a/drivers/usb/gadget/udc/mv_u3d_core.c
> +++ b/drivers/usb/gadget/udc/mv_u3d_core.c
> @@ -222,12 +222,8 @@ void mv_u3d_done(struct mv_u3d_ep *ep, struct mv_u3d_req *req, int status)
>  	}
>  
>  	spin_unlock(&ep->u3d->lock);
> -	/*
> -	 * complete() is from gadget layer,
> -	 * eg fsg->bulk_in_complete()
> -	 */
> -	if (req->req.complete)
> -		req->req.complete(&ep->ep, &req->req);
> +
> +	usb_gadget_giveback_request(&ep->ep, &req->req);
>  
>  	spin_lock(&ep->u3d->lock);
>  }
> diff --git a/drivers/usb/gadget/udc/mv_udc_core.c b/drivers/usb/gadget/udc/mv_udc_core.c
> index 040fb16..3c5db80 100644
> --- a/drivers/usb/gadget/udc/mv_udc_core.c
> +++ b/drivers/usb/gadget/udc/mv_udc_core.c
> @@ -248,12 +248,8 @@ static void done(struct mv_ep *ep, struct mv_req *req, int status)
>  	ep->stopped = 1;
>  
>  	spin_unlock(&ep->udc->lock);
> -	/*
> -	 * complete() is from gadget layer,
> -	 * eg fsg->bulk_in_complete()
> -	 */
> -	if (req->req.complete)
> -		req->req.complete(&ep->ep, &req->req);
> +
> +	usb_gadget_giveback_request(&ep->ep, &req->req);
>  
>  	spin_lock(&ep->udc->lock);
>  	ep->stopped = stopped;
> diff --git a/drivers/usb/gadget/udc/net2272.c b/drivers/usb/gadget/udc/net2272.c
> index 059cfe5..84d7162 100644
> --- a/drivers/usb/gadget/udc/net2272.c
> +++ b/drivers/usb/gadget/udc/net2272.c
> @@ -394,7 +394,7 @@ net2272_done(struct net2272_ep *ep, struct net2272_request *req, int status)
>  	/* don't modify queue heads during completion callback */
>  	ep->stopped = 1;
>  	spin_unlock(&dev->lock);
> -	req->req.complete(&ep->ep, &req->req);
> +	usb_gadget_giveback_request(&ep->ep, &req->req);
>  	spin_lock(&dev->lock);
>  	ep->stopped = stopped;
>  }
> diff --git a/drivers/usb/gadget/udc/net2280.c b/drivers/usb/gadget/udc/net2280.c
> index f4eac11..2cda166 100644
> --- a/drivers/usb/gadget/udc/net2280.c
> +++ b/drivers/usb/gadget/udc/net2280.c
> @@ -928,7 +928,7 @@ done(struct net2280_ep *ep, struct net2280_request *req, int status)
>  	/* don't modify queue heads during completion callback */
>  	ep->stopped = 1;
>  	spin_unlock(&dev->lock);
> -	req->req.complete(&ep->ep, &req->req);
> +	usb_gadget_giveback_request(&ep->ep, &req->req);
>  	spin_lock(&dev->lock);
>  	ep->stopped = stopped;
>  }
> diff --git a/drivers/usb/gadget/udc/omap_udc.c b/drivers/usb/gadget/udc/omap_udc.c
> index e731373..dcdfea4 100644
> --- a/drivers/usb/gadget/udc/omap_udc.c
> +++ b/drivers/usb/gadget/udc/omap_udc.c
> @@ -315,7 +315,7 @@ done(struct omap_ep *ep, struct omap_req *req, int status)
>  	/* don't modify queue heads during completion callback */
>  	ep->stopped = 1;
>  	spin_unlock(&ep->udc->lock);
> -	req->req.complete(&ep->ep, &req->req);
> +	usb_gadget_giveback_request(&ep->ep, &req->req);
>  	spin_lock(&ep->udc->lock);
>  	ep->stopped = stopped;
>  }
> diff --git a/drivers/usb/gadget/udc/pch_udc.c b/drivers/usb/gadget/udc/pch_udc.c
> index eb8c3be..0b69b32 100644
> --- a/drivers/usb/gadget/udc/pch_udc.c
> +++ b/drivers/usb/gadget/udc/pch_udc.c
> @@ -1486,7 +1486,7 @@ static void complete_req(struct pch_udc_ep *ep, struct pch_udc_request *req,
>  	spin_unlock(&dev->lock);
>  	if (!ep->in)
>  		pch_udc_ep_clear_rrdy(ep);
> -	req->req.complete(&ep->ep, &req->req);
> +	usb_gadget_giveback_request(&ep->ep, &req->req);
>  	spin_lock(&dev->lock);
>  	ep->halted = halted;
>  }
> diff --git a/drivers/usb/gadget/udc/pxa25x_udc.c b/drivers/usb/gadget/udc/pxa25x_udc.c
> index 251e4d5..42f7eeb 100644
> --- a/drivers/usb/gadget/udc/pxa25x_udc.c
> +++ b/drivers/usb/gadget/udc/pxa25x_udc.c
> @@ -347,7 +347,7 @@ static void done(struct pxa25x_ep *ep, struct pxa25x_request *req, int status)
>  
>  	/* don't modify queue heads during completion callback */
>  	ep->stopped = 1;
> -	req->req.complete(&ep->ep, &req->req);
> +	usb_gadget_giveback_request(&ep->ep, &req->req);
>  	ep->stopped = stopped;
>  }
>  
> diff --git a/drivers/usb/gadget/udc/pxa27x_udc.c b/drivers/usb/gadget/udc/pxa27x_udc.c
> index 597d39f..4868369 100644
> --- a/drivers/usb/gadget/udc/pxa27x_udc.c
> +++ b/drivers/usb/gadget/udc/pxa27x_udc.c
> @@ -758,7 +758,7 @@ static void req_done(struct pxa_ep *ep, struct pxa27x_request *req, int status,
>  	if (pflags)
>  		spin_unlock_irqrestore(&ep->lock, *pflags);
>  	local_irq_save(flags);
> -	req->req.complete(&req->udc_usb_ep->usb_ep, &req->req);
> +	usb_gadget_giveback_request(&req->udc_usb_ep->usb_ep, &req->req);
>  	local_irq_restore(flags);
>  	if (pflags)
>  		spin_lock_irqsave(&ep->lock, *pflags);
> diff --git a/drivers/usb/gadget/udc/r8a66597-udc.c b/drivers/usb/gadget/udc/r8a66597-udc.c
> index 4600842..0ec09a4 100644
> --- a/drivers/usb/gadget/udc/r8a66597-udc.c
> +++ b/drivers/usb/gadget/udc/r8a66597-udc.c
> @@ -925,7 +925,7 @@ __acquires(r8a66597->lock)
>  		sudmac_free_channel(ep->r8a66597, ep, req);
>  
>  	spin_unlock(&ep->r8a66597->lock);
> -	req->req.complete(&ep->ep, &req->req);
> +	usb_gadget_giveback_request(&ep->ep, &req->req);
>  	spin_lock(&ep->r8a66597->lock);
>  
>  	if (restart) {
> diff --git a/drivers/usb/gadget/udc/s3c-hsudc.c b/drivers/usb/gadget/udc/s3c-hsudc.c
> index 10c6a12..dfbf557 100644
> --- a/drivers/usb/gadget/udc/s3c-hsudc.c
> +++ b/drivers/usb/gadget/udc/s3c-hsudc.c
> @@ -258,8 +258,7 @@ static void s3c_hsudc_complete_request(struct s3c_hsudc_ep *hsep,
>  
>  	hsep->stopped = 1;
>  	spin_unlock(&hsudc->lock);
> -	if (hsreq->req.complete != NULL)
> -		hsreq->req.complete(&hsep->ep, &hsreq->req);
> +	usb_gadget_giveback_request(&hsep->ep, &hsreq->req);
>  	spin_lock(&hsudc->lock);
>  	hsep->stopped = stopped;
>  }
> diff --git a/drivers/usb/gadget/udc/s3c2410_udc.c b/drivers/usb/gadget/udc/s3c2410_udc.c
> index 357b58e..ff423d1 100644
> --- a/drivers/usb/gadget/udc/s3c2410_udc.c
> +++ b/drivers/usb/gadget/udc/s3c2410_udc.c
> @@ -272,7 +272,7 @@ static void s3c2410_udc_done(struct s3c2410_ep *ep,
>  		status = req->req.status;
>  
>  	ep->halted = 1;
> -	req->req.complete(&ep->ep, &req->req);
> +	usb_gadget_giveback_request(&ep->ep, &req->req);
>  	ep->halted = halted;
>  }
>  
> diff --git a/drivers/usb/gadget/udc/udc-core.c b/drivers/usb/gadget/udc/udc-core.c
> index b0d9817..2eb0ae4 100644
> --- a/drivers/usb/gadget/udc/udc-core.c
> +++ b/drivers/usb/gadget/udc/udc-core.c
> @@ -102,6 +102,15 @@ void usb_gadget_unmap_request(struct usb_gadget *gadget,
>  }
>  EXPORT_SYMBOL_GPL(usb_gadget_unmap_request);
>  
> +void usb_gadget_giveback_request(struct usb_ep *ep,
> +		struct usb_request *req)

you also need some kernel-doc for this function stating the context
where it needs to be called.

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[PATCH v3 2/2] usb: Add LED triggers for USB activity

[Michal Sojka]

With this patch, USB activity can be signaled by blinking a LED. There
are two triggers, one for activity on USB host and one for USB gadget.

Both trigger should work with all host/device controllers. Tested only
with musb.

Signed-off-by: Michal Sojka <sojka@merica.cz>
---
 drivers/usb/Kconfig                           | 11 ++++++
 drivers/usb/common/Makefile                   |  5 ++-
 drivers/usb/common/{usb-common.c => common.c} |  0
 drivers/usb/common/led.c                      | 56 +++++++++++++++++++++++++++
 drivers/usb/core/hcd.c                        |  2 +
 drivers/usb/gadget/udc/udc-core.c             |  4 ++
 include/linux/usb.h                           | 12 ++++++
 7 files changed, 89 insertions(+), 1 deletion(-)
 rename drivers/usb/common/{usb-common.c => common.c} (100%)
 create mode 100644 drivers/usb/common/led.c

diff --git a/drivers/usb/Kconfig b/drivers/usb/Kconfig
index e0cad441..fc90cda 100644
--- a/drivers/usb/Kconfig
+++ b/drivers/usb/Kconfig
@@ -147,4 +147,15 @@ source "drivers/usb/phy/Kconfig"
 
 source "drivers/usb/gadget/Kconfig"
 
+config USB_LED_TRIG
+	bool "USB LED Triggers"
+	depends on LEDS_CLASS && USB_COMMON
+	select LEDS_TRIGGERS
+	help
+	  This option adds LED triggers for USB host and/or gadget activity.
+
+	  Say Y here if you are working on a system with led-class supported
+	  LEDs and you want to use them as activity indicators for USB host or
+	  gadget.
+
 endif # USB_SUPPORT
diff --git a/drivers/usb/common/Makefile b/drivers/usb/common/Makefile
index 7526461..ca2f8bd 100644
--- a/drivers/usb/common/Makefile
+++ b/drivers/usb/common/Makefile
@@ -2,5 +2,8 @@
 # Makefile for the usb common parts.
 #
 
-obj-$(CONFIG_USB_COMMON) += usb-common.o
+obj-$(CONFIG_USB_COMMON)	  += usb-common.o
+usb-common-y			  += common.o
+usb-common-$(CONFIG_USB_LED_TRIG) += led.o
+
 obj-$(CONFIG_USB_OTG_FSM) += usb-otg-fsm.o
diff --git a/drivers/usb/common/usb-common.c b/drivers/usb/common/common.c
similarity index 100%
rename from drivers/usb/common/usb-common.c
rename to drivers/usb/common/common.c
diff --git a/drivers/usb/common/led.c b/drivers/usb/common/led.c
new file mode 100644
index 0000000..af3ce2c
--- /dev/null
+++ b/drivers/usb/common/led.c
@@ -0,0 +1,56 @@
+/*
+ * LED Triggers for USB Activity
+ *
+ * Copyright 2014 Michal Sojka <sojka@merica.cz>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ */
+
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/init.h>
+#include <linux/leds.h>
+#include <linux/usb.h>
+
+#define BLINK_DELAY 30
+
+static unsigned long usb_blink_delay = BLINK_DELAY;
+
+DEFINE_LED_TRIGGER(ledtrig_usb_gadget);
+DEFINE_LED_TRIGGER(ledtrig_usb_host);
+
+void usb_led_activity(enum usb_led_event ev)
+{
+	struct led_trigger *trig = NULL;
+
+	switch (ev) {
+	case USB_LED_EVENT_GADGET: trig = ledtrig_usb_gadget; break;
+	case USB_LED_EVENT_HOST:   trig = ledtrig_usb_host;    break;
+	}
+	led_trigger_blink_oneshot(trig, &usb_blink_delay, &usb_blink_delay, 0);
+}
+EXPORT_SYMBOL(usb_led_activity);
+
+
+static int __init ledtrig_usb_init(void)
+{
+#ifdef CONFIG_USB_GADGET
+	led_trigger_register_simple("usb-gadget", &ledtrig_usb_gadget);
+#endif
+#ifdef CONFIG_USB
+	led_trigger_register_simple("usb-host", &ledtrig_usb_host);
+#endif
+	return 0;
+}
+
+static void __exit ledtrig_usb_exit(void)
+{
+	led_trigger_unregister_simple(ledtrig_usb_gadget);
+	led_trigger_unregister_simple(ledtrig_usb_host);
+}
+
+module_init(ledtrig_usb_init);
+module_exit(ledtrig_usb_exit);
diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
index 487abcf..409cb95 100644
--- a/drivers/usb/core/hcd.c
+++ b/drivers/usb/core/hcd.c
@@ -1664,6 +1664,8 @@ static void __usb_hcd_giveback_urb(struct urb *urb)
 	usbmon_urb_complete(&hcd->self, urb, status);
 	usb_anchor_suspend_wakeups(anchor);
 	usb_unanchor_urb(urb);
+	if (likely(status == 0))
+		usb_led_activity(USB_LED_EVENT_HOST);
 
 	/* pass ownership to the completion handler */
 	urb->status = status;
diff --git a/drivers/usb/gadget/udc/udc-core.c b/drivers/usb/gadget/udc/udc-core.c
index 2eb0ae4..2ce42f9 100644
--- a/drivers/usb/gadget/udc/udc-core.c
+++ b/drivers/usb/gadget/udc/udc-core.c
@@ -27,6 +27,7 @@
 
 #include <linux/usb/ch9.h>
 #include <linux/usb/gadget.h>
+#include <linux/usb.h>
 
 /**
  * struct usb_udc - describes one usb device controller
@@ -105,6 +106,9 @@ EXPORT_SYMBOL_GPL(usb_gadget_unmap_request);
 void usb_gadget_giveback_request(struct usb_ep *ep,
 		struct usb_request *req)
 {
+	if (likely(req->status == 0))
+		usb_led_activity(USB_LED_EVENT_GADGET);
+
 	/* complete() is from gadget layer,
 	 * eg fsg->bulk_in_complete() */
 	if (req->complete)
diff --git a/include/linux/usb.h b/include/linux/usb.h
index d2465bc..447a7e2 100644
--- a/include/linux/usb.h
+++ b/include/linux/usb.h
@@ -1862,6 +1862,18 @@ extern void usb_unregister_notify(struct notifier_block *nb);
 /* debugfs stuff */
 extern struct dentry *usb_debug_root;
 
+/* LED triggers */
+enum usb_led_event {
+	USB_LED_EVENT_HOST = 0,
+	USB_LED_EVENT_GADGET = 1,
+};
+
+#ifdef CONFIG_USB_LED_TRIG
+extern void usb_led_activity(enum usb_led_event ev);
+#else
+static inline void usb_led_activity(enum usb_led_event ev) {}
+#endif
+
 #endif  /* __KERNEL__ */
 
 #endif
-- 
2.1.0

Re: [PATCH v3 2/2] usb: Add LED triggers for USB activity

[Greg Kroah-Hartman]

On Wed, Aug 27, 2014 at 03:03:45PM +0200, Michal Sojka wrote:
> With this patch, USB activity can be signaled by blinking a LED. There
> are two triggers, one for activity on USB host and one for USB gadget.
> 
> Both trigger should work with all host/device controllers. Tested only
> with musb.
> 
> Signed-off-by: Michal Sojka <sojka@merica.cz>
> ---
>  drivers/usb/Kconfig                           | 11 ++++++
>  drivers/usb/common/Makefile                   |  5 ++-
>  drivers/usb/common/{usb-common.c => common.c} |  0

I don't mind renaming files, but don't "hide" it in this patch, do it in
a separate patch please, and then base your next patch on this one.

thanks,

greg k-h

Re: [PATCH v3 2/2] usb: Add LED triggers for USB activity

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 863 bytes --]

On Wed, Aug 27, 2014 at 12:27:51PM -0700, Greg Kroah-Hartman wrote:
> On Wed, Aug 27, 2014 at 03:03:45PM +0200, Michal Sojka wrote:
> > With this patch, USB activity can be signaled by blinking a LED. There
> > are two triggers, one for activity on USB host and one for USB gadget.
> > 
> > Both trigger should work with all host/device controllers. Tested only
> > with musb.
> > 
> > Signed-off-by: Michal Sojka <sojka@merica.cz>
> > ---
> >  drivers/usb/Kconfig                           | 11 ++++++
> >  drivers/usb/common/Makefile                   |  5 ++-
> >  drivers/usb/common/{usb-common.c => common.c} |  0
> 
> I don't mind renaming files, but don't "hide" it in this patch, do it in
> a separate patch please, and then base your next patch on this one.

+1

yeah, this prevents us from reviewing the actual changes

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[PATCH v4 0/3] LED triggers for USB host and device

[Michal Sojka]

This adds LED triggers for USB host and device. First patch refactors
UDC drivers as requested by Felipe Balbi, second is a preparation for
the third, which adds the LED triggers.

Changes from v3:
- usb_gadget_giveback_request() moved outside of CONFIG_HAS_DMA
  conditioned block.
- Added kernel-doc for usb_gadget_giveback_request() (Felipe Balbi).
- Removed outdated comment (Alan Stern).
- req->complete == NULL is now a bug. Previously, this was ignored
  (Alan Stern).
- File rename moved to a separate commit (greg k-h).

Changes from v2:
- Host/gadget triggers merged to a single file in usb/common/ (Felipe
  Balbi).
- UDC drivers refactored so that LED trigger works for all of them.

Changes from v1:
- Moved from drivers/leds/ to drivers/usb/.
- Improved Kconfig help.
- Linked with other modules rather than being standalone modules.

Michal Sojka (2):
  usb: gadget: Refactor request completion
  usb: Add LED triggers for USB activity

 drivers/usb/Kconfig                     |  11 +++
 drivers/usb/chipidea/udc.c              |   6 +-
 drivers/usb/common/Makefile             |   5 +-
 drivers/usb/common/common.c             | 144 ++++++++++++++++++++++++++++++++
 drivers/usb/common/led.c                |  59 +++++++++++++
 drivers/usb/common/usb-common.c         | 144 --------------------------------
 drivers/usb/core/hcd.c                  |   2 +
 drivers/usb/dwc2/gadget.c               |   6 +-
 drivers/usb/dwc3/gadget.c               |   2 +-
 drivers/usb/gadget/udc/amd5536udc.c     |   2 +-
 drivers/usb/gadget/udc/at91_udc.c       |   2 +-
 drivers/usb/gadget/udc/atmel_usba_udc.c |   4 +-
 drivers/usb/gadget/udc/bcm63xx_udc.c    |   2 +-
 drivers/usb/gadget/udc/dummy_hcd.c      |  10 +--
 drivers/usb/gadget/udc/fotg210-udc.c    |   2 +-
 drivers/usb/gadget/udc/fsl_qe_udc.c     |   6 +-
 drivers/usb/gadget/udc/fsl_udc_core.c   |   6 +-
 drivers/usb/gadget/udc/fusb300_udc.c    |   2 +-
 drivers/usb/gadget/udc/goku_udc.c       |   2 +-
 drivers/usb/gadget/udc/gr_udc.c         |   2 +-
 drivers/usb/gadget/udc/lpc32xx_udc.c    |   2 +-
 drivers/usb/gadget/udc/m66592-udc.c     |   2 +-
 drivers/usb/gadget/udc/mv_u3d_core.c    |   8 +-
 drivers/usb/gadget/udc/mv_udc_core.c    |   8 +-
 drivers/usb/gadget/udc/net2272.c        |   2 +-
 drivers/usb/gadget/udc/net2280.c        |   2 +-
 drivers/usb/gadget/udc/omap_udc.c       |   2 +-
 drivers/usb/gadget/udc/pch_udc.c        |   2 +-
 drivers/usb/gadget/udc/pxa25x_udc.c     |   2 +-
 drivers/usb/gadget/udc/pxa27x_udc.c     |   2 +-
 drivers/usb/gadget/udc/r8a66597-udc.c   |   2 +-
 drivers/usb/gadget/udc/s3c-hsudc.c      |   3 +-
 drivers/usb/gadget/udc/s3c2410_udc.c    |   2 +-
 drivers/usb/gadget/udc/udc-core.c       |  13 +++
 drivers/usb/musb/musb_gadget.c          |   2 +-
 drivers/usb/renesas_usbhs/mod_gadget.c  |   2 +-
 include/linux/usb.h                     |  12 +++
 include/linux/usb/gadget.h              |   8 ++
 38 files changed, 294 insertions(+), 201 deletions(-)
 create mode 100644 drivers/usb/common/common.c
 create mode 100644 drivers/usb/common/led.c
 delete mode 100644 drivers/usb/common/usb-common.c

-- 
2.1.0

[PATCH v4 1/3] usb: gadget: Refactor request completion

[Michal Sojka]

All USB peripheral controller drivers called completion routines
directly. This patch moves the completion call from drivers to
usb_gadget_giveback_request(), in order to have a place where common
functionality can be added.

All places in drivers/usb/ matching "[-.]complete(" were replaced with a
call to usb_gadget_giveback_request(). This was compile-tested with all
ARM drivers enabled and runtime-tested for musb.

Signed-off-by: Michal Sojka <sojka@merica.cz>
---
 drivers/usb/chipidea/udc.c              |  6 +++---
 drivers/usb/dwc2/gadget.c               |  6 +++---
 drivers/usb/dwc3/gadget.c               |  2 +-
 drivers/usb/gadget/udc/amd5536udc.c     |  2 +-
 drivers/usb/gadget/udc/at91_udc.c       |  2 +-
 drivers/usb/gadget/udc/atmel_usba_udc.c |  4 ++--
 drivers/usb/gadget/udc/bcm63xx_udc.c    |  2 +-
 drivers/usb/gadget/udc/dummy_hcd.c      | 10 +++++-----
 drivers/usb/gadget/udc/fotg210-udc.c    |  2 +-
 drivers/usb/gadget/udc/fsl_qe_udc.c     |  6 +-----
 drivers/usb/gadget/udc/fsl_udc_core.c   |  6 ++----
 drivers/usb/gadget/udc/fusb300_udc.c    |  2 +-
 drivers/usb/gadget/udc/goku_udc.c       |  2 +-
 drivers/usb/gadget/udc/gr_udc.c         |  2 +-
 drivers/usb/gadget/udc/lpc32xx_udc.c    |  2 +-
 drivers/usb/gadget/udc/m66592-udc.c     |  2 +-
 drivers/usb/gadget/udc/mv_u3d_core.c    |  8 ++------
 drivers/usb/gadget/udc/mv_udc_core.c    |  8 ++------
 drivers/usb/gadget/udc/net2272.c        |  2 +-
 drivers/usb/gadget/udc/net2280.c        |  2 +-
 drivers/usb/gadget/udc/omap_udc.c       |  2 +-
 drivers/usb/gadget/udc/pch_udc.c        |  2 +-
 drivers/usb/gadget/udc/pxa25x_udc.c     |  2 +-
 drivers/usb/gadget/udc/pxa27x_udc.c     |  2 +-
 drivers/usb/gadget/udc/r8a66597-udc.c   |  2 +-
 drivers/usb/gadget/udc/s3c-hsudc.c      |  3 +--
 drivers/usb/gadget/udc/s3c2410_udc.c    |  2 +-
 drivers/usb/gadget/udc/udc-core.c       | 17 +++++++++++++++++
 drivers/usb/musb/musb_gadget.c          |  2 +-
 drivers/usb/renesas_usbhs/mod_gadget.c  |  2 +-
 include/linux/usb/gadget.h              |  8 ++++++++
 31 files changed, 66 insertions(+), 56 deletions(-)

diff --git a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c
index b8125aa..0444d3f 100644
--- a/drivers/usb/chipidea/udc.c
+++ b/drivers/usb/chipidea/udc.c
@@ -627,7 +627,7 @@ __acquires(hwep->lock)
 
 		if (hwreq->req.complete != NULL) {
 			spin_unlock(hwep->lock);
-			hwreq->req.complete(&hwep->ep, &hwreq->req);
+			usb_gadget_giveback_request(&hwep->ep, &hwreq->req);
 			spin_lock(hwep->lock);
 		}
 	}
@@ -922,7 +922,7 @@ __acquires(hwep->lock)
 			if ((hwep->type == USB_ENDPOINT_XFER_CONTROL) &&
 					hwreq->req.length)
 				hweptemp = hwep->ci->ep0in;
-			hwreq->req.complete(&hweptemp->ep, &hwreq->req);
+			usb_gadget_giveback_request(&hweptemp->ep, &hwreq->req);
 			spin_lock(hwep->lock);
 		}
 	}
@@ -1347,7 +1347,7 @@ static int ep_dequeue(struct usb_ep *ep, struct usb_request *req)
 
 	if (hwreq->req.complete != NULL) {
 		spin_unlock(hwep->lock);
-		hwreq->req.complete(&hwep->ep, &hwreq->req);
+		usb_gadget_giveback_request(&hwep->ep, &hwreq->req);
 		spin_lock(hwep->lock);
 	}
 
diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
index 0ba9c33..5a524a6 100644
--- a/drivers/usb/dwc2/gadget.c
+++ b/drivers/usb/dwc2/gadget.c
@@ -987,8 +987,8 @@ static int s3c_hsotg_process_req_feature(struct s3c_hsotg *hsotg,
 					hs_req = ep->req;
 					ep->req = NULL;
 					list_del_init(&hs_req->queue);
-					hs_req->req.complete(&ep->ep,
-							     &hs_req->req);
+					usb_gadget_giveback_request(&ep->ep,
+								    &hs_req->req);
 				}
 
 				/* If we have pending request, then start it */
@@ -1245,7 +1245,7 @@ static void s3c_hsotg_complete_request(struct s3c_hsotg *hsotg,
 
 	if (hs_req->req.complete) {
 		spin_unlock(&hsotg->lock);
-		hs_req->req.complete(&hs_ep->ep, &hs_req->req);
+		usb_gadget_giveback_request(&hs_ep->ep, &hs_req->req);
 		spin_lock(&hsotg->lock);
 	}
 
diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index 349cacc..b4b7a6b 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -268,7 +268,7 @@ void dwc3_gadget_giveback(struct dwc3_ep *dep, struct dwc3_request *req,
 			req->request.length, status);
 
 	spin_unlock(&dwc->lock);
-	req->request.complete(&dep->endpoint, &req->request);
+	usb_gadget_giveback_request(&dep->endpoint, &req->request);
 	spin_lock(&dwc->lock);
 }
 
diff --git a/drivers/usb/gadget/udc/amd5536udc.c b/drivers/usb/gadget/udc/amd5536udc.c
index 41b062e..3b9d138 100644
--- a/drivers/usb/gadget/udc/amd5536udc.c
+++ b/drivers/usb/gadget/udc/amd5536udc.c
@@ -841,7 +841,7 @@ __acquires(ep->dev->lock)
 		&req->req, req->req.length, ep->ep.name, sts);
 
 	spin_unlock(&dev->lock);
-	req->req.complete(&ep->ep, &req->req);
+	usb_gadget_giveback_request(&ep->ep, &req->req);
 	spin_lock(&dev->lock);
 	ep->halted = halted;
 }
diff --git a/drivers/usb/gadget/udc/at91_udc.c b/drivers/usb/gadget/udc/at91_udc.c
index cfd18bc..9968f53 100644
--- a/drivers/usb/gadget/udc/at91_udc.c
+++ b/drivers/usb/gadget/udc/at91_udc.c
@@ -267,7 +267,7 @@ static void done(struct at91_ep *ep, struct at91_request *req, int status)
 
 	ep->stopped = 1;
 	spin_unlock(&udc->lock);
-	req->req.complete(&ep->ep, &req->req);
+	usb_gadget_giveback_request(&ep->ep, &req->req);
 	spin_lock(&udc->lock);
 	ep->stopped = stopped;
 
diff --git a/drivers/usb/gadget/udc/atmel_usba_udc.c b/drivers/usb/gadget/udc/atmel_usba_udc.c
index 906e65f..e96032f 100644
--- a/drivers/usb/gadget/udc/atmel_usba_udc.c
+++ b/drivers/usb/gadget/udc/atmel_usba_udc.c
@@ -463,7 +463,7 @@ static void receive_data(struct usba_ep *ep)
 			list_del_init(&req->queue);
 			usba_ep_writel(ep, CTL_DIS, USBA_RX_BK_RDY);
 			spin_unlock(&udc->lock);
-			req->req.complete(&ep->ep, &req->req);
+			usb_gadget_giveback_request(&ep->ep, &req->req);
 			spin_lock(&udc->lock);
 		}
 
@@ -495,7 +495,7 @@ request_complete(struct usba_ep *ep, struct usba_request *req, int status)
 		ep->ep.name, req, req->req.status, req->req.actual);
 
 	spin_unlock(&udc->lock);
-	req->req.complete(&ep->ep, &req->req);
+	usb_gadget_giveback_request(&ep->ep, &req->req);
 	spin_lock(&udc->lock);
 }
 
diff --git a/drivers/usb/gadget/udc/bcm63xx_udc.c b/drivers/usb/gadget/udc/bcm63xx_udc.c
index e969eb8..2235b88 100644
--- a/drivers/usb/gadget/udc/bcm63xx_udc.c
+++ b/drivers/usb/gadget/udc/bcm63xx_udc.c
@@ -1088,7 +1088,7 @@ static int bcm63xx_ep_disable(struct usb_ep *ep)
 			breq->req.status = -ESHUTDOWN;
 
 			spin_unlock_irqrestore(&udc->lock, flags);
-			breq->req.complete(&iudma->bep->ep, &breq->req);
+			usb_gadget_giveback_request(&iudma->bep->ep, &breq->req);
 			spin_lock_irqsave(&udc->lock, flags);
 		}
 	}
diff --git a/drivers/usb/gadget/udc/dummy_hcd.c b/drivers/usb/gadget/udc/dummy_hcd.c
index 2b54955..81dc595 100644
--- a/drivers/usb/gadget/udc/dummy_hcd.c
+++ b/drivers/usb/gadget/udc/dummy_hcd.c
@@ -258,7 +258,7 @@ static void nuke(struct dummy *dum, struct dummy_ep *ep)
 		req->req.status = -ESHUTDOWN;
 
 		spin_unlock(&dum->lock);
-		req->req.complete(&ep->ep, &req->req);
+		usb_gadget_giveback_request(&ep->ep, &req->req);
 		spin_lock(&dum->lock);
 	}
 }
@@ -658,7 +658,7 @@ static int dummy_queue(struct usb_ep *_ep, struct usb_request *_req,
 		spin_unlock(&dum->lock);
 		_req->actual = _req->length;
 		_req->status = 0;
-		_req->complete(_ep, _req);
+		usb_gadget_giveback_request(_ep, _req);
 		spin_lock(&dum->lock);
 	}  else
 		list_add_tail(&req->queue, &ep->queue);
@@ -702,7 +702,7 @@ static int dummy_dequeue(struct usb_ep *_ep, struct usb_request *_req)
 		dev_dbg(udc_dev(dum),
 				"dequeued req %p from %s, len %d buf %p\n",
 				req, _ep->name, _req->length, _req->buf);
-		_req->complete(_ep, _req);
+		usb_gadget_giveback_request(_ep, _req);
 	}
 	local_irq_restore(flags);
 	return retval;
@@ -1385,7 +1385,7 @@ top:
 			list_del_init(&req->queue);
 
 			spin_unlock(&dum->lock);
-			req->req.complete(&ep->ep, &req->req);
+			usb_gadget_giveback_request(&ep->ep, &req->req);
 			spin_lock(&dum->lock);
 
 			/* requests might have been unlinked... */
@@ -1761,7 +1761,7 @@ restart:
 						req);
 
 				spin_unlock(&dum->lock);
-				req->req.complete(&ep->ep, &req->req);
+				usb_gadget_giveback_request(&ep->ep, &req->req);
 				spin_lock(&dum->lock);
 				ep->already_seen = 0;
 				goto restart;
diff --git a/drivers/usb/gadget/udc/fotg210-udc.c b/drivers/usb/gadget/udc/fotg210-udc.c
index e143d69..1d31592 100644
--- a/drivers/usb/gadget/udc/fotg210-udc.c
+++ b/drivers/usb/gadget/udc/fotg210-udc.c
@@ -70,7 +70,7 @@ static void fotg210_done(struct fotg210_ep *ep, struct fotg210_request *req,
 		req->req.status = status;
 
 	spin_unlock(&ep->fotg210->lock);
-	req->req.complete(&ep->ep, &req->req);
+	usb_gadget_giveback_request(&ep->ep, &req->req);
 	spin_lock(&ep->fotg210->lock);
 
 	if (ep->epnum) {
diff --git a/drivers/usb/gadget/udc/fsl_qe_udc.c b/drivers/usb/gadget/udc/fsl_qe_udc.c
index 7324308..dd18ea3 100644
--- a/drivers/usb/gadget/udc/fsl_qe_udc.c
+++ b/drivers/usb/gadget/udc/fsl_qe_udc.c
@@ -118,10 +118,7 @@ static void done(struct qe_ep *ep, struct qe_req *req, int status)
 	ep->stopped = 1;
 	spin_unlock(&udc->lock);
 
-	/* this complete() should a func implemented by gadget layer,
-	 * eg fsg->bulk_in_complete() */
-	if (req->req.complete)
-		req->req.complete(&ep->ep, &req->req);
+	usb_gadget_giveback_request(&ep->ep, &req->req);
 
 	spin_lock(&udc->lock);
 
@@ -2728,4 +2725,3 @@ module_platform_driver(udc_driver);
 MODULE_DESCRIPTION(DRIVER_DESC);
 MODULE_AUTHOR(DRIVER_AUTHOR);
 MODULE_LICENSE("GPL");
-
diff --git a/drivers/usb/gadget/udc/fsl_udc_core.c b/drivers/usb/gadget/udc/fsl_udc_core.c
index 75b23ea..c362079 100644
--- a/drivers/usb/gadget/udc/fsl_udc_core.c
+++ b/drivers/usb/gadget/udc/fsl_udc_core.c
@@ -197,10 +197,8 @@ __acquires(ep->udc->lock)
 	ep->stopped = 1;
 
 	spin_unlock(&ep->udc->lock);
-	/* complete() is from gadget layer,
-	 * eg fsg->bulk_in_complete() */
-	if (req->req.complete)
-		req->req.complete(&ep->ep, &req->req);
+
+	usb_gadget_giveback_request(&ep->ep, &req->req);
 
 	spin_lock(&ep->udc->lock);
 	ep->stopped = stopped;
diff --git a/drivers/usb/gadget/udc/fusb300_udc.c b/drivers/usb/gadget/udc/fusb300_udc.c
index d40255f..d52687d 100644
--- a/drivers/usb/gadget/udc/fusb300_udc.c
+++ b/drivers/usb/gadget/udc/fusb300_udc.c
@@ -876,7 +876,7 @@ static void done(struct fusb300_ep *ep, struct fusb300_request *req,
 		req->req.status = status;
 
 	spin_unlock(&ep->fusb300->lock);
-	req->req.complete(&ep->ep, &req->req);
+	usb_gadget_giveback_request(&ep->ep, &req->req);
 	spin_lock(&ep->fusb300->lock);
 
 	if (ep->epnum) {
diff --git a/drivers/usb/gadget/udc/goku_udc.c b/drivers/usb/gadget/udc/goku_udc.c
index 6c85839..bf9c5ef 100644
--- a/drivers/usb/gadget/udc/goku_udc.c
+++ b/drivers/usb/gadget/udc/goku_udc.c
@@ -320,7 +320,7 @@ done(struct goku_ep *ep, struct goku_request *req, int status)
 	/* don't modify queue heads during completion callback */
 	ep->stopped = 1;
 	spin_unlock(&dev->lock);
-	req->req.complete(&ep->ep, &req->req);
+	usb_gadget_giveback_request(&ep->ep, &req->req);
 	spin_lock(&dev->lock);
 	ep->stopped = stopped;
 }
diff --git a/drivers/usb/gadget/udc/gr_udc.c b/drivers/usb/gadget/udc/gr_udc.c
index 08df5c4..924533b 100644
--- a/drivers/usb/gadget/udc/gr_udc.c
+++ b/drivers/usb/gadget/udc/gr_udc.c
@@ -339,7 +339,7 @@ static void gr_finish_request(struct gr_ep *ep, struct gr_request *req,
 	} else if (req->req.complete) {
 		spin_unlock(&dev->lock);
 
-		req->req.complete(&ep->ep, &req->req);
+		usb_gadget_giveback_request(&ep->ep, &req->req);
 
 		spin_lock(&dev->lock);
 	}
diff --git a/drivers/usb/gadget/udc/lpc32xx_udc.c b/drivers/usb/gadget/udc/lpc32xx_udc.c
index 1629ad7..feab0ba 100644
--- a/drivers/usb/gadget/udc/lpc32xx_udc.c
+++ b/drivers/usb/gadget/udc/lpc32xx_udc.c
@@ -1479,7 +1479,7 @@ static void done(struct lpc32xx_ep *ep, struct lpc32xx_request *req, int status)
 
 	ep->req_pending = 0;
 	spin_unlock(&udc->lock);
-	req->req.complete(&ep->ep, &req->req);
+	usb_gadget_giveback_request(&ep->ep, &req->req);
 	spin_lock(&udc->lock);
 }
 
diff --git a/drivers/usb/gadget/udc/m66592-udc.c b/drivers/usb/gadget/udc/m66592-udc.c
index de88d33..8985656 100644
--- a/drivers/usb/gadget/udc/m66592-udc.c
+++ b/drivers/usb/gadget/udc/m66592-udc.c
@@ -729,7 +729,7 @@ __acquires(m66592->lock)
 		restart = 1;
 
 	spin_unlock(&ep->m66592->lock);
-	req->req.complete(&ep->ep, &req->req);
+	usb_gadget_giveback_request(&ep->ep, &req->req);
 	spin_lock(&ep->m66592->lock);
 
 	if (restart) {
diff --git a/drivers/usb/gadget/udc/mv_u3d_core.c b/drivers/usb/gadget/udc/mv_u3d_core.c
index 1624871..046a1f8 100644
--- a/drivers/usb/gadget/udc/mv_u3d_core.c
+++ b/drivers/usb/gadget/udc/mv_u3d_core.c
@@ -222,12 +222,8 @@ void mv_u3d_done(struct mv_u3d_ep *ep, struct mv_u3d_req *req, int status)
 	}
 
 	spin_unlock(&ep->u3d->lock);
-	/*
-	 * complete() is from gadget layer,
-	 * eg fsg->bulk_in_complete()
-	 */
-	if (req->req.complete)
-		req->req.complete(&ep->ep, &req->req);
+
+	usb_gadget_giveback_request(&ep->ep, &req->req);
 
 	spin_lock(&ep->u3d->lock);
 }
diff --git a/drivers/usb/gadget/udc/mv_udc_core.c b/drivers/usb/gadget/udc/mv_udc_core.c
index 040fb16..3c5db80 100644
--- a/drivers/usb/gadget/udc/mv_udc_core.c
+++ b/drivers/usb/gadget/udc/mv_udc_core.c
@@ -248,12 +248,8 @@ static void done(struct mv_ep *ep, struct mv_req *req, int status)
 	ep->stopped = 1;
 
 	spin_unlock(&ep->udc->lock);
-	/*
-	 * complete() is from gadget layer,
-	 * eg fsg->bulk_in_complete()
-	 */
-	if (req->req.complete)
-		req->req.complete(&ep->ep, &req->req);
+
+	usb_gadget_giveback_request(&ep->ep, &req->req);
 
 	spin_lock(&ep->udc->lock);
 	ep->stopped = stopped;
diff --git a/drivers/usb/gadget/udc/net2272.c b/drivers/usb/gadget/udc/net2272.c
index 059cfe5..84d7162 100644
--- a/drivers/usb/gadget/udc/net2272.c
+++ b/drivers/usb/gadget/udc/net2272.c
@@ -394,7 +394,7 @@ net2272_done(struct net2272_ep *ep, struct net2272_request *req, int status)
 	/* don't modify queue heads during completion callback */
 	ep->stopped = 1;
 	spin_unlock(&dev->lock);
-	req->req.complete(&ep->ep, &req->req);
+	usb_gadget_giveback_request(&ep->ep, &req->req);
 	spin_lock(&dev->lock);
 	ep->stopped = stopped;
 }
diff --git a/drivers/usb/gadget/udc/net2280.c b/drivers/usb/gadget/udc/net2280.c
index f4eac11..2cda166 100644
--- a/drivers/usb/gadget/udc/net2280.c
+++ b/drivers/usb/gadget/udc/net2280.c
@@ -928,7 +928,7 @@ done(struct net2280_ep *ep, struct net2280_request *req, int status)
 	/* don't modify queue heads during completion callback */
 	ep->stopped = 1;
 	spin_unlock(&dev->lock);
-	req->req.complete(&ep->ep, &req->req);
+	usb_gadget_giveback_request(&ep->ep, &req->req);
 	spin_lock(&dev->lock);
 	ep->stopped = stopped;
 }
diff --git a/drivers/usb/gadget/udc/omap_udc.c b/drivers/usb/gadget/udc/omap_udc.c
index e731373..dcdfea4 100644
--- a/drivers/usb/gadget/udc/omap_udc.c
+++ b/drivers/usb/gadget/udc/omap_udc.c
@@ -315,7 +315,7 @@ done(struct omap_ep *ep, struct omap_req *req, int status)
 	/* don't modify queue heads during completion callback */
 	ep->stopped = 1;
 	spin_unlock(&ep->udc->lock);
-	req->req.complete(&ep->ep, &req->req);
+	usb_gadget_giveback_request(&ep->ep, &req->req);
 	spin_lock(&ep->udc->lock);
 	ep->stopped = stopped;
 }
diff --git a/drivers/usb/gadget/udc/pch_udc.c b/drivers/usb/gadget/udc/pch_udc.c
index eb8c3be..0b69b32 100644
--- a/drivers/usb/gadget/udc/pch_udc.c
+++ b/drivers/usb/gadget/udc/pch_udc.c
@@ -1486,7 +1486,7 @@ static void complete_req(struct pch_udc_ep *ep, struct pch_udc_request *req,
 	spin_unlock(&dev->lock);
 	if (!ep->in)
 		pch_udc_ep_clear_rrdy(ep);
-	req->req.complete(&ep->ep, &req->req);
+	usb_gadget_giveback_request(&ep->ep, &req->req);
 	spin_lock(&dev->lock);
 	ep->halted = halted;
 }
diff --git a/drivers/usb/gadget/udc/pxa25x_udc.c b/drivers/usb/gadget/udc/pxa25x_udc.c
index 251e4d5..42f7eeb 100644
--- a/drivers/usb/gadget/udc/pxa25x_udc.c
+++ b/drivers/usb/gadget/udc/pxa25x_udc.c
@@ -347,7 +347,7 @@ static void done(struct pxa25x_ep *ep, struct pxa25x_request *req, int status)
 
 	/* don't modify queue heads during completion callback */
 	ep->stopped = 1;
-	req->req.complete(&ep->ep, &req->req);
+	usb_gadget_giveback_request(&ep->ep, &req->req);
 	ep->stopped = stopped;
 }
 
diff --git a/drivers/usb/gadget/udc/pxa27x_udc.c b/drivers/usb/gadget/udc/pxa27x_udc.c
index 597d39f..4868369 100644
--- a/drivers/usb/gadget/udc/pxa27x_udc.c
+++ b/drivers/usb/gadget/udc/pxa27x_udc.c
@@ -758,7 +758,7 @@ static void req_done(struct pxa_ep *ep, struct pxa27x_request *req, int status,
 	if (pflags)
 		spin_unlock_irqrestore(&ep->lock, *pflags);
 	local_irq_save(flags);
-	req->req.complete(&req->udc_usb_ep->usb_ep, &req->req);
+	usb_gadget_giveback_request(&req->udc_usb_ep->usb_ep, &req->req);
 	local_irq_restore(flags);
 	if (pflags)
 		spin_lock_irqsave(&ep->lock, *pflags);
diff --git a/drivers/usb/gadget/udc/r8a66597-udc.c b/drivers/usb/gadget/udc/r8a66597-udc.c
index 4600842..0ec09a4 100644
--- a/drivers/usb/gadget/udc/r8a66597-udc.c
+++ b/drivers/usb/gadget/udc/r8a66597-udc.c
@@ -925,7 +925,7 @@ __acquires(r8a66597->lock)
 		sudmac_free_channel(ep->r8a66597, ep, req);
 
 	spin_unlock(&ep->r8a66597->lock);
-	req->req.complete(&ep->ep, &req->req);
+	usb_gadget_giveback_request(&ep->ep, &req->req);
 	spin_lock(&ep->r8a66597->lock);
 
 	if (restart) {
diff --git a/drivers/usb/gadget/udc/s3c-hsudc.c b/drivers/usb/gadget/udc/s3c-hsudc.c
index 10c6a12..dfbf557 100644
--- a/drivers/usb/gadget/udc/s3c-hsudc.c
+++ b/drivers/usb/gadget/udc/s3c-hsudc.c
@@ -258,8 +258,7 @@ static void s3c_hsudc_complete_request(struct s3c_hsudc_ep *hsep,
 
 	hsep->stopped = 1;
 	spin_unlock(&hsudc->lock);
-	if (hsreq->req.complete != NULL)
-		hsreq->req.complete(&hsep->ep, &hsreq->req);
+	usb_gadget_giveback_request(&hsep->ep, &hsreq->req);
 	spin_lock(&hsudc->lock);
 	hsep->stopped = stopped;
 }
diff --git a/drivers/usb/gadget/udc/s3c2410_udc.c b/drivers/usb/gadget/udc/s3c2410_udc.c
index 357b58e..ff423d1 100644
--- a/drivers/usb/gadget/udc/s3c2410_udc.c
+++ b/drivers/usb/gadget/udc/s3c2410_udc.c
@@ -272,7 +272,7 @@ static void s3c2410_udc_done(struct s3c2410_ep *ep,
 		status = req->req.status;
 
 	ep->halted = 1;
-	req->req.complete(&ep->ep, &req->req);
+	usb_gadget_giveback_request(&ep->ep, &req->req);
 	ep->halted = halted;
 }
 
diff --git a/drivers/usb/gadget/udc/udc-core.c b/drivers/usb/gadget/udc/udc-core.c
index b0d9817..c1df19b 100644
--- a/drivers/usb/gadget/udc/udc-core.c
+++ b/drivers/usb/gadget/udc/udc-core.c
@@ -106,6 +106,23 @@ EXPORT_SYMBOL_GPL(usb_gadget_unmap_request);
 
 /* ------------------------------------------------------------------------- */
 
+/**
+ * usb_gadget_giveback_request - give the request back to the gadget layer
+ * Context: in_interrupt()
+ *
+ * This is called by device controller drivers in order to return the
+ * completed request back to the gadget layer.
+ */
+void usb_gadget_giveback_request(struct usb_ep *ep,
+		struct usb_request *req)
+{
+	BUG_ON(req->complete == NULL);
+	req->complete(ep, req);
+}
+EXPORT_SYMBOL_GPL(usb_gadget_giveback_request);
+
+/* ------------------------------------------------------------------------- */
+
 static void usb_gadget_state_work(struct work_struct *work)
 {
 	struct usb_gadget	*gadget = work_to_gadget(work);
diff --git a/drivers/usb/musb/musb_gadget.c b/drivers/usb/musb/musb_gadget.c
index d4aa779..24c8c02 100644
--- a/drivers/usb/musb/musb_gadget.c
+++ b/drivers/usb/musb/musb_gadget.c
@@ -176,7 +176,7 @@ __acquires(ep->musb->lock)
 				ep->end_point.name, request,
 				req->request.actual, req->request.length,
 				request->status);
-	req->request.complete(&req->ep->end_point, &req->request);
+	usb_gadget_giveback_request(&req->ep->end_point, &req->request);
 	spin_lock(&musb->lock);
 	ep->busy = busy;
 }
diff --git a/drivers/usb/renesas_usbhs/mod_gadget.c b/drivers/usb/renesas_usbhs/mod_gadget.c
index 04e6505..2d17c10 100644
--- a/drivers/usb/renesas_usbhs/mod_gadget.c
+++ b/drivers/usb/renesas_usbhs/mod_gadget.c
@@ -129,7 +129,7 @@ static void usbhsg_queue_pop(struct usbhsg_uep *uep,
 	dev_dbg(dev, "pipe %d : queue pop\n", usbhs_pipe_number(pipe));
 
 	ureq->req.status = status;
-	ureq->req.complete(&uep->ep, &ureq->req);
+	usb_gadget_giveback_request(&uep->ep, &ureq->req);
 }
 
 static void usbhsg_queue_done(struct usbhs_priv *priv, struct usbhs_pkt *pkt)
diff --git a/include/linux/usb/gadget.h b/include/linux/usb/gadget.h
index c3a6185..f795e95 100644
--- a/include/linux/usb/gadget.h
+++ b/include/linux/usb/gadget.h
@@ -1013,6 +1013,14 @@ extern void usb_gadget_set_state(struct usb_gadget *gadget,
 
 /*-------------------------------------------------------------------------*/
 
+/* utility to give requests back to the gadget layer */
+
+extern void usb_gadget_giveback_request(struct usb_ep *ep,
+		struct usb_request *req);
+
+
+/*-------------------------------------------------------------------------*/
+
 /* utility wrapping a simple endpoint selection policy */
 
 extern struct usb_ep *usb_ep_autoconfig(struct usb_gadget *,
-- 
2.1.0

Re: [PATCH v4 1/3] usb: gadget: Refactor request completion

[Alan Stern]

On Wed, 27 Aug 2014, Michal Sojka wrote:

> +/**
> + * usb_gadget_giveback_request - give the request back to the gadget layer
> + * Context: in_interrupt()
> + *
> + * This is called by device controller drivers in order to return the
> + * completed request back to the gadget layer.
> + */
> +void usb_gadget_giveback_request(struct usb_ep *ep,
> +		struct usb_request *req)
> +{
> +	BUG_ON(req->complete == NULL);
> +	req->complete(ep, req);
> +}

I guess it doesn't hurt to have the BUG_ON, but it doesn't help either.  
Think about what would happen if req->complete was NULL and the BUG_ON 
wasn't present.

Alan Stern

Re: [PATCH v4 1/3] usb: gadget: Refactor request completion

[Greg Kroah-Hartman]

On Wed, Aug 27, 2014 at 10:57:58PM +0200, Michal Sojka wrote:
> All USB peripheral controller drivers called completion routines
> directly. This patch moves the completion call from drivers to
> usb_gadget_giveback_request(), in order to have a place where common
> functionality can be added.
> 
> All places in drivers/usb/ matching "[-.]complete(" were replaced with a
> call to usb_gadget_giveback_request(). This was compile-tested with all
> ARM drivers enabled and runtime-tested for musb.
> 
> Signed-off-by: Michal Sojka <sojka@merica.cz>
> ---
>  drivers/usb/chipidea/udc.c              |  6 +++---
>  drivers/usb/dwc2/gadget.c               |  6 +++---
>  drivers/usb/dwc3/gadget.c               |  2 +-
>  drivers/usb/gadget/udc/amd5536udc.c     |  2 +-
>  drivers/usb/gadget/udc/at91_udc.c       |  2 +-
>  drivers/usb/gadget/udc/atmel_usba_udc.c |  4 ++--
>  drivers/usb/gadget/udc/bcm63xx_udc.c    |  2 +-
>  drivers/usb/gadget/udc/dummy_hcd.c      | 10 +++++-----
>  drivers/usb/gadget/udc/fotg210-udc.c    |  2 +-
>  drivers/usb/gadget/udc/fsl_qe_udc.c     |  6 +-----
>  drivers/usb/gadget/udc/fsl_udc_core.c   |  6 ++----
>  drivers/usb/gadget/udc/fusb300_udc.c    |  2 +-
>  drivers/usb/gadget/udc/goku_udc.c       |  2 +-
>  drivers/usb/gadget/udc/gr_udc.c         |  2 +-
>  drivers/usb/gadget/udc/lpc32xx_udc.c    |  2 +-
>  drivers/usb/gadget/udc/m66592-udc.c     |  2 +-
>  drivers/usb/gadget/udc/mv_u3d_core.c    |  8 ++------
>  drivers/usb/gadget/udc/mv_udc_core.c    |  8 ++------
>  drivers/usb/gadget/udc/net2272.c        |  2 +-
>  drivers/usb/gadget/udc/net2280.c        |  2 +-
>  drivers/usb/gadget/udc/omap_udc.c       |  2 +-
>  drivers/usb/gadget/udc/pch_udc.c        |  2 +-
>  drivers/usb/gadget/udc/pxa25x_udc.c     |  2 +-
>  drivers/usb/gadget/udc/pxa27x_udc.c     |  2 +-
>  drivers/usb/gadget/udc/r8a66597-udc.c   |  2 +-
>  drivers/usb/gadget/udc/s3c-hsudc.c      |  3 +--
>  drivers/usb/gadget/udc/s3c2410_udc.c    |  2 +-
>  drivers/usb/gadget/udc/udc-core.c       | 17 +++++++++++++++++
>  drivers/usb/musb/musb_gadget.c          |  2 +-
>  drivers/usb/renesas_usbhs/mod_gadget.c  |  2 +-
>  include/linux/usb/gadget.h              |  8 ++++++++
>  31 files changed, 66 insertions(+), 56 deletions(-)
> 
> diff --git a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c
> index b8125aa..0444d3f 100644
> --- a/drivers/usb/chipidea/udc.c
> +++ b/drivers/usb/chipidea/udc.c
> @@ -627,7 +627,7 @@ __acquires(hwep->lock)
>  
>  		if (hwreq->req.complete != NULL) {
>  			spin_unlock(hwep->lock);
> -			hwreq->req.complete(&hwep->ep, &hwreq->req);
> +			usb_gadget_giveback_request(&hwep->ep, &hwreq->req);
>  			spin_lock(hwep->lock);
>  		}
>  	}
> @@ -922,7 +922,7 @@ __acquires(hwep->lock)
>  			if ((hwep->type == USB_ENDPOINT_XFER_CONTROL) &&
>  					hwreq->req.length)
>  				hweptemp = hwep->ci->ep0in;
> -			hwreq->req.complete(&hweptemp->ep, &hwreq->req);
> +			usb_gadget_giveback_request(&hweptemp->ep, &hwreq->req);
>  			spin_lock(hwep->lock);
>  		}
>  	}
> @@ -1347,7 +1347,7 @@ static int ep_dequeue(struct usb_ep *ep, struct usb_request *req)
>  
>  	if (hwreq->req.complete != NULL) {
>  		spin_unlock(hwep->lock);
> -		hwreq->req.complete(&hwep->ep, &hwreq->req);
> +		usb_gadget_giveback_request(&hwep->ep, &hwreq->req);
>  		spin_lock(hwep->lock);
>  	}
>  
> diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
> index 0ba9c33..5a524a6 100644
> --- a/drivers/usb/dwc2/gadget.c
> +++ b/drivers/usb/dwc2/gadget.c
> @@ -987,8 +987,8 @@ static int s3c_hsotg_process_req_feature(struct s3c_hsotg *hsotg,
>  					hs_req = ep->req;
>  					ep->req = NULL;
>  					list_del_init(&hs_req->queue);
> -					hs_req->req.complete(&ep->ep,
> -							     &hs_req->req);
> +					usb_gadget_giveback_request(&ep->ep,
> +								    &hs_req->req);
>  				}
>  
>  				/* If we have pending request, then start it */
> @@ -1245,7 +1245,7 @@ static void s3c_hsotg_complete_request(struct s3c_hsotg *hsotg,
>  
>  	if (hs_req->req.complete) {
>  		spin_unlock(&hsotg->lock);
> -		hs_req->req.complete(&hs_ep->ep, &hs_req->req);
> +		usb_gadget_giveback_request(&hs_ep->ep, &hs_req->req);
>  		spin_lock(&hsotg->lock);
>  	}
>  
> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
> index 349cacc..b4b7a6b 100644
> --- a/drivers/usb/dwc3/gadget.c
> +++ b/drivers/usb/dwc3/gadget.c
> @@ -268,7 +268,7 @@ void dwc3_gadget_giveback(struct dwc3_ep *dep, struct dwc3_request *req,
>  			req->request.length, status);
>  
>  	spin_unlock(&dwc->lock);
> -	req->request.complete(&dep->endpoint, &req->request);
> +	usb_gadget_giveback_request(&dep->endpoint, &req->request);
>  	spin_lock(&dwc->lock);
>  }
>  
> diff --git a/drivers/usb/gadget/udc/amd5536udc.c b/drivers/usb/gadget/udc/amd5536udc.c
> index 41b062e..3b9d138 100644
> --- a/drivers/usb/gadget/udc/amd5536udc.c
> +++ b/drivers/usb/gadget/udc/amd5536udc.c
> @@ -841,7 +841,7 @@ __acquires(ep->dev->lock)
>  		&req->req, req->req.length, ep->ep.name, sts);
>  
>  	spin_unlock(&dev->lock);
> -	req->req.complete(&ep->ep, &req->req);
> +	usb_gadget_giveback_request(&ep->ep, &req->req);
>  	spin_lock(&dev->lock);
>  	ep->halted = halted;
>  }
> diff --git a/drivers/usb/gadget/udc/at91_udc.c b/drivers/usb/gadget/udc/at91_udc.c
> index cfd18bc..9968f53 100644
> --- a/drivers/usb/gadget/udc/at91_udc.c
> +++ b/drivers/usb/gadget/udc/at91_udc.c
> @@ -267,7 +267,7 @@ static void done(struct at91_ep *ep, struct at91_request *req, int status)
>  
>  	ep->stopped = 1;
>  	spin_unlock(&udc->lock);
> -	req->req.complete(&ep->ep, &req->req);
> +	usb_gadget_giveback_request(&ep->ep, &req->req);
>  	spin_lock(&udc->lock);
>  	ep->stopped = stopped;
>  
> diff --git a/drivers/usb/gadget/udc/atmel_usba_udc.c b/drivers/usb/gadget/udc/atmel_usba_udc.c
> index 906e65f..e96032f 100644
> --- a/drivers/usb/gadget/udc/atmel_usba_udc.c
> +++ b/drivers/usb/gadget/udc/atmel_usba_udc.c
> @@ -463,7 +463,7 @@ static void receive_data(struct usba_ep *ep)
>  			list_del_init(&req->queue);
>  			usba_ep_writel(ep, CTL_DIS, USBA_RX_BK_RDY);
>  			spin_unlock(&udc->lock);
> -			req->req.complete(&ep->ep, &req->req);
> +			usb_gadget_giveback_request(&ep->ep, &req->req);
>  			spin_lock(&udc->lock);
>  		}
>  
> @@ -495,7 +495,7 @@ request_complete(struct usba_ep *ep, struct usba_request *req, int status)
>  		ep->ep.name, req, req->req.status, req->req.actual);
>  
>  	spin_unlock(&udc->lock);
> -	req->req.complete(&ep->ep, &req->req);
> +	usb_gadget_giveback_request(&ep->ep, &req->req);
>  	spin_lock(&udc->lock);
>  }
>  
> diff --git a/drivers/usb/gadget/udc/bcm63xx_udc.c b/drivers/usb/gadget/udc/bcm63xx_udc.c
> index e969eb8..2235b88 100644
> --- a/drivers/usb/gadget/udc/bcm63xx_udc.c
> +++ b/drivers/usb/gadget/udc/bcm63xx_udc.c
> @@ -1088,7 +1088,7 @@ static int bcm63xx_ep_disable(struct usb_ep *ep)
>  			breq->req.status = -ESHUTDOWN;
>  
>  			spin_unlock_irqrestore(&udc->lock, flags);
> -			breq->req.complete(&iudma->bep->ep, &breq->req);
> +			usb_gadget_giveback_request(&iudma->bep->ep, &breq->req);
>  			spin_lock_irqsave(&udc->lock, flags);
>  		}
>  	}
> diff --git a/drivers/usb/gadget/udc/dummy_hcd.c b/drivers/usb/gadget/udc/dummy_hcd.c
> index 2b54955..81dc595 100644
> --- a/drivers/usb/gadget/udc/dummy_hcd.c
> +++ b/drivers/usb/gadget/udc/dummy_hcd.c
> @@ -258,7 +258,7 @@ static void nuke(struct dummy *dum, struct dummy_ep *ep)
>  		req->req.status = -ESHUTDOWN;
>  
>  		spin_unlock(&dum->lock);
> -		req->req.complete(&ep->ep, &req->req);
> +		usb_gadget_giveback_request(&ep->ep, &req->req);
>  		spin_lock(&dum->lock);
>  	}
>  }
> @@ -658,7 +658,7 @@ static int dummy_queue(struct usb_ep *_ep, struct usb_request *_req,
>  		spin_unlock(&dum->lock);
>  		_req->actual = _req->length;
>  		_req->status = 0;
> -		_req->complete(_ep, _req);
> +		usb_gadget_giveback_request(_ep, _req);
>  		spin_lock(&dum->lock);
>  	}  else
>  		list_add_tail(&req->queue, &ep->queue);
> @@ -702,7 +702,7 @@ static int dummy_dequeue(struct usb_ep *_ep, struct usb_request *_req)
>  		dev_dbg(udc_dev(dum),
>  				"dequeued req %p from %s, len %d buf %p\n",
>  				req, _ep->name, _req->length, _req->buf);
> -		_req->complete(_ep, _req);
> +		usb_gadget_giveback_request(_ep, _req);
>  	}
>  	local_irq_restore(flags);
>  	return retval;
> @@ -1385,7 +1385,7 @@ top:
>  			list_del_init(&req->queue);
>  
>  			spin_unlock(&dum->lock);
> -			req->req.complete(&ep->ep, &req->req);
> +			usb_gadget_giveback_request(&ep->ep, &req->req);
>  			spin_lock(&dum->lock);
>  
>  			/* requests might have been unlinked... */
> @@ -1761,7 +1761,7 @@ restart:
>  						req);
>  
>  				spin_unlock(&dum->lock);
> -				req->req.complete(&ep->ep, &req->req);
> +				usb_gadget_giveback_request(&ep->ep, &req->req);
>  				spin_lock(&dum->lock);
>  				ep->already_seen = 0;
>  				goto restart;
> diff --git a/drivers/usb/gadget/udc/fotg210-udc.c b/drivers/usb/gadget/udc/fotg210-udc.c
> index e143d69..1d31592 100644
> --- a/drivers/usb/gadget/udc/fotg210-udc.c
> +++ b/drivers/usb/gadget/udc/fotg210-udc.c
> @@ -70,7 +70,7 @@ static void fotg210_done(struct fotg210_ep *ep, struct fotg210_request *req,
>  		req->req.status = status;
>  
>  	spin_unlock(&ep->fotg210->lock);
> -	req->req.complete(&ep->ep, &req->req);
> +	usb_gadget_giveback_request(&ep->ep, &req->req);
>  	spin_lock(&ep->fotg210->lock);
>  
>  	if (ep->epnum) {
> diff --git a/drivers/usb/gadget/udc/fsl_qe_udc.c b/drivers/usb/gadget/udc/fsl_qe_udc.c
> index 7324308..dd18ea3 100644
> --- a/drivers/usb/gadget/udc/fsl_qe_udc.c
> +++ b/drivers/usb/gadget/udc/fsl_qe_udc.c
> @@ -118,10 +118,7 @@ static void done(struct qe_ep *ep, struct qe_req *req, int status)
>  	ep->stopped = 1;
>  	spin_unlock(&udc->lock);
>  
> -	/* this complete() should a func implemented by gadget layer,
> -	 * eg fsg->bulk_in_complete() */
> -	if (req->req.complete)
> -		req->req.complete(&ep->ep, &req->req);
> +	usb_gadget_giveback_request(&ep->ep, &req->req);
>  
>  	spin_lock(&udc->lock);
>  
> @@ -2728,4 +2725,3 @@ module_platform_driver(udc_driver);
>  MODULE_DESCRIPTION(DRIVER_DESC);
>  MODULE_AUTHOR(DRIVER_AUTHOR);
>  MODULE_LICENSE("GPL");
> -
> diff --git a/drivers/usb/gadget/udc/fsl_udc_core.c b/drivers/usb/gadget/udc/fsl_udc_core.c
> index 75b23ea..c362079 100644
> --- a/drivers/usb/gadget/udc/fsl_udc_core.c
> +++ b/drivers/usb/gadget/udc/fsl_udc_core.c
> @@ -197,10 +197,8 @@ __acquires(ep->udc->lock)
>  	ep->stopped = 1;
>  
>  	spin_unlock(&ep->udc->lock);
> -	/* complete() is from gadget layer,
> -	 * eg fsg->bulk_in_complete() */
> -	if (req->req.complete)
> -		req->req.complete(&ep->ep, &req->req);
> +
> +	usb_gadget_giveback_request(&ep->ep, &req->req);
>  
>  	spin_lock(&ep->udc->lock);
>  	ep->stopped = stopped;
> diff --git a/drivers/usb/gadget/udc/fusb300_udc.c b/drivers/usb/gadget/udc/fusb300_udc.c
> index d40255f..d52687d 100644
> --- a/drivers/usb/gadget/udc/fusb300_udc.c
> +++ b/drivers/usb/gadget/udc/fusb300_udc.c
> @@ -876,7 +876,7 @@ static void done(struct fusb300_ep *ep, struct fusb300_request *req,
>  		req->req.status = status;
>  
>  	spin_unlock(&ep->fusb300->lock);
> -	req->req.complete(&ep->ep, &req->req);
> +	usb_gadget_giveback_request(&ep->ep, &req->req);
>  	spin_lock(&ep->fusb300->lock);
>  
>  	if (ep->epnum) {
> diff --git a/drivers/usb/gadget/udc/goku_udc.c b/drivers/usb/gadget/udc/goku_udc.c
> index 6c85839..bf9c5ef 100644
> --- a/drivers/usb/gadget/udc/goku_udc.c
> +++ b/drivers/usb/gadget/udc/goku_udc.c
> @@ -320,7 +320,7 @@ done(struct goku_ep *ep, struct goku_request *req, int status)
>  	/* don't modify queue heads during completion callback */
>  	ep->stopped = 1;
>  	spin_unlock(&dev->lock);
> -	req->req.complete(&ep->ep, &req->req);
> +	usb_gadget_giveback_request(&ep->ep, &req->req);
>  	spin_lock(&dev->lock);
>  	ep->stopped = stopped;
>  }
> diff --git a/drivers/usb/gadget/udc/gr_udc.c b/drivers/usb/gadget/udc/gr_udc.c
> index 08df5c4..924533b 100644
> --- a/drivers/usb/gadget/udc/gr_udc.c
> +++ b/drivers/usb/gadget/udc/gr_udc.c
> @@ -339,7 +339,7 @@ static void gr_finish_request(struct gr_ep *ep, struct gr_request *req,
>  	} else if (req->req.complete) {
>  		spin_unlock(&dev->lock);
>  
> -		req->req.complete(&ep->ep, &req->req);
> +		usb_gadget_giveback_request(&ep->ep, &req->req);
>  
>  		spin_lock(&dev->lock);
>  	}
> diff --git a/drivers/usb/gadget/udc/lpc32xx_udc.c b/drivers/usb/gadget/udc/lpc32xx_udc.c
> index 1629ad7..feab0ba 100644
> --- a/drivers/usb/gadget/udc/lpc32xx_udc.c
> +++ b/drivers/usb/gadget/udc/lpc32xx_udc.c
> @@ -1479,7 +1479,7 @@ static void done(struct lpc32xx_ep *ep, struct lpc32xx_request *req, int status)
>  
>  	ep->req_pending = 0;
>  	spin_unlock(&udc->lock);
> -	req->req.complete(&ep->ep, &req->req);
> +	usb_gadget_giveback_request(&ep->ep, &req->req);
>  	spin_lock(&udc->lock);
>  }
>  
> diff --git a/drivers/usb/gadget/udc/m66592-udc.c b/drivers/usb/gadget/udc/m66592-udc.c
> index de88d33..8985656 100644
> --- a/drivers/usb/gadget/udc/m66592-udc.c
> +++ b/drivers/usb/gadget/udc/m66592-udc.c
> @@ -729,7 +729,7 @@ __acquires(m66592->lock)
>  		restart = 1;
>  
>  	spin_unlock(&ep->m66592->lock);
> -	req->req.complete(&ep->ep, &req->req);
> +	usb_gadget_giveback_request(&ep->ep, &req->req);
>  	spin_lock(&ep->m66592->lock);
>  
>  	if (restart) {
> diff --git a/drivers/usb/gadget/udc/mv_u3d_core.c b/drivers/usb/gadget/udc/mv_u3d_core.c
> index 1624871..046a1f8 100644
> --- a/drivers/usb/gadget/udc/mv_u3d_core.c
> +++ b/drivers/usb/gadget/udc/mv_u3d_core.c
> @@ -222,12 +222,8 @@ void mv_u3d_done(struct mv_u3d_ep *ep, struct mv_u3d_req *req, int status)
>  	}
>  
>  	spin_unlock(&ep->u3d->lock);
> -	/*
> -	 * complete() is from gadget layer,
> -	 * eg fsg->bulk_in_complete()
> -	 */
> -	if (req->req.complete)
> -		req->req.complete(&ep->ep, &req->req);
> +
> +	usb_gadget_giveback_request(&ep->ep, &req->req);
>  
>  	spin_lock(&ep->u3d->lock);
>  }
> diff --git a/drivers/usb/gadget/udc/mv_udc_core.c b/drivers/usb/gadget/udc/mv_udc_core.c
> index 040fb16..3c5db80 100644
> --- a/drivers/usb/gadget/udc/mv_udc_core.c
> +++ b/drivers/usb/gadget/udc/mv_udc_core.c
> @@ -248,12 +248,8 @@ static void done(struct mv_ep *ep, struct mv_req *req, int status)
>  	ep->stopped = 1;
>  
>  	spin_unlock(&ep->udc->lock);
> -	/*
> -	 * complete() is from gadget layer,
> -	 * eg fsg->bulk_in_complete()
> -	 */
> -	if (req->req.complete)
> -		req->req.complete(&ep->ep, &req->req);
> +
> +	usb_gadget_giveback_request(&ep->ep, &req->req);
>  
>  	spin_lock(&ep->udc->lock);
>  	ep->stopped = stopped;
> diff --git a/drivers/usb/gadget/udc/net2272.c b/drivers/usb/gadget/udc/net2272.c
> index 059cfe5..84d7162 100644
> --- a/drivers/usb/gadget/udc/net2272.c
> +++ b/drivers/usb/gadget/udc/net2272.c
> @@ -394,7 +394,7 @@ net2272_done(struct net2272_ep *ep, struct net2272_request *req, int status)
>  	/* don't modify queue heads during completion callback */
>  	ep->stopped = 1;
>  	spin_unlock(&dev->lock);
> -	req->req.complete(&ep->ep, &req->req);
> +	usb_gadget_giveback_request(&ep->ep, &req->req);
>  	spin_lock(&dev->lock);
>  	ep->stopped = stopped;
>  }
> diff --git a/drivers/usb/gadget/udc/net2280.c b/drivers/usb/gadget/udc/net2280.c
> index f4eac11..2cda166 100644
> --- a/drivers/usb/gadget/udc/net2280.c
> +++ b/drivers/usb/gadget/udc/net2280.c
> @@ -928,7 +928,7 @@ done(struct net2280_ep *ep, struct net2280_request *req, int status)
>  	/* don't modify queue heads during completion callback */
>  	ep->stopped = 1;
>  	spin_unlock(&dev->lock);
> -	req->req.complete(&ep->ep, &req->req);
> +	usb_gadget_giveback_request(&ep->ep, &req->req);
>  	spin_lock(&dev->lock);
>  	ep->stopped = stopped;
>  }
> diff --git a/drivers/usb/gadget/udc/omap_udc.c b/drivers/usb/gadget/udc/omap_udc.c
> index e731373..dcdfea4 100644
> --- a/drivers/usb/gadget/udc/omap_udc.c
> +++ b/drivers/usb/gadget/udc/omap_udc.c
> @@ -315,7 +315,7 @@ done(struct omap_ep *ep, struct omap_req *req, int status)
>  	/* don't modify queue heads during completion callback */
>  	ep->stopped = 1;
>  	spin_unlock(&ep->udc->lock);
> -	req->req.complete(&ep->ep, &req->req);
> +	usb_gadget_giveback_request(&ep->ep, &req->req);
>  	spin_lock(&ep->udc->lock);
>  	ep->stopped = stopped;
>  }
> diff --git a/drivers/usb/gadget/udc/pch_udc.c b/drivers/usb/gadget/udc/pch_udc.c
> index eb8c3be..0b69b32 100644
> --- a/drivers/usb/gadget/udc/pch_udc.c
> +++ b/drivers/usb/gadget/udc/pch_udc.c
> @@ -1486,7 +1486,7 @@ static void complete_req(struct pch_udc_ep *ep, struct pch_udc_request *req,
>  	spin_unlock(&dev->lock);
>  	if (!ep->in)
>  		pch_udc_ep_clear_rrdy(ep);
> -	req->req.complete(&ep->ep, &req->req);
> +	usb_gadget_giveback_request(&ep->ep, &req->req);
>  	spin_lock(&dev->lock);
>  	ep->halted = halted;
>  }
> diff --git a/drivers/usb/gadget/udc/pxa25x_udc.c b/drivers/usb/gadget/udc/pxa25x_udc.c
> index 251e4d5..42f7eeb 100644
> --- a/drivers/usb/gadget/udc/pxa25x_udc.c
> +++ b/drivers/usb/gadget/udc/pxa25x_udc.c
> @@ -347,7 +347,7 @@ static void done(struct pxa25x_ep *ep, struct pxa25x_request *req, int status)
>  
>  	/* don't modify queue heads during completion callback */
>  	ep->stopped = 1;
> -	req->req.complete(&ep->ep, &req->req);
> +	usb_gadget_giveback_request(&ep->ep, &req->req);
>  	ep->stopped = stopped;
>  }
>  
> diff --git a/drivers/usb/gadget/udc/pxa27x_udc.c b/drivers/usb/gadget/udc/pxa27x_udc.c
> index 597d39f..4868369 100644
> --- a/drivers/usb/gadget/udc/pxa27x_udc.c
> +++ b/drivers/usb/gadget/udc/pxa27x_udc.c
> @@ -758,7 +758,7 @@ static void req_done(struct pxa_ep *ep, struct pxa27x_request *req, int status,
>  	if (pflags)
>  		spin_unlock_irqrestore(&ep->lock, *pflags);
>  	local_irq_save(flags);
> -	req->req.complete(&req->udc_usb_ep->usb_ep, &req->req);
> +	usb_gadget_giveback_request(&req->udc_usb_ep->usb_ep, &req->req);
>  	local_irq_restore(flags);
>  	if (pflags)
>  		spin_lock_irqsave(&ep->lock, *pflags);
> diff --git a/drivers/usb/gadget/udc/r8a66597-udc.c b/drivers/usb/gadget/udc/r8a66597-udc.c
> index 4600842..0ec09a4 100644
> --- a/drivers/usb/gadget/udc/r8a66597-udc.c
> +++ b/drivers/usb/gadget/udc/r8a66597-udc.c
> @@ -925,7 +925,7 @@ __acquires(r8a66597->lock)
>  		sudmac_free_channel(ep->r8a66597, ep, req);
>  
>  	spin_unlock(&ep->r8a66597->lock);
> -	req->req.complete(&ep->ep, &req->req);
> +	usb_gadget_giveback_request(&ep->ep, &req->req);
>  	spin_lock(&ep->r8a66597->lock);
>  
>  	if (restart) {
> diff --git a/drivers/usb/gadget/udc/s3c-hsudc.c b/drivers/usb/gadget/udc/s3c-hsudc.c
> index 10c6a12..dfbf557 100644
> --- a/drivers/usb/gadget/udc/s3c-hsudc.c
> +++ b/drivers/usb/gadget/udc/s3c-hsudc.c
> @@ -258,8 +258,7 @@ static void s3c_hsudc_complete_request(struct s3c_hsudc_ep *hsep,
>  
>  	hsep->stopped = 1;
>  	spin_unlock(&hsudc->lock);
> -	if (hsreq->req.complete != NULL)
> -		hsreq->req.complete(&hsep->ep, &hsreq->req);
> +	usb_gadget_giveback_request(&hsep->ep, &hsreq->req);
>  	spin_lock(&hsudc->lock);
>  	hsep->stopped = stopped;
>  }
> diff --git a/drivers/usb/gadget/udc/s3c2410_udc.c b/drivers/usb/gadget/udc/s3c2410_udc.c
> index 357b58e..ff423d1 100644
> --- a/drivers/usb/gadget/udc/s3c2410_udc.c
> +++ b/drivers/usb/gadget/udc/s3c2410_udc.c
> @@ -272,7 +272,7 @@ static void s3c2410_udc_done(struct s3c2410_ep *ep,
>  		status = req->req.status;
>  
>  	ep->halted = 1;
> -	req->req.complete(&ep->ep, &req->req);
> +	usb_gadget_giveback_request(&ep->ep, &req->req);
>  	ep->halted = halted;
>  }
>  
> diff --git a/drivers/usb/gadget/udc/udc-core.c b/drivers/usb/gadget/udc/udc-core.c
> index b0d9817..c1df19b 100644
> --- a/drivers/usb/gadget/udc/udc-core.c
> +++ b/drivers/usb/gadget/udc/udc-core.c
> @@ -106,6 +106,23 @@ EXPORT_SYMBOL_GPL(usb_gadget_unmap_request);
>  
>  /* ------------------------------------------------------------------------- */
>  
> +/**
> + * usb_gadget_giveback_request - give the request back to the gadget layer
> + * Context: in_interrupt()
> + *
> + * This is called by device controller drivers in order to return the
> + * completed request back to the gadget layer.
> + */
> +void usb_gadget_giveback_request(struct usb_ep *ep,
> +		struct usb_request *req)
> +{
> +	BUG_ON(req->complete == NULL);

No way for a user to see what happened and their device is now crashed?
Not nice :(

Never add BUG_ON() to driver code, just handle the issue properly and
move on.

Yes, there are other uses of BUG_ON in the gadget code, they all should
be removed one day...

thanks,

greg k-h

[PATCH v4 2/3] usb: Rename usb-common.c

[Michal Sojka]

In the next commit, we will want the usb-common module to be composed of
two object files. Since Kbuild cannot "append" another object to an
existing one, we need to rename usb-common.c to something
else (common.c) and create usb-common.o by linking the wanted objects
together. Currently, usb-common.o comprises only common.o.

Signed-off-by: Michal Sojka <sojka@merica.cz>
---
 drivers/usb/common/Makefile                   | 4 +++-
 drivers/usb/common/{usb-common.c => common.c} | 0
 2 files changed, 3 insertions(+), 1 deletion(-)
 rename drivers/usb/common/{usb-common.c => common.c} (100%)

diff --git a/drivers/usb/common/Makefile b/drivers/usb/common/Makefile
index 7526461..052c120 100644
--- a/drivers/usb/common/Makefile
+++ b/drivers/usb/common/Makefile
@@ -2,5 +2,7 @@
 # Makefile for the usb common parts.
 #
 
-obj-$(CONFIG_USB_COMMON) += usb-common.o
+obj-$(CONFIG_USB_COMMON)	  += usb-common.o
+usb-common-y			  += common.o
+
 obj-$(CONFIG_USB_OTG_FSM) += usb-otg-fsm.o
diff --git a/drivers/usb/common/usb-common.c b/drivers/usb/common/common.c
similarity index 100%
rename from drivers/usb/common/usb-common.c
rename to drivers/usb/common/common.c
-- 
2.1.0

[PATCH v4 3/3] usb: Add LED triggers for USB activity

[Michal Sojka]

With this patch, USB activity can be signaled by blinking a LED. There
are two triggers, one for activity on USB host and one for USB gadget.

Both trigger should work with all host/device controllers. Tested only
with musb.

Signed-off-by: Michal Sojka <sojka@merica.cz>
---
 drivers/usb/Kconfig               | 11 ++++++++
 drivers/usb/common/Makefile       |  1 +
 drivers/usb/common/led.c          | 56 +++++++++++++++++++++++++++++++++++++++
 drivers/usb/core/hcd.c            |  2 ++
 drivers/usb/gadget/udc/udc-core.c |  4 +++
 include/linux/usb.h               | 12 +++++++++
 6 files changed, 86 insertions(+)
 create mode 100644 drivers/usb/common/led.c

diff --git a/drivers/usb/Kconfig b/drivers/usb/Kconfig
index e0cad441..fc90cda 100644
--- a/drivers/usb/Kconfig
+++ b/drivers/usb/Kconfig
@@ -147,4 +147,15 @@ source "drivers/usb/phy/Kconfig"
 
 source "drivers/usb/gadget/Kconfig"
 
+config USB_LED_TRIG
+	bool "USB LED Triggers"
+	depends on LEDS_CLASS && USB_COMMON
+	select LEDS_TRIGGERS
+	help
+	  This option adds LED triggers for USB host and/or gadget activity.
+
+	  Say Y here if you are working on a system with led-class supported
+	  LEDs and you want to use them as activity indicators for USB host or
+	  gadget.
+
 endif # USB_SUPPORT
diff --git a/drivers/usb/common/Makefile b/drivers/usb/common/Makefile
index 052c120..ca2f8bd 100644
--- a/drivers/usb/common/Makefile
+++ b/drivers/usb/common/Makefile
@@ -4,5 +4,6 @@
 
 obj-$(CONFIG_USB_COMMON)	  += usb-common.o
 usb-common-y			  += common.o
+usb-common-$(CONFIG_USB_LED_TRIG) += led.o
 
 obj-$(CONFIG_USB_OTG_FSM) += usb-otg-fsm.o
diff --git a/drivers/usb/common/led.c b/drivers/usb/common/led.c
new file mode 100644
index 0000000..af3ce2c
--- /dev/null
+++ b/drivers/usb/common/led.c
@@ -0,0 +1,56 @@
+/*
+ * LED Triggers for USB Activity
+ *
+ * Copyright 2014 Michal Sojka <sojka@merica.cz>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ */
+
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/init.h>
+#include <linux/leds.h>
+#include <linux/usb.h>
+
+#define BLINK_DELAY 30
+
+static unsigned long usb_blink_delay = BLINK_DELAY;
+
+DEFINE_LED_TRIGGER(ledtrig_usb_gadget);
+DEFINE_LED_TRIGGER(ledtrig_usb_host);
+
+void usb_led_activity(enum usb_led_event ev)
+{
+	struct led_trigger *trig = NULL;
+
+	switch (ev) {
+	case USB_LED_EVENT_GADGET: trig = ledtrig_usb_gadget; break;
+	case USB_LED_EVENT_HOST:   trig = ledtrig_usb_host;    break;
+	}
+	led_trigger_blink_oneshot(trig, &usb_blink_delay, &usb_blink_delay, 0);
+}
+EXPORT_SYMBOL(usb_led_activity);
+
+
+static int __init ledtrig_usb_init(void)
+{
+#ifdef CONFIG_USB_GADGET
+	led_trigger_register_simple("usb-gadget", &ledtrig_usb_gadget);
+#endif
+#ifdef CONFIG_USB
+	led_trigger_register_simple("usb-host", &ledtrig_usb_host);
+#endif
+	return 0;
+}
+
+static void __exit ledtrig_usb_exit(void)
+{
+	led_trigger_unregister_simple(ledtrig_usb_gadget);
+	led_trigger_unregister_simple(ledtrig_usb_host);
+}
+
+module_init(ledtrig_usb_init);
+module_exit(ledtrig_usb_exit);
diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
index 487abcf..409cb95 100644
--- a/drivers/usb/core/hcd.c
+++ b/drivers/usb/core/hcd.c
@@ -1664,6 +1664,8 @@ static void __usb_hcd_giveback_urb(struct urb *urb)
 	usbmon_urb_complete(&hcd->self, urb, status);
 	usb_anchor_suspend_wakeups(anchor);
 	usb_unanchor_urb(urb);
+	if (likely(status == 0))
+		usb_led_activity(USB_LED_EVENT_HOST);
 
 	/* pass ownership to the completion handler */
 	urb->status = status;
diff --git a/drivers/usb/gadget/udc/udc-core.c b/drivers/usb/gadget/udc/udc-core.c
index c1df19b..9fc4a36 100644
--- a/drivers/usb/gadget/udc/udc-core.c
+++ b/drivers/usb/gadget/udc/udc-core.c
@@ -27,6 +27,7 @@
 
 #include <linux/usb/ch9.h>
 #include <linux/usb/gadget.h>
+#include <linux/usb.h>
 
 /**
  * struct usb_udc - describes one usb device controller
@@ -116,6 +117,9 @@ EXPORT_SYMBOL_GPL(usb_gadget_unmap_request);
 void usb_gadget_giveback_request(struct usb_ep *ep,
 		struct usb_request *req)
 {
+	if (likely(req->status == 0))
+		usb_led_activity(USB_LED_EVENT_GADGET);
+
 	BUG_ON(req->complete == NULL);
 	req->complete(ep, req);
 }
diff --git a/include/linux/usb.h b/include/linux/usb.h
index d2465bc..447a7e2 100644
--- a/include/linux/usb.h
+++ b/include/linux/usb.h
@@ -1862,6 +1862,18 @@ extern void usb_unregister_notify(struct notifier_block *nb);
 /* debugfs stuff */
 extern struct dentry *usb_debug_root;
 
+/* LED triggers */
+enum usb_led_event {
+	USB_LED_EVENT_HOST = 0,
+	USB_LED_EVENT_GADGET = 1,
+};
+
+#ifdef CONFIG_USB_LED_TRIG
+extern void usb_led_activity(enum usb_led_event ev);
+#else
+static inline void usb_led_activity(enum usb_led_event ev) {}
+#endif
+
 #endif  /* __KERNEL__ */
 
 #endif
-- 
2.1.0

Re: [PATCH v4 3/3] usb: Add LED triggers for USB activity

[Greg Kroah-Hartman]

On Wed, Aug 27, 2014 at 10:58:00PM +0200, Michal Sojka wrote:
> With this patch, USB activity can be signaled by blinking a LED. There
> are two triggers, one for activity on USB host and one for USB gadget.
> 
> Both trigger should work with all host/device controllers. Tested only
> with musb.
> 
> Signed-off-by: Michal Sojka <sojka@merica.cz>
> ---
>  drivers/usb/Kconfig               | 11 ++++++++
>  drivers/usb/common/Makefile       |  1 +
>  drivers/usb/common/led.c          | 56 +++++++++++++++++++++++++++++++++++++++
>  drivers/usb/core/hcd.c            |  2 ++
>  drivers/usb/gadget/udc/udc-core.c |  4 +++
>  include/linux/usb.h               | 12 +++++++++
>  6 files changed, 86 insertions(+)
>  create mode 100644 drivers/usb/common/led.c
> 
> diff --git a/drivers/usb/Kconfig b/drivers/usb/Kconfig
> index e0cad441..fc90cda 100644
> --- a/drivers/usb/Kconfig
> +++ b/drivers/usb/Kconfig
> @@ -147,4 +147,15 @@ source "drivers/usb/phy/Kconfig"
>  
>  source "drivers/usb/gadget/Kconfig"
>  
> +config USB_LED_TRIG
> +	bool "USB LED Triggers"
> +	depends on LEDS_CLASS && USB_COMMON
> +	select LEDS_TRIGGERS

I hate select lines, can't we just depend on LEDS_TRIGGERS as well as
LEDS_CLASS?


> +	help
> +	  This option adds LED triggers for USB host and/or gadget activity.
> +
> +	  Say Y here if you are working on a system with led-class supported
> +	  LEDs and you want to use them as activity indicators for USB host or
> +	  gadget.
> +
>  endif # USB_SUPPORT
> diff --git a/drivers/usb/common/Makefile b/drivers/usb/common/Makefile
> index 052c120..ca2f8bd 100644
> --- a/drivers/usb/common/Makefile
> +++ b/drivers/usb/common/Makefile
> @@ -4,5 +4,6 @@
>  
>  obj-$(CONFIG_USB_COMMON)	  += usb-common.o
>  usb-common-y			  += common.o
> +usb-common-$(CONFIG_USB_LED_TRIG) += led.o
>  
>  obj-$(CONFIG_USB_OTG_FSM) += usb-otg-fsm.o
> diff --git a/drivers/usb/common/led.c b/drivers/usb/common/led.c
> new file mode 100644
> index 0000000..af3ce2c
> --- /dev/null
> +++ b/drivers/usb/common/led.c
> @@ -0,0 +1,56 @@
> +/*
> + * LED Triggers for USB Activity
> + *
> + * Copyright 2014 Michal Sojka <sojka@merica.cz>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + */
> +
> +#include <linux/module.h>
> +#include <linux/kernel.h>
> +#include <linux/init.h>
> +#include <linux/leds.h>
> +#include <linux/usb.h>
> +
> +#define BLINK_DELAY 30
> +
> +static unsigned long usb_blink_delay = BLINK_DELAY;
> +
> +DEFINE_LED_TRIGGER(ledtrig_usb_gadget);
> +DEFINE_LED_TRIGGER(ledtrig_usb_host);
> +
> +void usb_led_activity(enum usb_led_event ev)
> +{
> +	struct led_trigger *trig = NULL;
> +
> +	switch (ev) {
> +	case USB_LED_EVENT_GADGET: trig = ledtrig_usb_gadget; break;
> +	case USB_LED_EVENT_HOST:   trig = ledtrig_usb_host;    break;
> +	}

Very odd formatting, please use the correct one and don't try to put
case expressions all on one line.

> +	led_trigger_blink_oneshot(trig, &usb_blink_delay, &usb_blink_delay, 0);

What happens if trig is NULL?

> +}
> +EXPORT_SYMBOL(usb_led_activity);

EXPORT_SYMBOL_GPL() please.

> +static int __init ledtrig_usb_init(void)
> +{
> +#ifdef CONFIG_USB_GADGET
> +	led_trigger_register_simple("usb-gadget", &ledtrig_usb_gadget);
> +#endif
> +#ifdef CONFIG_USB
> +	led_trigger_register_simple("usb-host", &ledtrig_usb_host);
> +#endif

Why not just always register both?

> +	return 0;
> +}
> +
> +static void __exit ledtrig_usb_exit(void)
> +{
> +	led_trigger_unregister_simple(ledtrig_usb_gadget);
> +	led_trigger_unregister_simple(ledtrig_usb_host);

So you can unregister things that you never registered with no issues?


> +}
> +
> +module_init(ledtrig_usb_init);
> +module_exit(ledtrig_usb_exit);
> diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
> index 487abcf..409cb95 100644
> --- a/drivers/usb/core/hcd.c
> +++ b/drivers/usb/core/hcd.c
> @@ -1664,6 +1664,8 @@ static void __usb_hcd_giveback_urb(struct urb *urb)
>  	usbmon_urb_complete(&hcd->self, urb, status);
>  	usb_anchor_suspend_wakeups(anchor);
>  	usb_unanchor_urb(urb);
> +	if (likely(status == 0))
> +		usb_led_activity(USB_LED_EVENT_HOST);

That's a _really_ hot code path, how much is this going to cause in
overhead?

>  	/* pass ownership to the completion handler */
>  	urb->status = status;
> diff --git a/drivers/usb/gadget/udc/udc-core.c b/drivers/usb/gadget/udc/udc-core.c
> index c1df19b..9fc4a36 100644
> --- a/drivers/usb/gadget/udc/udc-core.c
> +++ b/drivers/usb/gadget/udc/udc-core.c
> @@ -27,6 +27,7 @@
>  
>  #include <linux/usb/ch9.h>
>  #include <linux/usb/gadget.h>
> +#include <linux/usb.h>
>  
>  /**
>   * struct usb_udc - describes one usb device controller
> @@ -116,6 +117,9 @@ EXPORT_SYMBOL_GPL(usb_gadget_unmap_request);
>  void usb_gadget_giveback_request(struct usb_ep *ep,
>  		struct usb_request *req)
>  {
> +	if (likely(req->status == 0))
> +		usb_led_activity(USB_LED_EVENT_GADGET);

Same here.

>  	BUG_ON(req->complete == NULL);
>  	req->complete(ep, req);
>  }
> diff --git a/include/linux/usb.h b/include/linux/usb.h
> index d2465bc..447a7e2 100644
> --- a/include/linux/usb.h
> +++ b/include/linux/usb.h
> @@ -1862,6 +1862,18 @@ extern void usb_unregister_notify(struct notifier_block *nb);
>  /* debugfs stuff */
>  extern struct dentry *usb_debug_root;
>  
> +/* LED triggers */
> +enum usb_led_event {
> +	USB_LED_EVENT_HOST = 0,
> +	USB_LED_EVENT_GADGET = 1,
> +};
> +
> +#ifdef CONFIG_USB_LED_TRIG
> +extern void usb_led_activity(enum usb_led_event ev);
> +#else
> +static inline void usb_led_activity(enum usb_led_event ev) {}
> +#endif
> +
>  #endif  /* __KERNEL__ */
>  
>  #endif
> -- 
> 2.1.0
> 
> --
> 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

Re: [PATCH v4 3/3] usb: Add LED triggers for USB activity

[Michal Sojka]

On Wed, Aug 27 2014, Greg Kroah-Hartman wrote:
> On Wed, Aug 27, 2014 at 10:58:00PM +0200, Michal Sojka wrote:
>> With this patch, USB activity can be signaled by blinking a LED. There
>> are two triggers, one for activity on USB host and one for USB gadget.
>> 
>> Both trigger should work with all host/device controllers. Tested only
>> with musb.
>> 
>> Signed-off-by: Michal Sojka <sojka@merica.cz>
>> ---
>>  drivers/usb/Kconfig               | 11 ++++++++
>>  drivers/usb/common/Makefile       |  1 +
>>  drivers/usb/common/led.c          | 56 +++++++++++++++++++++++++++++++++++++++
>>  drivers/usb/core/hcd.c            |  2 ++
>>  drivers/usb/gadget/udc/udc-core.c |  4 +++
>>  include/linux/usb.h               | 12 +++++++++
>>  6 files changed, 86 insertions(+)
>>  create mode 100644 drivers/usb/common/led.c
>> 
>> diff --git a/drivers/usb/Kconfig b/drivers/usb/Kconfig
>> index e0cad441..fc90cda 100644
>> --- a/drivers/usb/Kconfig
>> +++ b/drivers/usb/Kconfig
>> @@ -147,4 +147,15 @@ source "drivers/usb/phy/Kconfig"
>>  
>>  source "drivers/usb/gadget/Kconfig"
>>  
>> +config USB_LED_TRIG
>> +	bool "USB LED Triggers"
>> +	depends on LEDS_CLASS && USB_COMMON
>> +	select LEDS_TRIGGERS
>
> I hate select lines, can't we just depend on LEDS_TRIGGERS as well as
> LEDS_CLASS?
>
>
>> +	help
>> +	  This option adds LED triggers for USB host and/or gadget activity.
>> +
>> +	  Say Y here if you are working on a system with led-class supported
>> +	  LEDs and you want to use them as activity indicators for USB host or
>> +	  gadget.
>> +
>>  endif # USB_SUPPORT
>> diff --git a/drivers/usb/common/Makefile b/drivers/usb/common/Makefile
>> index 052c120..ca2f8bd 100644
>> --- a/drivers/usb/common/Makefile
>> +++ b/drivers/usb/common/Makefile
>> @@ -4,5 +4,6 @@
>>  
>>  obj-$(CONFIG_USB_COMMON)	  += usb-common.o
>>  usb-common-y			  += common.o
>> +usb-common-$(CONFIG_USB_LED_TRIG) += led.o
>>  
>>  obj-$(CONFIG_USB_OTG_FSM) += usb-otg-fsm.o
>> diff --git a/drivers/usb/common/led.c b/drivers/usb/common/led.c
>> new file mode 100644
>> index 0000000..af3ce2c
>> --- /dev/null
>> +++ b/drivers/usb/common/led.c
>> @@ -0,0 +1,56 @@
>> +/*
>> + * LED Triggers for USB Activity
>> + *
>> + * Copyright 2014 Michal Sojka <sojka@merica.cz>
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License version 2 as
>> + * published by the Free Software Foundation.
>> + *
>> + */
>> +
>> +#include <linux/module.h>
>> +#include <linux/kernel.h>
>> +#include <linux/init.h>
>> +#include <linux/leds.h>
>> +#include <linux/usb.h>
>> +
>> +#define BLINK_DELAY 30
>> +
>> +static unsigned long usb_blink_delay = BLINK_DELAY;
>> +
>> +DEFINE_LED_TRIGGER(ledtrig_usb_gadget);
>> +DEFINE_LED_TRIGGER(ledtrig_usb_host);
>> +
>> +void usb_led_activity(enum usb_led_event ev)
>> +{
>> +	struct led_trigger *trig = NULL;
>> +
>> +	switch (ev) {
>> +	case USB_LED_EVENT_GADGET: trig = ledtrig_usb_gadget; break;
>> +	case USB_LED_EVENT_HOST:   trig = ledtrig_usb_host;    break;
>> +	}
>
> Very odd formatting, please use the correct one and don't try to put
> case expressions all on one line.
>
>> +	led_trigger_blink_oneshot(trig, &usb_blink_delay, &usb_blink_delay, 0);
>
> What happens if trig is NULL?

This will work correctly - I added a comment about it in v5.

>> +}
>> +EXPORT_SYMBOL(usb_led_activity);
>
> EXPORT_SYMBOL_GPL() please.
>
>> +static int __init ledtrig_usb_init(void)
>> +{
>> +#ifdef CONFIG_USB_GADGET
>> +	led_trigger_register_simple("usb-gadget", &ledtrig_usb_gadget);
>> +#endif
>> +#ifdef CONFIG_USB
>> +	led_trigger_register_simple("usb-host", &ledtrig_usb_host);
>> +#endif
>
> Why not just always register both?

I didn't want to offer users a trigger that doesn't do anything useful
on their system. Since you are the second person suggesting registering
both, I did just that in v5.

>> +	return 0;
>> +}
>> +
>> +static void __exit ledtrig_usb_exit(void)
>> +{
>> +	led_trigger_unregister_simple(ledtrig_usb_gadget);
>> +	led_trigger_unregister_simple(ledtrig_usb_host);
>
> So you can unregister things that you never registered with no issues?

Yes, but it doesn't matter anymore in v5.

>> +}
>> +
>> +module_init(ledtrig_usb_init);
>> +module_exit(ledtrig_usb_exit);
>> diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
>> index 487abcf..409cb95 100644
>> --- a/drivers/usb/core/hcd.c
>> +++ b/drivers/usb/core/hcd.c
>> @@ -1664,6 +1664,8 @@ static void __usb_hcd_giveback_urb(struct urb *urb)
>>  	usbmon_urb_complete(&hcd->self, urb, status);
>>  	usb_anchor_suspend_wakeups(anchor);
>>  	usb_unanchor_urb(urb);
>> +	if (likely(status == 0))
>> +		usb_led_activity(USB_LED_EVENT_HOST);
>
> That's a _really_ hot code path, how much is this going to cause in
> overhead?

I measured the overheads on ARM Cortex-A8 (TI AM335x) running on 600 MHz.

Duration of usb_led_activity():
- with no LED attached to the trigger:        2 ± 1 µs
- with one GPIO LED attached to the trigger:  2 ± 1 µs or 8 ± 2 µs (two peaks in histogram)

Duration of functions calling usb_led_activity()
(with my patches applied and no led attached to the trigger):
- __usb_hcd_giveback_urb():    10 - 25 µs
- usb_gadget_giveback_request(): 2 - 6 µs

With no LED attached, usb_led_activity() consists basically of
read_lock() and read_unlock().

I'll send v5 in a while.

Thanks,
-Michal

[PATCH v5 0/3] LED triggers for USB host and device

[Michal Sojka]

This adds LED triggers for USB host and device. First patch refactors
UDC drivers as requested by Felipe Balbi, second is a preparation for
the third, which adds the LED triggers.

Changes from v4:
- Replaced BUG_ON with pr_err (Alan Stern, greg k-h).
- Used proper coding style for switch statement (greg k-h).
- Added comment about NULL argument (greg k-h).
- EXPORT_SYMBOL changed to EXPORT_SYMBOL_GPL (greg k-h).
- Both triggers are now registerd even if host or gagdet subsystem
  is not enabled (Bryan Wu, greg k-h).
- Added performance numbers to the commit message of the last patch
  (greg k-h).

Changes from v3:
- usb_gadget_giveback_request() moved outside of CONFIG_HAS_DMA
  conditioned block.
- Added kernel-doc for usb_gadget_giveback_request() (Felipe Balbi).
- Removed outdated comment (Alan Stern).
- req->complete == NULL is now a bug. Previously, this was ignored
  (Alan Stern).
- File rename moved to a separate commit (greg k-h).

Changes from v2:
- Host/gadget triggers merged to a single file in usb/common/ (Felipe
  Balbi).
- UDC drivers refactored so that LED trigger works for all of them.

Changes from v1:
- Moved from drivers/leds/ to drivers/usb/.
- Improved Kconfig help.
- Linked with other modules rather than being standalone modules.

Michal Sojka (3):
  usb: gadget: Refactor request completion
  usb: Rename usb-common.c
  usb: Add LED triggers for USB activity

 drivers/usb/Kconfig                           | 10 +++++
 drivers/usb/chipidea/udc.c                    |  6 +--
 drivers/usb/common/Makefile                   |  5 ++-
 drivers/usb/common/{usb-common.c => common.c} |  0
 drivers/usb/common/led.c                      | 57 +++++++++++++++++++++++++++
 drivers/usb/core/hcd.c                        |  2 +
 drivers/usb/dwc2/gadget.c                     |  6 +--
 drivers/usb/dwc3/gadget.c                     |  2 +-
 drivers/usb/gadget/udc/amd5536udc.c           |  2 +-
 drivers/usb/gadget/udc/at91_udc.c             |  2 +-
 drivers/usb/gadget/udc/atmel_usba_udc.c       |  4 +-
 drivers/usb/gadget/udc/bcm63xx_udc.c          |  2 +-
 drivers/usb/gadget/udc/dummy_hcd.c            | 10 ++---
 drivers/usb/gadget/udc/fotg210-udc.c          |  2 +-
 drivers/usb/gadget/udc/fsl_qe_udc.c           |  6 +--
 drivers/usb/gadget/udc/fsl_udc_core.c         |  6 +--
 drivers/usb/gadget/udc/fusb300_udc.c          |  2 +-
 drivers/usb/gadget/udc/goku_udc.c             |  2 +-
 drivers/usb/gadget/udc/gr_udc.c               |  2 +-
 drivers/usb/gadget/udc/lpc32xx_udc.c          |  2 +-
 drivers/usb/gadget/udc/m66592-udc.c           |  2 +-
 drivers/usb/gadget/udc/mv_u3d_core.c          |  8 +---
 drivers/usb/gadget/udc/mv_udc_core.c          |  8 +---
 drivers/usb/gadget/udc/net2272.c              |  2 +-
 drivers/usb/gadget/udc/net2280.c              |  2 +-
 drivers/usb/gadget/udc/omap_udc.c             |  2 +-
 drivers/usb/gadget/udc/pch_udc.c              |  2 +-
 drivers/usb/gadget/udc/pxa25x_udc.c           |  2 +-
 drivers/usb/gadget/udc/pxa27x_udc.c           |  2 +-
 drivers/usb/gadget/udc/r8a66597-udc.c         |  2 +-
 drivers/usb/gadget/udc/s3c-hsudc.c            |  3 +-
 drivers/usb/gadget/udc/s3c2410_udc.c          |  2 +-
 drivers/usb/gadget/udc/udc-core.c             | 23 +++++++++++
 drivers/usb/musb/musb_gadget.c                |  2 +-
 drivers/usb/renesas_usbhs/mod_gadget.c        |  2 +-
 include/linux/usb.h                           | 12 ++++++
 include/linux/usb/gadget.h                    |  8 ++++
 37 files changed, 157 insertions(+), 57 deletions(-)
 rename drivers/usb/common/{usb-common.c => common.c} (100%)
 create mode 100644 drivers/usb/common/led.c

-- 
2.1.0

[PATCH v5 1/3] usb: gadget: Refactor request completion

[Michal Sojka]

All USB peripheral controller drivers called completion routines
directly. This patch moves the completion call from drivers to
usb_gadget_giveback_request(), in order to have a place where common
functionality can be added.

All places in drivers/usb/ matching "[-.]complete(" were replaced with a
call to usb_gadget_giveback_request(). This was compile-tested with all
ARM drivers enabled and runtime-tested for musb.

Signed-off-by: Michal Sojka <sojka@merica.cz>
---
 drivers/usb/chipidea/udc.c              |  6 +++---
 drivers/usb/dwc2/gadget.c               |  6 +++---
 drivers/usb/dwc3/gadget.c               |  2 +-
 drivers/usb/gadget/udc/amd5536udc.c     |  2 +-
 drivers/usb/gadget/udc/at91_udc.c       |  2 +-
 drivers/usb/gadget/udc/atmel_usba_udc.c |  4 ++--
 drivers/usb/gadget/udc/bcm63xx_udc.c    |  2 +-
 drivers/usb/gadget/udc/dummy_hcd.c      | 10 +++++-----
 drivers/usb/gadget/udc/fotg210-udc.c    |  2 +-
 drivers/usb/gadget/udc/fsl_qe_udc.c     |  6 +-----
 drivers/usb/gadget/udc/fsl_udc_core.c   |  6 ++----
 drivers/usb/gadget/udc/fusb300_udc.c    |  2 +-
 drivers/usb/gadget/udc/goku_udc.c       |  2 +-
 drivers/usb/gadget/udc/gr_udc.c         |  2 +-
 drivers/usb/gadget/udc/lpc32xx_udc.c    |  2 +-
 drivers/usb/gadget/udc/m66592-udc.c     |  2 +-
 drivers/usb/gadget/udc/mv_u3d_core.c    |  8 ++------
 drivers/usb/gadget/udc/mv_udc_core.c    |  8 ++------
 drivers/usb/gadget/udc/net2272.c        |  2 +-
 drivers/usb/gadget/udc/net2280.c        |  2 +-
 drivers/usb/gadget/udc/omap_udc.c       |  2 +-
 drivers/usb/gadget/udc/pch_udc.c        |  2 +-
 drivers/usb/gadget/udc/pxa25x_udc.c     |  2 +-
 drivers/usb/gadget/udc/pxa27x_udc.c     |  2 +-
 drivers/usb/gadget/udc/r8a66597-udc.c   |  2 +-
 drivers/usb/gadget/udc/s3c-hsudc.c      |  3 +--
 drivers/usb/gadget/udc/s3c2410_udc.c    |  2 +-
 drivers/usb/gadget/udc/udc-core.c       | 19 +++++++++++++++++++
 drivers/usb/musb/musb_gadget.c          |  2 +-
 drivers/usb/renesas_usbhs/mod_gadget.c  |  2 +-
 include/linux/usb/gadget.h              |  8 ++++++++
 31 files changed, 68 insertions(+), 56 deletions(-)

diff --git a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c
index b8125aa..0444d3f 100644
--- a/drivers/usb/chipidea/udc.c
+++ b/drivers/usb/chipidea/udc.c
@@ -627,7 +627,7 @@ __acquires(hwep->lock)
 
 		if (hwreq->req.complete != NULL) {
 			spin_unlock(hwep->lock);
-			hwreq->req.complete(&hwep->ep, &hwreq->req);
+			usb_gadget_giveback_request(&hwep->ep, &hwreq->req);
 			spin_lock(hwep->lock);
 		}
 	}
@@ -922,7 +922,7 @@ __acquires(hwep->lock)
 			if ((hwep->type == USB_ENDPOINT_XFER_CONTROL) &&
 					hwreq->req.length)
 				hweptemp = hwep->ci->ep0in;
-			hwreq->req.complete(&hweptemp->ep, &hwreq->req);
+			usb_gadget_giveback_request(&hweptemp->ep, &hwreq->req);
 			spin_lock(hwep->lock);
 		}
 	}
@@ -1347,7 +1347,7 @@ static int ep_dequeue(struct usb_ep *ep, struct usb_request *req)
 
 	if (hwreq->req.complete != NULL) {
 		spin_unlock(hwep->lock);
-		hwreq->req.complete(&hwep->ep, &hwreq->req);
+		usb_gadget_giveback_request(&hwep->ep, &hwreq->req);
 		spin_lock(hwep->lock);
 	}
 
diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
index 0ba9c33..5a524a6 100644
--- a/drivers/usb/dwc2/gadget.c
+++ b/drivers/usb/dwc2/gadget.c
@@ -987,8 +987,8 @@ static int s3c_hsotg_process_req_feature(struct s3c_hsotg *hsotg,
 					hs_req = ep->req;
 					ep->req = NULL;
 					list_del_init(&hs_req->queue);
-					hs_req->req.complete(&ep->ep,
-							     &hs_req->req);
+					usb_gadget_giveback_request(&ep->ep,
+								    &hs_req->req);
 				}
 
 				/* If we have pending request, then start it */
@@ -1245,7 +1245,7 @@ static void s3c_hsotg_complete_request(struct s3c_hsotg *hsotg,
 
 	if (hs_req->req.complete) {
 		spin_unlock(&hsotg->lock);
-		hs_req->req.complete(&hs_ep->ep, &hs_req->req);
+		usb_gadget_giveback_request(&hs_ep->ep, &hs_req->req);
 		spin_lock(&hsotg->lock);
 	}
 
diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index 349cacc..b4b7a6b 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -268,7 +268,7 @@ void dwc3_gadget_giveback(struct dwc3_ep *dep, struct dwc3_request *req,
 			req->request.length, status);
 
 	spin_unlock(&dwc->lock);
-	req->request.complete(&dep->endpoint, &req->request);
+	usb_gadget_giveback_request(&dep->endpoint, &req->request);
 	spin_lock(&dwc->lock);
 }
 
diff --git a/drivers/usb/gadget/udc/amd5536udc.c b/drivers/usb/gadget/udc/amd5536udc.c
index 41b062e..3b9d138 100644
--- a/drivers/usb/gadget/udc/amd5536udc.c
+++ b/drivers/usb/gadget/udc/amd5536udc.c
@@ -841,7 +841,7 @@ __acquires(ep->dev->lock)
 		&req->req, req->req.length, ep->ep.name, sts);
 
 	spin_unlock(&dev->lock);
-	req->req.complete(&ep->ep, &req->req);
+	usb_gadget_giveback_request(&ep->ep, &req->req);
 	spin_lock(&dev->lock);
 	ep->halted = halted;
 }
diff --git a/drivers/usb/gadget/udc/at91_udc.c b/drivers/usb/gadget/udc/at91_udc.c
index cfd18bc..9968f53 100644
--- a/drivers/usb/gadget/udc/at91_udc.c
+++ b/drivers/usb/gadget/udc/at91_udc.c
@@ -267,7 +267,7 @@ static void done(struct at91_ep *ep, struct at91_request *req, int status)
 
 	ep->stopped = 1;
 	spin_unlock(&udc->lock);
-	req->req.complete(&ep->ep, &req->req);
+	usb_gadget_giveback_request(&ep->ep, &req->req);
 	spin_lock(&udc->lock);
 	ep->stopped = stopped;
 
diff --git a/drivers/usb/gadget/udc/atmel_usba_udc.c b/drivers/usb/gadget/udc/atmel_usba_udc.c
index 906e65f..e96032f 100644
--- a/drivers/usb/gadget/udc/atmel_usba_udc.c
+++ b/drivers/usb/gadget/udc/atmel_usba_udc.c
@@ -463,7 +463,7 @@ static void receive_data(struct usba_ep *ep)
 			list_del_init(&req->queue);
 			usba_ep_writel(ep, CTL_DIS, USBA_RX_BK_RDY);
 			spin_unlock(&udc->lock);
-			req->req.complete(&ep->ep, &req->req);
+			usb_gadget_giveback_request(&ep->ep, &req->req);
 			spin_lock(&udc->lock);
 		}
 
@@ -495,7 +495,7 @@ request_complete(struct usba_ep *ep, struct usba_request *req, int status)
 		ep->ep.name, req, req->req.status, req->req.actual);
 
 	spin_unlock(&udc->lock);
-	req->req.complete(&ep->ep, &req->req);
+	usb_gadget_giveback_request(&ep->ep, &req->req);
 	spin_lock(&udc->lock);
 }
 
diff --git a/drivers/usb/gadget/udc/bcm63xx_udc.c b/drivers/usb/gadget/udc/bcm63xx_udc.c
index e969eb8..2235b88 100644
--- a/drivers/usb/gadget/udc/bcm63xx_udc.c
+++ b/drivers/usb/gadget/udc/bcm63xx_udc.c
@@ -1088,7 +1088,7 @@ static int bcm63xx_ep_disable(struct usb_ep *ep)
 			breq->req.status = -ESHUTDOWN;
 
 			spin_unlock_irqrestore(&udc->lock, flags);
-			breq->req.complete(&iudma->bep->ep, &breq->req);
+			usb_gadget_giveback_request(&iudma->bep->ep, &breq->req);
 			spin_lock_irqsave(&udc->lock, flags);
 		}
 	}
diff --git a/drivers/usb/gadget/udc/dummy_hcd.c b/drivers/usb/gadget/udc/dummy_hcd.c
index 2b54955..81dc595 100644
--- a/drivers/usb/gadget/udc/dummy_hcd.c
+++ b/drivers/usb/gadget/udc/dummy_hcd.c
@@ -258,7 +258,7 @@ static void nuke(struct dummy *dum, struct dummy_ep *ep)
 		req->req.status = -ESHUTDOWN;
 
 		spin_unlock(&dum->lock);
-		req->req.complete(&ep->ep, &req->req);
+		usb_gadget_giveback_request(&ep->ep, &req->req);
 		spin_lock(&dum->lock);
 	}
 }
@@ -658,7 +658,7 @@ static int dummy_queue(struct usb_ep *_ep, struct usb_request *_req,
 		spin_unlock(&dum->lock);
 		_req->actual = _req->length;
 		_req->status = 0;
-		_req->complete(_ep, _req);
+		usb_gadget_giveback_request(_ep, _req);
 		spin_lock(&dum->lock);
 	}  else
 		list_add_tail(&req->queue, &ep->queue);
@@ -702,7 +702,7 @@ static int dummy_dequeue(struct usb_ep *_ep, struct usb_request *_req)
 		dev_dbg(udc_dev(dum),
 				"dequeued req %p from %s, len %d buf %p\n",
 				req, _ep->name, _req->length, _req->buf);
-		_req->complete(_ep, _req);
+		usb_gadget_giveback_request(_ep, _req);
 	}
 	local_irq_restore(flags);
 	return retval;
@@ -1385,7 +1385,7 @@ top:
 			list_del_init(&req->queue);
 
 			spin_unlock(&dum->lock);
-			req->req.complete(&ep->ep, &req->req);
+			usb_gadget_giveback_request(&ep->ep, &req->req);
 			spin_lock(&dum->lock);
 
 			/* requests might have been unlinked... */
@@ -1761,7 +1761,7 @@ restart:
 						req);
 
 				spin_unlock(&dum->lock);
-				req->req.complete(&ep->ep, &req->req);
+				usb_gadget_giveback_request(&ep->ep, &req->req);
 				spin_lock(&dum->lock);
 				ep->already_seen = 0;
 				goto restart;
diff --git a/drivers/usb/gadget/udc/fotg210-udc.c b/drivers/usb/gadget/udc/fotg210-udc.c
index e143d69..1d31592 100644
--- a/drivers/usb/gadget/udc/fotg210-udc.c
+++ b/drivers/usb/gadget/udc/fotg210-udc.c
@@ -70,7 +70,7 @@ static void fotg210_done(struct fotg210_ep *ep, struct fotg210_request *req,
 		req->req.status = status;
 
 	spin_unlock(&ep->fotg210->lock);
-	req->req.complete(&ep->ep, &req->req);
+	usb_gadget_giveback_request(&ep->ep, &req->req);
 	spin_lock(&ep->fotg210->lock);
 
 	if (ep->epnum) {
diff --git a/drivers/usb/gadget/udc/fsl_qe_udc.c b/drivers/usb/gadget/udc/fsl_qe_udc.c
index 7324308..dd18ea3 100644
--- a/drivers/usb/gadget/udc/fsl_qe_udc.c
+++ b/drivers/usb/gadget/udc/fsl_qe_udc.c
@@ -118,10 +118,7 @@ static void done(struct qe_ep *ep, struct qe_req *req, int status)
 	ep->stopped = 1;
 	spin_unlock(&udc->lock);
 
-	/* this complete() should a func implemented by gadget layer,
-	 * eg fsg->bulk_in_complete() */
-	if (req->req.complete)
-		req->req.complete(&ep->ep, &req->req);
+	usb_gadget_giveback_request(&ep->ep, &req->req);
 
 	spin_lock(&udc->lock);
 
@@ -2728,4 +2725,3 @@ module_platform_driver(udc_driver);
 MODULE_DESCRIPTION(DRIVER_DESC);
 MODULE_AUTHOR(DRIVER_AUTHOR);
 MODULE_LICENSE("GPL");
-
diff --git a/drivers/usb/gadget/udc/fsl_udc_core.c b/drivers/usb/gadget/udc/fsl_udc_core.c
index 75b23ea..c362079 100644
--- a/drivers/usb/gadget/udc/fsl_udc_core.c
+++ b/drivers/usb/gadget/udc/fsl_udc_core.c
@@ -197,10 +197,8 @@ __acquires(ep->udc->lock)
 	ep->stopped = 1;
 
 	spin_unlock(&ep->udc->lock);
-	/* complete() is from gadget layer,
-	 * eg fsg->bulk_in_complete() */
-	if (req->req.complete)
-		req->req.complete(&ep->ep, &req->req);
+
+	usb_gadget_giveback_request(&ep->ep, &req->req);
 
 	spin_lock(&ep->udc->lock);
 	ep->stopped = stopped;
diff --git a/drivers/usb/gadget/udc/fusb300_udc.c b/drivers/usb/gadget/udc/fusb300_udc.c
index d40255f..d52687d 100644
--- a/drivers/usb/gadget/udc/fusb300_udc.c
+++ b/drivers/usb/gadget/udc/fusb300_udc.c
@@ -876,7 +876,7 @@ static void done(struct fusb300_ep *ep, struct fusb300_request *req,
 		req->req.status = status;
 
 	spin_unlock(&ep->fusb300->lock);
-	req->req.complete(&ep->ep, &req->req);
+	usb_gadget_giveback_request(&ep->ep, &req->req);
 	spin_lock(&ep->fusb300->lock);
 
 	if (ep->epnum) {
diff --git a/drivers/usb/gadget/udc/goku_udc.c b/drivers/usb/gadget/udc/goku_udc.c
index 6c85839..bf9c5ef 100644
--- a/drivers/usb/gadget/udc/goku_udc.c
+++ b/drivers/usb/gadget/udc/goku_udc.c
@@ -320,7 +320,7 @@ done(struct goku_ep *ep, struct goku_request *req, int status)
 	/* don't modify queue heads during completion callback */
 	ep->stopped = 1;
 	spin_unlock(&dev->lock);
-	req->req.complete(&ep->ep, &req->req);
+	usb_gadget_giveback_request(&ep->ep, &req->req);
 	spin_lock(&dev->lock);
 	ep->stopped = stopped;
 }
diff --git a/drivers/usb/gadget/udc/gr_udc.c b/drivers/usb/gadget/udc/gr_udc.c
index 08df5c4..924533b 100644
--- a/drivers/usb/gadget/udc/gr_udc.c
+++ b/drivers/usb/gadget/udc/gr_udc.c
@@ -339,7 +339,7 @@ static void gr_finish_request(struct gr_ep *ep, struct gr_request *req,
 	} else if (req->req.complete) {
 		spin_unlock(&dev->lock);
 
-		req->req.complete(&ep->ep, &req->req);
+		usb_gadget_giveback_request(&ep->ep, &req->req);
 
 		spin_lock(&dev->lock);
 	}
diff --git a/drivers/usb/gadget/udc/lpc32xx_udc.c b/drivers/usb/gadget/udc/lpc32xx_udc.c
index 1629ad7..feab0ba 100644
--- a/drivers/usb/gadget/udc/lpc32xx_udc.c
+++ b/drivers/usb/gadget/udc/lpc32xx_udc.c
@@ -1479,7 +1479,7 @@ static void done(struct lpc32xx_ep *ep, struct lpc32xx_request *req, int status)
 
 	ep->req_pending = 0;
 	spin_unlock(&udc->lock);
-	req->req.complete(&ep->ep, &req->req);
+	usb_gadget_giveback_request(&ep->ep, &req->req);
 	spin_lock(&udc->lock);
 }
 
diff --git a/drivers/usb/gadget/udc/m66592-udc.c b/drivers/usb/gadget/udc/m66592-udc.c
index de88d33..8985656 100644
--- a/drivers/usb/gadget/udc/m66592-udc.c
+++ b/drivers/usb/gadget/udc/m66592-udc.c
@@ -729,7 +729,7 @@ __acquires(m66592->lock)
 		restart = 1;
 
 	spin_unlock(&ep->m66592->lock);
-	req->req.complete(&ep->ep, &req->req);
+	usb_gadget_giveback_request(&ep->ep, &req->req);
 	spin_lock(&ep->m66592->lock);
 
 	if (restart) {
diff --git a/drivers/usb/gadget/udc/mv_u3d_core.c b/drivers/usb/gadget/udc/mv_u3d_core.c
index 1624871..046a1f8 100644
--- a/drivers/usb/gadget/udc/mv_u3d_core.c
+++ b/drivers/usb/gadget/udc/mv_u3d_core.c
@@ -222,12 +222,8 @@ void mv_u3d_done(struct mv_u3d_ep *ep, struct mv_u3d_req *req, int status)
 	}
 
 	spin_unlock(&ep->u3d->lock);
-	/*
-	 * complete() is from gadget layer,
-	 * eg fsg->bulk_in_complete()
-	 */
-	if (req->req.complete)
-		req->req.complete(&ep->ep, &req->req);
+
+	usb_gadget_giveback_request(&ep->ep, &req->req);
 
 	spin_lock(&ep->u3d->lock);
 }
diff --git a/drivers/usb/gadget/udc/mv_udc_core.c b/drivers/usb/gadget/udc/mv_udc_core.c
index 040fb16..3c5db80 100644
--- a/drivers/usb/gadget/udc/mv_udc_core.c
+++ b/drivers/usb/gadget/udc/mv_udc_core.c
@@ -248,12 +248,8 @@ static void done(struct mv_ep *ep, struct mv_req *req, int status)
 	ep->stopped = 1;
 
 	spin_unlock(&ep->udc->lock);
-	/*
-	 * complete() is from gadget layer,
-	 * eg fsg->bulk_in_complete()
-	 */
-	if (req->req.complete)
-		req->req.complete(&ep->ep, &req->req);
+
+	usb_gadget_giveback_request(&ep->ep, &req->req);
 
 	spin_lock(&ep->udc->lock);
 	ep->stopped = stopped;
diff --git a/drivers/usb/gadget/udc/net2272.c b/drivers/usb/gadget/udc/net2272.c
index 059cfe5..84d7162 100644
--- a/drivers/usb/gadget/udc/net2272.c
+++ b/drivers/usb/gadget/udc/net2272.c
@@ -394,7 +394,7 @@ net2272_done(struct net2272_ep *ep, struct net2272_request *req, int status)
 	/* don't modify queue heads during completion callback */
 	ep->stopped = 1;
 	spin_unlock(&dev->lock);
-	req->req.complete(&ep->ep, &req->req);
+	usb_gadget_giveback_request(&ep->ep, &req->req);
 	spin_lock(&dev->lock);
 	ep->stopped = stopped;
 }
diff --git a/drivers/usb/gadget/udc/net2280.c b/drivers/usb/gadget/udc/net2280.c
index f4eac11..2cda166 100644
--- a/drivers/usb/gadget/udc/net2280.c
+++ b/drivers/usb/gadget/udc/net2280.c
@@ -928,7 +928,7 @@ done(struct net2280_ep *ep, struct net2280_request *req, int status)
 	/* don't modify queue heads during completion callback */
 	ep->stopped = 1;
 	spin_unlock(&dev->lock);
-	req->req.complete(&ep->ep, &req->req);
+	usb_gadget_giveback_request(&ep->ep, &req->req);
 	spin_lock(&dev->lock);
 	ep->stopped = stopped;
 }
diff --git a/drivers/usb/gadget/udc/omap_udc.c b/drivers/usb/gadget/udc/omap_udc.c
index e731373..dcdfea4 100644
--- a/drivers/usb/gadget/udc/omap_udc.c
+++ b/drivers/usb/gadget/udc/omap_udc.c
@@ -315,7 +315,7 @@ done(struct omap_ep *ep, struct omap_req *req, int status)
 	/* don't modify queue heads during completion callback */
 	ep->stopped = 1;
 	spin_unlock(&ep->udc->lock);
-	req->req.complete(&ep->ep, &req->req);
+	usb_gadget_giveback_request(&ep->ep, &req->req);
 	spin_lock(&ep->udc->lock);
 	ep->stopped = stopped;
 }
diff --git a/drivers/usb/gadget/udc/pch_udc.c b/drivers/usb/gadget/udc/pch_udc.c
index eb8c3be..0b69b32 100644
--- a/drivers/usb/gadget/udc/pch_udc.c
+++ b/drivers/usb/gadget/udc/pch_udc.c
@@ -1486,7 +1486,7 @@ static void complete_req(struct pch_udc_ep *ep, struct pch_udc_request *req,
 	spin_unlock(&dev->lock);
 	if (!ep->in)
 		pch_udc_ep_clear_rrdy(ep);
-	req->req.complete(&ep->ep, &req->req);
+	usb_gadget_giveback_request(&ep->ep, &req->req);
 	spin_lock(&dev->lock);
 	ep->halted = halted;
 }
diff --git a/drivers/usb/gadget/udc/pxa25x_udc.c b/drivers/usb/gadget/udc/pxa25x_udc.c
index 251e4d5..42f7eeb 100644
--- a/drivers/usb/gadget/udc/pxa25x_udc.c
+++ b/drivers/usb/gadget/udc/pxa25x_udc.c
@@ -347,7 +347,7 @@ static void done(struct pxa25x_ep *ep, struct pxa25x_request *req, int status)
 
 	/* don't modify queue heads during completion callback */
 	ep->stopped = 1;
-	req->req.complete(&ep->ep, &req->req);
+	usb_gadget_giveback_request(&ep->ep, &req->req);
 	ep->stopped = stopped;
 }
 
diff --git a/drivers/usb/gadget/udc/pxa27x_udc.c b/drivers/usb/gadget/udc/pxa27x_udc.c
index 597d39f..4868369 100644
--- a/drivers/usb/gadget/udc/pxa27x_udc.c
+++ b/drivers/usb/gadget/udc/pxa27x_udc.c
@@ -758,7 +758,7 @@ static void req_done(struct pxa_ep *ep, struct pxa27x_request *req, int status,
 	if (pflags)
 		spin_unlock_irqrestore(&ep->lock, *pflags);
 	local_irq_save(flags);
-	req->req.complete(&req->udc_usb_ep->usb_ep, &req->req);
+	usb_gadget_giveback_request(&req->udc_usb_ep->usb_ep, &req->req);
 	local_irq_restore(flags);
 	if (pflags)
 		spin_lock_irqsave(&ep->lock, *pflags);
diff --git a/drivers/usb/gadget/udc/r8a66597-udc.c b/drivers/usb/gadget/udc/r8a66597-udc.c
index 4600842..0ec09a4 100644
--- a/drivers/usb/gadget/udc/r8a66597-udc.c
+++ b/drivers/usb/gadget/udc/r8a66597-udc.c
@@ -925,7 +925,7 @@ __acquires(r8a66597->lock)
 		sudmac_free_channel(ep->r8a66597, ep, req);
 
 	spin_unlock(&ep->r8a66597->lock);
-	req->req.complete(&ep->ep, &req->req);
+	usb_gadget_giveback_request(&ep->ep, &req->req);
 	spin_lock(&ep->r8a66597->lock);
 
 	if (restart) {
diff --git a/drivers/usb/gadget/udc/s3c-hsudc.c b/drivers/usb/gadget/udc/s3c-hsudc.c
index 10c6a12..dfbf557 100644
--- a/drivers/usb/gadget/udc/s3c-hsudc.c
+++ b/drivers/usb/gadget/udc/s3c-hsudc.c
@@ -258,8 +258,7 @@ static void s3c_hsudc_complete_request(struct s3c_hsudc_ep *hsep,
 
 	hsep->stopped = 1;
 	spin_unlock(&hsudc->lock);
-	if (hsreq->req.complete != NULL)
-		hsreq->req.complete(&hsep->ep, &hsreq->req);
+	usb_gadget_giveback_request(&hsep->ep, &hsreq->req);
 	spin_lock(&hsudc->lock);
 	hsep->stopped = stopped;
 }
diff --git a/drivers/usb/gadget/udc/s3c2410_udc.c b/drivers/usb/gadget/udc/s3c2410_udc.c
index 357b58e..ff423d1 100644
--- a/drivers/usb/gadget/udc/s3c2410_udc.c
+++ b/drivers/usb/gadget/udc/s3c2410_udc.c
@@ -272,7 +272,7 @@ static void s3c2410_udc_done(struct s3c2410_ep *ep,
 		status = req->req.status;
 
 	ep->halted = 1;
-	req->req.complete(&ep->ep, &req->req);
+	usb_gadget_giveback_request(&ep->ep, &req->req);
 	ep->halted = halted;
 }
 
diff --git a/drivers/usb/gadget/udc/udc-core.c b/drivers/usb/gadget/udc/udc-core.c
index b0d9817..29789f1 100644
--- a/drivers/usb/gadget/udc/udc-core.c
+++ b/drivers/usb/gadget/udc/udc-core.c
@@ -106,6 +106,25 @@ EXPORT_SYMBOL_GPL(usb_gadget_unmap_request);
 
 /* ------------------------------------------------------------------------- */
 
+/**
+ * usb_gadget_giveback_request - give the request back to the gadget layer
+ * Context: in_interrupt()
+ *
+ * This is called by device controller drivers in order to return the
+ * completed request back to the gadget layer.
+ */
+void usb_gadget_giveback_request(struct usb_ep *ep,
+		struct usb_request *req)
+{
+	if (likely(req->complete))
+		req->complete(ep, req);
+	else
+		pr_err("%s : req->complete must not be NULL\n", __func__);
+}
+EXPORT_SYMBOL_GPL(usb_gadget_giveback_request);
+
+/* ------------------------------------------------------------------------- */
+
 static void usb_gadget_state_work(struct work_struct *work)
 {
 	struct usb_gadget	*gadget = work_to_gadget(work);
diff --git a/drivers/usb/musb/musb_gadget.c b/drivers/usb/musb/musb_gadget.c
index d4aa779..24c8c02 100644
--- a/drivers/usb/musb/musb_gadget.c
+++ b/drivers/usb/musb/musb_gadget.c
@@ -176,7 +176,7 @@ __acquires(ep->musb->lock)
 				ep->end_point.name, request,
 				req->request.actual, req->request.length,
 				request->status);
-	req->request.complete(&req->ep->end_point, &req->request);
+	usb_gadget_giveback_request(&req->ep->end_point, &req->request);
 	spin_lock(&musb->lock);
 	ep->busy = busy;
 }
diff --git a/drivers/usb/renesas_usbhs/mod_gadget.c b/drivers/usb/renesas_usbhs/mod_gadget.c
index 04e6505..2d17c10 100644
--- a/drivers/usb/renesas_usbhs/mod_gadget.c
+++ b/drivers/usb/renesas_usbhs/mod_gadget.c
@@ -129,7 +129,7 @@ static void usbhsg_queue_pop(struct usbhsg_uep *uep,
 	dev_dbg(dev, "pipe %d : queue pop\n", usbhs_pipe_number(pipe));
 
 	ureq->req.status = status;
-	ureq->req.complete(&uep->ep, &ureq->req);
+	usb_gadget_giveback_request(&uep->ep, &ureq->req);
 }
 
 static void usbhsg_queue_done(struct usbhs_priv *priv, struct usbhs_pkt *pkt)
diff --git a/include/linux/usb/gadget.h b/include/linux/usb/gadget.h
index c3a6185..f795e95 100644
--- a/include/linux/usb/gadget.h
+++ b/include/linux/usb/gadget.h
@@ -1013,6 +1013,14 @@ extern void usb_gadget_set_state(struct usb_gadget *gadget,
 
 /*-------------------------------------------------------------------------*/
 
+/* utility to give requests back to the gadget layer */
+
+extern void usb_gadget_giveback_request(struct usb_ep *ep,
+		struct usb_request *req);
+
+
+/*-------------------------------------------------------------------------*/
+
 /* utility wrapping a simple endpoint selection policy */
 
 extern struct usb_ep *usb_ep_autoconfig(struct usb_gadget *,
-- 
2.1.0

[PATCH v5 2/3] usb: Rename usb-common.c

[Michal Sojka]

In the next commit, we will want the usb-common module to be composed of
two object files. Since Kbuild cannot "append" another object to an
existing one, we need to rename usb-common.c to something
else (common.c) and create usb-common.o by linking the wanted objects
together. Currently, usb-common.o comprises only common.o.

Signed-off-by: Michal Sojka <sojka@merica.cz>
---
 drivers/usb/common/Makefile                   | 4 +++-
 drivers/usb/common/{usb-common.c => common.c} | 0
 2 files changed, 3 insertions(+), 1 deletion(-)
 rename drivers/usb/common/{usb-common.c => common.c} (100%)

diff --git a/drivers/usb/common/Makefile b/drivers/usb/common/Makefile
index 7526461..052c120 100644
--- a/drivers/usb/common/Makefile
+++ b/drivers/usb/common/Makefile
@@ -2,5 +2,7 @@
 # Makefile for the usb common parts.
 #
 
-obj-$(CONFIG_USB_COMMON) += usb-common.o
+obj-$(CONFIG_USB_COMMON)	  += usb-common.o
+usb-common-y			  += common.o
+
 obj-$(CONFIG_USB_OTG_FSM) += usb-otg-fsm.o
diff --git a/drivers/usb/common/usb-common.c b/drivers/usb/common/common.c
similarity index 100%
rename from drivers/usb/common/usb-common.c
rename to drivers/usb/common/common.c
-- 
2.1.0

[PATCH v5 3/3] usb: Add LED triggers for USB activity

[Michal Sojka]

With this patch, USB activity can be signaled by blinking a LED. There
are two triggers, one for activity on USB host and one for USB gadget.

Both triggers should work with all host/device controllers. Tested only
with musb.

Performace: I measured performance overheads on ARM Cortex-A8 (TI
AM335x) running on 600 MHz.

Duration of usb_led_activity():
- with no LED attached to the trigger:        2 ± 1 µs
- with one GPIO LED attached to the trigger:  2 ± 1 µs or 8 ± 2 µs (two peaks in histogram)

Duration of functions calling usb_led_activity() (with this patch
applied and no LED attached to the trigger):
- __usb_hcd_giveback_urb():    10 - 25 µs
- usb_gadget_giveback_request(): 2 - 6 µs

Signed-off-by: Michal Sojka <sojka@merica.cz>
---
 drivers/usb/Kconfig               | 10 +++++++
 drivers/usb/common/Makefile       |  1 +
 drivers/usb/common/led.c          | 57 +++++++++++++++++++++++++++++++++++++++
 drivers/usb/core/hcd.c            |  2 ++
 drivers/usb/gadget/udc/udc-core.c |  4 +++
 include/linux/usb.h               | 12 +++++++++
 6 files changed, 86 insertions(+)
 create mode 100644 drivers/usb/common/led.c

diff --git a/drivers/usb/Kconfig b/drivers/usb/Kconfig
index e0cad441..9d9cc3b 100644
--- a/drivers/usb/Kconfig
+++ b/drivers/usb/Kconfig
@@ -147,4 +147,14 @@ source "drivers/usb/phy/Kconfig"
 
 source "drivers/usb/gadget/Kconfig"
 
+config USB_LED_TRIG
+	bool "USB LED Triggers"
+	depends on LEDS_CLASS && USB_COMMON && LEDS_TRIGGERS
+	help
+	  This option adds LED triggers for USB host and/or gadget activity.
+
+	  Say Y here if you are working on a system with led-class supported
+	  LEDs and you want to use them as activity indicators for USB host or
+	  gadget.
+
 endif # USB_SUPPORT
diff --git a/drivers/usb/common/Makefile b/drivers/usb/common/Makefile
index 052c120..ca2f8bd 100644
--- a/drivers/usb/common/Makefile
+++ b/drivers/usb/common/Makefile
@@ -4,5 +4,6 @@
 
 obj-$(CONFIG_USB_COMMON)	  += usb-common.o
 usb-common-y			  += common.o
+usb-common-$(CONFIG_USB_LED_TRIG) += led.o
 
 obj-$(CONFIG_USB_OTG_FSM) += usb-otg-fsm.o
diff --git a/drivers/usb/common/led.c b/drivers/usb/common/led.c
new file mode 100644
index 0000000..df23da0
--- /dev/null
+++ b/drivers/usb/common/led.c
@@ -0,0 +1,57 @@
+/*
+ * LED Triggers for USB Activity
+ *
+ * Copyright 2014 Michal Sojka <sojka@merica.cz>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ */
+
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/init.h>
+#include <linux/leds.h>
+#include <linux/usb.h>
+
+#define BLINK_DELAY 30
+
+static unsigned long usb_blink_delay = BLINK_DELAY;
+
+DEFINE_LED_TRIGGER(ledtrig_usb_gadget);
+DEFINE_LED_TRIGGER(ledtrig_usb_host);
+
+void usb_led_activity(enum usb_led_event ev)
+{
+	struct led_trigger *trig = NULL;
+
+	switch (ev) {
+	case USB_LED_EVENT_GADGET:
+		trig = ledtrig_usb_gadget;
+		break;
+	case USB_LED_EVENT_HOST:
+		trig = ledtrig_usb_host;
+		break;
+	}
+	/* led_trigger_blink_oneshot() handles trig == NULL gracefully */
+	led_trigger_blink_oneshot(trig, &usb_blink_delay, &usb_blink_delay, 0);
+}
+EXPORT_SYMBOL_GPL(usb_led_activity);
+
+
+static int __init ledtrig_usb_init(void)
+{
+	led_trigger_register_simple("usb-gadget", &ledtrig_usb_gadget);
+	led_trigger_register_simple("usb-host", &ledtrig_usb_host);
+	return 0;
+}
+
+static void __exit ledtrig_usb_exit(void)
+{
+	led_trigger_unregister_simple(ledtrig_usb_gadget);
+	led_trigger_unregister_simple(ledtrig_usb_host);
+}
+
+module_init(ledtrig_usb_init);
+module_exit(ledtrig_usb_exit);
diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
index 487abcf..409cb95 100644
--- a/drivers/usb/core/hcd.c
+++ b/drivers/usb/core/hcd.c
@@ -1664,6 +1664,8 @@ static void __usb_hcd_giveback_urb(struct urb *urb)
 	usbmon_urb_complete(&hcd->self, urb, status);
 	usb_anchor_suspend_wakeups(anchor);
 	usb_unanchor_urb(urb);
+	if (likely(status == 0))
+		usb_led_activity(USB_LED_EVENT_HOST);
 
 	/* pass ownership to the completion handler */
 	urb->status = status;
diff --git a/drivers/usb/gadget/udc/udc-core.c b/drivers/usb/gadget/udc/udc-core.c
index 29789f1..f272cf0 100644
--- a/drivers/usb/gadget/udc/udc-core.c
+++ b/drivers/usb/gadget/udc/udc-core.c
@@ -27,6 +27,7 @@
 
 #include <linux/usb/ch9.h>
 #include <linux/usb/gadget.h>
+#include <linux/usb.h>
 
 /**
  * struct usb_udc - describes one usb device controller
@@ -116,6 +117,9 @@ EXPORT_SYMBOL_GPL(usb_gadget_unmap_request);
 void usb_gadget_giveback_request(struct usb_ep *ep,
 		struct usb_request *req)
 {
+	if (likely(req->status == 0))
+		usb_led_activity(USB_LED_EVENT_GADGET);
+
 	if (likely(req->complete))
 		req->complete(ep, req);
 	else
diff --git a/include/linux/usb.h b/include/linux/usb.h
index d2465bc..447a7e2 100644
--- a/include/linux/usb.h
+++ b/include/linux/usb.h
@@ -1862,6 +1862,18 @@ extern void usb_unregister_notify(struct notifier_block *nb);
 /* debugfs stuff */
 extern struct dentry *usb_debug_root;
 
+/* LED triggers */
+enum usb_led_event {
+	USB_LED_EVENT_HOST = 0,
+	USB_LED_EVENT_GADGET = 1,
+};
+
+#ifdef CONFIG_USB_LED_TRIG
+extern void usb_led_activity(enum usb_led_event ev);
+#else
+static inline void usb_led_activity(enum usb_led_event ev) {}
+#endif
+
 #endif  /* __KERNEL__ */
 
 #endif
-- 
2.1.0

[PATCH v2 3/3] usb: Add LED trigger for USB gadget activity

[Michal Sojka]

With this patch, USB gadget activity can be signaled by blinking a LED.

Since there is no generic code where to put the trigger for all USB
controllers, each USB controller needs to call the trigger individually.
This patch adds the call only for the musb controller where I can test
it.

Signed-off-by: Michal Sojka <sojka@merica.cz>
---
 drivers/usb/gadget/Kconfig      | 12 ++++++++++++
 drivers/usb/gadget/udc/Makefile |  5 ++++-
 drivers/usb/gadget/udc/led.c    | 38 ++++++++++++++++++++++++++++++++++++++
 drivers/usb/musb/musb_gadget.c  |  5 +++--
 include/linux/usb/gadget.h      | 10 ++++++++++
 5 files changed, 67 insertions(+), 3 deletions(-)
 create mode 100644 drivers/usb/gadget/udc/led.c

diff --git a/drivers/usb/gadget/Kconfig b/drivers/usb/gadget/Kconfig
index 5c822af..612c859 100644
--- a/drivers/usb/gadget/Kconfig
+++ b/drivers/usb/gadget/Kconfig
@@ -127,6 +127,18 @@ config USB_GADGET_STORAGE_NUM_BUFFERS
 	   a module parameter as well.
 	   If unsure, say 2.
 
+config USB_GADGET_LED
+	bool "USB Gadget LED Trigger"
+	depends on (USB_MUSB_GADGET || USB_MUSB_DUAL_ROLE)
+	depends on LEDS_CLASS
+	select LEDS_TRIGGERS
+	help
+	  This option adds a LED trigger for USB gadgets. The trigger will
+	  only work with supported USB device controllers.
+
+	  Say Y here if you are working on a system with led-class supported
+	  LEDs and you want to use them as USB gadget activity indicators.
+
 source "drivers/usb/gadget/udc/Kconfig"
 
 #
diff --git a/drivers/usb/gadget/udc/Makefile b/drivers/usb/gadget/udc/Makefile
index 4096122..acbe3ca 100644
--- a/drivers/usb/gadget/udc/Makefile
+++ b/drivers/usb/gadget/udc/Makefile
@@ -1,7 +1,10 @@
 #
 # USB peripheral controller drivers
 #
-obj-$(CONFIG_USB_GADGET)	+= udc-core.o
+obj-$(CONFIG_USB_GADGET)	+= udc.o
+udc-y				:= udc-core.o
+udc-$(CONFIG_USB_GADGET_LED)	+= led.o
+
 obj-$(CONFIG_USB_DUMMY_HCD)	+= dummy_hcd.o
 obj-$(CONFIG_USB_NET2272)	+= net2272.o
 obj-$(CONFIG_USB_NET2280)	+= net2280.o
diff --git a/drivers/usb/gadget/udc/led.c b/drivers/usb/gadget/udc/led.c
new file mode 100644
index 0000000..29a8b3f
--- /dev/null
+++ b/drivers/usb/gadget/udc/led.c
@@ -0,0 +1,38 @@
+/*
+ * LED Trigger for USB Gadget Activity
+ *
+ * Copyright 2014 Michal Sojka <sojka@merica.cz>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ */
+
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/init.h>
+#include <linux/leds.h>
+
+#define BLINK_DELAY 30
+
+DEFINE_LED_TRIGGER(ledtrig_usbgadget);
+static unsigned long usbgadget_blink_delay = BLINK_DELAY;
+
+void usb_gadget_led_activity(void)
+{
+	led_trigger_blink_oneshot(ledtrig_usbgadget,
+				  &usbgadget_blink_delay, &usbgadget_blink_delay, 0);
+}
+EXPORT_SYMBOL(usb_gadget_led_activity);
+
+int __init ledtrig_usbgadget_init(void)
+{
+	led_trigger_register_simple("usb-gadget", &ledtrig_usbgadget);
+	return 0;
+}
+
+void __exit ledtrig_usbgadget_exit(void)
+{
+	led_trigger_unregister_simple(ledtrig_usbgadget);
+}
diff --git a/drivers/usb/musb/musb_gadget.c b/drivers/usb/musb/musb_gadget.c
index d4aa779..0bf06f9 100644
--- a/drivers/usb/musb/musb_gadget.c
+++ b/drivers/usb/musb/musb_gadget.c
@@ -167,11 +167,12 @@ __acquires(ep->musb->lock)
 	if (!dma_mapping_error(&musb->g.dev, request->dma))
 		unmap_dma_buffer(req, musb);
 
-	if (request->status == 0)
+	if (request->status == 0) {
 		dev_dbg(musb->controller, "%s done request %p,  %d/%d\n",
 				ep->end_point.name, request,
 				req->request.actual, req->request.length);
-	else
+		usb_gadget_led_activity();
+	} else
 		dev_dbg(musb->controller, "%s request %p, %d/%d fault %d\n",
 				ep->end_point.name, request,
 				req->request.actual, req->request.length,
diff --git a/include/linux/usb/gadget.h b/include/linux/usb/gadget.h
index c3a6185..69bd9bc 100644
--- a/include/linux/usb/gadget.h
+++ b/include/linux/usb/gadget.h
@@ -1025,4 +1025,14 @@ extern struct usb_ep *usb_ep_autoconfig_ss(struct usb_gadget *,
 
 extern void usb_ep_autoconfig_reset(struct usb_gadget *);
 
+/*-------------------------------------------------------------------------*/
+
+/* LED trigger */
+
+#ifdef CONFIG_USB_GADGET_LED
+extern void usb_gadget_led_activity(void);
+#else
+static inline void usb_gadget_led_activity(void) {}
+#endif
+
 #endif /* __LINUX_USB_GADGET_H */
-- 
2.1.0

Re: [PATCH v2 0/3] LED triggers for USB host and device

[Bryan Wu]

On Fri, Aug 22, 2014 at 5:08 PM, Michal Sojka <sojka@merica.cz> wrote:
> This adds LED triggers for USB host and device.
>
> Changes from v1:
> - Moved from drivers/leds/ to drivers/usb/
> - Improved Kconfig help
> - Linked with other modules rather than being standalone modules
>
> Michal Sojka (3):
>   usb: Add missing #include
>   usb: Add LED trigger for USB host activity
>   usb: Add LED trigger for USB gadget activity
>
>  drivers/usb/core/Kconfig        |  9 +++++++++
>  drivers/usb/core/Makefile       |  1 +
>  drivers/usb/core/hcd.c          |  2 ++
>  drivers/usb/core/led.c          | 38 ++++++++++++++++++++++++++++++++++++++

I suggest put host trigger and gadget trigger together in
drivers/usb/core/led.c.

>  drivers/usb/gadget/Kconfig      | 12 ++++++++++++
>  drivers/usb/gadget/udc/Makefile |  5 ++++-
>  drivers/usb/gadget/udc/led.c    | 38 ++++++++++++++++++++++++++++++++++++++

We don't need a duplicated version for gadget.

>  drivers/usb/musb/musb_gadget.c  |  5 +++--
>  include/linux/usb/gadget.h      | 10 ++++++++++
>  include/linux/usb/hcd.h         |  7 +++++++
>  10 files changed, 124 insertions(+), 3 deletions(-)
>  create mode 100644 drivers/usb/core/led.c
>  create mode 100644 drivers/usb/gadget/udc/led.c
>
> --
> 2.1.0
>