Skip to contentSkip to conversation

mmc: meson-gx: further smaller refactorings

PATCH New Heiner Kallweit 3 participants 9 messages Mar 22, 2017 – Mar 24, 2017

Subsystems (2)
Arm/Amlogic Meson SoC support MMC, SD and SDIO
lore
Share & tools
lore

Heiner Kallweit

author
#0 lore raw

cover mmc: meson-gx: further smaller refactorings

Before we come to the functional changes here one more series with smaller refactorings.

Heiner Kallweit (6):
mmc: meson-gx: remove unneeded configuration bit resets
mmc: meson-gx: replace cmd->data in meson_mmc_start_cmd
mmc: meson-gx: improve variable usage in meson_mmc_start_cmd
mmc: meson-gx: improve response reading
mmc: meson-gx: improve setting data->bytes_xfered
mmc: meson-gx: simplify setting timeout configuration parameters

 drivers/mmc/host/meson-gx-mmc.c | 104 ++++++++++++++++++----------------------
 1 file changed, 47 insertions(+), 57 deletions(-)
Signature · 1 line
2.12.0
author
#1 lore raw

PATCH 1/6 mmc: meson-gx: remove unneeded configuration bit resets

The variables holding the configuration bits are zero-initialized. So we can remove all bit resets.

Signed-off-by Heiner Kallweit <[email protected]>
---
 drivers/mmc/host/meson-gx-mmc.c | 5 -----
 1 file changed, 5 deletions(-)
drivers/mmc/host/meson-gx-mmc.c +0 -5
index bd075fef..04a55577 100644
@@ -441,7 +441,6 @@ static void meson_mmc_start_cmd(struct mmc_host *mmc, struct mmc_command *cmd)
 
 	/* Response */
 	if (cmd->flags & MMC_RSP_PRESENT) {
-		desc->cmd_cfg &= ~CMD_CFG_NO_RESP;
 		if (cmd->flags & MMC_RSP_136)
 			desc->cmd_cfg |= CMD_CFG_RESP_128;
 		desc->cmd_cfg |= CMD_CFG_RESP_NUM;

@@ -479,7 +478,6 @@ static void meson_mmc_start_cmd(struct mmc_host *mmc, struct mmc_command *cmd)
 				writel(cfg, host->regs + SD_EMMC_CFG);
 			}
 		} else {
-			desc->cmd_cfg &= ~CMD_CFG_BLOCK_MODE;
 			desc->cmd_cfg |=
 				(cmd->data->blksz & CMD_CFG_LENGTH_MASK) <<
 				CMD_CFG_LENGTH_SHIFT;

@@ -494,15 +492,12 @@ static void meson_mmc_start_cmd(struct mmc_host *mmc, struct mmc_command *cmd)
 					  host->bounce_buf, xfer_bytes);
 			cmd->data->bytes_xfered = xfer_bytes;
 			dma_wmb();
-		} else {
-			desc->cmd_cfg &= ~CMD_CFG_DATA_WR;
 		}
 
 		desc->cmd_data = host->bounce_dma_addr & CMD_DATA_MASK;
 
 		cmd_cfg_timeout = ilog2(SD_EMMC_CMD_TIMEOUT_DATA);
 	} else {
-		desc->cmd_cfg &= ~CMD_CFG_DATA_IO;
 		cmd_cfg_timeout = ilog2(SD_EMMC_CMD_TIMEOUT);
 	}
 	desc->cmd_cfg |= (cmd_cfg_timeout & CMD_CFG_TIMEOUT_MASK) <<
Signature · 1 line
2.12.0
author
#2 lore raw

PATCH 2/6 mmc: meson-gx: replace cmd->data in meson_mmc_start_cmd

In reply to Heiner Kallweit

Replace cmd->data with a local variable to simplify code a little.

Signed-off-by Heiner Kallweit <[email protected]>
---
 drivers/mmc/host/meson-gx-mmc.c | 25 +++++++++++++------------
 1 file changed, 13 insertions(+), 12 deletions(-)
drivers/mmc/host/meson-gx-mmc.c +13 -12
index 04a55577..1b20ec1a 100644
@@ -424,6 +424,7 @@ static void meson_mmc_request_done(struct mmc_host *mmc,
 static void meson_mmc_start_cmd(struct mmc_host *mmc, struct mmc_command *cmd)
 {
 	struct meson_host *host = mmc_priv(mmc);
+	struct mmc_data *data = cmd->data;
 	struct sd_emmc_desc *desc, desc_tmp;
 	u32 cfg;
 	u8 blk_len, cmd_cfg_timeout;

@@ -456,41 +457,41 @@ static void meson_mmc_start_cmd(struct mmc_host *mmc, struct mmc_command *cmd)
 	}
 
 	/* data? */
-	if (cmd->data) {
+	if (data) {
 		desc->cmd_cfg |= CMD_CFG_DATA_IO;
-		if (cmd->data->blocks > 1) {
+		if (data->blocks > 1) {
 			desc->cmd_cfg |= CMD_CFG_BLOCK_MODE;
 			desc->cmd_cfg |=
-				(cmd->data->blocks & CMD_CFG_LENGTH_MASK) <<
+				(data->blocks & CMD_CFG_LENGTH_MASK) <<
 				CMD_CFG_LENGTH_SHIFT;
 
 			/* check if block-size matches, if not update */
 			cfg = readl(host->regs + SD_EMMC_CFG);
 			blk_len = cfg & (CFG_BLK_LEN_MASK << CFG_BLK_LEN_SHIFT);
 			blk_len >>= CFG_BLK_LEN_SHIFT;
-			if (blk_len != ilog2(cmd->data->blksz)) {
+			if (blk_len != ilog2(data->blksz)) {
 				dev_dbg(host->dev, "%s: update blk_len %d -> %d\n",
 					__func__, blk_len,
-					ilog2(cmd->data->blksz));
-				blk_len = ilog2(cmd->data->blksz);
+					ilog2(data->blksz));
+				blk_len = ilog2(data->blksz);
 				cfg &= ~(CFG_BLK_LEN_MASK << CFG_BLK_LEN_SHIFT);
 				cfg |= blk_len << CFG_BLK_LEN_SHIFT;
 				writel(cfg, host->regs + SD_EMMC_CFG);
 			}
 		} else {
 			desc->cmd_cfg |=
-				(cmd->data->blksz & CMD_CFG_LENGTH_MASK) <<
+				(data->blksz & CMD_CFG_LENGTH_MASK) <<
 				CMD_CFG_LENGTH_SHIFT;
 		}
 
-		cmd->data->bytes_xfered = 0;
-		xfer_bytes = cmd->data->blksz * cmd->data->blocks;
-		if (cmd->data->flags & MMC_DATA_WRITE) {
+		data->bytes_xfered = 0;
+		xfer_bytes = data->blksz * data->blocks;
+		if (data->flags & MMC_DATA_WRITE) {
 			desc->cmd_cfg |= CMD_CFG_DATA_WR;
 			WARN_ON(xfer_bytes > host->bounce_buf_size);
-			sg_copy_to_buffer(cmd->data->sg, cmd->data->sg_len,
+			sg_copy_to_buffer(data->sg, data->sg_len,
 					  host->bounce_buf, xfer_bytes);
-			cmd->data->bytes_xfered = xfer_bytes;
+			data->bytes_xfered = xfer_bytes;
 			dma_wmb();
 		}
Signature · 1 line
2.12.0
author
#3 lore raw

PATCH 3/6 mmc: meson-gx: improve variable usage in meson_mmc_start_cmd

In reply to Heiner Kallweit

Simplify the code by using two variables instead of a struct sd_emmc_desc.

Signed-off-by Heiner Kallweit <[email protected]>
---
 drivers/mmc/host/meson-gx-mmc.c | 55 ++++++++++++++++++-----------------------
 1 file changed, 24 insertions(+), 31 deletions(-)
drivers/mmc/host/meson-gx-mmc.c +24 -31
index 1b20ec1a..8bf3eb54 100644
@@ -425,45 +425,39 @@ static void meson_mmc_start_cmd(struct mmc_host *mmc, struct mmc_command *cmd)
 {
 	struct meson_host *host = mmc_priv(mmc);
 	struct mmc_data *data = cmd->data;
-	struct sd_emmc_desc *desc, desc_tmp;
-	u32 cfg;
+	u32 cfg, cmd_cfg = 0, cmd_data = 0;
 	u8 blk_len, cmd_cfg_timeout;
 	unsigned int xfer_bytes = 0;
 
 	/* Setup descriptors */
 	dma_rmb();
-	desc = &desc_tmp;
-	memset(desc, 0, sizeof(struct sd_emmc_desc));
 
-	desc->cmd_cfg |= (cmd->opcode & CMD_CFG_CMD_INDEX_MASK)	<<
-		CMD_CFG_CMD_INDEX_SHIFT;
-	desc->cmd_cfg |= CMD_CFG_OWNER;  /* owned by CPU */
-	desc->cmd_arg = cmd->arg;
+	cmd_cfg |= (cmd->opcode & CMD_CFG_CMD_INDEX_MASK) <<
+		   CMD_CFG_CMD_INDEX_SHIFT;
+	cmd_cfg |= CMD_CFG_OWNER;  /* owned by CPU */
 
 	/* Response */
 	if (cmd->flags & MMC_RSP_PRESENT) {
 		if (cmd->flags & MMC_RSP_136)
-			desc->cmd_cfg |= CMD_CFG_RESP_128;
-		desc->cmd_cfg |= CMD_CFG_RESP_NUM;
-		desc->cmd_resp = 0;
+			cmd_cfg |= CMD_CFG_RESP_128;
+		cmd_cfg |= CMD_CFG_RESP_NUM;
 
 		if (!(cmd->flags & MMC_RSP_CRC))
-			desc->cmd_cfg |= CMD_CFG_RESP_NOCRC;
+			cmd_cfg |= CMD_CFG_RESP_NOCRC;
 
 		if (cmd->flags & MMC_RSP_BUSY)
-			desc->cmd_cfg |= CMD_CFG_R1B;
+			cmd_cfg |= CMD_CFG_R1B;
 	} else {
-		desc->cmd_cfg |= CMD_CFG_NO_RESP;
+		cmd_cfg |= CMD_CFG_NO_RESP;
 	}
 
 	/* data? */
 	if (data) {
-		desc->cmd_cfg |= CMD_CFG_DATA_IO;
+		cmd_cfg |= CMD_CFG_DATA_IO;
 		if (data->blocks > 1) {
-			desc->cmd_cfg |= CMD_CFG_BLOCK_MODE;
-			desc->cmd_cfg |=
-				(data->blocks & CMD_CFG_LENGTH_MASK) <<
-				CMD_CFG_LENGTH_SHIFT;
+			cmd_cfg |= CMD_CFG_BLOCK_MODE;
+			cmd_cfg |= (data->blocks & CMD_CFG_LENGTH_MASK) <<
+				   CMD_CFG_LENGTH_SHIFT;
 
 			/* check if block-size matches, if not update */
 			cfg = readl(host->regs + SD_EMMC_CFG);

@@ -479,15 +473,14 @@ static void meson_mmc_start_cmd(struct mmc_host *mmc, struct mmc_command *cmd)
 				writel(cfg, host->regs + SD_EMMC_CFG);
 			}
 		} else {
-			desc->cmd_cfg |=
-				(data->blksz & CMD_CFG_LENGTH_MASK) <<
-				CMD_CFG_LENGTH_SHIFT;
+			cmd_cfg |= (data->blksz & CMD_CFG_LENGTH_MASK) <<
+				   CMD_CFG_LENGTH_SHIFT;
 		}
 
 		data->bytes_xfered = 0;
 		xfer_bytes = data->blksz * data->blocks;
 		if (data->flags & MMC_DATA_WRITE) {
-			desc->cmd_cfg |= CMD_CFG_DATA_WR;
+			cmd_cfg |= CMD_CFG_DATA_WR;
 			WARN_ON(xfer_bytes > host->bounce_buf_size);
 			sg_copy_to_buffer(data->sg, data->sg_len,
 					  host->bounce_buf, xfer_bytes);

@@ -495,24 +488,24 @@ static void meson_mmc_start_cmd(struct mmc_host *mmc, struct mmc_command *cmd)
 			dma_wmb();
 		}
 
-		desc->cmd_data = host->bounce_dma_addr & CMD_DATA_MASK;
+		cmd_data = host->bounce_dma_addr & CMD_DATA_MASK;
 
 		cmd_cfg_timeout = ilog2(SD_EMMC_CMD_TIMEOUT_DATA);
 	} else {
 		cmd_cfg_timeout = ilog2(SD_EMMC_CMD_TIMEOUT);
 	}
-	desc->cmd_cfg |= (cmd_cfg_timeout & CMD_CFG_TIMEOUT_MASK) <<
-		CMD_CFG_TIMEOUT_SHIFT;
+	cmd_cfg |= (cmd_cfg_timeout & CMD_CFG_TIMEOUT_MASK) <<
+		   CMD_CFG_TIMEOUT_SHIFT;
 
 	host->cmd = cmd;
 
 	/* Last descriptor */
-	desc->cmd_cfg |= CMD_CFG_END_OF_CHAIN;
-	writel(desc->cmd_cfg, host->regs + SD_EMMC_CMD_CFG);
-	writel(desc->cmd_data, host->regs + SD_EMMC_CMD_DAT);
-	writel(desc->cmd_resp, host->regs + SD_EMMC_CMD_RSP);
+	cmd_cfg |= CMD_CFG_END_OF_CHAIN;
+	writel(cmd_cfg, host->regs + SD_EMMC_CMD_CFG);
+	writel(cmd_data, host->regs + SD_EMMC_CMD_DAT);
+	writel(0, host->regs + SD_EMMC_CMD_RSP);
 	wmb(); /* ensure descriptor is written before kicked */
-	writel(desc->cmd_arg, host->regs + SD_EMMC_CMD_ARG);
+	writel(cmd->arg, host->regs + SD_EMMC_CMD_ARG);
 }
 
 static void meson_mmc_request(struct mmc_host *mmc, struct mmc_request *mrq)
Signature · 1 line
2.12.0
author
#4 lore raw

PATCH 4/6 mmc: meson-gx: improve response reading

In reply to Heiner Kallweit

Response reading can be slightly improved by doing it in just one place.

Signed-off-by Heiner Kallweit <[email protected]>
---
 drivers/mmc/host/meson-gx-mmc.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)
drivers/mmc/host/meson-gx-mmc.c +3 -4
index 8bf3eb54..c4ddd820 100644
@@ -562,6 +562,8 @@ static irqreturn_t meson_mmc_irq(int irq, void *dev_id)
 		goto out;
 	}
 
+	meson_mmc_read_resp(host->mmc, cmd);
+
 	cmd->error = 0;
 	if (status & IRQ_RXD_ERR_MASK) {
 		dev_dbg(host->dev, "Unhandled IRQ: RXD error\n");

@@ -608,10 +610,8 @@ static irqreturn_t meson_mmc_irq(int irq, void *dev_id)
 	/* ack all (enabled) interrupts */
 	writel(status, host->regs + SD_EMMC_STATUS);
 
-	if (ret == IRQ_HANDLED) {
-		meson_mmc_read_resp(host->mmc, cmd);
+	if (ret == IRQ_HANDLED)
 		meson_mmc_request_done(host->mmc, cmd->mrq);
-	}
 
 	spin_unlock(&host->lock);
 	return ret;

@@ -636,7 +636,6 @@ static irqreturn_t meson_mmc_irq_thread(int irq, void *dev_id)
 		data->bytes_xfered = xfer_bytes;
 	}
 
-	meson_mmc_read_resp(host->mmc, cmd);
 	if (!data || !data->stop || cmd->mrq->sbc)
 		meson_mmc_request_done(host->mmc, cmd->mrq);
 	else
Signature · 1 line
2.12.0
author
#5 lore raw

PATCH 5/6 mmc: meson-gx: improve setting data->bytes_xfered

In reply to Heiner Kallweit

Centralize setting data->bytes_xfered in one place and make sure that the number of transfererd bytes is set only if we receive the end-of-chain interrupt and there was no error.

Signed-off-by Heiner Kallweit <[email protected]>
---
 drivers/mmc/host/meson-gx-mmc.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)
drivers/mmc/host/meson-gx-mmc.c +7 -4
index c4ddd820..87303404 100644
@@ -484,7 +484,6 @@ static void meson_mmc_start_cmd(struct mmc_host *mmc, struct mmc_command *cmd)
 			WARN_ON(xfer_bytes > host->bounce_buf_size);
 			sg_copy_to_buffer(data->sg, data->sg_len,
 					  host->bounce_buf, xfer_bytes);
-			data->bytes_xfered = xfer_bytes;
 			dma_wmb();
 		}
 

@@ -539,6 +538,7 @@ static irqreturn_t meson_mmc_irq(int irq, void *dev_id)
 {
 	struct meson_host *host = dev_id;
 	struct mmc_command *cmd;
+	struct mmc_data *data;
 	u32 irq_en, status, raw_status;
 	irqreturn_t ret = IRQ_HANDLED;
 

@@ -550,6 +550,8 @@ static irqreturn_t meson_mmc_irq(int irq, void *dev_id)
 	if (WARN_ON(!cmd))
 		return IRQ_NONE;
 
+	data = cmd->data;
+
 	spin_lock(&host->lock);
 	irq_en = readl(host->regs + SD_EMMC_IRQ_EN);
 	raw_status = readl(host->regs + SD_EMMC_STATUS);

@@ -590,9 +592,11 @@ static irqreturn_t meson_mmc_irq(int irq, void *dev_id)
 	if (status & IRQ_SDIO)
 		dev_dbg(host->dev, "Unhandled IRQ: SDIO.\n");
 
-	if (status & (IRQ_END_OF_CHAIN | IRQ_RESP_STATUS))
+	if (status & (IRQ_END_OF_CHAIN | IRQ_RESP_STATUS)) {
+		if (data && !cmd->error)
+			data->bytes_xfered = data->blksz * data->blocks;
 		ret = IRQ_WAKE_THREAD;
-	else  {
+	} else {
 		dev_warn(host->dev, "Unknown IRQ! status=0x%04x: MMC CMD%u arg=0x%08x flags=0x%08x stop=%d\n",
 			 status, cmd->opcode, cmd->arg,
 			 cmd->flags, cmd->mrq->stop ? 1 : 0);

@@ -633,7 +637,6 @@ static irqreturn_t meson_mmc_irq_thread(int irq, void *dev_id)
 		WARN_ON(xfer_bytes > host->bounce_buf_size);
 		sg_copy_from_buffer(data->sg, data->sg_len,
 				    host->bounce_buf, xfer_bytes);
-		data->bytes_xfered = xfer_bytes;
 	}
 
 	if (!data || !data->stop || cmd->mrq->sbc)
Signature · 1 line
2.12.0
author
#6 lore raw

PATCH 6/6 mmc: meson-gx: simplify setting timeout configuration parameters

In reply to Heiner Kallweit

We don't need variable cmd_cfg_timeout, so remove it and simplify the code a little.

Signed-off-by Heiner Kallweit <[email protected]>
---
 drivers/mmc/host/meson-gx-mmc.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)
drivers/mmc/host/meson-gx-mmc.c +5 -6
index 87303404..b917765c 100644
@@ -426,7 +426,7 @@ static void meson_mmc_start_cmd(struct mmc_host *mmc, struct mmc_command *cmd)
 	struct meson_host *host = mmc_priv(mmc);
 	struct mmc_data *data = cmd->data;
 	u32 cfg, cmd_cfg = 0, cmd_data = 0;
-	u8 blk_len, cmd_cfg_timeout;
+	u8 blk_len;
 	unsigned int xfer_bytes = 0;
 
 	/* Setup descriptors */

@@ -454,6 +454,9 @@ static void meson_mmc_start_cmd(struct mmc_host *mmc, struct mmc_command *cmd)
 	/* data? */
 	if (data) {
 		cmd_cfg |= CMD_CFG_DATA_IO;
+		cmd_cfg |= ilog2(SD_EMMC_CMD_TIMEOUT_DATA) <<
+			   CMD_CFG_TIMEOUT_SHIFT;
+
 		if (data->blocks > 1) {
 			cmd_cfg |= CMD_CFG_BLOCK_MODE;
 			cmd_cfg |= (data->blocks & CMD_CFG_LENGTH_MASK) <<

@@ -488,13 +491,9 @@ static void meson_mmc_start_cmd(struct mmc_host *mmc, struct mmc_command *cmd)
 		}
 
 		cmd_data = host->bounce_dma_addr & CMD_DATA_MASK;
-
-		cmd_cfg_timeout = ilog2(SD_EMMC_CMD_TIMEOUT_DATA);
 	} else {
-		cmd_cfg_timeout = ilog2(SD_EMMC_CMD_TIMEOUT);
+		cmd_cfg |= ilog2(SD_EMMC_CMD_TIMEOUT) << CMD_CFG_TIMEOUT_SHIFT;
 	}
-	cmd_cfg |= (cmd_cfg_timeout & CMD_CFG_TIMEOUT_MASK) <<
-		   CMD_CFG_TIMEOUT_SHIFT;
 
 	host->cmd = cmd;
Signature · 1 line
2.12.0
Applied
#7 lore raw

cover mmc: meson-gx: further smaller refactorings

In reply to Heiner Kallweit
Quoted 11 lines · Heiner Kallweit

Before we come to the functional changes here one more series with smaller refactorings.

Heiner Kallweit (6):
mmc: meson-gx: remove unneeded configuration bit resets
mmc: meson-gx: replace cmd->data in meson_mmc_start_cmd
mmc: meson-gx: improve variable usage in meson_mmc_start_cmd
mmc: meson-gx: improve response reading
mmc: meson-gx: improve setting data->bytes_xfered
mmc: meson-gx: simplify setting timeout configuration parameters

Quoted 6 lines
 drivers/mmc/host/meson-gx-mmc.c | 104 ++++++++++++++++++----------------------
 1 file changed, 47 insertions(+), 57 deletions(-)

--
2.12.0

Thanks, applied for next!

Kind regards
Uffe

maintainer
#8 lore raw

cover mmc: meson-gx: further smaller refactorings

In reply to Heiner Kallweit
Quoted 2 lines · Heiner Kallweit

Before we come to the functional changes here one more series with smaller refactorings.

Indeed, in looking at the DMA support series, my suggestion was going to be to separate out the minor cleanups into a separate series. Thanks for anticipating that.

In the future, please also describe in the cover letter what platforms this series was tested on.

Otherwise, this series looks good to me. Thanks again for your continued work in improving this driver. It's greatly appreciated!

For the series:

Reviewed-by Kevin Hilman <[email protected]>

Kevin