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

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

 



2012/2/2 Jaehoon Chung <jh80.chung@xxxxxxxxxxx>:
> On 02/02/2012 12:36 PM, Namjae Jeon wrote:
>
>> 2012/2/2 Jaehoon Chung <jh80.chung@xxxxxxxxxxx>:
>>> On 02/02/2012 09:34 AM, Namjae Jeon wrote:
>>>
>>>> 2012/2/1 Ulf Hansson <ulf.hansson@xxxxxxxxxxxxxx>:
>>>>> 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@xxxxxxxxxxxxxx>
>>>>> ---
>>>>>
>>>>> 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 ?
>
>>
>> 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@xxxxxxxxxxxxxxx
>>>>> 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@xxxxxxxxxxxxxxx
>>>> 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@xxxxxxxxxxxxxxx
>>> 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@xxxxxxxxxxxxxxx
>> 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@xxxxxxxxxxxxxxx
> 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@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