Re: [PATCH 2/8] mmc: k1: add sdhci platform driver

[Yao Zi <me@ziyao.cc>]

On Wed, Jun 24, 2026 at 10:23:35AM +0800, Eric Chung wrote:
> On Sat, Jun 13, 2026 at 2:44 PM 'Yao Zi' via uboot <uboot@riscstar.com> wrote:
> >
> > On Fri, Jun 12, 2026 at 04:18:55PM -0400, Raymond Mao wrote:
> > > From: Guodong Xu <guodong@riscstar.com>
> > >
> > > 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.
> >
> Excuse me. Is there any ABI breakage in this SD/eMMC device?

Yes, I only take a brief look, and at least the names for pinctrl states
differ from Linux upstream, as mentioned below.

So are there any reasons not to re-use it? This driver has been
carefully reviewed in Linux side, has the expected ABI, which would
reduce your work of upstreaming.

> > > Signed-off-by: Guodong Xu <guodong@riscstar.com>
> > > Signed-off-by: Raymond Mao <raymond.mao@riscstar.com>
> > > ---
> > >  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_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.
> >
> OK. Switch to "uhs". By the way, the upstream DTS file hasn't been
> synced into uboot yet. This is not easy to find.

I'm not sure what do you mean by "not easy to find". You could always
refer to the dt-bindings maintained by Linux side for ABI description,
and we want to keep aligned with it in U-Boot side as long as possible
to reduce maintenance burden and improve interoperability.

> > 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-6bcb46146e53@linux.spacemit.com

Regards,
Yao Zi