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

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

On 05/06/2015 12:21 PM, Jaehoon Chung wrote:
> Hi, Zhangfei.
>
> On 05/06/2015 11:16 AM, zhangfei wrote:
>>
>>
>> On 05/06/2015 09:39 AM, Jaehoon Chung wrote:
>>> Hi,
>>>
>>> On 05/06/2015 10:33 AM, zhangfei wrote:
>>>>
>>>>
>>>> On 05/06/2015 09:26 AM, Jaehoon Chung wrote:
>>>>> Hi,
>>>>>
>>>>> On 05/06/2015 10:14 AM, Zhangfei Gao wrote:
>>>>>> On 6 May 2015 at 08:36, Jaehoon Chung <jh80.chung@samsung.com> wrote:
>>>>>>> Hi, Zhangfei.
>>>>>>>
>>>>>>> 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
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 :(

So is it better support just using non-removable for emmc, aligning with 
mmc.txt.

> And i don't think that dw-mmc controller must use it since sdhci controller used.
> (I have known that sdhci controller is using that.)
>
> Well, If Ulf thinks this is tricky, i will consider this patch.
>

Thanks