[PATCH 0/3] Add SDHCI support for Canaan K230 SoC

[PATCH 0/3] Add SDHCI support for Canaan K230 SoC

[Jiayu Du]

This series is based on the k230 usbphy series[1].

This patch series adds SDHCI support for the Canaan K230 SoC,
which uses Synopsys DWCMSHC SDHCI controllers, include MMC0 and MMC1.
The MMC0 supports eMMC, while MMC1 supports SDIO. Detailed information
can be found in the vendor's manual[2].

Link: https://lore.kernel.org/all/20260121145526.14672-1-jiayu.riscv@isrc.iscas.ac.cn/ [1]
Link: https://www.kendryte.com/k230/zh/dev/00_hardware/K230_%E7%A1%AC%E4%BB%B6%E8%AE%BE%E8%AE%A1%E6%8C%87%E5%8D%97.html#mmc [2]

Jiayu Du (3):
  dt-bindings: mmc: Add sdhci support for Canaan k230
  mmc: sdhci-dwcmshc: Add Canaan K230 DWCMSHC controller support
  riscv: dts: canaan: Add mmc nodes for K230

 .../bindings/mmc/snps,dwcmshc-sdhci.yaml      |  22 ++
 .../boot/dts/canaan/k230-canmv-dshanpi.dts    |  17 ++
 .../dts/canaan/k230-canmv-module-dshanpi.dtsi |   7 +
 arch/riscv/boot/dts/canaan/k230.dtsi          |  26 ++
 drivers/mmc/host/sdhci-of-dwcmshc.c           | 247 ++++++++++++++++++
 5 files changed, 319 insertions(+)

-- 
2.52.0

[PATCH 1/3] dt-bindings: mmc: Add sdhci support for Canaan k230

[Jiayu Du]

The Canaan k230 uses the SDHCI from Synopsys. Add compatible strings
to the k230. The k230 has two controllers. MMC0 supports eMMC, while
MMC1 supports SDIO.

Signed-off-by: Jiayu Du <jiayu.riscv@isrc.iscas.ac.cn>
---
 .../bindings/mmc/snps,dwcmshc-sdhci.yaml      | 22 +++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/Documentation/devicetree/bindings/mmc/snps,dwcmshc-sdhci.yaml b/Documentation/devicetree/bindings/mmc/snps,dwcmshc-sdhci.yaml
index 7e7c55dc2440..cab33da3af7d 100644
--- a/Documentation/devicetree/bindings/mmc/snps,dwcmshc-sdhci.yaml
+++ b/Documentation/devicetree/bindings/mmc/snps,dwcmshc-sdhci.yaml
@@ -23,6 +23,8 @@ properties:
           - const: sophgo,sg2044-dwcmshc
           - const: sophgo,sg2042-dwcmshc
       - enum:
+          - canaan,k230-emmc
+          - canaan,k230-sdio
           - rockchip,rk3568-dwcmshc
           - rockchip,rk3588-dwcmshc
           - snps,dwcmshc-sdhci
@@ -87,6 +89,26 @@ required:
 allOf:
   - $ref: mmc-controller.yaml#
 
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - canaan,k230-emmc
+              - canaan,k230-sdio
+    then:
+      properties:
+        clocks:
+          minItems: 2
+          maxItems: 5
+        clock-names:
+          items:
+            - const: core
+            - const: bus
+            - const: axi
+            - const: block
+            - const: timer
+
   - if:
       properties:
         compatible:
-- 
2.52.0

[PATCH 2/3] mmc: sdhci-dwcmshc: Add Canaan K230 DWCMSHC controller support

[Jiayu Du]

Add SDHCI controller driver for Canaan k230 SoC. Implement custom
sdhci_ops for set_clock, phy init, init and reset.

Signed-off-by: Jiayu Du <jiayu.riscv@isrc.iscas.ac.cn>
---
 drivers/mmc/host/sdhci-of-dwcmshc.c | 247 ++++++++++++++++++++++++++++
 1 file changed, 247 insertions(+)

diff --git a/drivers/mmc/host/sdhci-of-dwcmshc.c b/drivers/mmc/host/sdhci-of-dwcmshc.c
index 204830b40587..bc427bfbba25 100644
--- a/drivers/mmc/host/sdhci-of-dwcmshc.c
+++ b/drivers/mmc/host/sdhci-of-dwcmshc.c
@@ -128,9 +128,11 @@
 #define PHY_CNFG_PHY_PWRGOOD_MASK	BIT_MASK(1) /* bit [1] */
 #define PHY_CNFG_PAD_SP_MASK		GENMASK(19, 16) /* bits [19:16] */
 #define PHY_CNFG_PAD_SP			0x0c /* PMOS TX drive strength */
+#define PHY_CNFG_PAD_SP_k230		0x09 /* PMOS TX drive strength for k230 */
 #define PHY_CNFG_PAD_SP_SG2042		0x09 /* PMOS TX drive strength for SG2042 */
 #define PHY_CNFG_PAD_SN_MASK		GENMASK(23, 20) /* bits [23:20] */
 #define PHY_CNFG_PAD_SN			0x0c /* NMOS TX drive strength */
+#define PHY_CNFG_PAD_SN_k230		0x08 /* NMOS TX drive strength for k230 */
 #define PHY_CNFG_PAD_SN_SG2042		0x08 /* NMOS TX drive strength for SG2042 */
 
 /* PHY command/response pad settings */
@@ -153,14 +155,22 @@
 #define PHY_PAD_RXSEL_3V3		0x2 /* Receiver type select for 3.3V */
 
 #define PHY_PAD_WEAKPULL_MASK		GENMASK(4, 3) /* bits [4:3] */
+#define PHY_PAD_WEAKPULL_DISABLED	0x0 /* Weak pull up and pull down disabled */
 #define PHY_PAD_WEAKPULL_PULLUP		0x1 /* Weak pull up enabled */
 #define PHY_PAD_WEAKPULL_PULLDOWN	0x2 /* Weak pull down enabled */
 
 #define PHY_PAD_TXSLEW_CTRL_P_MASK	GENMASK(8, 5) /* bits [8:5] */
 #define PHY_PAD_TXSLEW_CTRL_P		0x3 /* Slew control for P-Type pad TX */
+#define PHY_PAD_TXSLEW_CTRL_P_k230_VAL2	0x2 /* Slew control for P-Type pad TX for k230 */
 #define PHY_PAD_TXSLEW_CTRL_N_MASK	GENMASK(12, 9) /* bits [12:9] */
 #define PHY_PAD_TXSLEW_CTRL_N		0x3 /* Slew control for N-Type pad TX */
 #define PHY_PAD_TXSLEW_CTRL_N_SG2042	0x2 /* Slew control for N-Type pad TX for SG2042 */
+#define PHY_PAD_TXSLEW_CTRL_N_k230_VAL2	0x2 /* Slew control for N-Type pad TX for k230 */
+#define PHY_PAD_TXSLEW_CTRL_N_k230_VAL1	0x1 /* Slew control for N-Type pad TX for k230 */
+
+/* PHY Common DelayLine config settings */
+#define PHY_COMMDL_CNFG			(DWC_MSHC_PTR_PHY_R + 0x1c)
+#define PHY_COMMDL_CNFG_DLSTEP_SEL	BIT(0) /* DelayLine outputs on PAD enabled */
 
 /* PHY CLK delay line settings */
 #define PHY_SDCLKDL_CNFG_R		(DWC_MSHC_PTR_PHY_R + 0x1d)
@@ -174,7 +184,10 @@
 #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_EXTDLY_EN	BIT(0)
 #define PHY_SMPLDL_CNFG_BYPASS_EN	BIT(1)
+#define PHY_SMPLDL_CNFG_INPSEL_MASK	GENMASK(3, 2) /* bits [3:2] */
+#define PHY_SMPLDL_CNFG_INPSEL		0x3 /* delay line input source */
 
 /* PHY drift_cclk_rx delay line configuration setting */
 #define PHY_ATDL_CNFG_R			(DWC_MSHC_PTR_PHY_R + 0x21)
@@ -227,6 +240,14 @@
 /* SMC call for BlueField-3 eMMC RST_N */
 #define BLUEFIELD_SMC_SET_EMMC_RST_N	0x82000007
 
+/* Canaan specific Registers */
+#define SD0_CTRL			0x00
+#define SD0_HOST_REG_VOL_STABLE		BIT(4)
+#define SD0_CARD_WRITE_PROT		BIT(6)
+#define SD1_CTRL			0x08
+#define SD1_HOST_REG_VOL_STABLE		BIT(0)
+#define SD1_CARD_WRITE_PROT		BIT(2)
+
 /* Eswin specific Registers */
 #define EIC7700_CARD_CLK_STABLE		BIT(28)
 #define EIC7700_INT_BCLK_STABLE		BIT(16)
@@ -268,6 +289,12 @@ struct eic7700_priv {
 	unsigned int drive_impedance;
 };
 
+struct k230_priv  {
+	/* Kendryte k230 specific */
+	bool have_phy;
+	struct regmap *hi_sys_regmap;
+};
+
 #define DWCMSHC_MAX_OTHER_CLKS 3
 
 struct dwcmshc_priv {
@@ -1650,6 +1677,201 @@ static int eic7700_init(struct device *dev, struct sdhci_host *host, struct dwcm
 	return 0;
 }
 
+static void dwcmshc_k230_sdhci_set_clock(struct sdhci_host *host, unsigned int clock)
+{
+	u16 clk;
+
+	sdhci_set_clock(host, clock);
+
+	clk = sdhci_readw(host, SDHCI_CLOCK_CONTROL);
+	clk |= SDHCI_PROG_CLOCK_MODE;
+	sdhci_writew(host, clk, SDHCI_CLOCK_CONTROL);
+}
+
+static void sdhci_k230_config_phy_delay(struct sdhci_host *host)
+{
+	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
+	struct dwcmshc_priv *dwc_priv = sdhci_pltfm_priv(pltfm_host);
+	u32 val;
+
+	sdhci_writeb(host, PHY_COMMDL_CNFG_DLSTEP_SEL, PHY_COMMDL_CNFG);
+	sdhci_writeb(host, 0x0, PHY_SDCLKDL_CNFG_R);
+	sdhci_writeb(host, PHY_SDCLKDL_DC_INITIAL,
+		     PHY_SDCLKDL_DC_R);
+
+	val = PHY_SMPLDL_CNFG_EXTDLY_EN;
+	val |= FIELD_PREP(PHY_SMPLDL_CNFG_INPSEL_MASK,
+			  PHY_SMPLDL_CNFG_INPSEL);
+	sdhci_writeb(host, val, PHY_SMPLDL_CNFG_R);
+
+	sdhci_writeb(host, FIELD_PREP(PHY_ATDL_CNFG_INPSEL_MASK,
+		     PHY_ATDL_CNFG_INPSEL), PHY_ATDL_CNFG_R);
+
+	val = sdhci_readl(host, dwc_priv->vendor_specific_area1 +
+		     DWCMSHC_EMMC_ATCTRL);
+	val |= AT_CTRL_TUNE_CLK_STOP_EN;
+	val |= FIELD_PREP(AT_CTRL_PRE_CHANGE_DLY_MASK,
+			  AT_CTRL_PRE_CHANGE_DLY);
+	val |= FIELD_PREP(AT_CTRL_POST_CHANGE_DLY_MASK,
+			  AT_CTRL_POST_CHANGE_DLY);
+	sdhci_writel(host, val, dwc_priv->vendor_specific_area1 +
+		     DWCMSHC_EMMC_ATCTRL);
+	sdhci_writel(host, 0x0, dwc_priv->vendor_specific_area1 +
+		     DWCMSHC_AT_STAT);
+}
+
+static int dwcmshc_k230_phy_init(struct sdhci_host *host)
+{
+	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
+	struct dwcmshc_priv *priv = sdhci_pltfm_priv(pltfm_host);
+	u32 rxsel = PHY_PAD_RXSEL_3V3;
+	unsigned int timeout = 15000;
+	u32 val;
+	u32 reg;
+
+	/* reset phy */
+	sdhci_writew(host, 0, PHY_CNFG_R);
+
+	/* Disable the clock */
+	sdhci_writew(host, 0, SDHCI_CLOCK_CONTROL);
+
+	if (priv->flags & FLAG_IO_FIXED_1V8)
+		rxsel = PHY_PAD_RXSEL_1V8;
+
+	val = rxsel;
+	val |= FIELD_PREP(PHY_PAD_TXSLEW_CTRL_P_MASK, PHY_PAD_TXSLEW_CTRL_P_k230_VAL2);
+	val |= FIELD_PREP(PHY_PAD_TXSLEW_CTRL_N_MASK, PHY_PAD_TXSLEW_CTRL_N_k230_VAL2);
+	val |= FIELD_PREP(PHY_PAD_WEAKPULL_MASK, PHY_PAD_WEAKPULL_PULLUP);
+
+	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 = rxsel;
+	val |= FIELD_PREP(PHY_PAD_TXSLEW_CTRL_P_MASK, PHY_PAD_TXSLEW_CTRL_P_k230_VAL2);
+	val |= FIELD_PREP(PHY_PAD_TXSLEW_CTRL_N_MASK, PHY_PAD_TXSLEW_CTRL_N_k230_VAL2);
+	sdhci_writew(host, val, PHY_CLKPAD_CNFG_R);
+
+	val = rxsel;
+	val |= FIELD_PREP(PHY_PAD_WEAKPULL_MASK, PHY_PAD_WEAKPULL_PULLDOWN);
+	val |= FIELD_PREP(PHY_PAD_TXSLEW_CTRL_P_MASK, PHY_PAD_TXSLEW_CTRL_P_k230_VAL2);
+	val |= FIELD_PREP(PHY_PAD_TXSLEW_CTRL_N_MASK, PHY_PAD_TXSLEW_CTRL_N_k230_VAL2);
+	sdhci_writew(host, val, PHY_STBPAD_CNFG_R);
+
+	sdhci_k230_config_phy_delay(host);
+
+	/* Wait max 150 ms */
+	while (1) {
+		reg = sdhci_readl(host, PHY_CNFG_R);
+		if (reg & FIELD_PREP(PHY_CNFG_PHY_PWRGOOD_MASK, 1))
+			break;
+		if (!timeout)
+			return -ETIMEDOUT;
+		timeout--;
+		usleep_range(10, 15);
+	}
+
+	reg = FIELD_PREP(PHY_CNFG_PAD_SN_MASK, PHY_CNFG_PAD_SN_k230) |
+	      FIELD_PREP(PHY_CNFG_PAD_SP_MASK, PHY_CNFG_PAD_SP_k230);
+	sdhci_writel(host, reg, PHY_CNFG_R);
+
+	/* de-assert the phy */
+	reg |= PHY_CNFG_RSTN_DEASSERT;
+	sdhci_writel(host, reg, PHY_CNFG_R);
+
+	return 0;
+}
+
+static void dwcmshc_k230_sdhci_reset(struct sdhci_host *host, u8 mask)
+{
+	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
+	struct dwcmshc_priv *dwc_priv = sdhci_pltfm_priv(pltfm_host);
+	struct k230_priv *k230_priv = dwc_priv->priv;
+	u8 emmc_ctrl;
+
+	dwcmshc_reset(host, mask);
+
+	if (mask == SDHCI_RESET_ALL) {
+		emmc_ctrl = sdhci_readw(host,
+				       dwc_priv->vendor_specific_area1 +
+				       DWCMSHC_EMMC_CONTROL);
+		sdhci_writeb(host, emmc_ctrl,
+			     dwc_priv->vendor_specific_area1 +
+			     DWCMSHC_EMMC_CONTROL);
+
+		if (k230_priv->have_phy)
+			dwcmshc_k230_phy_init(host);
+		else
+			sdhci_writel(host, 0x0,
+				     dwc_priv->vendor_specific_area1 +
+				     DWCMSHC_HOST_CTRL3);
+	}
+}
+
+static int dwcmshc_k230_init(struct device *dev, struct sdhci_host *host,
+			     struct dwcmshc_priv *dwc_priv)
+{
+	static const char * const clk_ids[] = {"base", "timer", "ahb"};
+	struct device_node *usb_phy_node;
+	struct k230_priv *k230_priv;
+	u32 data;
+	int ret;
+
+	k230_priv = devm_kzalloc(dev, sizeof(struct k230_priv), GFP_KERNEL);
+	if (!k230_priv)
+		return -ENOMEM;
+	dwc_priv->priv = k230_priv;
+
+	usb_phy_node = of_find_compatible_node(NULL, NULL, "canaan,k230-usb-phy");
+	if (!usb_phy_node) {
+		return dev_err_probe(dev, -ENODEV,
+				     "Failed to find k230-usb-phy node\n");
+	}
+
+	k230_priv->hi_sys_regmap = device_node_to_regmap(usb_phy_node);
+	of_node_put(usb_phy_node);
+	if (IS_ERR(k230_priv->hi_sys_regmap)) {
+		return dev_err_probe(dev, PTR_ERR(k230_priv->hi_sys_regmap),
+				     "Failed to get k230-usb-phy regmap\n");
+	}
+
+	ret = dwcmshc_get_enable_other_clks(mmc_dev(host->mmc), dwc_priv,
+					    ARRAY_SIZE(clk_ids), clk_ids);
+	if (ret) {
+		return dev_err_probe(dev, ret,
+				     "Failed to get/enable k230 mmc other clocks\n");
+	}
+
+	if (of_device_is_compatible(dev->of_node, "canaan,k230-sdio")) {
+		k230_priv->have_phy = false;
+		host->mmc->caps |= MMC_CAP_SD_HIGHSPEED;
+		host->quirks2 |= SDHCI_QUIRK2_NO_1_8_V;
+		ret = regmap_read(k230_priv->hi_sys_regmap, SD1_CTRL, &data);
+		if (ret)
+			return dev_err_probe(dev, ret, "Failed to read SD1_CTRL\n");
+
+		data |= SD1_CARD_WRITE_PROT | SD1_HOST_REG_VOL_STABLE;
+		ret = regmap_write(k230_priv->hi_sys_regmap, SD1_CTRL, data);
+		if (ret)
+			return dev_err_probe(dev, ret, "Failed to write SD1_CTRL\n");
+	} else {
+		k230_priv->have_phy = true;
+		host->flags &= ~SDHCI_SIGNALING_330;
+		dwc_priv->flags |= FLAG_IO_FIXED_1V8;
+		ret = regmap_read(k230_priv->hi_sys_regmap, SD0_CTRL, &data);
+		if (ret)
+			return dev_err_probe(dev, ret, "Failed to read SD0_CTRL\n");
+
+		data |= SD0_CARD_WRITE_PROT | SD0_HOST_REG_VOL_STABLE;
+		ret = regmap_write(k230_priv->hi_sys_regmap, SD0_CTRL, data);
+		if (ret)
+			return dev_err_probe(dev, ret, "Failed to write SD0_CTRL\n");
+	}
+	host->quirks |= SDHCI_QUIRK_MULTIBLOCK_READ_ACMD12;
+
+	return 0;
+}
+
 static const struct sdhci_ops sdhci_dwcmshc_ops = {
 	.set_clock		= sdhci_set_clock,
 	.set_bus_width		= sdhci_set_bus_width,
@@ -1736,6 +1958,15 @@ static const struct sdhci_ops sdhci_dwcmshc_eic7700_ops = {
 	.platform_execute_tuning = sdhci_eic7700_executing_tuning,
 };
 
+static const struct sdhci_ops sdhci_dwcmshc_k230_ops = {
+	.set_clock = dwcmshc_k230_sdhci_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 = dwcmshc_k230_sdhci_reset,
+	.adma_write_desc = dwcmshc_adma_write_desc,
+};
+
 static const struct dwcmshc_pltfm_data sdhci_dwcmshc_pdata = {
 	.pdata = {
 		.ops = &sdhci_dwcmshc_ops,
@@ -1827,6 +2058,14 @@ static const struct dwcmshc_pltfm_data sdhci_dwcmshc_eic7700_pdata = {
 	.init = eic7700_init,
 };
 
+static const struct dwcmshc_pltfm_data sdhci_dwcmshc_k230_pdata = {
+	.pdata = {
+		.ops = &sdhci_dwcmshc_k230_ops,
+		.quirks = SDHCI_QUIRK_CAP_CLOCK_BASE_BROKEN,
+	},
+	.init = dwcmshc_k230_init,
+};
+
 static const struct cqhci_host_ops dwcmshc_cqhci_ops = {
 	.enable		= dwcmshc_sdhci_cqe_enable,
 	.disable	= sdhci_cqe_disable,
@@ -1899,6 +2138,14 @@ static void dwcmshc_cqhci_init(struct sdhci_host *host, struct platform_device *
 }
 
 static const struct of_device_id sdhci_dwcmshc_dt_ids[] = {
+	{
+		.compatible = "canaan,k230-emmc",
+		.data = &sdhci_dwcmshc_k230_pdata,
+	},
+	{
+		.compatible = "canaan,k230-sdio",
+		.data = &sdhci_dwcmshc_k230_pdata,
+	},
 	{
 		.compatible = "rockchip,rk3588-dwcmshc",
 		.data = &sdhci_dwcmshc_rk35xx_pdata,
-- 
2.52.0

[PATCH 3/3] riscv: dts: canaan: Add mmc nodes for K230

[Jiayu Du]

Add MMC nodes to K230, including eMMC and SDIO. Enable HS200 eMMC
on the SoM and SDIO high-speed on the board.

Signed-off-by: Jiayu Du <jiayu.riscv@isrc.iscas.ac.cn>
---
 .../boot/dts/canaan/k230-canmv-dshanpi.dts    | 17 ++++++++++++
 .../dts/canaan/k230-canmv-module-dshanpi.dtsi |  7 +++++
 arch/riscv/boot/dts/canaan/k230.dtsi          | 26 +++++++++++++++++++
 3 files changed, 50 insertions(+)

diff --git a/arch/riscv/boot/dts/canaan/k230-canmv-dshanpi.dts b/arch/riscv/boot/dts/canaan/k230-canmv-dshanpi.dts
index 55197cfc25b4..fdf6524db09b 100644
--- a/arch/riscv/boot/dts/canaan/k230-canmv-dshanpi.dts
+++ b/arch/riscv/boot/dts/canaan/k230-canmv-dshanpi.dts
@@ -15,6 +15,8 @@ / {
 
 	aliases {
 		serial0 = &uart0;
+		mmc0 = &emmc;
+		mmc1 = &sdio;
 	};
 
 	chosen {
@@ -75,6 +77,21 @@ uart0-rxd-cfg {
 	};
 };
 
+&emmc {
+	vmmc-supply = <&vdd_3v3>;
+	vqmmc-supply = <&vdd_1v8>;
+};
+
+&sdio {
+	bus-width = <4>;
+	max-frequency = <50000000>;
+	vmmc-supply = <&vdd_3v3>;
+	vqmmc-supply = <&vdd_3v3>;
+	cap-sd-highspeed;
+	no-1-8-v;
+	status = "okay";
+};
+
 &uart0 {
 	pinctrl-names = "default";
 	pinctrl-0 = <&uart0_pins>;
diff --git a/arch/riscv/boot/dts/canaan/k230-canmv-module-dshanpi.dtsi b/arch/riscv/boot/dts/canaan/k230-canmv-module-dshanpi.dtsi
index 566665d39239..1db7569253fe 100644
--- a/arch/riscv/boot/dts/canaan/k230-canmv-module-dshanpi.dtsi
+++ b/arch/riscv/boot/dts/canaan/k230-canmv-module-dshanpi.dtsi
@@ -24,3 +24,10 @@ &sysclk {
 	clocks = <&osc24m>, <&timerx_pulse_in>;
 	clock-names = "osc24m", "timer-pulse-in";
 };
+
+&emmc {
+	bus-width = <8>;
+	max-frequency = <200000000>;
+	mmc-hs200-1_8v;
+	status = "okay";
+};
diff --git a/arch/riscv/boot/dts/canaan/k230.dtsi b/arch/riscv/boot/dts/canaan/k230.dtsi
index b369b7d8dc83..296bf448403d 100644
--- a/arch/riscv/boot/dts/canaan/k230.dtsi
+++ b/arch/riscv/boot/dts/canaan/k230.dtsi
@@ -177,6 +177,32 @@ usb1: usb@91540000 {
 			status = "disabled";
 		};
 
+		emmc: mmc@91580000 {
+			compatible = "canaan,k230-emmc";
+			reg = <0x0 0x91580000 0x0 0x1000>;
+			clocks = <&sysclk K230_HS_SD0_AXI_GATE>,
+				 <&sysclk K230_HS_SD0_CARD_GATE>,
+				 <&sysclk K230_HS_SD0_BASE_GATE>,
+				 <&sysclk K230_HS_SD0_TIMER_GATE>,
+				 <&sysclk K230_HS_SD0_AHB_GATE>;
+			clock-names = "bus", "core", "base", "timer", "ahb";
+			interrupts = <142 IRQ_TYPE_LEVEL_HIGH>;
+			status = "disabled";
+		};
+
+		sdio: mmc@91581000 {
+			compatible = "canaan,k230-sdio";
+			reg = <0x0 0x91581000 0x0 0x1000>;
+			clocks = <&sysclk K230_HS_SD1_AXI_GATE>,
+				 <&sysclk K230_HS_SD1_CARD_GATE>,
+				 <&sysclk K230_HS_SD1_BASE_GATE>,
+				 <&sysclk K230_HS_SD1_TIMER_GATE>,
+				 <&sysclk K230_HS_SD1_AHB_GATE>;
+			clock-names = "bus", "core", "base", "timer", "ahb";
+			interrupts = <144 IRQ_TYPE_LEVEL_HIGH>;
+			status = "disabled";
+		};
+
 		usbphy: usb-phy@91585000 {
 			compatible = "canaan,k230-usb-phy";
 			reg = <0x0 0x91585000 0x0 0x400>;
-- 
2.52.0

Re: [PATCH 2/3] mmc: sdhci-dwcmshc: Add Canaan K230 DWCMSHC controller support

[Yao Zi]

On Wed, Feb 04, 2026 at 04:29:07PM +0800, Jiayu Du wrote:
> Add SDHCI controller driver for Canaan k230 SoC. Implement custom
> sdhci_ops for set_clock, phy init, init and reset.
> 
> Signed-off-by: Jiayu Du <jiayu.riscv@isrc.iscas.ac.cn>
> ---
>  drivers/mmc/host/sdhci-of-dwcmshc.c | 247 ++++++++++++++++++++++++++++
>  1 file changed, 247 insertions(+)
> 
> diff --git a/drivers/mmc/host/sdhci-of-dwcmshc.c b/drivers/mmc/host/sdhci-of-dwcmshc.c
> index 204830b40587..bc427bfbba25 100644
> --- a/drivers/mmc/host/sdhci-of-dwcmshc.c
> +++ b/drivers/mmc/host/sdhci-of-dwcmshc.c

...

> +static void dwcmshc_k230_sdhci_set_clock(struct sdhci_host *host, unsigned int clock)
> +{
> +	u16 clk;
> +
> +	sdhci_set_clock(host, clock);
> +
> +	clk = sdhci_readw(host, SDHCI_CLOCK_CONTROL);
> +	clk |= SDHCI_PROG_CLOCK_MODE;
> +	sdhci_writew(host, clk, SDHCI_CLOCK_CONTROL);

Why it's necessary to always set SDHCI_PROG_CLOCK_MODE? If it's a vendor
quirk, it deserves a comment to explain what's happening here.

> +}

...

> +static int dwcmshc_k230_phy_init(struct sdhci_host *host)
> +{
> +	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> +	struct dwcmshc_priv *priv = sdhci_pltfm_priv(pltfm_host);
> +	u32 rxsel = PHY_PAD_RXSEL_3V3;
> +	unsigned int timeout = 15000;
> +	u32 val;
> +	u32 reg;
> +
> +	/* reset phy */
> +	sdhci_writew(host, 0, PHY_CNFG_R);
> +
> +	/* Disable the clock */
> +	sdhci_writew(host, 0, SDHCI_CLOCK_CONTROL);
> +
> +	if (priv->flags & FLAG_IO_FIXED_1V8)
> +		rxsel = PHY_PAD_RXSEL_1V8;

This may be a little nitpick, but the logic looks cleaner to me if you
do

	rxsel = priv->flags & FLAG_IO_FIXED_1V8 ?
			PHY_PAD_RXSEL_1V8 : PHY_PAD_RXSEL_3V3;

...

> +	/* Wait max 150 ms */
> +	while (1) {
> +		reg = sdhci_readl(host, PHY_CNFG_R);
> +		if (reg & FIELD_PREP(PHY_CNFG_PHY_PWRGOOD_MASK, 1))
> +			break;
> +		if (!timeout)
> +			return -ETIMEDOUT;
> +		timeout--;
> +		usleep_range(10, 15);
> +	}

You could use readx_poll_timeout() with sdhci_readl() to simplify this
loop.

> +	reg = FIELD_PREP(PHY_CNFG_PAD_SN_MASK, PHY_CNFG_PAD_SN_k230) |
> +	      FIELD_PREP(PHY_CNFG_PAD_SP_MASK, PHY_CNFG_PAD_SP_k230);
> +	sdhci_writel(host, reg, PHY_CNFG_R);
> +
> +	/* de-assert the phy */
> +	reg |= PHY_CNFG_RSTN_DEASSERT;
> +	sdhci_writel(host, reg, PHY_CNFG_R);
> +
> +	return 0;
> +}
> +static int dwcmshc_k230_init(struct device *dev, struct sdhci_host *host,
> +			     struct dwcmshc_priv *dwc_priv)
> +{

...

> +	if (!usb_phy_node) {
> +		return dev_err_probe(dev, -ENODEV,
> +				     "Failed to find k230-usb-phy node\n");
> +	}

Please drop the curly braces, if there's only one statement in the body
of an if, they're not necessary. This is also preferred in kernel coding
style,

> Do not unnecessarily use braces where a single statement will do.[1]

Same for branches below.

> +	k230_priv->hi_sys_regmap = device_node_to_regmap(usb_phy_node);
> +	of_node_put(usb_phy_node);
> +	if (IS_ERR(k230_priv->hi_sys_regmap)) {
> +		return dev_err_probe(dev, PTR_ERR(k230_priv->hi_sys_regmap),
> +				     "Failed to get k230-usb-phy regmap\n");
> +	}

...

> +	host->quirks |= SDHCI_QUIRK_MULTIBLOCK_READ_ACMD12;

Why not set it in sdhci_dwcmshc_k230_data.pdata.quirks?

> +	return 0;
> +}

...

> +static const struct dwcmshc_pltfm_data sdhci_dwcmshc_k230_pdata = {
> +	.pdata = {
> +		.ops = &sdhci_dwcmshc_k230_ops,
> +		.quirks = SDHCI_QUIRK_CAP_CLOCK_BASE_BROKEN,
> +	},
> +	.init = dwcmshc_k230_init,
> +};

Best regards,
Yao Zi

[1]: https://elixir.bootlin.com/linux/v6.19-rc5/source/Documentation/process/coding-style.rst#L197

Re: [PATCH 1/3] dt-bindings: mmc: Add sdhci support for Canaan k230

[Conor Dooley]

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

On Wed, Feb 04, 2026 at 04:29:06PM +0800, Jiayu Du wrote:
> The Canaan k230 uses the SDHCI from Synopsys. Add compatible strings
> to the k230. The k230 has two controllers. MMC0 supports eMMC, while
> MMC1 supports SDIO.
> 
> Signed-off-by: Jiayu Du <jiayu.riscv@isrc.iscas.ac.cn>
> ---
>  .../bindings/mmc/snps,dwcmshc-sdhci.yaml      | 22 +++++++++++++++++++
>  1 file changed, 22 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/mmc/snps,dwcmshc-sdhci.yaml b/Documentation/devicetree/bindings/mmc/snps,dwcmshc-sdhci.yaml
> index 7e7c55dc2440..cab33da3af7d 100644
> --- a/Documentation/devicetree/bindings/mmc/snps,dwcmshc-sdhci.yaml
> +++ b/Documentation/devicetree/bindings/mmc/snps,dwcmshc-sdhci.yaml
> @@ -23,6 +23,8 @@ properties:
>            - const: sophgo,sg2044-dwcmshc
>            - const: sophgo,sg2042-dwcmshc
>        - enum:
> +          - canaan,k230-emmc
> +          - canaan,k230-sdio

I don't understand why there are two compatibles here, when the driver
is able to handle them both identically, using the common properties for
mmc controllers to differentiate. Is which is emmc and which sdio
actually just determined by how the k230 boards use them?

Or, if there are pinout differences, would pinctrl allow each to be used
for either?

>            - rockchip,rk3568-dwcmshc
>            - rockchip,rk3588-dwcmshc
>            - snps,dwcmshc-sdhci
> @@ -87,6 +89,26 @@ required:
>  allOf:
>    - $ref: mmc-controller.yaml#
>  
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            enum:
> +              - canaan,k230-emmc
> +              - canaan,k230-sdio
> +    then:
> +      properties:
> +        clocks:
> +          minItems: 2
> +          maxItems: 5
> +        clock-names:
> +          items:
> +            - const: core
> +            - const: bus
> +            - const: axi
> +            - const: block
> +            - const: timer
> +
>    - if:
>        properties:
>          compatible:
> -- 
> 2.52.0
> 

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

Re: [PATCH 1/3] dt-bindings: mmc: Add sdhci support for Canaan k230

[Jiayu Du]

On Wed, Feb 04, 2026 at 06:10:25PM +0000, Conor Dooley wrote:
> On Wed, Feb 04, 2026 at 04:29:06PM +0800, Jiayu Du wrote:
> > The Canaan k230 uses the SDHCI from Synopsys. Add compatible strings
> > to the k230. The k230 has two controllers. MMC0 supports eMMC, while
> > MMC1 supports SDIO.
> > 
> > Signed-off-by: Jiayu Du <jiayu.riscv@isrc.iscas.ac.cn>
> > ---
> >  .../bindings/mmc/snps,dwcmshc-sdhci.yaml      | 22 +++++++++++++++++++
> >  1 file changed, 22 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/mmc/snps,dwcmshc-sdhci.yaml b/Documentation/devicetree/bindings/mmc/snps,dwcmshc-sdhci.yaml
> > index 7e7c55dc2440..cab33da3af7d 100644
> > --- a/Documentation/devicetree/bindings/mmc/snps,dwcmshc-sdhci.yaml
> > +++ b/Documentation/devicetree/bindings/mmc/snps,dwcmshc-sdhci.yaml
> > @@ -23,6 +23,8 @@ properties:
> >            - const: sophgo,sg2044-dwcmshc
> >            - const: sophgo,sg2042-dwcmshc
> >        - enum:
> > +          - canaan,k230-emmc
> > +          - canaan,k230-sdio
> 
> I don't understand why there are two compatibles here, when the driver
> is able to handle them both identically, using the common properties for
> mmc controllers to differentiate. Is which is emmc and which sdio
> actually just determined by how the k230 boards use them?
> 
> Or, if there are pinout differences, would pinctrl allow each to be used
> for either?

Hi, Thanks for the review!

Actually, the two compatibles aren't just board choices. They come from
real SoC-level differences between the two MMC controllers.

From the K230 Hardware Guide [1]:
- MMC0 supports eMMC5.0 and SDIO3.0, usually for eMMC chips.
- MMC1 only does SDIO3.0 in 4/1-bit mode up to SDR104, and the manual
  clearly says it can't handle eMMC because of pin count and limits.

And the driver treats them a bit differently, such as whether there
is a phy and the initialization method. So I think keeping separate
compatibles makes sense to show these hardware differences clearly.

About pinctrl, MMC1 pins can mux to other things like GPIO, but MMC0
pins are fixed in hardware. In next version, I will add pinctrl node
for MMC1 in the board dts.

Link: https://github.com/kendryte/k230_docs/blob/main/en/00_hardware/K230_Hardware_Design_Guide.md#mmc-circuit [1]

Regards,
Jiayu Du

> 
> >            - rockchip,rk3568-dwcmshc
> >            - rockchip,rk3588-dwcmshc
> >            - snps,dwcmshc-sdhci
> > @@ -87,6 +89,26 @@ required:
> >  allOf:
> >    - $ref: mmc-controller.yaml#
> >  
> > +  - if:
> > +      properties:
> > +        compatible:
> > +          contains:
> > +            enum:
> > +              - canaan,k230-emmc
> > +              - canaan,k230-sdio
> > +    then:
> > +      properties:
> > +        clocks:
> > +          minItems: 2
> > +          maxItems: 5
> > +        clock-names:
> > +          items:
> > +            - const: core
> > +            - const: bus
> > +            - const: axi
> > +            - const: block
> > +            - const: timer
> > +
> >    - if:
> >        properties:
> >          compatible:
> > -- 
> > 2.52.0
> > 

Re: [PATCH 1/3] dt-bindings: mmc: Add sdhci support for Canaan k230

[Conor Dooley]

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

On Thu, Feb 05, 2026 at 03:13:00PM +0800, Jiayu Du wrote:
> On Wed, Feb 04, 2026 at 06:10:25PM +0000, Conor Dooley wrote:
> > On Wed, Feb 04, 2026 at 04:29:06PM +0800, Jiayu Du wrote:
> > > The Canaan k230 uses the SDHCI from Synopsys. Add compatible strings
> > > to the k230. The k230 has two controllers. MMC0 supports eMMC, while
> > > MMC1 supports SDIO.
> > > 
> > > Signed-off-by: Jiayu Du <jiayu.riscv@isrc.iscas.ac.cn>
> > > ---
> > >  .../bindings/mmc/snps,dwcmshc-sdhci.yaml      | 22 +++++++++++++++++++
> > >  1 file changed, 22 insertions(+)
> > > 
> > > diff --git a/Documentation/devicetree/bindings/mmc/snps,dwcmshc-sdhci.yaml b/Documentation/devicetree/bindings/mmc/snps,dwcmshc-sdhci.yaml
> > > index 7e7c55dc2440..cab33da3af7d 100644
> > > --- a/Documentation/devicetree/bindings/mmc/snps,dwcmshc-sdhci.yaml
> > > +++ b/Documentation/devicetree/bindings/mmc/snps,dwcmshc-sdhci.yaml
> > > @@ -23,6 +23,8 @@ properties:
> > >            - const: sophgo,sg2044-dwcmshc
> > >            - const: sophgo,sg2042-dwcmshc
> > >        - enum:
> > > +          - canaan,k230-emmc
> > > +          - canaan,k230-sdio
> > 
> > I don't understand why there are two compatibles here, when the driver
> > is able to handle them both identically, using the common properties for
> > mmc controllers to differentiate. Is which is emmc and which sdio
> > actually just determined by how the k230 boards use them?
> > 
> > Or, if there are pinout differences, would pinctrl allow each to be used
> > for either?
> 
> Hi, Thanks for the review!
> 
> Actually, the two compatibles aren't just board choices. They come from
> real SoC-level differences between the two MMC controllers.
> 
> From the K230 Hardware Guide [1]:
> - MMC0 supports eMMC5.0 and SDIO3.0, usually for eMMC chips.
> - MMC1 only does SDIO3.0 in 4/1-bit mode up to SDR104, and the manual
>   clearly says it can't handle eMMC because of pin count and limits.
> 
> And the driver treats them a bit differently, such as whether there
> is a phy and the initialization method. So I think keeping separate
> compatibles makes sense to show these hardware differences clearly.

Ah, I didn't notice that there was some logic, I just checked the match
data and I missed that. What you have is fine then I think.

> 
> About pinctrl, MMC1 pins can mux to other things like GPIO, but MMC0
> pins are fixed in hardware. In next version, I will add pinctrl node
> for MMC1 in the board dts.
> 
> Link: https://github.com/kendryte/k230_docs/blob/main/en/00_hardware/K230_Hardware_Design_Guide.md#mmc-circuit [1]
> > 
> > >            - rockchip,rk3568-dwcmshc
> > >            - rockchip,rk3588-dwcmshc
> > >            - snps,dwcmshc-sdhci
> > > @@ -87,6 +89,26 @@ required:
> > >  allOf:
> > >    - $ref: mmc-controller.yaml#
> > >  
> > > +  - if:
> > > +      properties:
> > > +        compatible:
> > > +          contains:
> > > +            enum:
> > > +              - canaan,k230-emmc
> > > +              - canaan,k230-sdio
> > > +    then:
> > > +      properties:
> > > +        clocks:
> > > +          minItems: 2
> > > +          maxItems: 5

Additionally, why does this have a range? Why is it not minItems: 5?
You've got only one instance per compatible, so the number of clocks for
each compatible is not variable.

> > > +        clock-names:
> > > +          items:
> > > +            - const: core
> > > +            - const: bus
> > > +            - const: axi
> > > +            - const: block
> > > +            - const: timer
> > > +
> > >    - if:
> > >        properties:
> > >          compatible:
> > > -- 
> > > 2.52.0
> > > 
> 
> 

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

Re: [PATCH 1/3] dt-bindings: mmc: Add sdhci support for Canaan k230

[Jiayu Du]

On Thu, Feb 05, 2026 at 07:19:36PM +0000, Conor Dooley wrote:
> On Thu, Feb 05, 2026 at 03:13:00PM +0800, Jiayu Du wrote:
> > On Wed, Feb 04, 2026 at 06:10:25PM +0000, Conor Dooley wrote:
> > > On Wed, Feb 04, 2026 at 04:29:06PM +0800, Jiayu Du wrote:
> > > > The Canaan k230 uses the SDHCI from Synopsys. Add compatible strings
> > > > to the k230. The k230 has two controllers. MMC0 supports eMMC, while
> > > > MMC1 supports SDIO.
> > > > 
> > > > Signed-off-by: Jiayu Du <jiayu.riscv@isrc.iscas.ac.cn>
> > > > ---
> > > >  .../bindings/mmc/snps,dwcmshc-sdhci.yaml      | 22 +++++++++++++++++++
> > > >  1 file changed, 22 insertions(+)
> > > > 
> > > > diff --git a/Documentation/devicetree/bindings/mmc/snps,dwcmshc-sdhci.yaml b/Documentation/devicetree/bindings/mmc/snps,dwcmshc-sdhci.yaml
> > > > index 7e7c55dc2440..cab33da3af7d 100644
> > > > --- a/Documentation/devicetree/bindings/mmc/snps,dwcmshc-sdhci.yaml
> > > > +++ b/Documentation/devicetree/bindings/mmc/snps,dwcmshc-sdhci.yaml
> > > > @@ -23,6 +23,8 @@ properties:
> > > >            - const: sophgo,sg2044-dwcmshc
> > > >            - const: sophgo,sg2042-dwcmshc
> > > >        - enum:
> > > > +          - canaan,k230-emmc
> > > > +          - canaan,k230-sdio
> > > 
> > > I don't understand why there are two compatibles here, when the driver
> > > is able to handle them both identically, using the common properties for
> > > mmc controllers to differentiate. Is which is emmc and which sdio
> > > actually just determined by how the k230 boards use them?
> > > 
> > > Or, if there are pinout differences, would pinctrl allow each to be used
> > > for either?
> > 
> > Hi, Thanks for the review!
> > 
> > Actually, the two compatibles aren't just board choices. They come from
> > real SoC-level differences between the two MMC controllers.
> > 
> > From the K230 Hardware Guide [1]:
> > - MMC0 supports eMMC5.0 and SDIO3.0, usually for eMMC chips.
> > - MMC1 only does SDIO3.0 in 4/1-bit mode up to SDR104, and the manual
> >   clearly says it can't handle eMMC because of pin count and limits.
> > 
> > And the driver treats them a bit differently, such as whether there
> > is a phy and the initialization method. So I think keeping separate
> > compatibles makes sense to show these hardware differences clearly.
> 
> Ah, I didn't notice that there was some logic, I just checked the match
> data and I missed that. What you have is fine then I think.
> 
> > 
> > About pinctrl, MMC1 pins can mux to other things like GPIO, but MMC0
> > pins are fixed in hardware. In next version, I will add pinctrl node
> > for MMC1 in the board dts.
> > 
> > Link: https://github.com/kendryte/k230_docs/blob/main/en/00_hardware/K230_Hardware_Design_Guide.md#mmc-circuit [1]
> > > 
> > > >            - rockchip,rk3568-dwcmshc
> > > >            - rockchip,rk3588-dwcmshc
> > > >            - snps,dwcmshc-sdhci
> > > > @@ -87,6 +89,26 @@ required:
> > > >  allOf:
> > > >    - $ref: mmc-controller.yaml#
> > > >  
> > > > +  - if:
> > > > +      properties:
> > > > +        compatible:
> > > > +          contains:
> > > > +            enum:
> > > > +              - canaan,k230-emmc
> > > > +              - canaan,k230-sdio
> > > > +    then:
> > > > +      properties:
> > > > +        clocks:
> > > > +          minItems: 2
> > > > +          maxItems: 5
> 
> Additionally, why does this have a range? Why is it not minItems: 5?
> You've got only one instance per compatible, so the number of clocks for
> each compatible is not variable.

You're right. In the next version, I will change it to minItems: 5.
Thank you for your review!

Regards,
Jiayu Du

> > > > +        clock-names:
> > > > +          items:
> > > > +            - const: core
> > > > +            - const: bus
> > > > +            - const: axi
> > > > +            - const: block
> > > > +            - const: timer
> > > > +
> > > >    - if:
> > > >        properties:
> > > >          compatible:
> > > > -- 
> > > > 2.52.0
> > > > 

Re: [PATCH 2/3] mmc: sdhci-dwcmshc: Add Canaan K230 DWCMSHC controller support

[Krzysztof Kozlowski]

On 04/02/2026 09:29, Jiayu Du wrote:
> +static int dwcmshc_k230_init(struct device *dev, struct sdhci_host *host,
> +			     struct dwcmshc_priv *dwc_priv)
> +{
> +	static const char * const clk_ids[] = {"base", "timer", "ahb"};
> +	struct device_node *usb_phy_node;
> +	struct k230_priv *k230_priv;
> +	u32 data;
> +	int ret;
> +
> +	k230_priv = devm_kzalloc(dev, sizeof(struct k230_priv), GFP_KERNEL);
> +	if (!k230_priv)
> +		return -ENOMEM;
> +	dwc_priv->priv = k230_priv;
> +
> +	usb_phy_node = of_find_compatible_node(NULL, NULL, "canaan,k230-usb-phy");

Hm? You should use phandles, not look for various nodes.

> +	if (!usb_phy_node) {

Please follow Linux coding style.

> +		return dev_err_probe(dev, -ENODEV,
> +				     "Failed to find k230-usb-phy node\n");
> +	}
> +
> +	k230_priv->hi_sys_regmap = device_node_to_regmap(usb_phy_node);
> +	of_node_put(usb_phy_node);
> +	if (IS_ERR(k230_priv->hi_sys_regmap)) {
> +		return dev_err_probe(dev, PTR_ERR(k230_priv->hi_sys_regmap),
> +				     "Failed to get k230-usb-phy regmap\n");
> +	}
> +
> +	ret = dwcmshc_get_enable_other_clks(mmc_dev(host->mmc), dwc_priv,
> +					    ARRAY_SIZE(clk_ids), clk_ids);
> +	if (ret) {
> +		return dev_err_probe(dev, ret,
> +				     "Failed to get/enable k230 mmc other clocks\n");
> +	}
> +
> +	if (of_device_is_compatible(dev->of_node, "canaan,k230-sdio")) {

Driver match data is for this.


Best regards,
Krzysztof

Re: [PATCH 2/3] mmc: sdhci-dwcmshc: Add Canaan K230 DWCMSHC controller support

[Jiayu Du]

On Fri, Feb 06, 2026 at 02:26:40PM +0100, Krzysztof Kozlowski wrote:
> On 04/02/2026 09:29, Jiayu Du wrote:
> > +static int dwcmshc_k230_init(struct device *dev, struct sdhci_host *host,
> > +			     struct dwcmshc_priv *dwc_priv)
> > +{
> > +	static const char * const clk_ids[] = {"base", "timer", "ahb"};
> > +	struct device_node *usb_phy_node;
> > +	struct k230_priv *k230_priv;
> > +	u32 data;
> > +	int ret;
> > +
> > +	k230_priv = devm_kzalloc(dev, sizeof(struct k230_priv), GFP_KERNEL);
> > +	if (!k230_priv)
> > +		return -ENOMEM;
> > +	dwc_priv->priv = k230_priv;
> > +
> > +	usb_phy_node = of_find_compatible_node(NULL, NULL, "canaan,k230-usb-phy");
> 
> Hm? You should use phandles, not look for various nodes.

Only one usbphy node has the canaan, k230-usb-phy compatibility.
So in this situation, is it ok to continue using of_find_compatible_node?

> > +	if (!usb_phy_node) {
> 
> Please follow Linux coding style.

I will fix it in next version.

> > +		return dev_err_probe(dev, -ENODEV,
> > +				     "Failed to find k230-usb-phy node\n");
> > +	}
> > +
> > +	k230_priv->hi_sys_regmap = device_node_to_regmap(usb_phy_node);
> > +	of_node_put(usb_phy_node);
> > +	if (IS_ERR(k230_priv->hi_sys_regmap)) {
> > +		return dev_err_probe(dev, PTR_ERR(k230_priv->hi_sys_regmap),
> > +				     "Failed to get k230-usb-phy regmap\n");
> > +	}
> > +
> > +	ret = dwcmshc_get_enable_other_clks(mmc_dev(host->mmc), dwc_priv,
> > +					    ARRAY_SIZE(clk_ids), clk_ids);
> > +	if (ret) {
> > +		return dev_err_probe(dev, ret,
> > +				     "Failed to get/enable k230 mmc other clocks\n");
> > +	}
> > +
> > +	if (of_device_is_compatible(dev->of_node, "canaan,k230-sdio")) {
> 
> Driver match data is for this.

What you mean is that I shouldn't use of_find_compatible_node, but I can
use device_get_match_data instead? Then I can continue to distinguish
between SDIO and eMMC to do parameter configuration

Or do you mean that I should put the parameters to be adjusted into the
pdata structure? But currently, the dwcmshc structure is not suitable for
containing vendor-specific properties.

Regards,
Jiayu Du
> 
> Best regards,
> Krzysztof

Re: [PATCH 2/3] mmc: sdhci-dwcmshc: Add Canaan K230 DWCMSHC controller support

[Krzysztof Kozlowski]

On 07/02/2026 09:45, Jiayu Du wrote:
> On Fri, Feb 06, 2026 at 02:26:40PM +0100, Krzysztof Kozlowski wrote:
>> On 04/02/2026 09:29, Jiayu Du wrote:
>>> +static int dwcmshc_k230_init(struct device *dev, struct sdhci_host *host,
>>> +			     struct dwcmshc_priv *dwc_priv)
>>> +{
>>> +	static const char * const clk_ids[] = {"base", "timer", "ahb"};
>>> +	struct device_node *usb_phy_node;
>>> +	struct k230_priv *k230_priv;
>>> +	u32 data;
>>> +	int ret;
>>> +
>>> +	k230_priv = devm_kzalloc(dev, sizeof(struct k230_priv), GFP_KERNEL);
>>> +	if (!k230_priv)
>>> +		return -ENOMEM;
>>> +	dwc_priv->priv = k230_priv;
>>> +
>>> +	usb_phy_node = of_find_compatible_node(NULL, NULL, "canaan,k230-usb-phy");
>>
>> Hm? You should use phandles, not look for various nodes.
> 
> Only one usbphy node has the canaan, k230-usb-phy compatibility.
> So in this situation, is it ok to continue using of_find_compatible_node?

Amount of nodes does not matter. This is not how you express
links/dependencies between devices. Phandle is for this. This is wrong
on many levels, including missing device links, bypassing kernel API/layers.


> 
>>> +	if (!usb_phy_node) {
>>
>> Please follow Linux coding style.
> 
> I will fix it in next version.
> 
>>> +		return dev_err_probe(dev, -ENODEV,
>>> +				     "Failed to find k230-usb-phy node\n");
>>> +	}
>>> +
>>> +	k230_priv->hi_sys_regmap = device_node_to_regmap(usb_phy_node);
>>> +	of_node_put(usb_phy_node);
>>> +	if (IS_ERR(k230_priv->hi_sys_regmap)) {
>>> +		return dev_err_probe(dev, PTR_ERR(k230_priv->hi_sys_regmap),
>>> +				     "Failed to get k230-usb-phy regmap\n");
>>> +	}
>>> +
>>> +	ret = dwcmshc_get_enable_other_clks(mmc_dev(host->mmc), dwc_priv,
>>> +					    ARRAY_SIZE(clk_ids), clk_ids);
>>> +	if (ret) {
>>> +		return dev_err_probe(dev, ret,
>>> +				     "Failed to get/enable k230 mmc other clocks\n");
>>> +	}
>>> +
>>> +	if (of_device_is_compatible(dev->of_node, "canaan,k230-sdio")) {
>>
>> Driver match data is for this.
> 
> What you mean is that I shouldn't use of_find_compatible_node, but I can
> use device_get_match_data instead? Then I can continue to distinguish
> between SDIO and eMMC to do parameter configuration
> 
> Or do you mean that I should put the parameters to be adjusted into the
> pdata structure? But currently, the dwcmshc structure is not suitable for
> containing vendor-specific properties.

Parameters should go to driver match data. I already requested this for
some other driver and this has to be fixed.

Best regards,
Krzysztof

Re: [PATCH 2/3] mmc: sdhci-dwcmshc: Add Canaan K230 DWCMSHC controller support

[Jiayu Du]

On Sat, Feb 07, 2026 at 10:32:55AM +0100, Krzysztof Kozlowski wrote:
> On 07/02/2026 09:45, Jiayu Du wrote:
> > On Fri, Feb 06, 2026 at 02:26:40PM +0100, Krzysztof Kozlowski wrote:
> >> On 04/02/2026 09:29, Jiayu Du wrote:
> >>> +static int dwcmshc_k230_init(struct device *dev, struct sdhci_host *host,
> >>> +			     struct dwcmshc_priv *dwc_priv)
> >>> +{
> >>> +	static const char * const clk_ids[] = {"base", "timer", "ahb"};
> >>> +	struct device_node *usb_phy_node;
> >>> +	struct k230_priv *k230_priv;
> >>> +	u32 data;
> >>> +	int ret;
> >>> +
> >>> +	k230_priv = devm_kzalloc(dev, sizeof(struct k230_priv), GFP_KERNEL);
> >>> +	if (!k230_priv)
> >>> +		return -ENOMEM;
> >>> +	dwc_priv->priv = k230_priv;
> >>> +
> >>> +	usb_phy_node = of_find_compatible_node(NULL, NULL, "canaan,k230-usb-phy");
> >>
> >> Hm? You should use phandles, not look for various nodes.
> > 
> > Only one usbphy node has the canaan, k230-usb-phy compatibility.
> > So in this situation, is it ok to continue using of_find_compatible_node?
> 
> Amount of nodes does not matter. This is not how you express
> links/dependencies between devices. Phandle is for this. This is wrong
> on many levels, including missing device links, bypassing kernel API/layers.

Thank you for your review. I will fix it.

> 
> 
> > 
> >>> +	if (!usb_phy_node) {
> >>
> >> Please follow Linux coding style.
> > 
> > I will fix it in next version.
> > 
> >>> +		return dev_err_probe(dev, -ENODEV,
> >>> +				     "Failed to find k230-usb-phy node\n");
> >>> +	}
> >>> +
> >>> +	k230_priv->hi_sys_regmap = device_node_to_regmap(usb_phy_node);
> >>> +	of_node_put(usb_phy_node);
> >>> +	if (IS_ERR(k230_priv->hi_sys_regmap)) {
> >>> +		return dev_err_probe(dev, PTR_ERR(k230_priv->hi_sys_regmap),
> >>> +				     "Failed to get k230-usb-phy regmap\n");
> >>> +	}
> >>> +
> >>> +	ret = dwcmshc_get_enable_other_clks(mmc_dev(host->mmc), dwc_priv,
> >>> +					    ARRAY_SIZE(clk_ids), clk_ids);
> >>> +	if (ret) {
> >>> +		return dev_err_probe(dev, ret,
> >>> +				     "Failed to get/enable k230 mmc other clocks\n");
> >>> +	}
> >>> +
> >>> +	if (of_device_is_compatible(dev->of_node, "canaan,k230-sdio")) {
> >>
> >> Driver match data is for this.
> > 
> > What you mean is that I shouldn't use of_find_compatible_node, but I can
> > use device_get_match_data instead? Then I can continue to distinguish
> > between SDIO and eMMC to do parameter configuration
> > 
> > Or do you mean that I should put the parameters to be adjusted into the
> > pdata structure? But currently, the dwcmshc structure is not suitable for
> > containing vendor-specific properties.
> 
> Parameters should go to driver match data. I already requested this for
> some other driver and this has to be fixed.

I will make fix to enable dwcmshc_pltfm_data to support the addition of
vendor-specific properties. And if possible, could you give me with some
examples? I would be very grateful.

> 
> Best regards,
> Krzysztof