Hi,
> 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) {
The regression bug I want to fix with this patch was introduced with
commit f1bddbb, not 855ed04. With 855ed04, the this if/else construct
was changed. But concerning the usb_gadget_probe_driver() return code,
this was ok. There was no code path were a value which was not meant to
be the function return code was accidentally returned. With commit
f1bddbb, and the introduction of a new code path for the flag
match_existing_only, this changed.
> 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.
Ok, I can confirm this bug. But it is not the issue which I am
addressing with this patch. This is just about the
usb_gadget_probe_driver() return code (on which other functions,
paritcularly gadget configfs's|gadget_dev_desc_UDC_store|() rely).
> 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")
As explained above, I think this would be wrong.
> 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>
Ok.
>
> Best regards,
Best Regards,
Felix
--
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
