Skip to contentSkip to conversation

mmc: mmci: Improvements and bugfixes for SDIO

PATCH New Ulf Hansson 1 participant 7 messages Dec 12, 2011 – Dec 13, 2011

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

cover mmc: mmci: Improvements and bugfixes for SDIO

This patchserie is based upon the below stated patchserie:
mmc: mmci: Improved PM support, cleanup and bugfixes

These patches are all related to the SDIO support. Some patches will anyway effect the code execution path for non-SDIO and more important also the "hotpath" for all PIO read transfers. The intent is to try to affect as little as possible.

Per Forlin (1):
mmc: mmci: Add constraints on alignment for SDIO

Stefan Nilsson XK (1):
mmc: mmci: Fix PIO read for small SDIO packets

Ulf Hansson (3):
mmc: mmci: Prepare for SDIO before setting up DMA job
mmc: mmci: Fix incorrect handling of HW flow control for SDIO
mmc: mmci: Support any block sizes for ux500v2 variant

 drivers/mmc/host/mmci.c |  120 +++++++++++++++++++++++++++++++++++-----------
 drivers/mmc/host/mmci.h |    7 +++
 2 files changed, 98 insertions(+), 29 deletions(-)
Signature · 1 line
1.7.5.4
#1 lore raw

PATCH 1/5 mmc: mmci: Prepare for SDIO before setting up DMA job

Move the SDIO preparation to be done before the DMA job is setup. This makes it possible to do DMA for SDIO transfers as well as the earlier supported pio mode.

---
 drivers/mmc/host/mmci.c |   10 +++++-----
 1 files changed, 5 insertions(+), 5 deletions(-)
drivers/mmc/host/mmci.c +5 -5
index b98ee98..d311f83 100644
@@ -638,6 +638,11 @@ static void mmci_start_data(struct mmci_host *host, struct mmc_data *data)
 	if (data->flags & MMC_DATA_READ)
 		datactrl |= MCI_DPSM_DIRECTION;
 
+	/* The ST Micro variants has a special bit to enable SDIO */
+	if (variant->sdio && host->mmc->card)
+		if (mmc_card_sdio(host->mmc->card))
+			datactrl |= MCI_ST_DPSM_SDIOEN;
+
 	/*
 	 * Attempt to use DMA operation mode, if this
 	 * should fail, fall back to PIO mode
@@ -666,11 +671,6 @@ static void mmci_start_data(struct mmci_host *host, struct mmc_data *data)
 		irqmask = MCI_TXFIFOHALFEMPTYMASK;
 	}
 
-	/* The ST Micro variants has a special bit to enable SDIO */
-	if (variant->sdio && host->mmc->card)
-		if (mmc_card_sdio(host->mmc->card))
-			datactrl |= MCI_ST_DPSM_SDIOEN;
-
 	writel(datactrl, base + MMCIDATACTRL);
 	writel(readl(base + MMCIMASK0) & ~MCI_DATAENDMASK, base + MMCIMASK0);
 	mmci_set_mask1(host, irqmask);
Signature · 1 line
1.7.5.4
#2 lore raw

PATCH 2/5 mmc: mmci: Fix incorrect handling of HW flow control for SDIO

In reply to Ulf Hansson

For data writes smaller than 8 bytes (only SDIO case), HW flow control was disabled but never re-enabled again. This meant that a following large read request could randomly give buffer overrun errors.

This patch is based upon a patch from Stefan Nilsson.

---
 drivers/mmc/host/mmci.c |   38 ++++++++++++++++++++------------------
 1 files changed, 20 insertions(+), 18 deletions(-)
drivers/mmc/host/mmci.c +20 -18
index d311f83..b949e07 100644
@@ -638,10 +638,28 @@ static void mmci_start_data(struct mmci_host *host, struct mmc_data *data)
 	if (data->flags & MMC_DATA_READ)
 		datactrl |= MCI_DPSM_DIRECTION;
 
-	/* The ST Micro variants has a special bit to enable SDIO */
 	if (variant->sdio && host->mmc->card)
-		if (mmc_card_sdio(host->mmc->card))
+		if (mmc_card_sdio(host->mmc->card)) {
+
+			/*
+			 * The ST Micro variant for SDIO write transfer sizes
+			 * less then 8 bytes must have clock H/W flow control
+			 * disabled.
+			 */
+			u32 clk;
+			if ((host->size < 8) && (data->flags & MMC_DATA_WRITE))
+				clk = host->clk_reg & ~variant->clkreg_enable;
+			else
+				clk = host->clk_reg | variant->clkreg_enable;
+
+			mmci_write_clkreg(host, clk);
+
+			/*
+			 * The ST Micro variants has a special bit
+			 * to enable SDIO.
+			 */
 			datactrl |= MCI_ST_DPSM_SDIOEN;
+		}
 
 	/*
 	 * Attempt to use DMA operation mode, if this
@@ -846,22 +864,6 @@ static int mmci_pio_write(struct mmci_host *host, char *buffer, unsigned int rem
 		count = min(remain, maxcnt);
 
 		/*
-		 * The ST Micro variant for SDIO transfer sizes
-		 * less then 8 bytes should have clock H/W flow
-		 * control disabled.
-		 */
-		if (variant->sdio &&
-		    mmc_card_sdio(host->mmc->card)) {
-			u32 clk;
-			if (count < 8)
-				clk = host->clk_reg & ~variant->clkreg_enable;
-			else
-				clk = host->clk_reg | variant->clkreg_enable;
-
-			mmci_write_clkreg(host, clk);
-		}
-
-		/*
 		 * SDIO especially may want to send something that is
 		 * not divisible by 4 (as opposed to card sectors
 		 * etc), and the FIFO only accept full 32-bit writes.
Signature · 1 line
1.7.5.4
#3 lore raw

PATCH 3/5 mmc: mmci: Add constraints on alignment for SDIO

In reply to Ulf Hansson

From: Per Forlin <[email protected]>

Buffers must be 4-byte aligned due to restrictions that the PL18x FIFO accesses must be done in a 4-byte aligned manner.

Moreover DPSM_DMAREQCTL must be enabled for SDIO to support writes for non 32-byte aligned sg element lengths. In PIO mode any buffer length can be handled as long as the buffer address is 4-byte aligned.

---
 drivers/mmc/host/mmci.c |   51 +++++++++++++++++++++++++++++++++++++++++-----
 drivers/mmc/host/mmci.h |    7 ++++++
 2 files changed, 52 insertions(+), 6 deletions(-)
drivers/mmc/host/mmci.c +45 -6
index b949e07..9ea2f13 100644
@@ -45,6 +45,7 @@ static unsigned int fmax = 515633;
  * struct variant_data - MMCI variant-specific quirks
  * @clkreg: default value for MCICLOCK register
  * @clkreg_enable: enable value for MMCICLOCK register
+ * @dma_sdio_req_ctrl: enable value for DMAREQCTL register for SDIO write
  * @datalength_bits: number of bits in the MMCIDATALENGTH register
  * @fifosize: number of bytes that can be written when MMCI_TXFIFOEMPTY
  *	      is asserted (likewise for RX)
@@ -60,6 +61,7 @@ static unsigned int fmax = 515633;
 struct variant_data {
 	unsigned int		clkreg;
 	unsigned int		clkreg_enable;
+	unsigned int		dma_sdio_req_ctrl;
 	unsigned int		datalength_bits;
 	unsigned int		fifosize;
 	unsigned int		fifohalfsize;
@@ -102,6 +104,7 @@ static struct variant_data variant_ux500 = {
 	.fifohalfsize		= 8 * 4,
 	.clkreg			= MCI_CLK_ENABLE,
 	.clkreg_enable		= MCI_ST_UX500_HWFCEN,
+	.dma_sdio_req_ctrl	= MCI_ST_DPSM_DMAREQCTL,
 	.datalength_bits	= 24,
 	.sdio			= true,
 	.st_clkdiv		= true,
@@ -114,6 +117,7 @@ static struct variant_data variant_ux500v2 = {
 	.fifohalfsize		= 8 * 4,
 	.clkreg			= MCI_CLK_ENABLE,
 	.clkreg_enable		= MCI_ST_UX500_HWFCEN,
+	.dma_sdio_req_ctrl	= MCI_ST_DPSM_DMAREQCTL,
 	.datalength_bits	= 24,
 	.sdio			= true,
 	.st_clkdiv		= true,
@@ -123,6 +127,30 @@ static struct variant_data variant_ux500v2 = {
 };
 
 /*
+ * Validate mmc prerequisites
+ */
+static int mmci_validate_data(struct mmci_host *host,
+			      struct mmc_data *data)
+{
+	if (!data)
+		return 0;
+
+	if (!is_power_of_2(data->blksz)) {
+		dev_err(mmc_dev(host->mmc),
+			"unsupported block size (%d bytes)\n", data->blksz);
+		return -EINVAL;
+	}
+
+	if (data->sg->offset & 3) {
+		dev_err(mmc_dev(host->mmc),
+			"unsupported alignment (0x%x)\n", data->sg->offset);
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+/*
  * This must be called with host->lock held
  */
 static void mmci_write_clkreg(struct mmci_host *host, u32 clk)
@@ -432,8 +460,12 @@ static int mmci_dma_prep_data(struct mmci_host *host, struct mmc_data *data,
 	if (!chan)
 		return -EINVAL;
 
-	/* If less than or equal to the fifo size, don't bother with DMA */
-	if (data->blksz * data->blocks <= variant->fifosize)
+	/*
+	 * If less than or equal to the fifo size, don't bother with DMA.
+	 * SDIO transfers may not be 4-byte aligned, fall back to PIO.
+	 */
+	if (data->blksz * data->blocks <= variant->fifosize ||
+	    (data->blksz * data->blocks) & 3)
 		return -EINVAL;
 
 	device = chan->device;
@@ -468,6 +500,7 @@ static int mmci_dma_start_data(struct mmci_host *host, unsigned int datactrl)
 {
 	int ret;
 	struct mmc_data *data = host->data;
+	struct variant_data *variant = host->variant;
 
 	ret = mmci_dma_prep_data(host, host->data, NULL);
 	if (ret)
@@ -482,6 +515,11 @@ static int mmci_dma_start_data(struct mmci_host *host, unsigned int datactrl)
 
 	datactrl |= MCI_DPSM_DMAENABLE;
 
+	/* Some hardware versions need special flags for SDIO DMA write. */
+	if (variant->sdio && host->mmc->card && mmc_card_sdio(host->mmc->card)
+	    && (data->flags & MMC_DATA_WRITE))
+		datactrl |= variant->dma_sdio_req_ctrl;
+
 	/* Trigger the DMA transfer */
 	writel(datactrl, host->base + MMCIDATACTRL);
 
@@ -526,6 +564,9 @@ static void mmci_pre_request(struct mmc_host *mmc, struct mmc_request *mrq,
 	if (!data)
 		return;
 
+	if (mmci_validate_data(host, mrq->data))
+		return;
+
 	if (data->host_cookie) {
 		data->host_cookie = 0;
 		return;
@@ -1019,10 +1060,8 @@ static void mmci_request(struct mmc_host *mmc, struct mmc_request *mrq)
 
 	WARN_ON(host->mrq != NULL);
 
-	if (mrq->data && !is_power_of_2(mrq->data->blksz)) {
-		dev_err(mmc_dev(mmc), "unsupported block size (%d bytes)\n",
-			mrq->data->blksz);
-		mrq->cmd->error = -EINVAL;
+	mrq->cmd->error = mmci_validate_data(host, mrq->data);
+	if (mrq->cmd->error) {
 		mmc_request_done(mmc, mrq);
 		return;
 	}
drivers/mmc/host/mmci.h +7 -0
index d437ccf..095c01c 100644
@@ -60,6 +60,13 @@
 #define MCI_ST_DPSM_RWMOD	(1 << 10)
 #define MCI_ST_DPSM_SDIOEN	(1 << 11)
 /* Control register extensions in the ST Micro Ux500 versions */
+/*
+ * DMA request control is required for write
+ * if transfer size is not 32-byte aligned.
+ * DMA request control is also needed if the total
+ * transfer size is 32-byte aligned but any of the
+ * sg element lengths are not 32-byte aligned.
+ */
 #define MCI_ST_DPSM_DMAREQCTL	(1 << 12)
 #define MCI_ST_DPSM_DBOOTMODEEN	(1 << 13)
 #define MCI_ST_DPSM_BUSYMODE	(1 << 14)
Signature · 1 line
1.7.5.4
#4 lore raw

PATCH 4/5 mmc: mmci: Fix PIO read for small SDIO packets

In reply to Ulf Hansson

From: Stefan Nilsson XK <[email protected]>

Corrects a bug in MMCI host driver which silently causes small reads (< 4 bytes as only used in SDIO) from PL-18X to fail.

---
 drivers/mmc/host/mmci.c |   19 ++++++++++++++++++-
 1 files changed, 18 insertions(+), 1 deletions(-)
drivers/mmc/host/mmci.c +18 -1
index 9ea2f13..94c04c3 100644
@@ -876,7 +876,24 @@ static int mmci_pio_read(struct mmci_host *host, char *buffer, unsigned int rema
 		if (count <= 0)
 			break;
 
-		readsl(base + MMCIFIFO, ptr, count >> 2);
+		/*
+		 * SDIO especially may want to send something that is
+		 * not divisible by 4 (as opposed to card sectors
+		 * etc). Therefore make sure to always read the last bytes
+		 * while only doing full 32-bit reads towards the FIFO.
+		 */
+		if (unlikely(count & 0x3)) {
+			if (count < 4) {
+				unsigned char buf[4];
+				readsl(base + MMCIFIFO, buf, 1);
+				memcpy(ptr, buf, count);
+			} else {
+				readsl(base + MMCIFIFO, ptr, count >> 2);
+				count &= ~0x3;
+			}
+		} else {
+			readsl(base + MMCIFIFO, ptr, count >> 2);
+		}
 
 		ptr += count;
 		remain -= count;
Signature · 1 line
1.7.5.4
#5 lore raw

PATCH 5/5 mmc: mmci: Support any block sizes for ux500v2 variant

In reply to Ulf Hansson

For the ux500v2 variant of the PL18x block, any block sizes are supported. This will make it possible to decrease data overhead for SDIO transfers.

This patch is based upon a patch from Stefan Nilsson.

---
 drivers/mmc/host/mmci.c |    8 ++++++--
 1 files changed, 6 insertions(+), 2 deletions(-)
drivers/mmc/host/mmci.c +6 -2
index 94c04c3..1c6c874 100644
@@ -57,6 +57,7 @@ static unsigned int fmax = 515633;
  * @pwrreg_powerup: power up value for MMCIPOWER register
  * @signal_direction: input/out direction of bus signals can be indicated
  * @pwrreg_ctrl_power: bits in MMCIPOWER register controls ext. power supply
+ * @any_blksize: true if block any sizes are supported
  */
 struct variant_data {
 	unsigned int		clkreg;
@@ -71,6 +72,7 @@ struct variant_data {
 	u32			pwrreg_powerup;
 	bool			signal_direction;
 	bool			pwrreg_ctrl_power;
+	bool			any_blksize;
 };
 
 static struct variant_data variant_arm = {
@@ -124,6 +126,7 @@ static struct variant_data variant_ux500v2 = {
 	.blksz_datactrl16	= true,
 	.pwrreg_powerup		= MCI_PWR_ON,
 	.signal_direction	= true,
+	.any_blksize		= true,
 };
 
 /*
@@ -132,10 +135,12 @@ static struct variant_data variant_ux500v2 = {
 static int mmci_validate_data(struct mmci_host *host,
 			      struct mmc_data *data)
 {
+	struct variant_data *variant = host->variant;
+
 	if (!data)
 		return 0;
 
-	if (!is_power_of_2(data->blksz)) {
+	if (!is_power_of_2(data->blksz) && !variant->any_blksize) {
 		dev_err(mmc_dev(host->mmc),
 			"unsupported block size (%d bytes)\n", data->blksz);
 		return -EINVAL;
@@ -669,7 +674,6 @@ static void mmci_start_data(struct mmci_host *host, struct mmc_data *data)
 	writel(host->size, base + MMCIDATALENGTH);
 
 	blksz_bits = ffs(data->blksz) - 1;
-	BUG_ON(1 << blksz_bits != data->blksz);
 
 	if (variant->blksz_datactrl16)
 		datactrl = MCI_DPSM_ENABLE | (data->blksz << 16);
Signature · 1 line
1.7.5.4
#6 lore raw

cover mmc: mmci: Improvements and bugfixes for SDIO

In reply to Ulf Hansson

Hi Russell,

I would just like to inform you that I just uploaded these patches into your patchtracker tool. I will ask Linus to help out testing this on RealView PB1176 board as well, since I unfortunately don't own such a board myself.

These patches is also based on Linux 3.2-rc4 and on top of the other mmci patch serie.

Br
Ulf Hansson

Quoted 23 lines · Ulf HANSSON

This patchserie is based upon the below stated patchserie:
mmc: mmci: Improved PM support, cleanup and bugfixes

These patches are all related to the SDIO support. Some patches will anyway effect the code execution path for non-SDIO and more important also the "hotpath" for all PIO read transfers. The intent is to try to affect as little as possible.

Per Forlin (1):
mmc: mmci: Add constraints on alignment for SDIO

Stefan Nilsson XK (1):
mmc: mmci: Fix PIO read for small SDIO packets

Ulf Hansson (3):
mmc: mmci: Prepare for SDIO before setting up DMA job
mmc: mmci: Fix incorrect handling of HW flow control for SDIO
mmc: mmci: Support any block sizes for ux500v2 variant

 drivers/mmc/host/mmci.c |  120 +++++++++++++++++++++++++++++++++++-----------
 drivers/mmc/host/mmci.h |    7 +++
 2 files changed, 98 insertions(+), 29 deletions(-)