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

Jiayu Du
2026-03-12T13:32:45Z

On Wed, Mar 11, 2026 at 04:04:03PM +0200, Adrian Hunter wrote:
> On 10/03/2026 08:45, 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 | 287 ++++++++++++++++++++++++++++
> > 1 file changed, 287 insertions(+)
> >
> > diff --git a/drivers/mmc/host/sdhci-of-dwcmshc.c b/drivers/mmc/host/sdhci-of-dwcmshc.c
> > index 2b75a36c096b..39d28574cfa8 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 {
> > @@ -275,6 +302,7 @@ struct dwcmshc_priv {
> > int vendor_specific_area1; /* P_VENDOR_SPECIFIC_AREA1 reg */
> > int vendor_specific_area2; /* P_VENDOR_SPECIFIC_AREA2 reg */
> >
> > + const void *match_data;
> > int num_other_clks;
> > struct clk_bulk_data other_clks[DWCMSHC_MAX_OTHER_CLKS];
> >
> > @@ -284,12 +312,34 @@ struct dwcmshc_priv {
> > };
> >
> > struct dwcmshc_pltfm_data {
> > + const void *match_data;
> > 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 void dwcmshc_enable_card_clk(struct sdhci_host *host)
> > {
> > u16 ctrl;
> > @@ -1656,6 +1706,205 @@ 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);
> > + /*
> > + * It is necessary to enable SDHCI_PROG_CLOCK_MODE. This is a
> > + * vendor-specific quirk. If this is not done, the eMMC will be
> > + * unable to read or write.
> > + */
> > + 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);
>
> Here and elsewhere. Please avoid unnecessary line wrapping.
> You can use 100 columns.

Thank you. I will correct all unnecessary line wrapping.

> > +
> > + 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);
>
> Can keep FIELD_PREP() all on one line:
>
> sdhci_writeb(host, FIELD_PREP(PHY_ATDL_CNFG_INPSEL_MASK, PHY_ATDL_CNFG_INPSEL),
> PHY_ATDL_CNFG_R);
>
> > +
> > + ret = read_poll_timeout(sdhci_readl, reg,
> > + (reg & FIELD_PREP(
> > + PHY_CNFG_PHY_PWRGOOD_MASK, 1)),
>
> Need not line wrap after "FIELD_PREP("
>
> > + if (mask == SDHCI_RESET_ALL) {
>
> Can reduce the indent by instead:
>
> if (mask != SDHCI_RESET_ALL)
> return;
>
> Then the remaining lines in this function do not need to be
> wrapped.
>
> > @@ -1834,6 +2092,26 @@ static const struct dwcmshc_pltfm_data sdhci_dwcmshc_eic7700_pdata = {
> > .init = eic7700_init,
> > };
> >
> > +static const struct dwcmshc_pltfm_data sdhci_dwcmshc_k230_emmc_pdata = {
> > + .pdata = {
> > + .ops = &sdhci_dwcmshc_k230_ops,
> > + .quirks = SDHCI_QUIRK_CAP_CLOCK_BASE_BROKEN |
> > + SDHCI_QUIRK_MULTIBLOCK_READ_ACMD12,
> > + },
> > + .init = dwcmshc_k230_init,
> > + .match_data = (void *)&k230_emmc_match_data,
> > +};
>
> Just an option if you like it:
> An alternative is to define your own platform data e.g.
>
> struct k230_pltfm_data {
> struct dwcmshc_pltfm_data dwcmshc_pdata;
> bool is_emmc;
> u32 ctrl_reg;
> u32 vol_stable_bit;
> u32 write_prot_bit;
> };
>
> static const struct k230_pltfm_data k230_emmc_data = {
> .dwcmshc_pdata = {
> .pdata = {
> .ops = &sdhci_dwcmshc_k230_ops,
> .quirks = SDHCI_QUIRK_CAP_CLOCK_BASE_BROKEN |
> SDHCI_QUIRK_MULTIBLOCK_READ_ACMD12,
> },
> .init = dwcmshc_k230_init,
> },
> .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 k230_pltfm_data k230_sdio_data = {
> .dwcmshc_pdata = {
> .pdata = {
> .ops = &sdhci_dwcmshc_k230_ops,
> .quirks = SDHCI_QUIRK_CAP_CLOCK_BASE_BROKEN |
> SDHCI_QUIRK_MULTIBLOCK_READ_ACMD12,
> },
> .init = dwcmshc_k230_init,
> },
> .is_emmc = false,
> .ctrl_reg = SD1_CTRL,
> .vol_stable_bit = SD1_HOST_REG_VOL_STABLE,
> .write_prot_bit = SD1_CARD_WRITE_PROT,
> };
>
> Then:
> .compatible = "canaan,k230-emmc",
> .data = &k230_emmc_data.dwcmshc_pdata,
> and
> .compatible = "canaan,k230-sdio",
> .data = &k230_sdio_data.dwcmshc_pdata,
>
> Although you would still need to add a pointer
> to struct dwcmshc_pltfm_data from struct dwcmshc_priv,
> but then you could access it like:
>
> #define to_k230_pdata(pd) container_of(pd, struct k230_pltfm_data, pd)
>
> struct dwcmshc_priv *dwc_priv = sdhci_pltfm_priv(pltfm_host);
> struct dwcmshc_pltfm_data *dwcmshc_pdata = dwc_priv->dwcmshc_pdata;
> struct k230_pltfm_data *k230_pdata = to_k230_pdata(dwcmshc_pdata);
>
Thank you for your suggestion. This looks better. I will make the changes.

Grateful regards,
Jiayu Du