| Message ID | 20241205133300.884353-1-mordan@ispras.ru (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | usb: ohci-spear: fix call balance of sohci_p->clk handling routines | expand |
On Thu, Dec 05, 2024 at 04:33:00PM +0300, Vitalii Mordan wrote: > If the clock sohci_p->clk was not enabled in spear_ohci_hcd_drv_probe, > it should not be disabled in any path. > > Conversely, if it was enabled in spear_ohci_hcd_drv_probe, it must be disabled > in all error paths to ensure proper cleanup. > > The check inside spear_ohci_hcd_drv_resume() actually doesn't prevent > the clock to be unconditionally disabled later during the driver removal but > it is still good to have the check at least so that the PM core would duly > print the errors in the system log. This would also be consistent with > the similar code paths in ->resume() functions of other usb drivers, e.g. in > exynos_ehci_resume(). > > Found by Linux Verification Center (linuxtesting.org) with Klever. > > Fixes: 1cc6ac59ffaa ("USB: OHCI: make ohci-spear a separate driver") > Cc: stable@vger.kernel.org > Signed-off-by: Vitalii Mordan <mordan@ispras.ru> > --- Acked-by: Alan Stern <stern@rowland.harvard.edu> > drivers/usb/host/ohci-spear.c | 12 ++++++++---- > 1 file changed, 8 insertions(+), 4 deletions(-) > > diff --git a/drivers/usb/host/ohci-spear.c b/drivers/usb/host/ohci-spear.c > index 993f347c5c28..6f6ae6fadfe5 100644 > --- a/drivers/usb/host/ohci-spear.c > +++ b/drivers/usb/host/ohci-spear.c > @@ -80,7 +80,9 @@ static int spear_ohci_hcd_drv_probe(struct platform_device *pdev) > sohci_p = to_spear_ohci(hcd); > sohci_p->clk = usbh_clk; > > - clk_prepare_enable(sohci_p->clk); > + retval = clk_prepare_enable(sohci_p->clk); > + if (retval) > + goto err_put_hcd; > > retval = usb_add_hcd(hcd, irq, 0); > if (retval == 0) { > @@ -103,8 +105,7 @@ static void spear_ohci_hcd_drv_remove(struct platform_device *pdev) > struct spear_ohci *sohci_p = to_spear_ohci(hcd); > > usb_remove_hcd(hcd); > - if (sohci_p->clk) > - clk_disable_unprepare(sohci_p->clk); > + clk_disable_unprepare(sohci_p->clk); > > usb_put_hcd(hcd); > } > @@ -137,12 +138,15 @@ static int spear_ohci_hcd_drv_resume(struct platform_device *dev) > struct usb_hcd *hcd = platform_get_drvdata(dev); > struct ohci_hcd *ohci = hcd_to_ohci(hcd); > struct spear_ohci *sohci_p = to_spear_ohci(hcd); > + int ret; > > if (time_before(jiffies, ohci->next_statechange)) > msleep(5); > ohci->next_statechange = jiffies; > > - clk_prepare_enable(sohci_p->clk); > + ret = clk_prepare_enable(sohci_p->clk); > + if (ret) > + return ret; > ohci_resume(hcd, false); > return 0; > } > -- > 2.25.1 >
diff --git a/drivers/usb/host/ohci-spear.c b/drivers/usb/host/ohci-spear.c index 993f347c5c28..6f6ae6fadfe5 100644 --- a/drivers/usb/host/ohci-spear.c +++ b/drivers/usb/host/ohci-spear.c @@ -80,7 +80,9 @@ static int spear_ohci_hcd_drv_probe(struct platform_device *pdev) sohci_p = to_spear_ohci(hcd); sohci_p->clk = usbh_clk; - clk_prepare_enable(sohci_p->clk); + retval = clk_prepare_enable(sohci_p->clk); + if (retval) + goto err_put_hcd; retval = usb_add_hcd(hcd, irq, 0); if (retval == 0) { @@ -103,8 +105,7 @@ static void spear_ohci_hcd_drv_remove(struct platform_device *pdev) struct spear_ohci *sohci_p = to_spear_ohci(hcd); usb_remove_hcd(hcd); - if (sohci_p->clk) - clk_disable_unprepare(sohci_p->clk); + clk_disable_unprepare(sohci_p->clk); usb_put_hcd(hcd); } @@ -137,12 +138,15 @@ static int spear_ohci_hcd_drv_resume(struct platform_device *dev) struct usb_hcd *hcd = platform_get_drvdata(dev); struct ohci_hcd *ohci = hcd_to_ohci(hcd); struct spear_ohci *sohci_p = to_spear_ohci(hcd); + int ret; if (time_before(jiffies, ohci->next_statechange)) msleep(5); ohci->next_statechange = jiffies; - clk_prepare_enable(sohci_p->clk); + ret = clk_prepare_enable(sohci_p->clk); + if (ret) + return ret; ohci_resume(hcd, false); return 0; }
If the clock sohci_p->clk was not enabled in spear_ohci_hcd_drv_probe, it should not be disabled in any path. Conversely, if it was enabled in spear_ohci_hcd_drv_probe, it must be disabled in all error paths to ensure proper cleanup. The check inside spear_ohci_hcd_drv_resume() actually doesn't prevent the clock to be unconditionally disabled later during the driver removal but it is still good to have the check at least so that the PM core would duly print the errors in the system log. This would also be consistent with the similar code paths in ->resume() functions of other usb drivers, e.g. in exynos_ehci_resume(). Found by Linux Verification Center (linuxtesting.org) with Klever. Fixes: 1cc6ac59ffaa ("USB: OHCI: make ohci-spear a separate driver") Cc: stable@vger.kernel.org Signed-off-by: Vitalii Mordan <mordan@ispras.ru> --- drivers/usb/host/ohci-spear.c | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-)