Re: [PATCH] mmc: core: Force a "detect" to handle non-properly removed cards

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

Adrian Hunter wrote:
> On 13/01/12 15:14, Ulf Hansson wrote:
>>>>>> In principles this means the following sequence:
>>>>>>
>>>>>> We will rely on that the get_cd function will return 0 (indicating card is
>>>>>> removed) when the card is "slowly" removed at the point when the rescan
>>>>>> function is calling it through the bus_ops->detect -->
>>>>>> _mmc_detect_card_removed function.
>>>>>>
>>>>>> This then becomes a race, meaning that the rescan function must be
>>>>>> executing
>>>>>> at the same time the get_cd function will returns 0. Otherwise the rescan
>>>>>> function will not remove the card.
>>>>>>
>>>>>> Thus my conclusion is that "[PATCH 2/4 v4] MMC/SD: Add callback
>>>>>> function to
>>>>>> detect card" will likely improve behavior but is not the safe solution to
>>>>>> handle "slowly" removed cards.
>>>>>>
>>>>>> Again, to be sure, we must let the mmc_detect_card_remove function
>>>>>> trigger a
>>>>>> rescan when _mmc_detect_card_removed has detected that the card is
>>>>>> removed.
>>>>>> This should be safe in all circumstances.
>>>>> sdhci has no problem because it does this:
>>>>>
>>>>>     - the host controller debounces the card detect line
>>>>>     - the host controller records whether or not the card is present
>>>>>     - the sdhci driver prevents (errors out) requests when the card is
>>>>>     not present
>>>> Debouncing will just be a way of triggering the problem more seldom. Or in
>>>> worst case, state the card has been removed even if it has not.
>>> If a delay is used with mmc_detect_change, debouncing is not necessary.
>>>
>>>> Just because you get a GPIO irq on the detect line does not mean the card is
>>>> removed, debouncing or not. I consider this as pure mechanical switch which
>>>> likely has glitches and I don't see that we should trust it fully. We only
>>>> want to trigger a detect work, which is exactly what is done in the patch
>>>> from Guennadi Liakhovetski "mmc: add a generic GPIO card-detect helper".
>>> The original problem was "slow card removal".  "Unreliable card detect"
>>> is a separate problem.  Currently there is polling (MMC_CAP_NEEDS_POLL)
>>> for that.  Alternatively there is MMC_CAP2_RESCAN_ON_ERROR as we have
>>> discussed.
>> I do not understand why you mention "Unreliable card detect"? That has
>> nothing to do with this patch.
>>
>> So to conclude the discussion, do you believe that this patch is acceptable
>> as long as we add a CAPS2 option "MMC_CAP2_RESCAN_ON_ERROR", which if not
>> set will prevent the detect work from being scheduled from
>> mmc_detect_card_removed?
> 
> Yes
> 

OK, but.. :-)

I were just about to update the patch according to your recommendation 
when I realized the following:

Once _mmc_detect_card_removed has set the card state as removed 
("mmc_card_set_removed"), the card will never be accessible for I/O 
requests any more, all I/O will "silently" be thrown away in the block 
layer. This leads to that there should definitely be no reason for _not_ 
letting a scheduled rescan remove the card as soon as possible. In other 
words the CAP2 should not be needed.

Did I miss something?

Agree?


BR
Ulf Hansson