On 8/31/2024 9:59 AM, Alan Stern wrote:
> On Fri, Aug 30, 2024 at 06:16:12PM +0530, Selvarasu Ganesan wrote:
>> Hi Alan,
>>
>> Thanks for your comments. I understand your suggestions. We already have
>> a similar reference check with the udc name before calling
>> usb_gadget_register_driver.
>> In the drivers/usb/gadget/configfs.c file, I am wondering if there might
>> be an issue with the check of udc_name before
>> usb_gadget_register_driver. This is the only way to allow
>> gadget_register to be called before releasing or unregistering an
>> existing udc. Do you think we need to add an additional check here,
>> referencing the UDC, to prevent gadget_register from being called before
>> the existing UDC is released?
> I don't understand what you're saying. There is no routine named
> "gadget_register". (And there is no variable named "udc_name" in the
> code below, although there is gi->composite.gadget_driver.udc_name --
> but that's not a variable, it is a field in a structure.)
>
>> drivers/usb/gadget/configfs.c : gadget_dev_desc_UDC_store()
>> ===========================================================
>> if (gi->composite.gadget_driver.udc_name) {
>> ret = -EBUSY;
>> goto err;
>> }
>> gi->composite.gadget_driver.udc_name = name;
> Are you talking about this check and assignment? Why do you think there
> might be a problem here?
>
> Are you worried that some UDC might be released while this code is
> running? If that happens, why would it be a problem?
I am talking here based on the call traces, we are observing the
following call traces at the time of failures. One specific point of
interest is the gadget_match_driver() function, which is called as part
of the usb_gadget_register_driver() function. I am wondering how the
usb_gadget_register_driver() function allows the registration of a new
driver even when an existing same UDC is not releasing. One possibility
is that gi->composite.gadget_driver.udc_name becomes NULL before the UDC
is released. However, as of now, we do not have any evidence to support
this theory. We are still trying to reproduce the same issue with added
more debugging logs.
CPU0: (ROLE SWITCH DEVICE <-> HOST)
==================================
->usb_role_switch_set_role()
->dwc3_usb_role_switch_set()
->dwc3_set_mode()
->__dwc3_set_mode()
->dwc3_gadget_exit()
->usb_del_gadget()
->device_unregister()
->put_device(dev)
->usb_udc_release()
CPU1 (echo "<dwc3 device name>" > <path of udc
config>/config/usb_gadget/g1/UDC)
=================================================================================
->configfs_write_iter()
->gadget_dev_desc_UDC_store()
->usb_gadget_register_driver()
->driver_register()
->bus_add_driver()
->driver_attach()
->bus_for_each_dev()
->__driver_attach()
->gadget_match_driver()
>
>> ret = usb_gadget_register_driver(&gi->composite.gadget_driver);
> Alan Stern
>
