diff mbox

mmc: dw_mmc: dw_mci_get_cd check MMC_CAP_NONREMOVABLE

Message ID 1430816089-8857-1-git-send-email-zhangfei.gao@linaro.org (mailing list archive)
State New, archived
Headers show

Commit Message

Zhangfei Gao May 5, 2015, 8:54 a.m. UTC
When non-removable is used for emmc,  MMC_CAP_NONREMOVABLE should
also be checked, otherwise detection fail since present=0

Signed-off-by: Zhangfei Gao <zhangfei.gao@linaro.org>
---
 drivers/mmc/host/dw_mmc.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Comments

Jaehoon Chung May 6, 2015, 12:36 a.m. UTC | #1
Hi, Zhangfei.

If you want to check it, use the "broken-cd" and "non-removable" properties into dt-file.
Did you use them?

Best Regards,
Jaehoon Chung

On 05/05/2015 05:54 PM, Zhangfei Gao wrote:
> When non-removable is used for emmc,  MMC_CAP_NONREMOVABLE should
> also be checked, otherwise detection fail since present=0
> 
> Signed-off-by: Zhangfei Gao <zhangfei.gao@linaro.org>
> ---
>  drivers/mmc/host/dw_mmc.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
> index 488a8af..5d327e4 100644
> --- a/drivers/mmc/host/dw_mmc.c
> +++ b/drivers/mmc/host/dw_mmc.c
> @@ -1308,7 +1308,8 @@ static int dw_mci_get_cd(struct mmc_host *mmc)
>  	int gpio_cd = mmc_gpio_get_cd(mmc);
>  
>  	/* Use platform get_cd function, else try onboard card detect */
> -	if (brd->quirks & DW_MCI_QUIRK_BROKEN_CARD_DETECTION)
> +	if ((brd->quirks & DW_MCI_QUIRK_BROKEN_CARD_DETECTION) ||
> +	    (mmc->caps & MMC_CAP_NONREMOVABLE))
>  		present = 1;
>  	else if (!IS_ERR_VALUE(gpio_cd))
>  		present = gpio_cd;
> 

--
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
Zhangfei Gao May 6, 2015, 1:14 a.m. UTC | #2
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"

Thanks
--
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
Jaehoon Chung May 6, 2015, 1:26 a.m. UTC | #3
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;
};

Best Regards,
Jaehoon Chung

> 
> Thanks
> 

--
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
Zhangfei Gao May 6, 2015, 1:33 a.m. UTC | #4
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.

Documentation/devicetree/bindings/mmc/mmc.txt
Card detection:
If no property below is supplied, host native card detect is used.
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.

work
  dwmmc0 {
  	broken-cd;
  };

NOT work
  dwmmc0 {
  	non-removable;
  };

Thanks
--
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
Jaehoon Chung May 6, 2015, 1:39 a.m. UTC | #5
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".

Best Regards,
Jaehoon Chung

> 
> Documentation/devicetree/bindings/mmc/mmc.txt
> Card detection:
> If no property below is supplied, host native card detect is used.
> 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.
> 
> work
>  dwmmc0 {
>      broken-cd;
>  };
> 
> NOT work
>  dwmmc0 {
>      non-removable;
>  };
> 
> Thanks
> 

--
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
Zhangfei Gao May 6, 2015, 1:53 a.m. UTC | #6
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".

Sorry, not understand.
Just want to use non-removable for emmc, and find it does not work.
Use broken-cd for emmc is not correct.

>
> Best Regards,
> Jaehoon Chung
>
>>
>> Documentation/devicetree/bindings/mmc/mmc.txt
>> Card detection:
>> If no property below is supplied, host native card detect is used.
>> 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.
>>
>> work
>>   dwmmc0 {
>>       broken-cd;
>>   };
>>
>> NOT work
>>   dwmmc0 {
>>       non-removable;
>>   };
>>
>> Thanks
>>
>
--
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
Zhangfei Gao May 6, 2015, 2:16 a.m. UTC | #7
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.

Thanks


>
> Best Regards,
> Jaehoon Chung
>
>>
>> Documentation/devicetree/bindings/mmc/mmc.txt
>> Card detection:
>> If no property below is supplied, host native card detect is used.
>> 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.
>>
>> work
>>   dwmmc0 {
>>       broken-cd;
>>   };
>>
>> NOT work
>>   dwmmc0 {
>>       non-removable;
>>   };
>>
>> Thanks
>>
>
--
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
Jaehoon Chung May 6, 2015, 4:21 a.m. UTC | #8
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?
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.

Best Regards,
Jaehoon Chung
> 
> Thanks
> 
> 
>>
>> Best Regards,
>> Jaehoon Chung
>>
>>>
>>> Documentation/devicetree/bindings/mmc/mmc.txt
>>> Card detection:
>>> If no property below is supplied, host native card detect is used.
>>> 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.
>>>
>>> work
>>>   dwmmc0 {
>>>       broken-cd;
>>>   };
>>>
>>> NOT work
>>>   dwmmc0 {
>>>       non-removable;
>>>   };
>>>
>>> Thanks
>>>
>>
> 

--
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
Zhangfei Gao May 6, 2015, 5:30 a.m. UTC | #9
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
--
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
Jaehoon Chung May 6, 2015, 6:13 a.m. UTC | #10
On 05/06/2015 02:30 PM, zhangfei wrote:
> 
> 
> 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

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?

> 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!

Best Regards,
Jaehoon Chung

> 
> 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
> 

--
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
Zhangfei Gao May 6, 2015, 6:30 a.m. UTC | #11
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.
--
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
Arnd Bergmann May 6, 2015, 7:23 a.m. UTC | #12
On Wednesday 06 May 2015 14:30:22 zhangfei wrote:
> On 05/06/2015 02:13 PM, Jaehoon Chung wrote:
> >>>>>>>>> If you want to check it, use the "broken-cd" and "non-removable" 
> >>>>
> >>>> 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.

I agree. The current behavior of dw_mmc is different from what the
documentation says it is, and different from what the other host
controllers do.

Your patch looks correct to me, as it will make the dw_mmc driver behave
like the others but keep existing files working when they rely on the
nonstandard behavior.

	Arnd

--
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
diff mbox

Patch

diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index 488a8af..5d327e4 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -1308,7 +1308,8 @@  static int dw_mci_get_cd(struct mmc_host *mmc)
 	int gpio_cd = mmc_gpio_get_cd(mmc);
 
 	/* Use platform get_cd function, else try onboard card detect */
-	if (brd->quirks & DW_MCI_QUIRK_BROKEN_CARD_DETECTION)
+	if ((brd->quirks & DW_MCI_QUIRK_BROKEN_CARD_DETECTION) ||
+	    (mmc->caps & MMC_CAP_NONREMOVABLE))
 		present = 1;
 	else if (!IS_ERR_VALUE(gpio_cd))
 		present = gpio_cd;