Re: [PATCH] mmc: dw_mmc: dw_mci_get_cd check MMC_CAP_NONREMOVABLE

[zhangfei <zhangfei.gao@linaro.org>]

On 05/06/2015 02:13 PM, Jaehoon Chung wrote:

>>>>>>>>> If you want to check it, use the "broken-cd" and "non-removable" properties into dt-file.
>>>>>>>>> Did you use them?
>>>>>>>>
>>>>>>>> Yes.
>>>>>>>> "broken-cd" can work, but mmc_rescan keeps running.
>>>>>>>> "non-removable" does NOT work, which should be used for emmc.
>>>>>>>> Since dw_mci_get_cd only checks DW_MCI_QUIRK_BROKEN_CARD_DETECTION, so
>>>>>>>> only checks "broken-cd" but not check "non-removable"
>>>>>>>
>>>>>>> Did you use the usage like the below..
>>>>>>>
>>>>>>> dwmmc0 {
>>>>>>>        non-removable;
>>>>>>>        broken-cd;
>>>>>>> };
>>>>>>
>>>>>> non-removable and broken-cd should be used only one.
>>>>>
>>>>> Did you check the code?
>>>>> If non-removable is set, broken-cd should be discarded.
>>>>>
>>>>> I think that the below usage is not "must not".
>>>>
>>>> I understand you meaning, you suggest
>>>>>>> dwmmc0 {
>>>>>>>        non-removable;
>>>>>>>        broken-cd;
>>>>>>> };
>>>>
>>>> drivers/mmc/host/dw_mmc.c checks broken-cd, while mmc_of_parse checks non-removable.
>>>> Yes, it works.
>>>>
>>>> But is it a workaround? and a little tricky.
>>>> It costs me some time to find why non-removable does not work, someone else may meet the same issue.
>>>> It does not align with Documentation/devicetree/bindings/mmc/mmc.txt, which is the guideline to write dts.
>>>> And see drivers/mmc/host/sdhci.c: sdhci_do_get_cd, it also checks both.
>>>
>>> "non-removable" is assumed that card is not removed.
>>> it's not also correct detect scheme. Then it's also able to say the broken card detection scheme.
>>> (if CDETECT register can't use.)
>>>
>>> BROKEN_CARD_DETECTION quirk means that it has unreliable card detection.
>>> When dw-mmc host controller has unreliable card detection scheme, it could be set.
>>> Is this tricky? i don't think so.
>>>
>>> Though non-removable doesn't set, it has to work fine, isn't?
>> If non-removable is not set, broken-cd has to be set.
>> Or set both, but usually we may not consider this at first.
>>
>> When we first want to enable emmc, we naturally use non-removable, according to Documentation/devicetree/bindings/mmc/mmc.txt
>
> Why do you use naturally non-removable? eMMC can be removed at some SoC. (It's assumption.)
> Is it common approach that consider how eMMC can be detected at host controller?
The emmc chip we use is folded on board and can not be removed.
non-removable will make mmc_rescan only executing once, while broken 
will make mmc_rescan polling.

>
>> Only one of the properties in this section should be supplied:
>>    - broken-cd: There is no card detection available; polling must be used.
>>    - cd-gpios: Specify GPIOs for card detection, see gpio binding
>>    - non-removable: non-removable slot (like eMMC); assume always present.
>>
>> But unfortunately we find it does not work and took half day to debug, happen to find broken-cd can work, though mmc_rescan is keeps running for a while, and we treat it as workaround.
>>
>> After some time, another person find broken-cd does not make sense, and debug again about non-removable, and took another half day.
>>
>> It really happens here :(
>
> Sorry for not saving your time.
> Hmm..To prevent your case, it seems better that apply your patch. :)
>
> Will apply at my dw-mmc tree.

Thanks Jaehoon.