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

From: Adrian Hunter <adrian.hunter@intel.com>
To: Wolfram Sang <wsa+renesas@sang-engineering.com>,
	Ulrich Hecht <uli@fpond.eu>, Ulf Hansson <ulf.hansson@linaro.org>,
	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: Mon, 21 Jun 2021 11:26:35 +0300	[thread overview]
Message-ID: <b5062770-ba5c-32d5-15f0-505a09bb4a2e@intel.com> (raw)
In-Reply-To: <YNBJq7Lrtlc/qExN@ninjato>

On 21/06/21 11:11 am, Wolfram Sang wrote:
> On 21/06/21 10:54 am, Adrian Hunter wrote:
>> On 21/06/21 10:32 am, Ulrich Hecht wrote:
>>>
>>>> On 06/21/2021 9:15 AM Adrian Hunter <adrian.hunter@intel.com> wrote:
>>>> Can we clarify, is the only problem that the error message is confusing?
>>>
>>> AFAICT there are no ill effects of the retune failing apart from the error message.
>>>
>> 
>> So maybe the simplest thing to do is just amend the message:
>> e.g.
>> 
>> diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
>> index 4e52eb14198a..5cbf05e331c4 100644
>> --- a/drivers/mmc/core/core.c
>> +++ b/drivers/mmc/core/core.c
>> @@ -936,13 +936,22 @@ int mmc_execute_tuning(struct mmc_card *card)
>>  		opcode = MMC_SEND_TUNING_BLOCK;
>>  
>>  	err = host->ops->execute_tuning(host, opcode);
>> -
>>  	if (err)
>> -		pr_err("%s: tuning execution failed: %d\n",
>> -			mmc_hostname(host), err);
>> -	else
>> -		mmc_retune_enable(host);
>> +		goto out_err;
>> +
>> +	mmc_retune_enable(host);
>>  
>> +	return 0;
>> +
>> +out_err:
>> +	if (mmc_card_is_removable(host)) {
>> +		if (err != -ENOMEDIUM)
>> +			pr_err("%s: tuning execution failed: %d (this is normal if card removed)\n",
>> +			       mmc_hostname(host), err);
> 
> Hmm, an error message saying "this is normal" doesn't look like a good
> option to me. Can't we surpress the message somehow or even avoid tuning
> somehow if the card is removed? Sorry, I can't look this up myself right
> now, working on another task today.

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.

Otherwise, you need to introduce a new card state or flag to indicate
that the card may not be present, and use that to suppress the message.

> >> +	} else {
>> +		pr_err("%s: tuning execution failed: %d\n",
>> +		       mmc_hostname(host), err);
>> +	}
>>  	return err;
>>  }
>>  
>> 
>> > 