USB: gadget: Add ID numbers to gadget names

Commit Message

Alan Stern May 10, 2022, 3:51 p.m. UTC

Putting USB gadgets on a new bus of their own encounters a problem
when multiple gadgets are present: They all have the same name!  The
driver core fails with a "sys: cannot create duplicate filename" error
when creating any of the /sys/bus/gadget/devices/<gadget-name>
symbolic links after the first.

This patch fixes the problem by adding a ".N" suffix to each gadget's
name when the gadget is registered (where N is a unique ID number),
thus making the names distinct.

Reported-and-tested-by: Geert Uytterhoeven <[e-mail]>
Signed-off-by: Alan Stern <[e-mail]>
Reviewed-by: Geert Uytterhoeven <[e-mail]>
Fixes: fc274c1e9973 ("USB: gadget: Add a new bus for gadgets")

---


[as1980]


 drivers/usb/gadget/udc/core.c |   16 ++++++++++++++--
 include/linux/usb/gadget.h    |    2 ++
 2 files changed, 16 insertions(+), 2 deletions(-)

Comments

Greg KH May 11, 2022, 3:17 p.m. UTC | #1

On Tue, May 10, 2022 at 11:51:29AM -0400, Alan Stern wrote:
> Putting USB gadgets on a new bus of their own encounters a problem
> when multiple gadgets are present: They all have the same name!  The
> driver core fails with a "sys: cannot create duplicate filename" error
> when creating any of the /sys/bus/gadget/devices/<gadget-name>
> symbolic links after the first.
> 
> This patch fixes the problem by adding a ".N" suffix to each gadget's
> name when the gadget is registered (where N is a unique ID number),
> thus making the names distinct.
> 
> Reported-and-tested-by: Geert Uytterhoeven <[e-mail]>
> Signed-off-by: Alan Stern <[e-mail]>
> Reviewed-by: Geert Uytterhoeven <[e-mail]>
> Fixes: fc274c1e9973 ("USB: gadget: Add a new bus for gadgets")
> 
> ---
> 
> 
> [as1980]

Thanks, let me run this through the Android test suite first.  You can
see the results here:
	https://android-review.googlesource.com/c/kernel/common/+/2095109

I'll let you know how it goes as I do not know if you are not logged in
if you can see the test results or not, gerrit is odd...

thanks,

greg k-h

Greg KH May 11, 2022, 4:58 p.m. UTC | #2

On Wed, May 11, 2022 at 05:17:48PM +0200, Greg KH wrote:
> On Tue, May 10, 2022 at 11:51:29AM -0400, Alan Stern wrote:
> > Putting USB gadgets on a new bus of their own encounters a problem
> > when multiple gadgets are present: They all have the same name!  The
> > driver core fails with a "sys: cannot create duplicate filename" error
> > when creating any of the /sys/bus/gadget/devices/<gadget-name>
> > symbolic links after the first.
> > 
> > This patch fixes the problem by adding a ".N" suffix to each gadget's
> > name when the gadget is registered (where N is a unique ID number),
> > thus making the names distinct.
> > 
> > Reported-and-tested-by: Geert Uytterhoeven <[e-mail]>
> > Signed-off-by: Alan Stern <[e-mail]>
> > Reviewed-by: Geert Uytterhoeven <[e-mail]>
> > Fixes: fc274c1e9973 ("USB: gadget: Add a new bus for gadgets")
> > 
> > ---
> > 
> > 
> > [as1980]
> 
> Thanks, let me run this through the Android test suite first.  You can
> see the results here:
> 	https://android-review.googlesource.com/c/kernel/common/+/2095109
> 
> I'll let you know how it goes as I do not know if you are not logged in
> if you can see the test results or not, gerrit is odd...

All passed, so I'll go queue this up now, thanks!

greg k-h

Patch

Index: usb-devel/drivers/usb/gadget/udc/core.c
===================================================================
--- usb-devel.orig/drivers/usb/gadget/udc/core.c
+++ usb-devel/drivers/usb/gadget/udc/core.c
@@ -12,6 +12,7 @@ 
 #include <linux/module.h>
 #include <linux/device.h>
 #include <linux/list.h>
+#include <linux/idr.h>
 #include <linux/err.h>
 #include <linux/dma-mapping.h>
 #include <linux/sched/task_stack.h>
@@ -23,6 +24,8 @@ 
 
 #include "trace.h"
 
+static DEFINE_IDA(gadget_id_numbers);
+
 static struct bus_type gadget_bus_type;
 
 /**
@@ -1248,7 +1251,6 @@  static void usb_udc_nop_release(struct d
 void usb_initialize_gadget(struct device *parent, struct usb_gadget *gadget,
 		void (*release)(struct device *dev))
 {
-	dev_set_name(&gadget->dev, "gadget");
 	INIT_WORK(&gadget->work, usb_gadget_state_work);
 	gadget->dev.parent = parent;
 
@@ -1304,12 +1306,21 @@  int usb_add_gadget(struct usb_gadget *ga
 	usb_gadget_set_state(gadget, USB_STATE_NOTATTACHED);
 	udc->vbus = true;
 
+	ret = ida_alloc(&gadget_id_numbers, GFP_KERNEL);
+	if (ret < 0)
+		goto err_del_udc;
+	gadget->id_number = ret;
+	dev_set_name(&gadget->dev, "gadget.%d", ret);
+
 	ret = device_add(&gadget->dev);
 	if (ret)
-		goto err_del_udc;
+		goto err_free_id;
 
 	return 0;
 
+ err_free_id:
+	ida_free(&gadget_id_numbers, gadget->id_number);
+
  err_del_udc:
 	flush_work(&gadget->work);
 	device_del(&udc->dev);
@@ -1417,6 +1428,7 @@  void usb_del_gadget(struct usb_gadget *g
 	kobject_uevent(&udc->dev.kobj, KOBJ_REMOVE);
 	flush_work(&gadget->work);
 	device_del(&gadget->dev);
+	ida_free(&gadget_id_numbers, gadget->id_number);
 	device_unregister(&udc->dev);
 }
 EXPORT_SYMBOL_GPL(usb_del_gadget);
Index: usb-devel/include/linux/usb/gadget.h
===================================================================
--- usb-devel.orig/include/linux/usb/gadget.h
+++ usb-devel/include/linux/usb/gadget.h
@@ -386,6 +386,7 @@  struct usb_gadget_ops {
  * @lpm_capable: If the gadget max_speed is FULL or HIGH, this flag
  *	indicates that it supports LPM as per the LPM ECN & errata.
  * @irq: the interrupt number for device controller.
+ * @id_number: a unique ID number for ensuring that gadget names are distinct
  *
  * Gadgets have a mostly-portable "gadget driver" implementing device
  * functions, handling all usb configurations and interfaces.  Gadget
@@ -446,6 +447,7 @@  struct usb_gadget {
 	unsigned			connected:1;
 	unsigned			lpm_capable:1;
 	int				irq;
+	int				id_number;
 };
 #define work_to_gadget(w)	(container_of((w), struct usb_gadget, work))