| Message ID | 20240404071350.4242-2-linux.amoon@gmail.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v2,1/6] usb: ehci-exynos: Use devm_clk_get_enabled() helpers | expand |
Hi Krzysztof, Greg. On Thu, 4 Apr 2024 at 19:24, Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote: > > On 04/04/2024 15:52, Anand Moon wrote: > > Hi Greg, > > > > On Thu, 4 Apr 2024 at 18:30, Greg Kroah-Hartman > > <gregkh@linuxfoundation.org> wrote: > >> > >> On Thu, Apr 04, 2024 at 12:43:17PM +0530, 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: drop the clk_disable_unprepare in suspend/resume functions > >>> fix the usb_put_hcd return before the devm_clk_get_enabled > >>> --- > >>> drivers/usb/host/ehci-exynos.c | 19 +++++-------------- > >>> 1 file changed, 5 insertions(+), 14 deletions(-) > >>> > >>> diff --git a/drivers/usb/host/ehci-exynos.c b/drivers/usb/host/ehci-exynos.c > >>> index f644b131cc0b..f00bfd0b13dc 100644 > >>> --- a/drivers/usb/host/ehci-exynos.c > >>> +++ b/drivers/usb/host/ehci-exynos.c > >>> @@ -159,20 +159,15 @@ static int exynos_ehci_probe(struct platform_device *pdev) > >>> > >>> err = exynos_ehci_get_phy(&pdev->dev, exynos_ehci); > >>> if (err) > >>> - goto fail_clk; > >>> - > >>> - exynos_ehci->clk = devm_clk_get(&pdev->dev, "usbhost"); > >>> + goto fail_io; > >>> > >>> + exynos_ehci->clk = devm_clk_get_enabled(&pdev->dev, "usbhost"); > >>> if (IS_ERR(exynos_ehci->clk)) { > >>> - dev_err(&pdev->dev, "Failed to get usbhost clock\n"); > >>> - err = PTR_ERR(exynos_ehci->clk); > >>> - goto fail_clk; > >>> + usb_put_hcd(hcd); > >>> + return dev_err_probe(&pdev->dev, PTR_ERR(exynos_ehci->clk), > >>> + "Failed to get usbhost clock\n"); > >> > >> Why is this logic changed? > >> > >> If you want to call dev_err_probe(), that's great, but do NOT mix it up > >> with a commit that does something totally different. > >> > >> When you say something like "while at it" in a changelog text, that is a > >> HUGE hint that it needs to be a separate commit. Because of that reason > >> alone, I can't take these, you know better :( > >> > >> thanks, > >> > > > > Ok, I will improve the commit message relevant to the code changes. > > Please read Greg's message one more time. He did not propose to fix > commit msg, right? > Ok I will drop dev_err_probe and keep the error flow as it is. It will not break the feature.. Thanks -Anand
diff --git a/drivers/usb/host/ehci-exynos.c b/drivers/usb/host/ehci-exynos.c index f644b131cc0b..f00bfd0b13dc 100644 --- a/drivers/usb/host/ehci-exynos.c +++ b/drivers/usb/host/ehci-exynos.c @@ -159,20 +159,15 @@ static int exynos_ehci_probe(struct platform_device *pdev) err = exynos_ehci_get_phy(&pdev->dev, exynos_ehci); if (err) - goto fail_clk; - - exynos_ehci->clk = devm_clk_get(&pdev->dev, "usbhost"); + goto fail_io; + exynos_ehci->clk = devm_clk_get_enabled(&pdev->dev, "usbhost"); if (IS_ERR(exynos_ehci->clk)) { - dev_err(&pdev->dev, "Failed to get usbhost clock\n"); - err = PTR_ERR(exynos_ehci->clk); - goto fail_clk; + usb_put_hcd(hcd); + return dev_err_probe(&pdev->dev, PTR_ERR(exynos_ehci->clk), + "Failed to get usbhost clock\n"); } - err = clk_prepare_enable(exynos_ehci->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); @@ -223,8 +218,6 @@ static int exynos_ehci_probe(struct platform_device *pdev) exynos_ehci_phy_disable(&pdev->dev); pdev->dev.of_node = exynos_ehci->of_node; fail_io: - clk_disable_unprepare(exynos_ehci->clk); -fail_clk: usb_put_hcd(hcd); return err; } @@ -240,8 +233,6 @@ static void exynos_ehci_remove(struct platform_device *pdev) exynos_ehci_phy_disable(&pdev->dev); - clk_disable_unprepare(exynos_ehci->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: drop the clk_disable_unprepare in suspend/resume functions fix the usb_put_hcd return before the devm_clk_get_enabled --- drivers/usb/host/ehci-exynos.c | 19 +++++-------------- 1 file changed, 5 insertions(+), 14 deletions(-)