Re: [PATCH v2] mci: only write blocks when card out of programming mode

[Sascha Hauer <sha@pengutronix.de>]

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@fc000000.of: command/data timeout
>   ERROR: atmel_mci fc000000.mmc@fc000000.of: 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@pengutronix.de/
> [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@pengutronix.de>
> ---
> 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 |