From: Chen Wang <[email protected]>
When I tried to add a new soc to sdhci-of-dwcmshc, I found that the
existing driver code could be optimized to facilitate expansion for
the new soc.
Thanks,
Chen
---
Chen Wang (1):
mmc: sdhci-of-dwcmshc: add callback framework for expansion
drivers/mmc/host/sdhci-of-dwcmshc.c | 185 ++++++++++++++++------------
1 file changed, 107 insertions(+), 78 deletions(-)
base-commit: 4cece764965020c22cff7665b18a012006359095
--
2.25.1
From: Chen Wang <[email protected]>
The current framework is not easily extended to support new SOCs.
For example, in the current code we see that the SOC-level
structure `rk35xx_priv` and related logic are distributed in
functions such as dwcmshc_probe/dwcmshc_remove/dwcmshc_suspend/......,
which is inappropriate.
The solution is to abstract some possible common operations of soc
into virtual members of `dwcmshc_priv`. Each soc implements its own
corresponding callback function and registers it in init function.
dwcmshc framework is responsible for calling these callback functions
in those dwcmshc_xxx functions.
Signed-off-by: Chen Wang <[email protected]>
---
drivers/mmc/host/sdhci-of-dwcmshc.c | 185 ++++++++++++++++------------
1 file changed, 107 insertions(+), 78 deletions(-)
diff --git a/drivers/mmc/host/sdhci-of-dwcmshc.c b/drivers/mmc/host/sdhci-of-dwcmshc.c
index ab4b964d4058..1ac8537361de 100644
--- a/drivers/mmc/host/sdhci-of-dwcmshc.c
+++ b/drivers/mmc/host/sdhci-of-dwcmshc.c
@@ -200,6 +200,10 @@ struct dwcmshc_priv {
void *priv; /* pointer to SoC private stuff */
u16 delay_line;
u16 flags;
+
+ void (*soc_postinit)(struct sdhci_host *host, struct dwcmshc_priv *dwc_priv);
+ int (*soc_clks_enable)(struct dwcmshc_priv *dwc_priv);
+ void (*soc_clks_disable)(struct dwcmshc_priv *dwc_priv);
};
/*
@@ -758,37 +762,81 @@ static const struct sdhci_pltfm_data sdhci_dwcmshc_cv18xx_pdata = {
.quirks2 = SDHCI_QUIRK2_PRESET_VALUE_BROKEN,
};
-static int dwcmshc_rk35xx_init(struct sdhci_host *host, struct dwcmshc_priv *dwc_priv)
+static void dwcmshc_rk35xx_postinit(struct sdhci_host *host, struct dwcmshc_priv *dwc_priv)
+{
+ /*
+ * Don't support highspeed bus mode with low clk speed as we
+ * cannot use DLL for this condition.
+ */
+ if (host->mmc->f_max <= 52000000) {
+ dev_info(mmc_dev(host->mmc), "Disabling HS200/HS400, frequency too low (%d)\n",
+ host->mmc->f_max);
+ host->mmc->caps2 &= ~(MMC_CAP2_HS200 | MMC_CAP2_HS400);
+ host->mmc->caps &= ~(MMC_CAP_3_3V_DDR | MMC_CAP_1_8V_DDR);
+ }
+}
+
+static int dwcmshc_rk35xx_clks_enable(struct dwcmshc_priv *dwc_priv)
+{
+ struct rk35xx_priv *soc = dwc_priv->priv;
+ int ret = 0;
+
+ if (soc)
+ ret = clk_bulk_prepare_enable(RK35xx_MAX_CLKS, soc->rockchip_clks);
+ return ret;
+}
+
+static void dwcmshc_rk35xx_clks_disable(struct dwcmshc_priv *dwc_priv)
+{
+ struct rk35xx_priv *rk_priv = dwc_priv->priv;
+
+ if (rk_priv)
+ clk_bulk_disable_unprepare(RK35xx_MAX_CLKS,
+ rk_priv->rockchip_clks);
+}
+
+static int dwcmshc_rk35xx_init(struct device *dev,
+ struct sdhci_host *host,
+ struct dwcmshc_priv *dwc_priv)
{
int err;
- struct rk35xx_priv *priv = dwc_priv->priv;
+ struct rk35xx_priv *rk_priv;
- priv->reset = devm_reset_control_array_get_optional_exclusive(mmc_dev(host->mmc));
- if (IS_ERR(priv->reset)) {
- err = PTR_ERR(priv->reset);
+ rk_priv = devm_kzalloc(dev, sizeof(struct rk35xx_priv), GFP_KERNEL);
+ if (!rk_priv)
+ return -ENOMEM;
+
+ if (of_device_is_compatible(dev->of_node, "rockchip,rk3588-dwcmshc"))
+ rk_priv->devtype = DWCMSHC_RK3588;
+ else
+ rk_priv->devtype = DWCMSHC_RK3568;
+
+ rk_priv->reset = devm_reset_control_array_get_optional_exclusive(mmc_dev(host->mmc));
+ if (IS_ERR(rk_priv->reset)) {
+ err = PTR_ERR(rk_priv->reset);
dev_err(mmc_dev(host->mmc), "failed to get reset control %d\n", err);
return err;
}
- priv->rockchip_clks[0].id = "axi";
- priv->rockchip_clks[1].id = "block";
- priv->rockchip_clks[2].id = "timer";
+ rk_priv->rockchip_clks[0].id = "axi";
+ rk_priv->rockchip_clks[1].id = "block";
+ rk_priv->rockchip_clks[2].id = "timer";
err = devm_clk_bulk_get_optional(mmc_dev(host->mmc), RK35xx_MAX_CLKS,
- priv->rockchip_clks);
+ rk_priv->rockchip_clks);
if (err) {
dev_err(mmc_dev(host->mmc), "failed to get clocks %d\n", err);
return err;
}
- err = clk_bulk_prepare_enable(RK35xx_MAX_CLKS, priv->rockchip_clks);
+ err = clk_bulk_prepare_enable(RK35xx_MAX_CLKS, rk_priv->rockchip_clks);
if (err) {
dev_err(mmc_dev(host->mmc), "failed to enable clocks %d\n", err);
return err;
}
if (of_property_read_u8(mmc_dev(host->mmc)->of_node, "rockchip,txclk-tapnum",
- &priv->txclk_tapnum))
- priv->txclk_tapnum = DLL_TXCLK_TAPNUM_DEFAULT;
+ &rk_priv->txclk_tapnum))
+ rk_priv->txclk_tapnum = DLL_TXCLK_TAPNUM_DEFAULT;
/* Disable cmd conflict check */
sdhci_writel(host, 0x0, dwc_priv->vendor_specific_area1 + DWCMSHC_HOST_CTRL3);
@@ -796,21 +844,41 @@ static int dwcmshc_rk35xx_init(struct sdhci_host *host, struct dwcmshc_priv *dwc
sdhci_writel(host, 0, DWCMSHC_EMMC_DLL_TXCLK);
sdhci_writel(host, 0, DWCMSHC_EMMC_DLL_STRBIN);
+ dwc_priv->priv = rk_priv;
+ dwc_priv->soc_postinit = dwcmshc_rk35xx_postinit;
+ dwc_priv->soc_clks_enable = dwcmshc_rk35xx_clks_enable;
+ dwc_priv->soc_clks_disable = dwcmshc_rk35xx_clks_disable;
+
return 0;
}
-static void dwcmshc_rk35xx_postinit(struct sdhci_host *host, struct dwcmshc_priv *dwc_priv)
+static int dwcmshc_th1520_init(struct device *dev,
+ struct sdhci_host *host,
+ struct dwcmshc_priv *dwc_priv)
{
+ dwc_priv->delay_line = PHY_SDCLKDL_DC_DEFAULT;
+
+ if (device_property_read_bool(dev, "mmc-ddr-1_8v") ||
+ device_property_read_bool(dev, "mmc-hs200-1_8v") ||
+ device_property_read_bool(dev, "mmc-hs400-1_8v"))
+ dwc_priv->flags |= FLAG_IO_FIXED_1V8;
+ else
+ dwc_priv->flags &= ~FLAG_IO_FIXED_1V8;
+
/*
- * Don't support highspeed bus mode with low clk speed as we
- * cannot use DLL for this condition.
+ * start_signal_voltage_switch() will try 3.3V first
+ * then 1.8V. Use SDHCI_SIGNALING_180 rather than
+ * SDHCI_SIGNALING_330 to avoid setting voltage to 3.3V
+ * in sdhci_start_signal_voltage_switch().
*/
- if (host->mmc->f_max <= 52000000) {
- dev_info(mmc_dev(host->mmc), "Disabling HS200/HS400, frequency too low (%d)\n",
- host->mmc->f_max);
- host->mmc->caps2 &= ~(MMC_CAP2_HS200 | MMC_CAP2_HS400);
- host->mmc->caps &= ~(MMC_CAP_3_3V_DDR | MMC_CAP_1_8V_DDR);
+ if (dwc_priv->flags & FLAG_IO_FIXED_1V8) {
+ host->flags &= ~SDHCI_SIGNALING_330;
+ host->flags |= SDHCI_SIGNALING_180;
}
+
+ sdhci_enable_v4_mode(host);
+
+ return 0;
}
static const struct of_device_id sdhci_dwcmshc_dt_ids[] = {
@@ -859,7 +927,6 @@ static int dwcmshc_probe(struct platform_device *pdev)
struct sdhci_pltfm_host *pltfm_host;
struct sdhci_host *host;
struct dwcmshc_priv *priv;
- struct rk35xx_priv *rk_priv = NULL;
const struct sdhci_pltfm_data *pltfm_data;
int err;
u32 extra;
@@ -915,46 +982,15 @@ static int dwcmshc_probe(struct platform_device *pdev)
host->mmc_host_ops.hs400_enhanced_strobe = dwcmshc_hs400_enhanced_strobe;
if (pltfm_data == &sdhci_dwcmshc_rk35xx_pdata) {
- rk_priv = devm_kzalloc(&pdev->dev, sizeof(struct rk35xx_priv), GFP_KERNEL);
- if (!rk_priv) {
- err = -ENOMEM;
- goto err_clk;
- }
-
- if (of_device_is_compatible(pdev->dev.of_node, "rockchip,rk3588-dwcmshc"))
- rk_priv->devtype = DWCMSHC_RK3588;
- else
- rk_priv->devtype = DWCMSHC_RK3568;
-
- priv->priv = rk_priv;
-
- err = dwcmshc_rk35xx_init(host, priv);
+ err = dwcmshc_rk35xx_init(dev, host, priv);
if (err)
goto err_clk;
}
if (pltfm_data == &sdhci_dwcmshc_th1520_pdata) {
- priv->delay_line = PHY_SDCLKDL_DC_DEFAULT;
-
- if (device_property_read_bool(dev, "mmc-ddr-1_8v") ||
- device_property_read_bool(dev, "mmc-hs200-1_8v") ||
- device_property_read_bool(dev, "mmc-hs400-1_8v"))
- priv->flags |= FLAG_IO_FIXED_1V8;
- else
- priv->flags &= ~FLAG_IO_FIXED_1V8;
-
- /*
- * start_signal_voltage_switch() will try 3.3V first
- * then 1.8V. Use SDHCI_SIGNALING_180 rather than
- * SDHCI_SIGNALING_330 to avoid setting voltage to 3.3V
- * in sdhci_start_signal_voltage_switch().
- */
- if (priv->flags & FLAG_IO_FIXED_1V8) {
- host->flags &= ~SDHCI_SIGNALING_330;
- host->flags |= SDHCI_SIGNALING_180;
- }
-
- sdhci_enable_v4_mode(host);
+ err = dwcmshc_th1520_init(dev, host, priv);
+ if (err)
+ goto err_clk;
}
#ifdef CONFIG_ACPI
@@ -972,8 +1008,8 @@ static int dwcmshc_probe(struct platform_device *pdev)
if (err)
goto err_rpm;
- if (rk_priv)
- dwcmshc_rk35xx_postinit(host, priv);
+ if (priv->soc_postinit)
+ priv->soc_postinit(host, priv);
err = __sdhci_add_host(host);
if (err)
@@ -991,9 +1027,8 @@ static int dwcmshc_probe(struct platform_device *pdev)
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);
+ if (priv->soc_clks_disable)
+ priv->soc_clks_disable(priv);
free_pltfm:
sdhci_pltfm_free(pdev);
return err;
@@ -1004,15 +1039,13 @@ static void dwcmshc_remove(struct platform_device *pdev)
struct sdhci_host *host = platform_get_drvdata(pdev);
struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
struct dwcmshc_priv *priv = sdhci_pltfm_priv(pltfm_host);
- struct rk35xx_priv *rk_priv = priv->priv;
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);
+ if (priv->soc_clks_disable)
+ priv->soc_clks_disable(priv);
sdhci_pltfm_free(pdev);
}
@@ -1022,7 +1055,6 @@ static int dwcmshc_suspend(struct device *dev)
struct sdhci_host *host = dev_get_drvdata(dev);
struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
struct dwcmshc_priv *priv = sdhci_pltfm_priv(pltfm_host);
- struct rk35xx_priv *rk_priv = priv->priv;
int ret;
pm_runtime_resume(dev);
@@ -1035,9 +1067,8 @@ static int dwcmshc_suspend(struct device *dev)
if (!IS_ERR(priv->bus_clk))
clk_disable_unprepare(priv->bus_clk);
- if (rk_priv)
- clk_bulk_disable_unprepare(RK35xx_MAX_CLKS,
- rk_priv->rockchip_clks);
+ if (priv->soc_clks_disable)
+ priv->soc_clks_disable(priv);
return ret;
}
@@ -1047,7 +1078,6 @@ static int dwcmshc_resume(struct device *dev)
struct sdhci_host *host = dev_get_drvdata(dev);
struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
struct dwcmshc_priv *priv = sdhci_pltfm_priv(pltfm_host);
- struct rk35xx_priv *rk_priv = priv->priv;
int ret;
ret = clk_prepare_enable(pltfm_host->clk);
@@ -1060,23 +1090,22 @@ static int dwcmshc_resume(struct device *dev)
goto disable_clk;
}
- if (rk_priv) {
- ret = clk_bulk_prepare_enable(RK35xx_MAX_CLKS,
- rk_priv->rockchip_clks);
+ if (priv->soc_clks_enable) {
+ ret = priv->soc_clks_enable(priv);
if (ret)
goto disable_bus_clk;
}
ret = sdhci_resume_host(host);
if (ret)
- goto disable_rockchip_clks;
+ goto disable_soc_clks;
return 0;
-disable_rockchip_clks:
- if (rk_priv)
- clk_bulk_disable_unprepare(RK35xx_MAX_CLKS,
- rk_priv->rockchip_clks);
+disable_soc_clks:
+ if (priv->soc_clks_disable)
+ priv->soc_clks_disable(priv);
+
disable_bus_clk:
if (!IS_ERR(priv->bus_clk))
clk_disable_unprepare(priv->bus_clk);
--
2.25.1
On 22/04/24 12:01, Chen Wang wrote:
> hi， any comments on this patch?
>
> Another patch [1] has dependency on this one, so I would hope someone can review and comment this one first, thanks.
Does not apply. Please re-base on mmc 'next' branch of:
git://git.kernel.org/pub/scm/linux/kernel/git/ulfh/mmc.git
>
> Link：https://lore.kernel.org/linux-riscv/[email protected]/ [1]
>
> On 2024/4/16 17:43, Chen Wang wrote:
>> From: Chen Wang <[email protected]>
>>
>> When I tried to add a new soc to sdhci-of-dwcmshc, I found that the
>> existing driver code could be optimized to facilitate expansion for
>> the new soc.
>>
>> Thanks,
>> Chen
>>
>> ---
>>
>> Chen Wang (1):
>> mmc: sdhci-of-dwcmshc: add callback framework for expansion
>>
>> drivers/mmc/host/sdhci-of-dwcmshc.c | 185 ++++++++++++++++------------
>> 1 file changed, 107 insertions(+), 78 deletions(-)
>>
>>
>> base-commit: 4cece764965020c22cff7665b18a012006359095