U-Boot Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: Yixun Lan <dlan@kernel.org>
To: "Kathpalia, Tanmay" <tanmay.kathpalia@altera.com>
Cc: Zixian Zeng <sycamoremoon376@gmail.com>,
	Dave Klotz <DaveWK225@protonmail.com>,
	Eric Chung <eric.chung@riscstar.com>, Peng Fan <peng.fan@nxp.com>,
	u-boot@lists.u-boot-project.org,
	Jaehoon Chung <jh80.chung@samsung.com>,
	Tom Rini <trini@konsulko.com>,
	u-boot-spacemit@groups.io
Subject: Re: [PATCH v2 2/3] mmc: k1: let the SPL read SD cards more than one block at a time
Date: Mon, 12 Oct 2026 15:22:32 +0000	[thread overview]
Message-ID: <20261012152232-GKK26088@kernel.org> (raw)
In-Reply-To: <a677e019-dd28-420b-a9d7-808808b56264@altera.com>

Hi Kathpalia,

On 20:20 Mon 12 Oct     , Kathpalia, Tanmay wrote:
> Hi Yixun,
> 
> On 12-10-2026 20:00, Yixun Lan wrote:
> > 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
> 
> Agreed. The host and I/O signaling rail should initially be at 3.3 V,
> and the card powers up using 3.3 V signaling. It may switch to 1.8 V
> later only when entering a supported UHS timing mode.
> 
Currently, the spacemit_k1_defconfig does not enable UHS config, so no
1.8v switching will happen, but I think that's another problem which
should investigate later..

> > 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
> 
> Thanks for testing the multi-block operation after dropping
> set_vdd_mmc() and without CONFIG_SPL_MMC_IO_VOLTAGE. Since the BootROM
> leaves the I/O rail at 3.3 V after a cold boot, the successful result is
> expected and confirms that set_vdd_mmc() incorrectly changes it to
> 1.8 V.
> 
> >
> > 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
> 
> I think both changes should be made:
> 
> 1. Initialize the MMC I/O rail to 3.3 V instead of 1.8 V.
For how to achieve on this, I'd suggest drop set_vdd_mmc() from spl.c
and the leave spacemit_sdhci.c to config the I/O rail accordingly

> 2. Enable CONFIG_SPL_MMC_IO_VOLTAGE so the MMC core can manage any
>     subsequent signal-voltage transitions correctly.
> 
I agree on this

> >> 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)

  reply	other threads:[~2026-10-12 15:22 UTC|newest]

Thread overview: 35+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-10-02 21:29 [PATCH 0/3] mmc: k1: fixes for the SpacemiT K1 SDHCI driver Dave Klotz
2026-10-02 21:29 ` [PATCH 1/3] mmc: k1: handle a missing max-frequency and clk_set_rate()'s return value Dave Klotz
2026-10-04  9:14   ` Kathpalia, Tanmay
2026-10-04  9:30   ` Kathpalia, Tanmay
2026-10-05 17:46     ` Dave Klotz
2026-10-02 21:29 ` [PATCH 2/3] mmc: k1: let the SPL read SD cards more than one block at a time Dave Klotz
2026-10-04  9:28   ` Kathpalia, Tanmay
2026-10-02 21:29 ` [PATCH 3/3] mmc: k1: do not release the shared AXI clock and reset on probe failure Dave Klotz
2026-10-04  9:28   ` Kathpalia, Tanmay
2026-10-02 22:06 ` [PATCH 0/3] mmc: k1: fixes for the SpacemiT K1 SDHCI driver Dave Klotz
2026-10-05 17:50 ` [PATCH v2 " Dave Klotz
2026-10-05 17:55   ` [PATCH v2 1/3] mmc: k1: handle a missing max-frequency and clk_set_rate()'s return value Dave Klotz
2026-10-06  5:40     ` Kathpalia, Tanmay
2026-10-10  8:45     ` Yixun Lan
2026-10-05 18:01   ` [PATCH v2 2/3] mmc: k1: let the SPL read SD cards more than one block at a time Dave Klotz
2026-10-10  6:17     ` Zixian Zeng
2026-10-10 19:02       ` Kathpalia, Tanmay
2026-10-10 19:16         ` Kathpalia, Tanmay
2026-10-11  3:38           ` Zixian Zeng
2026-10-11 14:31             ` Kathpalia, Tanmay
2026-10-11 15:33               ` Anand Moon
2026-10-11 19:41                 ` Kathpalia, Tanmay
2026-10-12 13:15               ` Zixian Zeng
2026-10-12 14:00                 ` Kathpalia, Tanmay
2026-10-12 14:30                   ` Yixun Lan
2026-10-12 14:50                     ` Kathpalia, Tanmay
2026-10-12 15:22                       ` Yixun Lan [this message]
2026-10-14 19:59                   ` Dave Klotz
2026-10-14 20:34                     ` Kathpalia, Tanmay
2026-10-12 16:07                 ` Anand Moon
2026-10-13  0:15                   ` Yixun Lan
2026-10-13  3:44                     ` Anand Moon
2026-10-05 18:11   ` [PATCH v2 3/3] mmc: k1: do not release the shared AXI clock and reset on probe failure Dave Klotz
2026-10-10  7:14     ` Yixun Lan
2026-10-10 19:27       ` Kathpalia, Tanmay

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20261012152232-GKK26088@kernel.org \
    --to=dlan@kernel.org \
    --cc=DaveWK225@protonmail.com \
    --cc=eric.chung@riscstar.com \
    --cc=jh80.chung@samsung.com \
    --cc=peng.fan@nxp.com \
    --cc=sycamoremoon376@gmail.com \
    --cc=tanmay.kathpalia@altera.com \
    --cc=trini@konsulko.com \
    --cc=u-boot-spacemit@groups.io \
    --cc=u-boot@lists.u-boot-project.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox