| Message ID | 20200629180314.2878638-1-martin.blumenstingl@googlemail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [RFC,for-5.8] usb: dwc2: Cleanup debugfs when usb_add_gadget_udc() fails | expand |
Hi Martin, On 6/29/2020 10:03 PM, Martin Blumenstingl wrote: > Call dwc2_debugfs_exit() when usb_add_gadget_udc() has failed. This > ensure that the debugfs entries created by dwc2_debugfs_init() are > cleaned up in the error path. > > Fixes: 207324a321a866 ("usb: dwc2: Postponed gadget registration to the udc class driver") > Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com> > --- > This patch is compile-tested only. I found this while trying to > understand the latest changes to dwc2/platform.c. > > > drivers/usb/dwc2/platform.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/drivers/usb/dwc2/platform.c b/drivers/usb/dwc2/platform.c > index c347d93eae64..02b6da7e21d7 100644 > --- a/drivers/usb/dwc2/platform.c > +++ b/drivers/usb/dwc2/platform.c > @@ -582,12 +582,14 @@ static int dwc2_driver_probe(struct platform_device *dev) > retval = usb_add_gadget_udc(hsotg->dev, &hsotg->gadget); > if (retval) { > dwc2_hsotg_remove(hsotg); > - goto error_init; > + goto error_debugfs; > } > } > #endif /* CONFIG_USB_DWC2_PERIPHERAL || CONFIG_USB_DWC2_DUAL_ROLE */ > return 0; > > +error_debugfs: > + dwc2_debugfs_exit(hsotg); > error_init: > if (hsotg->params.activate_stm_id_vb_detection) > regulator_disable(hsotg->usb33d); > I'm Ok with this fix. One more thing. I missed to remove hcd also in fail case. Could you please add dwc2_hcd_remove() call after dwc2_debugfs_exit(hsotg) and submit as patch: +error_debugfs: + dwc2_debugfs_exit(hsotg); + if (hsotg->hcd_enabled) + dwc2_hcd_remove(hsotg); error_init: Thanks, Minas
Minas Harutyunyan <Minas.Harutyunyan@synopsys.com> writes: > Hi Martin, > > On 6/29/2020 10:03 PM, Martin Blumenstingl wrote: >> Call dwc2_debugfs_exit() when usb_add_gadget_udc() has failed. This >> ensure that the debugfs entries created by dwc2_debugfs_init() are >> cleaned up in the error path. >> >> Fixes: 207324a321a866 ("usb: dwc2: Postponed gadget registration to the udc class driver") >> Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com> >> --- >> This patch is compile-tested only. I found this while trying to >> understand the latest changes to dwc2/platform.c. >> >> >> drivers/usb/dwc2/platform.c | 4 +++- >> 1 file changed, 3 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/usb/dwc2/platform.c b/drivers/usb/dwc2/platform.c >> index c347d93eae64..02b6da7e21d7 100644 >> --- a/drivers/usb/dwc2/platform.c >> +++ b/drivers/usb/dwc2/platform.c >> @@ -582,12 +582,14 @@ static int dwc2_driver_probe(struct platform_device *dev) >> retval = usb_add_gadget_udc(hsotg->dev, &hsotg->gadget); >> if (retval) { >> dwc2_hsotg_remove(hsotg); >> - goto error_init; >> + goto error_debugfs; >> } >> } >> #endif /* CONFIG_USB_DWC2_PERIPHERAL || CONFIG_USB_DWC2_DUAL_ROLE */ >> return 0; >> >> +error_debugfs: >> + dwc2_debugfs_exit(hsotg); >> error_init: >> if (hsotg->params.activate_stm_id_vb_detection) >> regulator_disable(hsotg->usb33d); >> > I'm Ok with this fix. One more thing. I missed to remove hcd also in > fail case. Could you please add dwc2_hcd_remove() call after > dwc2_debugfs_exit(hsotg) and submit as patch: > > +error_debugfs: > + dwc2_debugfs_exit(hsotg); > + if (hsotg->hcd_enabled) > + dwc2_hcd_remove(hsotg); > error_init: looks like it should be a separate patch though. Right?
Hi Felipe, On 7/23/2020 5:03 PM, Felipe Balbi wrote: > Minas Harutyunyan <Minas.Harutyunyan@synopsys.com> writes: > >> Hi Martin, >> >> On 6/29/2020 10:03 PM, Martin Blumenstingl wrote: >>> Call dwc2_debugfs_exit() when usb_add_gadget_udc() has failed. This >>> ensure that the debugfs entries created by dwc2_debugfs_init() are >>> cleaned up in the error path. >>> >>> Fixes: 207324a321a866 ("usb: dwc2: Postponed gadget registration to the udc class driver") >>> Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com> >>> --- >>> This patch is compile-tested only. I found this while trying to >>> understand the latest changes to dwc2/platform.c. >>> >>> >>> drivers/usb/dwc2/platform.c | 4 +++- >>> 1 file changed, 3 insertions(+), 1 deletion(-) >>> >>> diff --git a/drivers/usb/dwc2/platform.c b/drivers/usb/dwc2/platform.c >>> index c347d93eae64..02b6da7e21d7 100644 >>> --- a/drivers/usb/dwc2/platform.c >>> +++ b/drivers/usb/dwc2/platform.c >>> @@ -582,12 +582,14 @@ static int dwc2_driver_probe(struct platform_device *dev) >>> retval = usb_add_gadget_udc(hsotg->dev, &hsotg->gadget); >>> if (retval) { >>> dwc2_hsotg_remove(hsotg); >>> - goto error_init; >>> + goto error_debugfs; >>> } >>> } >>> #endif /* CONFIG_USB_DWC2_PERIPHERAL || CONFIG_USB_DWC2_DUAL_ROLE */ >>> return 0; >>> >>> +error_debugfs: >>> + dwc2_debugfs_exit(hsotg); >>> error_init: >>> if (hsotg->params.activate_stm_id_vb_detection) >>> regulator_disable(hsotg->usb33d); >>> >> I'm Ok with this fix. One more thing. I missed to remove hcd also in >> fail case. Could you please add dwc2_hcd_remove() call after >> dwc2_debugfs_exit(hsotg) and submit as patch: >> >> +error_debugfs: >> + dwc2_debugfs_exit(hsotg); >> + if (hsotg->hcd_enabled) >> + dwc2_hcd_remove(hsotg); >> error_init: > > looks like it should be a separate patch though. Right? v2 of this patch already submitted by Martin Blumenstingl <martin.blumenstingl@googlemail.com> on 7/4/2020 with fix related to dwc2_hcd_remove() and I acked it: [PATCH for-5.8 v2] usb: dwc2: Add missing cleanups when usb_add_gadget_udc() fails Thanks, Minas >
diff --git a/drivers/usb/dwc2/platform.c b/drivers/usb/dwc2/platform.c index c347d93eae64..02b6da7e21d7 100644 --- a/drivers/usb/dwc2/platform.c +++ b/drivers/usb/dwc2/platform.c @@ -582,12 +582,14 @@ static int dwc2_driver_probe(struct platform_device *dev) retval = usb_add_gadget_udc(hsotg->dev, &hsotg->gadget); if (retval) { dwc2_hsotg_remove(hsotg); - goto error_init; + goto error_debugfs; } } #endif /* CONFIG_USB_DWC2_PERIPHERAL || CONFIG_USB_DWC2_DUAL_ROLE */ return 0; +error_debugfs: + dwc2_debugfs_exit(hsotg); error_init: if (hsotg->params.activate_stm_id_vb_detection) regulator_disable(hsotg->usb33d);
Call dwc2_debugfs_exit() when usb_add_gadget_udc() has failed. This ensure that the debugfs entries created by dwc2_debugfs_init() are cleaned up in the error path. Fixes: 207324a321a866 ("usb: dwc2: Postponed gadget registration to the udc class driver") Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com> --- This patch is compile-tested only. I found this while trying to understand the latest changes to dwc2/platform.c. drivers/usb/dwc2/platform.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)