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.
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.
Faiz Abbas (2):
mmc: sdhci: Make sdhci_send_command() public
mmc: sdhci-omap: Don't finish_mrq() on a command error during tuning
drivers/mmc/host/sdhci-omap.c | 30 ++++++++++++++++++++++++++++++
drivers/mmc/host/sdhci.c | 3 ++-
drivers/mmc/host/sdhci.h | 1 +
3 files changed, 33 insertions(+), 1 deletion(-)
--
2.19.2
Make sdhci_send_command() public so that it can be called from platform
drivers.
Signed-off-by: Faiz Abbas <[email protected]>
---
drivers/mmc/host/sdhci.c | 3 ++-
drivers/mmc/host/sdhci.h | 1 +
2 files changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index a8141ff9be03..76d36e13a011 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,7 @@ 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)
{
diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
index 01002cba1359..49b133babf47 100644
--- a/drivers/mmc/host/sdhci.h
+++ b/drivers/mmc/host/sdhci.h
@@ -798,5 +798,6 @@ 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_finish_command(struct sdhci_host *host);
#endif /* __SDHCI_HW_H */
--
2.19.2
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().
Signed-off-by: Faiz Abbas <[email protected]>
---
drivers/mmc/host/sdhci-omap.c | 36 +++++++++++++++++++++++++++++++++++
1 file changed, 36 insertions(+)
diff --git a/drivers/mmc/host/sdhci-omap.c b/drivers/mmc/host/sdhci-omap.c
index b1a66ca3821a..765873dff7ca 100644
--- a/drivers/mmc/host/sdhci-omap.c
+++ b/drivers/mmc/host/sdhci-omap.c
@@ -797,6 +797,41 @@ 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)
+#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 && (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;
+
+ sdhci_finish_command(host);
+
+ /*
+ * 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 +842,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)
--
2.19.2
Hi Faiz,
On 19/03/19 12:05 PM, Faiz Abbas wrote:
> Make sdhci_send_command() public so that it can be called from platform
> drivers.
This should be sdhci_finish_command here and also in subject
How about using "mmc: sdhci: Export sdhci_finish_command()"?
Thanks
Kishon
>
> Signed-off-by: Faiz Abbas <[email protected]>
> ---
> drivers/mmc/host/sdhci.c | 3 ++-
> drivers/mmc/host/sdhci.h | 1 +
> 2 files changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
> index a8141ff9be03..76d36e13a011 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,7 @@ 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)
> {
> diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
> index 01002cba1359..49b133babf47 100644
> --- a/drivers/mmc/host/sdhci.h
> +++ b/drivers/mmc/host/sdhci.h
> @@ -798,5 +798,6 @@ 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_finish_command(struct sdhci_host *host);
>
> #endif /* __SDHCI_HW_H */
>
Hi Kishon,
On 19/03/19 1:28 PM, Kishon Vijay Abraham I wrote:
> Hi Faiz,
>
> On 19/03/19 12:05 PM, Faiz Abbas wrote:
>> Make sdhci_send_command() public so that it can be called from platform
>> drivers.
>
> This should be sdhci_finish_command here and also in subject
> How about using "mmc: sdhci: Export sdhci_finish_command()"?
>
My bad. Will update to "mmc: sdhci: Export sdhci_finish_command()" in
next version.
Thanks,
Faiz
Hi Faiz,
On 19/03/19 12:05 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().
>
> Signed-off-by: Faiz Abbas <[email protected]>
> ---
> drivers/mmc/host/sdhci-omap.c | 36 +++++++++++++++++++++++++++++++++++
> 1 file changed, 36 insertions(+)
>
> diff --git a/drivers/mmc/host/sdhci-omap.c b/drivers/mmc/host/sdhci-omap.c
> index b1a66ca3821a..765873dff7ca 100644
> --- a/drivers/mmc/host/sdhci-omap.c
> +++ b/drivers/mmc/host/sdhci-omap.c
> @@ -797,6 +797,41 @@ 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)
> +#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 && (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)
CMD_ERR_MASK doesn't have SDHCI_INT_TIMEOUT. So this will never be true.
Thanks
Kishon
Hi Kishon,
On 19/03/19 1:37 PM, Kishon Vijay Abraham I wrote:
> Hi Faiz,
>
> On 19/03/19 12:05 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().
>>
>> Signed-off-by: Faiz Abbas <[email protected]>
>> ---
>> drivers/mmc/host/sdhci-omap.c | 36 +++++++++++++++++++++++++++++++++++
>> 1 file changed, 36 insertions(+)
>>
>> diff --git a/drivers/mmc/host/sdhci-omap.c b/drivers/mmc/host/sdhci-omap.c
>> index b1a66ca3821a..765873dff7ca 100644
>> --- a/drivers/mmc/host/sdhci-omap.c
>> +++ b/drivers/mmc/host/sdhci-omap.c
>> @@ -797,6 +797,41 @@ 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)
>> +#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 && (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)
>
> CMD_ERR_MASK doesn't have SDHCI_INT_TIMEOUT. So this will never be true.
>
Yeah, need to SDHCI_INT_TIMEOUT to CMD_ERR_MASK.
Thanks,
Faiz