On 20-12-29 14:44:43, Jack Pham wrote:
> From: Chandana Kishori Chiluveru <cchiluve@xxxxxxxxxxxxxx>
>
> When binding the ConfigFS gadget to a UDC, the functions in each
> configuration are added in list order. However, if usb_add_function()
> fails, the failed function is put back on its configuration's
> func_list and purge_configs_funcs() is called to further clean up.
>
> purge_configs_funcs() iterates over the configurations and functions
> in forward order, calling unbind() on each of the previously added
> functions. But after doing so, each function gets moved to the
> tail of the configuration's func_list. This results in reshuffling
> the original order of the functions within a configuration such
> that the failed function now appears first even though it may have
> originally appeared in the middle or even end of the list. At this
> point if the ConfigFS gadget is attempted to re-bind to the UDC,
> the functions will be added in a different order than intended,
> with the only recourse being to remove and relink the functions all
> over again.
>
> An example of this as follows:
>
> ln -s functions/mass_storage.0 configs/c.1
> ln -s functions/ncm.0 configs/c.1
> ln -s functions/ffs.adb configs/c.1 # oops, forgot to start adbd
> echo "<udc device>" > UDC # fails
> start adbd
> echo "<udc device>" > UDC # now succeeds, but...
> # bind order is
> # "ADB", mass_storage, ncm
>
> [30133.118289] configfs-gadget gadget: adding 'Mass Storage Function'/ffffff810af87200 to config 'c'/ffffff817d6a2520
> [30133.119875] configfs-gadget gadget: adding 'cdc_network'/ffffff80f48d1a00 to config 'c'/ffffff817d6a2520
> [30133.119974] using random self ethernet address
> [30133.120002] using random host ethernet address
> [30133.139604] usb0: HOST MAC 3e:27:46:ba:3e:26
> [30133.140015] usb0: MAC 6e:28:7e:42:66:6a
> [30133.140062] configfs-gadget gadget: adding 'Function FS Gadget'/ffffff80f3868438 to config 'c'/ffffff817d6a2520
> [30133.140081] configfs-gadget gadget: adding 'Function FS Gadget'/ffffff80f3868438 --> -19
> [30133.140098] configfs-gadget gadget: unbind function 'Mass Storage Function'/ffffff810af87200
> [30133.140119] configfs-gadget gadget: unbind function 'cdc_network'/ffffff80f48d1a00
> [30133.173201] configfs-gadget a600000.dwc3: failed to start g1: -19
> [30136.661933] init: starting service 'adbd'...
> [30136.700126] read descriptors
> [30136.700413] read strings
> [30138.574484] configfs-gadget gadget: adding 'Function FS Gadget'/ffffff80f3868438 to config 'c'/ffffff817d6a2520
> [30138.575497] configfs-gadget gadget: adding 'Mass Storage Function'/ffffff810af87200 to config 'c'/ffffff817d6a2520
> [30138.575554] configfs-gadget gadget: adding 'cdc_network'/ffffff80f48d1a00 to config 'c'/ffffff817d6a2520
> [30138.575631] using random self ethernet address
> [30138.575660] using random host ethernet address
> [30138.595338] usb0: HOST MAC 2e:cf:43:cd:ca:c8
> [30138.597160] usb0: MAC 6a:f0:9f:ee:82:a0
> [30138.791490] configfs-gadget gadget: super-speed config #1: c
>
> Fix this by reversing the iteration order of the functions in
> purge_config_funcs() when unbinding them, and adding them back to
> the config's func_list at the head instead of the tail. This
> ensures that we unbind and unwind back to the original list order.
>
> Fixes: 88af8bbe4ef7 ("usb: gadget: the start of the configfs interface")
> Signed-off-by: Chandana Kishori Chiluveru <cchiluve@xxxxxxxxxxxxxx>
> Signed-off-by: Jack Pham <jackp@xxxxxxxxxxxxxx>
> ---
> v2: rebase on gregkh's usb-testing
>
> drivers/usb/gadget/configfs.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/usb/gadget/configfs.c b/drivers/usb/gadget/configfs.c
> index 56051bb97349..3dda1d63f231 100644
> --- a/drivers/usb/gadget/configfs.c
> +++ b/drivers/usb/gadget/configfs.c
> @@ -1248,9 +1248,9 @@ static void purge_configs_funcs(struct gadget_info *gi)
>
> cfg = container_of(c, struct config_usb_cfg, c);
>
> - list_for_each_entry_safe(f, tmp, &c->functions, list) {
> + list_for_each_entry_safe_reverse(f, tmp, &c->functions, list) {
>
> - list_move_tail(&f->list, &cfg->func_list);
> + list_move(&f->list, &cfg->func_list);
> if (f->unbind) {
> dev_dbg(&gi->cdev.gadget->dev,
> "unbind function '%s'/%p\n",
> --
Hi Jack,
I am curious what features are broken if the functions are added with
not planned order?
--
Thanks,
Peter Chen
