All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] mmc: dw_mmc: dw_mci_get_cd check MMC_CAP_NONREMOVABLE
@ 2015-05-05  8:54 Zhangfei Gao
  2015-05-06  0:36 ` Jaehoon Chung
  0 siblings, 1 reply; 13+ messages in thread
From: Zhangfei Gao @ 2015-05-05  8:54 UTC (permalink / raw)
  To: Jaehoon Chung; +Cc: linux-mmc, Zhangfei Gao

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


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH] mmc: dw_mmc: dw_mci_get_cd check MMC_CAP_NONREMOVABLE
  2015-05-05  8:54 [PATCH] mmc: dw_mmc: dw_mci_get_cd check MMC_CAP_NONREMOVABLE Zhangfei Gao
@ 2015-05-06  0:36 ` Jaehoon Chung
  2015-05-06  1:14   ` Zhangfei Gao
  0 siblings, 1 reply; 13+ messages in thread
From: Jaehoon Chung @ 2015-05-06  0:36 UTC (permalink / raw)
  To: Zhangfei Gao; +Cc: linux-mmc

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


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] mmc: dw_mmc: dw_mci_get_cd check MMC_CAP_NONREMOVABLE
  2015-05-06  0:36 ` Jaehoon Chung
@ 2015-05-06  1:14   ` Zhangfei Gao
  2015-05-06  1:26     ` Jaehoon Chung
  0 siblings, 1 reply; 13+ messages in thread
From: Zhangfei Gao @ 2015-05-06  1:14 UTC (permalink / raw)
  To: Jaehoon Chung; +Cc: linux-mmc

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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] mmc: dw_mmc: dw_mci_get_cd check MMC_CAP_NONREMOVABLE
  2015-05-06  1:14   ` Zhangfei Gao
@ 2015-05-06  1:26     ` Jaehoon Chung
  2015-05-06  1:33       ` zhangfei
  0 siblings, 1 reply; 13+ messages in thread
From: Jaehoon Chung @ 2015-05-06  1:26 UTC (permalink / raw)
  To: Zhangfei Gao; +Cc: linux-mmc

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
> 


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] mmc: dw_mmc: dw_mci_get_cd check MMC_CAP_NONREMOVABLE
  2015-05-06  1:26     ` Jaehoon Chung
@ 2015-05-06  1:33       ` zhangfei
  2015-05-06  1:39         ` Jaehoon Chung
  0 siblings, 1 reply; 13+ messages in thread
From: zhangfei @ 2015-05-06  1:33 UTC (permalink / raw)
  To: Jaehoon Chung; +Cc: linux-mmc



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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] mmc: dw_mmc: dw_mci_get_cd check MMC_CAP_NONREMOVABLE
  2015-05-06  1:33       ` zhangfei
@ 2015-05-06  1:39         ` Jaehoon Chung
  2015-05-06  1:53           ` zhangfei
  2015-05-06  2:16           ` zhangfei
  0 siblings, 2 replies; 13+ messages in thread
From: Jaehoon Chung @ 2015-05-06  1:39 UTC (permalink / raw)
  To: zhangfei; +Cc: linux-mmc, Ulf Hansson

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
> 


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] mmc: dw_mmc: dw_mci_get_cd check MMC_CAP_NONREMOVABLE
  2015-05-06  1:39         ` Jaehoon Chung
@ 2015-05-06  1:53           ` zhangfei
  2015-05-06  2:16           ` zhangfei
  1 sibling, 0 replies; 13+ messages in thread
From: zhangfei @ 2015-05-06  1:53 UTC (permalink / raw)
  To: Jaehoon Chung; +Cc: linux-mmc, Ulf Hansson



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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] mmc: dw_mmc: dw_mci_get_cd check MMC_CAP_NONREMOVABLE
  2015-05-06  1:39         ` Jaehoon Chung
  2015-05-06  1:53           ` zhangfei
@ 2015-05-06  2:16           ` zhangfei
  2015-05-06  4:21             ` Jaehoon Chung
  1 sibling, 1 reply; 13+ messages in thread
From: zhangfei @ 2015-05-06  2:16 UTC (permalink / raw)
  To: Jaehoon Chung; +Cc: linux-mmc, Ulf Hansson



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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] mmc: dw_mmc: dw_mci_get_cd check MMC_CAP_NONREMOVABLE
  2015-05-06  2:16           ` zhangfei
@ 2015-05-06  4:21             ` Jaehoon Chung
  2015-05-06  5:30               ` zhangfei
  0 siblings, 1 reply; 13+ messages in thread
From: Jaehoon Chung @ 2015-05-06  4:21 UTC (permalink / raw)
  To: zhangfei; +Cc: linux-mmc, Ulf Hansson

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


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] mmc: dw_mmc: dw_mci_get_cd check MMC_CAP_NONREMOVABLE
  2015-05-06  4:21             ` Jaehoon Chung
@ 2015-05-06  5:30               ` zhangfei
  2015-05-06  6:13                 ` Jaehoon Chung
  0 siblings, 1 reply; 13+ messages in thread
From: zhangfei @ 2015-05-06  5:30 UTC (permalink / raw)
  To: Jaehoon Chung; +Cc: linux-mmc, Ulf Hansson



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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] mmc: dw_mmc: dw_mci_get_cd check MMC_CAP_NONREMOVABLE
  2015-05-06  5:30               ` zhangfei
@ 2015-05-06  6:13                 ` Jaehoon Chung
  2015-05-06  6:30                   ` zhangfei
  0 siblings, 1 reply; 13+ messages in thread
From: Jaehoon Chung @ 2015-05-06  6:13 UTC (permalink / raw)
  To: zhangfei; +Cc: linux-mmc, Ulf Hansson

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
> 


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] mmc: dw_mmc: dw_mci_get_cd check MMC_CAP_NONREMOVABLE
  2015-05-06  6:13                 ` Jaehoon Chung
@ 2015-05-06  6:30                   ` zhangfei
  2015-05-06  7:23                     ` Arnd Bergmann
  0 siblings, 1 reply; 13+ messages in thread
From: zhangfei @ 2015-05-06  6:30 UTC (permalink / raw)
  To: Jaehoon Chung; +Cc: linux-mmc, Ulf Hansson



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.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] mmc: dw_mmc: dw_mci_get_cd check MMC_CAP_NONREMOVABLE
  2015-05-06  6:30                   ` zhangfei
@ 2015-05-06  7:23                     ` Arnd Bergmann
  0 siblings, 0 replies; 13+ messages in thread
From: Arnd Bergmann @ 2015-05-06  7:23 UTC (permalink / raw)
  To: zhangfei; +Cc: Jaehoon Chung, linux-mmc, Ulf Hansson

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


^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2015-05-06  7:24 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-05-05  8:54 [PATCH] mmc: dw_mmc: dw_mci_get_cd check MMC_CAP_NONREMOVABLE Zhangfei Gao
2015-05-06  0:36 ` Jaehoon Chung
2015-05-06  1:14   ` Zhangfei Gao
2015-05-06  1:26     ` Jaehoon Chung
2015-05-06  1:33       ` zhangfei
2015-05-06  1:39         ` Jaehoon Chung
2015-05-06  1:53           ` zhangfei
2015-05-06  2:16           ` zhangfei
2015-05-06  4:21             ` Jaehoon Chung
2015-05-06  5:30               ` zhangfei
2015-05-06  6:13                 ` Jaehoon Chung
2015-05-06  6:30                   ` zhangfei
2015-05-06  7:23                     ` Arnd Bergmann

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.