Support for TE SP-1 - #11217
Conversation
…arness stuff, cleanup more comments
tannewt
left a comment
There was a problem hiding this comment.
Thanks for the PR. I've got a few high level comments to start.
There was a problem hiding this comment.
Is EMMC a different protocol for SD cards? I'd rather this be generic instead of sp1 specific.
There was a problem hiding this comment.
My understand from talking with agent:
eMMC is a different protocol from SD, neither sdcardio (SPI-mode SD) nor sdioio (4-bit native SDIO peripherals) can drive this part. The differences this driver depends on:
- Init is CMD1 SEND_OP_COND polling, not SD's ACMD41 (+ CMD8 SEND_IF_COND).
- CMD8 means SEND_EXT_CSD here, a 512-byte data read — on SD it's the voltage handshake.
- Capacity comes from EXT_CSD[215:212] SEC_COUNT, not the CSD.
- Host assigns the RCA with CMD3 SET_RELATIVE_ADDR; on SD the card publishes it.
- High speed is CMD6 SWITCH writing EXT_CSD[185] HS_TIMING, gated on EXT_CSD[196] DEVICE_TYPE — completely different from
SD's CMD6 function-group switch. - eMMC ≥4.3 dropped SPI mode entirely, plus there's no card-detect, no insertion, and there is a hardware RST_n and a
separate VCCQ rail — all of which this module owns.
Lots of sp1emmc.c is a generic MMC protocol. CRC7/CRC16, command framing, the CMD0→CMD1→CMD2→CMD3→CMD7→CMD16 walk, EXT_CSD parse, CMD17/18 reads, CMD24/25+CMD12 writes with CRC-status and busy polling, the HS_TIMING switch, the blockdev ioctl. I think this could theoretically be moved out of the board dir if we wanted to re-use it for something else in this port.
sp1emmc_hw is specific to the SP-1. It hardcodes pins
Right now the bindings do not accept pins, so they're specific to this device. They could be changed to accept pins and maybe moved to shared-bindings as emmcio or similar if you foresee wanting them to be used more broadly, like for other ports that do have EMMC peripheral.
There was a problem hiding this comment.
Right now the bindings do not accept pins, so they're specific to this device. They could be changed to accept pins and maybe moved to
shared-bindingsasemmcioor similar if you foresee wanting them to be used more broadly, like for other ports that do have EMMC peripheral.
I feel like this would be worth doing. What version of emmc is this using?
| } | ||
| #endif | ||
|
|
||
| #if defined(CIRCUITPY_SP1EMMC) && CIRCUITPY_SP1EMMC |
There was a problem hiding this comment.
I'd definitely generalize emmcio because it is used here outside of ports.
The changes fall into these categories:
/sd. Somewhat specific to this hardware, but has some components that can be generalized for re-use if we come across another nordic device with EMMC wired similarly.asm volatilewas used as a workaround. I ran into this same symptom while working on this device (faders and 9 of the buttons all use analog). Claude diagnosed the issue and found a different potential cause for it and applied a fix. Honestly this part is over my head. I can confirm that the fixed code does work on this device though. I left somewhat verbose comments around these changes for now so that reviewers have more context to understand the root cause. I can remove or pare them down once someone with better understanding judges the identified cause and fix.external_clock=Truesupport. Previously only I2SIn supported that, but SP-1 needs it because the audio codec gets its clock from an onboard oscillator.