| Message ID | 20221213041203.21080-1-chanh@os.amperecomputing.com |
|---|---|
| State | New |
| Headers | show |
| Series | USB: gadget: Add ID numbers to configfs-gadget driver names | expand |
On 13/12/2022 14:22, Christophe JAILLET wrote: > Le 13/12/2022 à 05:12, Chanh Nguyen a écrit : >> 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 >> >> 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-shex6MNQR2J/SfDzf78azzKzEDxYleXD@public.gmane.org> >> --- >> drivers/usb/gadget/configfs.c | 42 +++++++++++++++++++++++++++++++++++ >> 1 file changed, 42 insertions(+) >> >> diff --git a/drivers/usb/gadget/configfs.c >> b/drivers/usb/gadget/configfs.c >> index 96121d1c8df4..d8c5156ad777 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,16 @@ >> #include "u_f.h" >> #include "u_os_desc.h" >> +static DEFINE_IDA(driver_id_numbers); >> + >> +/* >> + * Driver name has the form of "configfs-gadget.%d", where %d >> + * is id allocated by ida_alloc(). The max value returns by >> + * ida_alloc() is INT_MAX, in 64-bit system, it is about nine >> + * quintillion: 19 digits in decimal. Set the max length to 35. >> + */ >> +#define DRIVER_NAME_LENGTH_MAX 35 > > Hi, > > if paranoiac, the final \0 seems to be missing in the max length > computation, but see below. Thanks CJ! Indeed, I have missed that. > >> + >> int check_user_usb_string(const char *name, >> struct usb_gadget_strings *stringtab_dev) >> { >> @@ -52,6 +63,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) >> @@ -1582,6 +1596,8 @@ static struct config_group *gadgets_make( >> const char *name) >> { >> struct gadget_info *gi; >> + char *driver_name; >> + int ret; >> gi = kzalloc(sizeof(*gi), GFP_KERNEL); >> if (!gi) >> @@ -1623,6 +1639,21 @@ static struct config_group *gadgets_make( >> gi->composite.gadget_driver = configfs_driver_template; >> + ret = ida_alloc(&driver_id_numbers, GFP_KERNEL); >> + if (ret < 0) >> + goto err; >> + gi->driver_id_number = ret; >> + >> + driver_name = kmalloc(DRIVER_NAME_LENGTH_MAX, GFP_KERNEL); >> + if (!driver_name) >> + goto out_free_driver_id_number; >> + >> + ret = scnprintf(driver_name, DRIVER_NAME_LENGTH_MAX, >> + "configfs-gadget.%d", gi->driver_id_number); > > > using kasprintf() looks simpler here. > No need to kmalloc()+scnprintf(), and no need for DRIVER_NAME_LENGTH_MAX. > > Just my 2c, > > CJ Thanks CJ for the review! I've made some changes as below (in gadgets_make() to remove unnecessary variables) and now trying to test it as much as possible. Will re-post it as v2 if looks good soon. static inline struct gadget_info *to_gadget_info(struct config_item *item) @@ -1623,13 +1629,25 @@ 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); + 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_id_number; return &gi->group; + +out_free_driver_id_number: + ida_free(&driver_id_numbers, gi->driver_id_number); err: kfree(gi); return ERR_PTR(-ENOMEM); > >> + if (ret < 0) >> + goto out_free_driver_name; >> + >> + gi->composite.gadget_driver.driver.name = driver_name; >> gi->composite.gadget_driver.function = kstrdup(name, GFP_KERNEL); >> gi->composite.name = gi->composite.gadget_driver.function; >> @@ -1630,6 +1661,11 @@ static struct config_group *gadgets_make( >> goto err; >> return &gi->group; >> + >> +out_free_driver_name: >> + kfree(driver_name); >> +out_free_driver_id_number: >> + ida_free(&driver_id_numbers, gi->driver_id_number); >> err: >> kfree(gi); >> return ERR_PTR(-ENOMEM); >> @@ -1637,6 +1673,12 @@ static struct config_group *gadgets_make( >> static void gadgets_drop(struct config_group *group, struct >> config_item *item) >> { >> + struct gadget_info *gi = to_gadget_info(item); >> + >> + mutex_lock(&gi->lock); >> + kfree(gi->composite.gadget_driver.driver.name); >> + ida_free(&driver_id_numbers, gi->driver_id_number); >> + mutex_unlock(&gi->lock); >> config_item_put(item); >> } >
> > Caution: EXT Email > > On 13/12/2022 14:22, Christophe JAILLET wrote: > > Le 13/12/2022 à 05:12, Chanh Nguyen a écrit : > >> 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 > >> > >> 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-shex6MNQR2J/SfDzf78azzKzEDxYleXD@public.gmane.org> > >> --- > >> drivers/usb/gadget/configfs.c | 42 > +++++++++++++++++++++++++++++++++++ > >> 1 file changed, 42 insertions(+) > >> > >> diff --git a/drivers/usb/gadget/configfs.c > >> b/drivers/usb/gadget/configfs.c > >> index 96121d1c8df4..d8c5156ad777 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,16 @@ > >> #include "u_f.h" > >> #include "u_os_desc.h" > >> +static DEFINE_IDA(driver_id_numbers); > >> + > >> +/* > >> + * Driver name has the form of "configfs-gadget.%d", where %d > >> + * is id allocated by ida_alloc(). The max value returns by > >> + * ida_alloc() is INT_MAX, in 64-bit system, it is about nine > >> + * quintillion: 19 digits in decimal. Set the max length to 35. > >> + */ > >> +#define DRIVER_NAME_LENGTH_MAX 35 > > > > Hi, > > > > if paranoiac, the final \0 seems to be missing in the max length > > computation, but see below. > > Thanks CJ! Indeed, I have missed that. > > > > >> + > >> int check_user_usb_string(const char *name, > >> struct usb_gadget_strings *stringtab_dev) > >> { > >> @@ -52,6 +63,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) > >> @@ -1582,6 +1596,8 @@ static struct config_group *gadgets_make( > >> const char *name) > >> { > >> struct gadget_info *gi; > >> + char *driver_name; > >> + int ret; > >> gi = kzalloc(sizeof(*gi), GFP_KERNEL); > >> if (!gi) > >> @@ -1623,6 +1639,21 @@ static struct config_group *gadgets_make( > >> gi->composite.gadget_driver = configfs_driver_template; > >> + ret = ida_alloc(&driver_id_numbers, GFP_KERNEL); > >> + if (ret < 0) > >> + goto err; > >> + gi->driver_id_number = ret; > >> + > >> + driver_name = kmalloc(DRIVER_NAME_LENGTH_MAX, GFP_KERNEL); > >> + if (!driver_name) > >> + goto out_free_driver_id_number; > >> + > >> + ret = scnprintf(driver_name, DRIVER_NAME_LENGTH_MAX, > >> + "configfs-gadget.%d", gi->driver_id_number); > > > > > > using kasprintf() looks simpler here. > > No need to kmalloc()+scnprintf(), and no need for > DRIVER_NAME_LENGTH_MAX. > > > > Just my 2c, > > > > CJ > > Thanks CJ for the review! > > I've made some changes as below (in gadgets_make() to remove > unnecessary > variables) and now trying to test it as much as possible. Will re-post > it as v2 if looks good soon. > > static inline struct gadget_info *to_gadget_info(struct config_item *item) > @@ -1623,13 +1629,25 @@ 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); > + 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_id_number; > > return &gi->group; > + > +out_free_driver_id_number: > + ida_free(&driver_id_numbers, gi->driver_id_number); > err: > kfree(gi); > return ERR_PTR(-ENOMEM); > > > > > >> + if (ret < 0) > >> + goto out_free_driver_name; > >> + > >> + gi->composite.gadget_driver.driver.name = driver_name; > >> gi->composite.gadget_driver.function = kstrdup(name, GFP_KERNEL); > >> gi->composite.name = gi->composite.gadget_driver.function; > >> @@ -1630,6 +1661,11 @@ static struct config_group *gadgets_make( > >> goto err; > >> return &gi->group; > >> + > >> +out_free_driver_name: > >> + kfree(driver_name); > >> +out_free_driver_id_number: > >> + ida_free(&driver_id_numbers, gi->driver_id_number); > >> err: > >> kfree(gi); > >> return ERR_PTR(-ENOMEM); > >> @@ -1637,6 +1673,12 @@ static struct config_group *gadgets_make( > >> static void gadgets_drop(struct config_group *group, struct > >> config_item *item) > >> { > >> + struct gadget_info *gi = to_gadget_info(item); > >> + > >> + mutex_lock(&gi->lock); > >> + kfree(gi->composite.gadget_driver.driver.name); > >> + ida_free(&driver_id_numbers, gi->driver_id_number); > >> + mutex_unlock(&gi->lock); Move all free into gadget_info_attr_release(), just before kfree(gi) Driver.name and gi create at the same place, Free should be the same place also. Best regards Frank Li > >> config_item_put(item); > >> } > >
On 14/12/2022 11:20, Frank Li wrote: >> >> Caution: EXT Email >> >> On 13/12/2022 14:22, Christophe JAILLET wrote: >>> Le 13/12/2022 à 05:12, Chanh Nguyen a écrit : >>>> 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 >>>> >>>> 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-shex6MNQR2J/SfDzf78azzKzEDxYleXD@public.gmane.org> >>>> --- >>>> drivers/usb/gadget/configfs.c | 42 >> +++++++++++++++++++++++++++++++++++ >>>> 1 file changed, 42 insertions(+) >>>> >>>> diff --git a/drivers/usb/gadget/configfs.c >>>> b/drivers/usb/gadget/configfs.c >>>> index 96121d1c8df4..d8c5156ad777 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,16 @@ >>>> #include "u_f.h" >>>> #include "u_os_desc.h" >>>> +static DEFINE_IDA(driver_id_numbers); >>>> + >>>> +/* >>>> + * Driver name has the form of "configfs-gadget.%d", where %d >>>> + * is id allocated by ida_alloc(). The max value returns by >>>> + * ida_alloc() is INT_MAX, in 64-bit system, it is about nine >>>> + * quintillion: 19 digits in decimal. Set the max length to 35. >>>> + */ >>>> +#define DRIVER_NAME_LENGTH_MAX 35 >>> >>> Hi, >>> >>> if paranoiac, the final \0 seems to be missing in the max length >>> computation, but see below. >> >> Thanks CJ! Indeed, I have missed that. >> >>> >>>> + >>>> int check_user_usb_string(const char *name, >>>> struct usb_gadget_strings *stringtab_dev) >>>> { >>>> @@ -52,6 +63,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) >>>> @@ -1582,6 +1596,8 @@ static struct config_group *gadgets_make( >>>> const char *name) >>>> { >>>> struct gadget_info *gi; >>>> + char *driver_name; >>>> + int ret; >>>> gi = kzalloc(sizeof(*gi), GFP_KERNEL); >>>> if (!gi) >>>> @@ -1623,6 +1639,21 @@ static struct config_group *gadgets_make( >>>> gi->composite.gadget_driver = configfs_driver_template; >>>> + ret = ida_alloc(&driver_id_numbers, GFP_KERNEL); >>>> + if (ret < 0) >>>> + goto err; >>>> + gi->driver_id_number = ret; >>>> + >>>> + driver_name = kmalloc(DRIVER_NAME_LENGTH_MAX, GFP_KERNEL); >>>> + if (!driver_name) >>>> + goto out_free_driver_id_number; >>>> + >>>> + ret = scnprintf(driver_name, DRIVER_NAME_LENGTH_MAX, >>>> + "configfs-gadget.%d", gi->driver_id_number); >>> >>> >>> using kasprintf() looks simpler here. >>> No need to kmalloc()+scnprintf(), and no need for >> DRIVER_NAME_LENGTH_MAX. >>> >>> Just my 2c, >>> >>> CJ >> >> Thanks CJ for the review! >> >> I've made some changes as below (in gadgets_make() to remove >> unnecessary >> variables) and now trying to test it as much as possible. Will re-post >> it as v2 if looks good soon. >> >> static inline struct gadget_info *to_gadget_info(struct config_item *item) >> @@ -1623,13 +1629,25 @@ 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); >> + 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_id_number; >> >> return &gi->group; >> + >> +out_free_driver_id_number: >> + ida_free(&driver_id_numbers, gi->driver_id_number); >> err: >> kfree(gi); >> return ERR_PTR(-ENOMEM); >> >> >>> >>>> + if (ret < 0) >>>> + goto out_free_driver_name; >>>> + >>>> + gi->composite.gadget_driver.driver.name = driver_name; >>>> gi->composite.gadget_driver.function = kstrdup(name, GFP_KERNEL); >>>> gi->composite.name = gi->composite.gadget_driver.function; >>>> @@ -1630,6 +1661,11 @@ static struct config_group *gadgets_make( >>>> goto err; >>>> return &gi->group; >>>> + >>>> +out_free_driver_name: >>>> + kfree(driver_name); >>>> +out_free_driver_id_number: >>>> + ida_free(&driver_id_numbers, gi->driver_id_number); >>>> err: >>>> kfree(gi); >>>> return ERR_PTR(-ENOMEM); >>>> @@ -1637,6 +1673,12 @@ static struct config_group *gadgets_make( >>>> static void gadgets_drop(struct config_group *group, struct >>>> config_item *item) >>>> { >>>> + struct gadget_info *gi = to_gadget_info(item); >>>> + >>>> + mutex_lock(&gi->lock); >>>> + kfree(gi->composite.gadget_driver.driver.name); >>>> + ida_free(&driver_id_numbers, gi->driver_id_number); >>>> + mutex_unlock(&gi->lock); > > Move all free into gadget_info_attr_release(), just before kfree(gi) > Driver.name and gi create at the same place, > Free should be the same place also. > Thanks a lot for the quick review comment. As per my observation through the test, on the first mount, the virtual-media the gadgets_make() is called, then later, when unmount, the gadgets_drop() is called and followed by gadget_info_attr_release(). The gadget_info_attr_release() is registered as .release() of gadget_root_type for the gi->group via the call "config_group_init_type_name(&gi->group, name, &gadget_root_type);" In general, the .release() will be called only for the group. There is nothing to guarantee that the group will always be registered, ie: incase without the call to "config_group_init_type_name(&gi->group, name, &gadget_root_type);" In this patch, what is added is an ida number to be used to make up the composite driver name. This is done in gadgets_make() so we'd like to add the cleanup code to gadgets_drop() as they are registered together in the same place and would be a little easier to read than adding them to _release() as the code below: static struct configfs_group_operations gadgets_ops = { .make_group = &gadgets_make, .drop_item = &gadgets_drop, }; Anyway, we still doubt that there might be something that we have missed so please let me know the reason why putting cleanup codes to _release() would be a better solution. Thank you and best regards, - Chanh > >>>> config_item_put(item); >>>> } >>>
] > > > > Move all free into gadget_info_attr_release(), just before kfree(gi) > > Driver.name and gi create at the same place, > > Free should be the same place also. > > > > Thanks a lot for the quick review comment. > > As per my observation through the test, on the first mount, the > virtual-media the gadgets_make() is called, then later, when unmount, > the gadgets_drop() is called and followed by gadget_info_attr_release(). > > The gadget_info_attr_release() is registered as .release() of > gadget_root_type for the gi->group via the call > "config_group_init_type_name(&gi->group, name, &gadget_root_type);" > > In general, the .release() will be called only for the group. There is > nothing to guarantee that the group will always be registered, ie: > incase without the call to "config_group_init_type_name(&gi->group, > name, &gadget_root_type);" > > In this patch, what is added is an ida number to be used to make up the > composite driver name. This is done in gadgets_make() so we'd like to > add the cleanup code to gadgets_drop() as they are registered together > in the same place and would be a little easier to read than adding them > to _release() as the code below: > > static struct configfs_group_operations gadgets_ops = { > .make_group = &gadgets_make, > .drop_item = &gadgets_drop, > }; > > Anyway, we still doubt that there might be something that we have missed > so please let me know the reason why putting cleanup codes to _release() > would be a better solution. Let's simply the logic Gadget_make() { gi=kazalloc(); gi->composite.gadget_driver.driver.name = kasprintf(); gi->compositr.gadget_drver.function = kstrdup(); } this is only place to create gi and driver.name. no other place to set driver.name, so I think gi take ownship of driver.name. so we only track when gi free, just free driver.name before gi free. Related malloc and free should be pair together. Gadget_info_attr_release() { Kfree(gi->composite.gadget_driver.function) Kfree(gi->composiste.gadget_driver.driver.name); Kfree(gi); } Gadget_driver.driver.name should be similar as gadget_driver.function If driver.name is freed in gadget_drop(), gi can not actually be reused again. > > Thank you and best regards, > - Chanh > > > > >>>> config_item_put(item); > >>>> } > >>>
On 14/12/2022 22:24, Alan Stern wrote: > [EXTERNAL EMAIL NOTICE: This email originated from an external sender. Please be mindful of safe email handling and proprietary information protection practices.] > > > On Wed, Dec 14, 2022 at 11:15:48AM +0700, Chanh Nguyen wrote: > >> Thanks CJ for the review! >> >> I've made some changes as below (in gadgets_make() to remove unnecessary >> variables) and now trying to test it as much as possible. Will re-post it as >> v2 if looks good soon. >> >> static inline struct gadget_info *to_gadget_info(struct config_item *item) >> @@ -1623,13 +1629,25 @@ 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); >> + 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_id_number; > > This should goto out_free_driver_name. > Thanks Alan! I'll update that. > >> >> return &gi->group; >> + >> +out_free_driver_id_number: >> + ida_free(&driver_id_numbers, gi->driver_id_number); >> err: >> kfree(gi); >> return ERR_PTR(-ENOMEM);
diff --git a/drivers/usb/gadget/configfs.c b/drivers/usb/gadget/configfs.c index 96121d1c8df4..d8c5156ad777 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,16 @@ #include "u_f.h" #include "u_os_desc.h" +static DEFINE_IDA(driver_id_numbers); + +/* + * Driver name has the form of "configfs-gadget.%d", where %d + * is id allocated by ida_alloc(). The max value returns by + * ida_alloc() is INT_MAX, in 64-bit system, it is about nine + * quintillion: 19 digits in decimal. Set the max length to 35. + */ +#define DRIVER_NAME_LENGTH_MAX 35 + int check_user_usb_string(const char *name, struct usb_gadget_strings *stringtab_dev) { @@ -52,6 +63,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) @@ -1582,6 +1596,8 @@ static struct config_group *gadgets_make( const char *name) { struct gadget_info *gi; + char *driver_name; + int ret; gi = kzalloc(sizeof(*gi), GFP_KERNEL); if (!gi) @@ -1623,6 +1639,21 @@ static struct config_group *gadgets_make( gi->composite.gadget_driver = configfs_driver_template; + ret = ida_alloc(&driver_id_numbers, GFP_KERNEL); + if (ret < 0) + goto err; + gi->driver_id_number = ret; + + driver_name = kmalloc(DRIVER_NAME_LENGTH_MAX, GFP_KERNEL); + if (!driver_name) + goto out_free_driver_id_number; + + ret = scnprintf(driver_name, DRIVER_NAME_LENGTH_MAX, + "configfs-gadget.%d", gi->driver_id_number); + if (ret < 0) + goto out_free_driver_name; + + gi->composite.gadget_driver.driver.name = driver_name; gi->composite.gadget_driver.function = kstrdup(name, GFP_KERNEL); gi->composite.name = gi->composite.gadget_driver.function; @@ -1630,6 +1661,11 @@ static struct config_group *gadgets_make( goto err; return &gi->group; + +out_free_driver_name: + kfree(driver_name); +out_free_driver_id_number: + ida_free(&driver_id_numbers, gi->driver_id_number); err: kfree(gi); return ERR_PTR(-ENOMEM); @@ -1637,6 +1673,12 @@ static struct config_group *gadgets_make( static void gadgets_drop(struct config_group *group, struct config_item *item) { + struct gadget_info *gi = to_gadget_info(item); + + mutex_lock(&gi->lock); + kfree(gi->composite.gadget_driver.driver.name); + ida_free(&driver_id_numbers, gi->driver_id_number); + mutex_unlock(&gi->lock); config_item_put(item); }
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 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@os.amperecomputing.com> --- drivers/usb/gadget/configfs.c | 42 +++++++++++++++++++++++++++++++++++ 1 file changed, 42 insertions(+)