From: Yann Gautier <yann.gautier@foss.st.com>
To: Linus Walleij <linus.walleij@linaro.org>
Cc: linux-mmc <linux-mmc@vger.kernel.org>,
Ulf Hansson <ulf.hansson@linaro.org>,
stable <stable@vger.kernel.org>, <phone-devel@vger.kernel.org>,
Ludovic Barre <ludovic.barre@st.com>,
"Stephan Gerhold" <stephan@gerhold.net>,
Stefan Hansson <newbyte@disroot.org>
Subject: Re: [PATCH v3] mmc: core: Add a card quirk for non-hw busy detection
Date: Tue, 24 Aug 2021 17:50:36 +0200 [thread overview]
Message-ID: <1c458c1e-8730-ae74-de93-45d2d634beb4@foss.st.com> (raw)
In-Reply-To: <CACRpkdY2GnqNYqPPctqa_t5ax1SDo7nEc3a1jSncF8N-V-Da-g@mail.gmail.com>
On 8/17/21 12:37 AM, Linus Walleij wrote:
> On Mon, Aug 16, 2021 at 4:03 PM Yann Gautier <yann.gautier@foss.st.com> 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
next prev parent reply other threads:[~2021-08-24 15:50 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-07-20 14:41 [PATCH v3] mmc: core: Add a card quirk for non-hw busy detection Linus Walleij
2021-08-16 13:31 ` Ulf Hansson
2021-08-16 14:03 ` Yann Gautier
2021-08-16 22:37 ` Linus Walleij
2021-08-24 15:50 ` Yann Gautier [this message]
2022-02-13 23:56 ` Linus Walleij
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1c458c1e-8730-ae74-de93-45d2d634beb4@foss.st.com \
--to=yann.gautier@foss.st.com \
--cc=linus.walleij@linaro.org \
--cc=linux-mmc@vger.kernel.org \
--cc=ludovic.barre@st.com \
--cc=newbyte@disroot.org \
--cc=phone-devel@vger.kernel.org \
--cc=stable@vger.kernel.org \
--cc=stephan@gerhold.net \
--cc=ulf.hansson@linaro.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).