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

[Marek Vasut <marex@denx.de>]

On 6/4/23 18:30, Adrian Hunter wrote:

[...]

>>>>> diff --git a/drivers/mmc/core/sd.c b/drivers/mmc/core/sd.c
>>>>> index 72b664ed90cf..9c3123867a99 100644
>>>>> --- a/drivers/mmc/core/sd.c
>>>>> +++ b/drivers/mmc/core/sd.c
>>>>> @@ -1313,6 +1313,8 @@ static int sd_flush_cache(struct mmc_host *host)
>>>>>     {
>>>>>         struct mmc_card *card = host->card;
>>>>>         u8 *reg_buf, fno, page;
>>>>> +    unsigned long timeout;
>>>>> +    bool expired;
>>>>>         u16 offset;
>>>>>         int err;
>>>>>     @@ -1338,11 +1340,15 @@ static int sd_flush_cache(struct mmc_host *host)
>>>>>             goto out;
>>>>>         }
>>>>>     +    timeout = jiffies + msecs_to_jiffies(SD_WRITE_EXTR_SINGLE_TIMEOUT_MS) + 1;
>>>>> +again:
>>>>>         err = mmc_poll_for_busy(card, SD_WRITE_EXTR_SINGLE_TIMEOUT_MS, false,
>>>>>                     MMC_BUSY_EXTR_SINGLE);
>>>>>         if (err)
>>>>>             goto out;
>>>>>     +    expired = time_after(jiffies, timeout);
>>>>> +
>>>>>         /*
>>>>>          * Read the Flush Cache bit. The card shall reset it, to confirm that
>>>>>          * it's has completed the flushing of the cache.
>>>>> @@ -1354,8 +1360,12 @@ static int sd_flush_cache(struct mmc_host *host)
>>>>>             goto out;
>>>>>         }
>>>>>     -    if (reg_buf[0] & BIT(0))
>>>>> -        err = -ETIMEDOUT;
>>>>> +    if (reg_buf[0] & BIT(0)) {
>>>>
>>>> I am getting here, multiple times, with expired=0 .
>>>
>>> So either the host controller's busy detection does not work, or the
>>> card is not indicating busy by pulling down DAT0.
>>>
>>> Can you try to figure out which it is?
>>
>> The byte 261 bit 0 is never cleared, I had this looping for an hour and the 'Flush Cache' bit just never got cleared. The SD spec 6.00 and 9.00 both indicate the bit should be cleared by the card once cache flush is completed.
>>
>> I tried three different controllers now -- STM32MP15xx ARM MMCI, i.MX6Q uSDHC, laptop rtsx_pci_sdmmc , they all fail.
>>
>> I tried to find another card which also has cache, I cannot find any other card, all the rest report no cache. The kingston card SSR (see the 2ff in 6th field, the last f bit 2 is cache supported indication, SSR bit 330):
>>
>> 00000000:08000000:04009000:011b391e:00080000:0002ff00:03000000:00000000:00000000:00000000:00000000:00000000:00000000:00000000:00000000:00000000:
>>
>> So either this card is weird, or the cards with cache are so rare that nobody noticed the problem yet.
> 
> The patch set cover letter says it was tested with 64GB Sandisk Extreme PRO UHS-I A2 card
> 
> 	https://lore.kernel.org/linux-mmc/20210506145829.198823-1-ulf.hansson@linaro.org/

I got that one now, tested it, the cache bit is being cleared correctly. 
I also tested a few more cards and dumped their SSR too:

Kingston Canvas Go! Plus:
80000000:08000000:04009000:011b391e:00080000:0002ff00:03000000:00000000:00000000:00000000:00000000:00000000:00000000:00000000:00000000:00000000:
Flush never finishes

Sandisk Extreme PRO A2 64GiB:
80000000:08000000:04009000:0f05391e:00080000:0002fc00:03000000:00000000:00000000:00000000:00000000:00000000:00000000:00000000:00000000:00000000:
mmc0: flushing cache took 5 ms, 1 iterations, error 0

Goodram IRDM V30 A2 64GiB:
80000000:08000000:0400a001:00fd3a1e:00080000:00023c00:00000000:00000000:00000000:00000000:00000000:00000000:00000000:00000000:00000000:00000000:
mmc0: flushing cache took 5 ms, 1 iterations, error 0

Samsung Pro Plus 512GiB V30 A2 (ext reg general info is all zeroes, 
cache not enabled):
80000000:08000000:04009000:0811391e:00080000:0002fc00:00000000:00000000:00000000:00000000:00000000:00000000:00000000:00000000:00000000:00000000:

> I ordered a Kingston Canvas Go Plus card as you described but it won't arrive for a week.

I'm really interested in what you would find with that one.

Maybe the card I have here is defective, although I would expect not 
just the cache functionality to fail in such a case.

Is there anyone at kingston we could ask about the cache specifics on 
that card ?

> Currently I am thinking we could do a cache flush after enabling the cache, just
> to see if it works.  If not, then disable the cache.
> 
> It would also be interesting to read back the extended register to see if the
> enable bit has actually been set.

Both offset 260 and 261 read back as 0x01 during the endless flush.