From d996224cb2c5fff664bd48571043ed8c730deaf8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bo=20St=C3=A5le=20Kopperud?= Date: Thu, 6 Aug 2026 11:56:48 +0200 Subject: [PATCH 1/6] sdcard: poll for command completion until the controller interrupts WaitCmd() slept on the completion signal unconditionally, so a controller whose interrupt never arrives wedged the boot instead of reporting a timeout. Run the handler from the polling path until a real interrupt has proved the line works. Ported from work by John Knipper. --- rom/devs/sdcard/sdcard_bus.c | 39 ++++++++++++++++++++++++++++++++++-- rom/devs/sdcard/sdcard_bus.h | 2 ++ 2 files changed, 39 insertions(+), 2 deletions(-) diff --git a/rom/devs/sdcard/sdcard_bus.c b/rom/devs/sdcard/sdcard_bus.c index 85a6b3cc75d..1c5e845e73f 100644 --- a/rom/devs/sdcard/sdcard_bus.c +++ b/rom/devs/sdcard/sdcard_bus.c @@ -896,7 +896,35 @@ ULONG FNAME_SDCBUS(WaitCmd)(ULONG mask, ULONG timeout, struct sdcard_Bus *bus) if (bus->sdcb_Task == FindTask(NULL)) { - Wait(1L << bus->sdcb_CommandSig); + /* + * Sleep for the completion interrupt once the controller has shown + * that it delivers them. Until then, poll instead: a controller whose + * interrupt never arrives would otherwise wedge the boot here rather + * than report a timeout. + */ + if (bus->sdcb_BusFlags & AF_Bus_IRQSeen) + { + Wait(1L << bus->sdcb_CommandSig); + } + else + { + ULONG waited = initialTimeout ? initialTimeout : 1000; + + /* + * Nothing will service the controller if its interrupt does not + * reach us, and a data transfer needs servicing to move the bytes + * at all, so run the handler from here while polling. + */ + while (waited--) + { + bus->sdcb_BusIRQHandler(bus, NULL); + + if (SetSignal(0, 0) & (1L << bus->sdcb_CommandSig)) + break; + sdcard_Udelay(1000); + } + SetSignal(0, 1L << bus->sdcb_CommandSig); + } } else { @@ -1066,6 +1094,13 @@ ULONG FNAME_SDCBUS(Rsp136Unpack)(ULONG *buf, ULONG offset, const ULONG len) #undef DIRQ #define DIRQ(x) /* x */ +/* Only a real interrupt records that the controller delivers them. */ +static void FNAME_SDCBUS(BusIRQEntry)(struct sdcard_Bus *bus, void *data) +{ + bus->sdcb_BusFlags |= AF_Bus_IRQSeen; + FNAME_SDCBUS(BusIRQ)(bus, data); +} + void FNAME_SDCBUS(BusIRQ)(struct sdcard_Bus *bus, void *_unused) { #if defined(__AROSEXEC_SMP__) @@ -1262,7 +1297,7 @@ void FNAME_SDCBUS(BusTask)(struct sdcard_Bus *bus) /* Install IRQ handler (controller-specific) */ if (bus->sdcb_BusIRQHandler) { - if ((bus->sdcb_IRQHandle = KrnAddIRQHandler(bus->sdcb_BusIRQ, bus->sdcb_BusIRQHandler, bus, NULL)) != NULL) + if ((bus->sdcb_IRQHandle = KrnAddIRQHandler(bus->sdcb_BusIRQ, FNAME_SDCBUS(BusIRQEntry), bus, NULL)) != NULL) { DINIT(bug("[SDBus%02u] %s: IRQHandle @ 0x%p for IRQ#%ld\n", bus->sdcb_BusNum, __PRETTY_FUNCTION__, bus->sdcb_IRQHandle, bus->sdcb_BusIRQ)); } diff --git a/rom/devs/sdcard/sdcard_bus.h b/rom/devs/sdcard/sdcard_bus.h index 5eb65b2042c..7e52dcbcf0d 100644 --- a/rom/devs/sdcard/sdcard_bus.h +++ b/rom/devs/sdcard/sdcard_bus.h @@ -101,11 +101,13 @@ struct sdcard_Bus #define AB_Bus_MediaPresent 30 /* media available */ #define AB_Bus_MediaChanged 29 /* media changed */ #define AB_Bus_SPI 28 +#define AB_Bus_IRQSeen 27 /* controller has delivered an interrupt */ #define AB_Bus_Active 1 #define AF_Bus_MediaPresent (1 << AB_Bus_MediaPresent) #define AF_Bus_MediaChanged (1 << AB_Bus_MediaChanged) #define AF_Bus_SPI (1 << AB_Bus_SPI) +#define AF_Bus_IRQSeen (1 << AB_Bus_IRQSeen) #define AF_Bus_Active (1 << AB_Bus_Active) BOOL FNAME_SDCBUS(RegisterUnit)(struct sdcard_Bus *); From 9009d96e48d2d87c9d289c6cb0074891c97bcc4d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bo=20St=C3=A5le=20Kopperud?= Date: Thu, 6 Aug 2026 18:46:23 +0200 Subject: [PATCH 2/6] sdcard: the polling path must call the bus's own interrupt handler The wrapper that records a working interrupt line called the SDHCI handler by name instead of dispatching through the bus, so the SDHOST bus - which uses a different register layout and leaves the SDHCI accessors NULL - crashed as soon as an interrupt arrived. Dispatch through sdcb_BusIRQHandler, and treat it as optional, the way BusTask already does. Introduced when the polling fallback was brought in. --- rom/devs/sdcard/sdcard_bus.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/rom/devs/sdcard/sdcard_bus.c b/rom/devs/sdcard/sdcard_bus.c index 1c5e845e73f..08bf64d8ec3 100644 --- a/rom/devs/sdcard/sdcard_bus.c +++ b/rom/devs/sdcard/sdcard_bus.c @@ -917,7 +917,8 @@ ULONG FNAME_SDCBUS(WaitCmd)(ULONG mask, ULONG timeout, struct sdcard_Bus *bus) */ while (waited--) { - bus->sdcb_BusIRQHandler(bus, NULL); + if (bus->sdcb_BusIRQHandler) + bus->sdcb_BusIRQHandler(bus, NULL); if (SetSignal(0, 0) & (1L << bus->sdcb_CommandSig)) break; @@ -1098,7 +1099,12 @@ ULONG FNAME_SDCBUS(Rsp136Unpack)(ULONG *buf, ULONG offset, const ULONG len) static void FNAME_SDCBUS(BusIRQEntry)(struct sdcard_Bus *bus, void *data) { bus->sdcb_BusFlags |= AF_Bus_IRQSeen; - FNAME_SDCBUS(BusIRQ)(bus, data); + + /* Dispatch through the bus's own handler: not every controller uses the + SDHCI register layout, and the SDHOST bus leaves the accessors this + one reads through set to NULL. */ + if (bus->sdcb_BusIRQHandler) + bus->sdcb_BusIRQHandler(bus, data); } void FNAME_SDCBUS(BusIRQ)(struct sdcard_Bus *bus, void *_unused) From 49de8b809d0401e9768247457760afb5ef99ed1d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bo=20St=C3=A5le=20Kopperud?= Date: Thu, 6 Aug 2026 19:03:03 +0200 Subject: [PATCH 3/6] sdcard: ask what the EMMC2 clock can do, not what it is doing GETCLKRATE answers with the rate the clock is running at, and on BCM2711 the EMMC2 clock is parked until something asks for it - so the driver read a maximum of 0Hz, never clocked the card, and the bus task sat there until the boot wait gave up. Fall back to GETMAXCLKRATE. --- .../2708/include/hardware/videocore.h | 1 + .../broadcom/2708/sdcard/sdcard_bcm2708init.c | 25 +++++++++++++++++++ 2 files changed, 26 insertions(+) diff --git a/arch/arm-native/soc/broadcom/2708/include/hardware/videocore.h b/arch/arm-native/soc/broadcom/2708/include/hardware/videocore.h index b24eac2cd7a..6e77b2e6de7 100644 --- a/arch/arm-native/soc/broadcom/2708/include/hardware/videocore.h +++ b/arch/arm-native/soc/broadcom/2708/include/hardware/videocore.h @@ -63,6 +63,7 @@ #define VCTAG_GETCLKSTATE 0x00030001 #define VCTAG_GETCLKRATE 0x00030002 +#define VCTAG_GETMAXCLKRATE 0x00030004 #define VCTAG_GETVOLTAGE 0x00030003 #define VCTAG_GETCLKMAX 0x00030004 #define VCTAG_GETVOLTMAX 0x00030005 diff --git a/arch/arm-native/soc/broadcom/2708/sdcard/sdcard_bcm2708init.c b/arch/arm-native/soc/broadcom/2708/sdcard/sdcard_bcm2708init.c index 5f7d7ca7106..24602f0aee7 100644 --- a/arch/arm-native/soc/broadcom/2708/sdcard/sdcard_bcm2708init.c +++ b/arch/arm-native/soc/broadcom/2708/sdcard/sdcard_bcm2708init.c @@ -204,6 +204,31 @@ static int FNAME_BCMSDC(BCM2708Init)(struct SDCardBase *SDCardBase) __BCM2708Bus->sdcb_BusIRQ = ctrlIRQ; __BCM2708Bus->sdcb_ClockMax = AROS_LE2LONG(MBoxMessage[6]); + + /* + * GETCLKRATE answers with the rate the clock is running at, and on + * BCM2711 the EMMC2 clock is parked until something asks for it, so + * the answer is zero and the card never gets clocked. Ask what the + * clock can do instead. + */ + if (__BCM2708Bus->sdcb_ClockMax == 0) + { + MBoxMessage[0] = AROS_LONG2LE(8 * 4); + MBoxMessage[1] = AROS_LONG2LE(VCTAG_REQ); + MBoxMessage[2] = AROS_LONG2LE(VCTAG_GETMAXCLKRATE); + MBoxMessage[3] = AROS_LONG2LE(8); + MBoxMessage[4] = AROS_LONG2LE(4); + MBoxMessage[5] = AROS_LONG2LE(ctrlClock); + MBoxMessage[6] = 0; + MBoxMessage[7] = 0; + + MBoxWrite((APTR)VCMB_BASE, VCMB_PROPCHAN, MBoxMessage); + if (MBoxRead((APTR)VCMB_BASE, VCMB_PROPCHAN) == MBoxMessage) + __BCM2708Bus->sdcb_ClockMax = AROS_LE2LONG(MBoxMessage[6]); + + DINIT(bug("[SDCard--] %s: clock was parked, max rate %d Hz\n", + __PRETTY_FUNCTION__, __BCM2708Bus->sdcb_ClockMax)); + } __BCM2708Bus->sdcb_ClockMin = BCM2708SDCLOCK_MIN; __BCM2708Bus->sdcb_LEDCtrl = (BYTE (*)(int))FNAME_BCMSDCBUS(BCMLEDCtrl); From 9d066f80f538fb56037a5dc0f9c46eb45a1f9356 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bo=20St=C3=A5le=20Kopperud?= Date: Sun, 9 Aug 2026 11:14:29 +0200 Subject: [PATCH 4/6] sdcard: clock EMMC2 from the controller's own base rate The divider ran against the mailbox's maximum of 500MHz while the block runs off 100MHz, and read the register as a plain divisor rather than SDHCI's base/(2N), so the card was clocked at 5.5MHz instead of 50. High speed timing is selected too, which the card needs above 25MHz. --- .../broadcom/2708/sdcard/sdcard_bcm2708bus.c | 13 +++++++-- .../broadcom/2708/sdcard/sdcard_bcm2708init.c | 27 ++++++++++++++++--- rom/devs/sdcard/sdcard_bus.c | 25 ++++++++++++++++- 3 files changed, 59 insertions(+), 6 deletions(-) diff --git a/arch/arm-native/soc/broadcom/2708/sdcard/sdcard_bcm2708bus.c b/arch/arm-native/soc/broadcom/2708/sdcard/sdcard_bcm2708bus.c index 964c5a75f6d..1eb4cf7213c 100644 --- a/arch/arm-native/soc/broadcom/2708/sdcard/sdcard_bcm2708bus.c +++ b/arch/arm-native/soc/broadcom/2708/sdcard/sdcard_bcm2708bus.c @@ -29,8 +29,17 @@ ULONG FNAME_SDCBUS(GetClockDiv)(ULONG speed, struct sdcard_Bus *bus) { ULONG __BCMClkDiv; - for (__BCMClkDiv = 0; __BCMClkDiv < V300_MAXCLKDIV; __BCMClkDiv++) { - if ((bus->sdcb_ClockMax / (__BCMClkDiv + 1)) <= speed) + /* + * The value programmed into CLOCK_CONTROL is not a divisor: SDHCI runs + * the card at base/(2*N), and N of zero is the special case that passes + * the base clock straight through. V300_MAXCLKDIV is the largest divisor, + * so the largest N is half of it. + */ + if (speed >= bus->sdcb_ClockMax) + return 0; + + for (__BCMClkDiv = 1; __BCMClkDiv < (V300_MAXCLKDIV / 2); __BCMClkDiv++) { + if ((bus->sdcb_ClockMax / (__BCMClkDiv * 2)) <= speed) break; } diff --git a/arch/arm-native/soc/broadcom/2708/sdcard/sdcard_bcm2708init.c b/arch/arm-native/soc/broadcom/2708/sdcard/sdcard_bcm2708init.c index 24602f0aee7..a56588effbd 100644 --- a/arch/arm-native/soc/broadcom/2708/sdcard/sdcard_bcm2708init.c +++ b/arch/arm-native/soc/broadcom/2708/sdcard/sdcard_bcm2708init.c @@ -286,9 +286,6 @@ static int FNAME_BCMSDC(BCM2708Init)(struct SDCardBase *SDCardBase) FNAME_SDCBUS(SoftReset)(SDHCI_RESET_ALL, __BCM2708Bus); - DINIT(bug("[SDCard--] %s: SDHC Max Clock Rate : %dMHz\n", __PRETTY_FUNCTION__, __BCM2708Bus->sdcb_ClockMax / 1000000)); - DINIT(bug("[SDCard--] %s: SDHC Min Clock Rate : %dHz (hardcoded)\n", __PRETTY_FUNCTION__, __BCM2708Bus->sdcb_ClockMin)); - __BCM2708Bus->sdcb_Version = FNAME_BCMSDCBUS(BCMMMIOReadWord)(SDHCI_HOST_VERSION, __BCM2708Bus); __BCM2708Bus->sdcb_Capabilities = FNAME_BCMSDCBUS(BCMMMIOReadLong)(SDHCI_CAPABILITIES, __BCM2708Bus); __BCM2708Bus->sdcb_Quirks = AB_Quirk_MissingCapabilities|AF_Quirk_AtomicTMAndCMD; @@ -298,6 +295,30 @@ static int FNAME_BCMSDC(BCM2708Init)(struct SDCardBase *SDCardBase) DINIT(bug("[SDCard--] %s: SDHCI Capabilities : 0x%08x\n", __PRETTY_FUNCTION__, __BCM2708Bus->sdcb_Capabilities)); DINIT(bug("[SDCard--] %s: SDHCI Voltages : 0x%08x (hardcoded)\n", __PRETTY_FUNCTION__, __BCM2708Bus->sdcb_Power)); + /* + * The mailbox reports what the clock could be turned up to, which + * is not what the divider divides. The controller knows its own + * base clock, so believe that instead: on the BCM2711 the mailbox + * says 500MHz while the block actually runs off 100MHz, and using + * the wrong one clocks the card five times too slowly. + */ + { + ULONG sdcClockBase; + + if ((__BCM2708Bus->sdcb_Version & SDHCI_HVERS_SPEC_MASK) >= 2) + sdcClockBase = (__BCM2708Bus->sdcb_Capabilities & SDHCI_CLOCK_V3_BASE_MASK) >> SDHCI_CLOCK_BASE_SHIFT; + else + sdcClockBase = (__BCM2708Bus->sdcb_Capabilities & SDHCI_CLOCK_BASE_MASK) >> SDHCI_CLOCK_BASE_SHIFT; + + if (sdcClockBase) + __BCM2708Bus->sdcb_ClockMax = sdcClockBase * 1000000; + else + DINIT(bug("[SDCard--] %s: controller reports no base clock, keeping the mailbox rate\n", __PRETTY_FUNCTION__)); + } + + DINIT(bug("[SDCard--] %s: SDHC Base Clock Rate : %dMHz\n", __PRETTY_FUNCTION__, __BCM2708Bus->sdcb_ClockMax / 1000000)); + DINIT(bug("[SDCard--] %s: SDHC Min Clock Rate : %dHz (hardcoded)\n", __PRETTY_FUNCTION__, __BCM2708Bus->sdcb_ClockMin)); + __BCM2708Bus->sdcb_Private = (IPTR)sdcard_CurrentTime(); FNAME_SDC(RegisterBus)(__BCM2708Bus, SDCardBase); diff --git a/rom/devs/sdcard/sdcard_bus.c b/rom/devs/sdcard/sdcard_bus.c index 08bf64d8ec3..c538aab46dc 100644 --- a/rom/devs/sdcard/sdcard_bus.c +++ b/rom/devs/sdcard/sdcard_bus.c @@ -509,6 +509,7 @@ void FNAME_SDCBUS(SetClock)(ULONG speed, struct sdcard_Bus *bus) { ULONG sdcClkDiv, timeout; UWORD sdcClkCtrlCur, sdcClkCtrl; + BOOL sdcReprogram = FALSE; DFUNCS(bug("[SDBus%02u] %s()\n", bus->sdcb_BusNum, __PRETTY_FUNCTION__)); @@ -519,7 +520,29 @@ void FNAME_SDCBUS(SetClock)(ULONG speed, struct sdcard_Bus *bus) sdcClkCtrl = (sdcClkDiv & SDHCI_DIV_MASK) << SDHCI_DIVIDER_SHIFT; sdcClkCtrl |= ((sdcClkDiv & SDHCI_DIV_HI_MASK) >> SDHCI_DIV_MASK_LEN) << SDHCI_DIVIDER_HI_SHIFT; - if (sdcClkCtrl != (sdcClkCtrlCur & ~(SDHCI_CLOCK_INT_EN|SDHCI_CLOCK_INT_STABLE|SDHCI_CLOCK_CARD_EN))) + /* + * Past 25MHz the controller has to sample with high speed timing, and the + * bit that selects it may only be touched while the card clock is stopped. + */ + if (bus->sdcb_Capabilities & SDHCI_CAN_DO_HISPD) + { + UBYTE sdcCtrlCur = bus->sdcb_IOReadByte(SDHCI_HOST_CONTROL, bus); + UBYTE sdcCtrlNew = (speed > 25000000) ? (sdcCtrlCur | SDHCI_HCTRL_HISPD) + : (sdcCtrlCur & ~SDHCI_HCTRL_HISPD); + + if (sdcCtrlNew != sdcCtrlCur) + { + D(bug("[SDBus%02u] %s: %s high speed timing\n", bus->sdcb_BusNum, __PRETTY_FUNCTION__, + (sdcCtrlNew & SDHCI_HCTRL_HISPD) ? "Enabling" : "Disabling")); + + bus->sdcb_IOWriteWord(SDHCI_CLOCK_CONTROL, 0, bus); + bus->sdcb_IOWriteByte(SDHCI_HOST_CONTROL, sdcCtrlNew, bus); + sdcReprogram = TRUE; + } + } + + if (sdcReprogram || + (sdcClkCtrl != (sdcClkCtrlCur & ~(SDHCI_CLOCK_INT_EN|SDHCI_CLOCK_INT_STABLE|SDHCI_CLOCK_CARD_EN)))) { bus->sdcb_IOWriteWord(SDHCI_CLOCK_CONTROL, 0, bus); From 80dba2cba05d7b35ee7ee2f33f2a5bebabdbdd67 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bo=20St=C3=A5le=20Kopperud?= Date: Sun, 9 Aug 2026 11:14:44 +0200 Subject: [PATCH 5/6] sdcard: poll the controller at the scale the card works at Waiting a millisecond at a time cost more than the work being waited for: a 64K transfer takes about 2.7ms, so one overshoot was a quarter of it. The waits now run at 25us and the timeouts are measured against the system timer rather than counted in loop iterations, so the callers' limits are unchanged. --- rom/devs/sdcard/sdcard_bus.c | 50 ++++++++++++++++++++++++++---------- 1 file changed, 37 insertions(+), 13 deletions(-) diff --git a/rom/devs/sdcard/sdcard_bus.c b/rom/devs/sdcard/sdcard_bus.c index c538aab46dc..47499b81511 100644 --- a/rom/devs/sdcard/sdcard_bus.c +++ b/rom/devs/sdcard/sdcard_bus.c @@ -40,6 +40,16 @@ /* Generic "Bus Unit" Functions */ +/* How long a single block may keep the PIO transfer waiting */ +#define SDHCI_DATA_TIMEOUT_US 1000000 + +/* + * How often the controller is re-examined while waiting on it. Waiting a + * millisecond at a time costs more than the work being waited for: a 64K + * transfer only takes about 2.7ms, so a single overshoot is a quarter of it. + */ +#define SDHCI_POLL_US 25 + static const char *str_mmc0 = "MMC0"; BOOL FNAME_SDCBUS(StartUnit)(struct sdcard_Unit *sdcUnit) @@ -793,9 +803,9 @@ ULONG FNAME_SDCBUS(FinishData)(struct TagItem *DataTags, struct sdcard_Bus *bus) { DTRANS(UWORD sdCommand = (UWORD)GetTagData(SDCARD_TAG_CMD, 0, DataTags)); ULONG sdcStateMask, sdCommandMask, - sdData, sdDataMode, sdDataLen, sdcReg = 0; struct TagItem *sdDataLenTag = NULL; - ULONG timeout = 1000; + ULONG waitStart = 0; + BOOL waiting = FALSE; ULONG retVal = 0; DFUNCS(bug("[SDBus%02u] %s()\n", bus->sdcb_BusNum, __PRETTY_FUNCTION__)); @@ -853,12 +863,24 @@ ULONG FNAME_SDCBUS(FinishData)(struct TagItem *DataTags, struct sdcard_Bus *bus) } sdData += tranlen; sdDataLen -= tranlen; + waiting = FALSE; } else if (!(bus->sdcb_BusStatus & SDHCI_INT_DATA_END)) { - sdcard_Udelay(1000); + /* + * The card needs about 20us to put the next block in the + * buffer, so poll at that scale: waiting a millisecond for + * each block of a multi-block transfer costs more time than + * the transfer itself. + */ + if (!waiting) + { + waitStart = sdcard_CurrentTime(); + waiting = TRUE; + } + sdcard_Udelay(1); - if (timeout-- <= 0) + if ((sdcard_CurrentTime() - waitStart) > SDHCI_DATA_TIMEOUT_US) { bug("[SDBus%02u] %s: Timeout!\n", bus->sdcb_BusNum, __PRETTY_FUNCTION__); retVal = -1; @@ -915,7 +937,10 @@ ULONG FNAME_SDCBUS(WaitCmd)(ULONG mask, ULONG timeout, struct sdcard_Bus *bus) #if defined(__AROSEXEC_SMP__) struct SDCardBase *SDCardBase = bus->sdcb_DeviceBase; #endif - ULONG initialTimeout = timeout; + /* Callers count their timeout in milliseconds. */ + ULONG waitStart = sdcard_CurrentTime(); + ULONG waitLimit = (timeout ? timeout : 1000) * 1000; + BOOL timedOut = FALSE; if (bus->sdcb_Task == FindTask(NULL)) { @@ -931,32 +956,30 @@ ULONG FNAME_SDCBUS(WaitCmd)(ULONG mask, ULONG timeout, struct sdcard_Bus *bus) } else { - ULONG waited = initialTimeout ? initialTimeout : 1000; - /* * Nothing will service the controller if its interrupt does not * reach us, and a data transfer needs servicing to move the bytes * at all, so run the handler from here while polling. */ - while (waited--) + while ((sdcard_CurrentTime() - waitStart) < waitLimit) { if (bus->sdcb_BusIRQHandler) bus->sdcb_BusIRQHandler(bus, NULL); if (SetSignal(0, 0) & (1L << bus->sdcb_CommandSig)) break; - sdcard_Udelay(1000); + sdcard_Udelay(SDHCI_POLL_US); } SetSignal(0, 1L << bus->sdcb_CommandSig); } } else { - sdcard_Udelay(1000); + sdcard_Udelay(SDHCI_POLL_US); } while (bus->sdcb_IOReadLong(SDHCI_PRESENT_STATE, bus) & mask) { - sdcard_Udelay(1000); + sdcard_Udelay(SDHCI_POLL_US); /* * Lost interrupt recovery for data transfers. @@ -1016,12 +1039,13 @@ ULONG FNAME_SDCBUS(WaitCmd)(ULONG mask, ULONG timeout, struct sdcard_Bus *bus) if ((bus->sdcb_BusStatus & SDHCI_INT_ERROR) == SDHCI_INT_ERROR) break; - if (--timeout <= 0) + if ((sdcard_CurrentTime() - waitStart) > waitLimit) { bug("[SDBus%02u] WaitCmd: TIMEOUT! PS=%08x BS=%08x DL=%p\n", bus->sdcb_BusNum, bus->sdcb_IOReadLong(SDHCI_PRESENT_STATE, bus), bus->sdcb_BusStatus, bus->sdcb_DataListener); + timedOut = TRUE; break; } } @@ -1091,7 +1115,7 @@ ULONG FNAME_SDCBUS(WaitCmd)(ULONG mask, ULONG timeout, struct sdcard_Bus *bus) } } - if ((timeout <= 0) || (bus->sdcb_BusStatus & SDHCI_INT_ERROR)) + if (timedOut || (bus->sdcb_BusStatus & SDHCI_INT_ERROR)) { return -1; } From f5d7addc5759e236c9d45761a3e3f326716fb19a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bo=20St=C3=A5le=20Kopperud?= Date: Sun, 9 Aug 2026 11:15:04 +0200 Subject: [PATCH 6/6] sdcard: let the SDHCI controller fetch the data itself PIO reads a block at a time from inside the interrupt handler, which holds the machine for as long as the transfer lasts - 2.7ms for every 64K. ADMA2 is used where the controller offers it, and proves itself against a PIO read of the same sectors at startup before anything relies on it. --- .../broadcom/2708/sdcard/sdcard_bcm2708init.c | 5 +- rom/devs/sdcard/sdcard_bus.c | 277 +++++++++++++++++- rom/devs/sdcard/sdcard_bus.h | 42 +++ 3 files changed, 322 insertions(+), 2 deletions(-) diff --git a/arch/arm-native/soc/broadcom/2708/sdcard/sdcard_bcm2708init.c b/arch/arm-native/soc/broadcom/2708/sdcard/sdcard_bcm2708init.c index a56588effbd..0337e6c75cb 100644 --- a/arch/arm-native/soc/broadcom/2708/sdcard/sdcard_bcm2708init.c +++ b/arch/arm-native/soc/broadcom/2708/sdcard/sdcard_bcm2708init.c @@ -38,7 +38,8 @@ static void FNAME_BCMSDC(SDBusInit)(struct sdcard_Bus *bus) SDHCI_INT_END_BIT | SDHCI_INT_CRC | SDHCI_INT_TIMEOUT | SDHCI_INT_CARD_REMOVE | SDHCI_INT_CARD_INSERT | SDHCI_INT_DATA_AVAIL | SDHCI_INT_SPACE_AVAIL | - SDHCI_INT_DATA_END | SDHCI_INT_RESPONSE; + SDHCI_INT_DATA_END | SDHCI_INT_RESPONSE | + SDHCI_INT_ADMA_ERROR; FNAME_SDCBUS(SetClock)(bus->sdcb_ClockMin, bus); FNAME_SDCBUS(SetPowerLevel)(bus->sdcb_Power, FALSE, bus); @@ -319,6 +320,8 @@ static int FNAME_BCMSDC(BCM2708Init)(struct SDCardBase *SDCardBase) DINIT(bug("[SDCard--] %s: SDHC Base Clock Rate : %dMHz\n", __PRETTY_FUNCTION__, __BCM2708Bus->sdcb_ClockMax / 1000000)); DINIT(bug("[SDCard--] %s: SDHC Min Clock Rate : %dHz (hardcoded)\n", __PRETTY_FUNCTION__, __BCM2708Bus->sdcb_ClockMin)); + FNAME_SDCBUS(ADMAAlloc)(__BCM2708Bus); + __BCM2708Bus->sdcb_Private = (IPTR)sdcard_CurrentTime(); FNAME_SDC(RegisterBus)(__BCM2708Bus, SDCardBase); diff --git a/rom/devs/sdcard/sdcard_bus.c b/rom/devs/sdcard/sdcard_bus.c index 47499b81511..3308870b542 100644 --- a/rom/devs/sdcard/sdcard_bus.c +++ b/rom/devs/sdcard/sdcard_bus.c @@ -166,6 +166,9 @@ BOOL FNAME_SDCBUS(StartUnit)(struct sdcard_Unit *sdcUnit) } } } + + /* Prove the controller's own transfers land where we think they do. */ + FNAME_SDCBUS(ADMAVerify)(sdcUnit); } D(bug("[SDCard%02ld] %s: Done.\n", sdcUnit->sdcu_UnitNum, __PRETTY_FUNCTION__)); @@ -630,6 +633,225 @@ void FNAME_SDCBUS(SetPowerLevel)(ULONG supportedlvls, BOOL lowest, struct sdcard #undef D #define D(x) /*x*/ +/********** ADMA2 **************/ + +/* + * The address the controller puts on the bus. The EMMC2 block is an ARM side + * master and sees plain physical addresses, unlike the VideoCore DMA engine + * the SDHOST driver drives, which needs the 0xC0000000 alias. A controller + * that needs a different view should have it applied here and nowhere else. + */ +static IPTR FNAME_SDCBUS(ADMAPhys)(struct sdcard_Bus *bus, APTR virt) +{ + struct SDCardBase *SDCardBase = bus->sdcb_DeviceBase; + + return (IPTR)KrnVirtualToPhysical(virt); +} + +BOOL FNAME_SDCBUS(ADMAAlloc)(struct sdcard_Bus *bus) +{ + APTR desc, bounce; + + if (!(bus->sdcb_Capabilities & SDHCI_CAN_DO_ADMA2)) + { + bug("[SDBus%02u] controller has no ADMA2, transfers will use PIO\n", bus->sdcb_BusNum); + return FALSE; + } + + /* + * Both live below 2GB because the controller cannot address more, and + * both are padded out to whole cache lines: cache maintenance works a + * line at a time and would otherwise reach into whatever shares them. + */ + desc = AllocMem((ADMA2_MAX_DESC * sizeof(struct sdcard_ADMADesc)) + 64, + MEMF_PUBLIC | MEMF_CLEAR | MEMF_31BIT); + bounce = AllocMem(ADMA2_BOUNCE_SIZE + 64, MEMF_PUBLIC | MEMF_31BIT); + + if (!desc || !bounce) + { + bug("[SDBus%02u] no memory for ADMA2, transfers will use PIO\n", bus->sdcb_BusNum); + if (desc) + FreeMem(desc, (ADMA2_MAX_DESC * sizeof(struct sdcard_ADMADesc)) + 64); + if (bounce) + FreeMem(bounce, ADMA2_BOUNCE_SIZE + 64); + return FALSE; + } + + bus->sdcb_ADMADesc = (APTR)(((IPTR)desc + 63) & ~63); + bus->sdcb_ADMABounce = (APTR)(((IPTR)bounce + 63) & ~63); + bus->sdcb_ADMABounceSize = ADMA2_BOUNCE_SIZE; + bus->sdcb_BusFlags |= AF_Bus_DMA; + + D(bug("[SDBus%02u] ADMA2 descriptors @ 0x%p, bounce @ 0x%p\n", bus->sdcb_BusNum, + bus->sdcb_ADMADesc, bus->sdcb_ADMABounce)); + + return TRUE; +} + +/* + * Describe the caller's buffer to the controller. Returns FALSE to leave the + * transfer to PIO, which is always a valid thing to fall back on. + */ +static BOOL FNAME_SDCBUS(ADMASetup)(struct sdcard_Bus *bus, APTR data, ULONG len, BOOL isWrite) +{ + struct sdcard_ADMADesc *desc = bus->sdcb_ADMADesc; + UBYTE *pos; + ULONG remaining; + UBYTE sdcHostCtrl; + UWORD n = 0; + + if (!(bus->sdcb_BusFlags & AF_Bus_DMA) || (data == NULL) || (len == 0)) + return FALSE; + + bus->sdcb_BusFlags &= ~AF_Bus_DMABounced; + + /* + * Cache maintenance works a line at a time, so a buffer that starts or + * ends mid line would drag its neighbours along. Anything unaligned goes + * through the bounce buffer instead. + */ + if (((IPTR)data | (IPTR)len) & 63) + { + if (len > bus->sdcb_ADMABounceSize) + return FALSE; + + bus->sdcb_BusFlags |= AF_Bus_DMABounced; + } + + bus->sdcb_ADMAData = data; + bus->sdcb_ADMALen = len; + + pos = (bus->sdcb_BusFlags & AF_Bus_DMABounced) ? bus->sdcb_ADMABounce : data; + + if ((bus->sdcb_BusFlags & AF_Bus_DMABounced) && isWrite) + CopyMem(data, pos, len); + + /* + * Walk the buffer a page at a time, merging pages that turn out to be + * physically adjacent. On a flat mapping that collapses to one descriptor, + * and where it does not, ADMA2 handles the scatter for us. + */ + remaining = len; + while (remaining > 0) + { + IPTR phys = FNAME_SDCBUS(ADMAPhys)(bus, pos); + ULONG run = 4096 - ((ULONG)((IPTR)pos & 4095)); + + if (run > remaining) + run = remaining; + + while ((run < remaining) && (run < ADMA2_MAX_XFER) && + (FNAME_SDCBUS(ADMAPhys)(bus, pos + run) == (phys + run))) + { + ULONG more = ((remaining - run) > 4096) ? 4096 : (remaining - run); + + if ((run + more) > ADMA2_MAX_XFER) + more = ADMA2_MAX_XFER - run; + + run += more; + } + + /* Out of the controller's reach, or too fragmented to describe. */ + if ((n >= ADMA2_MAX_DESC) || (((UQUAD)phys + run) > 0x100000000ULL)) + { + DTRANS(bug("[SDBus%02u] %s: buffer needs PIO (desc %d, phys 0x%p)\n", + bus->sdcb_BusNum, __PRETTY_FUNCTION__, n, (APTR)phys)); + return FALSE; + } + + desc[n].ad_Attr = AROS_WORD2LE(ADMA2_ATTR_VALID | ADMA2_ATTR_ACT_TRAN); + desc[n].ad_Length = AROS_WORD2LE((UWORD)run); + desc[n].ad_Address = AROS_LONG2LE((ULONG)phys); + n++; + + pos += run; + remaining -= run; + } + + /* The last one closes the list and raises the completion interrupt. */ + desc[n - 1].ad_Attr = AROS_WORD2LE(ADMA2_ATTR_VALID | ADMA2_ATTR_END | + ADMA2_ATTR_INT | ADMA2_ATTR_ACT_TRAN); + + /* + * Push the descriptors and, for a write, the data out of the cache so the + * controller reads what we just wrote. For a read this also drops any + * dirty lines that would otherwise be written back over the result. + */ + CacheClearE(bus->sdcb_ADMADesc, n * sizeof(struct sdcard_ADMADesc), CACRF_ClearD); + CacheClearE((bus->sdcb_BusFlags & AF_Bus_DMABounced) ? bus->sdcb_ADMABounce : data, + len, CACRF_ClearD); + + bus->sdcb_IOWriteLong(SDHCI_ADMA_ADDRESS, + (ULONG)FNAME_SDCBUS(ADMAPhys)(bus, bus->sdcb_ADMADesc), bus); + + sdcHostCtrl = bus->sdcb_IOReadByte(SDHCI_HOST_CONTROL, bus); + sdcHostCtrl = (sdcHostCtrl & ~SDHCI_HCTRL_DMA_MASK) | SDHCI_HCTRL_ADMA32; + bus->sdcb_IOWriteByte(SDHCI_HOST_CONTROL, sdcHostCtrl, bus); + + bus->sdcb_BusFlags |= AF_Bus_DMAActive; + + DTRANS(bug("[SDBus%02u] %s: %d descriptor(s) for %d bytes%s\n", bus->sdcb_BusNum, + __PRETTY_FUNCTION__, n, len, + (bus->sdcb_BusFlags & AF_Bus_DMABounced) ? " (bounced)" : "")); + + return TRUE; +} + +/* + * Read the same sectors twice, once each way, and compare. Getting the address + * translation wrong would otherwise show up as quietly corrupted data rather + * than as an error, so prove it once at startup and fall back to PIO for good + * if the two disagree. + */ +void FNAME_SDCBUS(ADMAVerify)(struct sdcard_Unit *sdcUnit) +{ + struct sdcard_Bus *bus = sdcUnit->sdcu_Bus; + ULONG len = 4 << bus->sdcb_SectorShift; + UBYTE *viaDMA, *viaPIO; + ULONG act = 0; + BYTE err; + + if (!(bus->sdcb_BusFlags & AF_Bus_DMA)) + return; + + viaDMA = AllocMem(len, MEMF_PUBLIC | MEMF_CLEAR | MEMF_31BIT); + viaPIO = AllocMem(len, MEMF_PUBLIC | MEMF_CLEAR | MEMF_31BIT); + + if (!viaDMA || !viaPIO) + { + if (viaDMA) + FreeMem(viaDMA, len); + if (viaPIO) + FreeMem(viaPIO, len); + return; + } + + err = (sdcUnit->sdcu_Flags & AF_Card_HighCapacity) + ? sdcUnit->sdcu_Read64(sdcUnit, 0, 4, viaDMA, &act) + : sdcUnit->sdcu_Read32(sdcUnit, 0, 4, viaDMA, &act); + + bus->sdcb_BusFlags &= ~AF_Bus_DMA; + + if (err == 0) + err = (sdcUnit->sdcu_Flags & AF_Card_HighCapacity) + ? sdcUnit->sdcu_Read64(sdcUnit, 0, 4, viaPIO, &act) + : sdcUnit->sdcu_Read32(sdcUnit, 0, 4, viaPIO, &act); + + if ((err == 0) && (memcmp(viaDMA, viaPIO, len) == 0)) + { + bus->sdcb_BusFlags |= AF_Bus_DMA; + bug("[SDBus%02u] ADMA2 verified against PIO, using DMA for transfers\n", bus->sdcb_BusNum); + } + else + { + bug("[SDBus%02u] ADMA2 self test FAILED (err %ld) - staying on PIO\n", + bus->sdcb_BusNum, (LONG)err); + } + + FreeMem(viaDMA, len); + FreeMem(viaPIO, len); +} + ULONG FNAME_SDCBUS(SendCmd)(struct TagItem *CmdTags, struct sdcard_Bus *bus) { #if defined(__AROSEXEC_SMP__) @@ -713,6 +935,18 @@ D(bug("SendCmd(%d,%d)\n", sdCommand, sdDataLen)); if (sdDataFlags == MMC_DATA_READ) sdcTransMode |= SDHCI_TRANSMOD_READ; + /* + * Let the controller fetch the data itself where it can. PIO reads a + * block at a time from inside the interrupt handler, which holds the + * machine for as long as the transfer lasts. + */ + bus->sdcb_BusFlags &= ~AF_Bus_DMAActive; + if (FNAME_SDCBUS(ADMASetup)(bus, (APTR)GetTagData(SDCARD_TAG_DATA, 0, CmdTags), + sdDataLen, (sdDataFlags != MMC_DATA_READ))) + { + sdcTransMode |= SDHCI_TRANSMOD_DMA; + } + if (!(bus->sdcb_Quirks & AF_Quirk_AtomicTMAndCMD)) { bus->sdcb_IOWriteWord(SDHCI_TRANSFER_MODE, sdcTransMode, bus); @@ -803,6 +1037,7 @@ ULONG FNAME_SDCBUS(FinishData)(struct TagItem *DataTags, struct sdcard_Bus *bus) { DTRANS(UWORD sdCommand = (UWORD)GetTagData(SDCARD_TAG_CMD, 0, DataTags)); ULONG sdcStateMask, sdCommandMask, + sdData, sdDataMode = MMC_DATA_READ, sdDataLen, sdcReg = 0; struct TagItem *sdDataLenTag = NULL; ULONG waitStart = 0; BOOL waiting = FALSE; @@ -830,6 +1065,36 @@ ULONG FNAME_SDCBUS(FinishData)(struct TagItem *DataTags, struct sdcard_Bus *bus) } }; + /* + * When the controller did the transfer there is nothing to move; the bytes + * are already in memory. All that is left is to make them visible to the + * CPU and, if it went the long way round, to copy them back. + */ + if (bus->sdcb_BusFlags & AF_Bus_DMAActive) + { + bus->sdcb_BusFlags &= ~AF_Bus_DMAActive; + + CacheClearE((bus->sdcb_BusFlags & AF_Bus_DMABounced) + ? bus->sdcb_ADMABounce : bus->sdcb_ADMAData, + bus->sdcb_ADMALen, CACRF_ClearD); + + if (bus->sdcb_BusFlags & AF_Bus_DMABounced) + { + if (sdDataMode == MMC_DATA_READ) + CopyMem(bus->sdcb_ADMABounce, bus->sdcb_ADMAData, bus->sdcb_ADMALen); + + bus->sdcb_BusFlags &= ~AF_Bus_DMABounced; + } + + if (bus->sdcb_LEDCtrl) + bus->sdcb_LEDCtrl(LED_OFF); + + DTRANS(bug("[SDBus%02u] %s: ADMA2 moved %d bytes\n", bus->sdcb_BusNum, + __PRETTY_FUNCTION__, bus->sdcb_ADMALen)); + + return 0; + } + if (sdData) { DTRANS(bug("[SDBus%02u] %s: Transfering CMD %02d Data..\n", bus->sdcb_BusNum, __PRETTY_FUNCTION__, sdCommand)); @@ -1145,6 +1410,9 @@ ULONG FNAME_SDCBUS(Rsp136Unpack)(ULONG *buf, ULONG offset, const ULONG len) /* Only a real interrupt records that the controller delivers them. */ static void FNAME_SDCBUS(BusIRQEntry)(struct sdcard_Bus *bus, void *data) { + if (!(bus->sdcb_BusFlags & AF_Bus_IRQSeen)) + bug("[SDBus%02u] controller interrupt is being delivered\n", bus->sdcb_BusNum); + bus->sdcb_BusFlags |= AF_Bus_IRQSeen; /* Dispatch through the bus's own handler: not every controller uses the @@ -1239,12 +1507,19 @@ void FNAME_SDCBUS(BusIRQ)(struct sdcard_Bus *bus, void *_unused) if (bus->sdcb_BusStatus & SDHCI_INT_DATA_MASK) { struct TagItem *dataListener; + /* + * An ADMA2 transfer never reports a buffer ready - the controller + * is doing the moving - so its completion is the transfer end. + */ + ULONG claimMask = (bus->sdcb_BusFlags & AF_Bus_DMAActive) + ? SDHCI_INT_DATA_END + : (SDHCI_INT_DATA_AVAIL | SDHCI_INT_SPACE_AVAIL); #if defined(__AROSEXEC_SMP__) KrnSpinLock(&bus->sdcb_Lock, NULL, SPINLOCK_MODE_WRITE); #endif dataListener = bus->sdcb_DataListener; - if ((bus->sdcb_BusStatus & (SDHCI_INT_DATA_AVAIL|SDHCI_INT_SPACE_AVAIL)) && dataListener) + if ((bus->sdcb_BusStatus & claimMask) && dataListener) bus->sdcb_DataListener = NULL; else dataListener = NULL; diff --git a/rom/devs/sdcard/sdcard_bus.h b/rom/devs/sdcard/sdcard_bus.h index 7e52dcbcf0d..aac116d7f01 100644 --- a/rom/devs/sdcard/sdcard_bus.h +++ b/rom/devs/sdcard/sdcard_bus.h @@ -89,6 +89,18 @@ struct sdcard_Bus void (*sdcb_BusInit)(struct sdcard_Bus *); /* Scan-time HW init (clock, power, bus width) */ void (*sdcb_BusPostIRQInit)(struct sdcard_Bus *); /* Post-IRQ init (enable interrupts, card detect) */ + /* + * ADMA2 scatter/gather. The controller masters the transfer itself, so + * the descriptor table and any bounce buffer have to live where it can + * reach them: the capabilities register says whether it can address more + * than 32 bits, and on the BCM2711 it cannot. + */ + APTR sdcb_ADMADesc; /* descriptor table */ + APTR sdcb_ADMABounce; /* used when the caller's buffer will not do */ + ULONG sdcb_ADMABounceSize; + APTR sdcb_ADMAData; /* caller's buffer, for the copy back */ + ULONG sdcb_ADMALen; + /* Bus Instance Private/Internal */ IPTR sdcb_Private; @@ -102,14 +114,41 @@ struct sdcard_Bus #define AB_Bus_MediaChanged 29 /* media changed */ #define AB_Bus_SPI 28 #define AB_Bus_IRQSeen 27 /* controller has delivered an interrupt */ +#define AB_Bus_DMA 26 /* ADMA2 is available and trusted */ +#define AB_Bus_DMAActive 25 /* the transfer in flight is being done by ADMA2 */ +#define AB_Bus_DMABounced 24 /* that transfer went through the bounce buffer */ #define AB_Bus_Active 1 #define AF_Bus_MediaPresent (1 << AB_Bus_MediaPresent) #define AF_Bus_MediaChanged (1 << AB_Bus_MediaChanged) #define AF_Bus_SPI (1 << AB_Bus_SPI) #define AF_Bus_IRQSeen (1 << AB_Bus_IRQSeen) +#define AF_Bus_DMA (1 << AB_Bus_DMA) +#define AF_Bus_DMAActive (1 << AB_Bus_DMAActive) +#define AF_Bus_DMABounced (1 << AB_Bus_DMABounced) #define AF_Bus_Active (1 << AB_Bus_Active) +/* + * ADMA2 descriptor. Two byte attribute and length fields followed by a 32bit + * address; a length of zero means the maximum, 65536 bytes. + */ +struct sdcard_ADMADesc +{ + UWORD ad_Attr; + UWORD ad_Length; + ULONG ad_Address; +}; + +#define ADMA2_ATTR_VALID (1 << 0) +#define ADMA2_ATTR_END (1 << 1) +#define ADMA2_ATTR_INT (1 << 2) +#define ADMA2_ATTR_ACT_TRAN (2 << 4) + +/* Kept well under the 65536 a descriptor can hold, and a multiple of any sector size */ +#define ADMA2_MAX_XFER 32768 +#define ADMA2_MAX_DESC 32 +#define ADMA2_BOUNCE_SIZE (128 * 1024) + BOOL FNAME_SDCBUS(RegisterUnit)(struct sdcard_Bus *); BOOL FNAME_SDCBUS(StartUnit)(struct sdcard_Unit *); @@ -123,6 +162,9 @@ ULONG FNAME_SDCBUS(FinishCmd)(struct TagItem *, struct sdcard_Bus *); ULONG FNAME_SDCBUS(FinishData)(struct TagItem *, struct sdcard_Bus *); ULONG FNAME_SDCBUS(Rsp136Unpack)(ULONG *, ULONG, const ULONG); +BOOL FNAME_SDCBUS(ADMAAlloc)(struct sdcard_Bus *); +void FNAME_SDCBUS(ADMAVerify)(struct sdcard_Unit *); + void FNAME_SDCBUS(BusIRQ)(struct sdcard_Bus *, void *); void FNAME_SDCBUS(BusTask)(struct sdcard_Bus *);