linux-mmc.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v6 0/8] mmc: sdhci-of-dwcmshc: Add Sophgo SG2042 support
@ 2024-08-05  9:15 Chen Wang
  2024-08-05  9:17 ` [PATCH v6 1/8] mmc: sdhci-of-dwcmshc: add common bulk optional clocks support Chen Wang
                   ` (9 more replies)
  0 siblings, 10 replies; 22+ messages in thread
From: Chen Wang @ 2024-08-05  9:15 UTC (permalink / raw)
  To: adrian.hunter, aou, conor+dt, guoren, inochiama, jszhang,
	krzysztof.kozlowski+dt, palmer, paul.walmsley, robh, ulf.hansson,
	devicetree, linux-kernel, linux-mmc, linux-riscv, chao.wei,
	haijiao.liu, xiaoguang.xing, tingzhu.wang
  Cc: Chen Wang

From: Chen Wang <unicorn_wang@outlook.com>

This patchset is composed of two parts:
- one is the improvement of the sdhci-of-dwcmshc framework,
- the other is the support for sg2042 based on the improvement of the
  framework.
The reason for merging the two parts into one patchset is mainly to
facilitate review, especially to facilitate viewing why we need to
improve the framework and what benefits it will bring to us.

When I tried to add a new soc(SG2042) to sdhci-of-dwcmshc, I found
that the existing driver code could be optimized to facilitate expansion
for the new soc. Patch 1 ~ Patch 5 is for this.

Patch 6 ~ 7 are adding support for the mmc controller for Sophgo SG2042.
Adding corresponding new compatible strings, and implement
custom callbacks for SG2042 based on new framework.

Patch 8 is the change for DTS.

By the way, although I believe this patch only optimizes the framework
of the code and does not change the specific logic, simple verification
is certainly better. Since I don't have rk35xx/th1520 related hardware,
it would be greatly appreciated if someone could help verify it.

---

Changes in v6:

  The patch series is based on latest 'next' branch of [mmc-git].

  - Some minor improvements based on Adrian's review suggestions.
  - Added Reviewed-by and Tested-by signatures from Conor/Drew/Inochi.

Changes in v5:

  The patch series is based on latest 'next' branch of [mmc-git]. You can simply
  review or test the patches at the link [5].

  - Based on Adrian's suggestion, split the first part of the patch into 5.
  - Updated bindings and DTS as per suggestion from Krzysztof, Jisheng and Conor.

Changes in v4:

  The patch series is based on latest 'next' branch of [mmc-git]. You can simply
  review or test the patches at the link [4].

  Improved the dirvier code as per comments from Adrian Hunter, drop moving
  position and renaming for some helper functions.

  Put the sg2042 support as part of this series, improve the bindings and code
  as per comments from last review.

Changes in v3:
  
  The patch series is based on latest 'next' branch of [mmc-git]. You can simply
  review or test the patches at the link [3].

  Improved the dirvier code as per comments from Adrian Hunter.
  Define new structure for dwcmshc platform data/ops. In addition, I organized
  the code and classified the helper functions.

  Since the file changes were relatively large (though the functional logic did
  not change much), I split the original patch into four for the convenience of
  review.

Changes in v2:

  Rebased on latest 'next' branch of [mmc-git]. You can simply review or test the
  patches at the link [2].

Changes in v1:

  The patch series is based on v6.9-rc1. You can simply review or test the
  patches at the link [1].

Link: git://git.kernel.org/pub/scm/linux/kernel/git/ulfh/mmc.git [mmc-git]
Link: https://lore.kernel.org/linux-mmc/cover.1713257181.git.unicorn_wang@outlook.com/ [1]
Link: https://lore.kernel.org/linux-mmc/cover.1714270290.git.unicorn_wang@outlook.com/ [2]
Link: https://lore.kernel.org/linux-mmc/cover.1718241495.git.unicorn_wang@outlook.com/ [3]
Link: https://lore.kernel.org/linux-mmc/cover.1718697954.git.unicorn_wang@outlook.com/ [4]
Link: https://lore.kernel.org/linux-mmc/cover.1721377374.git.unicorn_wang@outlook.com/ [5]

---

Chen Wang (8):
  mmc: sdhci-of-dwcmshc: add common bulk optional clocks support
  mmc: sdhci-of-dwcmshc: move two rk35xx functions
  mmc: sdhci-of-dwcmshc: factor out code for th1520_init()
  mmc: sdhci-of-dwcmshc: factor out code into dwcmshc_rk35xx_init
  mmc: sdhci-of-dwcmshc: add dwcmshc_pltfm_data
  dt-bindings: mmc: sdhci-of-dwcmhsc: Add Sophgo SG2042 support
  mmc: sdhci-of-dwcmshc: Add support for Sophgo SG2042
  riscv: sophgo: dts: add mmc controllers for SG2042 SoC

 .../bindings/mmc/snps,dwcmshc-sdhci.yaml      |  60 ++-
 .../boot/dts/sophgo/sg2042-milkv-pioneer.dts  |  17 +
 arch/riscv/boot/dts/sophgo/sg2042.dtsi        |  28 ++
 drivers/mmc/host/sdhci-of-dwcmshc.c           | 451 ++++++++++++------
 4 files changed, 383 insertions(+), 173 deletions(-)


base-commit: 538076ce6b8dfe5e8e8d9d250298030f165d8457
-- 
2.34.1


^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH v6 1/8] mmc: sdhci-of-dwcmshc: add common bulk optional clocks support
  2024-08-05  9:15 [PATCH v6 0/8] mmc: sdhci-of-dwcmshc: Add Sophgo SG2042 support Chen Wang
@ 2024-08-05  9:17 ` Chen Wang
  2024-08-08  7:17   ` Adrian Hunter
  2025-07-22 18:33   ` Robin Murphy
  2024-08-05  9:17 ` [PATCH v6 2/8] mmc: sdhci-of-dwcmshc: move two rk35xx functions Chen Wang
                   ` (8 subsequent siblings)
  9 siblings, 2 replies; 22+ messages in thread
From: Chen Wang @ 2024-08-05  9:17 UTC (permalink / raw)
  To: adrian.hunter, aou, conor+dt, guoren, inochiama, jszhang,
	krzysztof.kozlowski+dt, palmer, paul.walmsley, robh, ulf.hansson,
	devicetree, linux-kernel, linux-mmc, linux-riscv, chao.wei,
	haijiao.liu, xiaoguang.xing, tingzhu.wang
  Cc: Chen Wang, Drew Fustini

From: Chen Wang <unicorn_wang@outlook.com>

In addition to the required core clock and optional
bus clock, the soc will expand its own clocks, so
the bulk clock mechanism is abstracted.

Note, I call the bulk clocks as "other clocks" due
to the bus clock has been called as "optional".

Signed-off-by: Chen Wang <unicorn_wang@outlook.com>
Tested-by: Drew Fustini <drew@pdp7.com> # TH1520
Tested-by: Inochi Amaoto <inochiama@outlook.com> # Duo and Huashan Pi
---
 drivers/mmc/host/sdhci-of-dwcmshc.c | 90 +++++++++++++++--------------
 1 file changed, 48 insertions(+), 42 deletions(-)

diff --git a/drivers/mmc/host/sdhci-of-dwcmshc.c b/drivers/mmc/host/sdhci-of-dwcmshc.c
index e79aa4b3b6c3..35401616fb2e 100644
--- a/drivers/mmc/host/sdhci-of-dwcmshc.c
+++ b/drivers/mmc/host/sdhci-of-dwcmshc.c
@@ -108,7 +108,6 @@
 #define DLL_LOCK_WO_TMOUT(x) \
 	((((x) & DWCMSHC_EMMC_DLL_LOCKED) == DWCMSHC_EMMC_DLL_LOCKED) && \
 	(((x) & DWCMSHC_EMMC_DLL_TIMEOUT) == 0))
-#define RK35xx_MAX_CLKS 3
 
 /* PHY register area pointer */
 #define DWC_MSHC_PTR_PHY_R	0x300
@@ -199,23 +198,54 @@ enum dwcmshc_rk_type {
 };
 
 struct rk35xx_priv {
-	/* Rockchip specified optional clocks */
-	struct clk_bulk_data rockchip_clks[RK35xx_MAX_CLKS];
 	struct reset_control *reset;
 	enum dwcmshc_rk_type devtype;
 	u8 txclk_tapnum;
 };
 
+#define DWCMSHC_MAX_OTHER_CLKS 3
+
 struct dwcmshc_priv {
 	struct clk	*bus_clk;
 	int vendor_specific_area1; /* P_VENDOR_SPECIFIC_AREA1 reg */
 	int vendor_specific_area2; /* P_VENDOR_SPECIFIC_AREA2 reg */
 
+	int num_other_clks;
+	struct clk_bulk_data other_clks[DWCMSHC_MAX_OTHER_CLKS];
+
 	void *priv; /* pointer to SoC private stuff */
 	u16 delay_line;
 	u16 flags;
 };
 
+static int dwcmshc_get_enable_other_clks(struct device *dev,
+					 struct dwcmshc_priv *priv,
+					 int num_clks,
+					 const char * const clk_ids[])
+{
+	int err;
+
+	if (num_clks > DWCMSHC_MAX_OTHER_CLKS)
+		return -EINVAL;
+
+	for (int i = 0; i < num_clks; i++)
+		priv->other_clks[i].id = clk_ids[i];
+
+	err = devm_clk_bulk_get_optional(dev, num_clks, priv->other_clks);
+	if (err) {
+		dev_err(dev, "failed to get clocks %d\n", err);
+		return err;
+	}
+
+	err = clk_bulk_prepare_enable(num_clks, priv->other_clks);
+	if (err)
+		dev_err(dev, "failed to enable clocks %d\n", err);
+
+	priv->num_other_clks = num_clks;
+
+	return err;
+}
+
 /*
  * If DMA addr spans 128MB boundary, we split the DMA transfer into two
  * so that each DMA transfer doesn't exceed the boundary.
@@ -1036,8 +1066,9 @@ static void dwcmshc_cqhci_init(struct sdhci_host *host, struct platform_device *
 
 static int dwcmshc_rk35xx_init(struct sdhci_host *host, struct dwcmshc_priv *dwc_priv)
 {
-	int err;
+	static const char * const clk_ids[] = {"axi", "block", "timer"};
 	struct rk35xx_priv *priv = dwc_priv->priv;
+	int err;
 
 	priv->reset = devm_reset_control_array_get_optional_exclusive(mmc_dev(host->mmc));
 	if (IS_ERR(priv->reset)) {
@@ -1046,21 +1077,10 @@ static int dwcmshc_rk35xx_init(struct sdhci_host *host, struct dwcmshc_priv *dwc
 		return err;
 	}
 
-	priv->rockchip_clks[0].id = "axi";
-	priv->rockchip_clks[1].id = "block";
-	priv->rockchip_clks[2].id = "timer";
-	err = devm_clk_bulk_get_optional(mmc_dev(host->mmc), RK35xx_MAX_CLKS,
-					 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);
-	if (err) {
-		dev_err(mmc_dev(host->mmc), "failed to enable clocks %d\n", err);
+	err = dwcmshc_get_enable_other_clks(mmc_dev(host->mmc), dwc_priv,
+					    ARRAY_SIZE(clk_ids), clk_ids);
+	if (err)
 		return err;
-	}
 
 	if (of_property_read_u8(mmc_dev(host->mmc)->of_node, "rockchip,txclk-tapnum",
 				&priv->txclk_tapnum))
@@ -1280,9 +1300,7 @@ 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);
+	clk_bulk_disable_unprepare(priv->num_other_clks, priv->other_clks);
 free_pltfm:
 	sdhci_pltfm_free(pdev);
 	return err;
@@ -1304,7 +1322,6 @@ 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;
 
 	pm_runtime_get_sync(&pdev->dev);
 	pm_runtime_disable(&pdev->dev);
@@ -1316,9 +1333,7 @@ static void dwcmshc_remove(struct platform_device *pdev)
 
 	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);
+	clk_bulk_disable_unprepare(priv->num_other_clks, priv->other_clks);
 	sdhci_pltfm_free(pdev);
 }
 
@@ -1328,7 +1343,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);
@@ -1347,9 +1361,7 @@ 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);
+	clk_bulk_disable_unprepare(priv->num_other_clks, priv->other_clks);
 
 	return ret;
 }
@@ -1359,7 +1371,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);
@@ -1372,29 +1383,24 @@ 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 (ret)
-			goto disable_bus_clk;
-	}
+	ret = clk_bulk_prepare_enable(priv->num_other_clks, priv->other_clks);
+	if (ret)
+		goto disable_bus_clk;
 
 	ret = sdhci_resume_host(host);
 	if (ret)
-		goto disable_rockchip_clks;
+		goto disable_other_clks;
 
 	if (host->mmc->caps2 & MMC_CAP2_CQE) {
 		ret = cqhci_resume(host->mmc);
 		if (ret)
-			goto disable_rockchip_clks;
+			goto disable_other_clks;
 	}
 
 	return 0;
 
-disable_rockchip_clks:
-	if (rk_priv)
-		clk_bulk_disable_unprepare(RK35xx_MAX_CLKS,
-					   rk_priv->rockchip_clks);
+disable_other_clks:
+	clk_bulk_disable_unprepare(priv->num_other_clks, priv->other_clks);
 disable_bus_clk:
 	if (!IS_ERR(priv->bus_clk))
 		clk_disable_unprepare(priv->bus_clk);
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH v6 2/8] mmc: sdhci-of-dwcmshc: move two rk35xx functions
  2024-08-05  9:15 [PATCH v6 0/8] mmc: sdhci-of-dwcmshc: Add Sophgo SG2042 support Chen Wang
  2024-08-05  9:17 ` [PATCH v6 1/8] mmc: sdhci-of-dwcmshc: add common bulk optional clocks support Chen Wang
@ 2024-08-05  9:17 ` Chen Wang
  2024-08-08  7:18   ` Adrian Hunter
  2024-08-05  9:17 ` [PATCH v6 3/8] mmc: sdhci-of-dwcmshc: factor out code for th1520_init() Chen Wang
                   ` (7 subsequent siblings)
  9 siblings, 1 reply; 22+ messages in thread
From: Chen Wang @ 2024-08-05  9:17 UTC (permalink / raw)
  To: adrian.hunter, aou, conor+dt, guoren, inochiama, jszhang,
	krzysztof.kozlowski+dt, palmer, paul.walmsley, robh, ulf.hansson,
	devicetree, linux-kernel, linux-mmc, linux-riscv, chao.wei,
	haijiao.liu, xiaoguang.xing, tingzhu.wang
  Cc: Chen Wang, Drew Fustini

From: Chen Wang <unicorn_wang@outlook.com>

This patch just move dwcmshc_rk35xx_init() and
dwcmshc_rk35xx_postinit() to put the functions
of rk35xx together as much as possible.

This change is an intermediate process before
further modification.

Signed-off-by: Chen Wang <unicorn_wang@outlook.com>
Tested-by: Drew Fustini <drew@pdp7.com> # TH1520
Tested-by: Inochi Amaoto <inochiama@outlook.com> # Duo and Huashan Pi
---
 drivers/mmc/host/sdhci-of-dwcmshc.c | 90 ++++++++++++++---------------
 1 file changed, 45 insertions(+), 45 deletions(-)

diff --git a/drivers/mmc/host/sdhci-of-dwcmshc.c b/drivers/mmc/host/sdhci-of-dwcmshc.c
index 35401616fb2e..a002636d51fd 100644
--- a/drivers/mmc/host/sdhci-of-dwcmshc.c
+++ b/drivers/mmc/host/sdhci-of-dwcmshc.c
@@ -711,6 +711,51 @@ static void rk35xx_sdhci_reset(struct sdhci_host *host, u8 mask)
 	sdhci_reset(host, mask);
 }
 
+static int dwcmshc_rk35xx_init(struct sdhci_host *host, struct dwcmshc_priv *dwc_priv)
+{
+	static const char * const clk_ids[] = {"axi", "block", "timer"};
+	struct rk35xx_priv *priv = dwc_priv->priv;
+	int err;
+
+	priv->reset = devm_reset_control_array_get_optional_exclusive(mmc_dev(host->mmc));
+	if (IS_ERR(priv->reset)) {
+		err = PTR_ERR(priv->reset);
+		dev_err(mmc_dev(host->mmc), "failed to get reset control %d\n", err);
+		return err;
+	}
+
+	err = dwcmshc_get_enable_other_clks(mmc_dev(host->mmc), dwc_priv,
+					    ARRAY_SIZE(clk_ids), clk_ids);
+	if (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;
+
+	/* Disable cmd conflict check */
+	sdhci_writel(host, 0x0, dwc_priv->vendor_specific_area1 + DWCMSHC_HOST_CTRL3);
+	/* Reset previous settings */
+	sdhci_writel(host, 0, DWCMSHC_EMMC_DLL_TXCLK);
+	sdhci_writel(host, 0, DWCMSHC_EMMC_DLL_STRBIN);
+
+	return 0;
+}
+
+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 th1520_execute_tuning(struct sdhci_host *host, u32 opcode)
 {
 	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
@@ -1064,51 +1109,6 @@ static void dwcmshc_cqhci_init(struct sdhci_host *host, struct platform_device *
 	host->mmc->caps2 &= ~(MMC_CAP2_CQE | MMC_CAP2_CQE_DCMD);
 }
 
-static int dwcmshc_rk35xx_init(struct sdhci_host *host, struct dwcmshc_priv *dwc_priv)
-{
-	static const char * const clk_ids[] = {"axi", "block", "timer"};
-	struct rk35xx_priv *priv = dwc_priv->priv;
-	int err;
-
-	priv->reset = devm_reset_control_array_get_optional_exclusive(mmc_dev(host->mmc));
-	if (IS_ERR(priv->reset)) {
-		err = PTR_ERR(priv->reset);
-		dev_err(mmc_dev(host->mmc), "failed to get reset control %d\n", err);
-		return err;
-	}
-
-	err = dwcmshc_get_enable_other_clks(mmc_dev(host->mmc), dwc_priv,
-					    ARRAY_SIZE(clk_ids), clk_ids);
-	if (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;
-
-	/* Disable cmd conflict check */
-	sdhci_writel(host, 0x0, dwc_priv->vendor_specific_area1 + DWCMSHC_HOST_CTRL3);
-	/* Reset previous settings */
-	sdhci_writel(host, 0, DWCMSHC_EMMC_DLL_TXCLK);
-	sdhci_writel(host, 0, DWCMSHC_EMMC_DLL_STRBIN);
-
-	return 0;
-}
-
-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 const struct of_device_id sdhci_dwcmshc_dt_ids[] = {
 	{
 		.compatible = "rockchip,rk3588-dwcmshc",
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH v6 3/8] mmc: sdhci-of-dwcmshc: factor out code for th1520_init()
  2024-08-05  9:15 [PATCH v6 0/8] mmc: sdhci-of-dwcmshc: Add Sophgo SG2042 support Chen Wang
  2024-08-05  9:17 ` [PATCH v6 1/8] mmc: sdhci-of-dwcmshc: add common bulk optional clocks support Chen Wang
  2024-08-05  9:17 ` [PATCH v6 2/8] mmc: sdhci-of-dwcmshc: move two rk35xx functions Chen Wang
@ 2024-08-05  9:17 ` Chen Wang
  2024-08-08  7:18   ` Adrian Hunter
  2024-08-05  9:18 ` [PATCH v6 4/8] mmc: sdhci-of-dwcmshc: factor out code into dwcmshc_rk35xx_init Chen Wang
                   ` (6 subsequent siblings)
  9 siblings, 1 reply; 22+ messages in thread
From: Chen Wang @ 2024-08-05  9:17 UTC (permalink / raw)
  To: adrian.hunter, aou, conor+dt, guoren, inochiama, jszhang,
	krzysztof.kozlowski+dt, palmer, paul.walmsley, robh, ulf.hansson,
	devicetree, linux-kernel, linux-mmc, linux-riscv, chao.wei,
	haijiao.liu, xiaoguang.xing, tingzhu.wang
  Cc: Chen Wang, Drew Fustini

From: Chen Wang <unicorn_wang@outlook.com>

Different socs have initialization operations in
the probe process, which are summarized as functions.

This patch first factor out init function for th1520.

Signed-off-by: Chen Wang <unicorn_wang@outlook.com>
Reviewed-by: Drew Fustini <drew@pdp7.com>
Tested-by: Drew Fustini <drew@pdp7.com> # TH1520
Tested-by: Inochi Amaoto <inochiama@outlook.com> # Duo and Huashan Pi
---
 drivers/mmc/host/sdhci-of-dwcmshc.c | 51 +++++++++++++++++------------
 1 file changed, 30 insertions(+), 21 deletions(-)

diff --git a/drivers/mmc/host/sdhci-of-dwcmshc.c b/drivers/mmc/host/sdhci-of-dwcmshc.c
index a002636d51fd..b272ec2ab232 100644
--- a/drivers/mmc/host/sdhci-of-dwcmshc.c
+++ b/drivers/mmc/host/sdhci-of-dwcmshc.c
@@ -830,6 +830,35 @@ static void th1520_sdhci_reset(struct sdhci_host *host, u8 mask)
 	}
 }
 
+static int 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;
+
+	/*
+	 * 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 (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 void cv18xx_sdhci_reset(struct sdhci_host *host, u8 mask)
 {
 	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
@@ -1231,27 +1260,7 @@ static int dwcmshc_probe(struct platform_device *pdev)
 	}
 
 	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);
+		th1520_init(dev, host, priv);
 	}
 
 #ifdef CONFIG_ACPI
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH v6 4/8] mmc: sdhci-of-dwcmshc: factor out code into dwcmshc_rk35xx_init
  2024-08-05  9:15 [PATCH v6 0/8] mmc: sdhci-of-dwcmshc: Add Sophgo SG2042 support Chen Wang
                   ` (2 preceding siblings ...)
  2024-08-05  9:17 ` [PATCH v6 3/8] mmc: sdhci-of-dwcmshc: factor out code for th1520_init() Chen Wang
@ 2024-08-05  9:18 ` Chen Wang
  2024-08-08  7:19   ` Adrian Hunter
  2024-08-05  9:18 ` [PATCH v6 5/8] mmc: sdhci-of-dwcmshc: add dwcmshc_pltfm_data Chen Wang
                   ` (5 subsequent siblings)
  9 siblings, 1 reply; 22+ messages in thread
From: Chen Wang @ 2024-08-05  9:18 UTC (permalink / raw)
  To: adrian.hunter, aou, conor+dt, guoren, inochiama, jszhang,
	krzysztof.kozlowski+dt, palmer, paul.walmsley, robh, ulf.hansson,
	devicetree, linux-kernel, linux-mmc, linux-riscv, chao.wei,
	haijiao.liu, xiaoguang.xing, tingzhu.wang
  Cc: Chen Wang, Drew Fustini

From: Chen Wang <unicorn_wang@outlook.com>

Continue factor out code fron probe into dwcmshc_rk35xx_init.

Signed-off-by: Chen Wang <unicorn_wang@outlook.com>
Tested-by: Drew Fustini <drew@pdp7.com> # TH1520
Tested-by: Inochi Amaoto <inochiama@outlook.com> # Duo and Huashan Pi
---
 drivers/mmc/host/sdhci-of-dwcmshc.c | 34 ++++++++++++++---------------
 1 file changed, 16 insertions(+), 18 deletions(-)

diff --git a/drivers/mmc/host/sdhci-of-dwcmshc.c b/drivers/mmc/host/sdhci-of-dwcmshc.c
index b272ec2ab232..55fba5ef37ba 100644
--- a/drivers/mmc/host/sdhci-of-dwcmshc.c
+++ b/drivers/mmc/host/sdhci-of-dwcmshc.c
@@ -711,12 +711,22 @@ static void rk35xx_sdhci_reset(struct sdhci_host *host, u8 mask)
 	sdhci_reset(host, mask);
 }
 
-static int dwcmshc_rk35xx_init(struct sdhci_host *host, struct dwcmshc_priv *dwc_priv)
+static int dwcmshc_rk35xx_init(struct device *dev, struct sdhci_host *host,
+			       struct dwcmshc_priv *dwc_priv)
 {
 	static const char * const clk_ids[] = {"axi", "block", "timer"};
-	struct rk35xx_priv *priv = dwc_priv->priv;
+	struct rk35xx_priv *priv;
 	int err;
 
+	priv = devm_kzalloc(dev, sizeof(struct rk35xx_priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	if (of_device_is_compatible(dev->of_node, "rockchip,rk3588-dwcmshc"))
+		priv->devtype = DWCMSHC_RK3588;
+	else
+		priv->devtype = DWCMSHC_RK3568;
+
 	priv->reset = devm_reset_control_array_get_optional_exclusive(mmc_dev(host->mmc));
 	if (IS_ERR(priv->reset)) {
 		err = PTR_ERR(priv->reset);
@@ -739,6 +749,8 @@ 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 = priv;
+
 	return 0;
 }
 
@@ -1184,7 +1196,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, caps;
@@ -1241,20 +1252,7 @@ static int dwcmshc_probe(struct platform_device *pdev)
 	host->mmc_host_ops.execute_tuning = dwcmshc_execute_tuning;
 
 	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;
 	}
@@ -1290,7 +1288,7 @@ static int dwcmshc_probe(struct platform_device *pdev)
 		dwcmshc_cqhci_init(host, pdev);
 	}
 
-	if (rk_priv)
+	if (pltfm_data == &sdhci_dwcmshc_rk35xx_pdata)
 		dwcmshc_rk35xx_postinit(host, priv);
 
 	err = __sdhci_add_host(host);
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH v6 5/8] mmc: sdhci-of-dwcmshc: add dwcmshc_pltfm_data
  2024-08-05  9:15 [PATCH v6 0/8] mmc: sdhci-of-dwcmshc: Add Sophgo SG2042 support Chen Wang
                   ` (3 preceding siblings ...)
  2024-08-05  9:18 ` [PATCH v6 4/8] mmc: sdhci-of-dwcmshc: factor out code into dwcmshc_rk35xx_init Chen Wang
@ 2024-08-05  9:18 ` Chen Wang
  2024-08-08  7:19   ` Adrian Hunter
  2024-08-05  9:19 ` [PATCH v6 6/8] dt-bindings: mmc: sdhci-of-dwcmhsc: Add Sophgo SG2042 support Chen Wang
                   ` (4 subsequent siblings)
  9 siblings, 1 reply; 22+ messages in thread
From: Chen Wang @ 2024-08-05  9:18 UTC (permalink / raw)
  To: adrian.hunter, aou, conor+dt, guoren, inochiama, jszhang,
	krzysztof.kozlowski+dt, palmer, paul.walmsley, robh, ulf.hansson,
	devicetree, linux-kernel, linux-mmc, linux-riscv, chao.wei,
	haijiao.liu, xiaoguang.xing, tingzhu.wang
  Cc: Chen Wang, Drew Fustini

From: Chen Wang <unicorn_wang@outlook.com>

Abstract dwcmshc_pltfm_data to hold the sdhci_pltfm_data
plus some comoon operations of soc such as init/postinit.

Signed-off-by: Chen Wang <unicorn_wang@outlook.com>
Tested-by: Drew Fustini <drew@pdp7.com> # TH1520
Tested-by: Inochi Amaoto <inochiama@outlook.com> # Duo and Huashan Pi
---
 drivers/mmc/host/sdhci-of-dwcmshc.c | 81 +++++++++++++++++------------
 1 file changed, 48 insertions(+), 33 deletions(-)

diff --git a/drivers/mmc/host/sdhci-of-dwcmshc.c b/drivers/mmc/host/sdhci-of-dwcmshc.c
index 55fba5ef37ba..16f420994519 100644
--- a/drivers/mmc/host/sdhci-of-dwcmshc.c
+++ b/drivers/mmc/host/sdhci-of-dwcmshc.c
@@ -218,6 +218,12 @@ struct dwcmshc_priv {
 	u16 flags;
 };
 
+struct dwcmshc_pltfm_data {
+	const struct sdhci_pltfm_data pdata;
+	int (*init)(struct device *dev, struct sdhci_host *host, struct dwcmshc_priv *dwc_priv);
+	void (*postinit)(struct sdhci_host *host, struct dwcmshc_priv *dwc_priv);
+};
+
 static int dwcmshc_get_enable_other_clks(struct device *dev,
 					 struct dwcmshc_priv *priv,
 					 int num_clks,
@@ -1048,39 +1054,52 @@ static const struct sdhci_ops sdhci_dwcmshc_cv18xx_ops = {
 	.platform_execute_tuning = cv18xx_sdhci_execute_tuning,
 };
 
-static const struct sdhci_pltfm_data sdhci_dwcmshc_pdata = {
-	.ops = &sdhci_dwcmshc_ops,
-	.quirks = SDHCI_QUIRK_CAP_CLOCK_BASE_BROKEN,
-	.quirks2 = SDHCI_QUIRK2_PRESET_VALUE_BROKEN,
+static const struct dwcmshc_pltfm_data sdhci_dwcmshc_pdata = {
+	.pdata = {
+		.ops = &sdhci_dwcmshc_ops,
+		.quirks = SDHCI_QUIRK_CAP_CLOCK_BASE_BROKEN,
+		.quirks2 = SDHCI_QUIRK2_PRESET_VALUE_BROKEN,
+	},
 };
 
 #ifdef CONFIG_ACPI
-static const struct sdhci_pltfm_data sdhci_dwcmshc_bf3_pdata = {
-	.ops = &sdhci_dwcmshc_ops,
-	.quirks = SDHCI_QUIRK_CAP_CLOCK_BASE_BROKEN,
-	.quirks2 = SDHCI_QUIRK2_PRESET_VALUE_BROKEN |
-		   SDHCI_QUIRK2_ACMD23_BROKEN,
+static const struct dwcmshc_pltfm_data sdhci_dwcmshc_bf3_pdata = {
+	.pdata = {
+		.ops = &sdhci_dwcmshc_ops,
+		.quirks = SDHCI_QUIRK_CAP_CLOCK_BASE_BROKEN,
+		.quirks2 = SDHCI_QUIRK2_PRESET_VALUE_BROKEN |
+			   SDHCI_QUIRK2_ACMD23_BROKEN,
+	},
 };
 #endif
 
-static const struct sdhci_pltfm_data sdhci_dwcmshc_rk35xx_pdata = {
-	.ops = &sdhci_dwcmshc_rk35xx_ops,
-	.quirks = SDHCI_QUIRK_CAP_CLOCK_BASE_BROKEN |
-		  SDHCI_QUIRK_BROKEN_TIMEOUT_VAL,
-	.quirks2 = SDHCI_QUIRK2_PRESET_VALUE_BROKEN |
-		   SDHCI_QUIRK2_CLOCK_DIV_ZERO_BROKEN,
+static const struct dwcmshc_pltfm_data sdhci_dwcmshc_rk35xx_pdata = {
+	.pdata = {
+		.ops = &sdhci_dwcmshc_rk35xx_ops,
+		.quirks = SDHCI_QUIRK_CAP_CLOCK_BASE_BROKEN |
+			  SDHCI_QUIRK_BROKEN_TIMEOUT_VAL,
+		.quirks2 = SDHCI_QUIRK2_PRESET_VALUE_BROKEN |
+			   SDHCI_QUIRK2_CLOCK_DIV_ZERO_BROKEN,
+	},
+	.init = dwcmshc_rk35xx_init,
+	.postinit = dwcmshc_rk35xx_postinit,
 };
 
-static const struct sdhci_pltfm_data sdhci_dwcmshc_th1520_pdata = {
-	.ops = &sdhci_dwcmshc_th1520_ops,
-	.quirks = SDHCI_QUIRK_CAP_CLOCK_BASE_BROKEN,
-	.quirks2 = SDHCI_QUIRK2_PRESET_VALUE_BROKEN,
+static const struct dwcmshc_pltfm_data sdhci_dwcmshc_th1520_pdata = {
+	.pdata = {
+		.ops = &sdhci_dwcmshc_th1520_ops,
+		.quirks = SDHCI_QUIRK_CAP_CLOCK_BASE_BROKEN,
+		.quirks2 = SDHCI_QUIRK2_PRESET_VALUE_BROKEN,
+	},
+	.init = th1520_init,
 };
 
-static const struct sdhci_pltfm_data sdhci_dwcmshc_cv18xx_pdata = {
-	.ops = &sdhci_dwcmshc_cv18xx_ops,
-	.quirks = SDHCI_QUIRK_CAP_CLOCK_BASE_BROKEN,
-	.quirks2 = SDHCI_QUIRK2_PRESET_VALUE_BROKEN,
+static const struct dwcmshc_pltfm_data sdhci_dwcmshc_cv18xx_pdata = {
+	.pdata = {
+		.ops = &sdhci_dwcmshc_cv18xx_ops,
+		.quirks = SDHCI_QUIRK_CAP_CLOCK_BASE_BROKEN,
+		.quirks2 = SDHCI_QUIRK2_PRESET_VALUE_BROKEN,
+	},
 };
 
 static const struct cqhci_host_ops dwcmshc_cqhci_ops = {
@@ -1196,7 +1215,7 @@ static int dwcmshc_probe(struct platform_device *pdev)
 	struct sdhci_pltfm_host *pltfm_host;
 	struct sdhci_host *host;
 	struct dwcmshc_priv *priv;
-	const struct sdhci_pltfm_data *pltfm_data;
+	const struct dwcmshc_pltfm_data *pltfm_data;
 	int err;
 	u32 extra, caps;
 
@@ -1206,7 +1225,7 @@ static int dwcmshc_probe(struct platform_device *pdev)
 		return -ENODEV;
 	}
 
-	host = sdhci_pltfm_init(pdev, pltfm_data,
+	host = sdhci_pltfm_init(pdev, &pltfm_data->pdata,
 				sizeof(struct dwcmshc_priv));
 	if (IS_ERR(host))
 		return PTR_ERR(host);
@@ -1251,16 +1270,12 @@ static int dwcmshc_probe(struct platform_device *pdev)
 	host->mmc_host_ops.hs400_enhanced_strobe = dwcmshc_hs400_enhanced_strobe;
 	host->mmc_host_ops.execute_tuning = dwcmshc_execute_tuning;
 
-	if (pltfm_data == &sdhci_dwcmshc_rk35xx_pdata) {
-		err = dwcmshc_rk35xx_init(dev, host, priv);
+	if (pltfm_data->init) {
+		err = pltfm_data->init(&pdev->dev, host, priv);
 		if (err)
 			goto err_clk;
 	}
 
-	if (pltfm_data == &sdhci_dwcmshc_th1520_pdata) {
-		th1520_init(dev, host, priv);
-	}
-
 #ifdef CONFIG_ACPI
 	if (pltfm_data == &sdhci_dwcmshc_bf3_pdata)
 		sdhci_enable_v4_mode(host);
@@ -1288,8 +1303,8 @@ static int dwcmshc_probe(struct platform_device *pdev)
 		dwcmshc_cqhci_init(host, pdev);
 	}
 
-	if (pltfm_data == &sdhci_dwcmshc_rk35xx_pdata)
-		dwcmshc_rk35xx_postinit(host, priv);
+	if (pltfm_data->postinit)
+		pltfm_data->postinit(host, priv);
 
 	err = __sdhci_add_host(host);
 	if (err)
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH v6 6/8] dt-bindings: mmc: sdhci-of-dwcmhsc: Add Sophgo SG2042 support
  2024-08-05  9:15 [PATCH v6 0/8] mmc: sdhci-of-dwcmshc: Add Sophgo SG2042 support Chen Wang
                   ` (4 preceding siblings ...)
  2024-08-05  9:18 ` [PATCH v6 5/8] mmc: sdhci-of-dwcmshc: add dwcmshc_pltfm_data Chen Wang
@ 2024-08-05  9:19 ` Chen Wang
  2024-08-05  9:19 ` [PATCH v6 7/8] mmc: sdhci-of-dwcmshc: Add support for Sophgo SG2042 Chen Wang
                   ` (3 subsequent siblings)
  9 siblings, 0 replies; 22+ messages in thread
From: Chen Wang @ 2024-08-05  9:19 UTC (permalink / raw)
  To: adrian.hunter, aou, conor+dt, guoren, inochiama, jszhang,
	krzysztof.kozlowski+dt, palmer, paul.walmsley, robh, ulf.hansson,
	devicetree, linux-kernel, linux-mmc, linux-riscv, chao.wei,
	haijiao.liu, xiaoguang.xing, tingzhu.wang
  Cc: Chen Wang, Conor Dooley

From: Chen Wang <unicorn_wang@outlook.com>

SG2042 use Synopsys dwcnshc IP for SD/eMMC controllers.

SG2042 defines 3 clocks for SD/eMMC controllers.
- EMMC_100M/SD_100M for cclk(Card clocks in DWC_mshc), so reuse
  existing "core".
- AXI_EMMC/AXI_SD for aclk/hclk(Bus interface clocks in DWC_mshc)
  and blck(Core Base Clock in DWC_mshc), these 3 clocks share one
  source, so reuse existing "bus".
- 100K_EMMC/100K_SD for cqetmclk(Timer clocks in DWC_mshc), so reuse
  existing "timer" which was added for rockchip specified.

Signed-off-by: Chen Wang <unicorn_wang@outlook.com>
Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
---
 .../bindings/mmc/snps,dwcmshc-sdhci.yaml      | 60 +++++++++++++------
 1 file changed, 43 insertions(+), 17 deletions(-)

diff --git a/Documentation/devicetree/bindings/mmc/snps,dwcmshc-sdhci.yaml b/Documentation/devicetree/bindings/mmc/snps,dwcmshc-sdhci.yaml
index 4d3031d9965f..80d50178d2e3 100644
--- a/Documentation/devicetree/bindings/mmc/snps,dwcmshc-sdhci.yaml
+++ b/Documentation/devicetree/bindings/mmc/snps,dwcmshc-sdhci.yaml
@@ -10,9 +10,6 @@ maintainers:
   - Ulf Hansson <ulf.hansson@linaro.org>
   - Jisheng Zhang <Jisheng.Zhang@synaptics.com>
 
-allOf:
-  - $ref: mmc-controller.yaml#
-
 properties:
   compatible:
     enum:
@@ -21,6 +18,7 @@ properties:
       - snps,dwcmshc-sdhci
       - sophgo,cv1800b-dwcmshc
       - sophgo,sg2002-dwcmshc
+      - sophgo,sg2042-dwcmshc
       - thead,th1520-dwcmshc
 
   reg:
@@ -31,22 +29,11 @@ properties:
 
   clocks:
     minItems: 1
-    items:
-      - description: core clock
-      - description: bus clock for optional
-      - description: axi clock for rockchip specified
-      - description: block clock for rockchip specified
-      - description: timer clock for rockchip specified
-
+    maxItems: 5
 
   clock-names:
     minItems: 1
-    items:
-      - const: core
-      - const: bus
-      - const: axi
-      - const: block
-      - const: timer
+    maxItems: 5
 
   resets:
     maxItems: 5
@@ -63,7 +50,6 @@ properties:
     description: Specify the number of delay for tx sampling.
     $ref: /schemas/types.yaml#/definitions/uint8
 
-
 required:
   - compatible
   - reg
@@ -71,6 +57,46 @@ required:
   - clocks
   - clock-names
 
+allOf:
+  - $ref: mmc-controller.yaml#
+
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: sophgo,sg2042-dwcmshc
+
+    then:
+      properties:
+        clocks:
+          items:
+            - description: core clock
+            - description: bus clock
+            - description: timer clock
+        clock-names:
+          items:
+            - const: core
+            - const: bus
+            - const: timer
+    else:
+      properties:
+        clocks:
+          minItems: 1
+          items:
+            - description: core clock
+            - description: bus clock for optional
+            - description: axi clock for rockchip specified
+            - description: block clock for rockchip specified
+            - description: timer clock for rockchip specified
+        clock-names:
+          minItems: 1
+          items:
+            - const: core
+            - const: bus
+            - const: axi
+            - const: block
+            - const: timer
+
 unevaluatedProperties: false
 
 examples:
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH v6 7/8] mmc: sdhci-of-dwcmshc: Add support for Sophgo SG2042
  2024-08-05  9:15 [PATCH v6 0/8] mmc: sdhci-of-dwcmshc: Add Sophgo SG2042 support Chen Wang
                   ` (5 preceding siblings ...)
  2024-08-05  9:19 ` [PATCH v6 6/8] dt-bindings: mmc: sdhci-of-dwcmhsc: Add Sophgo SG2042 support Chen Wang
@ 2024-08-05  9:19 ` Chen Wang
  2024-08-08  7:19   ` Adrian Hunter
  2024-08-05  9:19 ` [PATCH v6 8/8] riscv: sophgo: dts: add mmc controllers for SG2042 SoC Chen Wang
                   ` (2 subsequent siblings)
  9 siblings, 1 reply; 22+ messages in thread
From: Chen Wang @ 2024-08-05  9:19 UTC (permalink / raw)
  To: adrian.hunter, aou, conor+dt, guoren, inochiama, jszhang,
	krzysztof.kozlowski+dt, palmer, paul.walmsley, robh, ulf.hansson,
	devicetree, linux-kernel, linux-mmc, linux-riscv, chao.wei,
	haijiao.liu, xiaoguang.xing, tingzhu.wang
  Cc: Chen Wang

From: Chen Wang <unicorn_wang@outlook.com>

Add support for the mmc controller of Sophgo SG2042.

SG2042 uses Synopsys PHY the same as TH1520 so we reuse the tuning
logic from TH1520. Besides this, this patch implement some SG2042
specific work, such as clocks and reset ops.

Signed-off-by: Chen Wang <unicorn_wang@outlook.com>
---
 drivers/mmc/host/sdhci-of-dwcmshc.c | 125 ++++++++++++++++++++++++++--
 1 file changed, 118 insertions(+), 7 deletions(-)

diff --git a/drivers/mmc/host/sdhci-of-dwcmshc.c b/drivers/mmc/host/sdhci-of-dwcmshc.c
index 16f420994519..ba8960d8b2d4 100644
--- a/drivers/mmc/host/sdhci-of-dwcmshc.c
+++ b/drivers/mmc/host/sdhci-of-dwcmshc.c
@@ -113,12 +113,15 @@
 #define DWC_MSHC_PTR_PHY_R	0x300
 
 /* PHY general configuration */
-#define PHY_CNFG_R		(DWC_MSHC_PTR_PHY_R + 0x00)
-#define PHY_CNFG_RSTN_DEASSERT	0x1  /* Deassert PHY reset */
-#define PHY_CNFG_PAD_SP_MASK	GENMASK(19, 16) /* bits [19:16] */
-#define PHY_CNFG_PAD_SP		0x0c /* PMOS TX drive strength */
-#define PHY_CNFG_PAD_SN_MASK	GENMASK(23, 20) /* bits [23:20] */
-#define PHY_CNFG_PAD_SN		0x0c /* NMOS TX drive strength */
+#define PHY_CNFG_R			(DWC_MSHC_PTR_PHY_R + 0x00)
+#define PHY_CNFG_RSTN_DEASSERT		0x1  /* Deassert PHY reset */
+#define PHY_CNFG_PHY_PWRGOOD_MASK	BIT_MASK(1) /* bit [1] */
+#define PHY_CNFG_PAD_SP_MASK		GENMASK(19, 16) /* bits [19:16] */
+#define PHY_CNFG_PAD_SP			0x0c /* PMOS TX drive strength */
+#define PHY_CNFG_PAD_SP_SG2042		0x09 /* PMOS TX drive strength for SG2042 */
+#define PHY_CNFG_PAD_SN_MASK		GENMASK(23, 20) /* bits [23:20] */
+#define PHY_CNFG_PAD_SN			0x0c /* NMOS TX drive strength */
+#define PHY_CNFG_PAD_SN_SG2042		0x08 /* NMOS TX drive strength for SG2042 */
 
 /* PHY command/response pad settings */
 #define PHY_CMDPAD_CNFG_R	(DWC_MSHC_PTR_PHY_R + 0x04)
@@ -147,10 +150,12 @@
 #define PHY_PAD_TXSLEW_CTRL_P		0x3 /* Slew control for P-Type pad TX */
 #define PHY_PAD_TXSLEW_CTRL_N_MASK	GENMASK(12, 9) /* bits [12:9] */
 #define PHY_PAD_TXSLEW_CTRL_N		0x3 /* Slew control for N-Type pad TX */
+#define PHY_PAD_TXSLEW_CTRL_N_SG2042	0x2 /* Slew control for N-Type pad TX for SG2042 */
 
 /* PHY CLK delay line settings */
 #define PHY_SDCLKDL_CNFG_R		(DWC_MSHC_PTR_PHY_R + 0x1d)
-#define PHY_SDCLKDL_CNFG_UPDATE	BIT(4) /* set before writing to SDCLKDL_DC */
+#define PHY_SDCLKDL_CNFG_EXTDLY_EN	BIT(0)
+#define PHY_SDCLKDL_CNFG_UPDATE		BIT(4) /* set before writing to SDCLKDL_DC */
 
 /* PHY CLK delay line delay code */
 #define PHY_SDCLKDL_DC_R		(DWC_MSHC_PTR_PHY_R + 0x1e)
@@ -158,10 +163,14 @@
 #define PHY_SDCLKDL_DC_DEFAULT		0x32 /* default delay code */
 #define PHY_SDCLKDL_DC_HS400		0x18 /* delay code for HS400 mode */
 
+#define PHY_SMPLDL_CNFG_R		(DWC_MSHC_PTR_PHY_R + 0x20)
+#define PHY_SMPLDL_CNFG_BYPASS_EN	BIT(1)
+
 /* PHY drift_cclk_rx delay line configuration setting */
 #define PHY_ATDL_CNFG_R			(DWC_MSHC_PTR_PHY_R + 0x21)
 #define PHY_ATDL_CNFG_INPSEL_MASK	GENMASK(3, 2) /* bits [3:2] */
 #define PHY_ATDL_CNFG_INPSEL		0x3 /* delay line input source */
+#define PHY_ATDL_CNFG_INPSEL_SG2042	0x2 /* delay line input source for SG2042 */
 
 /* PHY DLL control settings */
 #define PHY_DLL_CTRL_R			(DWC_MSHC_PTR_PHY_R + 0x24)
@@ -1013,6 +1022,85 @@ static int cv18xx_sdhci_execute_tuning(struct sdhci_host *host, u32 opcode)
 	return ret;
 }
 
+static inline void sg2042_sdhci_phy_init(struct sdhci_host *host)
+{
+	u32 val;
+
+	/* Asset phy reset & set tx drive strength */
+	val = sdhci_readl(host, PHY_CNFG_R);
+	val &= ~PHY_CNFG_RSTN_DEASSERT;
+	val |= FIELD_PREP(PHY_CNFG_PHY_PWRGOOD_MASK, 1);
+	val |= FIELD_PREP(PHY_CNFG_PAD_SP_MASK, PHY_CNFG_PAD_SP_SG2042);
+	val |= FIELD_PREP(PHY_CNFG_PAD_SN_MASK, PHY_CNFG_PAD_SN_SG2042);
+	sdhci_writel(host, val, PHY_CNFG_R);
+
+	/* Configure phy pads */
+	val = PHY_PAD_RXSEL_3V3;
+	val |= FIELD_PREP(PHY_PAD_WEAKPULL_MASK, PHY_PAD_WEAKPULL_PULLUP);
+	val |= FIELD_PREP(PHY_PAD_TXSLEW_CTRL_P_MASK, PHY_PAD_TXSLEW_CTRL_P);
+	val |= FIELD_PREP(PHY_PAD_TXSLEW_CTRL_N_MASK, PHY_PAD_TXSLEW_CTRL_N_SG2042);
+	sdhci_writew(host, val, PHY_CMDPAD_CNFG_R);
+	sdhci_writew(host, val, PHY_DATAPAD_CNFG_R);
+	sdhci_writew(host, val, PHY_RSTNPAD_CNFG_R);
+
+	val = PHY_PAD_RXSEL_3V3;
+	val |= FIELD_PREP(PHY_PAD_TXSLEW_CTRL_P_MASK, PHY_PAD_TXSLEW_CTRL_P);
+	val |= FIELD_PREP(PHY_PAD_TXSLEW_CTRL_N_MASK, PHY_PAD_TXSLEW_CTRL_N_SG2042);
+	sdhci_writew(host, val, PHY_CLKPAD_CNFG_R);
+
+	val = PHY_PAD_RXSEL_3V3;
+	val |= FIELD_PREP(PHY_PAD_WEAKPULL_MASK, PHY_PAD_WEAKPULL_PULLDOWN);
+	val |= FIELD_PREP(PHY_PAD_TXSLEW_CTRL_P_MASK, PHY_PAD_TXSLEW_CTRL_P);
+	val |= FIELD_PREP(PHY_PAD_TXSLEW_CTRL_N_MASK, PHY_PAD_TXSLEW_CTRL_N_SG2042);
+	sdhci_writew(host, val, PHY_STBPAD_CNFG_R);
+
+	/* Configure delay line */
+	/* Enable fixed delay */
+	sdhci_writeb(host, PHY_SDCLKDL_CNFG_EXTDLY_EN, PHY_SDCLKDL_CNFG_R);
+	/*
+	 * Set delay line.
+	 * Its recommended that bit UPDATE_DC[4] is 1 when SDCLKDL_DC is being written.
+	 * Ensure UPDATE_DC[4] is '0' when not updating code.
+	 */
+	val = sdhci_readb(host, PHY_SDCLKDL_CNFG_R);
+	val |= PHY_SDCLKDL_CNFG_UPDATE;
+	sdhci_writeb(host, val, PHY_SDCLKDL_CNFG_R);
+	/* Add 10 * 70ps = 0.7ns for output delay */
+	sdhci_writeb(host, 10, PHY_SDCLKDL_DC_R);
+	val = sdhci_readb(host, PHY_SDCLKDL_CNFG_R);
+	val &= ~(PHY_SDCLKDL_CNFG_UPDATE);
+	sdhci_writeb(host, val, PHY_SDCLKDL_CNFG_R);
+
+	/* Set SMPLDL_CNFG, Bypass */
+	sdhci_writeb(host, PHY_SMPLDL_CNFG_BYPASS_EN, PHY_SMPLDL_CNFG_R);
+
+	/* Set ATDL_CNFG, tuning clk not use for init */
+	val = FIELD_PREP(PHY_ATDL_CNFG_INPSEL_MASK, PHY_ATDL_CNFG_INPSEL_SG2042);
+	sdhci_writeb(host, val, PHY_ATDL_CNFG_R);
+
+	/* Deasset phy reset */
+	val = sdhci_readl(host, PHY_CNFG_R);
+	val |= PHY_CNFG_RSTN_DEASSERT;
+	sdhci_writel(host, val, PHY_CNFG_R);
+}
+
+static void sg2042_sdhci_reset(struct sdhci_host *host, u8 mask)
+{
+	sdhci_reset(host, mask);
+
+	if (mask & SDHCI_RESET_ALL)
+		sg2042_sdhci_phy_init(host);
+}
+
+static int sg2042_init(struct device *dev, struct sdhci_host *host,
+		       struct dwcmshc_priv *dwc_priv)
+{
+	static const char * const clk_ids[] = {"timer"};
+
+	return dwcmshc_get_enable_other_clks(mmc_dev(host->mmc), dwc_priv,
+					     ARRAY_SIZE(clk_ids), clk_ids);
+}
+
 static const struct sdhci_ops sdhci_dwcmshc_ops = {
 	.set_clock		= sdhci_set_clock,
 	.set_bus_width		= sdhci_set_bus_width,
@@ -1054,6 +1142,16 @@ static const struct sdhci_ops sdhci_dwcmshc_cv18xx_ops = {
 	.platform_execute_tuning = cv18xx_sdhci_execute_tuning,
 };
 
+static const struct sdhci_ops sdhci_dwcmshc_sg2042_ops = {
+	.set_clock		= sdhci_set_clock,
+	.set_bus_width		= sdhci_set_bus_width,
+	.set_uhs_signaling	= dwcmshc_set_uhs_signaling,
+	.get_max_clock		= dwcmshc_get_max_clock,
+	.reset			= sg2042_sdhci_reset,
+	.adma_write_desc	= dwcmshc_adma_write_desc,
+	.platform_execute_tuning = th1520_execute_tuning,
+};
+
 static const struct dwcmshc_pltfm_data sdhci_dwcmshc_pdata = {
 	.pdata = {
 		.ops = &sdhci_dwcmshc_ops,
@@ -1102,6 +1200,15 @@ static const struct dwcmshc_pltfm_data sdhci_dwcmshc_cv18xx_pdata = {
 	},
 };
 
+static const struct dwcmshc_pltfm_data sdhci_dwcmshc_sg2042_pdata = {
+	.pdata = {
+		.ops = &sdhci_dwcmshc_sg2042_ops,
+		.quirks = SDHCI_QUIRK_CAP_CLOCK_BASE_BROKEN,
+		.quirks2 = SDHCI_QUIRK2_PRESET_VALUE_BROKEN,
+	},
+	.init = sg2042_init,
+};
+
 static const struct cqhci_host_ops dwcmshc_cqhci_ops = {
 	.enable		= dwcmshc_sdhci_cqe_enable,
 	.disable	= sdhci_cqe_disable,
@@ -1194,6 +1301,10 @@ static const struct of_device_id sdhci_dwcmshc_dt_ids[] = {
 		.compatible = "thead,th1520-dwcmshc",
 		.data = &sdhci_dwcmshc_th1520_pdata,
 	},
+	{
+		.compatible = "sophgo,sg2042-dwcmshc",
+		.data = &sdhci_dwcmshc_sg2042_pdata,
+	},
 	{},
 };
 MODULE_DEVICE_TABLE(of, sdhci_dwcmshc_dt_ids);
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH v6 8/8] riscv: sophgo: dts: add mmc controllers for SG2042 SoC
  2024-08-05  9:15 [PATCH v6 0/8] mmc: sdhci-of-dwcmshc: Add Sophgo SG2042 support Chen Wang
                   ` (6 preceding siblings ...)
  2024-08-05  9:19 ` [PATCH v6 7/8] mmc: sdhci-of-dwcmshc: Add support for Sophgo SG2042 Chen Wang
@ 2024-08-05  9:19 ` Chen Wang
  2024-08-20 11:49 ` [PATCH v6 0/8] mmc: sdhci-of-dwcmshc: Add Sophgo SG2042 support Ulf Hansson
  2024-08-26  2:59 ` (subset) " Inochi Amaoto
  9 siblings, 0 replies; 22+ messages in thread
From: Chen Wang @ 2024-08-05  9:19 UTC (permalink / raw)
  To: adrian.hunter, aou, conor+dt, guoren, inochiama, jszhang,
	krzysztof.kozlowski+dt, palmer, paul.walmsley, robh, ulf.hansson,
	devicetree, linux-kernel, linux-mmc, linux-riscv, chao.wei,
	haijiao.liu, xiaoguang.xing, tingzhu.wang
  Cc: Chen Wang

From: Chen Wang <unicorn_wang@outlook.com>

SG2042 has two MMC controller, one for emmc, another for sd-card.

Signed-off-by: Chen Wang <unicorn_wang@outlook.com>
---
 .../boot/dts/sophgo/sg2042-milkv-pioneer.dts  | 17 +++++++++++
 arch/riscv/boot/dts/sophgo/sg2042.dtsi        | 28 +++++++++++++++++++
 2 files changed, 45 insertions(+)

diff --git a/arch/riscv/boot/dts/sophgo/sg2042-milkv-pioneer.dts b/arch/riscv/boot/dts/sophgo/sg2042-milkv-pioneer.dts
index 80cb017974d8..da6596e9192e 100644
--- a/arch/riscv/boot/dts/sophgo/sg2042-milkv-pioneer.dts
+++ b/arch/riscv/boot/dts/sophgo/sg2042-milkv-pioneer.dts
@@ -26,6 +26,23 @@ &cgi_dpll1 {
 	clock-frequency = <25000000>;
 };
 
+&emmc {
+	bus-width = <4>;
+	no-sdio;
+	no-sd;
+	non-removable;
+	wp-inverted;
+	status = "okay";
+};
+
+&sd {
+	bus-width = <4>;
+	no-sdio;
+	no-mmc;
+	wp-inverted;
+	status = "okay";
+};
+
 &uart0 {
 	status = "okay";
 };
diff --git a/arch/riscv/boot/dts/sophgo/sg2042.dtsi b/arch/riscv/boot/dts/sophgo/sg2042.dtsi
index 34c802bd3f9b..f0ccefecc9c3 100644
--- a/arch/riscv/boot/dts/sophgo/sg2042.dtsi
+++ b/arch/riscv/boot/dts/sophgo/sg2042.dtsi
@@ -399,5 +399,33 @@ uart0: serial@7040000000 {
 			resets = <&rstgen RST_UART0>;
 			status = "disabled";
 		};
+
+		emmc: mmc@704002a000 {
+			compatible = "sophgo,sg2042-dwcmshc";
+			reg = <0x70 0x4002a000 0x0 0x1000>;
+			interrupt-parent = <&intc>;
+			interrupts = <134 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&clkgen GATE_CLK_EMMC_100M>,
+				 <&clkgen GATE_CLK_AXI_EMMC>,
+				 <&clkgen GATE_CLK_100K_EMMC>;
+			clock-names = "core",
+				      "bus",
+				      "timer";
+			status = "disabled";
+		};
+
+		sd: mmc@704002b000 {
+			compatible = "sophgo,sg2042-dwcmshc";
+			reg = <0x70 0x4002b000 0x0 0x1000>;
+			interrupt-parent = <&intc>;
+			interrupts = <136 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&clkgen GATE_CLK_SD_100M>,
+				 <&clkgen GATE_CLK_AXI_SD>,
+				 <&clkgen GATE_CLK_100K_SD>;
+			clock-names = "core",
+				      "bus",
+				      "timer";
+			status = "disabled";
+		};
 	};
 };
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* Re: [PATCH v6 1/8] mmc: sdhci-of-dwcmshc: add common bulk optional clocks support
  2024-08-05  9:17 ` [PATCH v6 1/8] mmc: sdhci-of-dwcmshc: add common bulk optional clocks support Chen Wang
@ 2024-08-08  7:17   ` Adrian Hunter
  2025-07-22 18:33   ` Robin Murphy
  1 sibling, 0 replies; 22+ messages in thread
From: Adrian Hunter @ 2024-08-08  7:17 UTC (permalink / raw)
  To: Chen Wang, aou, conor+dt, guoren, inochiama, jszhang,
	krzysztof.kozlowski+dt, palmer, paul.walmsley, robh, ulf.hansson,
	devicetree, linux-kernel, linux-mmc, linux-riscv, chao.wei,
	haijiao.liu, xiaoguang.xing, tingzhu.wang
  Cc: Chen Wang, Drew Fustini

On 5/08/24 12:17, Chen Wang wrote:
> From: Chen Wang <unicorn_wang@outlook.com>
> 
> In addition to the required core clock and optional
> bus clock, the soc will expand its own clocks, so
> the bulk clock mechanism is abstracted.
> 
> Note, I call the bulk clocks as "other clocks" due
> to the bus clock has been called as "optional".
> 
> Signed-off-by: Chen Wang <unicorn_wang@outlook.com>
> Tested-by: Drew Fustini <drew@pdp7.com> # TH1520
> Tested-by: Inochi Amaoto <inochiama@outlook.com> # Duo and Huashan Pi

Acked-by: Adrian Hunter <adrian.hunter@intel.com>

> ---
>  drivers/mmc/host/sdhci-of-dwcmshc.c | 90 +++++++++++++++--------------
>  1 file changed, 48 insertions(+), 42 deletions(-)
> 
> diff --git a/drivers/mmc/host/sdhci-of-dwcmshc.c b/drivers/mmc/host/sdhci-of-dwcmshc.c
> index e79aa4b3b6c3..35401616fb2e 100644
> --- a/drivers/mmc/host/sdhci-of-dwcmshc.c
> +++ b/drivers/mmc/host/sdhci-of-dwcmshc.c
> @@ -108,7 +108,6 @@
>  #define DLL_LOCK_WO_TMOUT(x) \
>  	((((x) & DWCMSHC_EMMC_DLL_LOCKED) == DWCMSHC_EMMC_DLL_LOCKED) && \
>  	(((x) & DWCMSHC_EMMC_DLL_TIMEOUT) == 0))
> -#define RK35xx_MAX_CLKS 3
>  
>  /* PHY register area pointer */
>  #define DWC_MSHC_PTR_PHY_R	0x300
> @@ -199,23 +198,54 @@ enum dwcmshc_rk_type {
>  };
>  
>  struct rk35xx_priv {
> -	/* Rockchip specified optional clocks */
> -	struct clk_bulk_data rockchip_clks[RK35xx_MAX_CLKS];
>  	struct reset_control *reset;
>  	enum dwcmshc_rk_type devtype;
>  	u8 txclk_tapnum;
>  };
>  
> +#define DWCMSHC_MAX_OTHER_CLKS 3
> +
>  struct dwcmshc_priv {
>  	struct clk	*bus_clk;
>  	int vendor_specific_area1; /* P_VENDOR_SPECIFIC_AREA1 reg */
>  	int vendor_specific_area2; /* P_VENDOR_SPECIFIC_AREA2 reg */
>  
> +	int num_other_clks;
> +	struct clk_bulk_data other_clks[DWCMSHC_MAX_OTHER_CLKS];
> +
>  	void *priv; /* pointer to SoC private stuff */
>  	u16 delay_line;
>  	u16 flags;
>  };
>  
> +static int dwcmshc_get_enable_other_clks(struct device *dev,
> +					 struct dwcmshc_priv *priv,
> +					 int num_clks,
> +					 const char * const clk_ids[])
> +{
> +	int err;
> +
> +	if (num_clks > DWCMSHC_MAX_OTHER_CLKS)
> +		return -EINVAL;
> +
> +	for (int i = 0; i < num_clks; i++)
> +		priv->other_clks[i].id = clk_ids[i];
> +
> +	err = devm_clk_bulk_get_optional(dev, num_clks, priv->other_clks);
> +	if (err) {
> +		dev_err(dev, "failed to get clocks %d\n", err);
> +		return err;
> +	}
> +
> +	err = clk_bulk_prepare_enable(num_clks, priv->other_clks);
> +	if (err)
> +		dev_err(dev, "failed to enable clocks %d\n", err);
> +
> +	priv->num_other_clks = num_clks;
> +
> +	return err;
> +}
> +
>  /*
>   * If DMA addr spans 128MB boundary, we split the DMA transfer into two
>   * so that each DMA transfer doesn't exceed the boundary.
> @@ -1036,8 +1066,9 @@ static void dwcmshc_cqhci_init(struct sdhci_host *host, struct platform_device *
>  
>  static int dwcmshc_rk35xx_init(struct sdhci_host *host, struct dwcmshc_priv *dwc_priv)
>  {
> -	int err;
> +	static const char * const clk_ids[] = {"axi", "block", "timer"};
>  	struct rk35xx_priv *priv = dwc_priv->priv;
> +	int err;
>  
>  	priv->reset = devm_reset_control_array_get_optional_exclusive(mmc_dev(host->mmc));
>  	if (IS_ERR(priv->reset)) {
> @@ -1046,21 +1077,10 @@ static int dwcmshc_rk35xx_init(struct sdhci_host *host, struct dwcmshc_priv *dwc
>  		return err;
>  	}
>  
> -	priv->rockchip_clks[0].id = "axi";
> -	priv->rockchip_clks[1].id = "block";
> -	priv->rockchip_clks[2].id = "timer";
> -	err = devm_clk_bulk_get_optional(mmc_dev(host->mmc), RK35xx_MAX_CLKS,
> -					 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);
> -	if (err) {
> -		dev_err(mmc_dev(host->mmc), "failed to enable clocks %d\n", err);
> +	err = dwcmshc_get_enable_other_clks(mmc_dev(host->mmc), dwc_priv,
> +					    ARRAY_SIZE(clk_ids), clk_ids);
> +	if (err)
>  		return err;
> -	}
>  
>  	if (of_property_read_u8(mmc_dev(host->mmc)->of_node, "rockchip,txclk-tapnum",
>  				&priv->txclk_tapnum))
> @@ -1280,9 +1300,7 @@ 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);
> +	clk_bulk_disable_unprepare(priv->num_other_clks, priv->other_clks);
>  free_pltfm:
>  	sdhci_pltfm_free(pdev);
>  	return err;
> @@ -1304,7 +1322,6 @@ 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;
>  
>  	pm_runtime_get_sync(&pdev->dev);
>  	pm_runtime_disable(&pdev->dev);
> @@ -1316,9 +1333,7 @@ static void dwcmshc_remove(struct platform_device *pdev)
>  
>  	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);
> +	clk_bulk_disable_unprepare(priv->num_other_clks, priv->other_clks);
>  	sdhci_pltfm_free(pdev);
>  }
>  
> @@ -1328,7 +1343,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);
> @@ -1347,9 +1361,7 @@ 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);
> +	clk_bulk_disable_unprepare(priv->num_other_clks, priv->other_clks);
>  
>  	return ret;
>  }
> @@ -1359,7 +1371,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);
> @@ -1372,29 +1383,24 @@ 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 (ret)
> -			goto disable_bus_clk;
> -	}
> +	ret = clk_bulk_prepare_enable(priv->num_other_clks, priv->other_clks);
> +	if (ret)
> +		goto disable_bus_clk;
>  
>  	ret = sdhci_resume_host(host);
>  	if (ret)
> -		goto disable_rockchip_clks;
> +		goto disable_other_clks;
>  
>  	if (host->mmc->caps2 & MMC_CAP2_CQE) {
>  		ret = cqhci_resume(host->mmc);
>  		if (ret)
> -			goto disable_rockchip_clks;
> +			goto disable_other_clks;
>  	}
>  
>  	return 0;
>  
> -disable_rockchip_clks:
> -	if (rk_priv)
> -		clk_bulk_disable_unprepare(RK35xx_MAX_CLKS,
> -					   rk_priv->rockchip_clks);
> +disable_other_clks:
> +	clk_bulk_disable_unprepare(priv->num_other_clks, priv->other_clks);
>  disable_bus_clk:
>  	if (!IS_ERR(priv->bus_clk))
>  		clk_disable_unprepare(priv->bus_clk);


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v6 2/8] mmc: sdhci-of-dwcmshc: move two rk35xx functions
  2024-08-05  9:17 ` [PATCH v6 2/8] mmc: sdhci-of-dwcmshc: move two rk35xx functions Chen Wang
@ 2024-08-08  7:18   ` Adrian Hunter
  0 siblings, 0 replies; 22+ messages in thread
From: Adrian Hunter @ 2024-08-08  7:18 UTC (permalink / raw)
  To: Chen Wang, aou, conor+dt, guoren, inochiama, jszhang,
	krzysztof.kozlowski+dt, palmer, paul.walmsley, robh, ulf.hansson,
	devicetree, linux-kernel, linux-mmc, linux-riscv, chao.wei,
	haijiao.liu, xiaoguang.xing, tingzhu.wang
  Cc: Chen Wang, Drew Fustini

On 5/08/24 12:17, Chen Wang wrote:
> From: Chen Wang <unicorn_wang@outlook.com>
> 
> This patch just move dwcmshc_rk35xx_init() and
> dwcmshc_rk35xx_postinit() to put the functions
> of rk35xx together as much as possible.
> 
> This change is an intermediate process before
> further modification.
> 
> Signed-off-by: Chen Wang <unicorn_wang@outlook.com>
> Tested-by: Drew Fustini <drew@pdp7.com> # TH1520
> Tested-by: Inochi Amaoto <inochiama@outlook.com> # Duo and Huashan Pi

Acked-by: Adrian Hunter <adrian.hunter@intel.com>

> ---
>  drivers/mmc/host/sdhci-of-dwcmshc.c | 90 ++++++++++++++---------------
>  1 file changed, 45 insertions(+), 45 deletions(-)
> 
> diff --git a/drivers/mmc/host/sdhci-of-dwcmshc.c b/drivers/mmc/host/sdhci-of-dwcmshc.c
> index 35401616fb2e..a002636d51fd 100644
> --- a/drivers/mmc/host/sdhci-of-dwcmshc.c
> +++ b/drivers/mmc/host/sdhci-of-dwcmshc.c
> @@ -711,6 +711,51 @@ static void rk35xx_sdhci_reset(struct sdhci_host *host, u8 mask)
>  	sdhci_reset(host, mask);
>  }
>  
> +static int dwcmshc_rk35xx_init(struct sdhci_host *host, struct dwcmshc_priv *dwc_priv)
> +{
> +	static const char * const clk_ids[] = {"axi", "block", "timer"};
> +	struct rk35xx_priv *priv = dwc_priv->priv;
> +	int err;
> +
> +	priv->reset = devm_reset_control_array_get_optional_exclusive(mmc_dev(host->mmc));
> +	if (IS_ERR(priv->reset)) {
> +		err = PTR_ERR(priv->reset);
> +		dev_err(mmc_dev(host->mmc), "failed to get reset control %d\n", err);
> +		return err;
> +	}
> +
> +	err = dwcmshc_get_enable_other_clks(mmc_dev(host->mmc), dwc_priv,
> +					    ARRAY_SIZE(clk_ids), clk_ids);
> +	if (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;
> +
> +	/* Disable cmd conflict check */
> +	sdhci_writel(host, 0x0, dwc_priv->vendor_specific_area1 + DWCMSHC_HOST_CTRL3);
> +	/* Reset previous settings */
> +	sdhci_writel(host, 0, DWCMSHC_EMMC_DLL_TXCLK);
> +	sdhci_writel(host, 0, DWCMSHC_EMMC_DLL_STRBIN);
> +
> +	return 0;
> +}
> +
> +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 th1520_execute_tuning(struct sdhci_host *host, u32 opcode)
>  {
>  	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> @@ -1064,51 +1109,6 @@ static void dwcmshc_cqhci_init(struct sdhci_host *host, struct platform_device *
>  	host->mmc->caps2 &= ~(MMC_CAP2_CQE | MMC_CAP2_CQE_DCMD);
>  }
>  
> -static int dwcmshc_rk35xx_init(struct sdhci_host *host, struct dwcmshc_priv *dwc_priv)
> -{
> -	static const char * const clk_ids[] = {"axi", "block", "timer"};
> -	struct rk35xx_priv *priv = dwc_priv->priv;
> -	int err;
> -
> -	priv->reset = devm_reset_control_array_get_optional_exclusive(mmc_dev(host->mmc));
> -	if (IS_ERR(priv->reset)) {
> -		err = PTR_ERR(priv->reset);
> -		dev_err(mmc_dev(host->mmc), "failed to get reset control %d\n", err);
> -		return err;
> -	}
> -
> -	err = dwcmshc_get_enable_other_clks(mmc_dev(host->mmc), dwc_priv,
> -					    ARRAY_SIZE(clk_ids), clk_ids);
> -	if (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;
> -
> -	/* Disable cmd conflict check */
> -	sdhci_writel(host, 0x0, dwc_priv->vendor_specific_area1 + DWCMSHC_HOST_CTRL3);
> -	/* Reset previous settings */
> -	sdhci_writel(host, 0, DWCMSHC_EMMC_DLL_TXCLK);
> -	sdhci_writel(host, 0, DWCMSHC_EMMC_DLL_STRBIN);
> -
> -	return 0;
> -}
> -
> -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 const struct of_device_id sdhci_dwcmshc_dt_ids[] = {
>  	{
>  		.compatible = "rockchip,rk3588-dwcmshc",


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v6 3/8] mmc: sdhci-of-dwcmshc: factor out code for th1520_init()
  2024-08-05  9:17 ` [PATCH v6 3/8] mmc: sdhci-of-dwcmshc: factor out code for th1520_init() Chen Wang
@ 2024-08-08  7:18   ` Adrian Hunter
  0 siblings, 0 replies; 22+ messages in thread
From: Adrian Hunter @ 2024-08-08  7:18 UTC (permalink / raw)
  To: Chen Wang, aou, conor+dt, guoren, inochiama, jszhang,
	krzysztof.kozlowski+dt, palmer, paul.walmsley, robh, ulf.hansson,
	devicetree, linux-kernel, linux-mmc, linux-riscv, chao.wei,
	haijiao.liu, xiaoguang.xing, tingzhu.wang
  Cc: Chen Wang, Drew Fustini

On 5/08/24 12:17, Chen Wang wrote:
> From: Chen Wang <unicorn_wang@outlook.com>
> 
> Different socs have initialization operations in
> the probe process, which are summarized as functions.
> 
> This patch first factor out init function for th1520.
> 
> Signed-off-by: Chen Wang <unicorn_wang@outlook.com>
> Reviewed-by: Drew Fustini <drew@pdp7.com>
> Tested-by: Drew Fustini <drew@pdp7.com> # TH1520
> Tested-by: Inochi Amaoto <inochiama@outlook.com> # Duo and Huashan Pi

Acked-by: Adrian Hunter <adrian.hunter@intel.com>

> ---
>  drivers/mmc/host/sdhci-of-dwcmshc.c | 51 +++++++++++++++++------------
>  1 file changed, 30 insertions(+), 21 deletions(-)
> 
> diff --git a/drivers/mmc/host/sdhci-of-dwcmshc.c b/drivers/mmc/host/sdhci-of-dwcmshc.c
> index a002636d51fd..b272ec2ab232 100644
> --- a/drivers/mmc/host/sdhci-of-dwcmshc.c
> +++ b/drivers/mmc/host/sdhci-of-dwcmshc.c
> @@ -830,6 +830,35 @@ static void th1520_sdhci_reset(struct sdhci_host *host, u8 mask)
>  	}
>  }
>  
> +static int 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;
> +
> +	/*
> +	 * 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 (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 void cv18xx_sdhci_reset(struct sdhci_host *host, u8 mask)
>  {
>  	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> @@ -1231,27 +1260,7 @@ static int dwcmshc_probe(struct platform_device *pdev)
>  	}
>  
>  	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);
> +		th1520_init(dev, host, priv);
>  	}
>  
>  #ifdef CONFIG_ACPI


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v6 4/8] mmc: sdhci-of-dwcmshc: factor out code into dwcmshc_rk35xx_init
  2024-08-05  9:18 ` [PATCH v6 4/8] mmc: sdhci-of-dwcmshc: factor out code into dwcmshc_rk35xx_init Chen Wang
@ 2024-08-08  7:19   ` Adrian Hunter
  0 siblings, 0 replies; 22+ messages in thread
From: Adrian Hunter @ 2024-08-08  7:19 UTC (permalink / raw)
  To: Chen Wang, aou, conor+dt, guoren, inochiama, jszhang,
	krzysztof.kozlowski+dt, palmer, paul.walmsley, robh, ulf.hansson,
	devicetree, linux-kernel, linux-mmc, linux-riscv, chao.wei,
	haijiao.liu, xiaoguang.xing, tingzhu.wang
  Cc: Chen Wang, Drew Fustini

On 5/08/24 12:18, Chen Wang wrote:
> From: Chen Wang <unicorn_wang@outlook.com>
> 
> Continue factor out code fron probe into dwcmshc_rk35xx_init.
> 
> Signed-off-by: Chen Wang <unicorn_wang@outlook.com>
> Tested-by: Drew Fustini <drew@pdp7.com> # TH1520
> Tested-by: Inochi Amaoto <inochiama@outlook.com> # Duo and Huashan Pi

Acked-by: Adrian Hunter <adrian.hunter@intel.com>

> ---
>  drivers/mmc/host/sdhci-of-dwcmshc.c | 34 ++++++++++++++---------------
>  1 file changed, 16 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/mmc/host/sdhci-of-dwcmshc.c b/drivers/mmc/host/sdhci-of-dwcmshc.c
> index b272ec2ab232..55fba5ef37ba 100644
> --- a/drivers/mmc/host/sdhci-of-dwcmshc.c
> +++ b/drivers/mmc/host/sdhci-of-dwcmshc.c
> @@ -711,12 +711,22 @@ static void rk35xx_sdhci_reset(struct sdhci_host *host, u8 mask)
>  	sdhci_reset(host, mask);
>  }
>  
> -static int dwcmshc_rk35xx_init(struct sdhci_host *host, struct dwcmshc_priv *dwc_priv)
> +static int dwcmshc_rk35xx_init(struct device *dev, struct sdhci_host *host,
> +			       struct dwcmshc_priv *dwc_priv)
>  {
>  	static const char * const clk_ids[] = {"axi", "block", "timer"};
> -	struct rk35xx_priv *priv = dwc_priv->priv;
> +	struct rk35xx_priv *priv;
>  	int err;
>  
> +	priv = devm_kzalloc(dev, sizeof(struct rk35xx_priv), GFP_KERNEL);
> +	if (!priv)
> +		return -ENOMEM;
> +
> +	if (of_device_is_compatible(dev->of_node, "rockchip,rk3588-dwcmshc"))
> +		priv->devtype = DWCMSHC_RK3588;
> +	else
> +		priv->devtype = DWCMSHC_RK3568;
> +
>  	priv->reset = devm_reset_control_array_get_optional_exclusive(mmc_dev(host->mmc));
>  	if (IS_ERR(priv->reset)) {
>  		err = PTR_ERR(priv->reset);
> @@ -739,6 +749,8 @@ 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 = priv;
> +
>  	return 0;
>  }
>  
> @@ -1184,7 +1196,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, caps;
> @@ -1241,20 +1252,7 @@ static int dwcmshc_probe(struct platform_device *pdev)
>  	host->mmc_host_ops.execute_tuning = dwcmshc_execute_tuning;
>  
>  	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;
>  	}
> @@ -1290,7 +1288,7 @@ static int dwcmshc_probe(struct platform_device *pdev)
>  		dwcmshc_cqhci_init(host, pdev);
>  	}
>  
> -	if (rk_priv)
> +	if (pltfm_data == &sdhci_dwcmshc_rk35xx_pdata)
>  		dwcmshc_rk35xx_postinit(host, priv);
>  
>  	err = __sdhci_add_host(host);


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v6 5/8] mmc: sdhci-of-dwcmshc: add dwcmshc_pltfm_data
  2024-08-05  9:18 ` [PATCH v6 5/8] mmc: sdhci-of-dwcmshc: add dwcmshc_pltfm_data Chen Wang
@ 2024-08-08  7:19   ` Adrian Hunter
  0 siblings, 0 replies; 22+ messages in thread
From: Adrian Hunter @ 2024-08-08  7:19 UTC (permalink / raw)
  To: Chen Wang, aou, conor+dt, guoren, inochiama, jszhang,
	krzysztof.kozlowski+dt, palmer, paul.walmsley, robh, ulf.hansson,
	devicetree, linux-kernel, linux-mmc, linux-riscv, chao.wei,
	haijiao.liu, xiaoguang.xing, tingzhu.wang
  Cc: Chen Wang, Drew Fustini

On 5/08/24 12:18, Chen Wang wrote:
> From: Chen Wang <unicorn_wang@outlook.com>
> 
> Abstract dwcmshc_pltfm_data to hold the sdhci_pltfm_data
> plus some comoon operations of soc such as init/postinit.
> 
> Signed-off-by: Chen Wang <unicorn_wang@outlook.com>
> Tested-by: Drew Fustini <drew@pdp7.com> # TH1520
> Tested-by: Inochi Amaoto <inochiama@outlook.com> # Duo and Huashan Pi

Acked-by: Adrian Hunter <adrian.hunter@intel.com>

> ---
>  drivers/mmc/host/sdhci-of-dwcmshc.c | 81 +++++++++++++++++------------
>  1 file changed, 48 insertions(+), 33 deletions(-)
> 
> diff --git a/drivers/mmc/host/sdhci-of-dwcmshc.c b/drivers/mmc/host/sdhci-of-dwcmshc.c
> index 55fba5ef37ba..16f420994519 100644
> --- a/drivers/mmc/host/sdhci-of-dwcmshc.c
> +++ b/drivers/mmc/host/sdhci-of-dwcmshc.c
> @@ -218,6 +218,12 @@ struct dwcmshc_priv {
>  	u16 flags;
>  };
>  
> +struct dwcmshc_pltfm_data {
> +	const struct sdhci_pltfm_data pdata;
> +	int (*init)(struct device *dev, struct sdhci_host *host, struct dwcmshc_priv *dwc_priv);
> +	void (*postinit)(struct sdhci_host *host, struct dwcmshc_priv *dwc_priv);
> +};
> +
>  static int dwcmshc_get_enable_other_clks(struct device *dev,
>  					 struct dwcmshc_priv *priv,
>  					 int num_clks,
> @@ -1048,39 +1054,52 @@ static const struct sdhci_ops sdhci_dwcmshc_cv18xx_ops = {
>  	.platform_execute_tuning = cv18xx_sdhci_execute_tuning,
>  };
>  
> -static const struct sdhci_pltfm_data sdhci_dwcmshc_pdata = {
> -	.ops = &sdhci_dwcmshc_ops,
> -	.quirks = SDHCI_QUIRK_CAP_CLOCK_BASE_BROKEN,
> -	.quirks2 = SDHCI_QUIRK2_PRESET_VALUE_BROKEN,
> +static const struct dwcmshc_pltfm_data sdhci_dwcmshc_pdata = {
> +	.pdata = {
> +		.ops = &sdhci_dwcmshc_ops,
> +		.quirks = SDHCI_QUIRK_CAP_CLOCK_BASE_BROKEN,
> +		.quirks2 = SDHCI_QUIRK2_PRESET_VALUE_BROKEN,
> +	},
>  };
>  
>  #ifdef CONFIG_ACPI
> -static const struct sdhci_pltfm_data sdhci_dwcmshc_bf3_pdata = {
> -	.ops = &sdhci_dwcmshc_ops,
> -	.quirks = SDHCI_QUIRK_CAP_CLOCK_BASE_BROKEN,
> -	.quirks2 = SDHCI_QUIRK2_PRESET_VALUE_BROKEN |
> -		   SDHCI_QUIRK2_ACMD23_BROKEN,
> +static const struct dwcmshc_pltfm_data sdhci_dwcmshc_bf3_pdata = {
> +	.pdata = {
> +		.ops = &sdhci_dwcmshc_ops,
> +		.quirks = SDHCI_QUIRK_CAP_CLOCK_BASE_BROKEN,
> +		.quirks2 = SDHCI_QUIRK2_PRESET_VALUE_BROKEN |
> +			   SDHCI_QUIRK2_ACMD23_BROKEN,
> +	},
>  };
>  #endif
>  
> -static const struct sdhci_pltfm_data sdhci_dwcmshc_rk35xx_pdata = {
> -	.ops = &sdhci_dwcmshc_rk35xx_ops,
> -	.quirks = SDHCI_QUIRK_CAP_CLOCK_BASE_BROKEN |
> -		  SDHCI_QUIRK_BROKEN_TIMEOUT_VAL,
> -	.quirks2 = SDHCI_QUIRK2_PRESET_VALUE_BROKEN |
> -		   SDHCI_QUIRK2_CLOCK_DIV_ZERO_BROKEN,
> +static const struct dwcmshc_pltfm_data sdhci_dwcmshc_rk35xx_pdata = {
> +	.pdata = {
> +		.ops = &sdhci_dwcmshc_rk35xx_ops,
> +		.quirks = SDHCI_QUIRK_CAP_CLOCK_BASE_BROKEN |
> +			  SDHCI_QUIRK_BROKEN_TIMEOUT_VAL,
> +		.quirks2 = SDHCI_QUIRK2_PRESET_VALUE_BROKEN |
> +			   SDHCI_QUIRK2_CLOCK_DIV_ZERO_BROKEN,
> +	},
> +	.init = dwcmshc_rk35xx_init,
> +	.postinit = dwcmshc_rk35xx_postinit,
>  };
>  
> -static const struct sdhci_pltfm_data sdhci_dwcmshc_th1520_pdata = {
> -	.ops = &sdhci_dwcmshc_th1520_ops,
> -	.quirks = SDHCI_QUIRK_CAP_CLOCK_BASE_BROKEN,
> -	.quirks2 = SDHCI_QUIRK2_PRESET_VALUE_BROKEN,
> +static const struct dwcmshc_pltfm_data sdhci_dwcmshc_th1520_pdata = {
> +	.pdata = {
> +		.ops = &sdhci_dwcmshc_th1520_ops,
> +		.quirks = SDHCI_QUIRK_CAP_CLOCK_BASE_BROKEN,
> +		.quirks2 = SDHCI_QUIRK2_PRESET_VALUE_BROKEN,
> +	},
> +	.init = th1520_init,
>  };
>  
> -static const struct sdhci_pltfm_data sdhci_dwcmshc_cv18xx_pdata = {
> -	.ops = &sdhci_dwcmshc_cv18xx_ops,
> -	.quirks = SDHCI_QUIRK_CAP_CLOCK_BASE_BROKEN,
> -	.quirks2 = SDHCI_QUIRK2_PRESET_VALUE_BROKEN,
> +static const struct dwcmshc_pltfm_data sdhci_dwcmshc_cv18xx_pdata = {
> +	.pdata = {
> +		.ops = &sdhci_dwcmshc_cv18xx_ops,
> +		.quirks = SDHCI_QUIRK_CAP_CLOCK_BASE_BROKEN,
> +		.quirks2 = SDHCI_QUIRK2_PRESET_VALUE_BROKEN,
> +	},
>  };
>  
>  static const struct cqhci_host_ops dwcmshc_cqhci_ops = {
> @@ -1196,7 +1215,7 @@ static int dwcmshc_probe(struct platform_device *pdev)
>  	struct sdhci_pltfm_host *pltfm_host;
>  	struct sdhci_host *host;
>  	struct dwcmshc_priv *priv;
> -	const struct sdhci_pltfm_data *pltfm_data;
> +	const struct dwcmshc_pltfm_data *pltfm_data;
>  	int err;
>  	u32 extra, caps;
>  
> @@ -1206,7 +1225,7 @@ static int dwcmshc_probe(struct platform_device *pdev)
>  		return -ENODEV;
>  	}
>  
> -	host = sdhci_pltfm_init(pdev, pltfm_data,
> +	host = sdhci_pltfm_init(pdev, &pltfm_data->pdata,
>  				sizeof(struct dwcmshc_priv));
>  	if (IS_ERR(host))
>  		return PTR_ERR(host);
> @@ -1251,16 +1270,12 @@ static int dwcmshc_probe(struct platform_device *pdev)
>  	host->mmc_host_ops.hs400_enhanced_strobe = dwcmshc_hs400_enhanced_strobe;
>  	host->mmc_host_ops.execute_tuning = dwcmshc_execute_tuning;
>  
> -	if (pltfm_data == &sdhci_dwcmshc_rk35xx_pdata) {
> -		err = dwcmshc_rk35xx_init(dev, host, priv);
> +	if (pltfm_data->init) {
> +		err = pltfm_data->init(&pdev->dev, host, priv);
>  		if (err)
>  			goto err_clk;
>  	}
>  
> -	if (pltfm_data == &sdhci_dwcmshc_th1520_pdata) {
> -		th1520_init(dev, host, priv);
> -	}
> -
>  #ifdef CONFIG_ACPI
>  	if (pltfm_data == &sdhci_dwcmshc_bf3_pdata)
>  		sdhci_enable_v4_mode(host);
> @@ -1288,8 +1303,8 @@ static int dwcmshc_probe(struct platform_device *pdev)
>  		dwcmshc_cqhci_init(host, pdev);
>  	}
>  
> -	if (pltfm_data == &sdhci_dwcmshc_rk35xx_pdata)
> -		dwcmshc_rk35xx_postinit(host, priv);
> +	if (pltfm_data->postinit)
> +		pltfm_data->postinit(host, priv);
>  
>  	err = __sdhci_add_host(host);
>  	if (err)


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v6 7/8] mmc: sdhci-of-dwcmshc: Add support for Sophgo SG2042
  2024-08-05  9:19 ` [PATCH v6 7/8] mmc: sdhci-of-dwcmshc: Add support for Sophgo SG2042 Chen Wang
@ 2024-08-08  7:19   ` Adrian Hunter
  0 siblings, 0 replies; 22+ messages in thread
From: Adrian Hunter @ 2024-08-08  7:19 UTC (permalink / raw)
  To: Chen Wang, aou, conor+dt, guoren, inochiama, jszhang,
	krzysztof.kozlowski+dt, palmer, paul.walmsley, robh, ulf.hansson,
	devicetree, linux-kernel, linux-mmc, linux-riscv, chao.wei,
	haijiao.liu, xiaoguang.xing, tingzhu.wang
  Cc: Chen Wang

On 5/08/24 12:19, Chen Wang wrote:
> From: Chen Wang <unicorn_wang@outlook.com>
> 
> Add support for the mmc controller of Sophgo SG2042.
> 
> SG2042 uses Synopsys PHY the same as TH1520 so we reuse the tuning
> logic from TH1520. Besides this, this patch implement some SG2042
> specific work, such as clocks and reset ops.
> 
> Signed-off-by: Chen Wang <unicorn_wang@outlook.com>

Acked-by: Adrian Hunter <adrian.hunter@intel.com>

> ---
>  drivers/mmc/host/sdhci-of-dwcmshc.c | 125 ++++++++++++++++++++++++++--
>  1 file changed, 118 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/mmc/host/sdhci-of-dwcmshc.c b/drivers/mmc/host/sdhci-of-dwcmshc.c
> index 16f420994519..ba8960d8b2d4 100644
> --- a/drivers/mmc/host/sdhci-of-dwcmshc.c
> +++ b/drivers/mmc/host/sdhci-of-dwcmshc.c
> @@ -113,12 +113,15 @@
>  #define DWC_MSHC_PTR_PHY_R	0x300
>  
>  /* PHY general configuration */
> -#define PHY_CNFG_R		(DWC_MSHC_PTR_PHY_R + 0x00)
> -#define PHY_CNFG_RSTN_DEASSERT	0x1  /* Deassert PHY reset */
> -#define PHY_CNFG_PAD_SP_MASK	GENMASK(19, 16) /* bits [19:16] */
> -#define PHY_CNFG_PAD_SP		0x0c /* PMOS TX drive strength */
> -#define PHY_CNFG_PAD_SN_MASK	GENMASK(23, 20) /* bits [23:20] */
> -#define PHY_CNFG_PAD_SN		0x0c /* NMOS TX drive strength */
> +#define PHY_CNFG_R			(DWC_MSHC_PTR_PHY_R + 0x00)
> +#define PHY_CNFG_RSTN_DEASSERT		0x1  /* Deassert PHY reset */
> +#define PHY_CNFG_PHY_PWRGOOD_MASK	BIT_MASK(1) /* bit [1] */
> +#define PHY_CNFG_PAD_SP_MASK		GENMASK(19, 16) /* bits [19:16] */
> +#define PHY_CNFG_PAD_SP			0x0c /* PMOS TX drive strength */
> +#define PHY_CNFG_PAD_SP_SG2042		0x09 /* PMOS TX drive strength for SG2042 */
> +#define PHY_CNFG_PAD_SN_MASK		GENMASK(23, 20) /* bits [23:20] */
> +#define PHY_CNFG_PAD_SN			0x0c /* NMOS TX drive strength */
> +#define PHY_CNFG_PAD_SN_SG2042		0x08 /* NMOS TX drive strength for SG2042 */
>  
>  /* PHY command/response pad settings */
>  #define PHY_CMDPAD_CNFG_R	(DWC_MSHC_PTR_PHY_R + 0x04)
> @@ -147,10 +150,12 @@
>  #define PHY_PAD_TXSLEW_CTRL_P		0x3 /* Slew control for P-Type pad TX */
>  #define PHY_PAD_TXSLEW_CTRL_N_MASK	GENMASK(12, 9) /* bits [12:9] */
>  #define PHY_PAD_TXSLEW_CTRL_N		0x3 /* Slew control for N-Type pad TX */
> +#define PHY_PAD_TXSLEW_CTRL_N_SG2042	0x2 /* Slew control for N-Type pad TX for SG2042 */
>  
>  /* PHY CLK delay line settings */
>  #define PHY_SDCLKDL_CNFG_R		(DWC_MSHC_PTR_PHY_R + 0x1d)
> -#define PHY_SDCLKDL_CNFG_UPDATE	BIT(4) /* set before writing to SDCLKDL_DC */
> +#define PHY_SDCLKDL_CNFG_EXTDLY_EN	BIT(0)
> +#define PHY_SDCLKDL_CNFG_UPDATE		BIT(4) /* set before writing to SDCLKDL_DC */
>  
>  /* PHY CLK delay line delay code */
>  #define PHY_SDCLKDL_DC_R		(DWC_MSHC_PTR_PHY_R + 0x1e)
> @@ -158,10 +163,14 @@
>  #define PHY_SDCLKDL_DC_DEFAULT		0x32 /* default delay code */
>  #define PHY_SDCLKDL_DC_HS400		0x18 /* delay code for HS400 mode */
>  
> +#define PHY_SMPLDL_CNFG_R		(DWC_MSHC_PTR_PHY_R + 0x20)
> +#define PHY_SMPLDL_CNFG_BYPASS_EN	BIT(1)
> +
>  /* PHY drift_cclk_rx delay line configuration setting */
>  #define PHY_ATDL_CNFG_R			(DWC_MSHC_PTR_PHY_R + 0x21)
>  #define PHY_ATDL_CNFG_INPSEL_MASK	GENMASK(3, 2) /* bits [3:2] */
>  #define PHY_ATDL_CNFG_INPSEL		0x3 /* delay line input source */
> +#define PHY_ATDL_CNFG_INPSEL_SG2042	0x2 /* delay line input source for SG2042 */
>  
>  /* PHY DLL control settings */
>  #define PHY_DLL_CTRL_R			(DWC_MSHC_PTR_PHY_R + 0x24)
> @@ -1013,6 +1022,85 @@ static int cv18xx_sdhci_execute_tuning(struct sdhci_host *host, u32 opcode)
>  	return ret;
>  }
>  
> +static inline void sg2042_sdhci_phy_init(struct sdhci_host *host)
> +{
> +	u32 val;
> +
> +	/* Asset phy reset & set tx drive strength */
> +	val = sdhci_readl(host, PHY_CNFG_R);
> +	val &= ~PHY_CNFG_RSTN_DEASSERT;
> +	val |= FIELD_PREP(PHY_CNFG_PHY_PWRGOOD_MASK, 1);
> +	val |= FIELD_PREP(PHY_CNFG_PAD_SP_MASK, PHY_CNFG_PAD_SP_SG2042);
> +	val |= FIELD_PREP(PHY_CNFG_PAD_SN_MASK, PHY_CNFG_PAD_SN_SG2042);
> +	sdhci_writel(host, val, PHY_CNFG_R);
> +
> +	/* Configure phy pads */
> +	val = PHY_PAD_RXSEL_3V3;
> +	val |= FIELD_PREP(PHY_PAD_WEAKPULL_MASK, PHY_PAD_WEAKPULL_PULLUP);
> +	val |= FIELD_PREP(PHY_PAD_TXSLEW_CTRL_P_MASK, PHY_PAD_TXSLEW_CTRL_P);
> +	val |= FIELD_PREP(PHY_PAD_TXSLEW_CTRL_N_MASK, PHY_PAD_TXSLEW_CTRL_N_SG2042);
> +	sdhci_writew(host, val, PHY_CMDPAD_CNFG_R);
> +	sdhci_writew(host, val, PHY_DATAPAD_CNFG_R);
> +	sdhci_writew(host, val, PHY_RSTNPAD_CNFG_R);
> +
> +	val = PHY_PAD_RXSEL_3V3;
> +	val |= FIELD_PREP(PHY_PAD_TXSLEW_CTRL_P_MASK, PHY_PAD_TXSLEW_CTRL_P);
> +	val |= FIELD_PREP(PHY_PAD_TXSLEW_CTRL_N_MASK, PHY_PAD_TXSLEW_CTRL_N_SG2042);
> +	sdhci_writew(host, val, PHY_CLKPAD_CNFG_R);
> +
> +	val = PHY_PAD_RXSEL_3V3;
> +	val |= FIELD_PREP(PHY_PAD_WEAKPULL_MASK, PHY_PAD_WEAKPULL_PULLDOWN);
> +	val |= FIELD_PREP(PHY_PAD_TXSLEW_CTRL_P_MASK, PHY_PAD_TXSLEW_CTRL_P);
> +	val |= FIELD_PREP(PHY_PAD_TXSLEW_CTRL_N_MASK, PHY_PAD_TXSLEW_CTRL_N_SG2042);
> +	sdhci_writew(host, val, PHY_STBPAD_CNFG_R);
> +
> +	/* Configure delay line */
> +	/* Enable fixed delay */
> +	sdhci_writeb(host, PHY_SDCLKDL_CNFG_EXTDLY_EN, PHY_SDCLKDL_CNFG_R);
> +	/*
> +	 * Set delay line.
> +	 * Its recommended that bit UPDATE_DC[4] is 1 when SDCLKDL_DC is being written.
> +	 * Ensure UPDATE_DC[4] is '0' when not updating code.
> +	 */
> +	val = sdhci_readb(host, PHY_SDCLKDL_CNFG_R);
> +	val |= PHY_SDCLKDL_CNFG_UPDATE;
> +	sdhci_writeb(host, val, PHY_SDCLKDL_CNFG_R);
> +	/* Add 10 * 70ps = 0.7ns for output delay */
> +	sdhci_writeb(host, 10, PHY_SDCLKDL_DC_R);
> +	val = sdhci_readb(host, PHY_SDCLKDL_CNFG_R);
> +	val &= ~(PHY_SDCLKDL_CNFG_UPDATE);
> +	sdhci_writeb(host, val, PHY_SDCLKDL_CNFG_R);
> +
> +	/* Set SMPLDL_CNFG, Bypass */
> +	sdhci_writeb(host, PHY_SMPLDL_CNFG_BYPASS_EN, PHY_SMPLDL_CNFG_R);
> +
> +	/* Set ATDL_CNFG, tuning clk not use for init */
> +	val = FIELD_PREP(PHY_ATDL_CNFG_INPSEL_MASK, PHY_ATDL_CNFG_INPSEL_SG2042);
> +	sdhci_writeb(host, val, PHY_ATDL_CNFG_R);
> +
> +	/* Deasset phy reset */
> +	val = sdhci_readl(host, PHY_CNFG_R);
> +	val |= PHY_CNFG_RSTN_DEASSERT;
> +	sdhci_writel(host, val, PHY_CNFG_R);
> +}
> +
> +static void sg2042_sdhci_reset(struct sdhci_host *host, u8 mask)
> +{
> +	sdhci_reset(host, mask);
> +
> +	if (mask & SDHCI_RESET_ALL)
> +		sg2042_sdhci_phy_init(host);
> +}
> +
> +static int sg2042_init(struct device *dev, struct sdhci_host *host,
> +		       struct dwcmshc_priv *dwc_priv)
> +{
> +	static const char * const clk_ids[] = {"timer"};
> +
> +	return dwcmshc_get_enable_other_clks(mmc_dev(host->mmc), dwc_priv,
> +					     ARRAY_SIZE(clk_ids), clk_ids);
> +}
> +
>  static const struct sdhci_ops sdhci_dwcmshc_ops = {
>  	.set_clock		= sdhci_set_clock,
>  	.set_bus_width		= sdhci_set_bus_width,
> @@ -1054,6 +1142,16 @@ static const struct sdhci_ops sdhci_dwcmshc_cv18xx_ops = {
>  	.platform_execute_tuning = cv18xx_sdhci_execute_tuning,
>  };
>  
> +static const struct sdhci_ops sdhci_dwcmshc_sg2042_ops = {
> +	.set_clock		= sdhci_set_clock,
> +	.set_bus_width		= sdhci_set_bus_width,
> +	.set_uhs_signaling	= dwcmshc_set_uhs_signaling,
> +	.get_max_clock		= dwcmshc_get_max_clock,
> +	.reset			= sg2042_sdhci_reset,
> +	.adma_write_desc	= dwcmshc_adma_write_desc,
> +	.platform_execute_tuning = th1520_execute_tuning,
> +};
> +
>  static const struct dwcmshc_pltfm_data sdhci_dwcmshc_pdata = {
>  	.pdata = {
>  		.ops = &sdhci_dwcmshc_ops,
> @@ -1102,6 +1200,15 @@ static const struct dwcmshc_pltfm_data sdhci_dwcmshc_cv18xx_pdata = {
>  	},
>  };
>  
> +static const struct dwcmshc_pltfm_data sdhci_dwcmshc_sg2042_pdata = {
> +	.pdata = {
> +		.ops = &sdhci_dwcmshc_sg2042_ops,
> +		.quirks = SDHCI_QUIRK_CAP_CLOCK_BASE_BROKEN,
> +		.quirks2 = SDHCI_QUIRK2_PRESET_VALUE_BROKEN,
> +	},
> +	.init = sg2042_init,
> +};
> +
>  static const struct cqhci_host_ops dwcmshc_cqhci_ops = {
>  	.enable		= dwcmshc_sdhci_cqe_enable,
>  	.disable	= sdhci_cqe_disable,
> @@ -1194,6 +1301,10 @@ static const struct of_device_id sdhci_dwcmshc_dt_ids[] = {
>  		.compatible = "thead,th1520-dwcmshc",
>  		.data = &sdhci_dwcmshc_th1520_pdata,
>  	},
> +	{
> +		.compatible = "sophgo,sg2042-dwcmshc",
> +		.data = &sdhci_dwcmshc_sg2042_pdata,
> +	},
>  	{},
>  };
>  MODULE_DEVICE_TABLE(of, sdhci_dwcmshc_dt_ids);


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v6 0/8] mmc: sdhci-of-dwcmshc: Add Sophgo SG2042 support
  2024-08-05  9:15 [PATCH v6 0/8] mmc: sdhci-of-dwcmshc: Add Sophgo SG2042 support Chen Wang
                   ` (7 preceding siblings ...)
  2024-08-05  9:19 ` [PATCH v6 8/8] riscv: sophgo: dts: add mmc controllers for SG2042 SoC Chen Wang
@ 2024-08-20 11:49 ` Ulf Hansson
  2024-08-26  2:59 ` (subset) " Inochi Amaoto
  9 siblings, 0 replies; 22+ messages in thread
From: Ulf Hansson @ 2024-08-20 11:49 UTC (permalink / raw)
  To: Chen Wang
  Cc: adrian.hunter, aou, conor+dt, guoren, inochiama, jszhang,
	krzysztof.kozlowski+dt, palmer, paul.walmsley, robh, devicetree,
	linux-kernel, linux-mmc, linux-riscv, chao.wei, haijiao.liu,
	xiaoguang.xing, tingzhu.wang, Chen Wang

On Mon, 5 Aug 2024 at 11:15, Chen Wang <unicornxw@gmail.com> wrote:
>
> From: Chen Wang <unicorn_wang@outlook.com>
>
> This patchset is composed of two parts:
> - one is the improvement of the sdhci-of-dwcmshc framework,
> - the other is the support for sg2042 based on the improvement of the
>   framework.
> The reason for merging the two parts into one patchset is mainly to
> facilitate review, especially to facilitate viewing why we need to
> improve the framework and what benefits it will bring to us.
>
> When I tried to add a new soc(SG2042) to sdhci-of-dwcmshc, I found
> that the existing driver code could be optimized to facilitate expansion
> for the new soc. Patch 1 ~ Patch 5 is for this.
>
> Patch 6 ~ 7 are adding support for the mmc controller for Sophgo SG2042.
> Adding corresponding new compatible strings, and implement
> custom callbacks for SG2042 based on new framework.
>
> Patch 8 is the change for DTS.
>
> By the way, although I believe this patch only optimizes the framework
> of the code and does not change the specific logic, simple verification
> is certainly better. Since I don't have rk35xx/th1520 related hardware,
> it would be greatly appreciated if someone could help verify it.
>
> ---
>
> Changes in v6:
>
>   The patch series is based on latest 'next' branch of [mmc-git].
>
>   - Some minor improvements based on Adrian's review suggestions.
>   - Added Reviewed-by and Tested-by signatures from Conor/Drew/Inochi.
>

Patch 1 -> 7 applied for next, thanks!

Kind regards
Uffe

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: (subset) [PATCH v6 0/8] mmc: sdhci-of-dwcmshc: Add Sophgo SG2042 support
  2024-08-05  9:15 [PATCH v6 0/8] mmc: sdhci-of-dwcmshc: Add Sophgo SG2042 support Chen Wang
                   ` (8 preceding siblings ...)
  2024-08-20 11:49 ` [PATCH v6 0/8] mmc: sdhci-of-dwcmshc: Add Sophgo SG2042 support Ulf Hansson
@ 2024-08-26  2:59 ` Inochi Amaoto
  9 siblings, 0 replies; 22+ messages in thread
From: Inochi Amaoto @ 2024-08-26  2:59 UTC (permalink / raw)
  To: adrian.hunter, aou, conor+dt, guoren, jszhang,
	krzysztof.kozlowski+dt, palmer, paul.walmsley, robh, ulf.hansson,
	devicetree, linux-kernel, linux-mmc, linux-riscv, chao.wei,
	haijiao.liu, xiaoguang.xing, tingzhu.wang, Chen Wang
  Cc: Inochi Amaoto, Chen Wang

On Mon, 5 Aug 2024 17:15:18 +0800, Chen Wang wrote:
> From: Chen Wang <unicorn_wang@outlook.com>
> 
> This patchset is composed of two parts:
> - one is the improvement of the sdhci-of-dwcmshc framework,
> - the other is the support for sg2042 based on the improvement of the
>   framework.
> The reason for merging the two parts into one patchset is mainly to
> facilitate review, especially to facilitate viewing why we need to
> improve the framework and what benefits it will bring to us.
> 
> [...]

Applied to sg2042/for-next, thanks!

[8/8] riscv: sophgo: dts: add mmc controllers for SG2042 SoC
      https://github.com/sophgo/linux/commit/06b2359ee13a0e8267c1777838ac8b66ca667237

Thanks,
Inochi


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v6 1/8] mmc: sdhci-of-dwcmshc: add common bulk optional clocks support
  2024-08-05  9:17 ` [PATCH v6 1/8] mmc: sdhci-of-dwcmshc: add common bulk optional clocks support Chen Wang
  2024-08-08  7:17   ` Adrian Hunter
@ 2025-07-22 18:33   ` Robin Murphy
  2025-07-23  5:33     ` Adrian Hunter
  1 sibling, 1 reply; 22+ messages in thread
From: Robin Murphy @ 2025-07-22 18:33 UTC (permalink / raw)
  To: Chen Wang, adrian.hunter, aou, conor+dt, guoren, inochiama,
	jszhang, krzysztof.kozlowski+dt, palmer, paul.walmsley, robh,
	ulf.hansson, devicetree, linux-kernel, linux-mmc, linux-riscv,
	chao.wei, haijiao.liu, xiaoguang.xing, tingzhu.wang
  Cc: Chen Wang, Drew Fustini, Diederik de Haas,
	open list:ARM/Rockchip SoC...

A bit late for a "review", but Diederik and I have just been
IRC-debugging a crash on RK3568 which by inspection seems to be caused
by this patch:

On 2024-08-05 10:17 am, Chen Wang wrote:
> From: Chen Wang <unicorn_wang@outlook.com>
> 
> In addition to the required core clock and optional
> bus clock, the soc will expand its own clocks, so
> the bulk clock mechanism is abstracted.
> 
> Note, I call the bulk clocks as "other clocks" due
> to the bus clock has been called as "optional".
> 
> Signed-off-by: Chen Wang <unicorn_wang@outlook.com>
> Tested-by: Drew Fustini <drew@pdp7.com> # TH1520
> Tested-by: Inochi Amaoto <inochiama@outlook.com> # Duo and Huashan Pi
> ---
[...]
> +static int dwcmshc_get_enable_other_clks(struct device *dev,
> +					 struct dwcmshc_priv *priv,
> +					 int num_clks,
> +					 const char * const clk_ids[])
> +{
> +	int err;
> +
> +	if (num_clks > DWCMSHC_MAX_OTHER_CLKS)
> +		return -EINVAL;
> +
> +	for (int i = 0; i < num_clks; i++)
> +		priv->other_clks[i].id = clk_ids[i];
> +
> +	err = devm_clk_bulk_get_optional(dev, num_clks, priv->other_clks);

This leaves a pointer into "priv" in the devres list...

> +	if (err) {
> +		dev_err(dev, "failed to get clocks %d\n", err);
> +		return err;
> +	}
> +
> +	err = clk_bulk_prepare_enable(num_clks, priv->other_clks);
> +	if (err)
> +		dev_err(dev, "failed to enable clocks %d\n", err);
> +
> +	priv->num_other_clks = num_clks;
> +
> +	return err;
> +}
> +
>   /*
>    * If DMA addr spans 128MB boundary, we split the DMA transfer into two
>    * so that each DMA transfer doesn't exceed the boundary.
[...]
> @@ -1280,9 +1300,7 @@ 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);
> +	clk_bulk_disable_unprepare(priv->num_other_clks, priv->other_clks);
>   free_pltfm:
>   	sdhci_pltfm_free(pdev);

...but upon, say, -EPROBE_DEFER from sdhci_setup_host() because a
regulator isn't ready yet, that "priv" is freed here, so by the time the
devres callbacks eventually run, that "devres->clks" pointer which used
to represent "priv->other_clocks" points to who knows what, and this
sort of thing happens:

[   12.470827] Unable to handle kernel paging request at virtual address 002df7b378917664
[   12.472104] Mem abort info:
[   12.472471]   ESR = 0x0000000096000004
[   12.475991]   EC = 0x25: DABT (current EL), IL = 32 bits
[   12.476657]   SET = 0, FnV = 0
[   12.477146]   EA = 0, S1PTW = 0
[   12.477547]   FSC = 0x04: level 0 translation fault
[   12.478127] Data abort info:
[   12.478126] rockchip-gpio fdd60000.gpio: probed /pinctrl/gpio@fdd60000
[   12.478413]   ISV = 0, ISS = 0x00000004, ISS2 = 0x00000000
[   12.479826]   CM = 0, WnR = 0, TnD = 0, TagAccess = 0
[   12.480418]   GCS = 0, Overlay = 0, DirtyBit = 0, Xs = 0
[   12.481282] [002df7b378917664] address between user and kernel address ranges
[   12.482421] Internal error: Oops: 0000000096000004 [#1]  SMP
[   12.482980] Modules linked in: sdhci_of_dwcmshc drm_dp_aux_bus gpio_rockchip(+) drm_display_helper dw_mmc_rockchip drm_client_lib sdhci_pltfm drm_dma_helper fwnode_mdio sdhci dw_mmc_pltf
m libphy fixed rockchip_dfi drm_kms_helper cqhci pl330(+) phy_rockchip_naneng_combphy dw_wdt phy_rockchip_snps_pcie3 phy_rockchip_inno_usb2 dw_mmc mdio_bus dwc3 ehci_platform ohci_platform
ehci_hcd drm ohci_hcd udc_core io_domain i2c_rk3x usbcore ulpi usb_common
[   12.486871] CPU: 0 UID: 0 PID: 64 Comm: kworker/u16:3 Not tainted 6.16-rc7-arm64-cknow #1 PREEMPTLAZY  Debian 6.16~rc7-1
[   12.487901] Hardware name: FriendlyElec NanoPi R5S (DT)
[   12.488412] Workqueue: async async_run_entry_fn
[   12.488879] pstate: 60400009 (nZCv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
[   12.489539] pc : __clk_put+0x2c/0x138
[   12.489913] lr : __clk_put+0x2c/0x138
[   12.490281] sp : ffff800080713b10
[   12.490607] x29: ffff800080713b10 x28: ffff0001f001a120 x27: 0000000000000000
[   12.491302] x26: ffff0001f98e01a0 x25: 0000000000000000 x24: ffff0001f0f35408
[   12.491995] x23: ffffa8da199b4b40 x22: ffff800080713bb0 x21: ffff0001f0f35010
[   12.492689] x20: ffff0001f94aafd0 x19: 0a2df7b378917634 x18: 00000000ffffffff
[   12.493381] x17: 3d4d455453595342 x16: 555300307075656b x15: ffff0001f4885650
[   12.494075] x14: 0000000000000000 x13: ffff0001f025b810 x12: 0000000000008000
[   12.494765] x11: ffffa8da1a73ef98 x10: ffffa8da1a460000 x9 : 0000000000000078
[   12.495454] x8 : 0000000000000049 x7 : ffffa8da18c2fbe0 x6 : 0000000000000001
[   12.496145] x5 : 0000000000000004 x4 : 000000006cb6bb63 x3 : 0000000000000000
[   12.496833] x2 : 0000000000000000 x1 : ffff0001f1365ac0 x0 : 0000000000000001
[   12.497524] Call trace:
[   12.497776]  __clk_put+0x2c/0x138 (P)
[   12.498154]  clk_put+0x18/0x30
[   12.498471]  clk_bulk_put+0x40/0x68
[   12.498825]  devm_clk_bulk_release+0x24/0x40
[   12.499248]  release_nodes+0x64/0xa0
[   12.499608]  devres_release_all+0x98/0xf8
[   12.500004]  device_unbind_cleanup+0x20/0x70
[   12.500426]  really_probe+0x1e8/0x3a0
[   12.500793]  __driver_probe_device+0x84/0x160
[   12.501225]  driver_probe_device+0x44/0x128
[   12.501640]  __driver_attach_async_helper+0x5c/0x108
[   12.502125]  async_run_entry_fn+0x40/0x180
[   12.502535]  process_one_work+0x23c/0x640
[   12.502939]  worker_thread+0x1b4/0x360
[   12.503315]  kthread+0x150/0x250
[   12.503646]  ret_from_fork+0x10/0x20
[   12.504015] Code: aa0003f3 b140041f 540006c8 97ffd9c4 (b9403260)
[   12.504598] ---[ end trace 0000000000000000 ]---


TBH I'm not sure what to do as a straight revert seems impractical by
now, so we hope someone else might have a good idea.

Thanks,
Robin.

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v6 1/8] mmc: sdhci-of-dwcmshc: add common bulk optional clocks support
  2025-07-22 18:33   ` Robin Murphy
@ 2025-07-23  5:33     ` Adrian Hunter
  2025-07-24 14:33       ` Diederik de Haas
  0 siblings, 1 reply; 22+ messages in thread
From: Adrian Hunter @ 2025-07-23  5:33 UTC (permalink / raw)
  To: Robin Murphy, Chen Wang, aou, conor+dt, guoren, inochiama,
	jszhang, krzysztof.kozlowski+dt, palmer, paul.walmsley, robh,
	ulf.hansson, devicetree, linux-kernel, linux-mmc, linux-riscv,
	chao.wei, haijiao.liu, xiaoguang.xing, tingzhu.wang
  Cc: Chen Wang, Drew Fustini, Diederik de Haas,
	open list:ARM/Rockchip SoC...

On 22/07/2025 21:33, Robin Murphy wrote:
> A bit late for a "review", but Diederik and I have just been
> IRC-debugging a crash on RK3568 which by inspection seems to be caused
> by this patch:
> 
> On 2024-08-05 10:17 am, Chen Wang wrote:
>> From: Chen Wang <unicorn_wang@outlook.com>
>>
>> In addition to the required core clock and optional
>> bus clock, the soc will expand its own clocks, so
>> the bulk clock mechanism is abstracted.
>>
>> Note, I call the bulk clocks as "other clocks" due
>> to the bus clock has been called as "optional".
>>
>> Signed-off-by: Chen Wang <unicorn_wang@outlook.com>
>> Tested-by: Drew Fustini <drew@pdp7.com> # TH1520
>> Tested-by: Inochi Amaoto <inochiama@outlook.com> # Duo and Huashan Pi
>> ---
> [...]
>> +static int dwcmshc_get_enable_other_clks(struct device *dev,
>> +                     struct dwcmshc_priv *priv,
>> +                     int num_clks,
>> +                     const char * const clk_ids[])
>> +{
>> +    int err;
>> +
>> +    if (num_clks > DWCMSHC_MAX_OTHER_CLKS)
>> +        return -EINVAL;
>> +
>> +    for (int i = 0; i < num_clks; i++)
>> +        priv->other_clks[i].id = clk_ids[i];
>> +
>> +    err = devm_clk_bulk_get_optional(dev, num_clks, priv->other_clks);
> 
> This leaves a pointer into "priv" in the devres list...
> 
>> +    if (err) {
>> +        dev_err(dev, "failed to get clocks %d\n", err);
>> +        return err;
>> +    }
>> +
>> +    err = clk_bulk_prepare_enable(num_clks, priv->other_clks);
>> +    if (err)
>> +        dev_err(dev, "failed to enable clocks %d\n", err);
>> +
>> +    priv->num_other_clks = num_clks;
>> +
>> +    return err;
>> +}
>> +
>>   /*
>>    * If DMA addr spans 128MB boundary, we split the DMA transfer into two
>>    * so that each DMA transfer doesn't exceed the boundary.
> [...]
>> @@ -1280,9 +1300,7 @@ 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);
>> +    clk_bulk_disable_unprepare(priv->num_other_clks, priv->other_clks);
>>   free_pltfm:
>>       sdhci_pltfm_free(pdev);
> 
> ...but upon, say, -EPROBE_DEFER from sdhci_setup_host() because a
> regulator isn't ready yet, that "priv" is freed here, so by the time the
> devres callbacks eventually run, that "devres->clks" pointer which used
> to represent "priv->other_clocks" points to who knows what, and this
> sort of thing happens:
> 
> [   12.470827] Unable to handle kernel paging request at virtual address 002df7b378917664
> [   12.472104] Mem abort info:
> [   12.472471]   ESR = 0x0000000096000004
> [   12.475991]   EC = 0x25: DABT (current EL), IL = 32 bits
> [   12.476657]   SET = 0, FnV = 0
> [   12.477146]   EA = 0, S1PTW = 0
> [   12.477547]   FSC = 0x04: level 0 translation fault
> [   12.478127] Data abort info:
> [   12.478126] rockchip-gpio fdd60000.gpio: probed /pinctrl/gpio@fdd60000
> [   12.478413]   ISV = 0, ISS = 0x00000004, ISS2 = 0x00000000
> [   12.479826]   CM = 0, WnR = 0, TnD = 0, TagAccess = 0
> [   12.480418]   GCS = 0, Overlay = 0, DirtyBit = 0, Xs = 0
> [   12.481282] [002df7b378917664] address between user and kernel address ranges
> [   12.482421] Internal error: Oops: 0000000096000004 [#1]  SMP
> [   12.482980] Modules linked in: sdhci_of_dwcmshc drm_dp_aux_bus gpio_rockchip(+) drm_display_helper dw_mmc_rockchip drm_client_lib sdhci_pltfm drm_dma_helper fwnode_mdio sdhci dw_mmc_pltf
> m libphy fixed rockchip_dfi drm_kms_helper cqhci pl330(+) phy_rockchip_naneng_combphy dw_wdt phy_rockchip_snps_pcie3 phy_rockchip_inno_usb2 dw_mmc mdio_bus dwc3 ehci_platform ohci_platform
> ehci_hcd drm ohci_hcd udc_core io_domain i2c_rk3x usbcore ulpi usb_common
> [   12.486871] CPU: 0 UID: 0 PID: 64 Comm: kworker/u16:3 Not tainted 6.16-rc7-arm64-cknow #1 PREEMPTLAZY  Debian 6.16~rc7-1
> [   12.487901] Hardware name: FriendlyElec NanoPi R5S (DT)
> [   12.488412] Workqueue: async async_run_entry_fn
> [   12.488879] pstate: 60400009 (nZCv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
> [   12.489539] pc : __clk_put+0x2c/0x138
> [   12.489913] lr : __clk_put+0x2c/0x138
> [   12.490281] sp : ffff800080713b10
> [   12.490607] x29: ffff800080713b10 x28: ffff0001f001a120 x27: 0000000000000000
> [   12.491302] x26: ffff0001f98e01a0 x25: 0000000000000000 x24: ffff0001f0f35408
> [   12.491995] x23: ffffa8da199b4b40 x22: ffff800080713bb0 x21: ffff0001f0f35010
> [   12.492689] x20: ffff0001f94aafd0 x19: 0a2df7b378917634 x18: 00000000ffffffff
> [   12.493381] x17: 3d4d455453595342 x16: 555300307075656b x15: ffff0001f4885650
> [   12.494075] x14: 0000000000000000 x13: ffff0001f025b810 x12: 0000000000008000
> [   12.494765] x11: ffffa8da1a73ef98 x10: ffffa8da1a460000 x9 : 0000000000000078
> [   12.495454] x8 : 0000000000000049 x7 : ffffa8da18c2fbe0 x6 : 0000000000000001
> [   12.496145] x5 : 0000000000000004 x4 : 000000006cb6bb63 x3 : 0000000000000000
> [   12.496833] x2 : 0000000000000000 x1 : ffff0001f1365ac0 x0 : 0000000000000001
> [   12.497524] Call trace:
> [   12.497776]  __clk_put+0x2c/0x138 (P)
> [   12.498154]  clk_put+0x18/0x30
> [   12.498471]  clk_bulk_put+0x40/0x68
> [   12.498825]  devm_clk_bulk_release+0x24/0x40
> [   12.499248]  release_nodes+0x64/0xa0
> [   12.499608]  devres_release_all+0x98/0xf8
> [   12.500004]  device_unbind_cleanup+0x20/0x70
> [   12.500426]  really_probe+0x1e8/0x3a0
> [   12.500793]  __driver_probe_device+0x84/0x160
> [   12.501225]  driver_probe_device+0x44/0x128
> [   12.501640]  __driver_attach_async_helper+0x5c/0x108
> [   12.502125]  async_run_entry_fn+0x40/0x180
> [   12.502535]  process_one_work+0x23c/0x640
> [   12.502939]  worker_thread+0x1b4/0x360
> [   12.503315]  kthread+0x150/0x250
> [   12.503646]  ret_from_fork+0x10/0x20
> [   12.504015] Code: aa0003f3 b140041f 540006c8 97ffd9c4 (b9403260)
> [   12.504598] ---[ end trace 0000000000000000 ]---
> 
> 
> TBH I'm not sure what to do as a straight revert seems impractical by
> now, so we hope someone else might have a good idea.

Presumably the problem has gone away with:

	commit 91a001a1a0749e5d24606d46ac5dfd4433c00956
	Author: Binbin Zhou <zhoubinbin@loongson.cn>
	Date:   Sat Jun 7 15:39:01 2025 +0800

	    mmc: sdhci-of-dwcmshc: Drop the use of sdhci_pltfm_free()

which is in next.

In which case a separate fix is needed for stable.


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v6 1/8] mmc: sdhci-of-dwcmshc: add common bulk optional clocks support
  2025-07-23  5:33     ` Adrian Hunter
@ 2025-07-24 14:33       ` Diederik de Haas
  2025-07-24 14:57         ` Adrian Hunter
  0 siblings, 1 reply; 22+ messages in thread
From: Diederik de Haas @ 2025-07-24 14:33 UTC (permalink / raw)
  To: Adrian Hunter, Robin Murphy, Chen Wang, aou, conor+dt, guoren,
	inochiama, jszhang, krzysztof.kozlowski+dt, palmer, paul.walmsley,
	robh, ulf.hansson, devicetree, linux-kernel, linux-mmc,
	linux-riscv, chao.wei, haijiao.liu, xiaoguang.xing, tingzhu.wang
  Cc: Chen Wang, Drew Fustini, linux-rockchip

[-- Attachment #1: Type: text/plain, Size: 7221 bytes --]

Hi Adrian,

On Wed Jul 23, 2025 at 7:33 AM CEST, Adrian Hunter wrote:
> On 22/07/2025 21:33, Robin Murphy wrote:
>> A bit late for a "review", but Diederik and I have just been
>> IRC-debugging a crash on RK3568 which by inspection seems to be caused
>> by this patch:
>> 
>> On 2024-08-05 10:17 am, Chen Wang wrote:
>>> From: Chen Wang <unicorn_wang@outlook.com>
>>>
>>> In addition to the required core clock and optional
>>> bus clock, the soc will expand its own clocks, so
>>> the bulk clock mechanism is abstracted.
>>>
>>> Note, I call the bulk clocks as "other clocks" due
>>> to the bus clock has been called as "optional".
>>>
>>> Signed-off-by: Chen Wang <unicorn_wang@outlook.com>
>>> Tested-by: Drew Fustini <drew@pdp7.com> # TH1520
>>> Tested-by: Inochi Amaoto <inochiama@outlook.com> # Duo and Huashan Pi
>>> ---
>> [...]
>>> +static int dwcmshc_get_enable_other_clks(struct device *dev,
>>> +                     struct dwcmshc_priv *priv,
>>> +                     int num_clks,
>>> +                     const char * const clk_ids[])
>>> +{
>>> +    int err;
>>> +
>>> +    if (num_clks > DWCMSHC_MAX_OTHER_CLKS)
>>> +        return -EINVAL;
>>> +
>>> +    for (int i = 0; i < num_clks; i++)
>>> +        priv->other_clks[i].id = clk_ids[i];
>>> +
>>> +    err = devm_clk_bulk_get_optional(dev, num_clks, priv->other_clks);
>> 
>> This leaves a pointer into "priv" in the devres list...
>> 
>>> +    if (err) {
>>> +        dev_err(dev, "failed to get clocks %d\n", err);
>>> +        return err;
>>> +    }
>>> +
>>> +    err = clk_bulk_prepare_enable(num_clks, priv->other_clks);
>>> +    if (err)
>>> +        dev_err(dev, "failed to enable clocks %d\n", err);
>>> +
>>> +    priv->num_other_clks = num_clks;
>>> +
>>> +    return err;
>>> +}
>>> +
>>>   /*
>>>    * If DMA addr spans 128MB boundary, we split the DMA transfer into two
>>>    * so that each DMA transfer doesn't exceed the boundary.
>> [...]
>>> @@ -1280,9 +1300,7 @@ 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);
>>> +    clk_bulk_disable_unprepare(priv->num_other_clks, priv->other_clks);
>>>   free_pltfm:
>>>       sdhci_pltfm_free(pdev);
>> 
>> ...but upon, say, -EPROBE_DEFER from sdhci_setup_host() because a
>> regulator isn't ready yet, that "priv" is freed here, so by the time the
>> devres callbacks eventually run, that "devres->clks" pointer which used
>> to represent "priv->other_clocks" points to who knows what, and this
>> sort of thing happens:
>> 
>> [   12.470827] Unable to handle kernel paging request at virtual address 002df7b378917664
>> [   12.472104] Mem abort info:
>> [   12.472471]   ESR = 0x0000000096000004
>> [   12.475991]   EC = 0x25: DABT (current EL), IL = 32 bits
>> [   12.476657]   SET = 0, FnV = 0
>> [   12.477146]   EA = 0, S1PTW = 0
>> [   12.477547]   FSC = 0x04: level 0 translation fault
>> [   12.478127] Data abort info:
>> [   12.478126] rockchip-gpio fdd60000.gpio: probed /pinctrl/gpio@fdd60000
>> [   12.478413]   ISV = 0, ISS = 0x00000004, ISS2 = 0x00000000
>> [   12.479826]   CM = 0, WnR = 0, TnD = 0, TagAccess = 0
>> [   12.480418]   GCS = 0, Overlay = 0, DirtyBit = 0, Xs = 0
>> [   12.481282] [002df7b378917664] address between user and kernel address ranges
>> [   12.482421] Internal error: Oops: 0000000096000004 [#1]  SMP
>> [   12.482980] Modules linked in: sdhci_of_dwcmshc drm_dp_aux_bus gpio_rockchip(+) drm_display_helper dw_mmc_rockchip drm_client_lib sdhci_pltfm drm_dma_helper fwnode_mdio sdhci dw_mmc_pltf
>> m libphy fixed rockchip_dfi drm_kms_helper cqhci pl330(+) phy_rockchip_naneng_combphy dw_wdt phy_rockchip_snps_pcie3 phy_rockchip_inno_usb2 dw_mmc mdio_bus dwc3 ehci_platform ohci_platform
>> ehci_hcd drm ohci_hcd udc_core io_domain i2c_rk3x usbcore ulpi usb_common
>> [   12.486871] CPU: 0 UID: 0 PID: 64 Comm: kworker/u16:3 Not tainted 6.16-rc7-arm64-cknow #1 PREEMPTLAZY  Debian 6.16~rc7-1
>> [   12.487901] Hardware name: FriendlyElec NanoPi R5S (DT)
>> [   12.488412] Workqueue: async async_run_entry_fn
>> [   12.488879] pstate: 60400009 (nZCv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
>> [   12.489539] pc : __clk_put+0x2c/0x138
>> [   12.489913] lr : __clk_put+0x2c/0x138
>> [   12.490281] sp : ffff800080713b10
>> [   12.490607] x29: ffff800080713b10 x28: ffff0001f001a120 x27: 0000000000000000
>> [   12.491302] x26: ffff0001f98e01a0 x25: 0000000000000000 x24: ffff0001f0f35408
>> [   12.491995] x23: ffffa8da199b4b40 x22: ffff800080713bb0 x21: ffff0001f0f35010
>> [   12.492689] x20: ffff0001f94aafd0 x19: 0a2df7b378917634 x18: 00000000ffffffff
>> [   12.493381] x17: 3d4d455453595342 x16: 555300307075656b x15: ffff0001f4885650
>> [   12.494075] x14: 0000000000000000 x13: ffff0001f025b810 x12: 0000000000008000
>> [   12.494765] x11: ffffa8da1a73ef98 x10: ffffa8da1a460000 x9 : 0000000000000078
>> [   12.495454] x8 : 0000000000000049 x7 : ffffa8da18c2fbe0 x6 : 0000000000000001
>> [   12.496145] x5 : 0000000000000004 x4 : 000000006cb6bb63 x3 : 0000000000000000
>> [   12.496833] x2 : 0000000000000000 x1 : ffff0001f1365ac0 x0 : 0000000000000001
>> [   12.497524] Call trace:
>> [   12.497776]  __clk_put+0x2c/0x138 (P)
>> [   12.498154]  clk_put+0x18/0x30
>> [   12.498471]  clk_bulk_put+0x40/0x68
>> [   12.498825]  devm_clk_bulk_release+0x24/0x40
>> [   12.499248]  release_nodes+0x64/0xa0
>> [   12.499608]  devres_release_all+0x98/0xf8
>> [   12.500004]  device_unbind_cleanup+0x20/0x70
>> [   12.500426]  really_probe+0x1e8/0x3a0
>> [   12.500793]  __driver_probe_device+0x84/0x160
>> [   12.501225]  driver_probe_device+0x44/0x128
>> [   12.501640]  __driver_attach_async_helper+0x5c/0x108
>> [   12.502125]  async_run_entry_fn+0x40/0x180
>> [   12.502535]  process_one_work+0x23c/0x640
>> [   12.502939]  worker_thread+0x1b4/0x360
>> [   12.503315]  kthread+0x150/0x250
>> [   12.503646]  ret_from_fork+0x10/0x20
>> [   12.504015] Code: aa0003f3 b140041f 540006c8 97ffd9c4 (b9403260)
>> [   12.504598] ---[ end trace 0000000000000000 ]---
>> 
>> 
>> TBH I'm not sure what to do as a straight revert seems impractical by
>> now, so we hope someone else might have a good idea.
>
> Presumably the problem has gone away with:
>
> 	commit 91a001a1a0749e5d24606d46ac5dfd4433c00956
> 	Author: Binbin Zhou <zhoubinbin@loongson.cn>
> 	Date:   Sat Jun 7 15:39:01 2025 +0800
>
> 	    mmc: sdhci-of-dwcmshc: Drop the use of sdhci_pltfm_free()
>
> which is in next.
>
> In which case a separate fix is needed for stable.

Adding that patch to my 6.16-rc7 kernel indeed stopped the OOPSies.
Thanks!

Cheers,
  Diederik

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v6 1/8] mmc: sdhci-of-dwcmshc: add common bulk optional clocks support
  2025-07-24 14:33       ` Diederik de Haas
@ 2025-07-24 14:57         ` Adrian Hunter
  2025-07-25 17:03           ` Diederik de Haas
  0 siblings, 1 reply; 22+ messages in thread
From: Adrian Hunter @ 2025-07-24 14:57 UTC (permalink / raw)
  To: Diederik de Haas, Robin Murphy, Chen Wang, aou, conor+dt, guoren,
	inochiama, jszhang, krzysztof.kozlowski+dt, palmer, paul.walmsley,
	robh, ulf.hansson, devicetree, linux-kernel, linux-mmc,
	linux-riscv, chao.wei, haijiao.liu, xiaoguang.xing, tingzhu.wang
  Cc: Chen Wang, Drew Fustini, linux-rockchip

On 24/07/2025 17:33, Diederik de Haas wrote:
> Hi Adrian,
> 
> On Wed Jul 23, 2025 at 7:33 AM CEST, Adrian Hunter wrote:
>> On 22/07/2025 21:33, Robin Murphy wrote:
>>> A bit late for a "review", but Diederik and I have just been
>>> IRC-debugging a crash on RK3568 which by inspection seems to be caused
>>> by this patch:
>>>
>>> On 2024-08-05 10:17 am, Chen Wang wrote:
>>>> From: Chen Wang <unicorn_wang@outlook.com>
>>>>
>>>> In addition to the required core clock and optional
>>>> bus clock, the soc will expand its own clocks, so
>>>> the bulk clock mechanism is abstracted.
>>>>
>>>> Note, I call the bulk clocks as "other clocks" due
>>>> to the bus clock has been called as "optional".
>>>>
>>>> Signed-off-by: Chen Wang <unicorn_wang@outlook.com>
>>>> Tested-by: Drew Fustini <drew@pdp7.com> # TH1520
>>>> Tested-by: Inochi Amaoto <inochiama@outlook.com> # Duo and Huashan Pi
>>>> ---
>>> [...]
>>>> +static int dwcmshc_get_enable_other_clks(struct device *dev,
>>>> +                     struct dwcmshc_priv *priv,
>>>> +                     int num_clks,
>>>> +                     const char * const clk_ids[])
>>>> +{
>>>> +    int err;
>>>> +
>>>> +    if (num_clks > DWCMSHC_MAX_OTHER_CLKS)
>>>> +        return -EINVAL;
>>>> +
>>>> +    for (int i = 0; i < num_clks; i++)
>>>> +        priv->other_clks[i].id = clk_ids[i];
>>>> +
>>>> +    err = devm_clk_bulk_get_optional(dev, num_clks, priv->other_clks);
>>>
>>> This leaves a pointer into "priv" in the devres list...
>>>
>>>> +    if (err) {
>>>> +        dev_err(dev, "failed to get clocks %d\n", err);
>>>> +        return err;
>>>> +    }
>>>> +
>>>> +    err = clk_bulk_prepare_enable(num_clks, priv->other_clks);
>>>> +    if (err)
>>>> +        dev_err(dev, "failed to enable clocks %d\n", err);
>>>> +
>>>> +    priv->num_other_clks = num_clks;
>>>> +
>>>> +    return err;
>>>> +}
>>>> +
>>>>   /*
>>>>    * If DMA addr spans 128MB boundary, we split the DMA transfer into two
>>>>    * so that each DMA transfer doesn't exceed the boundary.
>>> [...]
>>>> @@ -1280,9 +1300,7 @@ 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);
>>>> +    clk_bulk_disable_unprepare(priv->num_other_clks, priv->other_clks);
>>>>   free_pltfm:
>>>>       sdhci_pltfm_free(pdev);
>>>
>>> ...but upon, say, -EPROBE_DEFER from sdhci_setup_host() because a
>>> regulator isn't ready yet, that "priv" is freed here, so by the time the
>>> devres callbacks eventually run, that "devres->clks" pointer which used
>>> to represent "priv->other_clocks" points to who knows what, and this
>>> sort of thing happens:
>>>
>>> [   12.470827] Unable to handle kernel paging request at virtual address 002df7b378917664
>>> [   12.472104] Mem abort info:
>>> [   12.472471]   ESR = 0x0000000096000004
>>> [   12.475991]   EC = 0x25: DABT (current EL), IL = 32 bits
>>> [   12.476657]   SET = 0, FnV = 0
>>> [   12.477146]   EA = 0, S1PTW = 0
>>> [   12.477547]   FSC = 0x04: level 0 translation fault
>>> [   12.478127] Data abort info:
>>> [   12.478126] rockchip-gpio fdd60000.gpio: probed /pinctrl/gpio@fdd60000
>>> [   12.478413]   ISV = 0, ISS = 0x00000004, ISS2 = 0x00000000
>>> [   12.479826]   CM = 0, WnR = 0, TnD = 0, TagAccess = 0
>>> [   12.480418]   GCS = 0, Overlay = 0, DirtyBit = 0, Xs = 0
>>> [   12.481282] [002df7b378917664] address between user and kernel address ranges
>>> [   12.482421] Internal error: Oops: 0000000096000004 [#1]  SMP
>>> [   12.482980] Modules linked in: sdhci_of_dwcmshc drm_dp_aux_bus gpio_rockchip(+) drm_display_helper dw_mmc_rockchip drm_client_lib sdhci_pltfm drm_dma_helper fwnode_mdio sdhci dw_mmc_pltf
>>> m libphy fixed rockchip_dfi drm_kms_helper cqhci pl330(+) phy_rockchip_naneng_combphy dw_wdt phy_rockchip_snps_pcie3 phy_rockchip_inno_usb2 dw_mmc mdio_bus dwc3 ehci_platform ohci_platform
>>> ehci_hcd drm ohci_hcd udc_core io_domain i2c_rk3x usbcore ulpi usb_common
>>> [   12.486871] CPU: 0 UID: 0 PID: 64 Comm: kworker/u16:3 Not tainted 6.16-rc7-arm64-cknow #1 PREEMPTLAZY  Debian 6.16~rc7-1
>>> [   12.487901] Hardware name: FriendlyElec NanoPi R5S (DT)
>>> [   12.488412] Workqueue: async async_run_entry_fn
>>> [   12.488879] pstate: 60400009 (nZCv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
>>> [   12.489539] pc : __clk_put+0x2c/0x138
>>> [   12.489913] lr : __clk_put+0x2c/0x138
>>> [   12.490281] sp : ffff800080713b10
>>> [   12.490607] x29: ffff800080713b10 x28: ffff0001f001a120 x27: 0000000000000000
>>> [   12.491302] x26: ffff0001f98e01a0 x25: 0000000000000000 x24: ffff0001f0f35408
>>> [   12.491995] x23: ffffa8da199b4b40 x22: ffff800080713bb0 x21: ffff0001f0f35010
>>> [   12.492689] x20: ffff0001f94aafd0 x19: 0a2df7b378917634 x18: 00000000ffffffff
>>> [   12.493381] x17: 3d4d455453595342 x16: 555300307075656b x15: ffff0001f4885650
>>> [   12.494075] x14: 0000000000000000 x13: ffff0001f025b810 x12: 0000000000008000
>>> [   12.494765] x11: ffffa8da1a73ef98 x10: ffffa8da1a460000 x9 : 0000000000000078
>>> [   12.495454] x8 : 0000000000000049 x7 : ffffa8da18c2fbe0 x6 : 0000000000000001
>>> [   12.496145] x5 : 0000000000000004 x4 : 000000006cb6bb63 x3 : 0000000000000000
>>> [   12.496833] x2 : 0000000000000000 x1 : ffff0001f1365ac0 x0 : 0000000000000001
>>> [   12.497524] Call trace:
>>> [   12.497776]  __clk_put+0x2c/0x138 (P)
>>> [   12.498154]  clk_put+0x18/0x30
>>> [   12.498471]  clk_bulk_put+0x40/0x68
>>> [   12.498825]  devm_clk_bulk_release+0x24/0x40
>>> [   12.499248]  release_nodes+0x64/0xa0
>>> [   12.499608]  devres_release_all+0x98/0xf8
>>> [   12.500004]  device_unbind_cleanup+0x20/0x70
>>> [   12.500426]  really_probe+0x1e8/0x3a0
>>> [   12.500793]  __driver_probe_device+0x84/0x160
>>> [   12.501225]  driver_probe_device+0x44/0x128
>>> [   12.501640]  __driver_attach_async_helper+0x5c/0x108
>>> [   12.502125]  async_run_entry_fn+0x40/0x180
>>> [   12.502535]  process_one_work+0x23c/0x640
>>> [   12.502939]  worker_thread+0x1b4/0x360
>>> [   12.503315]  kthread+0x150/0x250
>>> [   12.503646]  ret_from_fork+0x10/0x20
>>> [   12.504015] Code: aa0003f3 b140041f 540006c8 97ffd9c4 (b9403260)
>>> [   12.504598] ---[ end trace 0000000000000000 ]---
>>>
>>>
>>> TBH I'm not sure what to do as a straight revert seems impractical by
>>> now, so we hope someone else might have a good idea.
>>
>> Presumably the problem has gone away with:
>>
>> 	commit 91a001a1a0749e5d24606d46ac5dfd4433c00956
>> 	Author: Binbin Zhou <zhoubinbin@loongson.cn>
>> 	Date:   Sat Jun 7 15:39:01 2025 +0800
>>
>> 	    mmc: sdhci-of-dwcmshc: Drop the use of sdhci_pltfm_free()
>>
>> which is in next.
>>
>> In which case a separate fix is needed for stable.
> 
> Adding that patch to my 6.16-rc7 kernel indeed stopped the OOPSies.
> Thanks!

You need the other patches that it depends on, otherwise you are
just leaking the memory.  Refer:

	https://lore.kernel.org/all/cover.1749127796.git.zhoubinbin@loongson.cn/


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v6 1/8] mmc: sdhci-of-dwcmshc: add common bulk optional clocks support
  2025-07-24 14:57         ` Adrian Hunter
@ 2025-07-25 17:03           ` Diederik de Haas
  0 siblings, 0 replies; 22+ messages in thread
From: Diederik de Haas @ 2025-07-25 17:03 UTC (permalink / raw)
  To: Adrian Hunter, Robin Murphy, Chen Wang, aou, conor+dt, guoren,
	inochiama, jszhang, krzysztof.kozlowski+dt, palmer, paul.walmsley,
	robh, ulf.hansson, devicetree, linux-kernel, linux-mmc,
	linux-riscv, chao.wei, haijiao.liu, xiaoguang.xing, tingzhu.wang
  Cc: Chen Wang, Drew Fustini, linux-rockchip

[-- Attachment #1: Type: text/plain, Size: 1707 bytes --]

On Thu Jul 24, 2025 at 4:57 PM CEST, Adrian Hunter wrote:
> On 24/07/2025 17:33, Diederik de Haas wrote:
>> On Wed Jul 23, 2025 at 7:33 AM CEST, Adrian Hunter wrote:
>>> On 22/07/2025 21:33, Robin Murphy wrote:
>>>> A bit late for a "review", but Diederik and I have just been
>>>> IRC-debugging a crash on RK3568 which by inspection seems to be caused
>>>> by this patch:
>>>>
>>>> On 2024-08-05 10:17 am, Chen Wang wrote:
>>>>> From: Chen Wang <unicorn_wang@outlook.com>
>>>>>
>>>>> In addition to the required core clock and optional
>>>>> bus clock, the soc will expand its own clocks, so
>>>>> the bulk clock mechanism is abstracted.
>>>>>
>>>>> Note, I call the bulk clocks as "other clocks" due
>>>>> to the bus clock has been called as "optional".
>>>>>
>>>>> Signed-off-by: Chen Wang <unicorn_wang@outlook.com>
>>>>> Tested-by: Drew Fustini <drew@pdp7.com> # TH1520
>>>>> Tested-by: Inochi Amaoto <inochiama@outlook.com> # Duo and Huashan Pi
>>>>> ---
>>>
>>> Presumably the problem has gone away with:
>>>
>>> 	commit 91a001a1a0749e5d24606d46ac5dfd4433c00956
>>> 	Author: Binbin Zhou <zhoubinbin@loongson.cn>
>>> 	Date:   Sat Jun 7 15:39:01 2025 +0800
>>>
>>> 	    mmc: sdhci-of-dwcmshc: Drop the use of sdhci_pltfm_free()
>>>
>>> which is in next.
>>>
>>> In which case a separate fix is needed for stable.
>> 
>> Adding that patch to my 6.16-rc7 kernel indeed stopped the OOPSies.
>> Thanks!
>
> You need the other patches that it depends on, otherwise you are
> just leaking the memory.  Refer:
>
> 	https://lore.kernel.org/all/cover.1749127796.git.zhoubinbin@loongson.cn/

Also with the other patches, the OOPSies stopped :-)

Cheers,
  Diederik

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

^ permalink raw reply	[flat|nested] 22+ messages in thread

end of thread, other threads:[~2025-07-25 17:04 UTC | newest]

Thread overview: 22+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-08-05  9:15 [PATCH v6 0/8] mmc: sdhci-of-dwcmshc: Add Sophgo SG2042 support Chen Wang
2024-08-05  9:17 ` [PATCH v6 1/8] mmc: sdhci-of-dwcmshc: add common bulk optional clocks support Chen Wang
2024-08-08  7:17   ` Adrian Hunter
2025-07-22 18:33   ` Robin Murphy
2025-07-23  5:33     ` Adrian Hunter
2025-07-24 14:33       ` Diederik de Haas
2025-07-24 14:57         ` Adrian Hunter
2025-07-25 17:03           ` Diederik de Haas
2024-08-05  9:17 ` [PATCH v6 2/8] mmc: sdhci-of-dwcmshc: move two rk35xx functions Chen Wang
2024-08-08  7:18   ` Adrian Hunter
2024-08-05  9:17 ` [PATCH v6 3/8] mmc: sdhci-of-dwcmshc: factor out code for th1520_init() Chen Wang
2024-08-08  7:18   ` Adrian Hunter
2024-08-05  9:18 ` [PATCH v6 4/8] mmc: sdhci-of-dwcmshc: factor out code into dwcmshc_rk35xx_init Chen Wang
2024-08-08  7:19   ` Adrian Hunter
2024-08-05  9:18 ` [PATCH v6 5/8] mmc: sdhci-of-dwcmshc: add dwcmshc_pltfm_data Chen Wang
2024-08-08  7:19   ` Adrian Hunter
2024-08-05  9:19 ` [PATCH v6 6/8] dt-bindings: mmc: sdhci-of-dwcmhsc: Add Sophgo SG2042 support Chen Wang
2024-08-05  9:19 ` [PATCH v6 7/8] mmc: sdhci-of-dwcmshc: Add support for Sophgo SG2042 Chen Wang
2024-08-08  7:19   ` Adrian Hunter
2024-08-05  9:19 ` [PATCH v6 8/8] riscv: sophgo: dts: add mmc controllers for SG2042 SoC Chen Wang
2024-08-20 11:49 ` [PATCH v6 0/8] mmc: sdhci-of-dwcmshc: Add Sophgo SG2042 support Ulf Hansson
2024-08-26  2:59 ` (subset) " Inochi Amaoto

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).