Linux-mmc Archive on lore.kernel.org
 help / color / Atom feed
* Re: [PATCH] mmc: sdhci: fix up CMD12 sending
       [not found] <20191114111814.35199-1-yangbo.lu@nxp.com>
@ 2019-11-29  9:17 ` Adrian Hunter
  2019-12-10  9:51 ` Ulf Hansson
  1 sibling, 0 replies; 5+ messages in thread
From: Adrian Hunter @ 2019-11-29  9:17 UTC (permalink / raw)
  To: Yangbo Lu, linux-mmc, Ulf Hansson

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);
> 


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] mmc: sdhci: fix up CMD12 sending
       [not found] <20191114111814.35199-1-yangbo.lu@nxp.com>
  2019-11-29  9:17 ` [PATCH] mmc: sdhci: fix up CMD12 sending Adrian Hunter
@ 2019-12-10  9:51 ` Ulf Hansson
  2020-01-08  9:37   ` Y.b. Lu
  1 sibling, 1 reply; 5+ messages in thread
From: Ulf Hansson @ 2019-12-10  9:51 UTC (permalink / raw)
  To: Yangbo Lu; +Cc: linux-mmc, Adrian Hunter

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
>

^ permalink raw reply	[flat|nested] 5+ messages in thread

* RE: [PATCH] mmc: sdhci: fix up CMD12 sending
  2019-12-10  9:51 ` Ulf Hansson
@ 2020-01-08  9:37   ` Y.b. Lu
  2020-01-16 15:36     ` Ulf Hansson
  0 siblings, 1 reply; 5+ messages in thread
From: Y.b. Lu @ 2020-01-08  9:37 UTC (permalink / raw)
  To: Ulf Hansson, Adrian Hunter; +Cc: linux-mmc, Sachin Miglani

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
> >

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] mmc: sdhci: fix up CMD12 sending
  2020-01-08  9:37   ` Y.b. Lu
@ 2020-01-16 15:36     ` Ulf Hansson
  2020-01-17 14:15       ` Adrian Hunter
  0 siblings, 1 reply; 5+ messages in thread
From: Ulf Hansson @ 2020-01-16 15:36 UTC (permalink / raw)
  To: Y.b. Lu; +Cc: Adrian Hunter, linux-mmc, Sachin Miglani

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

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] mmc: sdhci: fix up CMD12 sending
  2020-01-16 15:36     ` Ulf Hansson
@ 2020-01-17 14:15       ` Adrian Hunter
  0 siblings, 0 replies; 5+ messages in thread
From: Adrian Hunter @ 2020-01-17 14:15 UTC (permalink / raw)
  To: Ulf Hansson, Y.b. Lu; +Cc: linux-mmc, Sachin Miglani

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
> 


^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, back to index

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <20191114111814.35199-1-yangbo.lu@nxp.com>
2019-11-29  9:17 ` [PATCH] mmc: sdhci: fix up CMD12 sending Adrian Hunter
2019-12-10  9:51 ` Ulf Hansson
2020-01-08  9:37   ` Y.b. Lu
2020-01-16 15:36     ` Ulf Hansson
2020-01-17 14:15       ` Adrian Hunter

Linux-mmc Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-mmc/0 linux-mmc/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-mmc linux-mmc/ https://lore.kernel.org/linux-mmc \
		linux-mmc@vger.kernel.org
	public-inbox-index linux-mmc

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-mmc


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git