mmc: sdhci-of-dwcmshc: Add Sophgo SG2042 support

From: Chen Wang <[email protected]>

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 <[email protected]>

Tested-by Drew Fustini <[email protected]> # TH1520

Tested-by Inochi Amaoto <[email protected]> # Duo and Huashan Pi

---
 drivers/mmc/host/sdhci-of-dwcmshc.c | 90 +++++++++++++++--------------
 1 file changed, 48 insertions(+), 42 deletions(-)

drivers/mmc/host/sdhci-of-dwcmshc.c +48 -42

index e79aa4b3b6c3..35401616fb2e 100644
@@ -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);

From: Chen Wang <[email protected]>

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 <[email protected]>

Tested-by Drew Fustini <[email protected]> # TH1520

Tested-by Inochi Amaoto <[email protected]> # Duo and Huashan Pi

---

drivers/mmc/host/sdhci-of-dwcmshc.c +4 -0

+static int dwcmshc_get_enable_other_clks(struct device *dev,
+					 struct dwcmshc_priv *priv,
+					 int num_clks,
+					 const char * const clk_ids[])

drivers/mmc/host/sdhci-of-dwcmshc.c +14 -0

+	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.

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:

Quoted 13 lines · Chen Wang

From: Chen Wang <[email protected]>

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 <[email protected]>

Tested-by Drew Fustini <[email protected]> # TH1520

Tested-by Inochi Amaoto <[email protected]> # Duo and Huashan Pi

---

[...]

Quoted 4 lines

drivers/mmc/host/sdhci-of-dwcmshc.c +4 -0

+static int dwcmshc_get_enable_other_clks(struct device *dev,
+                     struct dwcmshc_priv *priv,
+                     int num_clks,
+                     const char * const clk_ids[])

drivers/mmc/host/sdhci-of-dwcmshc.c enum dwcmshc_rk_type { · line 234

Quoted 10 lines

drivers/mmc/host/sdhci-of-dwcmshc.c +10 -0

+{
+    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...

Quoted 17 lines

+    if (err) {
+        dev_err(dev, "failed to get clocks %d\n", err); +        return err;
+    }

drivers/mmc/host/sdhci-of-dwcmshc.c +10 -0

+
+    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.

[...]

Quoted 10 lines

(quoted diff) +0 -0

@@ -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.

Quoted 126 lines · Robin Murphy

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:

Quoted 13 lines · Chen Wang

From: Chen Wang <[email protected]>

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 <[email protected]>

Tested-by Drew Fustini <[email protected]> # TH1520

Tested-by Inochi Amaoto <[email protected]> # Duo and Huashan Pi

---

[...]

Quoted 4 lines

drivers/mmc/host/sdhci-of-dwcmshc.c +4 -0

+static int dwcmshc_get_enable_other_clks(struct device *dev,
+                     struct dwcmshc_priv *priv,
+                     int num_clks,
+                     const char * const clk_ids[])

drivers/mmc/host/sdhci-of-dwcmshc.c enum dwcmshc_rk_type { · line 234

Quoted 10 lines

drivers/mmc/host/sdhci-of-dwcmshc.c +10 -0

+{
+    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...

Quoted 17 lines

+    if (err) {
+        dev_err(dev, "failed to get clocks %d\n", err); +        return err;
+    }

drivers/mmc/host/sdhci-of-dwcmshc.c +10 -0

+
+    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.

[...]

Quoted 10 lines

(quoted diff) +0 -0

@@ -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 ]---

Quoted 2 lines

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 <[email protected]>
	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.

Hi Adrian,

Quoted 136 lines · Adrian Hunter

Quoted 126 lines · Robin Murphy

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:

Quoted 13 lines · Chen Wang

From: Chen Wang <[email protected]>

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 <[email protected]>

Tested-by Drew Fustini <[email protected]> # TH1520

Tested-by Inochi Amaoto <[email protected]> # Duo and Huashan Pi

---

[...]

Quoted 4 lines

drivers/mmc/host/sdhci-of-dwcmshc.c +4 -0

+static int dwcmshc_get_enable_other_clks(struct device *dev,
+                     struct dwcmshc_priv *priv,
+                     int num_clks,
+                     const char * const clk_ids[])

drivers/mmc/host/sdhci-of-dwcmshc.c enum dwcmshc_rk_type { · line 234

Quoted 10 lines

drivers/mmc/host/sdhci-of-dwcmshc.c +10 -0

+{
+    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...

Quoted 17 lines

+    if (err) {
+        dev_err(dev, "failed to get clocks %d\n", err); +        return err;
+    }

drivers/mmc/host/sdhci-of-dwcmshc.c +10 -0

+
+    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.

[...]

Quoted 10 lines

(quoted diff) +0 -0

@@ -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 ]---

Quoted 2 lines

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 <[email protected]>
	Date:   Sat Jun 7 15:39:01 2025 +0800

Quoted 5 lines

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!

Quoted 33 lines · Diederik de Haas

Quoted 31 lines · Adrian Hunter

Quoted 4 lines · Robin Murphy

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:

Quoted 14 lines

Quoted 13 lines · Chen Wang

From: Chen Wang <[email protected]>

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 <[email protected]>

Tested-by Drew Fustini <[email protected]> # TH1520

Tested-by Inochi Amaoto <[email protected]> # Duo and Huashan Pi

---

Presumably the problem has gone away with:

	commit 91a001a1a0749e5d24606d46ac5dfd4433c00956
	Author: Binbin Zhou <[email protected]>
	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.

Quoted 2 lines

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/[email protected]/

From: Chen Wang <[email protected]>

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 <[email protected]>

Tested-by Drew Fustini <[email protected]> # TH1520

Tested-by Inochi Amaoto <[email protected]> # Duo and Huashan Pi

---
 drivers/mmc/host/sdhci-of-dwcmshc.c | 90 ++++++++++++++---------------
 1 file changed, 45 insertions(+), 45 deletions(-)

drivers/mmc/host/sdhci-of-dwcmshc.c +45 -45

index 35401616fb2e..a002636d51fd 100644
@@ -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",

From: Chen Wang <[email protected]>

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 <[email protected]>

Reviewed-by Drew Fustini <[email protected]>

Tested-by Drew Fustini <[email protected]> # TH1520

Tested-by Inochi Amaoto <[email protected]> # Duo and Huashan Pi

---
 drivers/mmc/host/sdhci-of-dwcmshc.c | 51 +++++++++++++++++------------
 1 file changed, 30 insertions(+), 21 deletions(-)

drivers/mmc/host/sdhci-of-dwcmshc.c +30 -21

index a002636d51fd..b272ec2ab232 100644
@@ -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

From: Chen Wang <[email protected]>

Continue factor out code fron probe into dwcmshc_rk35xx_init.

Signed-off-by Chen Wang <[email protected]>

Tested-by Drew Fustini <[email protected]> # TH1520

Tested-by Inochi Amaoto <[email protected]> # Duo and Huashan Pi

---
 drivers/mmc/host/sdhci-of-dwcmshc.c | 34 ++++++++++++++---------------
 1 file changed, 16 insertions(+), 18 deletions(-)

drivers/mmc/host/sdhci-of-dwcmshc.c +16 -18

index b272ec2ab232..55fba5ef37ba 100644
@@ -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);

From: Chen Wang <[email protected]>

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 <[email protected]>

Tested-by Drew Fustini <[email protected]> # TH1520

Tested-by Inochi Amaoto <[email protected]> # Duo and Huashan Pi

---
 drivers/mmc/host/sdhci-of-dwcmshc.c | 81 +++++++++++++++++------------
 1 file changed, 48 insertions(+), 33 deletions(-)

drivers/mmc/host/sdhci-of-dwcmshc.c +48 -33

index 55fba5ef37ba..16f420994519 100644
@@ -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)

From: Chen Wang <[email protected]>

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 <[email protected]>

---
 drivers/mmc/host/sdhci-of-dwcmshc.c | 125 ++++++++++++++++++++++++++--
 1 file changed, 118 insertions(+), 7 deletions(-)

drivers/mmc/host/sdhci-of-dwcmshc.c +118 -7

index 16f420994519..ba8960d8b2d4 100644
@@ -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);

From: Chen Wang <[email protected]>

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.

From: Chen Wang <[email protected]>

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.

[...]