Hi
sdhci_pltfm_unregister() does:
clk_disable_unprepare(pltfm_host->clk)
which prevents drivers from using devm_clk_get_enabled() or similar.
Move it out, and where drivers are doing devm_clk_get*() immediately
followed by clk_prepare_enable(), combine them into devm_clk_get_*enabled().
sdhci_pltfm_register() and sdhci_pltfm_unregister() are not paired functions.
That are just helpers and effectively get renamed:
sdhci_pltfm_register() -> sdhci_pltfm_init_and_add_host()
sdhci_pltfm_unregister() -> sdhci_pltfm_remove()
Please note, the patches are based on top of some
"Convert to platform remove callback returning void"
patches by Yangtao Li, which were posted here:
https://lore.kernel.org/linux-mmc/[email protected]/
Patches can also be found here:
https://github.com/ahunter6/linux/commits/sdhci-pltfm-cleanup-1
Adrian Hunter (16):
mmc: sdhci-pltfm: Add sdhci_pltfm_remove()
mmc: sdhci-bcm-kona: Use sdhci_pltfm_remove()
mmc: sdhci-brcmstb: Use sdhci_pltfm_remove()
mmc: sdhci-cadence: Use sdhci_pltfm_remove()
mmc: sdhci-dove: Use sdhci_pltfm_remove()
mmc: sdhci_f_sdh30: Use sdhci_pltfm_remove()
mmc: sdhci-iproc: Use sdhci_pltfm_remove()
mmc: sdhci-of-arasan: Use sdhci_pltfm_remove()
mmc: sdhci-of-at91: Use sdhci_pltfm_remove()
mmc: sdhci-of-esdhc: Use sdhci_pltfm_remove()
mmc: sdhci-of-hlwd: Use sdhci_pltfm_remove()
mmc: sdhci-of-sparx5: Use sdhci_pltfm_remove()
mmc: sdhci-pxav2: Use sdhci_pltfm_remove()
mmc: sdhci-st: Use sdhci_pltfm_remove()
mmc: sdhci-pltfm: Remove sdhci_pltfm_unregister()
mmc: sdhci-pltfm: Rename sdhci_pltfm_register()
drivers/mmc/host/sdhci-bcm-kona.c | 12 +++++++++++-
drivers/mmc/host/sdhci-brcmstb.c | 18 +++++-------------
drivers/mmc/host/sdhci-cadence.c | 17 ++++-------------
drivers/mmc/host/sdhci-dove.c | 8 ++------
drivers/mmc/host/sdhci-iproc.c | 14 +++-----------
drivers/mmc/host/sdhci-of-arasan.c | 4 +++-
drivers/mmc/host/sdhci-of-at91.c | 2 +-
drivers/mmc/host/sdhci-of-esdhc.c | 2 +-
drivers/mmc/host/sdhci-of-hlwd.c | 4 ++--
drivers/mmc/host/sdhci-of-sparx5.c | 17 ++++++-----------
drivers/mmc/host/sdhci-pltfm.c | 14 ++++++--------
drivers/mmc/host/sdhci-pltfm.h | 8 ++++----
drivers/mmc/host/sdhci-pxav2.c | 19 ++++++-------------
drivers/mmc/host/sdhci-st.c | 4 +++-
drivers/mmc/host/sdhci_f_sdh30.c | 2 +-
15 files changed, 58 insertions(+), 87 deletions(-)
Regards
Adrian
Use sdhci_pltfm_remove() instead of sdhci_pltfm_unregister() so that
devm_clk_get_enabled() can be used for pltfm_host->clk.
This has the side effect that the order of operations on the error path
and remove path is not the same as it was before, but should be safe
nevertheless.
Signed-off-by: Adrian Hunter <[email protected]>
---
drivers/mmc/host/sdhci-pxav2.c | 19 ++++++-------------
1 file changed, 6 insertions(+), 13 deletions(-)
diff --git a/drivers/mmc/host/sdhci-pxav2.c b/drivers/mmc/host/sdhci-pxav2.c
index a0c8470e9214..b75cbea88b40 100644
--- a/drivers/mmc/host/sdhci-pxav2.c
+++ b/drivers/mmc/host/sdhci-pxav2.c
@@ -268,26 +268,21 @@ static int sdhci_pxav2_probe(struct platform_device *pdev)
pltfm_host = sdhci_priv(host);
pxav2_host = sdhci_pltfm_priv(pltfm_host);
- clk = devm_clk_get(dev, "io");
- if (IS_ERR(clk) && PTR_ERR(clk) != -EPROBE_DEFER)
- clk = devm_clk_get(dev, NULL);
+ clk = devm_clk_get_optional_enabled(dev, "io");
+ if (!clk)
+ clk = devm_clk_get_enabled(dev, NULL);
if (IS_ERR(clk)) {
ret = PTR_ERR(clk);
dev_err_probe(dev, ret, "failed to get io clock\n");
goto free;
}
pltfm_host->clk = clk;
- ret = clk_prepare_enable(clk);
- if (ret) {
- dev_err(dev, "failed to enable io clock\n");
- goto free;
- }
clk_core = devm_clk_get_optional_enabled(dev, "core");
if (IS_ERR(clk_core)) {
ret = PTR_ERR(clk_core);
dev_err_probe(dev, ret, "failed to enable core clock\n");
- goto disable_clk;
+ goto free;
}
host->quirks = SDHCI_QUIRK_BROKEN_ADMA
@@ -339,12 +334,10 @@ static int sdhci_pxav2_probe(struct platform_device *pdev)
ret = sdhci_add_host(host);
if (ret)
- goto disable_clk;
+ goto free;
return 0;
-disable_clk:
- clk_disable_unprepare(clk);
free:
sdhci_pltfm_free(pdev);
return ret;
@@ -358,7 +351,7 @@ static struct platform_driver sdhci_pxav2_driver = {
.pm = &sdhci_pltfm_pmops,
},
.probe = sdhci_pxav2_probe,
- .remove_new = sdhci_pltfm_unregister,
+ .remove_new = sdhci_pltfm_remove,
};
module_platform_driver(sdhci_pxav2_driver);
--
2.34.1
Use sdhci_pltfm_remove() instead of sdhci_pltfm_unregister() because
sdhci_pltfm_unregister() is going to be removed.
Signed-off-by: Adrian Hunter <[email protected]>
---
drivers/mmc/host/sdhci-st.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/mmc/host/sdhci-st.c b/drivers/mmc/host/sdhci-st.c
index 14fc2c386bd3..d12532b96b51 100644
--- a/drivers/mmc/host/sdhci-st.c
+++ b/drivers/mmc/host/sdhci-st.c
@@ -437,10 +437,12 @@ static void sdhci_st_remove(struct platform_device *pdev)
struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
struct st_mmc_platform_data *pdata = sdhci_pltfm_priv(pltfm_host);
struct reset_control *rstc = pdata->rstc;
+ struct clk *clk = pltfm_host->clk;
- sdhci_pltfm_unregister(pdev);
+ sdhci_pltfm_remove(pdev);
clk_disable_unprepare(pdata->icnclk);
+ clk_disable_unprepare(clk);
reset_control_assert(rstc);
}
--
2.34.1
On Fri, Aug 11, 2023 at 04:03:35PM +0300, Adrian Hunter wrote:
> Hi
>
> sdhci_pltfm_unregister() does:
>
> clk_disable_unprepare(pltfm_host->clk)
>
> which prevents drivers from using devm_clk_get_enabled() or similar.
>
> Move it out, and where drivers are doing devm_clk_get*() immediately
> followed by clk_prepare_enable(), combine them into devm_clk_get_*enabled().
>
> sdhci_pltfm_register() and sdhci_pltfm_unregister() are not paired functions.
> That are just helpers and effectively get renamed:
>
> sdhci_pltfm_register() -> sdhci_pltfm_init_and_add_host()
> sdhci_pltfm_unregister() -> sdhci_pltfm_remove()
>
> Please note, the patches are based on top of some
> "Convert to platform remove callback returning void"
> patches by Yangtao Li, which were posted here:
>
> https://lore.kernel.org/linux-mmc/[email protected]/
>
> Patches can also be found here:
>
> https://github.com/ahunter6/linux/commits/sdhci-pltfm-cleanup-1
All look good to me, thanks!
Reviewed-by: Andy Shevchenko <[email protected]>
> Adrian Hunter (16):
> mmc: sdhci-pltfm: Add sdhci_pltfm_remove()
> mmc: sdhci-bcm-kona: Use sdhci_pltfm_remove()
> mmc: sdhci-brcmstb: Use sdhci_pltfm_remove()
> mmc: sdhci-cadence: Use sdhci_pltfm_remove()
> mmc: sdhci-dove: Use sdhci_pltfm_remove()
> mmc: sdhci_f_sdh30: Use sdhci_pltfm_remove()
> mmc: sdhci-iproc: Use sdhci_pltfm_remove()
> mmc: sdhci-of-arasan: Use sdhci_pltfm_remove()
> mmc: sdhci-of-at91: Use sdhci_pltfm_remove()
> mmc: sdhci-of-esdhc: Use sdhci_pltfm_remove()
> mmc: sdhci-of-hlwd: Use sdhci_pltfm_remove()
> mmc: sdhci-of-sparx5: Use sdhci_pltfm_remove()
> mmc: sdhci-pxav2: Use sdhci_pltfm_remove()
> mmc: sdhci-st: Use sdhci_pltfm_remove()
> mmc: sdhci-pltfm: Remove sdhci_pltfm_unregister()
> mmc: sdhci-pltfm: Rename sdhci_pltfm_register()
>
> drivers/mmc/host/sdhci-bcm-kona.c | 12 +++++++++++-
> drivers/mmc/host/sdhci-brcmstb.c | 18 +++++-------------
> drivers/mmc/host/sdhci-cadence.c | 17 ++++-------------
> drivers/mmc/host/sdhci-dove.c | 8 ++------
> drivers/mmc/host/sdhci-iproc.c | 14 +++-----------
> drivers/mmc/host/sdhci-of-arasan.c | 4 +++-
> drivers/mmc/host/sdhci-of-at91.c | 2 +-
> drivers/mmc/host/sdhci-of-esdhc.c | 2 +-
> drivers/mmc/host/sdhci-of-hlwd.c | 4 ++--
> drivers/mmc/host/sdhci-of-sparx5.c | 17 ++++++-----------
> drivers/mmc/host/sdhci-pltfm.c | 14 ++++++--------
> drivers/mmc/host/sdhci-pltfm.h | 8 ++++----
> drivers/mmc/host/sdhci-pxav2.c | 19 ++++++-------------
> drivers/mmc/host/sdhci-st.c | 4 +++-
> drivers/mmc/host/sdhci_f_sdh30.c | 2 +-
> 15 files changed, 58 insertions(+), 87 deletions(-)
>
>
> Regards
> Adrian
--
With Best Regards,
Andy Shevchenko
Now that sdhci_pltfm_unregister() is unused, remove it.
Signed-off-by: Adrian Hunter <[email protected]>
---
drivers/mmc/host/sdhci-pltfm.c | 12 ------------
drivers/mmc/host/sdhci-pltfm.h | 1 -
2 files changed, 13 deletions(-)
diff --git a/drivers/mmc/host/sdhci-pltfm.c b/drivers/mmc/host/sdhci-pltfm.c
index 5a63c8818987..894f3bbe2b0f 100644
--- a/drivers/mmc/host/sdhci-pltfm.c
+++ b/drivers/mmc/host/sdhci-pltfm.c
@@ -187,18 +187,6 @@ int sdhci_pltfm_register(struct platform_device *pdev,
}
EXPORT_SYMBOL_GPL(sdhci_pltfm_register);
-void sdhci_pltfm_unregister(struct platform_device *pdev)
-{
- struct sdhci_host *host = platform_get_drvdata(pdev);
- struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
- int dead = (readl(host->ioaddr + SDHCI_INT_STATUS) == 0xffffffff);
-
- sdhci_remove_host(host, dead);
- clk_disable_unprepare(pltfm_host->clk);
- sdhci_pltfm_free(pdev);
-}
-EXPORT_SYMBOL_GPL(sdhci_pltfm_unregister);
-
void sdhci_pltfm_remove(struct platform_device *pdev)
{
struct sdhci_host *host = platform_get_drvdata(pdev);
diff --git a/drivers/mmc/host/sdhci-pltfm.h b/drivers/mmc/host/sdhci-pltfm.h
index 402f4edc6ca5..bebc450d5098 100644
--- a/drivers/mmc/host/sdhci-pltfm.h
+++ b/drivers/mmc/host/sdhci-pltfm.h
@@ -102,7 +102,6 @@ extern void sdhci_pltfm_free(struct platform_device *pdev);
extern int sdhci_pltfm_register(struct platform_device *pdev,
const struct sdhci_pltfm_data *pdata,
size_t priv_size);
-extern void sdhci_pltfm_unregister(struct platform_device *pdev);
extern void sdhci_pltfm_remove(struct platform_device *pdev);
extern unsigned int sdhci_pltfm_clk_get_max_clock(struct sdhci_host *host);
--
2.34.1