| Message ID | 20230821062303.185174-1-yiyang13@huawei.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v2] mmc: sdhci-of-dwcmshc: Use helper function devm_clk_get_enabled() | expand |
On 21/08/23 09:23, Yi Yang wrote: > Since commit 7ef9651e9792 ("clk: Provide new devm_clk helpers for > prepared and enabled clocks"), devm_clk_get() and clk_prepare_enable() > can now be replaced by devm_clk_get_enabled() when the driver enables > (and possibly prepares) the clocks for the whole lifetime of the device. > Moreover, it is no longer necessary to unprepare and disable the clocks > explicitly. > > Signed-off-by: Yi Yang <yiyang13@huawei.com> > --- > v2: Remove clk_disable_unprepare in dwcmshc_remove() > --- > drivers/mmc/host/sdhci-of-dwcmshc.c | 20 ++++++++------------ > 1 file changed, 8 insertions(+), 12 deletions(-) > > diff --git a/drivers/mmc/host/sdhci-of-dwcmshc.c b/drivers/mmc/host/sdhci-of-dwcmshc.c > index 31c1892f4ecd..08b566984733 100644 > --- a/drivers/mmc/host/sdhci-of-dwcmshc.c > +++ b/drivers/mmc/host/sdhci-of-dwcmshc.c > @@ -495,19 +495,19 @@ static int dwcmshc_probe(struct platform_device *pdev) > priv = sdhci_pltfm_priv(pltfm_host); > > if (dev->of_node) { > - pltfm_host->clk = devm_clk_get(dev, "core"); > + pltfm_host->clk = devm_clk_get_enabled(dev, "core"); > if (IS_ERR(pltfm_host->clk)) { > err = PTR_ERR(pltfm_host->clk); > - dev_err(dev, "failed to get core clk: %d\n", err); > + dev_err(dev, "failed to get or enable core clk: %d\n", err); > goto free_pltfm; > } > - err = clk_prepare_enable(pltfm_host->clk); > - if (err) > - goto free_pltfm; > > - priv->bus_clk = devm_clk_get(dev, "bus"); > - if (!IS_ERR(priv->bus_clk)) > - clk_prepare_enable(priv->bus_clk); > + priv->bus_clk = devm_clk_get_enabled(dev, "bus"); > + if (!IS_ERR(priv->bus_clk)) { "!" does not belong, should be "if (IS_ERR(priv->bus_clk))" instead > + err = PTR_ERR(priv->bus_clk); > + dev_err(dev, "failed to get or enable bus clk: %d\n", err); > + goto free_pltfm; > + } > } > > err = mmc_of_parse(host->mmc); > @@ -564,8 +564,6 @@ static int dwcmshc_probe(struct platform_device *pdev) > err_setup_host: > sdhci_cleanup_host(host); > err_clk: > - clk_disable_unprepare(pltfm_host->clk); > - clk_disable_unprepare(priv->bus_clk); > if (rk_priv) > clk_bulk_disable_unprepare(RK35xx_MAX_CLKS, > rk_priv->rockchip_clks); > @@ -583,8 +581,6 @@ static void dwcmshc_remove(struct platform_device *pdev) > > sdhci_remove_host(host, 0); > > - clk_disable_unprepare(pltfm_host->clk); > - clk_disable_unprepare(priv->bus_clk); > if (rk_priv) > clk_bulk_disable_unprepare(RK35xx_MAX_CLKS, > rk_priv->rockchip_clks);
On 24/08/23 10:37, Adrian Hunter wrote: > On 21/08/23 09:23, Yi Yang wrote: >> Since commit 7ef9651e9792 ("clk: Provide new devm_clk helpers for >> prepared and enabled clocks"), devm_clk_get() and clk_prepare_enable() >> can now be replaced by devm_clk_get_enabled() when the driver enables >> (and possibly prepares) the clocks for the whole lifetime of the device. >> Moreover, it is no longer necessary to unprepare and disable the clocks >> explicitly. >> >> Signed-off-by: Yi Yang <yiyang13@huawei.com> >> --- >> v2: Remove clk_disable_unprepare in dwcmshc_remove() >> --- >> drivers/mmc/host/sdhci-of-dwcmshc.c | 20 ++++++++------------ >> 1 file changed, 8 insertions(+), 12 deletions(-) >> >> diff --git a/drivers/mmc/host/sdhci-of-dwcmshc.c b/drivers/mmc/host/sdhci-of-dwcmshc.c >> index 31c1892f4ecd..08b566984733 100644 >> --- a/drivers/mmc/host/sdhci-of-dwcmshc.c >> +++ b/drivers/mmc/host/sdhci-of-dwcmshc.c >> @@ -495,19 +495,19 @@ static int dwcmshc_probe(struct platform_device *pdev) >> priv = sdhci_pltfm_priv(pltfm_host); >> >> if (dev->of_node) { >> - pltfm_host->clk = devm_clk_get(dev, "core"); >> + pltfm_host->clk = devm_clk_get_enabled(dev, "core"); >> if (IS_ERR(pltfm_host->clk)) { >> err = PTR_ERR(pltfm_host->clk); >> - dev_err(dev, "failed to get core clk: %d\n", err); >> + dev_err(dev, "failed to get or enable core clk: %d\n", err); >> goto free_pltfm; >> } >> - err = clk_prepare_enable(pltfm_host->clk); >> - if (err) >> - goto free_pltfm; >> >> - priv->bus_clk = devm_clk_get(dev, "bus"); >> - if (!IS_ERR(priv->bus_clk)) >> - clk_prepare_enable(priv->bus_clk); >> + priv->bus_clk = devm_clk_get_enabled(dev, "bus"); >> + if (!IS_ERR(priv->bus_clk)) { > > "!" does not belong, should be "if (IS_ERR(priv->bus_clk))" instead Although previously it seemed like the clock was optional. At least the error was not considered fatal. > >> + err = PTR_ERR(priv->bus_clk); >> + dev_err(dev, "failed to get or enable bus clk: %d\n", err); >> + goto free_pltfm; >> + } >> } >> >> err = mmc_of_parse(host->mmc); >> @@ -564,8 +564,6 @@ static int dwcmshc_probe(struct platform_device *pdev) >> err_setup_host: >> sdhci_cleanup_host(host); >> err_clk: >> - clk_disable_unprepare(pltfm_host->clk); >> - clk_disable_unprepare(priv->bus_clk); >> if (rk_priv) >> clk_bulk_disable_unprepare(RK35xx_MAX_CLKS, >> rk_priv->rockchip_clks); >> @@ -583,8 +581,6 @@ static void dwcmshc_remove(struct platform_device *pdev) >> >> sdhci_remove_host(host, 0); >> >> - clk_disable_unprepare(pltfm_host->clk); >> - clk_disable_unprepare(priv->bus_clk); >> if (rk_priv) >> clk_bulk_disable_unprepare(RK35xx_MAX_CLKS, >> rk_priv->rockchip_clks); >
On 2023/8/24 15:42, Adrian Hunter wrote: > On 24/08/23 10:37, Adrian Hunter wrote: >> On 21/08/23 09:23, Yi Yang wrote: >>> Since commit 7ef9651e9792 ("clk: Provide new devm_clk helpers for >>> prepared and enabled clocks"), devm_clk_get() and clk_prepare_enable() >>> can now be replaced by devm_clk_get_enabled() when the driver enables >>> (and possibly prepares) the clocks for the whole lifetime of the device. >>> Moreover, it is no longer necessary to unprepare and disable the clocks >>> explicitly. >>> >>> Signed-off-by: Yi Yang <yiyang13@huawei.com> >>> --- >>> v2: Remove clk_disable_unprepare in dwcmshc_remove() >>> --- >>> drivers/mmc/host/sdhci-of-dwcmshc.c | 20 ++++++++------------ >>> 1 file changed, 8 insertions(+), 12 deletions(-) >>> >>> diff --git a/drivers/mmc/host/sdhci-of-dwcmshc.c b/drivers/mmc/host/sdhci-of-dwcmshc.c >>> index 31c1892f4ecd..08b566984733 100644 >>> --- a/drivers/mmc/host/sdhci-of-dwcmshc.c >>> +++ b/drivers/mmc/host/sdhci-of-dwcmshc.c >>> @@ -495,19 +495,19 @@ static int dwcmshc_probe(struct platform_device *pdev) >>> priv = sdhci_pltfm_priv(pltfm_host); >>> >>> if (dev->of_node) { >>> - pltfm_host->clk = devm_clk_get(dev, "core"); >>> + pltfm_host->clk = devm_clk_get_enabled(dev, "core"); >>> if (IS_ERR(pltfm_host->clk)) { >>> err = PTR_ERR(pltfm_host->clk); >>> - dev_err(dev, "failed to get core clk: %d\n", err); >>> + dev_err(dev, "failed to get or enable core clk: %d\n", err); >>> goto free_pltfm; >>> } >>> - err = clk_prepare_enable(pltfm_host->clk); >>> - if (err) >>> - goto free_pltfm; >>> >>> - priv->bus_clk = devm_clk_get(dev, "bus"); >>> - if (!IS_ERR(priv->bus_clk)) >>> - clk_prepare_enable(priv->bus_clk); >>> + priv->bus_clk = devm_clk_get_enabled(dev, "bus"); >>> + if (!IS_ERR(priv->bus_clk)) { >> >> "!" does not belong, should be "if (IS_ERR(priv->bus_clk))" instead > I'm sorry for this low-level mistake. > Although previously it seemed like the clock was optional. > At least the error was not considered fatal. > Yes,this not a fatal mistake. >> >>> + err = PTR_ERR(priv->bus_clk); >>> + dev_err(dev, "failed to get or enable bus clk: %d\n", err); >>> + goto free_pltfm; >>> + } >>> } >>> >>> err = mmc_of_parse(host->mmc); >>> @@ -564,8 +564,6 @@ static int dwcmshc_probe(struct platform_device *pdev) >>> err_setup_host: >>> sdhci_cleanup_host(host); >>> err_clk: >>> - clk_disable_unprepare(pltfm_host->clk); >>> - clk_disable_unprepare(priv->bus_clk); >>> if (rk_priv) >>> clk_bulk_disable_unprepare(RK35xx_MAX_CLKS, >>> rk_priv->rockchip_clks); >>> @@ -583,8 +581,6 @@ static void dwcmshc_remove(struct platform_device *pdev) >>> >>> sdhci_remove_host(host, 0); >>> >>> - clk_disable_unprepare(pltfm_host->clk); >>> - clk_disable_unprepare(priv->bus_clk); >>> if (rk_priv) >>> clk_bulk_disable_unprepare(RK35xx_MAX_CLKS, >>> rk_priv->rockchip_clks); >> > > > . >
diff --git a/drivers/mmc/host/sdhci-of-dwcmshc.c b/drivers/mmc/host/sdhci-of-dwcmshc.c index 31c1892f4ecd..08b566984733 100644 --- a/drivers/mmc/host/sdhci-of-dwcmshc.c +++ b/drivers/mmc/host/sdhci-of-dwcmshc.c @@ -495,19 +495,19 @@ static int dwcmshc_probe(struct platform_device *pdev) priv = sdhci_pltfm_priv(pltfm_host); if (dev->of_node) { - pltfm_host->clk = devm_clk_get(dev, "core"); + pltfm_host->clk = devm_clk_get_enabled(dev, "core"); if (IS_ERR(pltfm_host->clk)) { err = PTR_ERR(pltfm_host->clk); - dev_err(dev, "failed to get core clk: %d\n", err); + dev_err(dev, "failed to get or enable core clk: %d\n", err); goto free_pltfm; } - err = clk_prepare_enable(pltfm_host->clk); - if (err) - goto free_pltfm; - priv->bus_clk = devm_clk_get(dev, "bus"); - if (!IS_ERR(priv->bus_clk)) - clk_prepare_enable(priv->bus_clk); + priv->bus_clk = devm_clk_get_enabled(dev, "bus"); + if (!IS_ERR(priv->bus_clk)) { + err = PTR_ERR(priv->bus_clk); + dev_err(dev, "failed to get or enable bus clk: %d\n", err); + goto free_pltfm; + } } err = mmc_of_parse(host->mmc); @@ -564,8 +564,6 @@ static int dwcmshc_probe(struct platform_device *pdev) err_setup_host: sdhci_cleanup_host(host); err_clk: - clk_disable_unprepare(pltfm_host->clk); - clk_disable_unprepare(priv->bus_clk); if (rk_priv) clk_bulk_disable_unprepare(RK35xx_MAX_CLKS, rk_priv->rockchip_clks); @@ -583,8 +581,6 @@ static void dwcmshc_remove(struct platform_device *pdev) sdhci_remove_host(host, 0); - clk_disable_unprepare(pltfm_host->clk); - clk_disable_unprepare(priv->bus_clk); if (rk_priv) clk_bulk_disable_unprepare(RK35xx_MAX_CLKS, rk_priv->rockchip_clks);
Since commit 7ef9651e9792 ("clk: Provide new devm_clk helpers for prepared and enabled clocks"), devm_clk_get() and clk_prepare_enable() can now be replaced by devm_clk_get_enabled() when the driver enables (and possibly prepares) the clocks for the whole lifetime of the device. Moreover, it is no longer necessary to unprepare and disable the clocks explicitly. Signed-off-by: Yi Yang <yiyang13@huawei.com> --- v2: Remove clk_disable_unprepare in dwcmshc_remove() --- drivers/mmc/host/sdhci-of-dwcmshc.c | 20 ++++++++------------ 1 file changed, 8 insertions(+), 12 deletions(-)