On 8/17/21 12:37 AM, Linus Walleij wrote:

On Mon, Aug 16, 2021 at 4:03 PM Yann Gautier [off-list ref] wrote:

quoted

I was just testing your patch on top of mmc/next.
Whereas mmc/next is fine, with your patch I fail to pass MMC test 5
(Multi-block write).
I've got this error on STM32MP157C-EV1 board:
[  108.956218] mmc0: Starting tests of card mmc0:aaaa...
[  108.959862] mmc0: Test case 5. Multi-block write...
[  108.995615] mmc0: Warning: Host did not wait for busy state to end.
[  109.000483] mmc0: Result: ERROR (-110)
Then nothing more happens.

The test was done on an SD-card Sandisk Extreme Pro SDXC UHS-I mark 3,
in DDR50 mode.

I'll try to add more traces to see what happens.

Hi Linus

What I think happens is:
- You are using the MMCI driver (correct?)

Yes

- My patch augments the driver to not use busydetect until we have
   determined that the card can do it (after reading extcsd etc)
- Before this patch, the MMCI would unconditionally use HW
   busy detect on any card.

I finally found the problem.
The assignment of host->card is done at the end of mmc_sd_init_card().
But mmci_set_max_busy_timeout() is called in mmc_sd_init_card() before 
that, and card is then NULL at that time. This let me a 
mmc->max_busy_timeout = 0. And this value is no more updated.
mmci_start_command() will then have a unexpected behavior with that 0 value.

Maybe we should not use mmci_use_busy_detect() in 
mmci_set_max_busy_timeout()?

If I use this patch on top of yours (reverting the 
mmci_set_max_busy_timeout() change), all the mmc tests pass on the 
SD-card I was testing:

@@ -1741,11 +1741,11 @@ static void mmci_request(struct mmc_host *mmc, 

struct mmc_request *mrq)
  static void mmci_set_max_busy_timeout(struct mmc_host *mmc)
  {
  	struct mmci_host *host = mmc_priv(mmc);
  	u32 max_busy_timeout = 0;

-	if (!mmci_use_busy_detect(host))
+	if (!host->variant->busy_detect)
  		return;

  	if (host->variant->busy_timeout && mmc->actual_clock)
  		max_busy_timeout = ~0UL / (mmc->actual_clock / MSEC_PER_SEC);

This was working, but disabling HW busy detection is not really what we 
want.

         .init                   = sdmmc_variant_init,

Or else there is a card that cannot work without busy detect which
I find unlikely.

Yours,
Linus Walleij >

I have the same kind of issues with the eMMC on the STM32MP157C-EV1 
board. But here it fails at boot when trying to enable HPI, in mmc_switch().


I then updated the patch like this:

@@ -357,7 +357,7 @@ static bool mmci_use_busy_detect(struct mmci_host *host)

         /* We don't allow this until we know that the card can handle it */
         if (!card)
-               return false;
+               return true;

And it then works for all my use-cases, but I suppose that's not what 
you wanted to do.

So I guess we need to have the mmc_card structure, to determine if we 
have the quirk, but not from the mmc_host. Through some new callback?


Best regards,
Yann