Re: [PATCH] mmc: disable tuning when checking card presence

From: Adrian Hunter <adrian.hunter@intel.com>
To: Ulf Hansson <ulf.hansson@linaro.org>,
	Wolfram Sang <wsa+renesas@sang-engineering.com>,
	Ulrich Hecht <uli@fpond.eu>,
	linux-mmc <linux-mmc@vger.kernel.org>,
	Linux-Renesas <linux-renesas-soc@vger.kernel.org>,
	Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
Subject: Re: [PATCH] mmc: disable tuning when checking card presence
Date: Tue, 29 Jun 2021 19:01:19 +0300	[thread overview]
Message-ID: <a7dc7105-1f9b-f394-76c2-4809e078abe6@intel.com> (raw)
In-Reply-To: <CAPDyKFrA--dB1G4wNpo825EQcRDSVmT5mjVoMLK=ojU0k49JAA@mail.gmail.com>

On 29/06/21 5:16 pm, Ulf Hansson wrote:
> On Sat, 26 Jun 2021 at 20:58, Wolfram Sang
> <wsa+renesas@sang-engineering.com> wrote:
>>
>> Hi Adrian, Ulf, everyone,
>>
>>> With the code above, if the host controller knows the card has been
>>> removed, it can return -ENOMEDIUM from ->execute_tuning() to suppress
>>> the message.
>>
>> On second thought, I like the idea with -ENOMEDIUM. Because tuning can
>> still fail for reasons other than a removed card and we want to see an
>> error message then.
>>
>> So, I checked when/how to return -ENOMEDIUM for the SDHI driver but this
>> lead me to more questions. The few driver which return this error code
>> all follow a similar pattern:
>>
>> xxx_request()
>> {
>>         if (host->get_cd == 1)
>>                 submit_mrq
>>         else
>>                 cmd->error = -ENOMEDIUM
>>                 mmc_request_done()
>> }
>>
>> So, my first question would be if we can't apply this pattern in the
>> core before calling the .request callback? A lot of drivers are not
>> implementing this pattern although it seems useful. Is it required?
> 
> It's required for some sdhci variants, because issuing a command when
> a card has been removed can hang (or completes after quite a long
> timeout, I don't recall, Adrian?).

If the host supports SDHCI's own card detect then after the card is
removed requests will not start nor error, until the 10 second
software timeout.  The logic to check SDHCI card present predated
the use of GPIO card-detect but the same approach was copied, although
it should be possible to do without it.

> 
>> Recommended? Nice to have? However, I could imagine an answer for moving
>> it into the core is "no, that should be checked atomically"? E.g. sdhci
>> does it, but atmel-mci and s3cmci do not. If I just look at moving the
>> card detection call into the core, I don't really see the reason for
>> atomic. Am I missing something?
> 
> My main concern would be performance/latency, as we would introduce
> some overhead for every single request. So, no, we don't want this in
> the core in my opinion.

I agree.

I would get rid of it from SDHCI but it looked like we might need to
rely on ->card_event() which won't work because it claims the host,
which will wait for the block driver to release it, which will wait
for the request anyway. That was as far as I got, thinking about it.