| Message ID | 20240902123020.29267-2-zhangzekun11@huawei.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Use helper function devm_clk_get_enabled() | expand |
On Mon, Sep 02, 2024, Zhang Zekun wrote: > devm_clk_get() and clk_prepare_enable() can be replaced by helper > function devm_clk_get_enabled(). Let's use devm_clk_get_enabled() to > simplify code and avoid calling clk_disable_unprepare(). > > Signed-off-by: Zhang Zekun <zhangzekun11@huawei.com> > --- > drivers/usb/dwc3/dwc3-imx8mp.c | 47 ++++++++-------------------------- > 1 file changed, 11 insertions(+), 36 deletions(-) > > diff --git a/drivers/usb/dwc3/dwc3-imx8mp.c b/drivers/usb/dwc3/dwc3-imx8mp.c > index 392fa1232788..a7e5ee797ae7 100644 > --- a/drivers/usb/dwc3/dwc3-imx8mp.c > +++ b/drivers/usb/dwc3/dwc3-imx8mp.c > @@ -178,37 +178,20 @@ static int dwc3_imx8mp_probe(struct platform_device *pdev) > return PTR_ERR(dwc3_imx->glue_base); > } > > - dwc3_imx->hsio_clk = devm_clk_get(dev, "hsio"); > - if (IS_ERR(dwc3_imx->hsio_clk)) { > - err = PTR_ERR(dwc3_imx->hsio_clk); > - dev_err(dev, "Failed to get hsio clk, err=%d\n", err); > - return err; > - } > + dwc3_imx->hsio_clk = devm_clk_get_enabled(dev, "hsio"); > + if (IS_ERR(dwc3_imx->hsio_clk)) > + return dev_err_probe(dev, PTR_ERR(dwc3_imx->hsio_clk), > + "Failed to get and enable hsio clk\n"); > > - err = clk_prepare_enable(dwc3_imx->hsio_clk); > - if (err) { > - dev_err(dev, "Failed to enable hsio clk, err=%d\n", err); > - return err; > - } > - > - dwc3_imx->suspend_clk = devm_clk_get(dev, "suspend"); > - if (IS_ERR(dwc3_imx->suspend_clk)) { > - err = PTR_ERR(dwc3_imx->suspend_clk); > - dev_err(dev, "Failed to get suspend clk, err=%d\n", err); > - goto disable_hsio_clk; > - } > - > - err = clk_prepare_enable(dwc3_imx->suspend_clk); > - if (err) { > - dev_err(dev, "Failed to enable suspend clk, err=%d\n", err); > - goto disable_hsio_clk; > - } > + dwc3_imx->suspend_clk = devm_clk_get_enabled(dev, "suspend"); > + if (IS_ERR(dwc3_imx->suspend_clk)) > + return dev_err_probe(dev, PTR_ERR(dwc3_imx->suspend_clk), > + "Failed to get and enable suspend clk\n"); > > irq = platform_get_irq(pdev, 0); > - if (irq < 0) { > - err = irq; > - goto disable_clks; > - } > + if (irq < 0) > + return irq; > + > dwc3_imx->irq = irq; > > imx8mp_configure_glue(dwc3_imx); > @@ -259,25 +242,17 @@ static int dwc3_imx8mp_probe(struct platform_device *pdev) > disable_rpm: > pm_runtime_disable(dev); > pm_runtime_put_noidle(dev); > -disable_clks: > - clk_disable_unprepare(dwc3_imx->suspend_clk); > -disable_hsio_clk: > - clk_disable_unprepare(dwc3_imx->hsio_clk); > > return err; > } > > static void dwc3_imx8mp_remove(struct platform_device *pdev) > { > - struct dwc3_imx8mp *dwc3_imx = platform_get_drvdata(pdev); > struct device *dev = &pdev->dev; > > pm_runtime_get_sync(dev); > of_platform_depopulate(dev); > > - clk_disable_unprepare(dwc3_imx->suspend_clk); > - clk_disable_unprepare(dwc3_imx->hsio_clk); > - > pm_runtime_disable(dev); > pm_runtime_put_noidle(dev); > } > -- > 2.17.1 > Krzysztof Kozlowski already submitted some cleanup related to this: https://lore.kernel.org/linux-usb/20240827012651.j2chqblkjha2vene@synopsys.com/T/#u BR, Thinh
On Mon, Sep 02, 2024 at 08:30:19PM +0800, Zhang Zekun wrote: > devm_clk_get() and clk_prepare_enable() can be replaced by helper > function devm_clk_get_enabled(). Let's use devm_clk_get_enabled() to > simplify code and avoid calling clk_disable_unprepare(). > > Signed-off-by: Zhang Zekun <zhangzekun11@huawei.com> Also, have you tested that this actually works? using devm can have tricky sync issues when shutting down so I'm going to start requiring that any conversions like this be proven to work properly on real hardware. thanks, greg k-h
在 2024/9/4 16:49, Greg KH 写道: > On Mon, Sep 02, 2024 at 08:30:19PM +0800, Zhang Zekun wrote: >> devm_clk_get() and clk_prepare_enable() can be replaced by helper >> function devm_clk_get_enabled(). Let's use devm_clk_get_enabled() to >> simplify code and avoid calling clk_disable_unprepare(). >> >> Signed-off-by: Zhang Zekun <zhangzekun11@huawei.com> > > Also, have you tested that this actually works? using devm can have > tricky sync issues when shutting down so I'm going to start requiring > that any conversions like this be proven to work properly on real > hardware. > > thanks, > > greg k-h Hi, greg, I make a compile test and have not test on a real hardware. I have read through the code logic but did not find a obvious problem. Best Regards, Zekun
diff --git a/drivers/usb/dwc3/dwc3-imx8mp.c b/drivers/usb/dwc3/dwc3-imx8mp.c index 392fa1232788..a7e5ee797ae7 100644 --- a/drivers/usb/dwc3/dwc3-imx8mp.c +++ b/drivers/usb/dwc3/dwc3-imx8mp.c @@ -178,37 +178,20 @@ static int dwc3_imx8mp_probe(struct platform_device *pdev) return PTR_ERR(dwc3_imx->glue_base); } - dwc3_imx->hsio_clk = devm_clk_get(dev, "hsio"); - if (IS_ERR(dwc3_imx->hsio_clk)) { - err = PTR_ERR(dwc3_imx->hsio_clk); - dev_err(dev, "Failed to get hsio clk, err=%d\n", err); - return err; - } + dwc3_imx->hsio_clk = devm_clk_get_enabled(dev, "hsio"); + if (IS_ERR(dwc3_imx->hsio_clk)) + return dev_err_probe(dev, PTR_ERR(dwc3_imx->hsio_clk), + "Failed to get and enable hsio clk\n"); - err = clk_prepare_enable(dwc3_imx->hsio_clk); - if (err) { - dev_err(dev, "Failed to enable hsio clk, err=%d\n", err); - return err; - } - - dwc3_imx->suspend_clk = devm_clk_get(dev, "suspend"); - if (IS_ERR(dwc3_imx->suspend_clk)) { - err = PTR_ERR(dwc3_imx->suspend_clk); - dev_err(dev, "Failed to get suspend clk, err=%d\n", err); - goto disable_hsio_clk; - } - - err = clk_prepare_enable(dwc3_imx->suspend_clk); - if (err) { - dev_err(dev, "Failed to enable suspend clk, err=%d\n", err); - goto disable_hsio_clk; - } + dwc3_imx->suspend_clk = devm_clk_get_enabled(dev, "suspend"); + if (IS_ERR(dwc3_imx->suspend_clk)) + return dev_err_probe(dev, PTR_ERR(dwc3_imx->suspend_clk), + "Failed to get and enable suspend clk\n"); irq = platform_get_irq(pdev, 0); - if (irq < 0) { - err = irq; - goto disable_clks; - } + if (irq < 0) + return irq; + dwc3_imx->irq = irq; imx8mp_configure_glue(dwc3_imx); @@ -259,25 +242,17 @@ static int dwc3_imx8mp_probe(struct platform_device *pdev) disable_rpm: pm_runtime_disable(dev); pm_runtime_put_noidle(dev); -disable_clks: - clk_disable_unprepare(dwc3_imx->suspend_clk); -disable_hsio_clk: - clk_disable_unprepare(dwc3_imx->hsio_clk); return err; } static void dwc3_imx8mp_remove(struct platform_device *pdev) { - struct dwc3_imx8mp *dwc3_imx = platform_get_drvdata(pdev); struct device *dev = &pdev->dev; pm_runtime_get_sync(dev); of_platform_depopulate(dev); - clk_disable_unprepare(dwc3_imx->suspend_clk); - clk_disable_unprepare(dwc3_imx->hsio_clk); - pm_runtime_disable(dev); pm_runtime_put_noidle(dev); }
devm_clk_get() and clk_prepare_enable() can be replaced by helper function devm_clk_get_enabled(). Let's use devm_clk_get_enabled() to simplify code and avoid calling clk_disable_unprepare(). Signed-off-by: Zhang Zekun <zhangzekun11@huawei.com> --- drivers/usb/dwc3/dwc3-imx8mp.c | 47 ++++++++-------------------------- 1 file changed, 11 insertions(+), 36 deletions(-)