On Wed, Dec 14, 2022 at 11:50:50AM +0100, Ahmad Fatoum wrote: > eMMC Spec v5.1 (JEDEC Standard No. 84-B51) is quite clear that: > > Due to legacy reasons, a Device may still treat CMD24/25 during > prg-state (while busy is active) as a legal or illegal command. > A host should not send CMD24/25 while the Device is in the prg > state and busy is active. > > So far, we had never evaluated MMC_CMD_SEND_STATUS. This patch > corrects this and thereby resolves an issue of timed out writes > with the atmel_mci driver: > > barebox@SAMA5D4:/ saveenv > ERROR: atmel_mci fc000000.mmc@xxxxxxxxxxx: command/data timeout > ERROR: atmel_mci fc000000.mmc@xxxxxxxxxxx: command/data timeout > > These issues are not new, but were first reported in 2018[0] along > with a workaround suggesting a delay at the end of atmci_request: > > if (cmdidx == MMC_CMD_STOP_TRANSMISSION) > mdelay(5) > > Just before the command timing out, we read 0x00c00_0025 from the > status register, which is > > CMDRDY | TXRDY | NOTBUSY | XFERDONE | FIFOEMPTY > > which suggests the issue is not at the MCI host side, but rather at > the card side. With this patch applied and debugging enabled, this > seems to be confirmed: > > barebox@SAMA5D4:/ saveenv > saving environment > mmc1: Ready polling took 0ms > mmc1: Ready polling took 4ms > > I compared with AT91Bootstrap[1], U-Boot[2] and Trusted Firmware-A[3] > and all of them poll MCI status after block writes. > > The sequence imported here is taken from U-Boot, but instead of doing > it after writes, we do it before them in hope that we need not always > incur the extra delay. > > I don't have an answer why this was only necessary on the SAMA5D3/4 and > such issues weren't observed with other drivers. Card was operated > at 50MHz (SD HS) and it didn't help trying other cards or going down > to 400kHz. I tested this change also on a Beaglebone Black where an > environment is also stored into FAT on a SD-Card operated with 50 MHz: > Ready polling took 0ms for each of the two writes. > > [0]: https://lore.barebox.org/barebox/20181102091156.26476-1-s.hauer@xxxxxxxxxxxxxx/ > [1]: https://github.com/linux4sam/at91bootstrap/blob/v4.0.5/driver/mci_media.c#L1187 > [2]: https://github.com/trini/u-boot/blob/v2023.01-rc3/drivers/mmc/mmc_write.c#L181 > [3]: https://github.com/ARM-software/arm-trusted-firmware/blob/v2.8.0/drivers/mmc/mmc.c#L718 > > Signed-off-by: Ahmad Fatoum <a.fatoum@xxxxxxxxxxxxxx> > --- > For next, not master. I intend to test this change across more devices > in the coming weeks. > > v1 -> v2: > - skip mci_poll_until_ready for SPI mode (Sascha) > - simplify timeout handling (Sascha) > - turn dev_info into dev_dbg > - remove comment about debugging message placement > - increase timeout from 10ms to 1s Applied, thanks Sascha -- Pengutronix e.K. | | Steuerwalder Str. 21 | http://www.pengutronix.de/ | 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
