mmc: mmci: Send a CMD12 to clear the DPSM at errors

Message ID 20190129143556.32705-1-ulf.hansson@linaro.org
State New
Headers show
Series
  • mmc: mmci: Send a CMD12 to clear the DPSM at errors
Related show

Commit Message

Ulf Hansson Jan. 29, 2019, 2:35 p.m.
The current approach with sending a CMD12 (STOP_TRANSMISSION) to complete a
data transfer request, either because of using the open-ended transmission
type or because of receiving an error during a pre-defined data transfer,
isn't sufficient for the STM32 sdmmc variant. More precisely, this variant
needs to clear the DPSM ("Data Path State Machine") by sending a CMD12, for
all failing ADTC commands.

Support this, by adding a struct mmc_command inside the struct mmci_host
and initialize it to a CMD12 during ->probe(). Let's also add checks for
the new conditions, to enable mmci_data_irq() and mmci_cmd_irq() to
postpone the calls to mmci_request_end(), but instead send the CMD12.

Cc: Ludovic Barre <ludovic.barre@st.com>
Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>

---
 drivers/mmc/host/mmci.c | 25 +++++++++++++++++++++++--
 drivers/mmc/host/mmci.h |  1 +
 2 files changed, 24 insertions(+), 2 deletions(-)

-- 
2.17.1

Comments

Ludovic BARRE Feb. 4, 2019, 2:17 p.m. | #1
hi Ulf

One comment below in mmci_cmd_irq function

when I rebase my next serie about "busy detect" I've a kernel panic
null pointer, could you give me some time to investigate this issue
before apply this patch.


On 1/29/19 3:35 PM, Ulf Hansson wrote:
> The current approach with sending a CMD12 (STOP_TRANSMISSION) to complete a

> data transfer request, either because of using the open-ended transmission

> type or because of receiving an error during a pre-defined data transfer,

> isn't sufficient for the STM32 sdmmc variant. More precisely, this variant

> needs to clear the DPSM ("Data Path State Machine") by sending a CMD12, for

> all failing ADTC commands.

> 

> Support this, by adding a struct mmc_command inside the struct mmci_host

> and initialize it to a CMD12 during ->probe(). Let's also add checks for

> the new conditions, to enable mmci_data_irq() and mmci_cmd_irq() to

> postpone the calls to mmci_request_end(), but instead send the CMD12.

> 

> Cc: Ludovic Barre <ludovic.barre@st.com>

> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>

> ---

>   drivers/mmc/host/mmci.c | 25 +++++++++++++++++++++++--

>   drivers/mmc/host/mmci.h |  1 +

>   2 files changed, 24 insertions(+), 2 deletions(-)

> 

> diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c

> index e352f5ad5801..45b97b41601c 100644

> --- a/drivers/mmc/host/mmci.c

> +++ b/drivers/mmc/host/mmci.c

> @@ -1127,6 +1127,12 @@ mmci_start_command(struct mmci_host *host, struct mmc_command *cmd, u32 c)

>   	writel(c, base + MMCICOMMAND);

>   }

>   

> +static void mmci_stop_command(struct mmci_host *host)

> +{

> +	host->stop_abort.error = 0;

> +	mmci_start_command(host, &host->stop_abort, 0);

> +}

> +

>   static void

>   mmci_data_irq(struct mmci_host *host, struct mmc_data *data,

>   	      unsigned int status)

> @@ -1196,10 +1202,16 @@ mmci_data_irq(struct mmci_host *host, struct mmc_data *data,

>   			/* The error clause is handled above, success! */

>   			data->bytes_xfered = data->blksz * data->blocks;

>   

> -		if (!data->stop || (host->mrq->sbc && !data->error))

> +		if (!data->stop) {

> +			if (host->variant->cmdreg_stop && data->error)

> +				mmci_stop_command(host);

> +			else

> +				mmci_request_end(host, data->mrq);

> +		} else if (host->mrq->sbc && !data->error) {

>   			mmci_request_end(host, data->mrq);

> -		else

> +		} else {

>   			mmci_start_command(host, data->stop, 0);

> +		}

>   	}

>   }

>   

> @@ -1298,6 +1310,10 @@ mmci_cmd_irq(struct mmci_host *host, struct mmc_command *cmd,

>   			mmci_dma_error(host);

>   

>   			mmci_stop_data(host);

> +			if (host->variant->cmdreg_stop && cmd->error) {

> +				mmci_stop_command(host);

> +				return;

> +			}


>   		}


In fact a cmd without host->data could fail with DPSM enabled.
example:
sent a cmd12 with MMC_RSP_BUSY flag (R1B command) and so no data.
In sdmmc variant the MMCIDATATIMER is used to busy timeout.
The DPSM is used to sample D0 line and is activated.
the DTIMEOUT flag is set after when no end of busy received before the 
timeout. (DPSM stays in BUSY and wait for Abort)

So The mmci_stop_command may be sent when there are cmd->error  and 
status & MCI_DATATIMEOUT


>   		mmci_request_end(host, host->mrq);

>   	} else if (sbc) {

> @@ -1956,6 +1972,11 @@ static int mmci_probe(struct amba_device *dev,

>   		mmc->max_busy_timeout = 0;

>   	}

>   

> +	/* Prepare a CMD12 - needed to clear the DPSM on some variants. */

> +	host->stop_abort.opcode = MMC_STOP_TRANSMISSION;

> +	host->stop_abort.arg = 0;

> +	host->stop_abort.flags = MMC_RSP_R1B | MMC_CMD_AC;

> +

>   	mmc->ops = &mmci_ops;

>   

>   	/* We support these PM capabilities. */

> diff --git a/drivers/mmc/host/mmci.h b/drivers/mmc/host/mmci.h

> index 24229097d05c..14df81054438 100644

> --- a/drivers/mmc/host/mmci.h

> +++ b/drivers/mmc/host/mmci.h

> @@ -377,6 +377,7 @@ struct mmci_host {

>   	void __iomem		*base;

>   	struct mmc_request	*mrq;

>   	struct mmc_command	*cmd;

> +	struct mmc_command	stop_abort;

>   	struct mmc_data		*data;

>   	struct mmc_host		*mmc;

>   	struct clk		*clk;

>
Ulf Hansson Feb. 4, 2019, 3:39 p.m. | #2
On Mon, 4 Feb 2019 at 15:18, Ludovic BARRE <ludovic.barre@st.com> wrote:
>

> hi Ulf

>

> One comment below in mmci_cmd_irq function

>

> when I rebase my next serie about "busy detect" I've a kernel panic

> null pointer, could you give me some time to investigate this issue

> before apply this patch.


Sure!

FYI, I have tested this on a ux500 and didn't observe any regressions.

>

>

> On 1/29/19 3:35 PM, Ulf Hansson wrote:

> > The current approach with sending a CMD12 (STOP_TRANSMISSION) to complete a

> > data transfer request, either because of using the open-ended transmission

> > type or because of receiving an error during a pre-defined data transfer,

> > isn't sufficient for the STM32 sdmmc variant. More precisely, this variant

> > needs to clear the DPSM ("Data Path State Machine") by sending a CMD12, for

> > all failing ADTC commands.

> >

> > Support this, by adding a struct mmc_command inside the struct mmci_host

> > and initialize it to a CMD12 during ->probe(). Let's also add checks for

> > the new conditions, to enable mmci_data_irq() and mmci_cmd_irq() to

> > postpone the calls to mmci_request_end(), but instead send the CMD12.

> >

> > Cc: Ludovic Barre <ludovic.barre@st.com>

> > Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>

> > ---

> >   drivers/mmc/host/mmci.c | 25 +++++++++++++++++++++++--

> >   drivers/mmc/host/mmci.h |  1 +

> >   2 files changed, 24 insertions(+), 2 deletions(-)

> >

> > diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c

> > index e352f5ad5801..45b97b41601c 100644

> > --- a/drivers/mmc/host/mmci.c

> > +++ b/drivers/mmc/host/mmci.c

> > @@ -1127,6 +1127,12 @@ mmci_start_command(struct mmci_host *host, struct mmc_command *cmd, u32 c)

> >       writel(c, base + MMCICOMMAND);

> >   }

> >

> > +static void mmci_stop_command(struct mmci_host *host)

> > +{

> > +     host->stop_abort.error = 0;

> > +     mmci_start_command(host, &host->stop_abort, 0);

> > +}

> > +

> >   static void

> >   mmci_data_irq(struct mmci_host *host, struct mmc_data *data,

> >             unsigned int status)

> > @@ -1196,10 +1202,16 @@ mmci_data_irq(struct mmci_host *host, struct mmc_data *data,

> >                       /* The error clause is handled above, success! */

> >                       data->bytes_xfered = data->blksz * data->blocks;

> >

> > -             if (!data->stop || (host->mrq->sbc && !data->error))

> > +             if (!data->stop) {

> > +                     if (host->variant->cmdreg_stop && data->error)

> > +                             mmci_stop_command(host);

> > +                     else

> > +                             mmci_request_end(host, data->mrq);

> > +             } else if (host->mrq->sbc && !data->error) {

> >                       mmci_request_end(host, data->mrq);

> > -             else

> > +             } else {

> >                       mmci_start_command(host, data->stop, 0);

> > +             }

> >       }

> >   }

> >

> > @@ -1298,6 +1310,10 @@ mmci_cmd_irq(struct mmci_host *host, struct mmc_command *cmd,

> >                       mmci_dma_error(host);

> >

> >                       mmci_stop_data(host);

> > +                     if (host->variant->cmdreg_stop && cmd->error) {

> > +                             mmci_stop_command(host);

> > +                             return;

> > +                     }

>

> >               }

>

> In fact a cmd without host->data could fail with DPSM enabled.

> example:

> sent a cmd12 with MMC_RSP_BUSY flag (R1B command) and so no data.

> In sdmmc variant the MMCIDATATIMER is used to busy timeout.

> The DPSM is used to sample D0 line and is activated.

> the DTIMEOUT flag is set after when no end of busy received before the

> timeout. (DPSM stays in BUSY and wait for Abort)


Oh, that I didn't know about! That make it quite complicated, I am afraid.

To send a new CMD12 to clear the DPSM, because the previous CDM12
failed seems just wrong. Why would the second CMD12 succeed and not
timeout again, if you see what I mean.

There are also other commands using R1B, that doesn't involve
transferring data. The CMD5 (sleep) for example. If any of these
commands would timeout, then why would a following CMD12 not do so as
well?

To me, it seems like a reset of the controller is the only really sane
way of treating these scenarios, isn't it? Of course, it means we need
to be able to restore the state of the controller after such a reset
as well. Is this possible?

>

> So The mmci_stop_command may be sent when there are cmd->error  and

> status & MCI_DATATIMEOUT

>

>

> >               mmci_request_end(host, host->mrq);

> >       } else if (sbc) {

> > @@ -1956,6 +1972,11 @@ static int mmci_probe(struct amba_device *dev,

> >               mmc->max_busy_timeout = 0;

> >       }

> >

> > +     /* Prepare a CMD12 - needed to clear the DPSM on some variants. */

> > +     host->stop_abort.opcode = MMC_STOP_TRANSMISSION;

> > +     host->stop_abort.arg = 0;

> > +     host->stop_abort.flags = MMC_RSP_R1B | MMC_CMD_AC;

> > +

> >       mmc->ops = &mmci_ops;

> >

> >       /* We support these PM capabilities. */

> > diff --git a/drivers/mmc/host/mmci.h b/drivers/mmc/host/mmci.h

> > index 24229097d05c..14df81054438 100644

> > --- a/drivers/mmc/host/mmci.h

> > +++ b/drivers/mmc/host/mmci.h

> > @@ -377,6 +377,7 @@ struct mmci_host {

> >       void __iomem            *base;

> >       struct mmc_request      *mrq;

> >       struct mmc_command      *cmd;

> > +     struct mmc_command      stop_abort;

> >       struct mmc_data         *data;

> >       struct mmc_host         *mmc;

> >       struct clk              *clk;

> >


Kind regards
Uffe
Ludovic BARRE Feb. 20, 2019, 10:27 a.m. | #3
hi Ulf

On 2/4/19 4:39 PM, Ulf Hansson wrote:
> On Mon, 4 Feb 2019 at 15:18, Ludovic BARRE <ludovic.barre@st.com> wrote:

>>

>> hi Ulf

>>

>> One comment below in mmci_cmd_irq function

>>

>> when I rebase my next serie about "busy detect" I've a kernel panic

>> null pointer, could you give me some time to investigate this issue

>> before apply this patch.

> 

> Sure!

> 

> FYI, I have tested this on a ux500 and didn't observe any regressions.


great

> 

>>

>>

>> On 1/29/19 3:35 PM, Ulf Hansson wrote:

>>> The current approach with sending a CMD12 (STOP_TRANSMISSION) to complete a

>>> data transfer request, either because of using the open-ended transmission

>>> type or because of receiving an error during a pre-defined data transfer,

>>> isn't sufficient for the STM32 sdmmc variant. More precisely, this variant

>>> needs to clear the DPSM ("Data Path State Machine") by sending a CMD12, for

>>> all failing ADTC commands.

>>>

>>> Support this, by adding a struct mmc_command inside the struct mmci_host

>>> and initialize it to a CMD12 during ->probe(). Let's also add checks for

>>> the new conditions, to enable mmci_data_irq() and mmci_cmd_irq() to

>>> postpone the calls to mmci_request_end(), but instead send the CMD12.

>>>

>>> Cc: Ludovic Barre <ludovic.barre@st.com>

>>> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>

>>> ---

>>>    drivers/mmc/host/mmci.c | 25 +++++++++++++++++++++++--

>>>    drivers/mmc/host/mmci.h |  1 +

>>>    2 files changed, 24 insertions(+), 2 deletions(-)

>>>

>>> diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c

>>> index e352f5ad5801..45b97b41601c 100644

>>> --- a/drivers/mmc/host/mmci.c

>>> +++ b/drivers/mmc/host/mmci.c

>>> @@ -1127,6 +1127,12 @@ mmci_start_command(struct mmci_host *host, struct mmc_command *cmd, u32 c)

>>>        writel(c, base + MMCICOMMAND);

>>>    }

>>>

>>> +static void mmci_stop_command(struct mmci_host *host)

>>> +{

>>> +     host->stop_abort.error = 0;

>>> +     mmci_start_command(host, &host->stop_abort, 0);

>>> +}

>>> +

>>>    static void

>>>    mmci_data_irq(struct mmci_host *host, struct mmc_data *data,

>>>              unsigned int status)

>>> @@ -1196,10 +1202,16 @@ mmci_data_irq(struct mmci_host *host, struct mmc_data *data,

>>>                        /* The error clause is handled above, success! */

>>>                        data->bytes_xfered = data->blksz * data->blocks;

>>>

>>> -             if (!data->stop || (host->mrq->sbc && !data->error))

>>> +             if (!data->stop) {

>>> +                     if (host->variant->cmdreg_stop && data->error)

>>> +                             mmci_stop_command(host);

>>> +                     else

>>> +                             mmci_request_end(host, data->mrq);

>>> +             } else if (host->mrq->sbc && !data->error) {

>>>                        mmci_request_end(host, data->mrq);

>>> -             else

>>> +             } else {

>>>                        mmci_start_command(host, data->stop, 0);

>>> +             }

>>>        }

>>>    }

>>>

>>> @@ -1298,6 +1310,10 @@ mmci_cmd_irq(struct mmci_host *host, struct mmc_command *cmd,

>>>                        mmci_dma_error(host);

>>>

>>>                        mmci_stop_data(host);

>>> +                     if (host->variant->cmdreg_stop && cmd->error) {

>>> +                             mmci_stop_command(host);

>>> +                             return;

>>> +                     }

>>

>>>                }

>>

>> In fact a cmd without host->data could fail with DPSM enabled.

>> example:

>> sent a cmd12 with MMC_RSP_BUSY flag (R1B command) and so no data.

>> In sdmmc variant the MMCIDATATIMER is used to busy timeout.

>> The DPSM is used to sample D0 line and is activated.

>> the DTIMEOUT flag is set after when no end of busy received before the

>> timeout. (DPSM stays in BUSY and wait for Abort)

> 

> Oh, that I didn't know about! That make it quite complicated, I am afraid.

> 

> To send a new CMD12 to clear the DPSM, because the previous CDM12

> failed seems just wrong. Why would the second CMD12 succeed and not

> timeout again, if you see what I mean.

> 

> There are also other commands using R1B, that doesn't involve

> transferring data. The CMD5 (sleep) for example. If any of these

> commands would timeout, then why would a following CMD12 not do so as

> well?

> 

> To me, it seems like a reset of the controller is the only really sane

> way of treating these scenarios, isn't it? Of course, it means we need

> to be able to restore the state of the controller after such a reset

> as well. Is this possible?

> 


POV sdmmc hardware block the second cmd12 just allows to clear the DPSM
(for stop abort cmd the R1B may not be set).
I agree with you, if a command fail due to R1B (busy timeout) the better
way is to reset the controller, protocol...

Do to that we could call mmc_hw_reset function
(drivers/mmc/core/core.c). However this function can't be call
in irq context and ongoing request must be completed.

proposition:
The host driver could set a error value (in cmd->error or data->error)
like EDEADLK or ECONNRESET (to be defined)...
The framework could check error value (for example in
mmc_wait_for_req_done) and call mmc_hw_reset if needed.

What do you think?

>>

>> So The mmci_stop_command may be sent when there are cmd->error  and

>> status & MCI_DATATIMEOUT

>>

>>

>>>                mmci_request_end(host, host->mrq);

>>>        } else if (sbc) {

>>> @@ -1956,6 +1972,11 @@ static int mmci_probe(struct amba_device *dev,

>>>                mmc->max_busy_timeout = 0;

>>>        }

>>>

>>> +     /* Prepare a CMD12 - needed to clear the DPSM on some variants. */

>>> +     host->stop_abort.opcode = MMC_STOP_TRANSMISSION;

>>> +     host->stop_abort.arg = 0;

>>> +     host->stop_abort.flags = MMC_RSP_R1B | MMC_CMD_AC;

>>> +

>>>        mmc->ops = &mmci_ops;

>>>

>>>        /* We support these PM capabilities. */

>>> diff --git a/drivers/mmc/host/mmci.h b/drivers/mmc/host/mmci.h

>>> index 24229097d05c..14df81054438 100644

>>> --- a/drivers/mmc/host/mmci.h

>>> +++ b/drivers/mmc/host/mmci.h

>>> @@ -377,6 +377,7 @@ struct mmci_host {

>>>        void __iomem            *base;

>>>        struct mmc_request      *mrq;

>>>        struct mmc_command      *cmd;

>>> +     struct mmc_command      stop_abort;

>>>        struct mmc_data         *data;

>>>        struct mmc_host         *mmc;

>>>        struct clk              *clk;

>>>

> 

> Kind regards

> Uffe

>
Ludovic BARRE Feb. 20, 2019, 5:12 p.m. | #4
hi Ulf

I think you can apply this patch.
The R1B problem (due to busy timeout) can be seen in a future series
about busy detect.
Thanks

Tested-by: Ludovic Barre <ludovic.barre@st.com>


Regards
Ludo

On 2/20/19 11:27 AM, Ludovic BARRE wrote:
> hi Ulf

> 

> On 2/4/19 4:39 PM, Ulf Hansson wrote:

>> On Mon, 4 Feb 2019 at 15:18, Ludovic BARRE <ludovic.barre@st.com> wrote:

>>>

>>> hi Ulf

>>>

>>> One comment below in mmci_cmd_irq function

>>>

>>> when I rebase my next serie about "busy detect" I've a kernel panic

>>> null pointer, could you give me some time to investigate this issue

>>> before apply this patch.

>>

>> Sure!

>>

>> FYI, I have tested this on a ux500 and didn't observe any regressions.

> 

> great

> 

>>

>>>

>>>

>>> On 1/29/19 3:35 PM, Ulf Hansson wrote:

>>>> The current approach with sending a CMD12 (STOP_TRANSMISSION) to 

>>>> complete a

>>>> data transfer request, either because of using the open-ended 

>>>> transmission

>>>> type or because of receiving an error during a pre-defined data 

>>>> transfer,

>>>> isn't sufficient for the STM32 sdmmc variant. More precisely, this 

>>>> variant

>>>> needs to clear the DPSM ("Data Path State Machine") by sending a 

>>>> CMD12, for

>>>> all failing ADTC commands.

>>>>

>>>> Support this, by adding a struct mmc_command inside the struct 

>>>> mmci_host

>>>> and initialize it to a CMD12 during ->probe(). Let's also add checks 

>>>> for

>>>> the new conditions, to enable mmci_data_irq() and mmci_cmd_irq() to

>>>> postpone the calls to mmci_request_end(), but instead send the CMD12.

>>>>

>>>> Cc: Ludovic Barre <ludovic.barre@st.com>

>>>> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>

>>>> ---

>>>>    drivers/mmc/host/mmci.c | 25 +++++++++++++++++++++++--

>>>>    drivers/mmc/host/mmci.h |  1 +

>>>>    2 files changed, 24 insertions(+), 2 deletions(-)

>>>>

>>>> diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c

>>>> index e352f5ad5801..45b97b41601c 100644

>>>> --- a/drivers/mmc/host/mmci.c

>>>> +++ b/drivers/mmc/host/mmci.c

>>>> @@ -1127,6 +1127,12 @@ mmci_start_command(struct mmci_host *host, 

>>>> struct mmc_command *cmd, u32 c)

>>>>        writel(c, base + MMCICOMMAND);

>>>>    }

>>>>

>>>> +static void mmci_stop_command(struct mmci_host *host)

>>>> +{

>>>> +     host->stop_abort.error = 0;

>>>> +     mmci_start_command(host, &host->stop_abort, 0);

>>>> +}

>>>> +

>>>>    static void

>>>>    mmci_data_irq(struct mmci_host *host, struct mmc_data *data,

>>>>              unsigned int status)

>>>> @@ -1196,10 +1202,16 @@ mmci_data_irq(struct mmci_host *host, struct 

>>>> mmc_data *data,

>>>>                        /* The error clause is handled above, 

>>>> success! */

>>>>                        data->bytes_xfered = data->blksz * data->blocks;

>>>>

>>>> -             if (!data->stop || (host->mrq->sbc && !data->error))

>>>> +             if (!data->stop) {

>>>> +                     if (host->variant->cmdreg_stop && data->error)

>>>> +                             mmci_stop_command(host);

>>>> +                     else

>>>> +                             mmci_request_end(host, data->mrq);

>>>> +             } else if (host->mrq->sbc && !data->error) {

>>>>                        mmci_request_end(host, data->mrq);

>>>> -             else

>>>> +             } else {

>>>>                        mmci_start_command(host, data->stop, 0);

>>>> +             }

>>>>        }

>>>>    }

>>>>

>>>> @@ -1298,6 +1310,10 @@ mmci_cmd_irq(struct mmci_host *host, struct 

>>>> mmc_command *cmd,

>>>>                        mmci_dma_error(host);

>>>>

>>>>                        mmci_stop_data(host);

>>>> +                     if (host->variant->cmdreg_stop && cmd->error) {

>>>> +                             mmci_stop_command(host);

>>>> +                             return;

>>>> +                     }

>>>

>>>>                }

>>>

>>> In fact a cmd without host->data could fail with DPSM enabled.

>>> example:

>>> sent a cmd12 with MMC_RSP_BUSY flag (R1B command) and so no data.

>>> In sdmmc variant the MMCIDATATIMER is used to busy timeout.

>>> The DPSM is used to sample D0 line and is activated.

>>> the DTIMEOUT flag is set after when no end of busy received before the

>>> timeout. (DPSM stays in BUSY and wait for Abort)

>>

>> Oh, that I didn't know about! That make it quite complicated, I am 

>> afraid.

>>

>> To send a new CMD12 to clear the DPSM, because the previous CDM12

>> failed seems just wrong. Why would the second CMD12 succeed and not

>> timeout again, if you see what I mean.

>>

>> There are also other commands using R1B, that doesn't involve

>> transferring data. The CMD5 (sleep) for example. If any of these

>> commands would timeout, then why would a following CMD12 not do so as

>> well?

>>

>> To me, it seems like a reset of the controller is the only really sane

>> way of treating these scenarios, isn't it? Of course, it means we need

>> to be able to restore the state of the controller after such a reset

>> as well. Is this possible?

>>

> 

> POV sdmmc hardware block the second cmd12 just allows to clear the DPSM

> (for stop abort cmd the R1B may not be set).

> I agree with you, if a command fail due to R1B (busy timeout) the better

> way is to reset the controller, protocol...

> 

> Do to that we could call mmc_hw_reset function

> (drivers/mmc/core/core.c). However this function can't be call

> in irq context and ongoing request must be completed.

> 

> proposition:

> The host driver could set a error value (in cmd->error or data->error)

> like EDEADLK or ECONNRESET (to be defined)...

> The framework could check error value (for example in

> mmc_wait_for_req_done) and call mmc_hw_reset if needed.

> 

> What do you think?

> 

>>>

>>> So The mmci_stop_command may be sent when there are cmd->error  and

>>> status & MCI_DATATIMEOUT

>>>

>>>

>>>>                mmci_request_end(host, host->mrq);

>>>>        } else if (sbc) {

>>>> @@ -1956,6 +1972,11 @@ static int mmci_probe(struct amba_device *dev,

>>>>                mmc->max_busy_timeout = 0;

>>>>        }

>>>>

>>>> +     /* Prepare a CMD12 - needed to clear the DPSM on some 

>>>> variants. */

>>>> +     host->stop_abort.opcode = MMC_STOP_TRANSMISSION;

>>>> +     host->stop_abort.arg = 0;

>>>> +     host->stop_abort.flags = MMC_RSP_R1B | MMC_CMD_AC;

>>>> +

>>>>        mmc->ops = &mmci_ops;

>>>>

>>>>        /* We support these PM capabilities. */

>>>> diff --git a/drivers/mmc/host/mmci.h b/drivers/mmc/host/mmci.h

>>>> index 24229097d05c..14df81054438 100644

>>>> --- a/drivers/mmc/host/mmci.h

>>>> +++ b/drivers/mmc/host/mmci.h

>>>> @@ -377,6 +377,7 @@ struct mmci_host {

>>>>        void __iomem            *base;

>>>>        struct mmc_request      *mrq;

>>>>        struct mmc_command      *cmd;

>>>> +     struct mmc_command      stop_abort;

>>>>        struct mmc_data         *data;

>>>>        struct mmc_host         *mmc;

>>>>        struct clk              *clk;

>>>>

>>

>> Kind regards

>> Uffe

>>
Ulf Hansson Feb. 22, 2019, 10:15 a.m. | #5
[...]

> >>>

> >>> @@ -1298,6 +1310,10 @@ mmci_cmd_irq(struct mmci_host *host, struct mmc_command *cmd,

> >>>                        mmci_dma_error(host);

> >>>

> >>>                        mmci_stop_data(host);

> >>> +                     if (host->variant->cmdreg_stop && cmd->error) {

> >>> +                             mmci_stop_command(host);

> >>> +                             return;

> >>> +                     }

> >>

> >>>                }

> >>

> >> In fact a cmd without host->data could fail with DPSM enabled.

> >> example:

> >> sent a cmd12 with MMC_RSP_BUSY flag (R1B command) and so no data.

> >> In sdmmc variant the MMCIDATATIMER is used to busy timeout.

> >> The DPSM is used to sample D0 line and is activated.

> >> the DTIMEOUT flag is set after when no end of busy received before the

> >> timeout. (DPSM stays in BUSY and wait for Abort)

> >

> > Oh, that I didn't know about! That make it quite complicated, I am afraid.

> >

> > To send a new CMD12 to clear the DPSM, because the previous CDM12

> > failed seems just wrong. Why would the second CMD12 succeed and not

> > timeout again, if you see what I mean.

> >

> > There are also other commands using R1B, that doesn't involve

> > transferring data. The CMD5 (sleep) for example. If any of these

> > commands would timeout, then why would a following CMD12 not do so as

> > well?

> >

> > To me, it seems like a reset of the controller is the only really sane

> > way of treating these scenarios, isn't it? Of course, it means we need

> > to be able to restore the state of the controller after such a reset

> > as well. Is this possible?

> >

>

> POV sdmmc hardware block the second cmd12 just allows to clear the DPSM

> (for stop abort cmd the R1B may not be set).

> I agree with you, if a command fail due to R1B (busy timeout) the better

> way is to reset the controller, protocol...

>

> Do to that we could call mmc_hw_reset function

> (drivers/mmc/core/core.c). However this function can't be call

> in irq context and ongoing request must be completed.


Actually, that is not really the purpose of that callback. That
callback is about resetting the card, however in some cases that may
involve some additional actions for the host.

>

> proposition:

> The host driver could set a error value (in cmd->error or data->error)

> like EDEADLK or ECONNRESET (to be defined)...

> The framework could check error value (for example in

> mmc_wait_for_req_done) and call mmc_hw_reset if needed.


Something along those lines sounds good to me, but I would rather add
a new separate callback, which main responsibility is to reset the
host HW.

>

> What do you think?


Well, even if the above sounds good and could benefit all hosts, the
suggestion/question I had earlier was more related to if you have a
special reset controller that can be used to reset the mmci IP!? We
have the drivers/reset/* subsystem that helps with these kind of
things.

In principle, if you have a reset controller, you should be able to
recover in the mmci driver, without having to involve the mmc core, if
you see what I mean.

[...]

Kind regards
Uffe
Ulf Hansson Feb. 22, 2019, 10:16 a.m. | #6
On Wed, 20 Feb 2019 at 18:13, Ludovic BARRE <ludovic.barre@st.com> wrote:
>

> hi Ulf

>

> I think you can apply this patch.

> The R1B problem (due to busy timeout) can be seen in a future series

> about busy detect.

> Thanks

>

> Tested-by: Ludovic Barre <ludovic.barre@st.com>


Okay, great, I queue this up.

Kind regards
Uffe

>

> Regards

> Ludo

>

> On 2/20/19 11:27 AM, Ludovic BARRE wrote:

> > hi Ulf

> >

> > On 2/4/19 4:39 PM, Ulf Hansson wrote:

> >> On Mon, 4 Feb 2019 at 15:18, Ludovic BARRE <ludovic.barre@st.com> wrote:

> >>>

> >>> hi Ulf

> >>>

> >>> One comment below in mmci_cmd_irq function

> >>>

> >>> when I rebase my next serie about "busy detect" I've a kernel panic

> >>> null pointer, could you give me some time to investigate this issue

> >>> before apply this patch.

> >>

> >> Sure!

> >>

> >> FYI, I have tested this on a ux500 and didn't observe any regressions.

> >

> > great

> >

> >>

> >>>

> >>>

> >>> On 1/29/19 3:35 PM, Ulf Hansson wrote:

> >>>> The current approach with sending a CMD12 (STOP_TRANSMISSION) to

> >>>> complete a

> >>>> data transfer request, either because of using the open-ended

> >>>> transmission

> >>>> type or because of receiving an error during a pre-defined data

> >>>> transfer,

> >>>> isn't sufficient for the STM32 sdmmc variant. More precisely, this

> >>>> variant

> >>>> needs to clear the DPSM ("Data Path State Machine") by sending a

> >>>> CMD12, for

> >>>> all failing ADTC commands.

> >>>>

> >>>> Support this, by adding a struct mmc_command inside the struct

> >>>> mmci_host

> >>>> and initialize it to a CMD12 during ->probe(). Let's also add checks

> >>>> for

> >>>> the new conditions, to enable mmci_data_irq() and mmci_cmd_irq() to

> >>>> postpone the calls to mmci_request_end(), but instead send the CMD12.

> >>>>

> >>>> Cc: Ludovic Barre <ludovic.barre@st.com>

> >>>> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>

> >>>> ---

> >>>>    drivers/mmc/host/mmci.c | 25 +++++++++++++++++++++++--

> >>>>    drivers/mmc/host/mmci.h |  1 +

> >>>>    2 files changed, 24 insertions(+), 2 deletions(-)

> >>>>

> >>>> diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c

> >>>> index e352f5ad5801..45b97b41601c 100644

> >>>> --- a/drivers/mmc/host/mmci.c

> >>>> +++ b/drivers/mmc/host/mmci.c

> >>>> @@ -1127,6 +1127,12 @@ mmci_start_command(struct mmci_host *host,

> >>>> struct mmc_command *cmd, u32 c)

> >>>>        writel(c, base + MMCICOMMAND);

> >>>>    }

> >>>>

> >>>> +static void mmci_stop_command(struct mmci_host *host)

> >>>> +{

> >>>> +     host->stop_abort.error = 0;

> >>>> +     mmci_start_command(host, &host->stop_abort, 0);

> >>>> +}

> >>>> +

> >>>>    static void

> >>>>    mmci_data_irq(struct mmci_host *host, struct mmc_data *data,

> >>>>              unsigned int status)

> >>>> @@ -1196,10 +1202,16 @@ mmci_data_irq(struct mmci_host *host, struct

> >>>> mmc_data *data,

> >>>>                        /* The error clause is handled above,

> >>>> success! */

> >>>>                        data->bytes_xfered = data->blksz * data->blocks;

> >>>>

> >>>> -             if (!data->stop || (host->mrq->sbc && !data->error))

> >>>> +             if (!data->stop) {

> >>>> +                     if (host->variant->cmdreg_stop && data->error)

> >>>> +                             mmci_stop_command(host);

> >>>> +                     else

> >>>> +                             mmci_request_end(host, data->mrq);

> >>>> +             } else if (host->mrq->sbc && !data->error) {

> >>>>                        mmci_request_end(host, data->mrq);

> >>>> -             else

> >>>> +             } else {

> >>>>                        mmci_start_command(host, data->stop, 0);

> >>>> +             }

> >>>>        }

> >>>>    }

> >>>>

> >>>> @@ -1298,6 +1310,10 @@ mmci_cmd_irq(struct mmci_host *host, struct

> >>>> mmc_command *cmd,

> >>>>                        mmci_dma_error(host);

> >>>>

> >>>>                        mmci_stop_data(host);

> >>>> +                     if (host->variant->cmdreg_stop && cmd->error) {

> >>>> +                             mmci_stop_command(host);

> >>>> +                             return;

> >>>> +                     }

> >>>

> >>>>                }

> >>>

> >>> In fact a cmd without host->data could fail with DPSM enabled.

> >>> example:

> >>> sent a cmd12 with MMC_RSP_BUSY flag (R1B command) and so no data.

> >>> In sdmmc variant the MMCIDATATIMER is used to busy timeout.

> >>> The DPSM is used to sample D0 line and is activated.

> >>> the DTIMEOUT flag is set after when no end of busy received before the

> >>> timeout. (DPSM stays in BUSY and wait for Abort)

> >>

> >> Oh, that I didn't know about! That make it quite complicated, I am

> >> afraid.

> >>

> >> To send a new CMD12 to clear the DPSM, because the previous CDM12

> >> failed seems just wrong. Why would the second CMD12 succeed and not

> >> timeout again, if you see what I mean.

> >>

> >> There are also other commands using R1B, that doesn't involve

> >> transferring data. The CMD5 (sleep) for example. If any of these

> >> commands would timeout, then why would a following CMD12 not do so as

> >> well?

> >>

> >> To me, it seems like a reset of the controller is the only really sane

> >> way of treating these scenarios, isn't it? Of course, it means we need

> >> to be able to restore the state of the controller after such a reset

> >> as well. Is this possible?

> >>

> >

> > POV sdmmc hardware block the second cmd12 just allows to clear the DPSM

> > (for stop abort cmd the R1B may not be set).

> > I agree with you, if a command fail due to R1B (busy timeout) the better

> > way is to reset the controller, protocol...

> >

> > Do to that we could call mmc_hw_reset function

> > (drivers/mmc/core/core.c). However this function can't be call

> > in irq context and ongoing request must be completed.

> >

> > proposition:

> > The host driver could set a error value (in cmd->error or data->error)

> > like EDEADLK or ECONNRESET (to be defined)...

> > The framework could check error value (for example in

> > mmc_wait_for_req_done) and call mmc_hw_reset if needed.

> >

> > What do you think?

> >

> >>>

> >>> So The mmci_stop_command may be sent when there are cmd->error  and

> >>> status & MCI_DATATIMEOUT

> >>>

> >>>

> >>>>                mmci_request_end(host, host->mrq);

> >>>>        } else if (sbc) {

> >>>> @@ -1956,6 +1972,11 @@ static int mmci_probe(struct amba_device *dev,

> >>>>                mmc->max_busy_timeout = 0;

> >>>>        }

> >>>>

> >>>> +     /* Prepare a CMD12 - needed to clear the DPSM on some

> >>>> variants. */

> >>>> +     host->stop_abort.opcode = MMC_STOP_TRANSMISSION;

> >>>> +     host->stop_abort.arg = 0;

> >>>> +     host->stop_abort.flags = MMC_RSP_R1B | MMC_CMD_AC;

> >>>> +

> >>>>        mmc->ops = &mmci_ops;

> >>>>

> >>>>        /* We support these PM capabilities. */

> >>>> diff --git a/drivers/mmc/host/mmci.h b/drivers/mmc/host/mmci.h

> >>>> index 24229097d05c..14df81054438 100644

> >>>> --- a/drivers/mmc/host/mmci.h

> >>>> +++ b/drivers/mmc/host/mmci.h

> >>>> @@ -377,6 +377,7 @@ struct mmci_host {

> >>>>        void __iomem            *base;

> >>>>        struct mmc_request      *mrq;

> >>>>        struct mmc_command      *cmd;

> >>>> +     struct mmc_command      stop_abort;

> >>>>        struct mmc_data         *data;

> >>>>        struct mmc_host         *mmc;

> >>>>        struct clk              *clk;

> >>>>

> >>

> >> Kind regards

> >> Uffe

> >>
Ludovic BARRE Feb. 22, 2019, 10:24 a.m. | #7
On 2/22/19 11:16 AM, Ulf Hansson wrote:
> On Wed, 20 Feb 2019 at 18:13, Ludovic BARRE <ludovic.barre@st.com> wrote:

>>

>> hi Ulf

>>

>> I think you can apply this patch.

>> The R1B problem (due to busy timeout) can be seen in a future series

>> about busy detect.

>> Thanks

>>

>> Tested-by: Ludovic Barre <ludovic.barre@st.com>

> 

> Okay, great, I queue this up.


thanks

> 

> Kind regards

> Uffe

> 

>>

>> Regards

>> Ludo

>>

>> On 2/20/19 11:27 AM, Ludovic BARRE wrote:

>>> hi Ulf

>>>

>>> On 2/4/19 4:39 PM, Ulf Hansson wrote:

>>>> On Mon, 4 Feb 2019 at 15:18, Ludovic BARRE <ludovic.barre@st.com> wrote:

>>>>>

>>>>> hi Ulf

>>>>>

>>>>> One comment below in mmci_cmd_irq function

>>>>>

>>>>> when I rebase my next serie about "busy detect" I've a kernel panic

>>>>> null pointer, could you give me some time to investigate this issue

>>>>> before apply this patch.

>>>>

>>>> Sure!

>>>>

>>>> FYI, I have tested this on a ux500 and didn't observe any regressions.

>>>

>>> great

>>>

>>>>

>>>>>

>>>>>

>>>>> On 1/29/19 3:35 PM, Ulf Hansson wrote:

>>>>>> The current approach with sending a CMD12 (STOP_TRANSMISSION) to

>>>>>> complete a

>>>>>> data transfer request, either because of using the open-ended

>>>>>> transmission

>>>>>> type or because of receiving an error during a pre-defined data

>>>>>> transfer,

>>>>>> isn't sufficient for the STM32 sdmmc variant. More precisely, this

>>>>>> variant

>>>>>> needs to clear the DPSM ("Data Path State Machine") by sending a

>>>>>> CMD12, for

>>>>>> all failing ADTC commands.

>>>>>>

>>>>>> Support this, by adding a struct mmc_command inside the struct

>>>>>> mmci_host

>>>>>> and initialize it to a CMD12 during ->probe(). Let's also add checks

>>>>>> for

>>>>>> the new conditions, to enable mmci_data_irq() and mmci_cmd_irq() to

>>>>>> postpone the calls to mmci_request_end(), but instead send the CMD12.

>>>>>>

>>>>>> Cc: Ludovic Barre <ludovic.barre@st.com>

>>>>>> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>

>>>>>> ---

>>>>>>     drivers/mmc/host/mmci.c | 25 +++++++++++++++++++++++--

>>>>>>     drivers/mmc/host/mmci.h |  1 +

>>>>>>     2 files changed, 24 insertions(+), 2 deletions(-)

>>>>>>

>>>>>> diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c

>>>>>> index e352f5ad5801..45b97b41601c 100644

>>>>>> --- a/drivers/mmc/host/mmci.c

>>>>>> +++ b/drivers/mmc/host/mmci.c

>>>>>> @@ -1127,6 +1127,12 @@ mmci_start_command(struct mmci_host *host,

>>>>>> struct mmc_command *cmd, u32 c)

>>>>>>         writel(c, base + MMCICOMMAND);

>>>>>>     }

>>>>>>

>>>>>> +static void mmci_stop_command(struct mmci_host *host)

>>>>>> +{

>>>>>> +     host->stop_abort.error = 0;

>>>>>> +     mmci_start_command(host, &host->stop_abort, 0);

>>>>>> +}

>>>>>> +

>>>>>>     static void

>>>>>>     mmci_data_irq(struct mmci_host *host, struct mmc_data *data,

>>>>>>               unsigned int status)

>>>>>> @@ -1196,10 +1202,16 @@ mmci_data_irq(struct mmci_host *host, struct

>>>>>> mmc_data *data,

>>>>>>                         /* The error clause is handled above,

>>>>>> success! */

>>>>>>                         data->bytes_xfered = data->blksz * data->blocks;

>>>>>>

>>>>>> -             if (!data->stop || (host->mrq->sbc && !data->error))

>>>>>> +             if (!data->stop) {

>>>>>> +                     if (host->variant->cmdreg_stop && data->error)

>>>>>> +                             mmci_stop_command(host);

>>>>>> +                     else

>>>>>> +                             mmci_request_end(host, data->mrq);

>>>>>> +             } else if (host->mrq->sbc && !data->error) {

>>>>>>                         mmci_request_end(host, data->mrq);

>>>>>> -             else

>>>>>> +             } else {

>>>>>>                         mmci_start_command(host, data->stop, 0);

>>>>>> +             }

>>>>>>         }

>>>>>>     }

>>>>>>

>>>>>> @@ -1298,6 +1310,10 @@ mmci_cmd_irq(struct mmci_host *host, struct

>>>>>> mmc_command *cmd,

>>>>>>                         mmci_dma_error(host);

>>>>>>

>>>>>>                         mmci_stop_data(host);

>>>>>> +                     if (host->variant->cmdreg_stop && cmd->error) {

>>>>>> +                             mmci_stop_command(host);

>>>>>> +                             return;

>>>>>> +                     }

>>>>>

>>>>>>                 }

>>>>>

>>>>> In fact a cmd without host->data could fail with DPSM enabled.

>>>>> example:

>>>>> sent a cmd12 with MMC_RSP_BUSY flag (R1B command) and so no data.

>>>>> In sdmmc variant the MMCIDATATIMER is used to busy timeout.

>>>>> The DPSM is used to sample D0 line and is activated.

>>>>> the DTIMEOUT flag is set after when no end of busy received before the

>>>>> timeout. (DPSM stays in BUSY and wait for Abort)

>>>>

>>>> Oh, that I didn't know about! That make it quite complicated, I am

>>>> afraid.

>>>>

>>>> To send a new CMD12 to clear the DPSM, because the previous CDM12

>>>> failed seems just wrong. Why would the second CMD12 succeed and not

>>>> timeout again, if you see what I mean.

>>>>

>>>> There are also other commands using R1B, that doesn't involve

>>>> transferring data. The CMD5 (sleep) for example. If any of these

>>>> commands would timeout, then why would a following CMD12 not do so as

>>>> well?

>>>>

>>>> To me, it seems like a reset of the controller is the only really sane

>>>> way of treating these scenarios, isn't it? Of course, it means we need

>>>> to be able to restore the state of the controller after such a reset

>>>> as well. Is this possible?

>>>>

>>>

>>> POV sdmmc hardware block the second cmd12 just allows to clear the DPSM

>>> (for stop abort cmd the R1B may not be set).

>>> I agree with you, if a command fail due to R1B (busy timeout) the better

>>> way is to reset the controller, protocol...

>>>

>>> Do to that we could call mmc_hw_reset function

>>> (drivers/mmc/core/core.c). However this function can't be call

>>> in irq context and ongoing request must be completed.

>>>

>>> proposition:

>>> The host driver could set a error value (in cmd->error or data->error)

>>> like EDEADLK or ECONNRESET (to be defined)...

>>> The framework could check error value (for example in

>>> mmc_wait_for_req_done) and call mmc_hw_reset if needed.

>>>

>>> What do you think?

>>>

>>>>>

>>>>> So The mmci_stop_command may be sent when there are cmd->error  and

>>>>> status & MCI_DATATIMEOUT

>>>>>

>>>>>

>>>>>>                 mmci_request_end(host, host->mrq);

>>>>>>         } else if (sbc) {

>>>>>> @@ -1956,6 +1972,11 @@ static int mmci_probe(struct amba_device *dev,

>>>>>>                 mmc->max_busy_timeout = 0;

>>>>>>         }

>>>>>>

>>>>>> +     /* Prepare a CMD12 - needed to clear the DPSM on some

>>>>>> variants. */

>>>>>> +     host->stop_abort.opcode = MMC_STOP_TRANSMISSION;

>>>>>> +     host->stop_abort.arg = 0;

>>>>>> +     host->stop_abort.flags = MMC_RSP_R1B | MMC_CMD_AC;

>>>>>> +

>>>>>>         mmc->ops = &mmci_ops;

>>>>>>

>>>>>>         /* We support these PM capabilities. */

>>>>>> diff --git a/drivers/mmc/host/mmci.h b/drivers/mmc/host/mmci.h

>>>>>> index 24229097d05c..14df81054438 100644

>>>>>> --- a/drivers/mmc/host/mmci.h

>>>>>> +++ b/drivers/mmc/host/mmci.h

>>>>>> @@ -377,6 +377,7 @@ struct mmci_host {

>>>>>>         void __iomem            *base;

>>>>>>         struct mmc_request      *mrq;

>>>>>>         struct mmc_command      *cmd;

>>>>>> +     struct mmc_command      stop_abort;

>>>>>>         struct mmc_data         *data;

>>>>>>         struct mmc_host         *mmc;

>>>>>>         struct clk              *clk;

>>>>>>

>>>>

>>>> Kind regards

>>>> Uffe

>>>>

Patch

diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
index e352f5ad5801..45b97b41601c 100644
--- a/drivers/mmc/host/mmci.c
+++ b/drivers/mmc/host/mmci.c
@@ -1127,6 +1127,12 @@  mmci_start_command(struct mmci_host *host, struct mmc_command *cmd, u32 c)
 	writel(c, base + MMCICOMMAND);
 }
 
+static void mmci_stop_command(struct mmci_host *host)
+{
+	host->stop_abort.error = 0;
+	mmci_start_command(host, &host->stop_abort, 0);
+}
+
 static void
 mmci_data_irq(struct mmci_host *host, struct mmc_data *data,
 	      unsigned int status)
@@ -1196,10 +1202,16 @@  mmci_data_irq(struct mmci_host *host, struct mmc_data *data,
 			/* The error clause is handled above, success! */
 			data->bytes_xfered = data->blksz * data->blocks;
 
-		if (!data->stop || (host->mrq->sbc && !data->error))
+		if (!data->stop) {
+			if (host->variant->cmdreg_stop && data->error)
+				mmci_stop_command(host);
+			else
+				mmci_request_end(host, data->mrq);
+		} else if (host->mrq->sbc && !data->error) {
 			mmci_request_end(host, data->mrq);
-		else
+		} else {
 			mmci_start_command(host, data->stop, 0);
+		}
 	}
 }
 
@@ -1298,6 +1310,10 @@  mmci_cmd_irq(struct mmci_host *host, struct mmc_command *cmd,
 			mmci_dma_error(host);
 
 			mmci_stop_data(host);
+			if (host->variant->cmdreg_stop && cmd->error) {
+				mmci_stop_command(host);
+				return;
+			}
 		}
 		mmci_request_end(host, host->mrq);
 	} else if (sbc) {
@@ -1956,6 +1972,11 @@  static int mmci_probe(struct amba_device *dev,
 		mmc->max_busy_timeout = 0;
 	}
 
+	/* Prepare a CMD12 - needed to clear the DPSM on some variants. */
+	host->stop_abort.opcode = MMC_STOP_TRANSMISSION;
+	host->stop_abort.arg = 0;
+	host->stop_abort.flags = MMC_RSP_R1B | MMC_CMD_AC;
+
 	mmc->ops = &mmci_ops;
 
 	/* We support these PM capabilities. */
diff --git a/drivers/mmc/host/mmci.h b/drivers/mmc/host/mmci.h
index 24229097d05c..14df81054438 100644
--- a/drivers/mmc/host/mmci.h
+++ b/drivers/mmc/host/mmci.h
@@ -377,6 +377,7 @@  struct mmci_host {
 	void __iomem		*base;
 	struct mmc_request	*mrq;
 	struct mmc_command	*cmd;
+	struct mmc_command	stop_abort;
 	struct mmc_data		*data;
 	struct mmc_host		*mmc;
 	struct clk		*clk;