| Message ID | 20240404071350.4242-4-linux.amoon@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v2,1/6] usb: ehci-exynos: Use devm_clk_get_enabled() helpers | expand |
On 04/04/2024 09:13, Anand Moon wrote: > The devm_clk_get_enabled() helpers: > - call devm_clk_get() > - call clk_prepare_enable() and register what is needed in order to > call clk_disable_unprepare() when needed, as a managed resource. > > This simplifies the code and avoids the calls to clk_disable_unprepare(). > > While at it, use dev_err_probe consistently, and use its return value > to return the error code. > > Signed-off-by: Anand Moon <linux.amoon@gmail.com> > --- > v2: new changes in this series. > --- > drivers/usb/host/ohci-exynos.c | 19 +++++-------------- > 1 file changed, 5 insertions(+), 14 deletions(-) > > diff --git a/drivers/usb/host/ohci-exynos.c b/drivers/usb/host/ohci-exynos.c > index 20e26a474591..85d04ae0ae40 100644 > --- a/drivers/usb/host/ohci-exynos.c > +++ b/drivers/usb/host/ohci-exynos.c > @@ -135,20 +135,15 @@ static int exynos_ohci_probe(struct platform_device *pdev) > > err = exynos_ohci_get_phy(&pdev->dev, exynos_ohci); > if (err) > - goto fail_clk; > - > - exynos_ohci->clk = devm_clk_get(&pdev->dev, "usbhost"); > + goto fail_io; > > + exynos_ohci->clk = devm_clk_get_enabled(&pdev->dev, "usbhost"); > if (IS_ERR(exynos_ohci->clk)) { > - dev_err(&pdev->dev, "Failed to get usbhost clock\n"); > - err = PTR_ERR(exynos_ohci->clk); > - goto fail_clk; > + usb_put_hcd(hcd); > + return dev_err_probe(&pdev->dev, PTR_ERR(exynos_ohci->clk), > + "Failed to get usbhost clock\n"); Why do you introduce entirely parallel exit paths? There is already single error handling part with labels. Use that. Best regards, Krzysztof
Hi Krzysztof, On Thu, 4 Apr 2024 at 12:50, Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote: > > On 04/04/2024 09:13, Anand Moon wrote: > > The devm_clk_get_enabled() helpers: > > - call devm_clk_get() > > - call clk_prepare_enable() and register what is needed in order to > > call clk_disable_unprepare() when needed, as a managed resource. > > > > This simplifies the code and avoids the calls to clk_disable_unprepare(). > > > > While at it, use dev_err_probe consistently, and use its return value > > to return the error code. > > > > Signed-off-by: Anand Moon <linux.amoon@gmail.com> > > --- > > v2: new changes in this series. > > --- > > drivers/usb/host/ohci-exynos.c | 19 +++++-------------- > > 1 file changed, 5 insertions(+), 14 deletions(-) > > > > diff --git a/drivers/usb/host/ohci-exynos.c b/drivers/usb/host/ohci-exynos.c > > index 20e26a474591..85d04ae0ae40 100644 > > --- a/drivers/usb/host/ohci-exynos.c > > +++ b/drivers/usb/host/ohci-exynos.c > > @@ -135,20 +135,15 @@ static int exynos_ohci_probe(struct platform_device *pdev) > > > > err = exynos_ohci_get_phy(&pdev->dev, exynos_ohci); > > if (err) > > - goto fail_clk; > > - > > - exynos_ohci->clk = devm_clk_get(&pdev->dev, "usbhost"); > > + goto fail_io; > > > > + exynos_ohci->clk = devm_clk_get_enabled(&pdev->dev, "usbhost"); > > if (IS_ERR(exynos_ohci->clk)) { > > - dev_err(&pdev->dev, "Failed to get usbhost clock\n"); > > - err = PTR_ERR(exynos_ohci->clk); > > - goto fail_clk; > > + usb_put_hcd(hcd); > > + return dev_err_probe(&pdev->dev, PTR_ERR(exynos_ohci->clk), > > + "Failed to get usbhost clock\n"); > > Why do you introduce entirely parallel exit paths? There is already > single error handling part with labels. Use that. > OK, I will use the labels to return over here, > > > Best regards, > Krzysztof > Thanks -Anand
diff --git a/drivers/usb/host/ohci-exynos.c b/drivers/usb/host/ohci-exynos.c index 20e26a474591..85d04ae0ae40 100644 --- a/drivers/usb/host/ohci-exynos.c +++ b/drivers/usb/host/ohci-exynos.c @@ -135,20 +135,15 @@ static int exynos_ohci_probe(struct platform_device *pdev) err = exynos_ohci_get_phy(&pdev->dev, exynos_ohci); if (err) - goto fail_clk; - - exynos_ohci->clk = devm_clk_get(&pdev->dev, "usbhost"); + goto fail_io; + exynos_ohci->clk = devm_clk_get_enabled(&pdev->dev, "usbhost"); if (IS_ERR(exynos_ohci->clk)) { - dev_err(&pdev->dev, "Failed to get usbhost clock\n"); - err = PTR_ERR(exynos_ohci->clk); - goto fail_clk; + usb_put_hcd(hcd); + return dev_err_probe(&pdev->dev, PTR_ERR(exynos_ohci->clk), + "Failed to get usbhost clock\n"); } - err = clk_prepare_enable(exynos_ohci->clk); - if (err) - goto fail_clk; - hcd->regs = devm_platform_get_and_ioremap_resource(pdev, 0, &res); if (IS_ERR(hcd->regs)) { err = PTR_ERR(hcd->regs); @@ -191,8 +186,6 @@ static int exynos_ohci_probe(struct platform_device *pdev) exynos_ohci_phy_disable(&pdev->dev); pdev->dev.of_node = exynos_ohci->of_node; fail_io: - clk_disable_unprepare(exynos_ohci->clk); -fail_clk: usb_put_hcd(hcd); return err; } @@ -208,8 +201,6 @@ static void exynos_ohci_remove(struct platform_device *pdev) exynos_ohci_phy_disable(&pdev->dev); - clk_disable_unprepare(exynos_ohci->clk); - usb_put_hcd(hcd); }
The devm_clk_get_enabled() helpers: - call devm_clk_get() - call clk_prepare_enable() and register what is needed in order to call clk_disable_unprepare() when needed, as a managed resource. This simplifies the code and avoids the calls to clk_disable_unprepare(). While at it, use dev_err_probe consistently, and use its return value to return the error code. Signed-off-by: Anand Moon <linux.amoon@gmail.com> --- v2: new changes in this series. --- drivers/usb/host/ohci-exynos.c | 19 +++++-------------- 1 file changed, 5 insertions(+), 14 deletions(-)