mmc: dw_mmc: dw_mci_get_cd check MMC_CAP_NONREMOVABLE

Message ID 1430816089-8857-1-git-send-email-zhangfei.gao@linaro.org
State New
Headers show

Commit Message

zhangfei May 5, 2015, 8:54 a.m.
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

zhangfei May 6, 2015, 1:14 a.m. | #1
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
zhangfei May 6, 2015, 1:33 a.m. | #2
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
zhangfei May 6, 2015, 1:53 a.m. | #3
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 May 6, 2015, 2:16 a.m. | #4
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
zhangfei May 6, 2015, 5:30 a.m. | #5
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
zhangfei May 6, 2015, 6:30 a.m. | #6
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

Patch hide | download patch | download mbox

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;