On 8/17/21 12:37 AM, Linus Walleij wrote:
On Mon, Aug 16, 2021 at 4:03 PM Yann Gautier <yann.gautier@xxxxxxxxxxx> wrote:
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);
Either we have managed to wire the MMCI driver so that it doesn't
work without HW busy detect anymore, you can easily test this
by doing this:
diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
index 3765e2f4ad98..3a35f65491c8 100644
--- a/drivers/mmc/host/mmci.c
+++ b/drivers/mmc/host/mmci.c
@@ -270,10 +270,10 @@ static struct variant_data variant_stm32_sdmmc = {
.datactrl_any_blocksz = true,
.datactrl_mask_sdio = MCI_DPSM_ST_SDIOEN,
.stm32_idmabsize_mask = GENMASK(12, 5),
- .busy_timeout = true,
- .busy_detect = true,
- .busy_detect_flag = MCI_STM32_BUSYD0,
- .busy_detect_mask = MCI_STM32_BUSYD0ENDMASK,
+ //.busy_timeout = true,
+ //.busy_detect = true,
+ //.busy_detect_flag = MCI_STM32_BUSYD0,
+ //.busy_detect_mask = MCI_STM32_BUSYD0ENDMASK,
.init = sdmmc_variant_init,
};
@@ -297,10 +297,10 @@ static struct variant_data variant_stm32_sdmmcv2 = {
.datactrl_mask_sdio = MCI_DPSM_ST_SDIOEN,
.stm32_idmabsize_mask = GENMASK(16, 5),
.dma_lli = true,
- .busy_timeout = true,
- .busy_detect = true,
- .busy_detect_flag = MCI_STM32_BUSYD0,
- .busy_detect_mask = MCI_STM32_BUSYD0ENDMASK,
+ //.busy_timeout = true,
+ //.busy_detect = true,
+ //.busy_detect_flag = MCI_STM32_BUSYD0,
+ //.busy_detect_mask = MCI_STM32_BUSYD0ENDMASK,
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
