[PATCH 1/1] mmc: dw_mmc: Fix card detection regression

[PATCH 1/1] mmc: dw_mmc: Fix card detection regression

[Sachin Kamat]

mmc_gpio_get_cd returns a negative error value upon failure.
However gpio_cd was initialised with the negated return value
of the above function. This negation resulted in losing of the
error value thereby triggering the code to take a wrong path as
IS_ERR_VALUE(gpio_cd) now returned 0 even when mmc_gpio_get_cd
returned an error value. This issue introduced by commit bf626e5550f2
("mmc: dw_mmc: use slot-gpio to handle cd pin") caused card detection
failure on Exynos5 boards which is now fixed by this patch.

Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org>
Cc: Zhangfei Gao <zhangfei.gao@linaro.org>
Cc: Jaehoon Chung <jh80.chung@samsung.com>
Cc: Arnd Bergmann <arnd@arndb.de>
---
 drivers/mmc/host/dw_mmc.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index a776f24f4311..f1683ba194ee 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -1033,7 +1033,7 @@ static int dw_mci_get_cd(struct mmc_host *mmc)
 	int present;
 	struct dw_mci_slot *slot = mmc_priv(mmc);
 	struct dw_mci_board *brd = slot->host->pdata;
-	int gpio_cd = !mmc_gpio_get_cd(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)
@@ -1041,7 +1041,7 @@ static int dw_mci_get_cd(struct mmc_host *mmc)
 	else if (brd->get_cd)
 		present = !brd->get_cd(slot->id);
 	else if (!IS_ERR_VALUE(gpio_cd))
-		present = !!gpio_cd;
+		present = !gpio_cd;
 	else
 		present = (mci_readl(slot->host, CDETECT) & (1 << slot->id))
 			== 0 ? 1 : 0;
-- 
1.7.9.5

Re: [PATCH 1/1] mmc: dw_mmc: Fix card detection regression

[zhangfei]

Hi, Sachin



On 01/15/2014 04:31 PM, Sachin Kamat wrote:
> mmc_gpio_get_cd returns a negative error value upon failure.
> However gpio_cd was initialised with the negated return value
> of the above function. This negation resulted in losing of the
> error value thereby triggering the code to take a wrong path as
> IS_ERR_VALUE(gpio_cd) now returned 0 even when mmc_gpio_get_cd
> returned an error value. This issue introduced by commit bf626e5550f2
> ("mmc: dw_mmc: use slot-gpio to handle cd pin") caused card detection
> failure on Exynos5 boards which is now fixed by this patch.
>
> Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org>
> Cc: Zhangfei Gao <zhangfei.gao@linaro.org>
> Cc: Jaehoon Chung <jh80.chung@samsung.com>
> Cc: Arnd Bergmann <arnd@arndb.de>
> ---

Thanks for the patch
I just submitted one patch to fix the issue, in case you missed it.
Also spin_lock is required for atomic accessing DW_MMC_CARD_PRESENT.
Otherwise sd detect may be failed sometimes.

Could you help take a look.

Thanks

Re: [PATCH 1/1] mmc: dw_mmc: Fix card detection regression

[Sachin Kamat]

Hi Zhangfei,

On 15 January 2014 14:11, zhangfei <zhangfei.gao@linaro.org> wrote:
> Hi, Sachin
>
>
>
>
> On 01/15/2014 04:31 PM, Sachin Kamat wrote:
>>
>> mmc_gpio_get_cd returns a negative error value upon failure.
>> However gpio_cd was initialised with the negated return value
>> of the above function. This negation resulted in losing of the
>> error value thereby triggering the code to take a wrong path as
>> IS_ERR_VALUE(gpio_cd) now returned 0 even when mmc_gpio_get_cd
>> returned an error value. This issue introduced by commit bf626e5550f2
>> ("mmc: dw_mmc: use slot-gpio to handle cd pin") caused card detection
>> failure on Exynos5 boards which is now fixed by this patch.
>>
>> Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org>
>> Cc: Zhangfei Gao <zhangfei.gao@linaro.org>
>> Cc: Jaehoon Chung <jh80.chung@samsung.com>
>> Cc: Arnd Bergmann <arnd@arndb.de>
>> ---
>
>
> Thanks for the patch
> I just submitted one patch to fix the issue, in case you missed it.

Yes I did as I am not subscribed to mmc list.

> Also spin_lock is required for atomic accessing DW_MMC_CARD_PRESENT.
> Otherwise sd detect may be failed sometimes.
>
> Could you help take a look.

I looked at and tested your patch [1] (which is similar to mine except
for the locking part).

Since I do not have your patch in my mailbox, FWIW,

Reviewed-by: Sachin Kamat <sachin.kamat@linaro.org>
Tested-by: Sachin Kamat <sachin.kamat@linaro.org>

[1] http://permalink.gmane.org/gmane.linux.kernel.mmc/24600
---
With warm regards,
Sachin

Re: [PATCH 1/1] mmc: dw_mmc: Fix card detection regression

[zhangfei]

On 01/15/2014 05:39 PM, Sachin Kamat wrote:
>> Thanks for the patch
>> I just submitted one patch to fix the issue, in case you missed it.
>
> Yes I did as I am not subscribed to mmc list.
>
>> Also spin_lock is required for atomic accessing DW_MMC_CARD_PRESENT.
>> Otherwise sd detect may be failed sometimes.
>>
>> Could you help take a look.
>
> I looked at and tested your patch [1] (which is similar to mine except
> for the locking part).
>
> Since I do not have your patch in my mailbox, FWIW,
>
> Reviewed-by: Sachin Kamat <sachin.kamat@linaro.org>
> Tested-by: Sachin Kamat <sachin.kamat@linaro.org>

Thanks Sachin,  that's great it works, all my mistake.
Then I resumit the patch with this and cc you.

Thanks

Re: [PATCH 1/1] mmc: dw_mmc: Fix card detection regression

[Sachin Kamat]

On 15 January 2014 15:13, zhangfei <zhangfei.gao@linaro.org> wrote:
>
>
> On 01/15/2014 05:39 PM, Sachin Kamat wrote:
>>>
>>> Thanks for the patch
>>> I just submitted one patch to fix the issue, in case you missed it.
>>
>>
>> Yes I did as I am not subscribed to mmc list.
>>
>>> Also spin_lock is required for atomic accessing DW_MMC_CARD_PRESENT.
>>> Otherwise sd detect may be failed sometimes.
>>>
>>> Could you help take a look.
>>
>>
>> I looked at and tested your patch [1] (which is similar to mine except
>> for the locking part).
>>
>> Since I do not have your patch in my mailbox, FWIW,
>>
>> Reviewed-by: Sachin Kamat <sachin.kamat@linaro.org>
>> Tested-by: Sachin Kamat <sachin.kamat@linaro.org>
>
>
> Thanks Sachin,  that's great it works, all my mistake.
> Then I resumit the patch with this and cc you.

If you are planning to re-submit, then you may please consider
revising the commit message as follows:

s/Introdeced/Introduced
Please provide patch title after the commit Id in braces ("<patch title>").
s/ingored/ignored

You may also elaborate on why IS_ERR_VALUE(!mmc_gpio_get_cd(mmc)) does not work.

-- 
With warm regards,
Sachin