On Sun, Jul 12, 2026 at 10:35 PM Bin Meng <[email protected]> wrote:
>
> On Wed, Jul 8, 2026 at 1:05 AM Tao Ding <[email protected]> wrote:
> >
> > During SDMA transfers, the controller raises an interrupt at buffer
> > boundaries
> > to request a system address update. The host driver will rewrite the
> > SDHC_SYSAD register.
> > (according to
> > PartA2_SD_Host_Controller_Simplified_Specification_Ver2.00.pdf section
> > 2.2.1)
> > However, the current code will ignore write operations to SDHC_SYSAD.
> >
> > To fix this bug, when SDMA encounters a boundary, a state is set to record
> > it.
> > In this state, SDHC_SYSAD can be written. In other cases, it is still
> > protected by TRANSFERRING_DATA.
> >
> > Version_id change to 2 for migration.
> >
> > Suggested-by: Bin Meng <[email protected]>
> > Signed-off-by: Tao Ding <[email protected]>
> > ---
> > The reviewer pointed out an issue in patch v3. When s->blkcnt decreases to
> > 1 the logic goes to
> > sdhci_sdma_transfer_single_block() where s->sdma_boundary_paused should be
> > cleared too.
> >
> > This bug may be encountered when using uboot with SDMA functionality, and
> > can be reproduced by following the steps below.
> > Uboot test:
> > 1. Prepare zImage, uboot and rootfs.cpio.gz, make sure uboot config
> > (MMC_SDHCI_SDMA=y).
> > for more detail, can obtained
> >
> > "https://lore.kernel.org/qemu-devel/[email protected]/T/#mcdf231e6bd762903f8393bf93253cbcb12ececcf";
> > 2. ./qemu-system-aarch64 -M xilinx-zynq-a9 -machine boot-mode=sd -m
> > 1024 -display none -serial null -serial stdio -monitor none \
> > -device loader,file=u-boot-dtb.bin,addr=0x04000000,cpu-num=0 \
> > -drive file=zynq-sd.img,format=raw,if=sd -D qemu.log
> >
> >
> > After applying the fix, success load sd card:
> >
> > U-Boot 2026.07-rc4-g1e80ee41441c (Jun 15 2026 - 19:05:05 +0800)
> >
> > Model: Xilinx ZC702 board
> > DRAM: ECC disabled 1 GiB
> > Core: 33 devices, 21 uclasses, devicetree: board
> > Flash: 0 Bytes
> > NAND: 0 MiB
> > MMC: mmc@e0100000: 0
> > Loading Environment from FAT... *** Error - No Valid Environment Area found
> > *** Warning - bad env area, using default environment
> >
> > In: serial@e0001000
> > Out: serial@e0001000
> > Err: serial@e0001000
> > Net:
> > ZYNQ GEM: e000b000, mdio bus e000b000, phyaddr 7, interface rgmii-id
> >
> > Warning: ethernet@e000b000 (eth0) using random MAC address -
> > 4a:63:5a:a4:d3:6b
> > eth0: ethernet@e000b000
> > Hit any key to stop autoboot: 0
> > Zynq> fatload mmc 0 ${kernel_addr_r} zImage
> > 10838528 bytes read in 4316 ms (2.4 MiB/s)
> > Zynq> fatload mmc 0 ${ramdisk_addr_r} rootfs-arm32.cpio.gz
> > 406531 bytes read in 199 ms (1.9 MiB/s)
> > Zynq> setenv ramdisk_size ${filesize}
> > Zynq> fatload mmc 0 ${fdt_addr_r} zynq-zc702.dtb
> > 15754 bytes read in 21 ms (732.4 KiB/s)
> > Zynq> setenv bootargs console=ttyPS0,115200 earlycon ignore_loglevel
> > rdinit=/init
> > Zynq> bootz ${kernel_addr_r} ${ramdisk_addr_r}:${ramdisk_size} ${fdt_addr_r}
> > Kernel image @ 0x2000000 [ 0x000000 - 0xa56200 ]
> > ## Flattened Device Tree blob at 01f00000
> > Booting using the fdt blob at 0x1f00000
> > Working FDT set to 1f00000
> > Loading Ramdisk to 2ff9c000, end 2ffff403 ... OK
> > Loading Device Tree to 2ff95000, end 2ff9bd89 ... OK
> > Working FDT set to 2ff95000
> >
> > Starting kernel ...
> >
> > hw/sd/sdhci.c | 14 +++++++++++---
> > include/hw/sd/sdhci.h | 2 ++
> > 2 files changed, 13 insertions(+), 3 deletions(-)
> >
> > diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c
> > index c86dfa281f..dc774fdf21 100644
> > --- a/hw/sd/sdhci.c
> > +++ b/hw/sd/sdhci.c
> > @@ -307,6 +307,7 @@ static void sdhci_reset(SDHCIState *s)
> > s->data_count = 0;
> > s->stopped_state = sdhc_not_stopped;
> > s->pending_insert_state = false;
> > + s->sdma_boundary_paused = false;
> > if (object_dynamic_cast(OBJECT(s), TYPE_FSL_ESDHC_BE) ||
> > object_dynamic_cast(OBJECT(s), TYPE_FSL_ESDHC_LE)) {
> > s->norintstsen = 0x013f;
> > @@ -620,6 +621,7 @@ static void sdhci_sdma_transfer_multi_blocks(SDHCIState
> > *s)
> > }
> >
> > s->prnsts |= SDHC_DATA_INHIBIT | SDHC_DAT_LINE_ACTIVE;
> > + s->sdma_boundary_paused = false;
> > if (s->trnmod & SDHC_TRNS_READ) {
> > s->prnsts |= SDHC_DOING_READ;
> > while (s->blkcnt) {
> > @@ -644,6 +646,7 @@ static void sdhci_sdma_transfer_multi_blocks(SDHCIState
> > *s)
> > s->data_count = 0;
> > }
> > if (page_aligned && boundary_count == 0) {
> > + s->sdma_boundary_paused = true;
>
> This one, and the following one
>
> > break;
> > }
> > }
> > @@ -669,6 +672,7 @@ static void sdhci_sdma_transfer_multi_blocks(SDHCIState
> > *s)
> > }
> > }
> > if (page_aligned && boundary_count == 0) {
> > + s->sdma_boundary_paused = true;
>
> can be moved to a single place before sdhci_update_irq() in the same function.
>
> > break;
> > }
> > }
> > @@ -679,6 +683,7 @@ static void sdhci_sdma_transfer_multi_blocks(SDHCIState
> > *s)
> > }
> >
> > if (s->blkcnt == 0) {
> > + s->sdma_boundary_paused = false;
>
> Move setting sdma_boundary_paused false to inside sdhci_end_transfer().
>
> > sdhci_end_transfer(s);
> > } else {
>
> Move s->sdma_boundary_paused = true here.
>
> > sdhci_update_irq(s);
> > @@ -690,6 +695,7 @@ static void sdhci_sdma_transfer_single_block(SDHCIState
> > *s)
> > {
> > uint32_t datacnt = s->blksize & BLOCK_SIZE_MASK;
> >
> > + s->sdma_boundary_paused = false;
>
> This is not needed.
>
> > if (s->trnmod & SDHC_TRNS_READ) {
> > sdbus_read_data(&s->sdbus, s->fifo_buffer, datacnt);
> > dma_memory_write(s->dma_as, s->sdmasysad, s->fifo_buffer, datacnt,
> > @@ -1164,6 +1170,7 @@ static inline void sdhci_reset_write(SDHCIState *s,
> > uint8_t value)
> > SDHC_DATA_INHIBIT | SDHC_DAT_LINE_ACTIVE);
> > s->blkgap &= ~(SDHC_STOP_AT_GAP_REQ | SDHC_CONTINUE_REQ);
> > s->stopped_state = sdhc_not_stopped;
> > + s->sdma_boundary_paused = false;
>
> Not needed.
I misread this one. This should be kept.
>
> > s->norintsts &= ~(SDHC_NIS_WBUFRDY | SDHC_NIS_RBUFRDY |
> > SDHC_NIS_DMA | SDHC_NIS_TRSCMP | SDHC_NIS_BLKGAP);
> > break;
> > @@ -1185,7 +1192,7 @@ sdhci_write(void *opaque, hwaddr offset, uint64_t
> > val, unsigned size)
> >
> > switch (offset & ~0x3) {
> > case SDHC_SYSAD:
> > - if (!TRANSFERRING_DATA(s->prnsts)) {
> > + if (!TRANSFERRING_DATA(s->prnsts) || s->sdma_boundary_paused) {
> > s->sdmasysad = (s->sdmasysad & mask) | value;
> > MASKED_WRITE(s->sdmasysad, mask, value);
> > /* Writing to last byte of sdmasysad might trigger transfer */
> > @@ -1477,8 +1484,8 @@ static const VMStateDescription
> > sdhci_pending_insert_vmstate = {
> >
> > const VMStateDescription sdhci_vmstate = {
> > .name = "sdhci",
> > - .version_id = 1,
> > - .minimum_version_id = 1,
> > + .version_id = 2,
> > + .minimum_version_id = 2,
>
> Use a subsection is better in this case. See example of
> sdhci_pending_insert_vmstate.
>
> > .fields = (const VMStateField[]) {
> > VMSTATE_UINT32(sdmasysad, SDHCIState),
> > VMSTATE_UINT16(blksize, SDHCIState),
> > @@ -1508,6 +1515,7 @@ const VMStateDescription sdhci_vmstate = {
> > VMSTATE_VBUFFER_UINT32(fifo_buffer, SDHCIState, 1, NULL,
> > buf_maxsz),
> > VMSTATE_TIMER_PTR(insert_timer, SDHCIState),
> > VMSTATE_TIMER_PTR(transfer_timer, SDHCIState),
> > + VMSTATE_BOOL(sdma_boundary_paused, SDHCIState),
> > VMSTATE_END_OF_LIST()
> > },
> > .subsections = (const VMStateDescription * const []) {
> > diff --git a/include/hw/sd/sdhci.h b/include/hw/sd/sdhci.h
> > index a9da6203fc..c485277744 100644
> > --- a/include/hw/sd/sdhci.h
> > +++ b/include/hw/sd/sdhci.h
> > @@ -103,6 +103,8 @@ struct SDHCIState {
> > * to be protected. Set wp_inverted to invert the signal.
> > */
> > bool wp_inverted;
> > + /* Indicate that SDMA transfer is paused due to hitting the boundary */
> > + bool sdma_boundary_paused;
> > };
> > typedef struct SDHCIState SDHCIState;
Regards,
Bin
