On Sun, Jul 12, 2026 at 9:38 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; > > 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; > > break; > > } > > } > > @@ -679,6 +683,7 @@ static void sdhci_sdma_transfer_multi_blocks(SDHCIState > > *s) > > } > > > > if (s->blkcnt == 0) { > > + s->sdma_boundary_paused = false; > > sdhci_end_transfer(s); > > } else { > > 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; > > 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; > > 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, > > .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; > > It looks we can eliminate the need of introducing a new state to > represent this scenario, instead we can do: > > static bool sdhci_sdma_transfer_active(SDHCIState *s) > { > return TRANSFERRING_DATA(s->prnsts) && s->blkcnt && > (s->blksize & BLOCK_SIZE_MASK) && > SDHC_DMA_TYPE(s->hostctl1) == SDHC_CTRL_SDMA; > } > > and add such a condition in SYSAD handling: > > if (!TRANSFERRING_DATA() || sdhci_sdma_transfer_active())
This logic has a flaw. Please ignore. The introduction of a new state seems inevitable. > > > }; > > typedef struct SDHCIState SDHCIState; > Regards, Bin
