On 22/12/2022 22:20, Andrzej Pietrasiewicz wrote:
Hi,
W dniu 21.12.2022 o 10:13, Chanh Nguyen pisze:
It is unable to use configfs to attach more than one gadget. When
attaching the second gadget, it always fails and the kernel message
prints out:
Error: Driver 'configfs-gadget' is already registered, aborting...
UDC core: g1: driver registration failed: -16
I assume you are interested in a scenario where there is more than one
UDC available which means you can have more than one active gadget?
Yes, I'd like to have more active gadgets. For example, mass_storage and
ecm usb, ...
This commit fixes the problem by a ".N" suffix added to each
configfs_gadget's driver name (where N is a unique ID number),
thus making the names distinct.
Fixes: fc274c1e9973 ("USB: gadget: Add a new bus for gadgets")
Signed-off-by: Chanh Nguyen <chanh@xxxxxxxxxxxxxxxxxxxxxx>
---
Changes in v2:
- Replace scnprintf() by kasprintf() to simplify the code [CJ]
- Move the clean up code from gadgets_drop() to
gadget_info_attr_release() [Frank Li]
- Correct the resource free up in gadges_make() [Alan Stern]
- Remove the unnecessary variable in gadgets_make() [Chanh]
- Fixes minor grammar issue in commit message [Chanh]
---
drivers/usb/gadget/configfs.c | 25 ++++++++++++++++++++++++-
1 file changed, 24 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/gadget/configfs.c
b/drivers/usb/gadget/configfs.c
index 96121d1c8df4..7faf68bfa716 100644
--- a/drivers/usb/gadget/configfs.c
+++ b/drivers/usb/gadget/configfs.c
@@ -3,6 +3,7 @@
#include <linux/module.h>
#include <linux/slab.h>
#include <linux/device.h>
+#include <linux/idr.h>
#include <linux/kstrtox.h>
#include <linux/nls.h>
#include <linux/usb/composite.h>
@@ -11,6 +12,8 @@
#include "u_f.h"
#include "u_os_desc.h"
+static DEFINE_IDA(driver_id_numbers);
+
int check_user_usb_string(const char *name,
struct usb_gadget_strings *stringtab_dev)
{
@@ -52,6 +55,9 @@ struct gadget_info {
char qw_sign[OS_STRING_QW_SIGN_LEN];
spinlock_t spinlock;
bool unbind;
+
+ /* Make driver names unique */
+ int driver_id_number;
};
static inline struct gadget_info *to_gadget_info(struct config_item
*item)
@@ -393,6 +399,8 @@ static void gadget_info_attr_release(struct
config_item *item)
WARN_ON(!list_empty(&gi->string_list));
WARN_ON(!list_empty(&gi->available_func));
kfree(gi->composite.gadget_driver.function);
+ kfree(gi->composite.gadget_driver.driver.name);
+ ida_free(&driver_id_numbers, gi->driver_id_number);
kfree(gi);
}
@@ -1623,13 +1631,28 @@ static struct config_group *gadgets_make(
gi->composite.gadget_driver = configfs_driver_template;
+ gi->driver_id_number = ida_alloc(&driver_id_numbers, GFP_KERNEL);
+ if (gi->driver_id_number < 0)
+ goto err;
+
+ gi->composite.gadget_driver.driver.name = kasprintf(GFP_KERNEL,
+ "configfs-gadget.%d",
+ gi->driver_id_number);
I'm wondering if it maybe makes more sense to use the gadget name as a
suffix
instead?
gi->composite.gadget_driver.driver.name =
kasprintf(GFP_KERNEL, "configfs-gadget.%s" name);
So that when you
mkdir g1
you will ultimately see /sys/bus/gadget/drivers/configfs-gadget.g1
instead of /sys/bus/gadget/drivers/configfs-gadget.0
Gadget names are guaranteed to be unique because they are created
as sibling subdirectories in configfs. Your patch would then be greatly
simplified (no need for ida).
Regards,
Andrzej
Thanks, Andrzej! I'll update that in patch v3
+ if (!gi->composite.gadget_driver.driver.name)
+ goto out_free_driver_id_number;
+
gi->composite.gadget_driver.function = kstrdup(name, GFP_KERNEL);
gi->composite.name = gi->composite.gadget_driver.function;
if (!gi->composite.gadget_driver.function)
- goto err;
+ goto out_free_driver_name;
return &gi->group;
+
+out_free_driver_name:
+ kfree(gi->composite.gadget_driver.driver.name);
+out_free_driver_id_number:
+ ida_free(&driver_id_numbers, gi->driver_id_number);
err:
kfree(gi);
return ERR_PTR(-ENOMEM);
