diff mbox

[v3,07/25] mmc: sdhci: command response CRC error handling

Message ID E1aO3qT-0005iB-CJ@rmk-PC.arm.linux.org.uk (mailing list archive)
State New, archived
Headers show

Commit Message

Russell King Jan. 26, 2016, 1:39 p.m. UTC
When we get a response CRC error on a command, it means that the
response we received back from the card was not correct.  It does not
mean that the card did not receive the command correctly.  If the
command is one which initiates a data transfer, the card can enter the
data transfer state, and start sending data.

Moreover, if the request contained a data phase, we do not clean this
up, and this results in the driver triggering DMA API debug warnings,
and also creates a race condition in the driver, between running the
finish_tasklet and the data transfer interrupts, which can trigger a
"Got data interrupt" state dump.

Fix this by handing a response CRC error slightly differently: record
the failure of the data initiating command, but allow the remainder of
the request to be processed normally.  This is safe as core MMC checks
the status of all commands and data transfer phases of the request.

If the card does not initiate a data transfer, then we should time out
according to the data transfer parameters.

Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
---
 drivers/mmc/host/sdhci.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

Comments

kernel test robot Jan. 26, 2016, 2:10 p.m. UTC | #1
Hi Russell,

[auto build test WARNING on ulf.hansson-mmc/next]
[also build test WARNING on v4.5-rc1 next-20160125]
[if your patch is applied to the wrong git tree, please drop us a note to help improving the system]

url:    https://github.com/0day-ci/linux/commits/Russell-King/mmc-core-shut-up-voltage-ranges-unspecified-pr_info/20160126-214738
base:   https://git.linaro.org/people/ulf.hansson/mmc next
config: x86_64-randconfig-x018-01260845 (attached as .config)
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All warnings (new ones prefixed by >>):

   In file included from include/linux/linkage.h:4:0,
                    from include/linux/kernel.h:6,
                    from include/linux/delay.h:10,
                    from drivers/mmc/host/sdhci.c:16:
   drivers/mmc/host/sdhci.c: In function 'sdhci_cmd_irq':
   drivers/mmc/host/sdhci.c:2344:15: warning: suggest parentheses around comparison in operand of '&' [-Wparentheses]
          intmask & (SDHCI_INT_CRC | SDHCI_INT_TIMEOUT) ==
                  ^
   include/linux/compiler.h:147:28: note: in definition of macro '__trace_if'
     if (__builtin_constant_p((cond)) ? !!(cond) :   \
                               ^
>> drivers/mmc/host/sdhci.c:2343:3: note: in expansion of macro 'if'
      if (host->cmd->data &&
      ^
   drivers/mmc/host/sdhci.c:2344:15: warning: suggest parentheses around comparison in operand of '&' [-Wparentheses]
          intmask & (SDHCI_INT_CRC | SDHCI_INT_TIMEOUT) ==
                  ^
   include/linux/compiler.h:147:40: note: in definition of macro '__trace_if'
     if (__builtin_constant_p((cond)) ? !!(cond) :   \
                                           ^
>> drivers/mmc/host/sdhci.c:2343:3: note: in expansion of macro 'if'
      if (host->cmd->data &&
      ^
   drivers/mmc/host/sdhci.c:2344:15: warning: suggest parentheses around comparison in operand of '&' [-Wparentheses]
          intmask & (SDHCI_INT_CRC | SDHCI_INT_TIMEOUT) ==
                  ^
   include/linux/compiler.h:158:16: note: in definition of macro '__trace_if'
      ______r = !!(cond);     \
                   ^
>> drivers/mmc/host/sdhci.c:2343:3: note: in expansion of macro 'if'
      if (host->cmd->data &&
      ^

vim +/if +2343 drivers/mmc/host/sdhci.c

  2327			       SDHCI_INT_END_BIT | SDHCI_INT_INDEX)) {
  2328			if (intmask & SDHCI_INT_TIMEOUT)
  2329				host->cmd->error = -ETIMEDOUT;
  2330			else
  2331				host->cmd->error = -EILSEQ;
  2332	
  2333			/*
  2334			 * If this command initiates a data phase and a response
  2335			 * CRC error is signalled, the card can start transferring
  2336			 * data - the card may have received the command without
  2337			 * error.  We must not terminate the mmc_request early.
  2338			 *
  2339			 * If the card did not receive the command or returned an
  2340			 * error which prevented it sending data, the data phase
  2341			 * will time out.
  2342			 */
> 2343			if (host->cmd->data &&
  2344			    intmask & (SDHCI_INT_CRC | SDHCI_INT_TIMEOUT) ==
  2345			     SDHCI_INT_CRC) {
  2346				host->cmd = NULL;
  2347				return;
  2348			}
  2349	
  2350			tasklet_schedule(&host->finish_tasklet);
  2351			return;

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
kernel test robot Jan. 26, 2016, 2:11 p.m. UTC | #2
Hi Russell,

[auto build test WARNING on ulf.hansson-mmc/next]
[also build test WARNING on v4.5-rc1 next-20160125]
[if your patch is applied to the wrong git tree, please drop us a note to help improving the system]

url:    https://github.com/0day-ci/linux/commits/Russell-King/mmc-core-shut-up-voltage-ranges-unspecified-pr_info/20160126-214738
base:   https://git.linaro.org/people/ulf.hansson/mmc next
config: x86_64-randconfig-x010-01260845 (attached as .config)
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All warnings (new ones prefixed by >>):

   drivers/mmc/host/sdhci.c: In function 'sdhci_cmd_irq':
>> drivers/mmc/host/sdhci.c:2344:15: warning: suggest parentheses around comparison in operand of '&' [-Wparentheses]
          intmask & (SDHCI_INT_CRC | SDHCI_INT_TIMEOUT) ==
                  ^

vim +2344 drivers/mmc/host/sdhci.c

  2328			if (intmask & SDHCI_INT_TIMEOUT)
  2329				host->cmd->error = -ETIMEDOUT;
  2330			else
  2331				host->cmd->error = -EILSEQ;
  2332	
  2333			/*
  2334			 * If this command initiates a data phase and a response
  2335			 * CRC error is signalled, the card can start transferring
  2336			 * data - the card may have received the command without
  2337			 * error.  We must not terminate the mmc_request early.
  2338			 *
  2339			 * If the card did not receive the command or returned an
  2340			 * error which prevented it sending data, the data phase
  2341			 * will time out.
  2342			 */
  2343			if (host->cmd->data &&
> 2344			    intmask & (SDHCI_INT_CRC | SDHCI_INT_TIMEOUT) ==
  2345			     SDHCI_INT_CRC) {
  2346				host->cmd = NULL;
  2347				return;
  2348			}
  2349	
  2350			tasklet_schedule(&host->finish_tasklet);
  2351			return;
  2352		}

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
diff mbox

Patch

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index ada227da39a6..cbc4145daaf8 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -2330,6 +2330,23 @@  static void sdhci_cmd_irq(struct sdhci_host *host, u32 intmask, u32 *mask)
 		else
 			host->cmd->error = -EILSEQ;
 
+		/*
+		 * If this command initiates a data phase and a response
+		 * CRC error is signalled, the card can start transferring
+		 * data - the card may have received the command without
+		 * error.  We must not terminate the mmc_request early.
+		 *
+		 * If the card did not receive the command or returned an
+		 * error which prevented it sending data, the data phase
+		 * will time out.
+		 */
+		if (host->cmd->data &&
+		    intmask & (SDHCI_INT_CRC | SDHCI_INT_TIMEOUT) ==
+		     SDHCI_INT_CRC) {
+			host->cmd = NULL;
+			return;
+		}
+
 		tasklet_schedule(&host->finish_tasklet);
 		return;
 	}