Re: [PATCH v2 2/3] mmc: k1: let the SPL read SD cards more than one block at a time

[Yixun Lan <dlan@kernel.org>]

Hi Kathpalia, Zexian,

On 19:30 Mon 12 Oct     , Kathpalia, Tanmay wrote:
> 
> On 12-10-2026 18:45, Zixian Zeng wrote:
> > On 2026/10/11 22:31, Kathpalia, Tanmay wrote:
> >> Hi Zixian,
> >>
> >> Thanks for the logs.
> >>
> >> I hit the same pattern while testing the Cadence SD6HC driver.
> >> Multi-block reads failed and single-block reads worked. On that board
> >> the host and the card were in 1.8 V mode, but the I/O supply had been
> >> left at 3.3 V. A short transfer got through; a longer one failed on the
> >> data lines.
> >> Both stages on K1 use the same PIO loop, so a FIFO that drains slower
> >> than the card delivers would show up in U-Boot proper as well. Your
> >> SPL/proper split fits an electrical setup difference.
> >>
> >> The SPL failure is irq=408020. Bit 22 is the Data End Bit Error (EDEB,
> >> SD mode only): the controller detected 0 at the end-bit position of
> >> read data on DAT, or of the write CRC status. That is a bad edge on
> >> the data line during the transfer.
> >>
> >> On the Banana Pi F3, aldo1 is vqmmc-supply of &sdhci0. The U-Boot
> >> dtsi names it vdd_1v8_mmc, and the SPL prints "vdd_1v8_mmc,
> >> value:1800000" before MMC2. set_vdd_mmc() in board/spacemit/k1/spl.c,
> >> called from board_init_f(), writes 1800000. The dts leaves aldo1 at
> >> 0.5 V to 3.4 V. SD High Speed (mode=2) uses 3.3 V signaling; 1.8 V is
> >> only after a UHS switch. mmc1_cfg is power-source 3300, and
> >> mmc1_uhs_cfg is the 1.8 V state. For this mode the rail should be 3.3 V.
> >>
> >> CONFIG_SPL_MMC_IO_VOLTAGE is not set, so mmc_set_signal_voltage() is a
> >> stub and signal_voltage stays MMC_SIGNAL_VOLTAGE_000. aldo1 stays at
> >> 1.8 V. U-Boot proper has CONFIG_MMC_IO_VOLTAGE. mmc_set_initial_state()
> >> requests MMC_SIGNAL_VOLTAGE_330, so the driver writes 3300000 to the
> >> same vqmmc-supply and clears SDHCI_CTRL_VDD_180 before the transfer
> >> that succeeds.
> >>
> >> CONFIG_SPL_PINCTRL is not set either. The driver calls
> >> pinctrl_select_state() under IS_ENABLED(CONFIG_PINCTRL), but that
> >> function is a stub unless CONFIG_IS_ENABLED(PINCTRL), and the return
> >> is ignored. SPL never applies mmc1_cfg. Proper selects "default" for
> >> SD_HS, which is that state.
> >>
> >> Keeping the multi-block path, set_vdd_mmc() to 3300000, or enable
> >> CONFIG_SPL_MMC_IO_VOLTAGE. If the 32-block CMD18 still fails, enable
> >> CONFIG_SPL_PINCTRL so mmc1_cfg is applied. If either makes the read
> >> succeed, this is the F3 failure.
> >>
> >> Regards,
> >> Tanmay
> >
> > Hi Tanmay,
> >
> > Your diagnosis was right - it is the I/O voltage.
> > Enabling CONFIG_SPL_MMC_IO_VOLTAGE makes
> > the multi-block read work on the Banana Pi-F3:
> >
> > Based on previous commit, I patched the following changes:
> >
> > diff --git a/configs/spacemit_k1_defconfig 
> > b/configs/spacemit_k1_defconfig
> > index 9c15d96eae2..f5c717e5965 100644
> > --- a/configs/spacemit_k1_defconfig
> > +++ b/configs/spacemit_k1_defconfig
> > @@ -95,6 +95,7 @@ CONFIG_MMC=y
> >  CONFIG_MMC_SDHCI=y
> >  CONFIG_MMC_SDHCI_SPACEMIT=y
> >  CONFIG_MMC_IO_VOLTAGE=y
> > +CONFIG_SPL_MMC_IO_VOLTAGE=y
> >  CONFIG_MMC_HS200_SUPPORT=y
> >  CONFIG_MMC_HS400_SUPPORT=y
> >  CONFIG_MMC_HS400_ES_SUPPORT=y
> > diff --git a/drivers/mmc/spacemit_sdhci.c b/drivers/mmc/spacemit_sdhci.c
> > index 23c655c6def..58374c482b3 100644
> > --- a/drivers/mmc/spacemit_sdhci.c
> > +++ b/drivers/mmc/spacemit_sdhci.c
> > @@ -251,6 +251,10 @@ static void spacemit_sdhci_set_voltage(struct 
> > sdhci_host *host)
> >                         bool ok;
> >                         int voltage_mv = to_180 ? 1800000 : 3300000;
> >
> > +                       printf("sdhci: voltage signal_voltage=%u 
> > vqmmc=%s -> %dmV\n",
> > +                              mmc->signal_voltage,
> > +                              mmc->vqmmc_supply ? 
> > mmc->vqmmc_supply->name : "none",
> > +                              voltage_mv);
> >                         if (spacemit_sdhci_set_vqmmc_voltage(mmc, 
> > voltage_mv))
> >                                 return;
> >                         if (!IS_SD(mmc))
> > @@ -274,7 +278,8 @@ static void spacemit_sdhci_set_voltage(struct 
> > sdhci_host *host)
> >                         break;
> >                 }
> >                 default:
> > -                       /* No signal voltage switch required */
> > +                       printf("sdhci: voltage signal_voltage=%u -> no 
> > switch\n",
> > +                              mmc->signal_voltage);
> >                         return;
> >                 }
> >         }
> >
> > Booting logs as follows:
> >
> > U-Boot SPL 2026.10-rc5-00643-g6a99fe92389a (Oct 12 2026 - 21:13:00 +0800)
> > Fail to detect board:-2
> > vdd_core, value:900000
> > vdd_1v8, value:1800000
> > vdd_1v8_mmc, value:1800000
> > DDR firmware: [0xc081d880]:0xf0227179, size:0x8d98
> > DDR is ready
> > No SPI controller (-19)
> > Trying to boot from MMC2
> > sdhci: mmc@d4280000 path=PIO flags=0 caps_sdma=1 caps_adma2=1 
> > max_clk=204800000
> > sdhci: voltage signal_voltage=0 -> no switch
> > sdhci: voltage signal_voltage=4 vqmmc=aldo1 -> 3300000mV
> > sdhci: voltage signal_voltage=4 vqmmc=aldo1 -> 3300000mV
> > sdhci: voltage signal_voltage=4 vqmmc=aldo1 -> 3300000mV
> > sdhci: voltage signal_voltage=4 vqmmc=aldo1 -> 3300000mV
> > sdhci: voltage signal_voltage=4 vqmmc=aldo1 -> 3300000mV
> > mmc: multi-blk blkcnt=32 clk=50000000 mode=2 width=4 caps=300207ef 
> > cmd23_host=1 scr0=2358043 cmd23_card=1
> > sdhci: multi cmd18 arg=2 blks=32 blksz=512 mode=32 dma=0
> > mmc: multi-blk blkcnt=1956 clk=50000000 mode=2 width=4 caps=300207ef 
> > cmd23_host=1 scr0=2358043 cmd23_card=1
> > sdhci: multi cmd18 arg=1000 blks=1956 blksz=512 mode=32 dma=0
> >
> > <debug_uart>
> >
> >
> > U-Boot 2026.10-rc5-00643-g6a99fe92389a (Oct 12 2026 - 21:13:00 +0800)
> >
> > DRAM:  4 GiB
> > Core:  338 devices, 22 uclasses, devicetree: separate
> > MMC:   sdhci: mmc@d4281000 path=PIO flags=0 caps_sdma=1 caps_adma2=1 
> > max_clk=204800000
> > sdhci: mmc@d4280000 path=PIO flags=0 caps_sdma=1 caps_adma2=1 
> > max_clk=204800000
> > mmc@d4280000: 1, mmc@d4281000: 0
> > Loading Environment from nowhere... OK
> > In:    serial@d4017000
> > Out:   serial@d4017000
> > Err:   serial@d4017000
> > Net:   No ethernet found.
> > =>
> >
> > With the voltage corrected, a single 1956 block PIO CMD18 loads
> > the whole FIT,so the controller is fine with the FIFO back pressure,
> > the slow SPL drain was a symptom, not the cause :-).
> >
> > Without the CONFIG_SPL_MMC_IO_VOLTAGE=y change the same driver printed
> > "sdhci: voltage signal_voltage=0 -> no switch".
> > Since mmc_set_signWith the voltage corrected, a single 1956 block PIO 
> > CMD18 loads
> > the whole FIT,so the controller is fine with the FIFO back pressure,
> > the slow SPL drain was a symptom, not the cause :-).
> >
> > Without the CONFIG_SPL_MMC_IO_VOLTAGE=y change the same driver printed
> > "sdhci: voltage signal_voltage=0 -> no switch".
> > Since mmc_set_signal_voltage() is a stub in SPL, so mmc->signal_voltage
> > stayed MMC_SIGNAL_VOLTAGE_000, spacemit_sdhci_set_voltage() took the
> > "no switch required" path, and aldo1 stayed at the initial 1800000 that
> > set_vdd_mmc() writes in pmic_init(). The 32 block CMD18 then failed
> > after ~20 of 32 blocks. Single block reads passed at 1.8 V, which is 
> > what hid it.
> >
> > On the pinctrl side I had already run the SPL with standalone
> > CONFIG_SPL_PINCTRL=y, with bootph-pre-ram on &pinctrl, mmc1_cfg and
> > mmc1_uhs_cfg plus their pin groups. I confirmed that
> > pinctrl_select_state() return successfully. The multi-block read still
> > failed the same way with the rail at 1.8V.
> >
> > Therefore, I suspect that configuring the MMC signal voltage to 1.8V
> > in pmic_init() may need to be modified.
> >
> > Thanks for your suggestions and guidance.
> >
> > Best regards,
> > Zixianal_voltage() is a stub in SPL, so mmc->signal_voltage
> > stayed MMC_SIGNAL_VOLTAGE_000, spacemit_sdhci_set_voltage() took the
> > "no switch required" path, and aldo1 stayed at the initial 1800000 that
> > set_vdd_mmc() writes in pmic_init(). The 32 block CMD18 then failed 
> > after ~20 of 32 blocks. Single block reads passed at 1.8 V, which is 
> > what hid it.
> >
> > On the pinctrl side I had already run the SPL with standalone
> > CONFIG_SPL_PINCTRL=y, with bootph-pre-ram on &pinctrl, mmc1_cfg and
> > mmc1_uhs_cfg plus their pin groups. I confirmed that
> > pinctrl_select_state() return successfully. The multi-block read still
> > failed the same way with the rail at 1.8V.
> >
> > Therefore, I suspect that configuring the MMC signal voltage to 1.8V
> > in pmic_init() may need to be modified.
> >
> > Thanks for your suggestions and guidance.
> >
> >
> 
> Hi Zixian,
> 
> Thank you for the quick testing and for posting detailed results. Your
> patience with the suggested tests helped us reach the root cause
> within two days.
> 
> With CONFIG_SPL_MMC_IO_VOLTAGE enabled, aldo1 is changed from its
> initial 1.8 V to 3.3 V and both the 32-block and 1956-block reads
> succeed.
> 
> I am still unsure why set_vdd_mmc() initializes this rail to 1.8 V.
I'm also looking into this..

It's definitely wrong here, and should be fixed in my opinion, refer
to SD spec, the initial IO should be 3.3v, and later may change to 1.8v
according to OCR report, the BROM code will set IO to 3.3v by default
since cold boot, and I've done a test on this, drop set_vdd_mmc() & without
CONFIG_SPL_MMC_IO_VOLTAGE enabled, the multi block read still works fine
on SPL stage

Also with Zexian's above test, it also prove that, enabling CONFIG_SPL_MMC_IO_VOLTAGE
will lead the mmc core calling set_voltage() which will set IO to 3.3v, which also
fix the problem

> Since we do not know the original hardware requirement, any change to
> that default should be reviewed by the code owner.
> 
> I suggest enabling both CONFIG_SPL_MMC_IO_VOLTAGE and
> CONFIG_SPL_PINCTRL. The former fixes the voltage issue, while the
> latter ensures SPL applies the defined MMC pad configuration instead
> of relying on the Boot ROM state.
> 
I agree, it's safe to do this

> Dave, I hope you are aligned with the changes discussed and tested.
> Could you please post a new version incorporating them?
> 
> Signed-off-by: Tanmay Kathpalia <tanmay.kathpalia@altera.com>
> 
> Regards,
> Tanmay
> 

-- 
Yixun Lan (dlan)