* [PATCH v2 0/2] Fixes for command errors during tuning
@ 2019-03-01 8:38 Faiz Abbas
2019-03-01 8:38 ` [PATCH v2 1/2] mmc: sdhci: Add platform_cmd_err() to sdhci_ops Faiz Abbas
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: Faiz Abbas @ 2019-03-01 8:38 UTC (permalink / raw)
To: linux-kernel, linux-mmc, linux-omap
Cc: ulf.hansson, kishon, adrian.hunter, faiz_abbas
These patches fix the following error message in dra7xx boards:
[4.833198] mmc1: Got data interrupt 0x00000002 even though no data
operation was in progress.
Tested with 100 times boot tests on dra71x-evm, dra72x-evm and
dra7xx-evm.
v2:
Added EXPORT_SYMBOL_GPL for sdhci_cmd_err and sdhci_send_command to fix
errors when built as module.
Faiz Abbas (2):
mmc: sdhci: Add platform_cmd_err() to sdhci_ops
mmc: sdhci-omap: Don't finish_mrq() on a command error during tuning
drivers/mmc/host/sdhci-omap.c | 24 +++++++++++++++++++++
drivers/mmc/host/sdhci.c | 40 +++++++++++++++++++++++------------
drivers/mmc/host/sdhci.h | 4 ++++
3 files changed, 54 insertions(+), 14 deletions(-)
--
2.19.2
^ permalink raw reply [flat|nested] 7+ messages in thread* [PATCH v2 1/2] mmc: sdhci: Add platform_cmd_err() to sdhci_ops 2019-03-01 8:38 [PATCH v2 0/2] Fixes for command errors during tuning Faiz Abbas @ 2019-03-01 8:38 ` Faiz Abbas 2019-03-01 8:38 ` [PATCH v2 2/2] mmc: sdhci-omap: Don't finish_mrq() on a command error during tuning Faiz Abbas 2019-03-06 10:18 ` [PATCH v2 0/2] Fixes for command errors " Faiz Abbas 2 siblings, 0 replies; 7+ messages in thread From: Faiz Abbas @ 2019-03-01 8:38 UTC (permalink / raw) To: linux-kernel, linux-mmc, linux-omap Cc: ulf.hansson, kishon, adrian.hunter, faiz_abbas Some platforms might need a custom method for handling command error interrupts. Add a callback to sdhci_ops to facilitate the same. Move default command error handling to its own non-static function so it can be called from platform drivers. Also make sdhci_finish_command() non-static. Fixes: 5b0d62108b46 ("mmc: sdhci-omap: Add platform specific reset callback") Signed-off-by: Faiz Abbas <faiz_abbas@ti.com> --- drivers/mmc/host/sdhci.c | 40 ++++++++++++++++++++++++++-------------- drivers/mmc/host/sdhci.h | 4 ++++ 2 files changed, 30 insertions(+), 14 deletions(-) diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c index a8141ff9be03..ff60b1830896 100644 --- a/drivers/mmc/host/sdhci.c +++ b/drivers/mmc/host/sdhci.c @@ -1445,7 +1445,7 @@ static void sdhci_read_rsp_136(struct sdhci_host *host, struct mmc_command *cmd) } } -static void sdhci_finish_command(struct sdhci_host *host) +void sdhci_finish_command(struct sdhci_host *host) { struct mmc_command *cmd = host->cmd; @@ -1495,6 +1495,8 @@ static void sdhci_finish_command(struct sdhci_host *host) sdhci_finish_mrq(host, cmd->mrq); } } +EXPORT_SYMBOL_GPL(sdhci_finish_command); + static u16 sdhci_get_preset_value(struct sdhci_host *host) { @@ -2780,6 +2782,26 @@ static void sdhci_timeout_data_timer(struct timer_list *t) * * \*****************************************************************************/ +void sdhci_cmd_err(struct sdhci_host *host, u32 intmask, u32 *intmask_p) +{ + if (intmask & SDHCI_INT_TIMEOUT) + host->cmd->error = -ETIMEDOUT; + else + host->cmd->error = -EILSEQ; + + /* Treat data command CRC error the same as data CRC error */ + if (host->cmd->data && + (intmask & (SDHCI_INT_CRC | SDHCI_INT_TIMEOUT)) == + SDHCI_INT_CRC) { + host->cmd = NULL; + *intmask_p |= SDHCI_INT_DATA_CRC; + return; + } + + sdhci_finish_mrq(host, host->cmd->mrq); +} +EXPORT_SYMBOL_GPL(sdhci_cmd_err); + static void sdhci_cmd_irq(struct sdhci_host *host, u32 intmask, u32 *intmask_p) { /* Handle auto-CMD12 error */ @@ -2813,21 +2835,11 @@ static void sdhci_cmd_irq(struct sdhci_host *host, u32 intmask, u32 *intmask_p) if (intmask & (SDHCI_INT_TIMEOUT | SDHCI_INT_CRC | SDHCI_INT_END_BIT | SDHCI_INT_INDEX)) { - if (intmask & SDHCI_INT_TIMEOUT) - host->cmd->error = -ETIMEDOUT; + if (host->ops->platform_cmd_err) + host->ops->platform_cmd_err(host, intmask, intmask_p); else - host->cmd->error = -EILSEQ; - - /* Treat data command CRC error the same as data CRC error */ - if (host->cmd->data && - (intmask & (SDHCI_INT_CRC | SDHCI_INT_TIMEOUT)) == - SDHCI_INT_CRC) { - host->cmd = NULL; - *intmask_p |= SDHCI_INT_DATA_CRC; - return; - } + sdhci_cmd_err(host, intmask, intmask_p); - sdhci_finish_mrq(host, host->cmd->mrq); return; } diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h index 01002cba1359..ca427d8efc29 100644 --- a/drivers/mmc/host/sdhci.h +++ b/drivers/mmc/host/sdhci.h @@ -645,6 +645,8 @@ struct sdhci_ops { void (*voltage_switch)(struct sdhci_host *host); void (*adma_write_desc)(struct sdhci_host *host, void **desc, dma_addr_t addr, int len, unsigned int cmd); + void (*platform_cmd_err)(struct sdhci_host *host, u32 intmask, + u32 *intmask_p); }; #ifdef CONFIG_MMC_SDHCI_IO_ACCESSORS @@ -798,5 +800,7 @@ void sdhci_start_tuning(struct sdhci_host *host); void sdhci_end_tuning(struct sdhci_host *host); void sdhci_reset_tuning(struct sdhci_host *host); void sdhci_send_tuning(struct sdhci_host *host, u32 opcode); +void sdhci_cmd_err(struct sdhci_host *host, u32 intmask, u32 *intmask_p); +void sdhci_finish_command(struct sdhci_host *host); #endif /* __SDHCI_HW_H */ -- 2.19.2 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH v2 2/2] mmc: sdhci-omap: Don't finish_mrq() on a command error during tuning 2019-03-01 8:38 [PATCH v2 0/2] Fixes for command errors during tuning Faiz Abbas 2019-03-01 8:38 ` [PATCH v2 1/2] mmc: sdhci: Add platform_cmd_err() to sdhci_ops Faiz Abbas @ 2019-03-01 8:38 ` Faiz Abbas 2019-03-06 12:09 ` Adrian Hunter 2019-03-06 10:18 ` [PATCH v2 0/2] Fixes for command errors " Faiz Abbas 2 siblings, 1 reply; 7+ messages in thread From: Faiz Abbas @ 2019-03-01 8:38 UTC (permalink / raw) To: linux-kernel, linux-mmc, linux-omap Cc: ulf.hansson, kishon, adrian.hunter, faiz_abbas commit 5b0d62108b46 ("mmc: sdhci-omap: Add platform specific reset callback") skips data resets during tuning operation. Because of this, a data error or data finish interrupt might still arrive after a command error has been handled and the mrq ended. This ends up with a "mmc0: Got data interrupt 0x00000002 even though no data operation was in progress" error message. Fix this by adding a platform specific callback for command errors. Mark the mrq as a failure but wait for a data interrupt instead of calling finish_mrq(). Fixes: 5b0d62108b46 ("mmc: sdhci-omap: Add platform specific reset callback") Signed-off-by: Faiz Abbas <faiz_abbas@ti.com> --- drivers/mmc/host/sdhci-omap.c | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/drivers/mmc/host/sdhci-omap.c b/drivers/mmc/host/sdhci-omap.c index b1a66ca3821a..67b361a403bc 100644 --- a/drivers/mmc/host/sdhci-omap.c +++ b/drivers/mmc/host/sdhci-omap.c @@ -797,6 +797,29 @@ void sdhci_omap_reset(struct sdhci_host *host, u8 mask) sdhci_reset(host, mask); } +void sdhci_omap_cmd_err(struct sdhci_host *host, u32 intmask, u32 *intmask_p) +{ + struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host); + struct sdhci_omap_host *omap_host = sdhci_pltfm_priv(pltfm_host); + + if (omap_host->is_tuning) { + /* + * Since we are not resetting data lines during tuning + * operation, data error or data complete interrupts + * might still arrive. Mark this request as a failure + * but still wait for the data interrupt + */ + if (intmask & SDHCI_INT_TIMEOUT) + host->cmd->error = -ETIMEDOUT; + else + host->cmd->error = -EILSEQ; + + sdhci_finish_command(host); + } else { + sdhci_cmd_err(host, intmask, intmask_p); + } +} + static struct sdhci_ops sdhci_omap_ops = { .set_clock = sdhci_omap_set_clock, .set_power = sdhci_omap_set_power, @@ -807,6 +830,7 @@ static struct sdhci_ops sdhci_omap_ops = { .platform_send_init_74_clocks = sdhci_omap_init_74_clocks, .reset = sdhci_omap_reset, .set_uhs_signaling = sdhci_omap_set_uhs_signaling, + .platform_cmd_err = sdhci_omap_cmd_err, }; static int sdhci_omap_set_capabilities(struct sdhci_omap_host *omap_host) -- 2.19.2 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v2 2/2] mmc: sdhci-omap: Don't finish_mrq() on a command error during tuning 2019-03-01 8:38 ` [PATCH v2 2/2] mmc: sdhci-omap: Don't finish_mrq() on a command error during tuning Faiz Abbas @ 2019-03-06 12:09 ` Adrian Hunter 2019-03-12 17:34 ` Rizvi, Mohammad Faiz Abbas 0 siblings, 1 reply; 7+ messages in thread From: Adrian Hunter @ 2019-03-06 12:09 UTC (permalink / raw) To: Faiz Abbas, linux-kernel, linux-mmc, linux-omap; +Cc: ulf.hansson, kishon On 1/03/19 10:38 AM, Faiz Abbas wrote: > commit 5b0d62108b46 ("mmc: sdhci-omap: Add platform specific reset > callback") skips data resets during tuning operation. Because of this, > a data error or data finish interrupt might still arrive after a command > error has been handled and the mrq ended. This ends up with a "mmc0: Got > data interrupt 0x00000002 even though no data operation was in progress" > error message. > > Fix this by adding a platform specific callback for command errors. Mark > the mrq as a failure but wait for a data interrupt instead of calling > finish_mrq(). > > Fixes: 5b0d62108b46 ("mmc: sdhci-omap: Add platform specific reset > callback") > Signed-off-by: Faiz Abbas <faiz_abbas@ti.com> > --- > drivers/mmc/host/sdhci-omap.c | 24 ++++++++++++++++++++++++ > 1 file changed, 24 insertions(+) > > diff --git a/drivers/mmc/host/sdhci-omap.c b/drivers/mmc/host/sdhci-omap.c > index b1a66ca3821a..67b361a403bc 100644 > --- a/drivers/mmc/host/sdhci-omap.c > +++ b/drivers/mmc/host/sdhci-omap.c > @@ -797,6 +797,29 @@ void sdhci_omap_reset(struct sdhci_host *host, u8 mask) > sdhci_reset(host, mask); > } > > +void sdhci_omap_cmd_err(struct sdhci_host *host, u32 intmask, u32 *intmask_p) > +{ > + struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host); > + struct sdhci_omap_host *omap_host = sdhci_pltfm_priv(pltfm_host); > + > + if (omap_host->is_tuning) { > + /* > + * Since we are not resetting data lines during tuning > + * operation, data error or data complete interrupts > + * might still arrive. Mark this request as a failure > + * but still wait for the data interrupt > + */ > + if (intmask & SDHCI_INT_TIMEOUT) > + host->cmd->error = -ETIMEDOUT; > + else > + host->cmd->error = -EILSEQ; > + > + sdhci_finish_command(host); > + } else { > + sdhci_cmd_err(host, intmask, intmask_p); > + } > +} Could this be done by the existing ->irq() callback? i.e. mask the error bits in intmask (have to write them back to SDHCI_INT_STATUS also) but set cmd->error. ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2 2/2] mmc: sdhci-omap: Don't finish_mrq() on a command error during tuning 2019-03-06 12:09 ` Adrian Hunter @ 2019-03-12 17:34 ` Rizvi, Mohammad Faiz Abbas 2019-03-13 9:35 ` Hunter, Adrian 0 siblings, 1 reply; 7+ messages in thread From: Rizvi, Mohammad Faiz Abbas @ 2019-03-12 17:34 UTC (permalink / raw) To: Adrian Hunter, linux-kernel, linux-mmc, linux-omap; +Cc: ulf.hansson, kishon Hi Adrian, On 3/6/2019 5:39 PM, Adrian Hunter wrote: > On 1/03/19 10:38 AM, Faiz Abbas wrote: >> commit 5b0d62108b46 ("mmc: sdhci-omap: Add platform specific reset >> callback") skips data resets during tuning operation. Because of this, >> a data error or data finish interrupt might still arrive after a command >> error has been handled and the mrq ended. This ends up with a "mmc0: Got >> data interrupt 0x00000002 even though no data operation was in progress" >> error message. >> >> Fix this by adding a platform specific callback for command errors. Mark >> the mrq as a failure but wait for a data interrupt instead of calling >> finish_mrq(). >> >> Fixes: 5b0d62108b46 ("mmc: sdhci-omap: Add platform specific reset >> callback") >> Signed-off-by: Faiz Abbas <faiz_abbas@ti.com> >> --- >> drivers/mmc/host/sdhci-omap.c | 24 ++++++++++++++++++++++++ >> 1 file changed, 24 insertions(+) >> >> diff --git a/drivers/mmc/host/sdhci-omap.c b/drivers/mmc/host/sdhci-omap.c >> index b1a66ca3821a..67b361a403bc 100644 >> --- a/drivers/mmc/host/sdhci-omap.c >> +++ b/drivers/mmc/host/sdhci-omap.c >> @@ -797,6 +797,29 @@ void sdhci_omap_reset(struct sdhci_host *host, u8 mask) >> sdhci_reset(host, mask); >> } >> >> +void sdhci_omap_cmd_err(struct sdhci_host *host, u32 intmask, u32 *intmask_p) >> +{ >> + struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host); >> + struct sdhci_omap_host *omap_host = sdhci_pltfm_priv(pltfm_host); >> + >> + if (omap_host->is_tuning) { >> + /* >> + * Since we are not resetting data lines during tuning >> + * operation, data error or data complete interrupts >> + * might still arrive. Mark this request as a failure >> + * but still wait for the data interrupt >> + */ >> + if (intmask & SDHCI_INT_TIMEOUT) >> + host->cmd->error = -ETIMEDOUT; >> + else >> + host->cmd->error = -EILSEQ; >> + >> + sdhci_finish_command(host); >> + } else { >> + sdhci_cmd_err(host, intmask, intmask_p); >> + } >> +} > > Could this be done by the existing ->irq() callback? i.e. mask the error > bits in intmask (have to write them back to SDHCI_INT_STATUS also) but set > cmd->error. > It is possible but I really don't want the callback to be unnecessarily called for every single interrupt that happens. I think we should only use the callback in the case of an actual error interrupt :-) Thanks, Faiz ^ permalink raw reply [flat|nested] 7+ messages in thread
* RE: [PATCH v2 2/2] mmc: sdhci-omap: Don't finish_mrq() on a command error during tuning 2019-03-12 17:34 ` Rizvi, Mohammad Faiz Abbas @ 2019-03-13 9:35 ` Hunter, Adrian 0 siblings, 0 replies; 7+ messages in thread From: Hunter, Adrian @ 2019-03-13 9:35 UTC (permalink / raw) To: Rizvi, Mohammad Faiz Abbas, linux-kernel, linux-mmc, linux-omap Cc: ulf.hansson, kishon > -----Original Message----- > From: Rizvi, Mohammad Faiz Abbas [mailto:faiz_abbas@ti.com] > Sent: Tuesday, March 12, 2019 7:35 PM > To: Hunter, Adrian <adrian.hunter@intel.com>; linux- > kernel@vger.kernel.org; linux-mmc@vger.kernel.org; linux- > omap@vger.kernel.org > Cc: ulf.hansson@linaro.org; kishon@ti.com > Subject: Re: [PATCH v2 2/2] mmc: sdhci-omap: Don't finish_mrq() on a > command error during tuning > > Hi Adrian, > > On 3/6/2019 5:39 PM, Adrian Hunter wrote: > > On 1/03/19 10:38 AM, Faiz Abbas wrote: > >> commit 5b0d62108b46 ("mmc: sdhci-omap: Add platform specific reset > >> callback") skips data resets during tuning operation. Because of > >> this, a data error or data finish interrupt might still arrive after > >> a command error has been handled and the mrq ended. This ends up with > >> a "mmc0: Got data interrupt 0x00000002 even though no data operation > was in progress" > >> error message. > >> > >> Fix this by adding a platform specific callback for command errors. > >> Mark the mrq as a failure but wait for a data interrupt instead of > >> calling finish_mrq(). > >> > >> Fixes: 5b0d62108b46 ("mmc: sdhci-omap: Add platform specific reset > >> callback") > >> Signed-off-by: Faiz Abbas <faiz_abbas@ti.com> > >> --- > >> drivers/mmc/host/sdhci-omap.c | 24 ++++++++++++++++++++++++ > >> 1 file changed, 24 insertions(+) > >> > >> diff --git a/drivers/mmc/host/sdhci-omap.c > >> b/drivers/mmc/host/sdhci-omap.c index b1a66ca3821a..67b361a403bc > >> 100644 > >> --- a/drivers/mmc/host/sdhci-omap.c > >> +++ b/drivers/mmc/host/sdhci-omap.c > >> @@ -797,6 +797,29 @@ void sdhci_omap_reset(struct sdhci_host *host, > u8 mask) > >> sdhci_reset(host, mask); > >> } > >> > >> +void sdhci_omap_cmd_err(struct sdhci_host *host, u32 intmask, u32 > >> +*intmask_p) { > >> + struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host); > >> + struct sdhci_omap_host *omap_host = > sdhci_pltfm_priv(pltfm_host); > >> + > >> + if (omap_host->is_tuning) { > >> + /* > >> + * Since we are not resetting data lines during tuning > >> + * operation, data error or data complete interrupts > >> + * might still arrive. Mark this request as a failure > >> + * but still wait for the data interrupt > >> + */ > >> + if (intmask & SDHCI_INT_TIMEOUT) > >> + host->cmd->error = -ETIMEDOUT; > >> + else > >> + host->cmd->error = -EILSEQ; > >> + > >> + sdhci_finish_command(host); > >> + } else { > >> + sdhci_cmd_err(host, intmask, intmask_p); > >> + } > >> +} > > > > Could this be done by the existing ->irq() callback? i.e. mask the > > error bits in intmask (have to write them back to SDHCI_INT_STATUS > > also) but set > > cmd->error. > > > > It is possible but I really don't want the callback to be unnecessarily called for > every single interrupt that happens. I think we should only use the callback in > the case of an actual error interrupt :-) You mean for performance reasons? I would have thought it would be only a handful of cycles to call into a function, find nothing to do, and return. That should be negligible compared to the rest of the interrupt handler. If that is the concern, then it might be worth measuring it. ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2 0/2] Fixes for command errors during tuning 2019-03-01 8:38 [PATCH v2 0/2] Fixes for command errors during tuning Faiz Abbas 2019-03-01 8:38 ` [PATCH v2 1/2] mmc: sdhci: Add platform_cmd_err() to sdhci_ops Faiz Abbas 2019-03-01 8:38 ` [PATCH v2 2/2] mmc: sdhci-omap: Don't finish_mrq() on a command error during tuning Faiz Abbas @ 2019-03-06 10:18 ` Faiz Abbas 2 siblings, 0 replies; 7+ messages in thread From: Faiz Abbas @ 2019-03-06 10:18 UTC (permalink / raw) To: linux-kernel, linux-mmc, linux-omap; +Cc: ulf.hansson, kishon, adrian.hunter Hi, On 01/03/19 2:08 PM, Faiz Abbas wrote: > These patches fix the following error message in dra7xx boards: > > [4.833198] mmc1: Got data interrupt 0x00000002 even though no data > operation was in progress. > > Tested with 100 times boot tests on dra71x-evm, dra72x-evm and > dra7xx-evm. > > v2: > Added EXPORT_SYMBOL_GPL for sdhci_cmd_err and sdhci_send_command to fix > errors when built as module. > > Faiz Abbas (2): > mmc: sdhci: Add platform_cmd_err() to sdhci_ops > mmc: sdhci-omap: Don't finish_mrq() on a command error during tuning > > drivers/mmc/host/sdhci-omap.c | 24 +++++++++++++++++++++ > drivers/mmc/host/sdhci.c | 40 +++++++++++++++++++++++------------ > drivers/mmc/host/sdhci.h | 4 ++++ > 3 files changed, 54 insertions(+), 14 deletions(-) > Gentle ping. Thanks, Faiz ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2019-03-13 9:35 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-03-01 8:38 [PATCH v2 0/2] Fixes for command errors during tuning Faiz Abbas 2019-03-01 8:38 ` [PATCH v2 1/2] mmc: sdhci: Add platform_cmd_err() to sdhci_ops Faiz Abbas 2019-03-01 8:38 ` [PATCH v2 2/2] mmc: sdhci-omap: Don't finish_mrq() on a command error during tuning Faiz Abbas 2019-03-06 12:09 ` Adrian Hunter 2019-03-12 17:34 ` Rizvi, Mohammad Faiz Abbas 2019-03-13 9:35 ` Hunter, Adrian 2019-03-06 10:18 ` [PATCH v2 0/2] Fixes for command errors " Faiz Abbas
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).