Re: [PATCH] [RFC] Revert "mmc: core: Fixup support for writeback-cache for eMMC and SD"

[Marek Vasut <marex@denx.de>]

On 5/31/23 15:08, Christian Loehle wrote:
> One thing that comes to my mind that would explain the behavior, since all commands itself complete fine is:
> Issuing sd cache flush:
> [   61.992588] mmc1: starting CMD49 arg 10020a00 flags 00000035
> [   61.998152] mmc1:     blksz 512 blocks 1 flags 00000100 tsac 3000 ms
> nsac 0
> [   62.005158] mmci-pl18x 58005000.mmc: blksz 0200 blks 0001 flags 00000100
> [   62.011778] mmci-pl18x 58005000.mmc: op 31 arg 10020a00 flags 00000035
> [   62.018317] mmci-pl18x 58005000.mmc: irq0 (data+cmd) 00000040
> [   62.024011] mmci-pl18x 58005000.mmc: irq0 (data+cmd) 00000100
> [   62.029801] mmc1: req done (CMD49): 0: 00000900 00000000 00000000
> 00000000
> [   62.036708] mmc1:     512 bytes transferred: 0
> [   62.041083] mmci-pl18x 58005000.mmc: irq0 (data+cmd) 00000000
> [   62.046988] mmc1: starting CMD48 arg 10020a00 flags 000000b5
> [   62.052487] mmc1:     blksz 512 blocks 1 flags 00000200 tsac 100 ms
> nsac 0
> [   62.059443] mmci-pl18x 58005000.mmc: blksz 0200 blks 0001 flags 00000200
> [   62.066154] mmci-pl18x 58005000.mmc: op 30 arg 10020a00 flags 000000b5
> [   62.072688] mmci-pl18x 58005000.mmc: irq0 (data+cmd) 00000040
> [   62.078382] mmci-pl18x 58005000.mmc: irq0 (data+cmd) 00000100
> 
> The kernel reads back the page to ensure it has completed.
> 
> [   62.084173] mmc1: req done (CMD48): 0: 00000900 00000000 00000000
> 00000000
> [   62.091081] mmc1:     512 bytes transferred: 0
> [   62.095455] mmci-pl18x 58005000.mmc: irq0 (data+cmd) 00000000
> 
> 
> [   62.101381] I/O error, dev mmcblk1, sector 8192 op 0x1:(WRITE) flags
> 0x800 phys_seg 1 prio class 2
> 
> Fail here implies that it did indeed fail right after CMD48.
> 
> I would expect a card that does not hold down busy the entire cache flush time would produce the observed behavior.
> The CMD48 is issued ~100ms after the CMD49, so not nearly close to the timeout of 1s.
> 
> That is just one possibility of course, it would need a full trace to confirm.

What kind of trace ?

> For you easier probably something like:
> 
> diff --git a/drivers/mmc/core/sd.c b/drivers/mmc/core/sd.c
> index 72b664ed90cf..c9d3cae0678d 100644
> --- a/drivers/mmc/core/sd.c
> +++ b/drivers/mmc/core/sd.c
> @@ -1354,8 +1354,11 @@ static int sd_flush_cache(struct mmc_host *host)
>                  goto out;
>          }
> 
> -       if (reg_buf[0] & BIT(0))
> +       if (reg_buf[0] & BIT(0)) {
> +               pr_warn("%s: Cache Flush bit still active after busy phase\n",
> +                       mmc_hostname(host));
>                  err = -ETIMEDOUT;
> +       }

Adrian provided a bit more complete patch, I tested that, see my reply 
there.