| Message ID | 1484305503-30723-1-git-send-email-ulf.hansson@linaro.org |
|---|---|
| State | New |
| Headers | show |
On Fri, Jan 13, 2017 at 12:05:03PM +0100, Ulf Hansson wrote: > Regressions for not being able to detect an eMMC HS DDR mode card has been > reported for the sdhci-esdhc-imx driver, but potentially other sdhci > variants may suffer from the similar problem. > > The commit e173f8911f09 ("mmc: core: Update CMD13 polling policy when > switch to HS DDR mode"), is causing the problem. It seems that change moved > one step to far, regarding changing the host's timing before polling for a > busy card. > > To fix this, let's move back to the behaviour when the host's timing is > updated after the polling, but before the switch status is fetched and > validated. > > In cases when polling with CMD13, we keep validating the switch status at > each attempt. However, to align with the other card busy detections > mechanism, let's fetch and validate the switch status also after the host's > timing is updated. > > Reported-by: Clemens Gruber <clemens.gruber@pqgruber.com> > Reported-by: Gary Bisson <gary.bisson@boundarydevices.com> > Fixes: e173f8911f09 ("mmc: core: Update CMD13 polling policy when switch..") > Cc: Shawn Lin <shawn.lin@rock-chips.com> > Cc: Dong Aisheng <aisheng.dong@nxp.com> > Cc: Haibo Chen <haibo.chen@nxp.com> > Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org> > --- > drivers/mmc/core/mmc_ops.c | 25 ++++++++++++------------- > 1 file changed, 12 insertions(+), 13 deletions(-) > > diff --git a/drivers/mmc/core/mmc_ops.c b/drivers/mmc/core/mmc_ops.c > index db2969f..fe80f26 100644 > --- a/drivers/mmc/core/mmc_ops.c > +++ b/drivers/mmc/core/mmc_ops.c > @@ -506,9 +506,6 @@ static int mmc_poll_for_busy(struct mmc_card *card, unsigned int timeout_ms, > } > } while (busy); > > - if (host->ops->card_busy && send_status) > - return mmc_switch_status(card); > - > return 0; > } > > @@ -577,24 +574,26 @@ int __mmc_switch(struct mmc_card *card, u8 set, u8 index, u8 value, > if (!use_busy_signal) > goto out; > > - /* Switch to new timing before poll and check switch status. */ > - if (timing) > - mmc_set_timing(host, timing); > - > /*If SPI or used HW busy detection above, then we don't need to poll. */ > if (((host->caps & MMC_CAP_WAIT_WHILE_BUSY) && use_r1b_resp) || > - mmc_host_is_spi(host)) { > - if (send_status) > - err = mmc_switch_status(card); > + mmc_host_is_spi(host)) > goto out_tim; > - } > > /* Let's try to poll to find out when the command is completed. */ > err = mmc_poll_for_busy(card, timeout_ms, send_status, retry_crc_err); > + if (err) > + goto out; > > out_tim: > - if (err && timing) > - mmc_set_timing(host, old_timing); > + /* Switch to new timing before check switch status. */ > + if (timing) > + mmc_set_timing(host, timing); > + > + if (send_status) { > + err = mmc_switch_status(card); > + if (err && timing) > + mmc_set_timing(host, old_timing); > + } > out: > mmc_retune_release(host); > > -- > 1.9.1 > Hi, this patch fixes the boot problem on my i.MX6Q board! Tested-by: Clemens Gruber <clemens.gruber@pqgruber.com> Thanks, Clemens -- 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
On Fri, Jan 13, 2017 at 1:25 PM, Clemens Gruber <clemens.gruber@pqgruber.com> wrote: > On Fri, Jan 13, 2017 at 12:05:03PM +0100, Ulf Hansson wrote: >> Regressions for not being able to detect an eMMC HS DDR mode card has been >> reported for the sdhci-esdhc-imx driver, but potentially other sdhci >> variants may suffer from the similar problem. >> >> The commit e173f8911f09 ("mmc: core: Update CMD13 polling policy when >> switch to HS DDR mode"), is causing the problem. It seems that change moved >> one step to far, regarding changing the host's timing before polling for a >> busy card. >> >> To fix this, let's move back to the behaviour when the host's timing is >> updated after the polling, but before the switch status is fetched and >> validated. >> >> In cases when polling with CMD13, we keep validating the switch status at >> each attempt. However, to align with the other card busy detections >> mechanism, let's fetch and validate the switch status also after the host's >> timing is updated. >> >> Reported-by: Clemens Gruber <clemens.gruber@pqgruber.com> >> Reported-by: Gary Bisson <gary.bisson@boundarydevices.com> >> Fixes: e173f8911f09 ("mmc: core: Update CMD13 polling policy when switch..") >> Cc: Shawn Lin <shawn.lin@rock-chips.com> >> Cc: Dong Aisheng <aisheng.dong@nxp.com> >> Cc: Haibo Chen <haibo.chen@nxp.com> >> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org> >> --- >> drivers/mmc/core/mmc_ops.c | 25 ++++++++++++------------- >> 1 file changed, 12 insertions(+), 13 deletions(-) >> >> diff --git a/drivers/mmc/core/mmc_ops.c b/drivers/mmc/core/mmc_ops.c >> index db2969f..fe80f26 100644 >> --- a/drivers/mmc/core/mmc_ops.c >> +++ b/drivers/mmc/core/mmc_ops.c >> @@ -506,9 +506,6 @@ static int mmc_poll_for_busy(struct mmc_card *card, unsigned int timeout_ms, >> } >> } while (busy); >> >> - if (host->ops->card_busy && send_status) >> - return mmc_switch_status(card); >> - >> return 0; >> } >> >> @@ -577,24 +574,26 @@ int __mmc_switch(struct mmc_card *card, u8 set, u8 index, u8 value, >> if (!use_busy_signal) >> goto out; >> >> - /* Switch to new timing before poll and check switch status. */ >> - if (timing) >> - mmc_set_timing(host, timing); >> - >> /*If SPI or used HW busy detection above, then we don't need to poll. */ >> if (((host->caps & MMC_CAP_WAIT_WHILE_BUSY) && use_r1b_resp) || >> - mmc_host_is_spi(host)) { >> - if (send_status) >> - err = mmc_switch_status(card); >> + mmc_host_is_spi(host)) >> goto out_tim; >> - } >> >> /* Let's try to poll to find out when the command is completed. */ >> err = mmc_poll_for_busy(card, timeout_ms, send_status, retry_crc_err); >> + if (err) >> + goto out; >> >> out_tim: >> - if (err && timing) >> - mmc_set_timing(host, old_timing); >> + /* Switch to new timing before check switch status. */ >> + if (timing) >> + mmc_set_timing(host, timing); >> + >> + if (send_status) { >> + err = mmc_switch_status(card); >> + if (err && timing) >> + mmc_set_timing(host, old_timing); >> + } >> out: >> mmc_retune_release(host); >> >> -- >> 1.9.1 >> > > Hi, > > this patch fixes the boot problem on my i.MX6Q board! > > Tested-by: Clemens Gruber <clemens.gruber@pqgruber.com> Tested on imx6q and imx6ul boards, with format, write and read. Tested-by: Jagan Teki <jagan@amarulasolutions.com> thanks! -- Jagan Teki Free Software Engineer | www.openedev.com U-Boot, Linux | Upstream Maintainer Hyderabad, India. -- 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
On 2017/1/13 19:05, Ulf Hansson wrote: > Regressions for not being able to detect an eMMC HS DDR mode card has been > reported for the sdhci-esdhc-imx driver, but potentially other sdhci > variants may suffer from the similar problem. > > The commit e173f8911f09 ("mmc: core: Update CMD13 polling policy when > switch to HS DDR mode"), is causing the problem. It seems that change moved > one step to far, regarding changing the host's timing before polling for a > busy card. > > To fix this, let's move back to the behaviour when the host's timing is > updated after the polling, but before the switch status is fetched and > validated. > > In cases when polling with CMD13, we keep validating the switch status at > each attempt. However, to align with the other card busy detections > mechanism, let's fetch and validate the switch status also after the host's > timing is updated. > Reviewed-by: Shawn Lin <shawn.lin@rock-chips.com> > Reported-by: Clemens Gruber <clemens.gruber@pqgruber.com> > Reported-by: Gary Bisson <gary.bisson@boundarydevices.com> > Fixes: e173f8911f09 ("mmc: core: Update CMD13 polling policy when switch..") > Cc: Shawn Lin <shawn.lin@rock-chips.com> > Cc: Dong Aisheng <aisheng.dong@nxp.com> > Cc: Haibo Chen <haibo.chen@nxp.com> > Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org> > --- > drivers/mmc/core/mmc_ops.c | 25 ++++++++++++------------- > 1 file changed, 12 insertions(+), 13 deletions(-) > > diff --git a/drivers/mmc/core/mmc_ops.c b/drivers/mmc/core/mmc_ops.c > index db2969f..fe80f26 100644 > --- a/drivers/mmc/core/mmc_ops.c > +++ b/drivers/mmc/core/mmc_ops.c > @@ -506,9 +506,6 @@ static int mmc_poll_for_busy(struct mmc_card *card, unsigned int timeout_ms, > } > } while (busy); > > - if (host->ops->card_busy && send_status) > - return mmc_switch_status(card); > - > return 0; > } > > @@ -577,24 +574,26 @@ int __mmc_switch(struct mmc_card *card, u8 set, u8 index, u8 value, > if (!use_busy_signal) > goto out; > > - /* Switch to new timing before poll and check switch status. */ > - if (timing) > - mmc_set_timing(host, timing); > - > /*If SPI or used HW busy detection above, then we don't need to poll. */ > if (((host->caps & MMC_CAP_WAIT_WHILE_BUSY) && use_r1b_resp) || > - mmc_host_is_spi(host)) { > - if (send_status) > - err = mmc_switch_status(card); > + mmc_host_is_spi(host)) > goto out_tim; > - } > > /* Let's try to poll to find out when the command is completed. */ > err = mmc_poll_for_busy(card, timeout_ms, send_status, retry_crc_err); > + if (err) > + goto out; > > out_tim: > - if (err && timing) > - mmc_set_timing(host, old_timing); > + /* Switch to new timing before check switch status. */ > + if (timing) > + mmc_set_timing(host, timing); > + > + if (send_status) { > + err = mmc_switch_status(card); > + if (err && timing) > + mmc_set_timing(host, old_timing); > + } > out: > mmc_retune_release(host); > > -- Best Regards Shawn Lin -- 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
> -----Original Message----- > From: Ulf Hansson [mailto:ulf.hansson@linaro.org] > Sent: Friday, January 13, 2017 7:05 PM > To: linux-mmc@vger.kernel.org; Ulf Hansson <ulf.hansson@linaro.org> > Cc: Jaehoon Chung <jh80.chung@samsung.com>; Adrian Hunter > <adrian.hunter@intel.com>; Linus Walleij <linus.walleij@linaro.org>; Chaotian > Jing <chaotian.jing@mediatek.com>; Yong Mao <yong.mao@mediatek.com>; > Shawn Lin <shawn.lin@rock-chips.com>; Clemens Gruber > <clemens.gruber@pqgruber.com>; Gary Bisson > <gary.bisson@boundarydevices.com>; Dong Aisheng <dongas86@gmail.com>; > Fabio Estevam <festevam@gmail.com>; A.S. Dong <aisheng.dong@nxp.com>; > Bough Chen <haibo.chen@nxp.com> > Subject: [PATCH] mmc: core: Restore parts of the polling policy when switch to > HS/HS DDR > > Regressions for not being able to detect an eMMC HS DDR mode card has been > reported for the sdhci-esdhc-imx driver, but potentially other sdhci variants > may suffer from the similar problem. > > The commit e173f8911f09 ("mmc: core: Update CMD13 polling policy when > switch to HS DDR mode"), is causing the problem. It seems that change moved > one step to far, regarding changing the host's timing before polling for a busy > card. > > To fix this, let's move back to the behaviour when the host's timing is updated > after the polling, but before the switch status is fetched and validated. > > In cases when polling with CMD13, we keep validating the switch status at each > attempt. However, to align with the other card busy detections mechanism, > let's fetch and validate the switch status also after the host's timing is updated. > > Reported-by: Clemens Gruber <clemens.gruber@pqgruber.com> > Reported-by: Gary Bisson <gary.bisson@boundarydevices.com> > Fixes: e173f8911f09 ("mmc: core: Update CMD13 polling policy when switch..") > Cc: Shawn Lin <shawn.lin@rock-chips.com> > Cc: Dong Aisheng <aisheng.dong@nxp.com> > Cc: Haibo Chen <haibo.chen@nxp.com> > Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org> > --- > drivers/mmc/core/mmc_ops.c | 25 ++++++++++++------------- > 1 file changed, 12 insertions(+), 13 deletions(-) > > diff --git a/drivers/mmc/core/mmc_ops.c b/drivers/mmc/core/mmc_ops.c > index db2969f..fe80f26 100644 > --- a/drivers/mmc/core/mmc_ops.c > +++ b/drivers/mmc/core/mmc_ops.c > @@ -506,9 +506,6 @@ static int mmc_poll_for_busy(struct mmc_card *card, > unsigned int timeout_ms, > } > } while (busy); > > - if (host->ops->card_busy && send_status) > - return mmc_switch_status(card); > - > return 0; > } > > @@ -577,24 +574,26 @@ int __mmc_switch(struct mmc_card *card, u8 set, u8 > index, u8 value, > if (!use_busy_signal) > goto out; > > - /* Switch to new timing before poll and check switch status. */ > - if (timing) > - mmc_set_timing(host, timing); > - > /*If SPI or used HW busy detection above, then we don't need to poll. > */ > if (((host->caps & MMC_CAP_WAIT_WHILE_BUSY) && use_r1b_resp) > || > - mmc_host_is_spi(host)) { > - if (send_status) > - err = mmc_switch_status(card); > + mmc_host_is_spi(host)) > goto out_tim; > - } > > /* Let's try to poll to find out when the command is completed. */ > err = mmc_poll_for_busy(card, timeout_ms, send_status, > retry_crc_err); > + if (err) > + goto out; > > out_tim: > - if (err && timing) > - mmc_set_timing(host, old_timing); > + /* Switch to new timing before check switch status. */ > + if (timing) > + mmc_set_timing(host, timing); > + > + if (send_status) { > + err = mmc_switch_status(card); > + if (err && timing) > + mmc_set_timing(host, old_timing); > + } > out: > mmc_retune_release(host); > > -- > 1.9.1 Test this patch on i.MX6Q-SABRESD board, it can fix the issue. Tested-by: Haibo Chen <haibo.chen@nxp.com> Best Regards, Hiabo Chen -- 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
Hi Ulf, On Fri, Jan 13, 2017 at 7:05 PM, Ulf Hansson <ulf.hansson@linaro.org> wrote: > Regressions for not being able to detect an eMMC HS DDR mode card has been > reported for the sdhci-esdhc-imx driver, but potentially other sdhci > variants may suffer from the similar problem. > > The commit e173f8911f09 ("mmc: core: Update CMD13 polling policy when > switch to HS DDR mode"), is causing the problem. It seems that change moved > one step to far, regarding changing the host's timing before polling for a > busy card. > > To fix this, let's move back to the behaviour when the host's timing is > updated after the polling, but before the switch status is fetched and > validated. > > In cases when polling with CMD13, we keep validating the switch status at > each attempt. However, to align with the other card busy detections > mechanism, let's fetch and validate the switch status also after the host's > timing is updated. > > Reported-by: Clemens Gruber <clemens.gruber@pqgruber.com> > Reported-by: Gary Bisson <gary.bisson@boundarydevices.com> > Fixes: e173f8911f09 ("mmc: core: Update CMD13 polling policy when switch..") > Cc: Shawn Lin <shawn.lin@rock-chips.com> > Cc: Dong Aisheng <aisheng.dong@nxp.com> > Cc: Haibo Chen <haibo.chen@nxp.com> > Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org> Thanks for the fix. Reviewed-by: Dong Aisheng <aisheng.dong@nxp.com> BTW, i wonder if we could then remove the CMD13 Polling method or give a temporarily WARN_ONCE to indicate OBSOLETED using and force all hosts to provide card_busy() callback. Then we can permanently fix the potential timing mismatch issue. Regards Dong Aisheng > --- > drivers/mmc/core/mmc_ops.c | 25 ++++++++++++------------- > 1 file changed, 12 insertions(+), 13 deletions(-) > > diff --git a/drivers/mmc/core/mmc_ops.c b/drivers/mmc/core/mmc_ops.c > index db2969f..fe80f26 100644 > --- a/drivers/mmc/core/mmc_ops.c > +++ b/drivers/mmc/core/mmc_ops.c > @@ -506,9 +506,6 @@ static int mmc_poll_for_busy(struct mmc_card *card, unsigned int timeout_ms, > } > } while (busy); > > - if (host->ops->card_busy && send_status) > - return mmc_switch_status(card); > - > return 0; > } > > @@ -577,24 +574,26 @@ int __mmc_switch(struct mmc_card *card, u8 set, u8 index, u8 value, > if (!use_busy_signal) > goto out; > > - /* Switch to new timing before poll and check switch status. */ > - if (timing) > - mmc_set_timing(host, timing); > - > /*If SPI or used HW busy detection above, then we don't need to poll. */ > if (((host->caps & MMC_CAP_WAIT_WHILE_BUSY) && use_r1b_resp) || > - mmc_host_is_spi(host)) { > - if (send_status) > - err = mmc_switch_status(card); > + mmc_host_is_spi(host)) > goto out_tim; > - } > > /* Let's try to poll to find out when the command is completed. */ > err = mmc_poll_for_busy(card, timeout_ms, send_status, retry_crc_err); > + if (err) > + goto out; > > out_tim: > - if (err && timing) > - mmc_set_timing(host, old_timing); > + /* Switch to new timing before check switch status. */ > + if (timing) > + mmc_set_timing(host, timing); > + > + if (send_status) { > + err = mmc_switch_status(card); > + if (err && timing) > + mmc_set_timing(host, old_timing); > + } > out: > mmc_retune_release(host); > > -- > 1.9.1 > -- 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
On 16 January 2017 at 04:48, Dong Aisheng <dongas86@gmail.com> wrote: > Hi Ulf, > > On Fri, Jan 13, 2017 at 7:05 PM, Ulf Hansson <ulf.hansson@linaro.org> wrote: >> Regressions for not being able to detect an eMMC HS DDR mode card has been >> reported for the sdhci-esdhc-imx driver, but potentially other sdhci >> variants may suffer from the similar problem. >> >> The commit e173f8911f09 ("mmc: core: Update CMD13 polling policy when >> switch to HS DDR mode"), is causing the problem. It seems that change moved >> one step to far, regarding changing the host's timing before polling for a >> busy card. >> >> To fix this, let's move back to the behaviour when the host's timing is >> updated after the polling, but before the switch status is fetched and >> validated. >> >> In cases when polling with CMD13, we keep validating the switch status at >> each attempt. However, to align with the other card busy detections >> mechanism, let's fetch and validate the switch status also after the host's >> timing is updated. >> >> Reported-by: Clemens Gruber <clemens.gruber@pqgruber.com> >> Reported-by: Gary Bisson <gary.bisson@boundarydevices.com> >> Fixes: e173f8911f09 ("mmc: core: Update CMD13 polling policy when switch..") >> Cc: Shawn Lin <shawn.lin@rock-chips.com> >> Cc: Dong Aisheng <aisheng.dong@nxp.com> >> Cc: Haibo Chen <haibo.chen@nxp.com> >> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org> > > Thanks for the fix. > > Reviewed-by: Dong Aisheng <aisheng.dong@nxp.com> Thanks for reviewing! > > BTW, i wonder if we could then remove the CMD13 Polling method or give a > temporarily WARN_ONCE to indicate OBSOLETED using and force all hosts > to provide card_busy() callback. > Then we can permanently fix the potential timing mismatch issue. In the end it seems like the issue is host specific. I have successfully used CM13 polling on my ux500 board (mmci.c, with some hacks to force CMD13). If we see any further issues, perhaps we should invent a new host cap, which forbids the CMD13 polling and then those host drivers that has issues, can set it. Wouldn't that be okay for you? [...] Kind regards Uffe -- 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
On 13 January 2017 at 12:05, Ulf Hansson <ulf.hansson@linaro.org> wrote: > Regressions for not being able to detect an eMMC HS DDR mode card has been > reported for the sdhci-esdhc-imx driver, but potentially other sdhci > variants may suffer from the similar problem. > > The commit e173f8911f09 ("mmc: core: Update CMD13 polling policy when > switch to HS DDR mode"), is causing the problem. It seems that change moved > one step to far, regarding changing the host's timing before polling for a > busy card. > > To fix this, let's move back to the behaviour when the host's timing is > updated after the polling, but before the switch status is fetched and > validated. > > In cases when polling with CMD13, we keep validating the switch status at > each attempt. However, to align with the other card busy detections > mechanism, let's fetch and validate the switch status also after the host's > timing is updated. > > Reported-by: Clemens Gruber <clemens.gruber@pqgruber.com> > Reported-by: Gary Bisson <gary.bisson@boundarydevices.com> > Fixes: e173f8911f09 ("mmc: core: Update CMD13 polling policy when switch..") > Cc: Shawn Lin <shawn.lin@rock-chips.com> > Cc: Dong Aisheng <aisheng.dong@nxp.com> > Cc: Haibo Chen <haibo.chen@nxp.com> > Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org> Applied for fixes! Thanks everybody for reviews and tests! Kind regards Uffe > --- > drivers/mmc/core/mmc_ops.c | 25 ++++++++++++------------- > 1 file changed, 12 insertions(+), 13 deletions(-) > > diff --git a/drivers/mmc/core/mmc_ops.c b/drivers/mmc/core/mmc_ops.c > index db2969f..fe80f26 100644 > --- a/drivers/mmc/core/mmc_ops.c > +++ b/drivers/mmc/core/mmc_ops.c > @@ -506,9 +506,6 @@ static int mmc_poll_for_busy(struct mmc_card *card, unsigned int timeout_ms, > } > } while (busy); > > - if (host->ops->card_busy && send_status) > - return mmc_switch_status(card); > - > return 0; > } > > @@ -577,24 +574,26 @@ int __mmc_switch(struct mmc_card *card, u8 set, u8 index, u8 value, > if (!use_busy_signal) > goto out; > > - /* Switch to new timing before poll and check switch status. */ > - if (timing) > - mmc_set_timing(host, timing); > - > /*If SPI or used HW busy detection above, then we don't need to poll. */ > if (((host->caps & MMC_CAP_WAIT_WHILE_BUSY) && use_r1b_resp) || > - mmc_host_is_spi(host)) { > - if (send_status) > - err = mmc_switch_status(card); > + mmc_host_is_spi(host)) > goto out_tim; > - } > > /* Let's try to poll to find out when the command is completed. */ > err = mmc_poll_for_busy(card, timeout_ms, send_status, retry_crc_err); > + if (err) > + goto out; > > out_tim: > - if (err && timing) > - mmc_set_timing(host, old_timing); > + /* Switch to new timing before check switch status. */ > + if (timing) > + mmc_set_timing(host, timing); > + > + if (send_status) { > + err = mmc_switch_status(card); > + if (err && timing) > + mmc_set_timing(host, old_timing); > + } > out: > mmc_retune_release(host); > > -- > 1.9.1 > -- 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
Hi Ulf, On Mon, Jan 16, 2017 at 11:13 PM, Ulf Hansson <ulf.hansson@linaro.org> wrote: > On 16 January 2017 at 04:48, Dong Aisheng <dongas86@gmail.com> wrote: >> Hi Ulf, >> >> On Fri, Jan 13, 2017 at 7:05 PM, Ulf Hansson <ulf.hansson@linaro.org> wrote: >>> Regressions for not being able to detect an eMMC HS DDR mode card has been >>> reported for the sdhci-esdhc-imx driver, but potentially other sdhci >>> variants may suffer from the similar problem. >>> >>> The commit e173f8911f09 ("mmc: core: Update CMD13 polling policy when >>> switch to HS DDR mode"), is causing the problem. It seems that change moved >>> one step to far, regarding changing the host's timing before polling for a >>> busy card. >>> >>> To fix this, let's move back to the behaviour when the host's timing is >>> updated after the polling, but before the switch status is fetched and >>> validated. >>> >>> In cases when polling with CMD13, we keep validating the switch status at >>> each attempt. However, to align with the other card busy detections >>> mechanism, let's fetch and validate the switch status also after the host's >>> timing is updated. >>> >>> Reported-by: Clemens Gruber <clemens.gruber@pqgruber.com> >>> Reported-by: Gary Bisson <gary.bisson@boundarydevices.com> >>> Fixes: e173f8911f09 ("mmc: core: Update CMD13 polling policy when switch..") >>> Cc: Shawn Lin <shawn.lin@rock-chips.com> >>> Cc: Dong Aisheng <aisheng.dong@nxp.com> >>> Cc: Haibo Chen <haibo.chen@nxp.com> >>> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org> >> >> Thanks for the fix. >> >> Reviewed-by: Dong Aisheng <aisheng.dong@nxp.com> > > Thanks for reviewing! > >> >> BTW, i wonder if we could then remove the CMD13 Polling method or give a >> temporarily WARN_ONCE to indicate OBSOLETED using and force all hosts >> to provide card_busy() callback. >> Then we can permanently fix the potential timing mismatch issue. > > In the end it seems like the issue is host specific. I have > successfully used CM13 polling on my ux500 board (mmci.c, with some > hacks to force CMD13). > > If we see any further issues, perhaps we should invent a new host cap, > which forbids the CMD13 polling and then those host drivers that has > issues, can set it. Wouldn't that be okay for you? > I'm fine with it. Thanks Regards Dong Aisheng > [...] > > Kind regards > Uffe -- 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 --git a/drivers/mmc/core/mmc_ops.c b/drivers/mmc/core/mmc_ops.c index db2969f..fe80f26 100644 --- a/drivers/mmc/core/mmc_ops.c +++ b/drivers/mmc/core/mmc_ops.c @@ -506,9 +506,6 @@ static int mmc_poll_for_busy(struct mmc_card *card, unsigned int timeout_ms, } } while (busy); - if (host->ops->card_busy && send_status) - return mmc_switch_status(card); - return 0; } @@ -577,24 +574,26 @@ int __mmc_switch(struct mmc_card *card, u8 set, u8 index, u8 value, if (!use_busy_signal) goto out; - /* Switch to new timing before poll and check switch status. */ - if (timing) - mmc_set_timing(host, timing); - /*If SPI or used HW busy detection above, then we don't need to poll. */ if (((host->caps & MMC_CAP_WAIT_WHILE_BUSY) && use_r1b_resp) || - mmc_host_is_spi(host)) { - if (send_status) - err = mmc_switch_status(card); + mmc_host_is_spi(host)) goto out_tim; - } /* Let's try to poll to find out when the command is completed. */ err = mmc_poll_for_busy(card, timeout_ms, send_status, retry_crc_err); + if (err) + goto out; out_tim: - if (err && timing) - mmc_set_timing(host, old_timing); + /* Switch to new timing before check switch status. */ + if (timing) + mmc_set_timing(host, timing); + + if (send_status) { + err = mmc_switch_status(card); + if (err && timing) + mmc_set_timing(host, old_timing); + } out: mmc_retune_release(host);
Regressions for not being able to detect an eMMC HS DDR mode card has been reported for the sdhci-esdhc-imx driver, but potentially other sdhci variants may suffer from the similar problem. The commit e173f8911f09 ("mmc: core: Update CMD13 polling policy when switch to HS DDR mode"), is causing the problem. It seems that change moved one step to far, regarding changing the host's timing before polling for a busy card. To fix this, let's move back to the behaviour when the host's timing is updated after the polling, but before the switch status is fetched and validated. In cases when polling with CMD13, we keep validating the switch status at each attempt. However, to align with the other card busy detections mechanism, let's fetch and validate the switch status also after the host's timing is updated. Reported-by: Clemens Gruber <clemens.gruber@pqgruber.com> Reported-by: Gary Bisson <gary.bisson@boundarydevices.com> Fixes: e173f8911f09 ("mmc: core: Update CMD13 polling policy when switch..") Cc: Shawn Lin <shawn.lin@rock-chips.com> Cc: Dong Aisheng <aisheng.dong@nxp.com> Cc: Haibo Chen <haibo.chen@nxp.com> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org> --- drivers/mmc/core/mmc_ops.c | 25 ++++++++++++------------- 1 file changed, 12 insertions(+), 13 deletions(-) -- 1.9.1