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

From: Ulf Hansson

Date: Thu Mar 05 2026 - 06:55:26 EST

On Thu, 26 Feb 2026 at 12:59, Jiayu Du <jiayu.riscv@xxxxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxxxx>
> ---
> drivers/mmc/host/sdhci-of-dwcmshc.c | 288 ++++++++++++++++++++++++++++
> 1 file changed, 288 insertions(+)
>
> diff --git a/drivers/mmc/host/sdhci-of-dwcmshc.c b/drivers/mmc/host/sdhci-of-dwcmshc.c
> index 2b75a36c096b..21c77e908d77 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 */
> + struct regmap *hi_sys_regmap;
> + const struct dwcmshc_k230_match_data *match_data;
> +};
> +
> #define DWCMSHC_MAX_OTHER_CLKS 3
>
> struct dwcmshc_priv {
> @@ -284,12 +311,34 @@ struct dwcmshc_priv {
> };
>
> struct dwcmshc_pltfm_data {
> + const void *match_data;

This makes sense to me!

Although, I realized that dwcmshc_rk35xx_init() could also move its
assignment of "devtype" into this match_data.

Can you please create a follow-up patch to fixup this and to avoid
storing this type of data in two different ways?

> const struct sdhci_pltfm_data pdata;
> const struct cqhci_host_ops *cqhci_host_ops;
> int (*init)(struct device *dev, struct sdhci_host *host, struct dwcmshc_priv *dwc_priv);
> void (*postinit)(struct sdhci_host *host, struct dwcmshc_priv *dwc_priv);
> };
>
> +struct dwcmshc_k230_match_data {
> + bool is_emmc;
> + u32 ctrl_reg;
> + u32 vol_stable_bit;
> + u32 write_prot_bit;
> +};
> +
> +static const struct dwcmshc_k230_match_data k230_emmc_match_data = {
> + .is_emmc = true,
> + .ctrl_reg = SD0_CTRL,
> + .vol_stable_bit = SD0_HOST_REG_VOL_STABLE,
> + .write_prot_bit = SD0_CARD_WRITE_PROT,
> +};
> +
> +static const struct dwcmshc_k230_match_data k230_sdio_match_data = {
> + .is_emmc = false,
> + .ctrl_reg = SD1_CTRL,
> + .vol_stable_bit = SD1_HOST_REG_VOL_STABLE,
> + .write_prot_bit = SD1_CARD_WRITE_PROT,
> +};

[...]

> +
> +static int dwcmshc_k230_init(struct device *dev, struct sdhci_host *host,
> + struct dwcmshc_priv *dwc_priv)
> +{
> + static const char * const clk_ids[] = {"block", "timer", "axi"};
> + const struct dwcmshc_k230_match_data *match_data;
> + const struct dwcmshc_pltfm_data *pltfm_data;
> + struct device_node *usb_phy_node;
> + struct k230_priv *k230_priv;
> + u32 data;
> + int ret;
> +
> + pltfm_data = device_get_match_data(dev);
> +
> + if (!pltfm_data || !pltfm_data->match_data) {
> + dev_err(dev, "No vendor data found for K230\n");
> + return -EINVAL;
> + }
> + match_data = pltfm_data->match_data;

I don't think this should be specific to dwcmshc_k230_init().

Instead I suggest adding a "const void *match_data" to the "struct
dwcmshc_priv" - and copy the pointer in the common dwcmshc_probe()
instead. In this way, all variants will be able to use it.

> +
> + k230_priv = devm_kzalloc(dev, sizeof(struct k230_priv), GFP_KERNEL);
> + if (!k230_priv)
> + return -ENOMEM;
> +
> + k230_priv->match_data = match_data;
> + dwc_priv->priv = k230_priv;
> +
> + usb_phy_node = of_parse_phandle(dev->of_node, "canaan,usb-phy", 0);
> + if (!usb_phy_node)
> + return dev_err_probe(dev, -ENODEV,
> + "Failed to find canaan,usb-phy phandle\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 (match_data->is_emmc) {
> + host->flags &= ~SDHCI_SIGNALING_330;
> + dwc_priv->flags |= FLAG_IO_FIXED_1V8;
> + } else {
> + host->mmc->caps |= MMC_CAP_SD_HIGHSPEED;
> + host->quirks2 |= SDHCI_QUIRK2_NO_1_8_V;
> + }
> +
> + ret = regmap_read(k230_priv->hi_sys_regmap, match_data->ctrl_reg, &data);
> + if (ret)
> + return dev_err_probe(dev, ret, "Failed to read control reg 0x%x\n",
> + match_data->ctrl_reg);
> +
> + data |= match_data->write_prot_bit | match_data->vol_stable_bit;
> + ret = regmap_write(k230_priv->hi_sys_regmap, match_data->ctrl_reg, data);
> + if (ret)
> + return dev_err_probe(dev, ret, "Failed to write control reg 0x%x\n",
> + match_data->ctrl_reg);
> +
> + return 0;
> +}

[...]

Kind regards
Uffe