On Thu, Aug 06, 2020 at 05:07:16PM +0800, Peter Chen wrote:
> The current code uses commit fac323471df6 ("usb: udc: allow adding
> and removing the same gadget device") as the workaround to let
> the gadget device is re-used, but it is not allowed from driver
> core point. In this commit, we allocate gadget structure dynamically,
> and free it at its release function. Since the gadget device's
> driver_data has already occupied by usb_composite_dev structure, we have
> to use gadget device's platform data to store dwc3 structure.
>
> Cc: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
> Cc: Alan Stern <stern@xxxxxxxxxxxxxxxxxxx>
> Signed-off-by: Peter Chen <peter.chen@xxxxxxx>
There's one problem with this patch...
> @@ -3670,21 +3685,22 @@ int dwc3_gadget_init(struct dwc3 *dwc)
>
> ret = dwc3_gadget_init_endpoints(dwc, dwc->num_eps);
> if (ret)
> - goto err3;
> + goto err4;
>
> - ret = usb_add_gadget_udc(dwc->dev, &dwc->gadget);
> + ret = usb_add_gadget_udc_release(dwc->dev, dwc->gadget, dwc_gadget_release);
> if (ret) {
> dev_err(dwc->dev, "failed to register udc\n");
> - goto err4;
> + goto err5;
> }
>
> - dwc3_gadget_set_speed(&dwc->gadget, dwc->maximum_speed);
> + dwc3_gadget_set_speed(dwc->gadget, dwc->maximum_speed);
>
> return 0;
>
> -err4:
> +err5:
> dwc3_gadget_free_endpoints(dwc);
> -
> +err4:
> + kfree(dwc->gadget);
Once you call usb_add_gadget_udc_release() -- even if the call fails --
you must not call kfree() directly. Instead you have to do
put_device(&dwc->gadget.dev). That's a little awkward given the code
arrangement here.
One of the advantages of the patch set I posted recently was that it
straightens out this whole situation. Immediately after allocating
the gadget structure you can call usb_initialize_gadget(), and after
that you call usb_put_gadget() to decrement the refcount -- just like
everything else in the device model.
Perhaps you should rewrite this part of your patch to rely on my work.
Alan Stern
