[PATCH 0/1] mmc: sdhci-of-dwcmshc: enhance framework

[PATCH 0/1] mmc: sdhci-of-dwcmshc: enhance framework

[Chen Wang]

From: Chen Wang <unicorn_wang@outlook.com>

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

[PATCH 1/1] mmc: sdhci-of-dwcmshc: add callback framework for expansion

[Chen Wang]

From: Chen Wang <unicorn_wang@outlook.com>

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 <unicorn_wang@outlook.com>
---
 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

Re: [PATCH 0/1] mmc: sdhci-of-dwcmshc: enhance framework

[Chen Wang]

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.

Link：https://lore.kernel.org/linux-riscv/cover.1713258948.git.unicorn_wang@outlook.com/ 
[1]

On 2024/4/16 17:43, Chen Wang wrote:
> From: Chen Wang <unicorn_wang@outlook.com>
>
> 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

Re: [PATCH 0/1] mmc: sdhci-of-dwcmshc: enhance framework

[Adrian Hunter]

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/cover.1713258948.git.unicorn_wang@outlook.com/ [1]
> 
> On 2024/4/16 17:43, Chen Wang wrote:
>> From: Chen Wang <unicorn_wang@outlook.com>
>>
>> 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

Re: [PATCH 0/1] mmc: sdhci-of-dwcmshc: enhance framework

[Chen Wang]

On 2024/4/22 18:44, Adrian Hunter wrote:
> 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
ok
>> Link：https://lore.kernel.org/linux-riscv/cover.1713258948.git.unicorn_wang@outlook.com/ [1]
>>
>> On 2024/4/16 17:43, Chen Wang wrote:
>>> From: Chen Wang <unicorn_wang@outlook.com>
>>>
>>> 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