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

From: Chen Wang <unicorn_wang@outlook.com>

Add support for the mmc controller for Sophgo SG2042.
Adding corresponding new compatible strings, and implement
custom sdhci_ops.

This patchset is based on v6.9-rc1 and depends on following pathsets:

- [PATCH 0/1] mmc: sdhci-of-dwcmshc: enhance framework [1]
- [PATCH v14 0/5] riscv: sophgo: add clock support for sg2042 [2]
- [PATCH v1] mmc: sdhci-of-dwcmshc: th1520: Increase tuning loop count to 128 [3]


Link: https://lore.kernel.org/linux-kernel/cover.1713257181.git.unicorn_wang@outlook.com/ [1]
Link: https://lore.kernel.org/linux-riscv/cover.1713164546.git.unicorn_wang@outlook.com/ [2]
Link: https://lore.kernel.org/all/20240402093539.184287-1-bigunclemax@gmail.com/ [3]


---

Chen Wang (3):
  dt-bindings: mmc: sdhci-of-dwcmhsc: Add Sophgo SG2042 support
  mmc: sdhci-of-dwcmshc: Add support for Sophgo SG2042
  riscv: dts: add mmc controllers for Sophgo SG2042 SoC

 .../bindings/mmc/snps,dwcmshc-sdhci.yaml      |  67 +++++--
 .../boot/dts/sophgo/sg2042-milkv-pioneer.dts  |  15 ++
 arch/riscv/boot/dts/sophgo/sg2042.dtsi        |  32 ++++
 drivers/mmc/host/sdhci-of-dwcmshc.c           | 173 +++++++++++++++++-
 4 files changed, 264 insertions(+), 23 deletions(-)

-- 
2.25.1


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

* [PATCH 1/3] dt-bindings: mmc: sdhci-of-dwcmhsc: Add Sophgo SG2042 support
  2024-04-16  9:50 [PATCH 0/3] mmc: sdhci-of-dwcmshc: support Sophgo SG2042 Chen Wang
@ 2024-04-16  9:50 ` Chen Wang
  2024-04-16 16:44   ` Conor Dooley
  2024-04-16  9:50 ` [PATCH 2/3] mmc: sdhci-of-dwcmshc: Add support for Sophgo SG2042 Chen Wang
  2024-04-16  9:51 ` [PATCH 3/3] riscv: dts: add mmc controllers for Sophgo SG2042 SoC Chen Wang
  2 siblings, 1 reply; 8+ messages in thread
From: Chen Wang @ 2024-04-16  9:50 UTC (permalink / raw)
  To: adrian.hunter, aou, conor+dt, guoren, inochiama, jszhang,
	krzysztof.kozlowski+dt, palmer, paul.walmsley, robh, ulf.hansson,
	devicetree, linux-kernel, linux-mmc, linux-riscv, chao.wei,
	haijiao.liu, xiaoguang.xing, tingzhu.wang
  Cc: Chen Wang

From: Chen Wang <unicorn_wang@outlook.com>

SG2042 use Synopsys dwcnshc IP for SD/eMMC controllers.

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

Adding some examples.

Signed-off-by: Chen Wang <unicorn_wang@outlook.com>
---
 .../bindings/mmc/snps,dwcmshc-sdhci.yaml      | 67 ++++++++++++++-----
 1 file changed, 51 insertions(+), 16 deletions(-)

diff --git a/Documentation/devicetree/bindings/mmc/snps,dwcmshc-sdhci.yaml b/Documentation/devicetree/bindings/mmc/snps,dwcmshc-sdhci.yaml
index 4d3031d9965f..a04ccae216cf 100644
--- a/Documentation/devicetree/bindings/mmc/snps,dwcmshc-sdhci.yaml
+++ b/Documentation/devicetree/bindings/mmc/snps,dwcmshc-sdhci.yaml
@@ -21,6 +21,7 @@ properties:
       - snps,dwcmshc-sdhci
       - sophgo,cv1800b-dwcmshc
       - sophgo,sg2002-dwcmshc
+      - sophgo,sg2042-dwcmshc
       - thead,th1520-dwcmshc
 
   reg:
@@ -30,23 +31,36 @@ properties:
     maxItems: 1
 
   clocks:
-    minItems: 1
-    items:
-      - description: core clock
-      - description: bus clock for optional
-      - description: axi clock for rockchip specified
-      - description: block clock for rockchip specified
-      - description: timer clock for rockchip specified
-
+    anyOf:
+      - minItems: 1
+        items:
+          - description: core clock
+          - description: bus clock for optional
+          - description: axi clock for rockchip specified
+          - description: block clock for rockchip specified
+          - description: timer clock for rockchip specified
+
+      - minItems: 1
+        items:
+          - description: core clock
+          - description: timer clock
+          - description: card clock
 
   clock-names:
-    minItems: 1
-    items:
-      - const: core
-      - const: bus
-      - const: axi
-      - const: block
-      - const: timer
+    anyOf:
+      - minItems: 1
+        items:
+          - const: core
+          - const: bus
+          - const: axi
+          - const: block
+          - const: timer
+
+      - minItems: 1
+        items:
+          - const: core
+          - const: timer
+          - const: card
 
   resets:
     maxItems: 5
@@ -96,5 +110,26 @@ examples:
       #address-cells = <1>;
       #size-cells = <0>;
     };
-
+  - |
+    mmc@bb0000 {
+      compatible = "snps,dwcmshc-sdhci";
+      reg = <0xbb000 0x1000>;
+      interrupts = <0 25 0x4>;
+      clocks = <&cru 17>;
+      clock-names = "core";
+      bus-width = <8>;
+      #address-cells = <1>;
+      #size-cells = <0>;
+    };
+  - |
+    mmc@cc0000 {
+      compatible = "snps,dwcmshc-sdhci";
+      reg = <0xcc000 0x1000>;
+      interrupts = <0 25 0x4>;
+      clocks = <&cru 17>, <&cru 18>, <&cru 19>;
+      clock-names = "core", "timer", "card";
+      bus-width = <8>;
+      #address-cells = <1>;
+      #size-cells = <0>;
+    };
 ...
-- 
2.25.1


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

* [PATCH 2/3] mmc: sdhci-of-dwcmshc: Add support for Sophgo SG2042
  2024-04-16  9:50 [PATCH 0/3] mmc: sdhci-of-dwcmshc: support Sophgo SG2042 Chen Wang
  2024-04-16  9:50 ` [PATCH 1/3] dt-bindings: mmc: sdhci-of-dwcmhsc: Add Sophgo SG2042 support Chen Wang
@ 2024-04-16  9:50 ` Chen Wang
  2024-04-16 15:37   ` Jisheng Zhang
  2024-04-16  9:51 ` [PATCH 3/3] riscv: dts: add mmc controllers for Sophgo SG2042 SoC Chen Wang
  2 siblings, 1 reply; 8+ messages in thread
From: Chen Wang @ 2024-04-16  9:50 UTC (permalink / raw)
  To: adrian.hunter, aou, conor+dt, guoren, inochiama, jszhang,
	krzysztof.kozlowski+dt, palmer, paul.walmsley, robh, ulf.hansson,
	devicetree, linux-kernel, linux-mmc, linux-riscv, chao.wei,
	haijiao.liu, xiaoguang.xing, tingzhu.wang
  Cc: Chen Wang

From: Chen Wang <unicorn_wang@outlook.com>

Add support for the mmc controller of Sophgo SG2042.

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

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

diff --git a/drivers/mmc/host/sdhci-of-dwcmshc.c b/drivers/mmc/host/sdhci-of-dwcmshc.c
index 1a0b7ded7f9f..432ce0398163 100644
--- a/drivers/mmc/host/sdhci-of-dwcmshc.c
+++ b/drivers/mmc/host/sdhci-of-dwcmshc.c
@@ -106,12 +106,13 @@
 #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_SN_MASK		GENMASK(23, 20) /* bits [23:20] */
+#define PHY_CNFG_PAD_SN			0x0c /* NMOS TX drive strength */
 
 /* PHY command/response pad settings */
 #define PHY_CMDPAD_CNFG_R	(DWC_MSHC_PTR_PHY_R + 0x04)
@@ -143,7 +144,8 @@
 
 /* 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)
@@ -151,6 +153,9 @@
 #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] */
@@ -194,6 +199,11 @@ struct rk35xx_priv {
 	u8 txclk_tapnum;
 };
 
+#define SG2042_MAX_CLKS 2
+struct sg2042_priv {
+	struct clk_bulk_data clks[SG2042_MAX_CLKS];
+};
+
 struct dwcmshc_priv {
 	struct clk	*bus_clk;
 	int vendor_specific_area1; /* P_VENDOR_SPECIFIC_AREA reg */
@@ -690,6 +700,76 @@ static void cv18xx_sdhci_reset(struct sdhci_host *host, u8 mask)
 	sdhci_writel(host, val, priv->vendor_specific_area1 + CV18XX_SDHCI_PHY_TX_RX_DLY);
 }
 
+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, 9);
+	val |= FIELD_PREP(PHY_CNFG_PAD_SN_MASK, 8);
+	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, 3);
+	val |= FIELD_PREP(PHY_PAD_TXSLEW_CTRL_N_MASK, 2);
+	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, 3);
+	val |= FIELD_PREP(PHY_PAD_TXSLEW_CTRL_N_MASK, 2);
+	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, 3);
+	val |= FIELD_PREP(PHY_PAD_TXSLEW_CTRL_N_MASK, 2);
+	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, 2);
+	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 const struct sdhci_ops sdhci_dwcmshc_ops = {
 	.set_clock		= sdhci_set_clock,
 	.set_bus_width		= sdhci_set_bus_width,
@@ -728,6 +808,16 @@ static const struct sdhci_ops sdhci_dwcmshc_cv18xx_ops = {
 	.adma_write_desc	= dwcmshc_adma_write_desc,
 };
 
+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,
+	.platform_execute_tuning = th1520_execute_tuning,
+	.reset                  = sg2042_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,
@@ -763,6 +853,13 @@ static const struct sdhci_pltfm_data sdhci_dwcmshc_cv18xx_pdata = {
 	.quirks2 = SDHCI_QUIRK2_PRESET_VALUE_BROKEN,
 };
 
+static const struct sdhci_pltfm_data sdhci_dwcmshc_sg2042_pdata = {
+	.ops = &sdhci_dwcmshc_sg2042_ops,
+	.quirks = SDHCI_QUIRK_CAP_CLOCK_BASE_BROKEN |
+		  SDHCI_QUIRK_INVERTED_WRITE_PROTECT,
+	.quirks2 = SDHCI_QUIRK2_PRESET_VALUE_BROKEN,
+};
+
 static void dwcmshc_rk35xx_postinit(struct sdhci_host *host, struct dwcmshc_priv *dwc_priv)
 {
 	/*
@@ -882,6 +979,58 @@ static int dwcmshc_th1520_init(struct device *dev,
 	return 0;
 }
 
+static int dwcmshc_sg2042_clks_enable(struct dwcmshc_priv *dwc_priv)
+{
+	int ret = 0;
+	struct sg2042_priv *soc = dwc_priv->priv;
+
+	if (soc)
+		ret = clk_bulk_prepare_enable(SG2042_MAX_CLKS, soc->clks);
+	return ret;
+}
+
+static void dwcmshc_sg2042_clks_disable(struct dwcmshc_priv *dwc_priv)
+{
+	struct sg2042_priv *soc = dwc_priv->priv;
+
+	if (soc)
+		clk_bulk_disable_unprepare(SG2042_MAX_CLKS,
+					   soc->clks);
+}
+
+static int dwcmshc_sg2042_init(struct device *dev,
+			       struct sdhci_host *host,
+			       struct dwcmshc_priv *dwc_priv)
+{
+	int err;
+	struct sg2042_priv *soc = NULL;
+
+	soc = devm_kzalloc(dev, sizeof(struct sg2042_priv), GFP_KERNEL);
+	if (!soc)
+		return -ENOMEM;
+
+	soc->clks[0].id = "card";
+	soc->clks[1].id = "timer";
+	err = devm_clk_bulk_get_optional(mmc_dev(host->mmc), SG2042_MAX_CLKS,
+					 soc->clks);
+	if (err) {
+		dev_err(mmc_dev(host->mmc), "failed to get clocks %d\n", err);
+		return err;
+	}
+
+	err = clk_bulk_prepare_enable(SG2042_MAX_CLKS, soc->clks);
+	if (err) {
+		dev_err(mmc_dev(host->mmc), "failed to enable clocks %d\n", err);
+		return err;
+	}
+
+	dwc_priv->priv = soc;
+	dwc_priv->soc_clks_enable = dwcmshc_sg2042_clks_enable;
+	dwc_priv->soc_clks_disable = dwcmshc_sg2042_clks_disable;
+
+	return 0;
+}
+
 static const struct of_device_id sdhci_dwcmshc_dt_ids[] = {
 	{
 		.compatible = "rockchip,rk3588-dwcmshc",
@@ -907,6 +1056,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);
@@ -994,6 +1147,12 @@ static int dwcmshc_probe(struct platform_device *pdev)
 			goto err_clk;
 	}
 
+	if (pltfm_data == &sdhci_dwcmshc_sg2042_pdata) {
+		err = dwcmshc_sg2042_init(dev, host, priv);
+		if (err)
+			goto err_clk;
+	}
+
 #ifdef CONFIG_ACPI
 	if (pltfm_data == &sdhci_dwcmshc_bf3_pdata)
 		sdhci_enable_v4_mode(host);
-- 
2.25.1


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

* [PATCH 3/3] riscv: dts: add mmc controllers for Sophgo SG2042 SoC
  2024-04-16  9:50 [PATCH 0/3] mmc: sdhci-of-dwcmshc: support Sophgo SG2042 Chen Wang
  2024-04-16  9:50 ` [PATCH 1/3] dt-bindings: mmc: sdhci-of-dwcmhsc: Add Sophgo SG2042 support Chen Wang
  2024-04-16  9:50 ` [PATCH 2/3] mmc: sdhci-of-dwcmshc: Add support for Sophgo SG2042 Chen Wang
@ 2024-04-16  9:51 ` Chen Wang
  2 siblings, 0 replies; 8+ messages in thread
From: Chen Wang @ 2024-04-16  9:51 UTC (permalink / raw)
  To: adrian.hunter, aou, conor+dt, guoren, inochiama, jszhang,
	krzysztof.kozlowski+dt, palmer, paul.walmsley, robh, ulf.hansson,
	devicetree, linux-kernel, linux-mmc, linux-riscv, chao.wei,
	haijiao.liu, xiaoguang.xing, tingzhu.wang
  Cc: Chen Wang

From: Chen Wang <unicorn_wang@outlook.com>

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

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

diff --git a/arch/riscv/boot/dts/sophgo/sg2042-milkv-pioneer.dts b/arch/riscv/boot/dts/sophgo/sg2042-milkv-pioneer.dts
index 80cb017974d8..c4037eff5c97 100644
--- a/arch/riscv/boot/dts/sophgo/sg2042-milkv-pioneer.dts
+++ b/arch/riscv/boot/dts/sophgo/sg2042-milkv-pioneer.dts
@@ -26,6 +26,21 @@ &cgi_dpll1 {
 	clock-frequency = <25000000>;
 };
 
+&emmc {
+	bus-width = <4>;
+	no-sdio;
+	no-sd;
+	non-removable;
+	status = "okay";
+};
+
+&sd {
+	bus-width = <4>;
+	no-sdio;
+	no-mmc;
+	status = "okay";
+};
+
 &uart0 {
 	status = "okay";
 };
diff --git a/arch/riscv/boot/dts/sophgo/sg2042.dtsi b/arch/riscv/boot/dts/sophgo/sg2042.dtsi
index 8aab027cf730..0b176712a43c 100644
--- a/arch/riscv/boot/dts/sophgo/sg2042.dtsi
+++ b/arch/riscv/boot/dts/sophgo/sg2042.dtsi
@@ -393,5 +393,37 @@ uart0: serial@7040000000 {
 			resets = <&rstgen RST_UART0>;
 			status = "disabled";
 		};
+
+		emmc: mmc@704002a000 {
+			compatible = "sophgo,sg2042-dwcmshc";
+			reg = <0x70 0x4002A000 0x0 0x1000>;
+			interrupt-parent = <&intc>;
+			interrupts = <134 IRQ_TYPE_LEVEL_HIGH>;
+			clocks =
+				<&clkgen GATE_CLK_AXI_EMMC>,
+				<&clkgen GATE_CLK_100K_EMMC>,
+				<&clkgen GATE_CLK_EMMC_100M>;
+			clock-names =
+				"core",
+				"timer",
+				"card";
+			status = "disabled";
+		};
+
+		sd: mmc@704002b000 {
+			compatible = "sophgo,sg2042-dwcmshc";
+			reg = <0x70 0x4002B000 0x0 0x1000>;
+			interrupt-parent = <&intc>;
+			interrupts = <136 IRQ_TYPE_LEVEL_HIGH>;
+			clocks =
+				<&clkgen GATE_CLK_AXI_SD>,
+				<&clkgen GATE_CLK_100K_SD>,
+				<&clkgen GATE_CLK_SD_100M>;
+			clock-names =
+				"core",
+				"timer",
+				"card";
+			status = "disabled";
+		};
 	};
 };
-- 
2.25.1


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

* Re: [PATCH 2/3] mmc: sdhci-of-dwcmshc: Add support for Sophgo SG2042
  2024-04-16  9:50 ` [PATCH 2/3] mmc: sdhci-of-dwcmshc: Add support for Sophgo SG2042 Chen Wang
@ 2024-04-16 15:37   ` Jisheng Zhang
  2024-04-16 23:53     ` Chen Wang
  0 siblings, 1 reply; 8+ messages in thread
From: Jisheng Zhang @ 2024-04-16 15:37 UTC (permalink / raw)
  To: Chen Wang
  Cc: adrian.hunter, aou, conor+dt, guoren, inochiama,
	krzysztof.kozlowski+dt, palmer, paul.walmsley, robh, ulf.hansson,
	devicetree, linux-kernel, linux-mmc, linux-riscv, chao.wei,
	haijiao.liu, xiaoguang.xing, tingzhu.wang, Chen Wang

On Tue, Apr 16, 2024 at 05:50:57PM +0800, Chen Wang wrote:
> From: Chen Wang <unicorn_wang@outlook.com>
> 
> Add support for the mmc controller of Sophgo SG2042.
> 
> SG2042 uses Synopsys PHY the same as TH1520 so we reuse the tuning
> logic from TH1520. Besides this, this patch implement some SG2042
> specific work, such as clocks and reset ops.
> 
> Signed-off-by: Chen Wang <unicorn_wang@outlook.com>
> ---
>  drivers/mmc/host/sdhci-of-dwcmshc.c | 173 ++++++++++++++++++++++++++--
>  1 file changed, 166 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/mmc/host/sdhci-of-dwcmshc.c b/drivers/mmc/host/sdhci-of-dwcmshc.c
> index 1a0b7ded7f9f..432ce0398163 100644
> --- a/drivers/mmc/host/sdhci-of-dwcmshc.c
> +++ b/drivers/mmc/host/sdhci-of-dwcmshc.c
> @@ -106,12 +106,13 @@
>  #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_SN_MASK		GENMASK(23, 20) /* bits [23:20] */
> +#define PHY_CNFG_PAD_SN			0x0c /* NMOS TX drive strength */
>  
>  /* PHY command/response pad settings */
>  #define PHY_CMDPAD_CNFG_R	(DWC_MSHC_PTR_PHY_R + 0x04)
> @@ -143,7 +144,8 @@
>  
>  /* 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)
> @@ -151,6 +153,9 @@
>  #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] */
> @@ -194,6 +199,11 @@ struct rk35xx_priv {
>  	u8 txclk_tapnum;
>  };
>  
> +#define SG2042_MAX_CLKS 2

I don't think "bulk" is suitable here for max 2 clks, no?

> +struct sg2042_priv {
> +	struct clk_bulk_data clks[SG2042_MAX_CLKS];

useless either

> +};
> +
>  struct dwcmshc_priv {
>  	struct clk	*bus_clk;
>  	int vendor_specific_area1; /* P_VENDOR_SPECIFIC_AREA reg */
> @@ -690,6 +700,76 @@ static void cv18xx_sdhci_reset(struct sdhci_host *host, u8 mask)
>  	sdhci_writel(host, val, priv->vendor_specific_area1 + CV18XX_SDHCI_PHY_TX_RX_DLY);
>  }
>  
> +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, 9);
> +	val |= FIELD_PREP(PHY_CNFG_PAD_SN_MASK, 8);
> +	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, 3);
> +	val |= FIELD_PREP(PHY_PAD_TXSLEW_CTRL_N_MASK, 2);
> +	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, 3);
> +	val |= FIELD_PREP(PHY_PAD_TXSLEW_CTRL_N_MASK, 2);
> +	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, 3);
> +	val |= FIELD_PREP(PHY_PAD_TXSLEW_CTRL_N_MASK, 2);
> +	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, 2);

magic "2" needs a meaningful macro definition.

> +	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 const struct sdhci_ops sdhci_dwcmshc_ops = {
>  	.set_clock		= sdhci_set_clock,
>  	.set_bus_width		= sdhci_set_bus_width,
> @@ -728,6 +808,16 @@ static const struct sdhci_ops sdhci_dwcmshc_cv18xx_ops = {
>  	.adma_write_desc	= dwcmshc_adma_write_desc,
>  };
>  
> +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,
> +	.platform_execute_tuning = th1520_execute_tuning,
> +	.reset                  = sg2042_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,
> @@ -763,6 +853,13 @@ static const struct sdhci_pltfm_data sdhci_dwcmshc_cv18xx_pdata = {
>  	.quirks2 = SDHCI_QUIRK2_PRESET_VALUE_BROKEN,
>  };
>  
> +static const struct sdhci_pltfm_data sdhci_dwcmshc_sg2042_pdata = {
> +	.ops = &sdhci_dwcmshc_sg2042_ops,
> +	.quirks = SDHCI_QUIRK_CAP_CLOCK_BASE_BROKEN |
> +		  SDHCI_QUIRK_INVERTED_WRITE_PROTECT,

is "wp-inverted" property better?

> +	.quirks2 = SDHCI_QUIRK2_PRESET_VALUE_BROKEN,
> +};
> +
>  static void dwcmshc_rk35xx_postinit(struct sdhci_host *host, struct dwcmshc_priv *dwc_priv)
>  {
>  	/*
> @@ -882,6 +979,58 @@ static int dwcmshc_th1520_init(struct device *dev,
>  	return 0;
>  }
>  
> +static int dwcmshc_sg2042_clks_enable(struct dwcmshc_priv *dwc_priv)
> +{
> +	int ret = 0;
> +	struct sg2042_priv *soc = dwc_priv->priv;
> +
> +	if (soc)
> +		ret = clk_bulk_prepare_enable(SG2042_MAX_CLKS, soc->clks);
> +	return ret;
> +}
> +
> +static void dwcmshc_sg2042_clks_disable(struct dwcmshc_priv *dwc_priv)
> +{
> +	struct sg2042_priv *soc = dwc_priv->priv;
> +
> +	if (soc)
> +		clk_bulk_disable_unprepare(SG2042_MAX_CLKS,
> +					   soc->clks);
> +}
> +
> +static int dwcmshc_sg2042_init(struct device *dev,
> +			       struct sdhci_host *host,
> +			       struct dwcmshc_priv *dwc_priv)
> +{
> +	int err;
> +	struct sg2042_priv *soc = NULL;
> +
> +	soc = devm_kzalloc(dev, sizeof(struct sg2042_priv), GFP_KERNEL);
> +	if (!soc)
> +		return -ENOMEM;
> +
> +	soc->clks[0].id = "card";
> +	soc->clks[1].id = "timer";

Interesting, only "card" and "timer", so which clk is for clk input of the ip?

> +	err = devm_clk_bulk_get_optional(mmc_dev(host->mmc), SG2042_MAX_CLKS,
> +					 soc->clks);
> +	if (err) {
> +		dev_err(mmc_dev(host->mmc), "failed to get clocks %d\n", err);
> +		return err;
> +	}
> +
> +	err = clk_bulk_prepare_enable(SG2042_MAX_CLKS, soc->clks);
> +	if (err) {
> +		dev_err(mmc_dev(host->mmc), "failed to enable clocks %d\n", err);
> +		return err;
> +	}
> +
> +	dwc_priv->priv = soc;
> +	dwc_priv->soc_clks_enable = dwcmshc_sg2042_clks_enable;
> +	dwc_priv->soc_clks_disable = dwcmshc_sg2042_clks_disable;
> +
> +	return 0;
> +}
> +
>  static const struct of_device_id sdhci_dwcmshc_dt_ids[] = {
>  	{
>  		.compatible = "rockchip,rk3588-dwcmshc",
> @@ -907,6 +1056,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);
> @@ -994,6 +1147,12 @@ static int dwcmshc_probe(struct platform_device *pdev)
>  			goto err_clk;
>  	}
>  
> +	if (pltfm_data == &sdhci_dwcmshc_sg2042_pdata) {
> +		err = dwcmshc_sg2042_init(dev, host, priv);
> +		if (err)
> +			goto err_clk;
> +	}
> +
>  #ifdef CONFIG_ACPI
>  	if (pltfm_data == &sdhci_dwcmshc_bf3_pdata)
>  		sdhci_enable_v4_mode(host);
> -- 
> 2.25.1
> 

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

* Re: [PATCH 1/3] dt-bindings: mmc: sdhci-of-dwcmhsc: Add Sophgo SG2042 support
  2024-04-16  9:50 ` [PATCH 1/3] dt-bindings: mmc: sdhci-of-dwcmhsc: Add Sophgo SG2042 support Chen Wang
@ 2024-04-16 16:44   ` Conor Dooley
  2024-04-17  0:00     ` Chen Wang
  0 siblings, 1 reply; 8+ messages in thread
From: Conor Dooley @ 2024-04-16 16:44 UTC (permalink / raw)
  To: Chen Wang
  Cc: adrian.hunter, aou, conor+dt, guoren, inochiama, jszhang,
	krzysztof.kozlowski+dt, palmer, paul.walmsley, robh, ulf.hansson,
	devicetree, linux-kernel, linux-mmc, linux-riscv, chao.wei,
	haijiao.liu, xiaoguang.xing, tingzhu.wang, Chen Wang

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

On Tue, Apr 16, 2024 at 05:50:37PM +0800, Chen Wang wrote:
> From: Chen Wang <unicorn_wang@outlook.com>
> 
> SG2042 use Synopsys dwcnshc IP for SD/eMMC controllers.
> 
> SG2042 defines 3 clocks for SD/eMMC controllers.
> - AXI_EMMC/AXI_SD for aclk/hclk(Bus interface clocks in DWC_mshc)
>   and blck(Core Base Clock in DWC_mshc), these 3 clocks share one
>   source, so reuse existing "core".
> - 100K_EMMC/100K_SD for cqetmclk(Timer clocks in DWC_mshc), so reuse
>   existing "timer" which was added for rockchip specified.
> - EMMC_100M/SD_100M for cclk(Card clocks in DWC_mshc), add new "card".
> 
> Adding some examples.
> 
> Signed-off-by: Chen Wang <unicorn_wang@outlook.com>
> ---
>  .../bindings/mmc/snps,dwcmshc-sdhci.yaml      | 67 ++++++++++++++-----
>  1 file changed, 51 insertions(+), 16 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/mmc/snps,dwcmshc-sdhci.yaml b/Documentation/devicetree/bindings/mmc/snps,dwcmshc-sdhci.yaml
> index 4d3031d9965f..a04ccae216cf 100644
> --- a/Documentation/devicetree/bindings/mmc/snps,dwcmshc-sdhci.yaml
> +++ b/Documentation/devicetree/bindings/mmc/snps,dwcmshc-sdhci.yaml
> @@ -21,6 +21,7 @@ properties:
>        - snps,dwcmshc-sdhci
>        - sophgo,cv1800b-dwcmshc
>        - sophgo,sg2002-dwcmshc
> +      - sophgo,sg2042-dwcmshc
>        - thead,th1520-dwcmshc
>  
>    reg:
> @@ -30,23 +31,36 @@ properties:
>      maxItems: 1
>  
>    clocks:
> -    minItems: 1
> -    items:
> -      - description: core clock
> -      - description: bus clock for optional
> -      - description: axi clock for rockchip specified
> -      - description: block clock for rockchip specified
> -      - description: timer clock for rockchip specified
> -
> +    anyOf:
> +      - minItems: 1
> +        items:
> +          - description: core clock
> +          - description: bus clock for optional
> +          - description: axi clock for rockchip specified
> +          - description: block clock for rockchip specified
> +          - description: timer clock for rockchip specified
> +
> +      - minItems: 1

I don't think this minItems is needed, this is for one device which has
all 3, no?

I also think this combination should only be permitted for the sg2042,
since it is not valid for the existing devices.

Cheers,
Conor.

> +        items:
> +          - description: core clock
> +          - description: timer clock
> +          - description: card clock
>  
>    clock-names:
> -    minItems: 1
> -    items:
> -      - const: core
> -      - const: bus
> -      - const: axi
> -      - const: block
> -      - const: timer
> +    anyOf:
> +      - minItems: 1
> +        items:
> +          - const: core
> +          - const: bus
> +          - const: axi
> +          - const: block
> +          - const: timer
> +
> +      - minItems: 1
> +        items:
> +          - const: core
> +          - const: timer
> +          - const: card
>  
>    resets:
>      maxItems: 5
> @@ -96,5 +110,26 @@ examples:
>        #address-cells = <1>;
>        #size-cells = <0>;
>      };
> -
> +  - |
> +    mmc@bb0000 {
> +      compatible = "snps,dwcmshc-sdhci";
> +      reg = <0xbb000 0x1000>;
> +      interrupts = <0 25 0x4>;
> +      clocks = <&cru 17>;
> +      clock-names = "core";
> +      bus-width = <8>;
> +      #address-cells = <1>;
> +      #size-cells = <0>;
> +    };
> +  - |
> +    mmc@cc0000 {
> +      compatible = "snps,dwcmshc-sdhci";
> +      reg = <0xcc000 0x1000>;
> +      interrupts = <0 25 0x4>;
> +      clocks = <&cru 17>, <&cru 18>, <&cru 19>;
> +      clock-names = "core", "timer", "card";
> +      bus-width = <8>;
> +      #address-cells = <1>;
> +      #size-cells = <0>;
> +    };
>  ...
> -- 
> 2.25.1
> 

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

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

* Re: [PATCH 2/3] mmc: sdhci-of-dwcmshc: Add support for Sophgo SG2042
  2024-04-16 15:37   ` Jisheng Zhang
@ 2024-04-16 23:53     ` Chen Wang
  0 siblings, 0 replies; 8+ messages in thread
From: Chen Wang @ 2024-04-16 23:53 UTC (permalink / raw)
  To: Jisheng Zhang, Chen Wang
  Cc: adrian.hunter, aou, conor+dt, guoren, inochiama,
	krzysztof.kozlowski+dt, palmer, paul.walmsley, robh, ulf.hansson,
	devicetree, linux-kernel, linux-mmc, linux-riscv, chao.wei,
	haijiao.liu, xiaoguang.xing, tingzhu.wang


On 2024/4/16 23:37, Jisheng Zhang wrote:
> On Tue, Apr 16, 2024 at 05:50:57PM +0800, Chen Wang wrote:
[......]
>>   
>> +#define SG2042_MAX_CLKS 2
> I don't think "bulk" is suitable here for max 2 clks, no?
Without "bulk",  I have to prepare/disable two times for each of clocks 
and handle the exception if first one failed etc. I learn this from 
rockchip, it has 3. Why you think it is  not suitable, please advise me, 
thanks.
>> +struct sg2042_priv {
>> +	struct clk_bulk_data clks[SG2042_MAX_CLKS];
> useless either

Sorry, what's this mean?

[......]

>> +
>> +	/* Set ATDL_CNFG, tuning clk not use for init */
>> +	val = FIELD_PREP(PHY_ATDL_CNFG_INPSEL_MASK, 2);
> magic "2" needs a meaningful macro definition.

Agree, will improve this in next version.

[......]

>>   
>> +static const struct sdhci_pltfm_data sdhci_dwcmshc_sg2042_pdata = {
>> +	.ops = &sdhci_dwcmshc_sg2042_ops,
>> +	.quirks = SDHCI_QUIRK_CAP_CLOCK_BASE_BROKEN |
>> +		  SDHCI_QUIRK_INVERTED_WRITE_PROTECT,
> is "wp-inverted" property better?

Yes, l will use this in next revision, thanks.

[......]

>> +
>> +static int dwcmshc_sg2042_init(struct device *dev,
>> +			       struct sdhci_host *host,
>> +			       struct dwcmshc_priv *dwc_priv)
>> +{
>> +	int err;
>> +	struct sg2042_priv *soc = NULL;
>> +
>> +	soc = devm_kzalloc(dev, sizeof(struct sg2042_priv), GFP_KERNEL);
>> +	if (!soc)
>> +		return -ENOMEM;
>> +
>> +	soc->clks[0].id = "card";
>> +	soc->clks[1].id = "timer";
> Interesting, only "card" and "timer", so which clk is for clk input of the ip?

Copy my comments from bindings patch here for quick reference:

 > SG2042 defines 3 clocks for SD/eMMC controllers.

 >- AXI_EMMC/AXI_SD for aclk/hclk(Bus interface clocks in DWC_mshc)
 >  and bclk(Core Base Clock in DWC_mshc), these 3 clocks share one
 >  source, so reuse existing "core".
 >- 100K_EMMC/100K_SD for cqetmclk(Timer clocks in DWC_mshc), so reuse
 >  existing "timer" which was added for rockchip specified.

 >- EMMC_100M/SD_100M for cclk(Card clocks in DWC_mshc), add new "card".

What you meant "clk input of the ip" is "core", right?

[......]


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

* Re: [PATCH 1/3] dt-bindings: mmc: sdhci-of-dwcmhsc: Add Sophgo SG2042 support
  2024-04-16 16:44   ` Conor Dooley
@ 2024-04-17  0:00     ` Chen Wang
  0 siblings, 0 replies; 8+ messages in thread
From: Chen Wang @ 2024-04-17  0:00 UTC (permalink / raw)
  To: Conor Dooley, Chen Wang
  Cc: adrian.hunter, aou, conor+dt, guoren, inochiama, jszhang,
	krzysztof.kozlowski+dt, palmer, paul.walmsley, robh, ulf.hansson,
	devicetree, linux-kernel, linux-mmc, linux-riscv, chao.wei,
	haijiao.liu, xiaoguang.xing, tingzhu.wang


On 2024/4/17 0:44, Conor Dooley wrote:
> On Tue, Apr 16, 2024 at 05:50:37PM +0800, Chen Wang wrote:
[......]
>> +    anyOf:
>> +      - minItems: 1
>> +        items:
>> +          - description: core clock
>> +          - description: bus clock for optional
>> +          - description: axi clock for rockchip specified
>> +          - description: block clock for rockchip specified
>> +          - description: timer clock for rockchip specified
>> +
>> +      - minItems: 1
> I don't think this minItems is needed, this is for one device which has
> all 3, no?
Yes, SG2042 requires all the 3 clocks presented,  I will remove this 
minItems.
> I also think this combination should only be permitted for the sg2042,
> since it is not valid for the existing devices.
Yes, I will add condition to allow this combination only for sg2042, 
thanks.
> Cheers,
> Conor.

[......]


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

end of thread, other threads:[~2024-04-17  0:00 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-04-16  9:50 [PATCH 0/3] mmc: sdhci-of-dwcmshc: support Sophgo SG2042 Chen Wang
2024-04-16  9:50 ` [PATCH 1/3] dt-bindings: mmc: sdhci-of-dwcmhsc: Add Sophgo SG2042 support Chen Wang
2024-04-16 16:44   ` Conor Dooley
2024-04-17  0:00     ` Chen Wang
2024-04-16  9:50 ` [PATCH 2/3] mmc: sdhci-of-dwcmshc: Add support for Sophgo SG2042 Chen Wang
2024-04-16 15:37   ` Jisheng Zhang
2024-04-16 23:53     ` Chen Wang
2024-04-16  9:51 ` [PATCH 3/3] riscv: dts: add mmc controllers for Sophgo SG2042 SoC Chen Wang

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