Re: [PATCH] usb: gadget: udc: core: fix return code of usb_gadget_probe_driver()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi,

Good catch but I have a few comments to commit message:

On 12/01/2016 12:24 AM, Felix Hädicke wrote:
> This fixes a regression which was introduced by commit f1bddbb, by
> reverting a small fragment of commit 855ed04.
> 

Not exactly. The regression has been introduced by commit 855ed04
itself. This commit replaced previous construction similar what you sent
now with simple if():

@@ -535,11 +556,7 @@ int usb_gadget_probe_driver(struct
usb_gadget_driver *driver)
                        if (!ret)
                                break;
                }
-               if (ret)
-                       ret = -ENODEV;
-               else if (udc->driver)
-                       ret = -EBUSY;
-               else
+               if (!ret && !udc->driver)
                        goto found;
        } else {
                list_for_each_entry(udc, &udc_list, list) {

After that commit, if UDC is busy, gadget is added to the list of
pending gadgets. Unfortunately this list is not rescanned in
usb_gadget_unregister_driver(). This means that such gadget is going to
stay on this list forever so it's a bug.

Commit f1bddbb make this bug more visible (as it causes NULL pointer
dereference) as gadget has not been added to the list of pending gadgets
and we try to remove it from there.

Summing up, in my personal opinion I think that you should add:

Fixes: 855ed04 ("usb: gadget: udc-core: independent registration of
gadgets and gadget drivers")

above your sign off and fix the commit message as this bug has been
introduced before f1bddbb, just symptoms were a little bit different.

After fixing this you may add:

Tested-by: Krzysztof Opasiak <k.opasiak@xxxxxxxxxxx>

Best regards,
-- 
Krzysztof Opasiak
Samsung R&D Institute Poland
Samsung Electronics
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux