| Message ID | 20200703225043.387769-1-martin.blumenstingl@googlemail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [for-5.8,v2] usb: dwc2: Add missing cleanups when usb_add_gadget_udc() fails | expand |
On 7/4/2020 2:50 AM, Martin Blumenstingl wrote: > Call dwc2_debugfs_exit() and dwc2_hcd_remove() (if the HCD was enabled > earlier) when usb_add_gadget_udc() has failed. This ensures that the > debugfs entries created by dwc2_debugfs_init() as well as the HCD 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> Acked-by: Minas Harutyunyan <hminas@synopsys.com> > --- > Changes since v1 at [0] > - also cleanup the HCD as suggested by Minas (thank you!) > - updated the subject accordingly > > > [0] https://urldefense.com/v3/__https://patchwork.kernel.org/patch/11631381/__;!!A4F2R9G_pg!K0ZJIzYVYtrwy2VgxTI28Z_qA995uqmg1dFrMNx3XMQ653ROMKPkt_zQdJe1OOk$ > > > drivers/usb/dwc2/platform.c | 6 +++++- > 1 file changed, 5 insertions(+), 1 deletion(-) > > diff --git a/drivers/usb/dwc2/platform.c b/drivers/usb/dwc2/platform.c > index c347d93eae64..9febae441069 100644 > --- a/drivers/usb/dwc2/platform.c > +++ b/drivers/usb/dwc2/platform.c > @@ -582,12 +582,16 @@ 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); > + if (hsotg->hcd_enabled) > + dwc2_hcd_remove(hsotg); > error_init: > if (hsotg->params.activate_stm_id_vb_detection) > regulator_disable(hsotg->usb33d); >
Hi Martin, On 7/4/2020 2:50 AM, Martin Blumenstingl wrote: > Call dwc2_debugfs_exit() and dwc2_hcd_remove() (if the HCD was enabled > earlier) when usb_add_gadget_udc() has failed. This ensures that the > debugfs entries created by dwc2_debugfs_init() as well as the HCD 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> > --- > Changes since v1 at [0] > - also cleanup the HCD as suggested by Minas (thank you!) > - updated the subject accordingly > > > [0] https://urldefense.com/v3/__https://patchwork.kernel.org/patch/11631381/__;!!A4F2R9G_pg!K0ZJIzYVYtrwy2VgxTI28Z_qA995uqmg1dFrMNx3XMQ653ROMKPkt_zQdJe1OOk$ > > > drivers/usb/dwc2/platform.c | 6 +++++- > 1 file changed, 5 insertions(+), 1 deletion(-) > > diff --git a/drivers/usb/dwc2/platform.c b/drivers/usb/dwc2/platform.c > index c347d93eae64..9febae441069 100644 > --- a/drivers/usb/dwc2/platform.c > +++ b/drivers/usb/dwc2/platform.c > @@ -582,12 +582,16 @@ 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; > Kernel test robot found issue: >> warning: unused label 'error_debugfs' [-Wunused-label] error_debugfs: ^~~~~~~~~~~~~~ 1 warning generated. So, 'error_debugfs:' label should be under #if/#endif: #if IS_ENABLED(CONFIG_USB_DWC2_PERIPHERAL) || \ IS_ENABLED(CONFIG_USB_DWC2_DUAL_ROLE) error_debugfs: #endif /* CONFIG_USB_DWC2_PERIPHERAL || CONFIG_USB_DWC2_DUAL_ROLE */ Or you have other suggestion? Could you please fix this issue and submit new version of patch. Thanks, Minas > +error_debugfs: > + dwc2_debugfs_exit(hsotg); > + if (hsotg->hcd_enabled) > + dwc2_hcd_remove(hsotg); > error_init: > if (hsotg->params.activate_stm_id_vb_detection) > regulator_disable(hsotg->usb33d); >
Hello Minas, On Sun, Jul 26, 2020 at 12:04 PM Minas Harutyunyan <Minas.Harutyunyan@synopsys.com> wrote: [...] > Kernel test robot found issue: > >> warning: unused label 'error_debugfs' [-Wunused-label] > error_debugfs: > ^~~~~~~~~~~~~~ > 1 warning generated. > > So, 'error_debugfs:' label should be under #if/#endif: > > #if IS_ENABLED(CONFIG_USB_DWC2_PERIPHERAL) || \ > IS_ENABLED(CONFIG_USB_DWC2_DUAL_ROLE) > error_debugfs: > #endif /* CONFIG_USB_DWC2_PERIPHERAL || CONFIG_USB_DWC2_DUAL_ROLE */ > > Or you have other suggestion? unfortunately I have no better idea > Could you please fix this issue and submit new version of patch. I'm going to cover everything I add inside the same #if (not just the error label). if any additional label is added (above) in the future then the code that I'm adding must not be executed when that #if evaluates to false v3 is coming in the next few minutes. Best regards, Martin
diff --git a/drivers/usb/dwc2/platform.c b/drivers/usb/dwc2/platform.c index c347d93eae64..9febae441069 100644 --- a/drivers/usb/dwc2/platform.c +++ b/drivers/usb/dwc2/platform.c @@ -582,12 +582,16 @@ 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); + if (hsotg->hcd_enabled) + dwc2_hcd_remove(hsotg); error_init: if (hsotg->params.activate_stm_id_vb_detection) regulator_disable(hsotg->usb33d);
Call dwc2_debugfs_exit() and dwc2_hcd_remove() (if the HCD was enabled earlier) when usb_add_gadget_udc() has failed. This ensures that the debugfs entries created by dwc2_debugfs_init() as well as the HCD 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> --- Changes since v1 at [0] - also cleanup the HCD as suggested by Minas (thank you!) - updated the subject accordingly [0] https://patchwork.kernel.org/patch/11631381/ drivers/usb/dwc2/platform.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)