Am 3. Oktober 2026 17:01:59 UTC schrieb Bin Meng <[email protected]>:
>The SDHCI model processes the first ADMA descriptor batch inside the
>command MMIO write, then resumes pending batches before every later
>SDHCI MMIO read or write. This makes register access perform bulk DMA
>work and can delay a status read even when completion bits are set.
>
>On the Icicle Kit, U-Boot uses ADMA2-64 and CMD18 to load a roughly
>19 MiB FIT image from SD. It can report:
>
> Timeout for status update: 00000001 00000001
> Timeout for status update: 00000003 00000001
>
>The status already contains the requested Command Complete bit, and may
>also contain Transfer Complete. However, reading the interrupt status
>first executes a pending ADMA batch. The MMIO read can therefore return
>after U-Boot has reached its one-second polling deadline.
>
>Turn the existing transfer timer into an ADMA-only engine. Starting an
>ADMA command arms the engine directly, and each timer callback runs one
>bounded batch before yielding. MMIO reads and writes no longer perform
>or schedule ADMA work. Keep PIO and SDMA synchronous.
>
>Cancel the engine and clear ADMA activity and error state on DATA or ALL
>reset.
>
>Signed-off-by: Bin Meng <[email protected]>
>---
>
>(no changes since v1)
Hi Bin,
I've accidentally left some comments on v2 after you sent v3:
https://lore.kernel.org/qemu-devel/[email protected]/
. Generally patch LGTM.
Best regards,
Bernhard
>
> hw/sd/sdhci.c | 143 ++++++++++++++++++++++++++++++--------------------
> 1 file changed, 87 insertions(+), 56 deletions(-)
>
>diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c
>index bd74c60f72..14c312a691 100644
>--- a/hw/sd/sdhci.c
>+++ b/hw/sd/sdhci.c
>@@ -277,6 +277,7 @@ static void sdhci_set_inserted(DeviceState *dev, bool
>level)
> }
> } else {
> s->prnsts = 0x1fa0000;
>+ timer_del(s->transfer_timer);
> s->pwrcon &= ~SDHC_POWER_ON;
> s->clkcon &= ~SDHC_CLOCK_SDCLK_EN;
> if (s->norintstsen & SDHC_NISEN_REMOVE) {
>@@ -348,7 +349,34 @@ static void sdhci_poweron_reset(DeviceState *dev)
> }
> }
>
>-static void sdhci_data_transfer(void *opaque);
>+static void sdhci_data_transfer(SDHCIState *s);
>+static void sdhci_adma_engine(void *opaque);
>+
>+static void sdhci_set_transfer_active(SDHCIState *s)
>+{
>+ s->prnsts |= SDHC_DATA_INHIBIT | SDHC_DAT_LINE_ACTIVE;
>+ if (s->trnmod & SDHC_TRNS_READ) {
>+ s->prnsts |= SDHC_DOING_READ;
>+ } else {
>+ s->prnsts |= SDHC_DOING_WRITE;
>+ }
>+}
>+
>+static void sdhci_schedule_adma(SDHCIState *s)
>+{
>+ timer_mod(s->transfer_timer,
>+ qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) + SDHC_TRANSFER_DELAY);
>+}
>+
>+static bool sdhci_adma_transfer_active(SDHCIState *s)
>+{
>+ return TRANSFERRING_DATA(s->prnsts) &&
>+ (s->trnmod & SDHC_TRNS_DMA) &&
>+ (s->cmdreg & SDHC_CMD_DATA_PRESENT) &&
>+ sdhci_get_dma_type(s) != SDHC_CTRL_SDMA &&
>+ !(s->admaerr & SDHC_ADMAERR_STATE_MASK) &&
>+ !(s->errintsts & SDHC_EIS_ADMAERR);
>+}
>
> #define BLOCK_SIZE_MASK (4 * KiB - 1)
>
>@@ -405,7 +433,13 @@ static void sdhci_send_command(SDHCIState *s)
> if (!timeout && (s->blksize & BLOCK_SIZE_MASK) &&
> (s->cmdreg & SDHC_CMD_DATA_PRESENT)) {
> s->data_count = 0;
>- sdhci_data_transfer(s);
>+ if ((s->trnmod & SDHC_TRNS_DMA) &&
>+ sdhci_get_dma_type(s) != SDHC_CTRL_SDMA) {
>+ sdhci_set_transfer_active(s);
>+ sdhci_schedule_adma(s);
>+ } else {
>+ sdhci_data_transfer(s);
>+ }
> }
> }
>
>@@ -850,7 +884,7 @@ static void get_adma_description(SDHCIState *s, ADMADescr
>*dscr)
>
> /* Advanced DMA data transfer */
>
>-static void sdhci_do_adma(SDHCIState *s)
>+static void sdhci_adma_run_batch(SDHCIState *s)
> {
> unsigned int begin, length;
> const uint16_t block_size = s->blksize & BLOCK_SIZE_MASK;
>@@ -956,15 +990,17 @@ static void sdhci_do_adma(SDHCIState *s)
> }
> if (res != MEMTX_OK) {
> s->data_count = 0;
>+ s->admaerr &= ~SDHC_ADMAERR_STATE_MASK;
>+ s->admaerr |= SDHC_ADMAERR_STATE_ST_TFR;
> if (s->errintstsen & SDHC_EISEN_ADMAERR) {
> trace_sdhci_error("Set ADMA error flag");
> s->errintsts |= SDHC_EIS_ADMAERR;
> s->norintsts |= SDHC_NIS_ERR;
> }
> sdhci_update_irq(s);
>- } else {
>- s->admasysaddr += dscr.incr;
>+ return;
> }
>+ s->admasysaddr += dscr.incr;
> break;
> case SDHC_ADMA_ATTR_ACT_LINK: /* link to next descriptor table */
> s->admasysaddr = dscr.addr;
>@@ -1012,50 +1048,52 @@ static void sdhci_do_adma(SDHCIState *s)
> }
>
> /* we have unfinished business - reschedule to continue ADMA */
>- timer_mod(s->transfer_timer,
>- qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) +
>SDHC_TRANSFER_DELAY);
>+ sdhci_schedule_adma(s);
> }
>
>-/* Perform data transfer according to controller configuration */
>-
>-static void sdhci_data_transfer(void *opaque)
>+/* Run one independently scheduled, quota-bounded ADMA batch */
>+static void sdhci_adma_engine(void *opaque)
> {
> SDHCIState *s = (SDHCIState *)opaque;
>
>- if (s->trnmod & SDHC_TRNS_DMA) {
>- switch (sdhci_get_dma_type(s)) {
>- case SDHC_CTRL_SDMA:
>- sdhci_sdma_transfer(s);
>- break;
>- case SDHC_CTRL_ADMA1_32:
>- if (!(s->capareg & R_SDHC_CAPAB_ADMA1_MASK)) {
>- trace_sdhci_error("ADMA1 not supported");
>- break;
>- }
>+ if (!sdhci_adma_transfer_active(s)) {
>+ return;
>+ }
>
>- sdhci_do_adma(s);
>- break;
>- case SDHC_CTRL_ADMA2_32:
>- if (!(s->capareg & R_SDHC_CAPAB_ADMA2_MASK)) {
>- trace_sdhci_error("ADMA2 not supported");
>- break;
>- }
>+ switch (sdhci_get_dma_type(s)) {
>+ case SDHC_CTRL_ADMA1_32:
>+ if (!(s->capareg & R_SDHC_CAPAB_ADMA1_MASK)) {
>+ trace_sdhci_error("ADMA1 not supported");
>+ return;
>+ }
>+ break;
>+ case SDHC_CTRL_ADMA2_32:
>+ if (!(s->capareg & R_SDHC_CAPAB_ADMA2_MASK)) {
>+ trace_sdhci_error("ADMA2 not supported");
>+ return;
>+ }
>+ break;
>+ case SDHC_CTRL_ADMA2_64:
>+ if (!(s->capareg & R_SDHC_CAPAB_ADMA2_MASK) ||
>+ !(s->capareg & R_SDHC_CAPAB_BUS64BIT_MASK)) {
>+ trace_sdhci_error("64 bit ADMA not supported");
>+ return;
>+ }
>+ break;
>+ default:
>+ trace_sdhci_error("Unsupported ADMA type");
>+ return;
>+ }
>
>- sdhci_do_adma(s);
>- break;
>- case SDHC_CTRL_ADMA2_64:
>- if (!(s->capareg & R_SDHC_CAPAB_ADMA2_MASK) ||
>- !(s->capareg & R_SDHC_CAPAB_BUS64BIT_MASK)) {
>- trace_sdhci_error("64 bit ADMA not supported");
>- break;
>- }
>+ sdhci_adma_run_batch(s);
>+}
>
>- sdhci_do_adma(s);
>- break;
>- default:
>- trace_sdhci_error("Unsupported DMA type");
>- break;
>- }
>+/* Perform synchronous PIO or SDMA data transfer */
>+static void sdhci_data_transfer(SDHCIState *s)
>+{
>+ if (s->trnmod & SDHC_TRNS_DMA) {
>+ assert(sdhci_get_dma_type(s) == SDHC_CTRL_SDMA);
>+ sdhci_sdma_transfer(s);
> } else {
> if ((s->trnmod & SDHC_TRNS_READ) && sdbus_data_ready(&s->sdbus)) {
> s->prnsts |= SDHC_DOING_READ | SDHC_DATA_INHIBIT |
>@@ -1098,21 +1136,11 @@ sdhci_buff_access_is_sequential(SDHCIState *s,
>unsigned byte_num)
> return true;
> }
>
>-static void sdhci_resume_pending_transfer(SDHCIState *s)
>-{
>- timer_del(s->transfer_timer);
>- sdhci_data_transfer(s);
>-}
>-
> static uint64_t sdhci_read(void *opaque, hwaddr offset, unsigned size)
> {
> SDHCIState *s = (SDHCIState *)opaque;
> uint32_t ret = 0;
>
>- if (timer_pending(s->transfer_timer)) {
>- sdhci_resume_pending_transfer(s);
>- }
>-
> switch (offset & ~0x3) {
> case SDHC_SYSAD:
> ret = s->sdmasysad;
>@@ -1235,7 +1263,10 @@ static inline void sdhci_reset_write(SDHCIState *s,
>uint8_t value)
> s->norintsts &= ~SDHC_NIS_CMDCMP;
> break;
> case SDHC_RESET_DATA:
>+ timer_del(s->transfer_timer);
> s->data_count = 0;
>+ s->admaerr = 0;
>+ s->errintsts &= ~SDHC_EIS_ADMAERR;
> s->prnsts &= ~(SDHC_SPACE_AVAILABLE | SDHC_DATA_AVAILABLE |
> SDHC_DOING_READ | SDHC_DOING_WRITE |
> SDHC_DATA_INHIBIT | SDHC_DAT_LINE_ACTIVE);
>@@ -1244,6 +1275,10 @@ static inline void sdhci_reset_write(SDHCIState *s,
>uint8_t value)
> s->sdma_boundary_paused = false;
> s->norintsts &= ~(SDHC_NIS_WBUFRDY | SDHC_NIS_RBUFRDY |
> SDHC_NIS_DMA | SDHC_NIS_TRSCMP | SDHC_NIS_BLKGAP);
>+ if (!s->errintsts) {
>+ s->norintsts &= ~SDHC_NIS_ERR;
>+ }
>+ sdhci_update_irq(s);
> break;
> }
> }
>@@ -1257,10 +1292,6 @@ sdhci_write(void *opaque, hwaddr offset, uint64_t val,
>unsigned size)
> uint32_t value = val;
> value <<= shift;
>
>- if (timer_pending(s->transfer_timer)) {
>- sdhci_resume_pending_transfer(s);
>- }
>-
> switch (offset & ~0x3) {
> case SDHC_SYSAD:
> if (!TRANSFERRING_DATA(s->prnsts) ||
>@@ -1521,7 +1552,7 @@ void sdhci_initfn(SDHCIState *s)
> s->insert_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL,
> sdhci_raise_insertion_irq, s);
> s->transfer_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL,
>- sdhci_data_transfer, s);
>+ sdhci_adma_engine, s);
>
> s->io_ops = &sdhci_mmio_ops;
> }
