| Message ID | 20190329121042.24415-1-faiz_abbas@ti.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v5] mmc: sdhci-omap: Don't finish_mrq() on a command error during tuning | expand |
On 29/03/19 2:10 PM, 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 sdhci_irq. 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> Acked-by: Adrian Hunter <adrian.hunter@intel.com> > --- > Tested with 100 times boot tests on dra71x-evm, dra72x-evm and > dra7xx-evm. > > v5: > Drop patch 1 and replace call to sdhci_finish_command() with > host->cmd = NULL > > v4: > Fixed commit description for patch 1. > Added SDHCI_INT_TIMEOUT to CMD_ERR_MASK in patch 2. > > v3: > Removed the command error specific callback and using the already > existing sdhci_irq callback instead. No measurable drop in performance. > > v2: > Added EXPORT_SYMBOL_GPL for sdhci_cmd_err and sdhci_send_command to fix > errors when built as module. > > drivers/mmc/host/sdhci-omap.c | 38 +++++++++++++++++++++++++++++++++++ > 1 file changed, 38 insertions(+) > > diff --git a/drivers/mmc/host/sdhci-omap.c b/drivers/mmc/host/sdhci-omap.c > index 5bbed477c9b1..9f2af89ae9b5 100644 > --- a/drivers/mmc/host/sdhci-omap.c > +++ b/drivers/mmc/host/sdhci-omap.c > @@ -797,6 +797,43 @@ void sdhci_omap_reset(struct sdhci_host *host, u8 mask) > sdhci_reset(host, mask); > } > > +#define CMD_ERR_MASK (SDHCI_INT_CRC | SDHCI_INT_END_BIT | SDHCI_INT_INDEX |\ > + SDHCI_INT_TIMEOUT) > +#define CMD_MASK (CMD_ERR_MASK | SDHCI_INT_RESPONSE) > + > +static irqreturn_t sdhci_omap_irq(struct sdhci_host *host, u32 intmask) > +{ > + 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 && host->cmd && !host->data_early && > + (intmask & CMD_ERR_MASK)) { > + > + /* > + * 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; > + > + host->cmd = NULL; > + > + /* > + * Sometimes command error interrupts and command complete > + * interrupt will arrive together. Clear all command related > + * interrupts here. > + */ > + sdhci_writel(host, intmask & CMD_MASK, SDHCI_INT_STATUS); > + intmask &= ~CMD_MASK; > + } > + > + return intmask; > +} > + > static struct sdhci_ops sdhci_omap_ops = { > .set_clock = sdhci_omap_set_clock, > .set_power = sdhci_omap_set_power, > @@ -807,6 +844,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, > + .irq = sdhci_omap_irq, > }; > > static int sdhci_omap_set_capabilities(struct sdhci_omap_host *omap_host) >
Hi Faiz,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on ulf.hansson-mmc/next]
[also build test WARNING on v5.1-rc2 next-20190329]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Faiz-Abbas/mmc-sdhci-omap-Don-t-finish_mrq-on-a-command-error-during-tuning/20190330-093057
base: git://git.kernel.org/pub/scm/linux/kernel/git/ulfh/mmc.git next
reproduce:
# apt-get install sparse
make ARCH=x86_64 allmodconfig
make C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__'
sparse warnings: (new ones prefixed by >>)
drivers/mmc/host/sdhci-omap.c:386:31: sparse: expression using sizeof(void)
drivers/mmc/host/sdhci-omap.c:386:31: sparse: expression using sizeof(void)
>> drivers/mmc/host/sdhci-omap.c:847:16: sparse: incorrect type in initializer (different signedness) @@ expected unsigned int ( *irq )( ... ) @@ got int enunsigned int ( *irq )( ... ) @@
drivers/mmc/host/sdhci-omap.c:847:16: expected unsigned int ( *irq )( ... )
drivers/mmc/host/sdhci-omap.c:847:16: got int enum irqreturn ( *<noident> )( ... )
include/linux/device.h:688:13: sparse: undefined identifier '__builtin_mul_overflow'
include/linux/device.h:688:13: sparse: call with no type!
vim +847 drivers/mmc/host/sdhci-omap.c
836
837 static struct sdhci_ops sdhci_omap_ops = {
838 .set_clock = sdhci_omap_set_clock,
839 .set_power = sdhci_omap_set_power,
840 .enable_dma = sdhci_omap_enable_dma,
841 .get_max_clock = sdhci_pltfm_clk_get_max_clock,
842 .get_min_clock = sdhci_omap_get_min_clock,
843 .set_bus_width = sdhci_omap_set_bus_width,
844 .platform_send_init_74_clocks = sdhci_omap_init_74_clocks,
845 .reset = sdhci_omap_reset,
846 .set_uhs_signaling = sdhci_omap_set_uhs_signaling,
> 847 .irq = sdhci_omap_irq,
848 };
849
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
diff --git a/drivers/mmc/host/sdhci-omap.c b/drivers/mmc/host/sdhci-omap.c index 5bbed477c9b1..9f2af89ae9b5 100644 --- a/drivers/mmc/host/sdhci-omap.c +++ b/drivers/mmc/host/sdhci-omap.c @@ -797,6 +797,43 @@ void sdhci_omap_reset(struct sdhci_host *host, u8 mask) sdhci_reset(host, mask); } +#define CMD_ERR_MASK (SDHCI_INT_CRC | SDHCI_INT_END_BIT | SDHCI_INT_INDEX |\ + SDHCI_INT_TIMEOUT) +#define CMD_MASK (CMD_ERR_MASK | SDHCI_INT_RESPONSE) + +static irqreturn_t sdhci_omap_irq(struct sdhci_host *host, u32 intmask) +{ + 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 && host->cmd && !host->data_early && + (intmask & CMD_ERR_MASK)) { + + /* + * 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; + + host->cmd = NULL; + + /* + * Sometimes command error interrupts and command complete + * interrupt will arrive together. Clear all command related + * interrupts here. + */ + sdhci_writel(host, intmask & CMD_MASK, SDHCI_INT_STATUS); + intmask &= ~CMD_MASK; + } + + return intmask; +} + static struct sdhci_ops sdhci_omap_ops = { .set_clock = sdhci_omap_set_clock, .set_power = sdhci_omap_set_power, @@ -807,6 +844,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, + .irq = sdhci_omap_irq, }; static int sdhci_omap_set_capabilities(struct sdhci_omap_host *omap_host)
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 sdhci_irq. 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> --- Tested with 100 times boot tests on dra71x-evm, dra72x-evm and dra7xx-evm. v5: Drop patch 1 and replace call to sdhci_finish_command() with host->cmd = NULL v4: Fixed commit description for patch 1. Added SDHCI_INT_TIMEOUT to CMD_ERR_MASK in patch 2. v3: Removed the command error specific callback and using the already existing sdhci_irq callback instead. No measurable drop in performance. v2: Added EXPORT_SYMBOL_GPL for sdhci_cmd_err and sdhci_send_command to fix errors when built as module. drivers/mmc/host/sdhci-omap.c | 38 +++++++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+)