[PATCH v4 1/3] dt-bindings: mmc: sdhci-of-dwcmhsc: Convert to yaml file

[PATCH v4 1/3] dt-bindings: mmc: sdhci-of-dwcmhsc: Convert to yaml file

[Shawn Lin]

This patch converts sdhci-of-dwcmshc.txt to sdhci-of-dwcmshc.yaml

Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
Reviewed-by: Rob Herring <robh@kernel.org>
---

Changes in v4:
- add tag from Rob
Changes in v3:
- fix filename and other improvments suggested by Rob

 .../devicetree/bindings/mmc/sdhci-of-dwcmshc.txt   | 20 -------
 .../bindings/mmc/snps,dwcmshc-sdhci.yaml           | 63 ++++++++++++++++++++++
 2 files changed, 63 insertions(+), 20 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/mmc/sdhci-of-dwcmshc.txt
 create mode 100644 Documentation/devicetree/bindings/mmc/snps,dwcmshc-sdhci.yaml

diff --git a/Documentation/devicetree/bindings/mmc/sdhci-of-dwcmshc.txt b/Documentation/devicetree/bindings/mmc/sdhci-of-dwcmshc.txt
deleted file mode 100644
index ee4253b..0000000
--- a/Documentation/devicetree/bindings/mmc/sdhci-of-dwcmshc.txt
+++ /dev/null
@@ -1,20 +0,0 @@
-* Synopsys DesignWare Cores Mobile Storage Host Controller
-
-Required properties:
-- compatible: should be one of the following:
-    "snps,dwcmshc-sdhci"
-- reg: offset and length of the register set for the device.
-- interrupts: a single interrupt specifier.
-- clocks: Array of clocks required for SDHCI; requires at least one for
-    core clock.
-- clock-names: Array of names corresponding to clocks property; shall be
-    "core" for core clock and "bus" for optional bus clock.
-
-Example:
-	sdhci2: sdhci@aa0000 {
-		compatible = "snps,dwcmshc-sdhci";
-		reg = <0xaa0000 0x1000>;
-		interrupts = <GIC_SPI 21 IRQ_TYPE_LEVEL_HIGH>;
-		clocks = <&emmcclk>;
-		bus-width = <8>;
-	}
diff --git a/Documentation/devicetree/bindings/mmc/snps,dwcmshc-sdhci.yaml b/Documentation/devicetree/bindings/mmc/snps,dwcmshc-sdhci.yaml
new file mode 100644
index 0000000..f99fb9f
--- /dev/null
+++ b/Documentation/devicetree/bindings/mmc/snps,dwcmshc-sdhci.yaml
@@ -0,0 +1,63 @@
+# SPDX-License-Identifier: GPL-2.0-only
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/mmc/snps,dwcmshc-sdhci.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Synopsys Designware Mobile Storage Host Controller Binding
+
+maintainers:
+  - Ulf Hansson <ulf.hansson@linaro.org>
+  - Jisheng Zhang <Jisheng.Zhang@synaptics.com>
+
+allOf:
+  - $ref: mmc-controller.yaml#
+
+properties:
+  compatible:
+    enum:
+      - snps,dwcmshc-sdhci
+
+  reg:
+    minItems: 1
+    items:
+      - description: Offset and length of the register set for the device
+
+  interrupts:
+    maxItems: 1
+
+  clocks:
+    minItems: 1
+    items:
+      - description: core clock
+      - description: bus clock for optional
+
+  clock-names:
+    minItems: 1
+    items:
+      - const: core
+      - const: bus
+
+required:
+  - compatible
+  - reg
+  - interrupts
+  - clocks
+  - clock-names
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    mmc@aa0000 {
+      compatible = "snps,dwcmshc-sdhci";
+      reg = <0xaa000 0x1000>;
+      interrupts = <0 25 0x4>;
+      clocks = <&cru 17>, <&cru 18>;
+      clock-names = "core", "bus";
+      bus-width = <8>;
+      #address-cells = <1>;
+      #size-cells = <0>;
+    };
+
+...
-- 
2.7.4

[PATCH v4 2/3] dt-bindings: mmc: sdhci-of-dwcmhsc: Add rockchip support

[Shawn Lin]

This patch adds rockchip support in sdhci-of-dwcmhsc.yaml

Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
---

Changes in v4:
- rename compatible to rockchip,rk3568-dwcmshc
- constrains rockchip,txclk-tapnum to u8 to match the register map

 .../bindings/mmc/snps,dwcmshc-sdhci.yaml           | 24 ++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/Documentation/devicetree/bindings/mmc/snps,dwcmshc-sdhci.yaml b/Documentation/devicetree/bindings/mmc/snps,dwcmshc-sdhci.yaml
index f99fb9f..e6c9a2f 100644
--- a/Documentation/devicetree/bindings/mmc/snps,dwcmshc-sdhci.yaml
+++ b/Documentation/devicetree/bindings/mmc/snps,dwcmshc-sdhci.yaml
@@ -16,6 +16,7 @@ allOf:
 properties:
   compatible:
     enum:
+      - rockchip,rk3568-dwcmshc
       - snps,dwcmshc-sdhci
 
   reg:
@@ -31,12 +32,24 @@ properties:
     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
+
+  rockchip,txclk-tapnum:
+    description: Specify the number of delay for tx sampling.
+    $ref: /schemas/types.yaml#/definitions/uint8
+
 
 required:
   - compatible
@@ -49,6 +62,17 @@ unevaluatedProperties: false
 
 examples:
   - |
+    mmc@fe310000 {
+      compatible = "rockchip,rk3568-dwcmshc";
+      reg = <0xfe310000 0x10000>;
+      interrupts = <0 25 0x4>;
+      clocks = <&cru 17>, <&cru 18>, <&cru 19>, <&cru 20>, <&cru 21>;
+      clock-names = "core", "bus", "axi", "block", "timer";
+      bus-width = <8>;
+      #address-cells = <1>;
+      #size-cells = <0>;
+    };
+  - |
     mmc@aa0000 {
       compatible = "snps,dwcmshc-sdhci";
       reg = <0xaa000 0x1000>;
-- 
2.7.4

[PATCH v4 3/3] mmc: sdhci-of-dwcmshc: add rockchip platform support

[Shawn Lin]

sdhci based synopsys MMC IP is also used on some rockchip platforms,
so add a basic support here.

Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
Acked-by: Adrian Hunter <adrian.hunter@intel.com>
---

Changes in v4:
- add comments for disabling rx invert
- add tag from Adrian

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

diff --git a/drivers/mmc/host/sdhci-of-dwcmshc.c b/drivers/mmc/host/sdhci-of-dwcmshc.c
index 59d8d96..dabc1ec 100644
--- a/drivers/mmc/host/sdhci-of-dwcmshc.c
+++ b/drivers/mmc/host/sdhci-of-dwcmshc.c
@@ -9,9 +9,11 @@
 
 #include <linux/clk.h>
 #include <linux/dma-mapping.h>
+#include <linux/iopoll.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/of.h>
+#include <linux/of_device.h>
 #include <linux/sizes.h>
 
 #include "sdhci-pltfm.h"
@@ -21,11 +23,43 @@
 /* DWCMSHC specific Mode Select value */
 #define DWCMSHC_CTRL_HS400		0x7
 
+/* Rockchip specific Registers */
+#define DWCMSHC_HOST_CTRL3		0x508
+#define DWCMSHC_EMMC_CONTROL		0x52c
+#define DWCMSHC_EMMC_ATCTRL		0x540
+#define DWCMSHC_EMMC_DLL_CTRL		0x800
+#define DWCMSHC_EMMC_DLL_RXCLK		0x804
+#define DWCMSHC_EMMC_DLL_TXCLK		0x808
+#define DWCMSHC_EMMC_DLL_STRBIN		0x80c
+#define DWCMSHC_EMMC_DLL_STATUS0	0x840
+#define DWCMSHC_EMMC_DLL_START		BIT(0)
+#define DWCMSHC_EMMC_DLL_RXCLK_SRCSEL	29
+#define DWCMSHC_EMMC_DLL_START_POINT	16
+#define DWCMSHC_EMMC_DLL_INC		8
+#define DWCMSHC_EMMC_DLL_DLYENA		BIT(27)
+#define DLL_TXCLK_TAPNUM_DEFAULT	0x8
+#define DLL_STRBIN_TAPNUM_DEFAULT	0x8
+#define DLL_TXCLK_TAPNUM_FROM_SW	BIT(24)
+#define DLL_STRBIN_TAPNUM_FROM_SW	BIT(24)
+#define DWCMSHC_EMMC_DLL_LOCKED		BIT(8)
+#define DWCMSHC_EMMC_DLL_TIMEOUT	BIT(9)
+#define DLL_RXCLK_NO_INVERTER		1
+#define DLL_RXCLK_INVERTER		0
+#define DWCMSHC_ENHANCED_STROBE		BIT(8)
+#define DLL_LOCK_WO_TMOUT(x) \
+	((((x) & DWCMSHC_EMMC_DLL_LOCKED) == DWCMSHC_EMMC_DLL_LOCKED) && \
+	(((x) & DWCMSHC_EMMC_DLL_TIMEOUT) == 0))
+#define ROCKCHIP_MAX_CLKS		3
+
 #define BOUNDARY_OK(addr, len) \
 	((addr | (SZ_128M - 1)) == ((addr + len - 1) | (SZ_128M - 1)))
 
 struct dwcmshc_priv {
 	struct clk	*bus_clk;
+
+	/* Rockchip specified optional clocks */
+	struct clk_bulk_data rockchip_clks[ROCKCHIP_MAX_CLKS];
+	u8 txclk_tapnum;
 };
 
 /*
@@ -100,6 +134,102 @@ static void dwcmshc_set_uhs_signaling(struct sdhci_host *host,
 	sdhci_writew(host, ctrl_2, SDHCI_HOST_CONTROL2);
 }
 
+static void dwcmshc_rk_hs400_enhanced_strobe(struct mmc_host *mmc,
+					     struct mmc_ios *ios)
+{
+	u32 vendor;
+	struct sdhci_host *host = mmc_priv(mmc);
+
+	vendor = sdhci_readl(host, DWCMSHC_EMMC_CONTROL);
+	if (ios->enhanced_strobe)
+		vendor |= DWCMSHC_ENHANCED_STROBE;
+	else
+		vendor &= ~DWCMSHC_ENHANCED_STROBE;
+
+	sdhci_writel(host, vendor, DWCMSHC_EMMC_CONTROL);
+}
+
+static void dwcmshc_rk_set_clock(struct sdhci_host *host, unsigned int clock)
+{
+	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
+	struct dwcmshc_priv *priv = sdhci_pltfm_priv(pltfm_host);
+	u8 txclk_tapnum = DLL_TXCLK_TAPNUM_DEFAULT;
+	u32 extra;
+	int err;
+
+	host->mmc->actual_clock = 0;
+
+	/*
+	 * DO NOT TOUCH THIS SETTING. RX clk inverter unit is enabled
+	 * by default, but it shouldn't be enabled. We should anyway
+	 * disable it before issuing any cmds.
+	 */
+	extra = DWCMSHC_EMMC_DLL_DLYENA |
+		DLL_RXCLK_NO_INVERTER << DWCMSHC_EMMC_DLL_RXCLK_SRCSEL;
+	sdhci_writel(host, extra, DWCMSHC_EMMC_DLL_RXCLK);
+
+	if (clock == 0)
+		return;
+
+	/* Rockchip platform only support 375KHz for identify mode */
+	if (clock <= 400000)
+		clock = 375000;
+
+	err = clk_set_rate(pltfm_host->clk, clock);
+	if (err)
+		dev_err(mmc_dev(host->mmc), "fail to set clock %d", clock);
+
+	sdhci_set_clock(host, clock);
+
+	/* Disable cmd conflict check */
+	extra = sdhci_readl(host, DWCMSHC_HOST_CTRL3);
+	extra &= ~BIT(0);
+	sdhci_writel(host, extra, DWCMSHC_HOST_CTRL3);
+
+	if (clock <= 400000) {
+		/* Disable DLL to reset sample clock */
+		sdhci_writel(host, 0, DWCMSHC_EMMC_DLL_CTRL);
+		return;
+	}
+
+	/* Reset DLL */
+	sdhci_writel(host, BIT(1), DWCMSHC_EMMC_DLL_CTRL);
+	udelay(1);
+	sdhci_writel(host, 0x0, DWCMSHC_EMMC_DLL_CTRL);
+
+	/* Init DLL settings */
+	extra = 0x5 << DWCMSHC_EMMC_DLL_START_POINT |
+		0x2 << DWCMSHC_EMMC_DLL_INC |
+		DWCMSHC_EMMC_DLL_START;
+	sdhci_writel(host, extra, DWCMSHC_EMMC_DLL_CTRL);
+	err = readl_poll_timeout(host->ioaddr + DWCMSHC_EMMC_DLL_STATUS0,
+				 extra, DLL_LOCK_WO_TMOUT(extra), 1,
+				 500 * USEC_PER_MSEC);
+	if (err) {
+		dev_err(mmc_dev(host->mmc), "DLL lock timeout!\n");
+		return;
+	}
+
+	extra = 0x1 << 16 | /* tune clock stop en */
+		0x2 << 17 | /* pre-change delay */
+		0x3 << 19;  /* post-change delay */
+	sdhci_writel(host, extra, DWCMSHC_EMMC_ATCTRL);
+
+	if (host->mmc->ios.timing == MMC_TIMING_MMC_HS200 ||
+	    host->mmc->ios.timing == MMC_TIMING_MMC_HS400)
+		txclk_tapnum = priv->txclk_tapnum;
+
+	extra = DWCMSHC_EMMC_DLL_DLYENA |
+		DLL_TXCLK_TAPNUM_FROM_SW |
+		txclk_tapnum;
+	sdhci_writel(host, extra, DWCMSHC_EMMC_DLL_TXCLK);
+
+	extra = DWCMSHC_EMMC_DLL_DLYENA |
+		DLL_STRBIN_TAPNUM_DEFAULT |
+		DLL_STRBIN_TAPNUM_FROM_SW;
+	sdhci_writel(host, extra, DWCMSHC_EMMC_DLL_STRBIN);
+}
+
 static const struct sdhci_ops sdhci_dwcmshc_ops = {
 	.set_clock		= sdhci_set_clock,
 	.set_bus_width		= sdhci_set_bus_width,
@@ -109,21 +239,91 @@ static const struct sdhci_ops sdhci_dwcmshc_ops = {
 	.adma_write_desc	= dwcmshc_adma_write_desc,
 };
 
+static const struct sdhci_ops sdhci_dwcmshc_rk_ops = {
+	.set_clock		= dwcmshc_rk_set_clock,
+	.set_bus_width		= sdhci_set_bus_width,
+	.set_uhs_signaling	= dwcmshc_set_uhs_signaling,
+	.get_max_clock		= sdhci_pltfm_clk_get_max_clock,
+	.reset			= sdhci_reset,
+	.adma_write_desc	= dwcmshc_adma_write_desc,
+};
+
 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 sdhci_pltfm_data sdhci_dwcmshc_rk_pdata = {
+	.ops = &sdhci_dwcmshc_rk_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 int rockchip_pltf_init(struct sdhci_host *host, struct dwcmshc_priv *priv)
+{
+	int 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), ROCKCHIP_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(ROCKCHIP_MAX_CLKS, priv->rockchip_clks);
+	if (err) {
+		dev_err(mmc_dev(host->mmc), "failed to enable clocks %d\n", err);
+		return err;
+	}
+
+	if (of_property_read_u8(mmc_dev(host->mmc)->of_node, "rockchip,txclk-tapnum",
+				&priv->txclk_tapnum))
+		priv->txclk_tapnum = DLL_TXCLK_TAPNUM_DEFAULT;
+
+	/* Disable cmd conflict check */
+	sdhci_writel(host, 0x0, 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 const struct of_device_id sdhci_dwcmshc_dt_ids[] = {
+	{
+		.compatible = "snps,dwcmshc-sdhci",
+		.data = &sdhci_dwcmshc_pdata,
+	},
+	{
+		.compatible = "rockchip,dwcmshc-sdhci",
+		.data = &sdhci_dwcmshc_rk_pdata,
+	},
+	{},
+};
+MODULE_DEVICE_TABLE(of, sdhci_dwcmshc_dt_ids);
+
 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;
 	int err;
 	u32 extra;
 
-	host = sdhci_pltfm_init(pdev, &sdhci_dwcmshc_pdata,
+	pltfm_data = of_device_get_match_data(&pdev->dev);
+	if (!pltfm_data) {
+		dev_err(&pdev->dev, "Error: No device match data found\n");
+		return -ENODEV;
+	}
+
+	host = sdhci_pltfm_init(pdev, pltfm_data,
 				sizeof(struct dwcmshc_priv));
 	if (IS_ERR(host))
 		return PTR_ERR(host);
@@ -161,6 +361,15 @@ static int dwcmshc_probe(struct platform_device *pdev)
 
 	host->mmc_host_ops.request = dwcmshc_request;
 
+	if (pltfm_data == &sdhci_dwcmshc_rk_pdata) {
+		host->mmc_host_ops.hs400_enhanced_strobe =
+			dwcmshc_rk_hs400_enhanced_strobe;
+
+		err = rockchip_pltf_init(host, priv);
+		if (err)
+			goto err_clk;
+	}
+
 	err = sdhci_add_host(host);
 	if (err)
 		goto err_clk;
@@ -170,6 +379,7 @@ static int dwcmshc_probe(struct platform_device *pdev)
 err_clk:
 	clk_disable_unprepare(pltfm_host->clk);
 	clk_disable_unprepare(priv->bus_clk);
+	clk_bulk_disable_unprepare(ROCKCHIP_MAX_CLKS, priv->rockchip_clks);
 free_pltfm:
 	sdhci_pltfm_free(pdev);
 	return err;
@@ -185,6 +395,7 @@ static int dwcmshc_remove(struct platform_device *pdev)
 
 	clk_disable_unprepare(pltfm_host->clk);
 	clk_disable_unprepare(priv->bus_clk);
+	clk_bulk_disable_unprepare(ROCKCHIP_MAX_CLKS, priv->rockchip_clks);
 
 	sdhci_pltfm_free(pdev);
 
@@ -207,6 +418,8 @@ static int dwcmshc_suspend(struct device *dev)
 	if (!IS_ERR(priv->bus_clk))
 		clk_disable_unprepare(priv->bus_clk);
 
+	clk_bulk_disable_unprepare(ROCKCHIP_MAX_CLKS, priv->rockchip_clks);
+
 	return ret;
 }
 
@@ -227,18 +440,16 @@ static int dwcmshc_resume(struct device *dev)
 			return ret;
 	}
 
+	ret = clk_bulk_prepare_enable(ROCKCHIP_MAX_CLKS, priv->rockchip_clks);
+	if (ret)
+		return ret;
+
 	return sdhci_resume_host(host);
 }
 #endif
 
 static SIMPLE_DEV_PM_OPS(dwcmshc_pmops, dwcmshc_suspend, dwcmshc_resume);
 
-static const struct of_device_id sdhci_dwcmshc_dt_ids[] = {
-	{ .compatible = "snps,dwcmshc-sdhci" },
-	{}
-};
-MODULE_DEVICE_TABLE(of, sdhci_dwcmshc_dt_ids);
-
 static struct platform_driver sdhci_dwcmshc_driver = {
 	.driver	= {
 		.name	= "sdhci-dwcmshc",
-- 
2.7.4

Re: [PATCH v4 3/3] mmc: sdhci-of-dwcmshc: add rockchip platform support

[Jisheng Zhang]

Hi Shawn,

On Thu, 11 Mar 2021 14:21:24 +0800 Shawn Lin <shawn.lin@rock-chips.com> wrote:

> 
> sdhci based synopsys MMC IP is also used on some rockchip platforms,
> so add a basic support here.
> 
> Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
> Acked-by: Adrian Hunter <adrian.hunter@intel.com>
> ---
> 
> Changes in v4:
> - add comments for disabling rx invert
> - add tag from Adrian
> 
>  drivers/mmc/host/sdhci-of-dwcmshc.c | 225 ++++++++++++++++++++++++++++++++++--
>  1 file changed, 218 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/mmc/host/sdhci-of-dwcmshc.c b/drivers/mmc/host/sdhci-of-dwcmshc.c
> index 59d8d96..dabc1ec 100644
> --- a/drivers/mmc/host/sdhci-of-dwcmshc.c
> +++ b/drivers/mmc/host/sdhci-of-dwcmshc.c
> @@ -9,9 +9,11 @@
> 
>  #include <linux/clk.h>
>  #include <linux/dma-mapping.h>
> +#include <linux/iopoll.h>
>  #include <linux/kernel.h>
>  #include <linux/module.h>
>  #include <linux/of.h>
> +#include <linux/of_device.h>
>  #include <linux/sizes.h>
> 
>  #include "sdhci-pltfm.h"
> @@ -21,11 +23,43 @@
>  /* DWCMSHC specific Mode Select value */
>  #define DWCMSHC_CTRL_HS400             0x7
> 
> +/* Rockchip specific Registers */
> +#define DWCMSHC_HOST_CTRL3             0x508

Maybe 0x500 can be read from VENDOR_PTR_R while 0x8 is the offset?

> +#define DWCMSHC_EMMC_CONTROL           0x52c
> +#define DWCMSHC_EMMC_ATCTRL            0x540
> +#define DWCMSHC_EMMC_DLL_CTRL          0x800

Maybe 0x800 is phy offset? while 0, 4, 8 etc. below are phy registers?

> +#define DWCMSHC_EMMC_DLL_RXCLK         0x804
> +#define DWCMSHC_EMMC_DLL_TXCLK         0x808
> +#define DWCMSHC_EMMC_DLL_STRBIN                0x80c
> +#define DWCMSHC_EMMC_DLL_STATUS0       0x840
> +#define DWCMSHC_EMMC_DLL_START         BIT(0)
> +#define DWCMSHC_EMMC_DLL_RXCLK_SRCSEL  29
> +#define DWCMSHC_EMMC_DLL_START_POINT   16
> +#define DWCMSHC_EMMC_DLL_INC           8
> +#define DWCMSHC_EMMC_DLL_DLYENA                BIT(27)
> +#define DLL_TXCLK_TAPNUM_DEFAULT       0x8
> +#define DLL_STRBIN_TAPNUM_DEFAULT      0x8
> +#define DLL_TXCLK_TAPNUM_FROM_SW       BIT(24)
> +#define DLL_STRBIN_TAPNUM_FROM_SW      BIT(24)
> +#define DWCMSHC_EMMC_DLL_LOCKED                BIT(8)
> +#define DWCMSHC_EMMC_DLL_TIMEOUT       BIT(9)
> +#define DLL_RXCLK_NO_INVERTER          1
> +#define DLL_RXCLK_INVERTER             0
> +#define DWCMSHC_ENHANCED_STROBE                BIT(8)

Is it better to let bits' definition grouped with register together, e.g this
definition could be moved to the corresponding CONTROL register MACRO

> +#define DLL_LOCK_WO_TMOUT(x) \
> +       ((((x) & DWCMSHC_EMMC_DLL_LOCKED) == DWCMSHC_EMMC_DLL_LOCKED) && \
> +       (((x) & DWCMSHC_EMMC_DLL_TIMEOUT) == 0))
> +#define ROCKCHIP_MAX_CLKS              3
> +
>  #define BOUNDARY_OK(addr, len) \
>         ((addr | (SZ_128M - 1)) == ((addr + len - 1) | (SZ_128M - 1)))
> 
>  struct dwcmshc_priv {
>         struct clk      *bus_clk;
> +
> +       /* Rockchip specified optional clocks */
> +       struct clk_bulk_data rockchip_clks[ROCKCHIP_MAX_CLKS];
> +       u8 txclk_tapnum;
>  };
> 
>  /*
> @@ -100,6 +134,102 @@ static void dwcmshc_set_uhs_signaling(struct sdhci_host *host,
>         sdhci_writew(host, ctrl_2, SDHCI_HOST_CONTROL2);
>  }
> 
> +static void dwcmshc_rk_hs400_enhanced_strobe(struct mmc_host *mmc,
> +                                            struct mmc_ios *ios)
> +{
> +       u32 vendor;
> +       struct sdhci_host *host = mmc_priv(mmc);
> +
> +       vendor = sdhci_readl(host, DWCMSHC_EMMC_CONTROL);
> +       if (ios->enhanced_strobe)
> +               vendor |= DWCMSHC_ENHANCED_STROBE;
> +       else
> +               vendor &= ~DWCMSHC_ENHANCED_STROBE;
> +
> +       sdhci_writel(host, vendor, DWCMSHC_EMMC_CONTROL);
> +}
> +
> +static void dwcmshc_rk_set_clock(struct sdhci_host *host, unsigned int clock)
> +{
> +       struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> +       struct dwcmshc_priv *priv = sdhci_pltfm_priv(pltfm_host);
> +       u8 txclk_tapnum = DLL_TXCLK_TAPNUM_DEFAULT;
> +       u32 extra;
> +       int err;
> +
> +       host->mmc->actual_clock = 0;
> +
> +       /*
> +        * DO NOT TOUCH THIS SETTING. RX clk inverter unit is enabled
> +        * by default, but it shouldn't be enabled. We should anyway
> +        * disable it before issuing any cmds.
> +        */
> +       extra = DWCMSHC_EMMC_DLL_DLYENA |
> +               DLL_RXCLK_NO_INVERTER << DWCMSHC_EMMC_DLL_RXCLK_SRCSEL;
> +       sdhci_writel(host, extra, DWCMSHC_EMMC_DLL_RXCLK);
> +
> +       if (clock == 0)
> +               return;
> +
> +       /* Rockchip platform only support 375KHz for identify mode */
> +       if (clock <= 400000)
> +               clock = 375000;
> +
> +       err = clk_set_rate(pltfm_host->clk, clock);
> +       if (err)
> +               dev_err(mmc_dev(host->mmc), "fail to set clock %d", clock);
> +
> +       sdhci_set_clock(host, clock);
> +
> +       /* Disable cmd conflict check */
> +       extra = sdhci_readl(host, DWCMSHC_HOST_CTRL3);
> +       extra &= ~BIT(0);
> +       sdhci_writel(host, extra, DWCMSHC_HOST_CTRL3);
> +
> +       if (clock <= 400000) {
> +               /* Disable DLL to reset sample clock */
> +               sdhci_writel(host, 0, DWCMSHC_EMMC_DLL_CTRL);
> +               return;
> +       }
> +
> +       /* Reset DLL */
> +       sdhci_writel(host, BIT(1), DWCMSHC_EMMC_DLL_CTRL);
> +       udelay(1);
> +       sdhci_writel(host, 0x0, DWCMSHC_EMMC_DLL_CTRL);
> +
> +       /* Init DLL settings */
> +       extra = 0x5 << DWCMSHC_EMMC_DLL_START_POINT |
> +               0x2 << DWCMSHC_EMMC_DLL_INC |
> +               DWCMSHC_EMMC_DLL_START;
> +       sdhci_writel(host, extra, DWCMSHC_EMMC_DLL_CTRL);
> +       err = readl_poll_timeout(host->ioaddr + DWCMSHC_EMMC_DLL_STATUS0,
> +                                extra, DLL_LOCK_WO_TMOUT(extra), 1,
> +                                500 * USEC_PER_MSEC);
> +       if (err) {
> +               dev_err(mmc_dev(host->mmc), "DLL lock timeout!\n");
> +               return;
> +       }
> +
> +       extra = 0x1 << 16 | /* tune clock stop en */
> +               0x2 << 17 | /* pre-change delay */
> +               0x3 << 19;  /* post-change delay */
> +       sdhci_writel(host, extra, DWCMSHC_EMMC_ATCTRL);
> +
> +       if (host->mmc->ios.timing == MMC_TIMING_MMC_HS200 ||
> +           host->mmc->ios.timing == MMC_TIMING_MMC_HS400)
> +               txclk_tapnum = priv->txclk_tapnum;
> +
> +       extra = DWCMSHC_EMMC_DLL_DLYENA |
> +               DLL_TXCLK_TAPNUM_FROM_SW |
> +               txclk_tapnum;
> +       sdhci_writel(host, extra, DWCMSHC_EMMC_DLL_TXCLK);
> +
> +       extra = DWCMSHC_EMMC_DLL_DLYENA |
> +               DLL_STRBIN_TAPNUM_DEFAULT |
> +               DLL_STRBIN_TAPNUM_FROM_SW;
> +       sdhci_writel(host, extra, DWCMSHC_EMMC_DLL_STRBIN);
> +}
> +
>  static const struct sdhci_ops sdhci_dwcmshc_ops = {
>         .set_clock              = sdhci_set_clock,
>         .set_bus_width          = sdhci_set_bus_width,
> @@ -109,21 +239,91 @@ static const struct sdhci_ops sdhci_dwcmshc_ops = {
>         .adma_write_desc        = dwcmshc_adma_write_desc,
>  };
> 
> +static const struct sdhci_ops sdhci_dwcmshc_rk_ops = {
> +       .set_clock              = dwcmshc_rk_set_clock,
> +       .set_bus_width          = sdhci_set_bus_width,
> +       .set_uhs_signaling      = dwcmshc_set_uhs_signaling,
> +       .get_max_clock          = sdhci_pltfm_clk_get_max_clock,
> +       .reset                  = sdhci_reset,
> +       .adma_write_desc        = dwcmshc_adma_write_desc,
> +};
> +
>  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 sdhci_pltfm_data sdhci_dwcmshc_rk_pdata = {
> +       .ops = &sdhci_dwcmshc_rk_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 int rockchip_pltf_init(struct sdhci_host *host, struct dwcmshc_priv *priv)
> +{
> +       int 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), ROCKCHIP_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(ROCKCHIP_MAX_CLKS, priv->rockchip_clks);
> +       if (err) {
> +               dev_err(mmc_dev(host->mmc), "failed to enable clocks %d\n", err);
> +               return err;
> +       }
> +
> +       if (of_property_read_u8(mmc_dev(host->mmc)->of_node, "rockchip,txclk-tapnum",
> +                               &priv->txclk_tapnum))
> +               priv->txclk_tapnum = DLL_TXCLK_TAPNUM_DEFAULT;
> +
> +       /* Disable cmd conflict check */
> +       sdhci_writel(host, 0x0, 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 const struct of_device_id sdhci_dwcmshc_dt_ids[] = {
> +       {
> +               .compatible = "snps,dwcmshc-sdhci",
> +               .data = &sdhci_dwcmshc_pdata,
> +       },
> +       {
> +               .compatible = "rockchip,dwcmshc-sdhci",
> +               .data = &sdhci_dwcmshc_rk_pdata,
> +       },
> +       {},
> +};
> +MODULE_DEVICE_TABLE(of, sdhci_dwcmshc_dt_ids);
> +
>  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;
>         int err;
>         u32 extra;
> 
> -       host = sdhci_pltfm_init(pdev, &sdhci_dwcmshc_pdata,
> +       pltfm_data = of_device_get_match_data(&pdev->dev);
> +       if (!pltfm_data) {
> +               dev_err(&pdev->dev, "Error: No device match data found\n");
> +               return -ENODEV;
> +       }
> +
> +       host = sdhci_pltfm_init(pdev, pltfm_data,
>                                 sizeof(struct dwcmshc_priv));
>         if (IS_ERR(host))
>                 return PTR_ERR(host);
> @@ -161,6 +361,15 @@ static int dwcmshc_probe(struct platform_device *pdev)
> 
>         host->mmc_host_ops.request = dwcmshc_request;
> 
> +       if (pltfm_data == &sdhci_dwcmshc_rk_pdata) {
> +               host->mmc_host_ops.hs400_enhanced_strobe =
> +                       dwcmshc_rk_hs400_enhanced_strobe;
> +
> +               err = rockchip_pltf_init(host, priv);
> +               if (err)
> +                       goto err_clk;
> +       }
> +
>         err = sdhci_add_host(host);
>         if (err)
>                 goto err_clk;
> @@ -170,6 +379,7 @@ static int dwcmshc_probe(struct platform_device *pdev)
>  err_clk:
>         clk_disable_unprepare(pltfm_host->clk);
>         clk_disable_unprepare(priv->bus_clk);
> +       clk_bulk_disable_unprepare(ROCKCHIP_MAX_CLKS, priv->rockchip_clks);
>  free_pltfm:
>         sdhci_pltfm_free(pdev);
>         return err;
> @@ -185,6 +395,7 @@ static int dwcmshc_remove(struct platform_device *pdev)
> 
>         clk_disable_unprepare(pltfm_host->clk);
>         clk_disable_unprepare(priv->bus_clk);
> +       clk_bulk_disable_unprepare(ROCKCHIP_MAX_CLKS, priv->rockchip_clks);
> 
>         sdhci_pltfm_free(pdev);
> 
> @@ -207,6 +418,8 @@ static int dwcmshc_suspend(struct device *dev)
>         if (!IS_ERR(priv->bus_clk))
>                 clk_disable_unprepare(priv->bus_clk);
> 
> +       clk_bulk_disable_unprepare(ROCKCHIP_MAX_CLKS, priv->rockchip_clks);
> +
>         return ret;
>  }
> 
> @@ -227,18 +440,16 @@ static int dwcmshc_resume(struct device *dev)
>                         return ret;
>         }
> 
> +       ret = clk_bulk_prepare_enable(ROCKCHIP_MAX_CLKS, priv->rockchip_clks);
> +       if (ret)
> +               return ret;
> +
>         return sdhci_resume_host(host);
>  }
>  #endif
> 
>  static SIMPLE_DEV_PM_OPS(dwcmshc_pmops, dwcmshc_suspend, dwcmshc_resume);
> 
> -static const struct of_device_id sdhci_dwcmshc_dt_ids[] = {
> -       { .compatible = "snps,dwcmshc-sdhci" },
> -       {}
> -};
> -MODULE_DEVICE_TABLE(of, sdhci_dwcmshc_dt_ids);
> -
>  static struct platform_driver sdhci_dwcmshc_driver = {
>         .driver = {
>                 .name   = "sdhci-dwcmshc",
> --
> 2.7.4
> 
> 
> 

Re: [PATCH v4 3/3] mmc: sdhci-of-dwcmshc: add rockchip platform support

[Shawn Lin]

Hi Jisheng

On 2021/3/11 14:59, Jisheng Zhang wrote:
> Hi Shawn,
> 
> On Thu, 11 Mar 2021 14:21:24 +0800 Shawn Lin <shawn.lin@rock-chips.com> wrote:
> 
>>
>> sdhci based synopsys MMC IP is also used on some rockchip platforms,
>> so add a basic support here.
>>
>> Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
>> Acked-by: Adrian Hunter <adrian.hunter@intel.com>
>> ---
>>
>> Changes in v4:
>> - add comments for disabling rx invert
>> - add tag from Adrian
>>
>>   drivers/mmc/host/sdhci-of-dwcmshc.c | 225 ++++++++++++++++++++++++++++++++++--
>>   1 file changed, 218 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/mmc/host/sdhci-of-dwcmshc.c b/drivers/mmc/host/sdhci-of-dwcmshc.c
>> index 59d8d96..dabc1ec 100644
>> --- a/drivers/mmc/host/sdhci-of-dwcmshc.c
>> +++ b/drivers/mmc/host/sdhci-of-dwcmshc.c
>> @@ -9,9 +9,11 @@
>>
>>   #include <linux/clk.h>
>>   #include <linux/dma-mapping.h>
>> +#include <linux/iopoll.h>
>>   #include <linux/kernel.h>
>>   #include <linux/module.h>
>>   #include <linux/of.h>
>> +#include <linux/of_device.h>
>>   #include <linux/sizes.h>
>>
>>   #include "sdhci-pltfm.h"
>> @@ -21,11 +23,43 @@
>>   /* DWCMSHC specific Mode Select value */
>>   #define DWCMSHC_CTRL_HS400             0x7
>>
>> +/* Rockchip specific Registers */
>> +#define DWCMSHC_HOST_CTRL3             0x508
> 
> Maybe 0x500 can be read from VENDOR_PTR_R while 0x8 is the offset?

It should be but we didn't add this info for this IP so we have
to hardcode the register offset.

> 
>> +#define DWCMSHC_EMMC_CONTROL           0x52c
>> +#define DWCMSHC_EMMC_ATCTRL            0x540
>> +#define DWCMSHC_EMMC_DLL_CTRL          0x800
> 
> Maybe 0x800 is phy offset? while 0, 4, 8 etc. below are phy registers?
> 
>> +#define DWCMSHC_EMMC_DLL_RXCLK         0x804
>> +#define DWCMSHC_EMMC_DLL_TXCLK         0x808
>> +#define DWCMSHC_EMMC_DLL_STRBIN                0x80c
>> +#define DWCMSHC_EMMC_DLL_STATUS0       0x840
>> +#define DWCMSHC_EMMC_DLL_START         BIT(0)
>> +#define DWCMSHC_EMMC_DLL_RXCLK_SRCSEL  29
>> +#define DWCMSHC_EMMC_DLL_START_POINT   16
>> +#define DWCMSHC_EMMC_DLL_INC           8
>> +#define DWCMSHC_EMMC_DLL_DLYENA                BIT(27)
>> +#define DLL_TXCLK_TAPNUM_DEFAULT       0x8
>> +#define DLL_STRBIN_TAPNUM_DEFAULT      0x8
>> +#define DLL_TXCLK_TAPNUM_FROM_SW       BIT(24)
>> +#define DLL_STRBIN_TAPNUM_FROM_SW      BIT(24)
>> +#define DWCMSHC_EMMC_DLL_LOCKED                BIT(8)
>> +#define DWCMSHC_EMMC_DLL_TIMEOUT       BIT(9)
>> +#define DLL_RXCLK_NO_INVERTER          1
>> +#define DLL_RXCLK_INVERTER             0
>> +#define DWCMSHC_ENHANCED_STROBE                BIT(8)
> 
> Is it better to let bits' definition grouped with register together, e.g this
> definition could be moved to the corresponding CONTROL register MACRO

Make sense to me. Will improve it.

> 
>> +#define DLL_LOCK_WO_TMOUT(x) \
>> +       ((((x) & DWCMSHC_EMMC_DLL_LOCKED) == DWCMSHC_EMMC_DLL_LOCKED) && \
>> +       (((x) & DWCMSHC_EMMC_DLL_TIMEOUT) == 0))
>> +#define ROCKCHIP_MAX_CLKS              3
>> +
>>   #define BOUNDARY_OK(addr, len) \
>>          ((addr | (SZ_128M - 1)) == ((addr + len - 1) | (SZ_128M - 1)))
>>
>>   struct dwcmshc_priv {
>>          struct clk      *bus_clk;
>> +
>> +       /* Rockchip specified optional clocks */
>> +       struct clk_bulk_data rockchip_clks[ROCKCHIP_MAX_CLKS];
>> +       u8 txclk_tapnum;
>>   };
>>
>>   /*
>> @@ -100,6 +134,102 @@ static void dwcmshc_set_uhs_signaling(struct sdhci_host *host,
>>          sdhci_writew(host, ctrl_2, SDHCI_HOST_CONTROL2);
>>   }
>>
>> +static void dwcmshc_rk_hs400_enhanced_strobe(struct mmc_host *mmc,
>> +                                            struct mmc_ios *ios)
>> +{
>> +       u32 vendor;
>> +       struct sdhci_host *host = mmc_priv(mmc);
>> +
>> +       vendor = sdhci_readl(host, DWCMSHC_EMMC_CONTROL);
>> +       if (ios->enhanced_strobe)
>> +               vendor |= DWCMSHC_ENHANCED_STROBE;
>> +       else
>> +               vendor &= ~DWCMSHC_ENHANCED_STROBE;
>> +
>> +       sdhci_writel(host, vendor, DWCMSHC_EMMC_CONTROL);
>> +}
>> +
>> +static void dwcmshc_rk_set_clock(struct sdhci_host *host, unsigned int clock)
>> +{
>> +       struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
>> +       struct dwcmshc_priv *priv = sdhci_pltfm_priv(pltfm_host);
>> +       u8 txclk_tapnum = DLL_TXCLK_TAPNUM_DEFAULT;
>> +       u32 extra;
>> +       int err;
>> +
>> +       host->mmc->actual_clock = 0;
>> +
>> +       /*
>> +        * DO NOT TOUCH THIS SETTING. RX clk inverter unit is enabled
>> +        * by default, but it shouldn't be enabled. We should anyway
>> +        * disable it before issuing any cmds.
>> +        */
>> +       extra = DWCMSHC_EMMC_DLL_DLYENA |
>> +               DLL_RXCLK_NO_INVERTER << DWCMSHC_EMMC_DLL_RXCLK_SRCSEL;
>> +       sdhci_writel(host, extra, DWCMSHC_EMMC_DLL_RXCLK);
>> +
>> +       if (clock == 0)
>> +               return;
>> +
>> +       /* Rockchip platform only support 375KHz for identify mode */
>> +       if (clock <= 400000)
>> +               clock = 375000;
>> +
>> +       err = clk_set_rate(pltfm_host->clk, clock);
>> +       if (err)
>> +               dev_err(mmc_dev(host->mmc), "fail to set clock %d", clock);
>> +
>> +       sdhci_set_clock(host, clock);
>> +
>> +       /* Disable cmd conflict check */
>> +       extra = sdhci_readl(host, DWCMSHC_HOST_CTRL3);
>> +       extra &= ~BIT(0);
>> +       sdhci_writel(host, extra, DWCMSHC_HOST_CTRL3);
>> +
>> +       if (clock <= 400000) {
>> +               /* Disable DLL to reset sample clock */
>> +               sdhci_writel(host, 0, DWCMSHC_EMMC_DLL_CTRL);
>> +               return;
>> +       }
>> +
>> +       /* Reset DLL */
>> +       sdhci_writel(host, BIT(1), DWCMSHC_EMMC_DLL_CTRL);
>> +       udelay(1);
>> +       sdhci_writel(host, 0x0, DWCMSHC_EMMC_DLL_CTRL);
>> +
>> +       /* Init DLL settings */
>> +       extra = 0x5 << DWCMSHC_EMMC_DLL_START_POINT |
>> +               0x2 << DWCMSHC_EMMC_DLL_INC |
>> +               DWCMSHC_EMMC_DLL_START;
>> +       sdhci_writel(host, extra, DWCMSHC_EMMC_DLL_CTRL);
>> +       err = readl_poll_timeout(host->ioaddr + DWCMSHC_EMMC_DLL_STATUS0,
>> +                                extra, DLL_LOCK_WO_TMOUT(extra), 1,
>> +                                500 * USEC_PER_MSEC);
>> +       if (err) {
>> +               dev_err(mmc_dev(host->mmc), "DLL lock timeout!\n");
>> +               return;
>> +       }
>> +
>> +       extra = 0x1 << 16 | /* tune clock stop en */
>> +               0x2 << 17 | /* pre-change delay */
>> +               0x3 << 19;  /* post-change delay */
>> +       sdhci_writel(host, extra, DWCMSHC_EMMC_ATCTRL);
>> +
>> +       if (host->mmc->ios.timing == MMC_TIMING_MMC_HS200 ||
>> +           host->mmc->ios.timing == MMC_TIMING_MMC_HS400)
>> +               txclk_tapnum = priv->txclk_tapnum;
>> +
>> +       extra = DWCMSHC_EMMC_DLL_DLYENA |
>> +               DLL_TXCLK_TAPNUM_FROM_SW |
>> +               txclk_tapnum;
>> +       sdhci_writel(host, extra, DWCMSHC_EMMC_DLL_TXCLK);
>> +
>> +       extra = DWCMSHC_EMMC_DLL_DLYENA |
>> +               DLL_STRBIN_TAPNUM_DEFAULT |
>> +               DLL_STRBIN_TAPNUM_FROM_SW;
>> +       sdhci_writel(host, extra, DWCMSHC_EMMC_DLL_STRBIN);
>> +}
>> +
>>   static const struct sdhci_ops sdhci_dwcmshc_ops = {
>>          .set_clock              = sdhci_set_clock,
>>          .set_bus_width          = sdhci_set_bus_width,
>> @@ -109,21 +239,91 @@ static const struct sdhci_ops sdhci_dwcmshc_ops = {
>>          .adma_write_desc        = dwcmshc_adma_write_desc,
>>   };
>>
>> +static const struct sdhci_ops sdhci_dwcmshc_rk_ops = {
>> +       .set_clock              = dwcmshc_rk_set_clock,
>> +       .set_bus_width          = sdhci_set_bus_width,
>> +       .set_uhs_signaling      = dwcmshc_set_uhs_signaling,
>> +       .get_max_clock          = sdhci_pltfm_clk_get_max_clock,
>> +       .reset                  = sdhci_reset,
>> +       .adma_write_desc        = dwcmshc_adma_write_desc,
>> +};
>> +
>>   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 sdhci_pltfm_data sdhci_dwcmshc_rk_pdata = {
>> +       .ops = &sdhci_dwcmshc_rk_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 int rockchip_pltf_init(struct sdhci_host *host, struct dwcmshc_priv *priv)
>> +{
>> +       int 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), ROCKCHIP_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(ROCKCHIP_MAX_CLKS, priv->rockchip_clks);
>> +       if (err) {
>> +               dev_err(mmc_dev(host->mmc), "failed to enable clocks %d\n", err);
>> +               return err;
>> +       }
>> +
>> +       if (of_property_read_u8(mmc_dev(host->mmc)->of_node, "rockchip,txclk-tapnum",
>> +                               &priv->txclk_tapnum))
>> +               priv->txclk_tapnum = DLL_TXCLK_TAPNUM_DEFAULT;
>> +
>> +       /* Disable cmd conflict check */
>> +       sdhci_writel(host, 0x0, 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 const struct of_device_id sdhci_dwcmshc_dt_ids[] = {
>> +       {
>> +               .compatible = "snps,dwcmshc-sdhci",
>> +               .data = &sdhci_dwcmshc_pdata,
>> +       },
>> +       {
>> +               .compatible = "rockchip,dwcmshc-sdhci",
>> +               .data = &sdhci_dwcmshc_rk_pdata,
>> +       },
>> +       {},
>> +};
>> +MODULE_DEVICE_TABLE(of, sdhci_dwcmshc_dt_ids);
>> +
>>   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;
>>          int err;
>>          u32 extra;
>>
>> -       host = sdhci_pltfm_init(pdev, &sdhci_dwcmshc_pdata,
>> +       pltfm_data = of_device_get_match_data(&pdev->dev);
>> +       if (!pltfm_data) {
>> +               dev_err(&pdev->dev, "Error: No device match data found\n");
>> +               return -ENODEV;
>> +       }
>> +
>> +       host = sdhci_pltfm_init(pdev, pltfm_data,
>>                                  sizeof(struct dwcmshc_priv));
>>          if (IS_ERR(host))
>>                  return PTR_ERR(host);
>> @@ -161,6 +361,15 @@ static int dwcmshc_probe(struct platform_device *pdev)
>>
>>          host->mmc_host_ops.request = dwcmshc_request;
>>
>> +       if (pltfm_data == &sdhci_dwcmshc_rk_pdata) {
>> +               host->mmc_host_ops.hs400_enhanced_strobe =
>> +                       dwcmshc_rk_hs400_enhanced_strobe;
>> +
>> +               err = rockchip_pltf_init(host, priv);
>> +               if (err)
>> +                       goto err_clk;
>> +       }
>> +
>>          err = sdhci_add_host(host);
>>          if (err)
>>                  goto err_clk;
>> @@ -170,6 +379,7 @@ static int dwcmshc_probe(struct platform_device *pdev)
>>   err_clk:
>>          clk_disable_unprepare(pltfm_host->clk);
>>          clk_disable_unprepare(priv->bus_clk);
>> +       clk_bulk_disable_unprepare(ROCKCHIP_MAX_CLKS, priv->rockchip_clks);
>>   free_pltfm:
>>          sdhci_pltfm_free(pdev);
>>          return err;
>> @@ -185,6 +395,7 @@ static int dwcmshc_remove(struct platform_device *pdev)
>>
>>          clk_disable_unprepare(pltfm_host->clk);
>>          clk_disable_unprepare(priv->bus_clk);
>> +       clk_bulk_disable_unprepare(ROCKCHIP_MAX_CLKS, priv->rockchip_clks);
>>
>>          sdhci_pltfm_free(pdev);
>>
>> @@ -207,6 +418,8 @@ static int dwcmshc_suspend(struct device *dev)
>>          if (!IS_ERR(priv->bus_clk))
>>                  clk_disable_unprepare(priv->bus_clk);
>>
>> +       clk_bulk_disable_unprepare(ROCKCHIP_MAX_CLKS, priv->rockchip_clks);
>> +
>>          return ret;
>>   }
>>
>> @@ -227,18 +440,16 @@ static int dwcmshc_resume(struct device *dev)
>>                          return ret;
>>          }
>>
>> +       ret = clk_bulk_prepare_enable(ROCKCHIP_MAX_CLKS, priv->rockchip_clks);
>> +       if (ret)
>> +               return ret;
>> +
>>          return sdhci_resume_host(host);
>>   }
>>   #endif
>>
>>   static SIMPLE_DEV_PM_OPS(dwcmshc_pmops, dwcmshc_suspend, dwcmshc_resume);
>>
>> -static const struct of_device_id sdhci_dwcmshc_dt_ids[] = {
>> -       { .compatible = "snps,dwcmshc-sdhci" },
>> -       {}
>> -};
>> -MODULE_DEVICE_TABLE(of, sdhci_dwcmshc_dt_ids);
>> -
>>   static struct platform_driver sdhci_dwcmshc_driver = {
>>          .driver = {
>>                  .name   = "sdhci-dwcmshc",
>> --
>> 2.7.4
>>
>>
>>
> 
> 
> 
> 

Re: [PATCH v4 3/3] mmc: sdhci-of-dwcmshc: add rockchip platform support

[Johan Jonker]

Hi Shawn,

Some comments. Have a look if useful or that you disagee with.

The compatibility string was changed.
Then change the driver from platform to Soc focus too.

On 3/11/21 7:21 AM, Shawn Lin wrote:
> sdhci based synopsys MMC IP is also used on some rockchip platforms,
> so add a basic support here.
> 
> Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>

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

Remove ack, because things will change in version 5.
In need for a new review.

> ---
> 
> Changes in v4:
> - add comments for disabling rx invert
> - add tag from Adrian
> 
>  drivers/mmc/host/sdhci-of-dwcmshc.c | 225 ++++++++++++++++++++++++++++++++++--
>  1 file changed, 218 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/mmc/host/sdhci-of-dwcmshc.c b/drivers/mmc/host/sdhci-of-dwcmshc.c
> index 59d8d96..dabc1ec 100644
> --- a/drivers/mmc/host/sdhci-of-dwcmshc.c
> +++ b/drivers/mmc/host/sdhci-of-dwcmshc.c
> @@ -9,9 +9,11 @@
>  
>  #include <linux/clk.h>
>  #include <linux/dma-mapping.h>
> +#include <linux/iopoll.h>
>  #include <linux/kernel.h>
>  #include <linux/module.h>

>  #include <linux/of.h>
> +#include <linux/of_device.h>

sort includes

>  #include <linux/sizes.h>
>  
>  #include "sdhci-pltfm.h"
> @@ -21,11 +23,43 @@
>  /* DWCMSHC specific Mode Select value */
>  #define DWCMSHC_CTRL_HS400		0x7
>  
> +/* Rockchip specific Registers */
> +#define DWCMSHC_HOST_CTRL3		0x508
> +#define DWCMSHC_EMMC_CONTROL		0x52c
> +#define DWCMSHC_EMMC_ATCTRL		0x540
> +#define DWCMSHC_EMMC_DLL_CTRL		0x800
> +#define DWCMSHC_EMMC_DLL_RXCLK		0x804
> +#define DWCMSHC_EMMC_DLL_TXCLK		0x808
> +#define DWCMSHC_EMMC_DLL_STRBIN		0x80c
> +#define DWCMSHC_EMMC_DLL_STATUS0	0x840
> +#define DWCMSHC_EMMC_DLL_START		BIT(0)
> +#define DWCMSHC_EMMC_DLL_RXCLK_SRCSEL	29
> +#define DWCMSHC_EMMC_DLL_START_POINT	16
> +#define DWCMSHC_EMMC_DLL_INC		8
> +#define DWCMSHC_EMMC_DLL_DLYENA		BIT(27)
> +#define DLL_TXCLK_TAPNUM_DEFAULT	0x8
> +#define DLL_STRBIN_TAPNUM_DEFAULT	0x8
> +#define DLL_TXCLK_TAPNUM_FROM_SW	BIT(24)
> +#define DLL_STRBIN_TAPNUM_FROM_SW	BIT(24)
> +#define DWCMSHC_EMMC_DLL_LOCKED		BIT(8)
> +#define DWCMSHC_EMMC_DLL_TIMEOUT	BIT(9)
> +#define DLL_RXCLK_NO_INVERTER		1
> +#define DLL_RXCLK_INVERTER		0
> +#define DWCMSHC_ENHANCED_STROBE		BIT(8)
> +#define DLL_LOCK_WO_TMOUT(x) \
> +	((((x) & DWCMSHC_EMMC_DLL_LOCKED) == DWCMSHC_EMMC_DLL_LOCKED) && \
> +	(((x) & DWCMSHC_EMMC_DLL_TIMEOUT) == 0))

> +#define ROCKCHIP_MAX_CLKS		3

#define RK3568_MAX_CLKS		3

> +
>  #define BOUNDARY_OK(addr, len) \
>  	((addr | (SZ_128M - 1)) == ((addr + len - 1) | (SZ_128M - 1)))
>  

>  struct dwcmshc_priv {
>  	struct clk	*bus_clk;

      void *priv; ?? pointer to SoC private stuff ??

Please advise how private stuff of multiple SoCs should be stored.
Structures should be expandable.

}

common private stuff

  struct rk3568_priv {
> +
> +	/* Rockchip specified optional clocks */
> +	struct clk_bulk_data rockchip_clks[ROCKCHIP_MAX_CLKS];

RK3568_MAX_CLKS

> +	u8 txclk_tapnum;
>  };

rk3568 private stuff

>  
>  /*
> @@ -100,6 +134,102 @@ static void dwcmshc_set_uhs_signaling(struct sdhci_host *host,
>  	sdhci_writew(host, ctrl_2, SDHCI_HOST_CONTROL2);
>  }
>  

> +static void dwcmshc_rk_hs400_enhanced_strobe(struct mmc_host *mmc,

static void dwcmshc_rk3568_hs400_enhanced_strobe(struct mmc_host *mmc,

> +					     struct mmc_ios *ios)
> +{
> +	u32 vendor;
> +	struct sdhci_host *host = mmc_priv(mmc);
> +
> +	vendor = sdhci_readl(host, DWCMSHC_EMMC_CONTROL);
> +	if (ios->enhanced_strobe)
> +		vendor |= DWCMSHC_ENHANCED_STROBE;
> +	else
> +		vendor &= ~DWCMSHC_ENHANCED_STROBE;
> +
> +	sdhci_writel(host, vendor, DWCMSHC_EMMC_CONTROL);
> +}
> +

> +static void dwcmshc_rk_set_clock(struct sdhci_host *host, unsigned int clock)

static void dwcmshc_rk3568_set_clock(struct sdhci_host *host, unsigned
int clock)

For FTRACE it is needed that all function names start with the same prefix.
Change from platform to Soc.

> +{
> +	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);

> +	struct dwcmshc_priv *priv = sdhci_pltfm_priv(pltfm_host);

rk3568_priv

> +	u8 txclk_tapnum = DLL_TXCLK_TAPNUM_DEFAULT;
> +	u32 extra;
> +	int err;
> +
> +	host->mmc->actual_clock = 0;
> +
> +	/*
> +	 * DO NOT TOUCH THIS SETTING. RX clk inverter unit is enabled
> +	 * by default, but it shouldn't be enabled. We should anyway
> +	 * disable it before issuing any cmds.
> +	 */
> +	extra = DWCMSHC_EMMC_DLL_DLYENA |
> +		DLL_RXCLK_NO_INVERTER << DWCMSHC_EMMC_DLL_RXCLK_SRCSEL;
> +	sdhci_writel(host, extra, DWCMSHC_EMMC_DLL_RXCLK);
> +
> +	if (clock == 0)
> +		return;
> +
> +	/* Rockchip platform only support 375KHz for identify mode */
> +	if (clock <= 400000)
> +		clock = 375000;
> +
> +	err = clk_set_rate(pltfm_host->clk, clock);
> +	if (err)
> +		dev_err(mmc_dev(host->mmc), "fail to set clock %d", clock);
> +
> +	sdhci_set_clock(host, clock);
> +
> +	/* Disable cmd conflict check */
> +	extra = sdhci_readl(host, DWCMSHC_HOST_CTRL3);
> +	extra &= ~BIT(0);
> +	sdhci_writel(host, extra, DWCMSHC_HOST_CTRL3);
> +
> +	if (clock <= 400000) {
> +		/* Disable DLL to reset sample clock */
> +		sdhci_writel(host, 0, DWCMSHC_EMMC_DLL_CTRL);
> +		return;
> +	}
> +
> +	/* Reset DLL */
> +	sdhci_writel(host, BIT(1), DWCMSHC_EMMC_DLL_CTRL);
> +	udelay(1);
> +	sdhci_writel(host, 0x0, DWCMSHC_EMMC_DLL_CTRL);
> +
> +	/* Init DLL settings */
> +	extra = 0x5 << DWCMSHC_EMMC_DLL_START_POINT |
> +		0x2 << DWCMSHC_EMMC_DLL_INC |
> +		DWCMSHC_EMMC_DLL_START;
> +	sdhci_writel(host, extra, DWCMSHC_EMMC_DLL_CTRL);
> +	err = readl_poll_timeout(host->ioaddr + DWCMSHC_EMMC_DLL_STATUS0,
> +				 extra, DLL_LOCK_WO_TMOUT(extra), 1,
> +				 500 * USEC_PER_MSEC);
> +	if (err) {
> +		dev_err(mmc_dev(host->mmc), "DLL lock timeout!\n");
> +		return;
> +	}
> +
> +	extra = 0x1 << 16 | /* tune clock stop en */
> +		0x2 << 17 | /* pre-change delay */
> +		0x3 << 19;  /* post-change delay */
> +	sdhci_writel(host, extra, DWCMSHC_EMMC_ATCTRL);
> +
> +	if (host->mmc->ios.timing == MMC_TIMING_MMC_HS200 ||
> +	    host->mmc->ios.timing == MMC_TIMING_MMC_HS400)
> +		txclk_tapnum = priv->txclk_tapnum;
> +
> +	extra = DWCMSHC_EMMC_DLL_DLYENA |
> +		DLL_TXCLK_TAPNUM_FROM_SW |
> +		txclk_tapnum;
> +	sdhci_writel(host, extra, DWCMSHC_EMMC_DLL_TXCLK);
> +
> +	extra = DWCMSHC_EMMC_DLL_DLYENA |
> +		DLL_STRBIN_TAPNUM_DEFAULT |
> +		DLL_STRBIN_TAPNUM_FROM_SW;
> +	sdhci_writel(host, extra, DWCMSHC_EMMC_DLL_STRBIN);
> +}
> +
>  static const struct sdhci_ops sdhci_dwcmshc_ops = {
>  	.set_clock		= sdhci_set_clock,
>  	.set_bus_width		= sdhci_set_bus_width,
> @@ -109,21 +239,91 @@ static const struct sdhci_ops sdhci_dwcmshc_ops = {
>  	.adma_write_desc	= dwcmshc_adma_write_desc,
>  };
>  

> +static const struct sdhci_ops sdhci_dwcmshc_rk_ops = {

static const struct sdhci_ops rk3568-dwcmshc_ops = {

> +	.set_clock		= dwcmshc_rk_set_clock,

	.set_clock		= dwcmshc_rk3568_set_clock,

> +	.set_bus_width		= sdhci_set_bus_width,
> +	.set_uhs_signaling	= dwcmshc_set_uhs_signaling,
> +	.get_max_clock		= sdhci_pltfm_clk_get_max_clock,
> +	.reset			= sdhci_reset,
> +	.adma_write_desc	= dwcmshc_adma_write_desc,
> +};
> +
>  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 sdhci_pltfm_data sdhci_dwcmshc_rk_pdata = {

static const struct sdhci_pltfm_data rk3568-dwcmshc_pdata = {

> +	.ops = &sdhci_dwcmshc_rk_ops,

	.ops = &rk3568-dwcmshc_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 int rockchip_pltf_init(struct sdhci_host *host, struct dwcmshc_priv *priv)

static int dwcmshc_rk3568_init(struct sdhci_host *host, struct
rk3568_priv *priv)

For FTRACE it is needed that all function names start with the same prefix.

> +{
> +	int 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), ROCKCHIP_MAX_CLKS,

RK3568_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(ROCKCHIP_MAX_CLKS, priv->rockchip_clks);

RK3568_MAX_CLKS

> +	if (err) {
> +		dev_err(mmc_dev(host->mmc), "failed to enable clocks %d\n", err);
> +		return err;
> +	}
> +
> +	if (of_property_read_u8(mmc_dev(host->mmc)->of_node, "rockchip,txclk-tapnum",
> +				&priv->txclk_tapnum))
> +		priv->txclk_tapnum = DLL_TXCLK_TAPNUM_DEFAULT;
> +
> +	/* Disable cmd conflict check */
> +	sdhci_writel(host, 0x0, 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 const struct of_device_id sdhci_dwcmshc_dt_ids[] = {


	{
		.compatible = "rockchip,rk3568-dwcmshc",
		.data = &rk3568-dwcmshc_pdata,
	},


Keep in sort order for if later other manufactures and SoCs are added.

> +	{
> +		.compatible = "snps,dwcmshc-sdhci",
> +		.data = &sdhci_dwcmshc_pdata,
> +	},

> +	{
> +		.compatible = "rockchip,dwcmshc-sdhci",
> +		.data = &sdhci_dwcmshc_rk_pdata,
> +	},

remove

> +	{},
> +};
> +MODULE_DEVICE_TABLE(of, sdhci_dwcmshc_dt_ids);
> +
>  static int dwcmshc_probe(struct platform_device *pdev)
>  {
>  	struct sdhci_pltfm_host *pltfm_host;
>  	struct sdhci_host *host;

>  	struct dwcmshc_priv *priv;
Separate common private and rk3568 private stuff.

> +	const struct sdhci_pltfm_data *pltfm_data;
>  	int err;
>  	u32 extra;
>  
> -	host = sdhci_pltfm_init(pdev, &sdhci_dwcmshc_pdata,
> +	pltfm_data = of_device_get_match_data(&pdev->dev);
> +	if (!pltfm_data) {
> +		dev_err(&pdev->dev, "Error: No device match data found\n");
> +		return -ENODEV;
> +	}
> +
> +	host = sdhci_pltfm_init(pdev, pltfm_data,
>  				sizeof(struct dwcmshc_priv));

fix common and private

>  	if (IS_ERR(host))
>  		return PTR_ERR(host);
> @@ -161,6 +361,15 @@ static int dwcmshc_probe(struct platform_device *pdev)
>  

[..]

	pltfm_host = sdhci_priv(host);
	priv = sdhci_pltfm_priv(pltfm_host);

----

	pltfm_host->clk = devm_clk_get(&pdev->dev, "core");

Maybe make this common private too?

	if (IS_ERR(pltfm_host->clk)) {
		err = PTR_ERR(pltfm_host->clk);
		dev_err(&pdev->dev, "failed to get core clk: %d\n", err);
		goto free_pltfm;
	}
	err = clk_prepare_enable(pltfm_host->clk);
	if (err)
		goto free_pltfm;

	priv->bus_clk = devm_clk_get(&pdev->dev, "bus");

This is common private.

	if (!IS_ERR(priv->bus_clk))
		clk_prepare_enable(priv->bus_clk);


You are doing mixed private stuff in a common function.

----

	err = mmc_of_parse(host->mmc);
	if (err)
		goto err_clk;

	sdhci_get_of_property(pdev);

[..]

>  	host->mmc_host_ops.request = dwcmshc_request;
>  
> +	if (pltfm_data == &sdhci_dwcmshc_rk_pdata) {

	if (pltfm_data == &rk3568-dwcmshc_pdata) {

> +		host->mmc_host_ops.hs400_enhanced_strobe =
> +			dwcmshc_rk_hs400_enhanced_strobe;

dwcmshc_rk3568_hs400_enhanced_strobe;

> +

> +		err = rockchip_pltf_init(host, priv);

		err = dwcmshc_rk3568_init(host, priv);




> +		if (err)
> +			goto err_clk;
> +	}
> +
>  	err = sdhci_add_host(host);
>  	if (err)
>  		goto err_clk;
> @@ -170,6 +379,7 @@ static int dwcmshc_probe(struct platform_device *pdev)
>  err_clk:

>  	clk_disable_unprepare(pltfm_host->clk);
>  	clk_disable_unprepare(priv->bus_clk);
> +	clk_bulk_disable_unprepare(ROCKCHIP_MAX_CLKS, priv->rockchip_clks);

dito
keep private stuff separate.
RK3568_MAX_CLKS

>  free_pltfm:
>  	sdhci_pltfm_free(pdev);
>  	return err;
> @@ -185,6 +395,7 @@ static int dwcmshc_remove(struct platform_device *pdev)
>  

>  	clk_disable_unprepare(pltfm_host->clk);
>  	clk_disable_unprepare(priv->bus_clk);

> +	clk_bulk_disable_unprepare(ROCKCHIP_MAX_CLKS, priv->rockchip_clks);

dito
only rk3568
RK3568_MAX_CLKS

>  
>  	sdhci_pltfm_free(pdev);
>  
> @@ -207,6 +418,8 @@ static int dwcmshc_suspend(struct device *dev)
>  	if (!IS_ERR(priv->bus_clk))
>  		clk_disable_unprepare(priv->bus_clk);
>  

> +	clk_bulk_disable_unprepare(ROCKCHIP_MAX_CLKS, priv->rockchip_clks);
> +

dito
RK3568_MAX_CLKS

>  	return ret;
>  }
>  
> @@ -227,18 +440,16 @@ static int dwcmshc_resume(struct device *dev)
>  			return ret;
>  	}
>  
> +	ret = clk_bulk_prepare_enable(ROCKCHIP_MAX_CLKS, priv->rockchip_clks);
> +	if (ret)
> +		return ret;
> +

dito
RK3568_MAX_CLKS

>  	return sdhci_resume_host(host);
>  }
>  #endif
>  
>  static SIMPLE_DEV_PM_OPS(dwcmshc_pmops, dwcmshc_suspend, dwcmshc_resume);
>  
> -static const struct of_device_id sdhci_dwcmshc_dt_ids[] = {
> -	{ .compatible = "snps,dwcmshc-sdhci" },
> -	{}
> -};
> -MODULE_DEVICE_TABLE(of, sdhci_dwcmshc_dt_ids);
> -
>  static struct platform_driver sdhci_dwcmshc_driver = {
>  	.driver	= {
>  		.name	= "sdhci-dwcmshc",
> 

Re: [PATCH v4 3/3] mmc: sdhci-of-dwcmshc: add rockchip platform support

[Jisheng Zhang]

Hi

On Thu, 11 Mar 2021 15:08:03 +0800
Shawn Lin <shawn.lin@rock-chips.com> wrote:


> 
> Hi Jisheng
> 
> On 2021/3/11 14:59, Jisheng Zhang wrote:
> > Hi Shawn,
> >
> > On Thu, 11 Mar 2021 14:21:24 +0800 Shawn Lin <shawn.lin@rock-chips.com> wrote:
> >  
> >>
> >> sdhci based synopsys MMC IP is also used on some rockchip platforms,
> >> so add a basic support here.
> >>
> >> Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
> >> Acked-by: Adrian Hunter <adrian.hunter@intel.com>
> >> ---
> >>
> >> Changes in v4:
> >> - add comments for disabling rx invert
> >> - add tag from Adrian
> >>
> >>   drivers/mmc/host/sdhci-of-dwcmshc.c | 225 ++++++++++++++++++++++++++++++++++--
> >>   1 file changed, 218 insertions(+), 7 deletions(-)
> >>
> >> diff --git a/drivers/mmc/host/sdhci-of-dwcmshc.c b/drivers/mmc/host/sdhci-of-dwcmshc.c
> >> index 59d8d96..dabc1ec 100644
> >> --- a/drivers/mmc/host/sdhci-of-dwcmshc.c
> >> +++ b/drivers/mmc/host/sdhci-of-dwcmshc.c
> >> @@ -9,9 +9,11 @@
> >>
> >>   #include <linux/clk.h>
> >>   #include <linux/dma-mapping.h>
> >> +#include <linux/iopoll.h>
> >>   #include <linux/kernel.h>
> >>   #include <linux/module.h>
> >>   #include <linux/of.h>
> >> +#include <linux/of_device.h>
> >>   #include <linux/sizes.h>
> >>
> >>   #include "sdhci-pltfm.h"
> >> @@ -21,11 +23,43 @@
> >>   /* DWCMSHC specific Mode Select value */
> >>   #define DWCMSHC_CTRL_HS400             0x7
> >>
> >> +/* Rockchip specific Registers */
> >> +#define DWCMSHC_HOST_CTRL3             0x508  
> >
> > Maybe 0x500 can be read from VENDOR_PTR_R while 0x8 is the offset?  
> 
> It should be but we didn't add this info for this IP so we have
> to hardcode the register offset.

Per my understanding, this register always exists, so mind to double check
whether you can access the register and read out 0x500. If the vendor offset
can't be read out on your side, mind to add var such as vendor_ptr etc. then
configure it as 0x500 for RK? I believe HS400 ES support code is common
to this IP, the only difference maybe the vendor offset can be dynamically
read out while RK may need to hardcode the offset as you said.

Thanks