Re: [PATCH V2] mmc: core: Detect card removal on I/O error

[Ulf Hansson <ulf.hansson@stericsson.com>]

Namjae Jeon wrote:
> 2012/2/2 Jaehoon Chung <jh80.chung@samsung.com>:
>> On 02/02/2012 12:36 PM, Namjae Jeon wrote:
>>
>>> 2012/2/2 Jaehoon Chung <jh80.chung@samsung.com>:
>>>> On 02/02/2012 09:34 AM, Namjae Jeon wrote:
>>>>
>>>>> 2012/2/1 Ulf Hansson <ulf.hansson@stericsson.com>:
>>>>>> To prevent I/O as soon as possible at card removal, a new
>>>>>> detect work is re-scheduled without a delay to let a rescan
>>>>>> remove the card device as soon a possible.
>>>>>>
>>>>>> Additionally, MMC_CAP2_DETECT_ON_ERR can now be used to handle
>>>>>> "slowly" removed cards that a scheduled detect work did not
>>>>>> detect as removed. To prevent further I/O requests for these
>>>>>> lingering removed cards, check if card has been removed and then
>>>>>> schedule a detect work to properly remove it.
>>>>>>
>>>>>> Signed-off-by: Ulf Hansson <ulf.hansson@stericsson.com>
>>>>>> ---
>>>>>>
>>>>>> Changes in v2:
>>>>>>        - Updated according to review comments.
>>>>>>        - Merging two patches for this feature into one.
>>>>>>
>>>>>> ---
>>>>>>  drivers/mmc/core/core.c  |   19 +++++++++++++++++--
>>>>>>  include/linux/mmc/host.h |    1 +
>>>>>>  2 files changed, 18 insertions(+), 2 deletions(-)
>>>>>>
>>>>>> diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
>>>>>> index bec0bf2..26d3a66 100644
>>>>>> --- a/drivers/mmc/core/core.c
>>>>>> +++ b/drivers/mmc/core/core.c
>>>>>> @@ -2077,18 +2077,33 @@ int _mmc_detect_card_removed(struct mmc_host *host)
>>>>>>  int mmc_detect_card_removed(struct mmc_host *host)
>>>>>>  {
>>>>>>        struct mmc_card *card = host->card;
>>>>>> +       int ret;
>>>>>>
>>>>>>        WARN_ON(!host->claimed);
>>>>>>        /*
>>>>>>         * The card will be considered unchanged unless we have been asked to
>>>>>>         * detect a change or host requires polling to provide card detection.
>>>>>>         */
>>>>>> -       if (card && !host->detect_change && !(host->caps & MMC_CAP_NEEDS_POLL))
>>>>>> +       if (card && !host->detect_change && !(host->caps & MMC_CAP_NEEDS_POLL)
>>>>>> +           && !(host->caps2 & MMC_CAP2_DETECT_ON_ERR))
>>>>>>                return mmc_card_removed(card);
>>>>>>
>>>>>>        host->detect_change = 0;
>>>>>>
>>>>>> -       return _mmc_detect_card_removed(host);
>>>>>> +       ret = mmc_card_removed(card);
>>>>> Hi. Ulf.
>>>>> 1. Is there no case that card is null ?
>>>>> 2. and Eventually mmc_card_removed is once called in any case. can we
>>>>> change it like this ?
>>>>>
>>>>> if (card)
>>>>>    ret = mmc_card_removed(card);
>>>>>
>>>>>    if(!host->detect_change && !(host->caps & MMC_CAP_NEEDS_POLL)
>>>>>            && !(host->caps2 & MMC_CAP2_DETECT_ON_ERR))
>>>>>                   return ret;
>>>> Hi Mr.Jeon
>>>> If card is NULL, maybe this code didn't run.
>>>> Only called this function is called in block.c (for my understanding)
>>>> before called mmc_detect_card_removed(card->host), check "if (mmc_card_removed(card)".
>>>> I think that need not to check.
>>> Hi. Chung.
>>> I found directly using this function in other file except block.c
>>> ./core/sdio.c:  err = _mmc_detect_card_removed(host);
>>> ./core/core.c:  return _mmc_detect_card_removed(host);
>>> ./core/sd.c:    err = _mmc_detect_card_removed(host);
>>> ./core/mmc.c:   err = _mmc_detect_card_removed(host);
>> Sorry..this patch is code in mmc_detect_card_removed(), isn't?
>> what is your comment related with _mmc_detect_card_removed()?
> I am confused. sorry, you're right.
> And Do you agree like below changing patch ?

Good comment. I will update the patch according to your suggestion and 
send a v3 patch.

>>> But I know to already check null of card(BUG_ON(!host->card)) before
>>> using _mmc_detect_card_removed.
>>> So don't need to check card is null now.
>>> I think that it is better if patch is modified like the below.
>>> -------------------------------------------------------------------------------------
>>> ret = mmc_card_removed(card);
>>>
>>>   if(!host->detect_change && !(host->caps & MMC_CAP_NEEDS_POLL)
>>>           && !(host->caps2 & MMC_CAP2_DETECT_ON_ERR))
>>>                  return ret;
>>> -----------------------------------------------------------------------------------------
>>> Thanks.
>>>> Best Regards,
>>>> Jaehoon Chung
>>>>
>>>>>> +       if (!ret) {
>>>>>> +               ret = _mmc_detect_card_removed(host);
>>>>>> +               if (ret) {
>>>>>> +                       /*
>>>>>> +                        * Schedule a detect work as soon as possible to let a
>>>>>> +                        * rescan handle the card removal.
>>>>>> +                        */
>>>>>> +                       cancel_delayed_work(&host->detect);
>>>>>> +                       mmc_detect_change(host, 0);
>>>>>> +               }
>>>>>> +       }
>>>>>> +
>>>>>> +       return ret;
>>>>>>  }
>>>>>>  EXPORT_SYMBOL(mmc_detect_card_removed);
>>>>>>
>>>>>> diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
>>>>>> index dd13e05..368a2b9 100644
>>>>>> --- a/include/linux/mmc/host.h
>>>>>> +++ b/include/linux/mmc/host.h
>>>>>> @@ -257,6 +257,7 @@ struct mmc_host {
>>>>>>  #define MMC_CAP2_HS200_1_2V_SDR        (1 << 6)        /* can support */
>>>>>>  #define MMC_CAP2_HS200         (MMC_CAP2_HS200_1_8V_SDR | \
>>>>>>                                 MMC_CAP2_HS200_1_2V_SDR)
>>>>>> +#define MMC_CAP2_DETECT_ON_ERR (1 << 7)        /* On I/O err check card removal */
>>>>>>
>>>>>>        mmc_pm_flag_t           pm_caps;        /* supported pm features */
>>>>>>        unsigned int        power_notify_type;
>>>>>> --
>>>>>> 1.7.5.4
>>>>>>
>>>>>> --
>>>>>> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
>>>>>> the body of a message to majordomo@vger.kernel.org
>>>>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>>>> --
>>>>> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
>>>>> the body of a message to majordomo@vger.kernel.org
>>>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>>>>
>>>>
>>>> --
>>>> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
>>>> the body of a message to majordomo@vger.kernel.org
>>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
>>> the body of a message to majordomo@vger.kernel.org
>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>>
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Br
Ulf Hansson