On Fri, Jun 12, 2026 at 04:18:55PM -0400, Raymond Mao wrote: > From: Guodong Xu <[email protected]> > > Add SDHCI platform driver support for SpacemiT K1 SoC. This driver > implements the necessary platform-specific operations for the SDHCI > controller, enabling MMC/SD card functionality on K1-based platforms.
Is there any reason not to re-use Linux-side K1 MMC driver, sdhci-of-k1.c, and its companion ABI? Since Linux commit e9cb83c10071 (mmc: sdhci-of-k1: add comprehensive SDR tuning support, 2026-05-11) it is now capable of operating on SD cards, too. > Signed-off-by: Guodong Xu <[email protected]> > Signed-off-by: Raymond Mao <[email protected]> > --- > drivers/mmc/Kconfig | 7 + > drivers/mmc/Makefile | 1 + > drivers/mmc/spacemit_sdhci.c | 934 +++++++++++++++++++++++++++++++++++ > 3 files changed, 942 insertions(+) > create mode 100644 drivers/mmc/spacemit_sdhci.c ... > diff --git a/drivers/mmc/spacemit_sdhci.c b/drivers/mmc/spacemit_sdhci.c > new file mode 100644 > index 00000000000..392ca389fa9 > --- /dev/null > +++ b/drivers/mmc/spacemit_sdhci.c ... > +static void spacemit_sdhci_set_aib_mmc1_io(struct sdhci_host *host, int > voltage) > +{ > + struct mmc *mmc = host->mmc; > + struct spacemit_sdhci_plat *plat = dev_get_plat(mmc->dev); > + void __iomem *aib_mmc1_io, *apbc_asfar, *apbc_assar; > + u32 reg; > + > + if (!plat->aib_mmc1_io_reg || !plat->apbc_asfar_reg || > + !plat->apbc_assar_reg) > + return; > + > + aib_mmc1_io = map_sysmem((uintptr_t)plat->aib_mmc1_io_reg, > + sizeof(u32)); > + apbc_asfar = map_sysmem((uintptr_t)plat->apbc_asfar_reg, > + sizeof(u32)); > + apbc_assar = map_sysmem((uintptr_t)plat->apbc_assar_reg, > + sizeof(u32)); AFAIK aib_mmc1_io region is related to pinctrl settings, and shouldn't be handled by the MMC driver directly. Quoting my own reply[1] to the series adding SD support to sdhci-of-k1.c in Linux side, > Also, the pin controller on K1 SoC seems to have some undocumented > registers to select the IO voltage of SD pins, which should be > adjusted when switching IO voltage. > > I think these pins should be implemented in the pinctrl driver, then > you could create two pinctrl states, one for 1.8v operation, one for > 3.3v, and switch between them through pinctrl_lookup_state() when > changing IO voltage. This idea is later confirmed by SpacemiT guy's series[2], > On K1, IO domain power control registers determine whether a GPIO bank > operates at 1.8V or 3.3V. These registers default to 3.3V operation, > which may lead to functional failures when GPIO banks are externally > supplied with 1.8V but internally remain configured for 3.3V. > > The IO power domain registers are implemented as secure registers and > require an explicit unlock sequence via the AIB Secure Access Register > (ASAR), located in the APBC register space. Thus I think you should move the logic to pinctrl driver instead, like what has been done in the Linux upstream driver. > + writel(AKEY_ASFAR, apbc_asfar); > + writel(AKEY_ASSAR, apbc_assar); > + reg = readl(aib_mmc1_io); > + > + switch (voltage) { > + case MMC_SIGNAL_VOLTAGE_180: > + reg |= MMC1_IO_V18EN; > + break; > + default: > + reg &= ~MMC1_IO_V18EN; > + break; > + } > + writel(AKEY_ASFAR, apbc_asfar); > + writel(AKEY_ASSAR, apbc_assar); > + writel(reg, aib_mmc1_io); > +} ... > +static void spacemit_sdhci_set_control_reg(struct sdhci_host *host) > +{ ... > + /* Set pinctrl state */ > + if (IS_ENABLED(CONFIG_PINCTRL)) { > + if (mmc->clock >= 200000000) > + pinctrl_select_state(mmc->dev, "fast"); > + else > + pinctrl_select_state(mmc->dev, "default"); > + } This doesn't match Linux side ABI, where when card operates in UHS mode, pinctrl state "uhs" is selected. Regards, Yao Zi [1]: https://lore.kernel.org/linux-riscv/aUDwDkd8k_4gD1yc@pie/ [2]: https://lore.kernel.org/linux-riscv/20260108-kx-pinctrl-aib-io-pwr-domain-v2-0-6bcb46146...@linux.spacemit.com
