| Message ID | 1389774667-18309-1-git-send-email-sachin.kamat@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
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
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
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
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.
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;
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(-)