Re: [PATCH V5 02/15] mmc: core: Enable / disable re-tuning

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 16/04/15 15:00, Ulf Hansson wrote:
> On 16 April 2015 at 11:26, Adrian Hunter <adrian.hunter@xxxxxxxxx> wrote:
>> On 16/04/15 11:57, Ulf Hansson wrote:
>>> On 14 April 2015 at 15:12, Adrian Hunter <adrian.hunter@xxxxxxxxx> wrote:
>>>> Enable re-tuning when tuning is executed and
>>>> disable re-tuning when card is no longer initialized.
>>>>
>>>> Signed-off-by: Adrian Hunter <adrian.hunter@xxxxxxxxx>
>>>> ---
>>>>  drivers/mmc/core/core.c | 4 ++++
>>>>  1 file changed, 4 insertions(+)
>>>>
>>>> diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
>>>> index c296bc0..dd43f9b 100644
>>>> --- a/drivers/mmc/core/core.c
>>>> +++ b/drivers/mmc/core/core.c
>>>> @@ -1109,6 +1109,8 @@ int mmc_execute_tuning(struct mmc_card *card)
>>>>
>>>>         if (err)
>>>>                 pr_err("%s: tuning execution failed\n", mmc_hostname(host));
>>>> +       else
>>>> +               mmc_retune_enable(host);
>>>>
>>>>         return err;
>>>>  }
>>>> @@ -1140,6 +1142,8 @@ void mmc_set_bus_width(struct mmc_host *host, unsigned int width)
>>>>   */
>>>>  void mmc_set_initial_state(struct mmc_host *host)
>>>>  {
>>>> +       mmc_retune_disable(host);
>>>> +
>>>>         if (mmc_host_is_spi(host))
>>>>                 host->ios.chip_select = MMC_CS_HIGH;
>>>>         else
>>>> --
>>>> 1.9.1
>>>
>>> I don't think you have fully considered these cases for the mmc/sd/sdio cards.
>>
>> Thanks for look at this, but I don't see a problem - see below.
>>
>>>
>>> 1) Card removal/detect (hold/release?)
>>
>> Re-tuning will be done if needed before detect (because it is done before a
>> request), which is necessary because detect can fail if tuning is needed.
>>
>> Re-tuning is done before a request. Requests aren't started if the card has
>> been removed i.e. mmc_start_request() calls mmc_card_removed()
>>
>> If tuning is executed while a card is being removed, then the tuning will
>> fail and the request will be errored out.
> 
> So you are saying that we instead of relying on the CMD13 (for SD/MMC)
> to check whether the card is still alive, it's fine to trigger a
> re-tuning instead.
> 
> I don't think that is an effective way to remove a card.
> 
> Moreover, I find it quite unreasonable to say the check for an alive
> card, would fail because of that a re-tuning is needed. That would in
> principle mean that we never should be able to hold re-tune for any
> commands sequences.
> 
>>
>>> 2) system PM (disable?)
>>
>> System pm does mmc_power_off() which calls mmc_set_initial_state()
> 
> At System PM other commands will be sent to put the card into sleep
> state. For example CMD5. These commands are invoked prior
> mmc_power_off() is called.

You can't do *any* commands after CMD5 except CMD0 or another CMD5 to wake up.

So if you want to wake-up from sleep, then you need to hold re-tuning, but
that is not what is happening at the moment.

> 
> In the SDIO case, mmc_power_off() might not even be called at all,
> since SDIO func drivers might have enabled MMC_PM_KEEP_POWER.

If the card is not going to be re-initialized then disabling is not necessary.

> 
>>
>>> 3) runtime PM (disable?)
>>
>> If the card powers off then mmc_set_initial_state() will called.
> 
> Again that's too late, since for example the CMD5 might have been sent
> before this.

CMD5 is only used by _mmc_suspend()

Yes if it were used elsewhere then re-tuning would have to be held, which is
why I added a comment before mmc_sleep()

	/* If necessary, callers must hold re-tuning */
	static int mmc_sleep(struct mmc_host *host)

> 
>>
>> For anything else the card might be doing, the mmc_retune_hold() /
>> mmc_retune_release() functions are used.
>>
>>> 4) reset (?)
>>
>> Reset goes through mmc_set_initial_state()
> 
> In the mmc case, CMD13 is sent prior that. Shouldn't we hold re-tune
> during that period?

If reset happens, then the next command will fail, whether it is re-tuning
or CMD13, so no different.

If reset doesn't happen, then it is no different to normal operations.

--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux USB Devel]     [Linux Media]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux