lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <4965C120.3010705@nokia.com>
Date:	Thu, 08 Jan 2009 11:02:24 +0200
From:	Adrian Hunter <ext-adrian.hunter@...ia.com>
To:	Jean Pihet <jpihet@...sta.com>
CC:	linux-arm-kernel@...ts.arm.linux.org.uk,
	Tony Lindgren <tony@...mide.com>,
	Pierre Ossman <drzeus-mmc@...eus.cx>,
	linux-kernel@...r.kernel.org, linux-omap@...r.kernel.org,
	Madhusudhan Chikkature <madhu.cr@...com>,
	David Brownell <david-b@...bell.net>
Subject: Re: [PATCH] OMAP: MMC: recover from transfer failures

Jean Pihet wrote:
> Tony, Pierre,
> 
> Here is a patch that fixes a MMC host controller deadlock. The problem happens 
> when removing the MMC/SD device when a transfer is on-going.
> 
> It has been tested on OMAP3430 but this fix should apply to OMAP2 chips as 
> well, as seen from the TRMs.
> 
> What do you think?
> 
> Regards,
> Jean.
> 
>     OMAP: MMC: recover from transfer failures
> 
>     Timeouts during a command that has a data phase can result in the
>     next command issued after the command that failed not being processed,
>     i.e. no interrupt ever occurs to indicate the command has completed.
>     This failure can result in a deadlock.
> 
>     The indication that the mmc host controller is in this error state is
>     that the DATI bit is set in the PSTATE register.  This patch checks
>     to see if the DATI bit is set before starting a new command, and if it
>     is set it resets the data state machine to clear the error.
> 
>     Tested on OMAP3430 chip and intensive MMC/SD device removal while
>     transferring data.
> 
> Regards,
> Jean
> 
> 
> From 48a40925a4d57f6e7e45809a4b5c09b20f9314d7 Mon Sep 17 00:00:00 2001
> From: Jean Pihet <jpihet@...sta.com>
> Date: Tue, 16 Dec 2008 19:44:12 +0100
> Subject: [PATCH] OMAP: MMC: recover from transfer failures
> 
> Timeouts during a command that has a data phase can result in the
> next command issued after the command that failed not being processed,
> i.e. no interrupt ever occurs to indicate the command has completed.
> This failure can result in a deadlock.
> 
> The indication that the mmc host controller is in this error state is
> that the DATI bit is set in the PSTATE register.  This patch checks
> to see if the DATI bit is set before starting a new command, and if it
> is set it resets the data state machine to clear the error.
> 
> Tested on OMAP3430 chip and intensive MMC/SD device removal while
> transferring data.
> 
> Signed-off-by: Andy Lowe <alowe@...sta.com>
> Signed-off-by: Jean Pihet <jpihet@...sta.com>
> ---
>  drivers/mmc/host/omap_hsmmc.c |   11 +++++++++++
>  1 files changed, 11 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c
> index 1fcc544..b7d111e 100644
> --- a/drivers/mmc/host/omap_hsmmc.c
> +++ b/drivers/mmc/host/omap_hsmmc.c
> @@ -44,6 +44,7 @@
>  #define OMAP_HSMMC_RSP54       0x0118
>  #define OMAP_HSMMC_RSP76       0x011C
>  #define OMAP_HSMMC_DATA                0x0120
> +#define OMAP_HSMMC_PSTATE      0x0124
>  #define OMAP_HSMMC_HCTL                0x0128
>  #define OMAP_HSMMC_SYSCTL      0x012C
>  #define OMAP_HSMMC_STAT                0x0130
> @@ -89,6 +90,7 @@
>  #define DUAL_VOLT_OCR_BIT      7
>  #define SRC                    (1 << 25)
>  #define SRD                    (1 << 26)
> +#define DATI                   (1 << 1)
>  
>  /*
>   * FIXME: Most likely all the data using these _DEVID defines should come
> @@ -273,6 +275,15 @@ mmc_omap_start_command(struct mmc_omap_host *host, struct 
> mmc_command *cmd,
>                 mmc_hostname(host->mmc), cmd->opcode, cmd->arg);
>         host->cmd = cmd;
>  
> +       if (OMAP_HSMMC_READ(host->base, PSTATE) & DATI) {
> +               dev_dbg(mmc_dev(host->mmc), "mmc_omap_start_command: "
> +                       " resetting data state machine\n");
> +               OMAP_HSMMC_WRITE(host->base, SYSCTL,
> +                       OMAP_HSMMC_READ(host->base, SYSCTL) | SRD);
> +               while (OMAP_HSMMC_READ(host->base, SYSCTL) & SRD)
> +                       ;
> +       }
> +
>         /*
>          * Clear status bits and enable interrupts
>          */
> 

Why not do the reset in mmc_omap_irq() like the other resets?
e.g.

static irqreturn_t mmc_omap_irq(int irq, void *dev_id)
{
	struct mmc_omap_host *host = dev_id;
	struct mmc_data *data;
	int end_cmd = 0, end_trans = 0, status;

	if (host->cmd == NULL && host->data == NULL) {
		OMAP_HSMMC_WRITE(host->base, STAT,
			OMAP_HSMMC_READ(host->base, STAT));
		return IRQ_HANDLED;
	}

	data = host->data;
	status = OMAP_HSMMC_READ(host->base, STAT);
	dev_dbg(mmc_dev(host->mmc), "IRQ Status is %x\n", status);

	if (status & ERR) {
#ifdef CONFIG_MMC_DEBUG
		mmc_omap_report_irq(host, status);
#endif
		if ((status & CMD_TIMEOUT) ||
			(status & CMD_CRC)) {
			if (host->cmd) {
				if (status & CMD_TIMEOUT) {
					OMAP_HSMMC_WRITE(host->base, SYSCTL,
						OMAP_HSMMC_READ(host->base,
								SYSCTL) | SRC);
					while (OMAP_HSMMC_READ(host->base,
							SYSCTL) & SRC)
						;

					host->cmd->error = -ETIMEDOUT;
				} else {
					host->cmd->error = -EILSEQ;
				}
				end_cmd = 1;
			}
-			if (host->data)
+			if (host->data) {
				mmc_dma_cleanup(host);
+				OMAP_HSMMC_WRITE(host->base, SYSCTL,
+					OMAP_HSMMC_READ(host->base,
+							SYSCTL) | SRD);
+				while (OMAP_HSMMC_READ(host->base,
+						SYSCTL) & SRD)
+					;
+			}
		}
		if ((status & DATA_TIMEOUT) ||
			(status & DATA_CRC)) {
			if (host->data) {
				if (status & DATA_TIMEOUT)
					mmc_dma_cleanup(host);
				else
					host->data->error = -EILSEQ;
				OMAP_HSMMC_WRITE(host->base, SYSCTL,
					OMAP_HSMMC_READ(host->base,
							SYSCTL) | SRD);
				while (OMAP_HSMMC_READ(host->base,
						SYSCTL) & SRD)
					;
				end_trans = 1;
			}
		}
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ