| Message ID | 20191114111814.35199-1-yangbo.lu@nxp.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | mmc: sdhci: fix up CMD12 sending | expand |
Hi Uffe and Adrian, Actually I'm not very sure that software CMD12 won't be sent for data error of multiple blocks r/w command with AUTO CMD12. I will appreciate your help to review changes and confirm. Thanks a lot. Best regards, Yangbo Lu > -----Original Message----- > From: Yangbo Lu <yangbo.lu@nxp.com> > Sent: Thursday, November 14, 2019 7:18 PM > To: linux-mmc@vger.kernel.org; Ulf Hansson <ulf.hansson@linaro.org>; Adrian > Hunter <adrian.hunter@intel.com> > Cc: Y.b. Lu <yangbo.lu@nxp.com> > Subject: [PATCH] mmc: sdhci: fix up CMD12 sending > > The STOP command is disabled for multiple blocks r/w commands with auto > CMD12, when start to send. However, if there is data error, software still needs > to send CMD12 according to SD spec. > This patch is to allow software CMD12 sending for this case. > > Signed-off-by: Yangbo Lu <yangbo.lu@nxp.com> > --- > drivers/mmc/host/sdhci.c | 17 +++-------------- > 1 file changed, 3 insertions(+), 14 deletions(-) > > diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c index > 09cdbe8..3041c39 100644 > --- a/drivers/mmc/host/sdhci.c > +++ b/drivers/mmc/host/sdhci.c > @@ -1326,12 +1326,12 @@ static void sdhci_finish_data(struct sdhci_host > *host) > > /* > * Need to send CMD12 if - > - * a) open-ended multiblock transfer (no CMD23) > + * a) open-ended multiblock transfer not using auto CMD12 (no CMD23) > * b) error in multiblock transfer > */ > if (data->stop && > - (data->error || > - !data->mrq->sbc)) { > + ((!data->mrq->sbc && !sdhci_auto_cmd12(host, data->mrq)) || > + data->error)) { > /* > * 'cap_cmd_during_tfr' request must not use the command line > * after mmc_command_done() has been called. It is upper layer's > @@ -1825,17 +1825,6 @@ void sdhci_request(struct mmc_host *mmc, struct > mmc_request *mrq) > > sdhci_led_activate(host); > > - /* > - * Ensure we don't send the STOP for non-SET_BLOCK_COUNTED > - * requests if Auto-CMD12 is enabled. > - */ > - if (sdhci_auto_cmd12(host, mrq)) { > - if (mrq->stop) { > - mrq->data->stop = NULL; > - mrq->stop = NULL; > - } > - } > - > if (!present || host->flags & SDHCI_DEVICE_DEAD) { > mrq->cmd->error = -ENOMEDIUM; > sdhci_finish_mrq(host, mrq); > -- > 2.7.4
On Thu, 14 Nov 2019 at 12:18, Yangbo Lu <yangbo.lu@nxp.com> wrote: > > The STOP command is disabled for multiple blocks r/w commands > with auto CMD12, when start to send. However, if there is data > error, software still needs to send CMD12 according to SD spec. > This patch is to allow software CMD12 sending for this case. > > Signed-off-by: Yangbo Lu <yangbo.lu@nxp.com> > --- > drivers/mmc/host/sdhci.c | 17 +++-------------- > 1 file changed, 3 insertions(+), 14 deletions(-) > > diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c > index 09cdbe8..3041c39 100644 > --- a/drivers/mmc/host/sdhci.c > +++ b/drivers/mmc/host/sdhci.c > @@ -1326,12 +1326,12 @@ static void sdhci_finish_data(struct sdhci_host *host) > > /* > * Need to send CMD12 if - > - * a) open-ended multiblock transfer (no CMD23) > + * a) open-ended multiblock transfer not using auto CMD12 (no CMD23) > * b) error in multiblock transfer > */ > if (data->stop && > - (data->error || > - !data->mrq->sbc)) { > + ((!data->mrq->sbc && !sdhci_auto_cmd12(host, data->mrq)) || > + data->error)) { Per your other reply to this thread, I don't think there is any harm in always sending a CMD12 if there is an error, at least from the card's point of view. The worst thing that can happen is that there are two CMD12 sent to the card and I don't think that is a problem for the error path. My only concern, is to understand if $subject patch causes other changes in behaviours for the SDHCI driver. Let's see what Adrian thinks. > /* > * 'cap_cmd_during_tfr' request must not use the command line > * after mmc_command_done() has been called. It is upper layer's > @@ -1825,17 +1825,6 @@ void sdhci_request(struct mmc_host *mmc, struct mmc_request *mrq) > > sdhci_led_activate(host); > > - /* > - * Ensure we don't send the STOP for non-SET_BLOCK_COUNTED > - * requests if Auto-CMD12 is enabled. > - */ > - if (sdhci_auto_cmd12(host, mrq)) { > - if (mrq->stop) { > - mrq->data->stop = NULL; > - mrq->stop = NULL; > - } > - } In general, I am not very fond of when host drivers change these structures under the hood of the core, which means I like this part. > - > if (!present || host->flags & SDHCI_DEVICE_DEAD) { > mrq->cmd->error = -ENOMEDIUM; > sdhci_finish_mrq(host, mrq); > -- > 2.7.4 > Kind regards Uffe
Hi, > -----Original Message----- > From: Ulf Hansson <ulf.hansson@linaro.org> > Sent: Thursday, November 14, 2019 9:54 PM > To: Y.b. Lu <yangbo.lu@nxp.com> > Cc: linux-mmc@vger.kernel.org; Adrian Hunter <adrian.hunter@intel.com> > Subject: Re: [PATCH] mmc: sdhci: fix up CMD12 sending > > On Thu, 14 Nov 2019 at 12:18, Yangbo Lu <yangbo.lu@nxp.com> wrote: > > > > The STOP command is disabled for multiple blocks r/w commands with > > auto CMD12, when start to send. However, if there is data error, > > software still needs to send CMD12 according to SD spec. > > This patch is to allow software CMD12 sending for this case. > > > > Signed-off-by: Yangbo Lu <yangbo.lu@nxp.com> > > --- > > drivers/mmc/host/sdhci.c | 17 +++-------------- > > 1 file changed, 3 insertions(+), 14 deletions(-) > > > > diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c index > > 09cdbe8..3041c39 100644 > > --- a/drivers/mmc/host/sdhci.c > > +++ b/drivers/mmc/host/sdhci.c > > @@ -1326,12 +1326,12 @@ static void sdhci_finish_data(struct > > sdhci_host *host) > > > > /* > > * Need to send CMD12 if - > > - * a) open-ended multiblock transfer (no CMD23) > > + * a) open-ended multiblock transfer not using auto CMD12 (no > > + CMD23) > > * b) error in multiblock transfer > > */ > > if (data->stop && > > - (data->error || > > - !data->mrq->sbc)) { > > + ((!data->mrq->sbc && !sdhci_auto_cmd12(host, data->mrq)) || > > + data->error)) { > > Per your other reply to this thread, I don't think there is any harm in always > sending a CMD12 if there is an error, at least from the card's point of view. > > The worst thing that can happen is that there are two CMD12 sent to the card > and I don't think that is a problem for the error path. > > My only concern, is to understand if $subject patch causes other changes in > behaviours for the SDHCI driver. Let's see what Adrian thinks. [Y.b. Lu] Yes. The purpose is to avoid no CMD12 sent if get data error. That will makes block driver failed at block r/w recovery when sending CMD13 to get status. Our platform on some boards at old kernel 4.14 seems to hit this case. Hi Adrian, Could you help to review the changes? Thanks. > > > /* > > * 'cap_cmd_during_tfr' request must not use the > command line > > * after mmc_command_done() has been called. It is > > upper layer's @@ -1825,17 +1825,6 @@ void sdhci_request(struct > > mmc_host *mmc, struct mmc_request *mrq) > > > > sdhci_led_activate(host); > > > > - /* > > - * Ensure we don't send the STOP for non-SET_BLOCK_COUNTED > > - * requests if Auto-CMD12 is enabled. > > - */ > > - if (sdhci_auto_cmd12(host, mrq)) { > > - if (mrq->stop) { > > - mrq->data->stop = NULL; > > - mrq->stop = NULL; > > - } > > - } > > In general, I am not very fond of when host drivers change these structures > under the hood of the core, which means I like this part. > > > - > > if (!present || host->flags & SDHCI_DEVICE_DEAD) { > > mrq->cmd->error = -ENOMEDIUM; > > sdhci_finish_mrq(host, mrq); > > -- > > 2.7.4 > > > > Kind regards > Uffe
On 21/11/19 4:21 AM, Y.b. Lu wrote: > Hi, > >> -----Original Message----- >> From: Ulf Hansson <ulf.hansson@linaro.org> >> Sent: Thursday, November 14, 2019 9:54 PM >> To: Y.b. Lu <yangbo.lu@nxp.com> >> Cc: linux-mmc@vger.kernel.org; Adrian Hunter <adrian.hunter@intel.com> >> Subject: Re: [PATCH] mmc: sdhci: fix up CMD12 sending >> >> On Thu, 14 Nov 2019 at 12:18, Yangbo Lu <yangbo.lu@nxp.com> wrote: >>> >>> The STOP command is disabled for multiple blocks r/w commands with >>> auto CMD12, when start to send. However, if there is data error, >>> software still needs to send CMD12 according to SD spec. >>> This patch is to allow software CMD12 sending for this case. >>> >>> Signed-off-by: Yangbo Lu <yangbo.lu@nxp.com> >>> --- >>> drivers/mmc/host/sdhci.c | 17 +++-------------- >>> 1 file changed, 3 insertions(+), 14 deletions(-) >>> >>> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c index >>> 09cdbe8..3041c39 100644 >>> --- a/drivers/mmc/host/sdhci.c >>> +++ b/drivers/mmc/host/sdhci.c >>> @@ -1326,12 +1326,12 @@ static void sdhci_finish_data(struct >>> sdhci_host *host) >>> >>> /* >>> * Need to send CMD12 if - >>> - * a) open-ended multiblock transfer (no CMD23) >>> + * a) open-ended multiblock transfer not using auto CMD12 (no >>> + CMD23) >>> * b) error in multiblock transfer >>> */ >>> if (data->stop && >>> - (data->error || >>> - !data->mrq->sbc)) { >>> + ((!data->mrq->sbc && !sdhci_auto_cmd12(host, data->mrq)) || >>> + data->error)) { >> >> Per your other reply to this thread, I don't think there is any harm in always >> sending a CMD12 if there is an error, at least from the card's point of view. >> >> The worst thing that can happen is that there are two CMD12 sent to the card >> and I don't think that is a problem for the error path. >> >> My only concern, is to understand if $subject patch causes other changes in >> behaviours for the SDHCI driver. Let's see what Adrian thinks. > > [Y.b. Lu] Yes. The purpose is to avoid no CMD12 sent if get data error. That will makes block driver failed at block r/w recovery when sending CMD13 to get status. > Our platform on some boards at old kernel 4.14 seems to hit this case. > > Hi Adrian, > Could you help to review the changes? I don't think the auto-cmd12 error handling was ever done properly, so it will take we a while to review it.
> -----Original Message----- > From: linux-mmc-owner@vger.kernel.org <linux-mmc-owner@vger.kernel.org> > On Behalf Of Adrian Hunter > Sent: Thursday, November 21, 2019 7:36 PM > To: Y.b. Lu <yangbo.lu@nxp.com>; Ulf Hansson <ulf.hansson@linaro.org> > Cc: linux-mmc@vger.kernel.org > Subject: Re: [PATCH] mmc: sdhci: fix up CMD12 sending > > On 21/11/19 4:21 AM, Y.b. Lu wrote: > > Hi, > > > >> -----Original Message----- > >> From: Ulf Hansson <ulf.hansson@linaro.org> > >> Sent: Thursday, November 14, 2019 9:54 PM > >> To: Y.b. Lu <yangbo.lu@nxp.com> > >> Cc: linux-mmc@vger.kernel.org; Adrian Hunter > >> <adrian.hunter@intel.com> > >> Subject: Re: [PATCH] mmc: sdhci: fix up CMD12 sending > >> > >> On Thu, 14 Nov 2019 at 12:18, Yangbo Lu <yangbo.lu@nxp.com> wrote: > >>> > >>> The STOP command is disabled for multiple blocks r/w commands with > >>> auto CMD12, when start to send. However, if there is data error, > >>> software still needs to send CMD12 according to SD spec. > >>> This patch is to allow software CMD12 sending for this case. > >>> > >>> Signed-off-by: Yangbo Lu <yangbo.lu@nxp.com> > >>> --- > >>> drivers/mmc/host/sdhci.c | 17 +++-------------- > >>> 1 file changed, 3 insertions(+), 14 deletions(-) > >>> > >>> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c > >>> index > >>> 09cdbe8..3041c39 100644 > >>> --- a/drivers/mmc/host/sdhci.c > >>> +++ b/drivers/mmc/host/sdhci.c > >>> @@ -1326,12 +1326,12 @@ static void sdhci_finish_data(struct > >>> sdhci_host *host) > >>> > >>> /* > >>> * Need to send CMD12 if - > >>> - * a) open-ended multiblock transfer (no CMD23) > >>> + * a) open-ended multiblock transfer not using auto CMD12 > >>> + (no > >>> + CMD23) > >>> * b) error in multiblock transfer > >>> */ > >>> if (data->stop && > >>> - (data->error || > >>> - !data->mrq->sbc)) { > >>> + ((!data->mrq->sbc && !sdhci_auto_cmd12(host, data->mrq)) > || > >>> + data->error)) { > >> > >> Per your other reply to this thread, I don't think there is any harm > >> in always sending a CMD12 if there is an error, at least from the card's point > of view. > >> > >> The worst thing that can happen is that there are two CMD12 sent to > >> the card and I don't think that is a problem for the error path. > >> > >> My only concern, is to understand if $subject patch causes other > >> changes in behaviours for the SDHCI driver. Let's see what Adrian thinks. > > > > [Y.b. Lu] Yes. The purpose is to avoid no CMD12 sent if get data error. That > will makes block driver failed at block r/w recovery when sending CMD13 to > get status. > > Our platform on some boards at old kernel 4.14 seems to hit this case. > > > > Hi Adrian, > > Could you help to review the changes? > > I don't think the auto-cmd12 error handling was ever done properly, so it will > take we a while to review it. [Y.b. Lu] Thanks Adrian. I am confused where the initial error recovery should start for multi-block r/w (with AUTO CMD12) error. I can see two places which may send CMD12. 1. In sdhci_finish_data(), as my patch changes, I think for data->error, the CMD12 should be sent. 2. In mmc_blk_mq_rw_recovery() in block.c, CMD13 will be sent with one more retry (retuning may happen), before CMD12. And I doubt the recovery 1 if we can still use CC/TC/DTOE interrupts to check CMD12 success or not (or we should just poll DAT0), for data error in tuning mode. Thanks.
On 14/11/19 1:18 pm, Yangbo Lu wrote: > The STOP command is disabled for multiple blocks r/w commands > with auto CMD12, when start to send. However, if there is data > error, software still needs to send CMD12 according to SD spec. > This patch is to allow software CMD12 sending for this case. > > Signed-off-by: Yangbo Lu <yangbo.lu@nxp.com> Sorry for the delay. This looks good to me. Sending a STOP command on the error path in the auto-CMD12 case should be fine whether it has been sent already or not. Acked-by: Adrian Hunter <adrian.hunter@intel.com> > --- > drivers/mmc/host/sdhci.c | 17 +++-------------- > 1 file changed, 3 insertions(+), 14 deletions(-) > > diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c > index 09cdbe8..3041c39 100644 > --- a/drivers/mmc/host/sdhci.c > +++ b/drivers/mmc/host/sdhci.c > @@ -1326,12 +1326,12 @@ static void sdhci_finish_data(struct sdhci_host *host) > > /* > * Need to send CMD12 if - > - * a) open-ended multiblock transfer (no CMD23) > + * a) open-ended multiblock transfer not using auto CMD12 (no CMD23) > * b) error in multiblock transfer > */ > if (data->stop && > - (data->error || > - !data->mrq->sbc)) { > + ((!data->mrq->sbc && !sdhci_auto_cmd12(host, data->mrq)) || > + data->error)) { > /* > * 'cap_cmd_during_tfr' request must not use the command line > * after mmc_command_done() has been called. It is upper layer's > @@ -1825,17 +1825,6 @@ void sdhci_request(struct mmc_host *mmc, struct mmc_request *mrq) > > sdhci_led_activate(host); > > - /* > - * Ensure we don't send the STOP for non-SET_BLOCK_COUNTED > - * requests if Auto-CMD12 is enabled. > - */ > - if (sdhci_auto_cmd12(host, mrq)) { > - if (mrq->stop) { > - mrq->data->stop = NULL; > - mrq->stop = NULL; > - } > - } > - > if (!present || host->flags & SDHCI_DEVICE_DEAD) { > mrq->cmd->error = -ENOMEDIUM; > sdhci_finish_mrq(host, mrq); >
On Thu, 14 Nov 2019 at 12:18, Yangbo Lu <yangbo.lu@nxp.com> wrote: > > The STOP command is disabled for multiple blocks r/w commands > with auto CMD12, when start to send. However, if there is data > error, software still needs to send CMD12 according to SD spec. > This patch is to allow software CMD12 sending for this case. > > Signed-off-by: Yangbo Lu <yangbo.lu@nxp.com> Applied for next, thanks! Let's see how things go, then we can decide whether to add stable tag as well. Kind regards Uffe > --- > drivers/mmc/host/sdhci.c | 17 +++-------------- > 1 file changed, 3 insertions(+), 14 deletions(-) > > diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c > index 09cdbe8..3041c39 100644 > --- a/drivers/mmc/host/sdhci.c > +++ b/drivers/mmc/host/sdhci.c > @@ -1326,12 +1326,12 @@ static void sdhci_finish_data(struct sdhci_host *host) > > /* > * Need to send CMD12 if - > - * a) open-ended multiblock transfer (no CMD23) > + * a) open-ended multiblock transfer not using auto CMD12 (no CMD23) > * b) error in multiblock transfer > */ > if (data->stop && > - (data->error || > - !data->mrq->sbc)) { > + ((!data->mrq->sbc && !sdhci_auto_cmd12(host, data->mrq)) || > + data->error)) { > /* > * 'cap_cmd_during_tfr' request must not use the command line > * after mmc_command_done() has been called. It is upper layer's > @@ -1825,17 +1825,6 @@ void sdhci_request(struct mmc_host *mmc, struct mmc_request *mrq) > > sdhci_led_activate(host); > > - /* > - * Ensure we don't send the STOP for non-SET_BLOCK_COUNTED > - * requests if Auto-CMD12 is enabled. > - */ > - if (sdhci_auto_cmd12(host, mrq)) { > - if (mrq->stop) { > - mrq->data->stop = NULL; > - mrq->stop = NULL; > - } > - } > - > if (!present || host->flags & SDHCI_DEVICE_DEAD) { > mrq->cmd->error = -ENOMEDIUM; > sdhci_finish_mrq(host, mrq); > -- > 2.7.4 >
Hi Uffe and Adrian, Back again on this topic. Actually we are trying to make the error recovery work after data error of CMD18 in linux-4.14. With this patch, when CMD18 data error got, manual CMD12 would be sent. And then went into mmc_blk_cmd_recovery(). (Should be mmc_blk_mq_rw_recovery() in latest linux-5.5-rc2.) In mmc_blk_cmd_recovery(), re-tuning would fail before sending CMD13 on our specific board. This may be some issue related to specific eMMC card we are investigating. The above is just background introduction, and you may not care about that:) I'd like to have some queries on CMD12 usage in MMC driver. 1. It seems CMD12 is always not using ABORT type for sending in sdhci.c. The SDHCI_CMD_ABORTCMD hasn't been used. Is this issue? 2. In block.c, CMD12 uses R1 response for data reading and R1B response for writing. Is it ok to use R1 response for SD? The SD spec mentions only R1B response for CMD12. Appreciate your helps. Thanks. Best regards, Yangbo Lu > -----Original Message----- > From: Ulf Hansson <ulf.hansson@linaro.org> > Sent: Tuesday, December 10, 2019 5:52 PM > To: Y.b. Lu <yangbo.lu@nxp.com> > Cc: linux-mmc@vger.kernel.org; Adrian Hunter <adrian.hunter@intel.com> > Subject: Re: [PATCH] mmc: sdhci: fix up CMD12 sending > > On Thu, 14 Nov 2019 at 12:18, Yangbo Lu <yangbo.lu@nxp.com> wrote: > > > > The STOP command is disabled for multiple blocks r/w commands > > with auto CMD12, when start to send. However, if there is data > > error, software still needs to send CMD12 according to SD spec. > > This patch is to allow software CMD12 sending for this case. > > > > Signed-off-by: Yangbo Lu <yangbo.lu@nxp.com> > > Applied for next, thanks! > > Let's see how things go, then we can decide whether to add stable tag as well. > > Kind regards > Uffe > > > > --- > > drivers/mmc/host/sdhci.c | 17 +++-------------- > > 1 file changed, 3 insertions(+), 14 deletions(-) > > > > diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c > > index 09cdbe8..3041c39 100644 > > --- a/drivers/mmc/host/sdhci.c > > +++ b/drivers/mmc/host/sdhci.c > > @@ -1326,12 +1326,12 @@ static void sdhci_finish_data(struct sdhci_host > *host) > > > > /* > > * Need to send CMD12 if - > > - * a) open-ended multiblock transfer (no CMD23) > > + * a) open-ended multiblock transfer not using auto CMD12 (no > CMD23) > > * b) error in multiblock transfer > > */ > > if (data->stop && > > - (data->error || > > - !data->mrq->sbc)) { > > + ((!data->mrq->sbc && !sdhci_auto_cmd12(host, data->mrq)) || > > + data->error)) { > > /* > > * 'cap_cmd_during_tfr' request must not use the > command line > > * after mmc_command_done() has been called. It is > upper layer's > > @@ -1825,17 +1825,6 @@ void sdhci_request(struct mmc_host *mmc, > struct mmc_request *mrq) > > > > sdhci_led_activate(host); > > > > - /* > > - * Ensure we don't send the STOP for non-SET_BLOCK_COUNTED > > - * requests if Auto-CMD12 is enabled. > > - */ > > - if (sdhci_auto_cmd12(host, mrq)) { > > - if (mrq->stop) { > > - mrq->data->stop = NULL; > > - mrq->stop = NULL; > > - } > > - } > > - > > if (!present || host->flags & SDHCI_DEVICE_DEAD) { > > mrq->cmd->error = -ENOMEDIUM; > > sdhci_finish_mrq(host, mrq); > > -- > > 2.7.4 > >
On Wed, 8 Jan 2020 at 10:37, Y.b. Lu <yangbo.lu@nxp.com> wrote: > > Hi Uffe and Adrian, > > Back again on this topic. Actually we are trying to make the error recovery work after data error of CMD18 in linux-4.14. > With this patch, when CMD18 data error got, manual CMD12 would be sent. And then went into mmc_blk_cmd_recovery(). (Should be mmc_blk_mq_rw_recovery() in latest linux-5.5-rc2.) > In mmc_blk_cmd_recovery(), re-tuning would fail before sending CMD13 on our specific board. > This may be some issue related to specific eMMC card we are investigating. > > The above is just background introduction, and you may not care about that:) > I'd like to have some queries on CMD12 usage in MMC driver. > 1. It seems CMD12 is always not using ABORT type for sending in sdhci.c. The SDHCI_CMD_ABORTCMD hasn't been used. Is this issue? I defer that question to Adrian. > 2. In block.c, CMD12 uses R1 response for data reading and R1B response for writing. Is it ok to use R1 response for SD? The SD spec mentions only R1B response for CMD12. I think the specs isn't that clear on this. In this case, the R1B, is an R1 with an *optional* busy signaling on DAT0, unless it has been changed lately. Additionally, as far as can tell, there have been no reports about problems with the current approach for "reads". Are you saying there is? [...] Kind regards Uffe
On 16/01/20 5:36 pm, Ulf Hansson wrote: > On Wed, 8 Jan 2020 at 10:37, Y.b. Lu <yangbo.lu@nxp.com> wrote: >> >> Hi Uffe and Adrian, >> >> Back again on this topic. Actually we are trying to make the error recovery work after data error of CMD18 in linux-4.14. >> With this patch, when CMD18 data error got, manual CMD12 would be sent. And then went into mmc_blk_cmd_recovery(). (Should be mmc_blk_mq_rw_recovery() in latest linux-5.5-rc2.) >> In mmc_blk_cmd_recovery(), re-tuning would fail before sending CMD13 on our specific board. >> This may be some issue related to specific eMMC card we are investigating. >> >> The above is just background introduction, and you may not care about that:) >> I'd like to have some queries on CMD12 usage in MMC driver. >> 1. It seems CMD12 is always not using ABORT type for sending in sdhci.c. The SDHCI_CMD_ABORTCMD hasn't been used. Is this issue? > AFAIK it is not an issue, but support for it can be added if you need it. > >> 2. In block.c, CMD12 uses R1 response for data reading and R1B response for writing. Is it ok to use R1 response for SD? The SD spec mentions only R1B response for CMD12. > > I think the specs isn't that clear on this. In this case, the R1B, is > an R1 with an *optional* busy signaling on DAT0, unless it has been > changed lately. Also SDHCI offers SDHCI_QUIRK2_STOP_WITH_TC to turn all CMD12 responses into R1B > > Additionally, as far as can tell, there have been no reports about > problems with the current approach for "reads". Are you saying there > is? > > [...] > > Kind regards > Uffe >
diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c index 09cdbe8..3041c39 100644 --- a/drivers/mmc/host/sdhci.c +++ b/drivers/mmc/host/sdhci.c @@ -1326,12 +1326,12 @@ static void sdhci_finish_data(struct sdhci_host *host) /* * Need to send CMD12 if - - * a) open-ended multiblock transfer (no CMD23) + * a) open-ended multiblock transfer not using auto CMD12 (no CMD23) * b) error in multiblock transfer */ if (data->stop && - (data->error || - !data->mrq->sbc)) { + ((!data->mrq->sbc && !sdhci_auto_cmd12(host, data->mrq)) || + data->error)) { /* * 'cap_cmd_during_tfr' request must not use the command line * after mmc_command_done() has been called. It is upper layer's @@ -1825,17 +1825,6 @@ void sdhci_request(struct mmc_host *mmc, struct mmc_request *mrq) sdhci_led_activate(host); - /* - * Ensure we don't send the STOP for non-SET_BLOCK_COUNTED - * requests if Auto-CMD12 is enabled. - */ - if (sdhci_auto_cmd12(host, mrq)) { - if (mrq->stop) { - mrq->data->stop = NULL; - mrq->stop = NULL; - } - } - if (!present || host->flags & SDHCI_DEVICE_DEAD) { mrq->cmd->error = -ENOMEDIUM; sdhci_finish_mrq(host, mrq);
The STOP command is disabled for multiple blocks r/w commands with auto CMD12, when start to send. However, if there is data error, software still needs to send CMD12 according to SD spec. This patch is to allow software CMD12 sending for this case. Signed-off-by: Yangbo Lu <yangbo.lu@nxp.com> --- drivers/mmc/host/sdhci.c | 17 +++-------------- 1 file changed, 3 insertions(+), 14 deletions(-)