| Message ID | 1479988969-1747-2-git-send-email-adrian.hunter@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 24/11/16 14:02, Adrian Hunter wrote: > The JEDEC specification indicates CMD13 can be used after a HS200 switch > to check for errors. However in practice some boards experience CRC errors > in the CMD13 response. Consequently, for HS200, CRC errors are not a > reliable way to know the switch failed. If there really is a problem, we > would expect tuning will fail and the result ends up the same. So change > the error condition to ignore CRC errors in that case. > > Signed-off-by: Adrian Hunter <adrian.hunter@intel.com> Any comments on this? > --- > drivers/mmc/core/mmc.c | 15 ++++++++++++++- > 1 file changed, 14 insertions(+), 1 deletion(-) > > diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c > index 3268fcd3378d..34d30e2a09ff 100644 > --- a/drivers/mmc/core/mmc.c > +++ b/drivers/mmc/core/mmc.c > @@ -1223,7 +1223,12 @@ int mmc_hs400_to_hs200(struct mmc_card *card) > mmc_set_timing(host, MMC_TIMING_MMC_HS200); > > err = mmc_switch_status(card); > - if (err) > + /* > + * For HS200, CRC errors are not a reliable way to know the switch > + * failed. If there really is a problem, we would expect tuning will > + * fail and the result ends up the same. > + */ > + if (err && err != -EILSEQ) > goto out_err; > > mmc_set_bus_speed(card); > @@ -1387,6 +1392,14 @@ static int mmc_select_hs200(struct mmc_card *card) > > err = mmc_switch_status(card); > /* > + * For HS200, CRC errors are not a reliable way to know the > + * switch failed. If there really is a problem, we would expect > + * tuning will fail and the result ends up the same. > + */ > + if (err == -EILSEQ) > + err = 0; > + > + /* > * mmc_select_timing() assumes timing has not changed if > * it is a switch error. > */ > -- 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 Adrian, On 12/01/2016 05:15 PM, Adrian Hunter wrote: > On 24/11/16 14:02, Adrian Hunter wrote: >> The JEDEC specification indicates CMD13 can be used after a HS200 switch >> to check for errors. However in practice some boards experience CRC errors >> in the CMD13 response. Consequently, for HS200, CRC errors are not a >> reliable way to know the switch failed. If there really is a problem, we >> would expect tuning will fail and the result ends up the same. So change >> the error condition to ignore CRC errors in that case. >> >> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com> > > Any comments on this? I agreed your opinion..CMD13 can't know whether switch is failed or not with CRC error. It might just know whether HS200 is working fine or not with CRC error. If CRC error is occurred, then user can knows when transfer some data. Then i think it's more easier to debug than now..does it make sense? Acked-by: Jaehoon Chung <jh80.chung@samsung.com> Best Regards, Jaehoon Chung > > >> --- >> drivers/mmc/core/mmc.c | 15 ++++++++++++++- >> 1 file changed, 14 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c >> index 3268fcd3378d..34d30e2a09ff 100644 >> --- a/drivers/mmc/core/mmc.c >> +++ b/drivers/mmc/core/mmc.c >> @@ -1223,7 +1223,12 @@ int mmc_hs400_to_hs200(struct mmc_card *card) >> mmc_set_timing(host, MMC_TIMING_MMC_HS200); >> >> err = mmc_switch_status(card); >> - if (err) >> + /* >> + * For HS200, CRC errors are not a reliable way to know the switch >> + * failed. If there really is a problem, we would expect tuning will >> + * fail and the result ends up the same. >> + */ >> + if (err && err != -EILSEQ) >> goto out_err; >> >> mmc_set_bus_speed(card); >> @@ -1387,6 +1392,14 @@ static int mmc_select_hs200(struct mmc_card *card) >> >> err = mmc_switch_status(card); >> /* >> + * For HS200, CRC errors are not a reliable way to know the >> + * switch failed. If there really is a problem, we would expect >> + * tuning will fail and the result ends up the same. >> + */ >> + if (err == -EILSEQ) >> + err = 0; >> + >> + /* >> * mmc_select_timing() assumes timing has not changed if >> * it is a switch error. >> */ >> > > > > -- 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 24 November 2016 at 13:02, Adrian Hunter <adrian.hunter@intel.com> wrote: > The JEDEC specification indicates CMD13 can be used after a HS200 switch > to check for errors. However in practice some boards experience CRC errors > in the CMD13 response. Consequently, for HS200, CRC errors are not a > reliable way to know the switch failed. If there really is a problem, we > would expect tuning will fail and the result ends up the same. So change > the error condition to ignore CRC errors in that case. I agree, this seems like a good idea. However... > > Signed-off-by: Adrian Hunter <adrian.hunter@intel.com> > --- > drivers/mmc/core/mmc.c | 15 ++++++++++++++- > 1 file changed, 14 insertions(+), 1 deletion(-) > > diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c > index 3268fcd3378d..34d30e2a09ff 100644 > --- a/drivers/mmc/core/mmc.c > +++ b/drivers/mmc/core/mmc.c > @@ -1223,7 +1223,12 @@ int mmc_hs400_to_hs200(struct mmc_card *card) > mmc_set_timing(host, MMC_TIMING_MMC_HS200); > > err = mmc_switch_status(card); > - if (err) > + /* > + * For HS200, CRC errors are not a reliable way to know the switch > + * failed. If there really is a problem, we would expect tuning will > + * fail and the result ends up the same. > + */ > + if (err && err != -EILSEQ) I would rather change mmc_switch_status() to take a new parameter, which tells it about ignoring CRC errors. The reason is simply that I think these changes should probably apply to HS400 as well, and then it just gets lots of duplicated code for the same error check. > goto out_err; > > mmc_set_bus_speed(card); > @@ -1387,6 +1392,14 @@ static int mmc_select_hs200(struct mmc_card *card) > > err = mmc_switch_status(card); > /* > + * For HS200, CRC errors are not a reliable way to know the > + * switch failed. If there really is a problem, we would expect > + * tuning will fail and the result ends up the same. > + */ > + if (err == -EILSEQ) > + err = 0; > + > + /* > * mmc_select_timing() assumes timing has not changed if > * it is a switch error. > */ > -- > 1.9.1 > Moreover, and somewhat related to this change: Do we want to change mmc_switch_status() to be able to tell mmc_send_status() about the number of command retries? Currently it defaults to MMC_CMD_RETRIES, but I guess we only want to send *one* CMD13? 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.c b/drivers/mmc/core/mmc.c index 3268fcd3378d..34d30e2a09ff 100644 --- a/drivers/mmc/core/mmc.c +++ b/drivers/mmc/core/mmc.c @@ -1223,7 +1223,12 @@ int mmc_hs400_to_hs200(struct mmc_card *card) mmc_set_timing(host, MMC_TIMING_MMC_HS200); err = mmc_switch_status(card); - if (err) + /* + * For HS200, CRC errors are not a reliable way to know the switch + * failed. If there really is a problem, we would expect tuning will + * fail and the result ends up the same. + */ + if (err && err != -EILSEQ) goto out_err; mmc_set_bus_speed(card); @@ -1387,6 +1392,14 @@ static int mmc_select_hs200(struct mmc_card *card) err = mmc_switch_status(card); /* + * For HS200, CRC errors are not a reliable way to know the + * switch failed. If there really is a problem, we would expect + * tuning will fail and the result ends up the same. + */ + if (err == -EILSEQ) + err = 0; + + /* * mmc_select_timing() assumes timing has not changed if * it is a switch error. */
The JEDEC specification indicates CMD13 can be used after a HS200 switch to check for errors. However in practice some boards experience CRC errors in the CMD13 response. Consequently, for HS200, CRC errors are not a reliable way to know the switch failed. If there really is a problem, we would expect tuning will fail and the result ends up the same. So change the error condition to ignore CRC errors in that case. Signed-off-by: Adrian Hunter <adrian.hunter@intel.com> --- drivers/mmc/core/mmc.c | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-)