[PATCH 01/18] mmc: meson-gx: set segment host parameters

[PATCH 01/18] mmc: meson-gx: set segment host parameters

[Heiner Kallweit]

Setting the segment parameters boosted the read performance from
13 MB/s to 32 MB/s in my case.
Default for segment size is only 4k (PAGE_SIZE).

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/mmc/host/meson-gx-mmc.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/mmc/host/meson-gx-mmc.c b/drivers/mmc/host/meson-gx-mmc.c
index 5a959783..1ad66f84 100644
--- a/drivers/mmc/host/meson-gx-mmc.c
+++ b/drivers/mmc/host/meson-gx-mmc.c
@@ -118,6 +118,7 @@
 #define SD_EMMC_CFG_RESP_TIMEOUT 256 /* in clock cycles */
 #define SD_EMMC_CFG_CMD_GAP 16 /* in clock cycles */
 #define MUX_CLK_NUM_PARENTS 2
+#define MESON_MMC_MAX_SEGS 512
 
 struct meson_host {
 	struct	device		*dev;
@@ -789,8 +790,10 @@ static int meson_mmc_probe(struct platform_device *pdev)
 	if (ret)
 		goto free_host;
 
+	mmc->max_segs = MESON_MMC_MAX_SEGS;
 	mmc->max_blk_count = CMD_CFG_LENGTH_MASK;
 	mmc->max_req_size = mmc->max_blk_count * mmc->max_blk_size;
+	mmc->max_seg_size = mmc->max_req_size;
 
 	/* data bounce buffer */
 	host->bounce_buf_size = mmc->max_req_size;
-- 
2.11.1

[PATCH 01/18] mmc: meson-gx: set segment host parameters

[Heiner Kallweit]

Setting the segment parameters boosted the read performance from
13 MB/s to 32 MB/s in my case.
Default for segment size is only 4k (PAGE_SIZE).

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/mmc/host/meson-gx-mmc.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/mmc/host/meson-gx-mmc.c b/drivers/mmc/host/meson-gx-mmc.c
index 5a959783..1ad66f84 100644
--- a/drivers/mmc/host/meson-gx-mmc.c
+++ b/drivers/mmc/host/meson-gx-mmc.c
@@ -118,6 +118,7 @@
 #define SD_EMMC_CFG_RESP_TIMEOUT 256 /* in clock cycles */
 #define SD_EMMC_CFG_CMD_GAP 16 /* in clock cycles */
 #define MUX_CLK_NUM_PARENTS 2
+#define MESON_MMC_MAX_SEGS 512
 
 struct meson_host {
 	struct	device		*dev;
@@ -789,8 +790,10 @@ static int meson_mmc_probe(struct platform_device *pdev)
 	if (ret)
 		goto free_host;
 
+	mmc->max_segs = MESON_MMC_MAX_SEGS;
 	mmc->max_blk_count = CMD_CFG_LENGTH_MASK;
 	mmc->max_req_size = mmc->max_blk_count * mmc->max_blk_size;
+	mmc->max_seg_size = mmc->max_req_size;
 
 	/* data bounce buffer */
 	host->bounce_buf_size = mmc->max_req_size;
-- 
2.11.1

[PATCH 02/18] mmc: meson-gx: remove code for unsupported CMD23

CMD23 isn't supported on meson-gx and therefore the "set block count"
command isn't used. So remove this dead code.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/mmc/host/meson-gx-mmc.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/drivers/mmc/host/meson-gx-mmc.c b/drivers/mmc/host/meson-gx-mmc.c
index 1ad66f84..e89bdf5f 100644
--- a/drivers/mmc/host/meson-gx-mmc.c
+++ b/drivers/mmc/host/meson-gx-mmc.c
@@ -555,11 +555,7 @@ static void meson_mmc_request(struct mmc_host *mmc, struct mmc_request *mrq)
 	writel(0, host->regs + SD_EMMC_START);
 
 	host->mrq = mrq;
-
-	if (mrq->sbc)
-		meson_mmc_start_cmd(mmc, mrq->sbc);
-	else
-		meson_mmc_start_cmd(mmc, mrq->cmd);
+	meson_mmc_start_cmd(mmc, mrq->cmd);
 }
 
 static int meson_mmc_read_resp(struct mmc_host *mmc, struct mmc_command *cmd)
@@ -690,7 +686,7 @@ static irqreturn_t meson_mmc_irq_thread(int irq, void *dev_id)
 	}
 
 	meson_mmc_read_resp(host->mmc, cmd);
-	if (!data || !data->stop || mrq->sbc)
+	if (!data || !data->stop)
 		meson_mmc_request_done(host->mmc, mrq);
 	else
 		meson_mmc_start_cmd(host->mmc, data->stop);
-- 
2.11.1

[PATCH 02/18] mmc: meson-gx: remove code for unsupported CMD23

[Heiner Kallweit]

CMD23 isn't supported on meson-gx and therefore the "set block count"
command isn't used. So remove this dead code.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/mmc/host/meson-gx-mmc.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/drivers/mmc/host/meson-gx-mmc.c b/drivers/mmc/host/meson-gx-mmc.c
index 1ad66f84..e89bdf5f 100644
--- a/drivers/mmc/host/meson-gx-mmc.c
+++ b/drivers/mmc/host/meson-gx-mmc.c
@@ -555,11 +555,7 @@ static void meson_mmc_request(struct mmc_host *mmc, struct mmc_request *mrq)
 	writel(0, host->regs + SD_EMMC_START);
 
 	host->mrq = mrq;
-
-	if (mrq->sbc)
-		meson_mmc_start_cmd(mmc, mrq->sbc);
-	else
-		meson_mmc_start_cmd(mmc, mrq->cmd);
+	meson_mmc_start_cmd(mmc, mrq->cmd);
 }
 
 static int meson_mmc_read_resp(struct mmc_host *mmc, struct mmc_command *cmd)
@@ -690,7 +686,7 @@ static irqreturn_t meson_mmc_irq_thread(int irq, void *dev_id)
 	}
 
 	meson_mmc_read_resp(host->mmc, cmd);
-	if (!data || !data->stop || mrq->sbc)
+	if (!data || !data->stop)
 		meson_mmc_request_done(host->mmc, mrq);
 	else
 		meson_mmc_start_cmd(host->mmc, data->stop);
-- 
2.11.1

[PATCH 03/18] mmc: meson-gx: explicitely call stop command for multi-block commands only

[Heiner Kallweit]

Explicitely call stop command for multi-block commands only.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/mmc/host/meson-gx-mmc.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/mmc/host/meson-gx-mmc.c b/drivers/mmc/host/meson-gx-mmc.c
index e89bdf5f..ece38b44 100644
--- a/drivers/mmc/host/meson-gx-mmc.c
+++ b/drivers/mmc/host/meson-gx-mmc.c
@@ -686,10 +686,10 @@ static irqreturn_t meson_mmc_irq_thread(int irq, void *dev_id)
 	}
 
 	meson_mmc_read_resp(host->mmc, cmd);
-	if (!data || !data->stop)
-		meson_mmc_request_done(host->mmc, mrq);
-	else
+	if (mmc_op_multi(cmd->opcode))
 		meson_mmc_start_cmd(host->mmc, data->stop);
+	else
+		meson_mmc_request_done(host->mmc, mrq);
 
 	return IRQ_HANDLED;
 }
-- 
2.11.1

[PATCH 03/18] mmc: meson-gx: explicitely call stop command for multi-block commands only

[Heiner Kallweit]

Explicitely call stop command for multi-block commands only.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/mmc/host/meson-gx-mmc.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/mmc/host/meson-gx-mmc.c b/drivers/mmc/host/meson-gx-mmc.c
index e89bdf5f..ece38b44 100644
--- a/drivers/mmc/host/meson-gx-mmc.c
+++ b/drivers/mmc/host/meson-gx-mmc.c
@@ -686,10 +686,10 @@ static irqreturn_t meson_mmc_irq_thread(int irq, void *dev_id)
 	}
 
 	meson_mmc_read_resp(host->mmc, cmd);
-	if (!data || !data->stop)
-		meson_mmc_request_done(host->mmc, mrq);
-	else
+	if (mmc_op_multi(cmd->opcode))
 		meson_mmc_start_cmd(host->mmc, data->stop);
+	else
+		meson_mmc_request_done(host->mmc, mrq);
 
 	return IRQ_HANDLED;
 }
-- 
2.11.1

[PATCH 04/18] mmc: meson-gx: improve meson_mmc_start_cmd

[Heiner Kallweit]

Remove use of unneeded members cmd_arg and cmd_resp.
Setting SD_EMMC_CMD_RSP is only needed if CMD_CFG_RESP_NUM is set,
so don't write this register in all other cases.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/mmc/host/meson-gx-mmc.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/mmc/host/meson-gx-mmc.c b/drivers/mmc/host/meson-gx-mmc.c
index ece38b44..630e0590 100644
--- a/drivers/mmc/host/meson-gx-mmc.c
+++ b/drivers/mmc/host/meson-gx-mmc.c
@@ -456,7 +456,6 @@ static void meson_mmc_start_cmd(struct mmc_host *mmc, struct mmc_command *cmd)
 	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;
 
 	/* Response */
 	if (cmd->flags & MMC_RSP_PRESENT) {
@@ -464,7 +463,7 @@ static void meson_mmc_start_cmd(struct mmc_host *mmc, struct mmc_command *cmd)
 		if (cmd->flags & MMC_RSP_136)
 			desc->cmd_cfg |= CMD_CFG_RESP_128;
 		desc->cmd_cfg |= CMD_CFG_RESP_NUM;
-		desc->cmd_resp = 0;
+		writel(0, host->regs + SD_EMMC_CMD_RSP);
 
 		if (!(cmd->flags & MMC_RSP_CRC))
 			desc->cmd_cfg |= CMD_CFG_RESP_NOCRC;
@@ -540,9 +539,8 @@ static void meson_mmc_start_cmd(struct mmc_host *mmc, struct mmc_command *cmd)
 	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);
 	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)
-- 
2.11.1

[PATCH 04/18] mmc: meson-gx: improve meson_mmc_start_cmd

[Heiner Kallweit]

Remove use of unneeded members cmd_arg and cmd_resp.
Setting SD_EMMC_CMD_RSP is only needed if CMD_CFG_RESP_NUM is set,
so don't write this register in all other cases.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/mmc/host/meson-gx-mmc.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/mmc/host/meson-gx-mmc.c b/drivers/mmc/host/meson-gx-mmc.c
index ece38b44..630e0590 100644
--- a/drivers/mmc/host/meson-gx-mmc.c
+++ b/drivers/mmc/host/meson-gx-mmc.c
@@ -456,7 +456,6 @@ static void meson_mmc_start_cmd(struct mmc_host *mmc, struct mmc_command *cmd)
 	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;
 
 	/* Response */
 	if (cmd->flags & MMC_RSP_PRESENT) {
@@ -464,7 +463,7 @@ static void meson_mmc_start_cmd(struct mmc_host *mmc, struct mmc_command *cmd)
 		if (cmd->flags & MMC_RSP_136)
 			desc->cmd_cfg |= CMD_CFG_RESP_128;
 		desc->cmd_cfg |= CMD_CFG_RESP_NUM;
-		desc->cmd_resp = 0;
+		writel(0, host->regs + SD_EMMC_CMD_RSP);
 
 		if (!(cmd->flags & MMC_RSP_CRC))
 			desc->cmd_cfg |= CMD_CFG_RESP_NOCRC;
@@ -540,9 +539,8 @@ static void meson_mmc_start_cmd(struct mmc_host *mmc, struct mmc_command *cmd)
 	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);
 	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)
-- 
2.11.1

[PATCH 05/18] mmc: meson-gx: eliminate struct sd_emmc_data

[Heiner Kallweit]

Struct sd_emmc_data is used in meson_mmc_start_cmd only, so we can get
rid of this struct and replace the two used members with local variables.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/mmc/host/meson-gx-mmc.c | 69 +++++++++++++++++------------------------
 1 file changed, 29 insertions(+), 40 deletions(-)

diff --git a/drivers/mmc/host/meson-gx-mmc.c b/drivers/mmc/host/meson-gx-mmc.c
index 630e0590..c7bf0e12 100644
--- a/drivers/mmc/host/meson-gx-mmc.c
+++ b/drivers/mmc/host/meson-gx-mmc.c
@@ -146,12 +146,6 @@ struct meson_host {
 	bool vqmmc_enabled;
 };
 
-struct sd_emmc_desc {
-	u32 cmd_cfg;
-	u32 cmd_arg;
-	u32 cmd_data;
-	u32 cmd_resp;
-};
 #define CMD_CFG_LENGTH_SHIFT 0
 #define CMD_CFG_LENGTH_MASK 0x1ff
 #define CMD_CFG_BLOCK_MODE BIT(9)
@@ -443,45 +437,41 @@ static int meson_mmc_request_done(struct mmc_host *mmc, struct mmc_request *mrq)
 static void meson_mmc_start_cmd(struct mmc_host *mmc, struct mmc_command *cmd)
 {
 	struct meson_host *host = mmc_priv(mmc);
-	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 */
+	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) {
-		desc->cmd_cfg &= ~CMD_CFG_NO_RESP;
+		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;
+			cmd_cfg |= CMD_CFG_RESP_128;
+		cmd_cfg |= CMD_CFG_RESP_NUM;
 		writel(0, host->regs + SD_EMMC_CMD_RSP);
 
 		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 (cmd->data) {
-		desc->cmd_cfg |= CMD_CFG_DATA_IO;
+		cmd_cfg |= CMD_CFG_DATA_IO;
 		if (cmd->data->blocks > 1) {
-			desc->cmd_cfg |= CMD_CFG_BLOCK_MODE;
-			desc->cmd_cfg |=
-				(cmd->data->blocks & CMD_CFG_LENGTH_MASK) <<
-				CMD_CFG_LENGTH_SHIFT;
+			cmd_cfg |= CMD_CFG_BLOCK_MODE;
+			cmd_cfg |= (cmd->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);
@@ -497,48 +487,47 @@ 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;
+			cmd_cfg &= ~CMD_CFG_BLOCK_MODE;
+			cmd_cfg |= (cmd->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) {
-			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(cmd->data->sg, cmd->data->sg_len,
 					  host->bounce_buf, xfer_bytes);
 			cmd->data->bytes_xfered = xfer_bytes;
 			dma_wmb();
 		} else {
-			desc->cmd_cfg &= ~CMD_CFG_DATA_WR;
+			cmd_cfg &= ~CMD_CFG_DATA_WR;
 		}
 
 		if (xfer_bytes > 0) {
-			desc->cmd_cfg &= ~CMD_CFG_DATA_NUM;
-			desc->cmd_data = host->bounce_dma_addr & CMD_DATA_MASK;
+			cmd_cfg &= ~CMD_CFG_DATA_NUM;
+			cmd_data = host->bounce_dma_addr & CMD_DATA_MASK;
 		} else {
 			/* write data to data_addr */
-			desc->cmd_cfg |= CMD_CFG_DATA_NUM;
-			desc->cmd_data = 0;
+			cmd_cfg |= CMD_CFG_DATA_NUM;
+			cmd_data = 0;
 		}
 
 		cmd_cfg_timeout = 12;
 	} else {
-		desc->cmd_cfg &= ~CMD_CFG_DATA_IO;
+		cmd_cfg &= ~CMD_CFG_DATA_IO;
 		cmd_cfg_timeout = 10;
 	}
-	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);
+	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);
 	wmb(); /* ensure descriptor is written before kicked */
 	writel(cmd->arg, host->regs + SD_EMMC_CMD_ARG);
 }
-- 
2.11.1

[PATCH 05/18] mmc: meson-gx: eliminate struct sd_emmc_data

[Heiner Kallweit]

Struct sd_emmc_data is used in meson_mmc_start_cmd only, so we can get
rid of this struct and replace the two used members with local variables.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/mmc/host/meson-gx-mmc.c | 69 +++++++++++++++++------------------------
 1 file changed, 29 insertions(+), 40 deletions(-)

diff --git a/drivers/mmc/host/meson-gx-mmc.c b/drivers/mmc/host/meson-gx-mmc.c
index 630e0590..c7bf0e12 100644
--- a/drivers/mmc/host/meson-gx-mmc.c
+++ b/drivers/mmc/host/meson-gx-mmc.c
@@ -146,12 +146,6 @@ struct meson_host {
 	bool vqmmc_enabled;
 };
 
-struct sd_emmc_desc {
-	u32 cmd_cfg;
-	u32 cmd_arg;
-	u32 cmd_data;
-	u32 cmd_resp;
-};
 #define CMD_CFG_LENGTH_SHIFT 0
 #define CMD_CFG_LENGTH_MASK 0x1ff
 #define CMD_CFG_BLOCK_MODE BIT(9)
@@ -443,45 +437,41 @@ static int meson_mmc_request_done(struct mmc_host *mmc, struct mmc_request *mrq)
 static void meson_mmc_start_cmd(struct mmc_host *mmc, struct mmc_command *cmd)
 {
 	struct meson_host *host = mmc_priv(mmc);
-	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 */
+	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) {
-		desc->cmd_cfg &= ~CMD_CFG_NO_RESP;
+		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;
+			cmd_cfg |= CMD_CFG_RESP_128;
+		cmd_cfg |= CMD_CFG_RESP_NUM;
 		writel(0, host->regs + SD_EMMC_CMD_RSP);
 
 		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 (cmd->data) {
-		desc->cmd_cfg |= CMD_CFG_DATA_IO;
+		cmd_cfg |= CMD_CFG_DATA_IO;
 		if (cmd->data->blocks > 1) {
-			desc->cmd_cfg |= CMD_CFG_BLOCK_MODE;
-			desc->cmd_cfg |=
-				(cmd->data->blocks & CMD_CFG_LENGTH_MASK) <<
-				CMD_CFG_LENGTH_SHIFT;
+			cmd_cfg |= CMD_CFG_BLOCK_MODE;
+			cmd_cfg |= (cmd->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);
@@ -497,48 +487,47 @@ 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;
+			cmd_cfg &= ~CMD_CFG_BLOCK_MODE;
+			cmd_cfg |= (cmd->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) {
-			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(cmd->data->sg, cmd->data->sg_len,
 					  host->bounce_buf, xfer_bytes);
 			cmd->data->bytes_xfered = xfer_bytes;
 			dma_wmb();
 		} else {
-			desc->cmd_cfg &= ~CMD_CFG_DATA_WR;
+			cmd_cfg &= ~CMD_CFG_DATA_WR;
 		}
 
 		if (xfer_bytes > 0) {
-			desc->cmd_cfg &= ~CMD_CFG_DATA_NUM;
-			desc->cmd_data = host->bounce_dma_addr & CMD_DATA_MASK;
+			cmd_cfg &= ~CMD_CFG_DATA_NUM;
+			cmd_data = host->bounce_dma_addr & CMD_DATA_MASK;
 		} else {
 			/* write data to data_addr */
-			desc->cmd_cfg |= CMD_CFG_DATA_NUM;
-			desc->cmd_data = 0;
+			cmd_cfg |= CMD_CFG_DATA_NUM;
+			cmd_data = 0;
 		}
 
 		cmd_cfg_timeout = 12;
 	} else {
-		desc->cmd_cfg &= ~CMD_CFG_DATA_IO;
+		cmd_cfg &= ~CMD_CFG_DATA_IO;
 		cmd_cfg_timeout = 10;
 	}
-	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);
+	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);
 	wmb(); /* ensure descriptor is written before kicked */
 	writel(cmd->arg, host->regs + SD_EMMC_CMD_ARG);
 }
-- 
2.11.1

[PATCH 06/18] mmc: meson-gx: simplify bounce buffer setting in meson_mmc_start_cmd

[Heiner Kallweit]

If xfer_bytes is zero we can configure the chip to use the bounce
buffer as well because it won't get used anyway.
And we don't have to reset bit CMD_CFG_DATA_NUM because cmd_cfg
was zero-initialized and this bit isn't set.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/mmc/host/meson-gx-mmc.c | 10 +---------
 1 file changed, 1 insertion(+), 9 deletions(-)

diff --git a/drivers/mmc/host/meson-gx-mmc.c b/drivers/mmc/host/meson-gx-mmc.c
index c7bf0e12..423b4b17 100644
--- a/drivers/mmc/host/meson-gx-mmc.c
+++ b/drivers/mmc/host/meson-gx-mmc.c
@@ -505,15 +505,7 @@ static void meson_mmc_start_cmd(struct mmc_host *mmc, struct mmc_command *cmd)
 			cmd_cfg &= ~CMD_CFG_DATA_WR;
 		}
 
-		if (xfer_bytes > 0) {
-			cmd_cfg &= ~CMD_CFG_DATA_NUM;
-			cmd_data = host->bounce_dma_addr & CMD_DATA_MASK;
-		} else {
-			/* write data to data_addr */
-			cmd_cfg |= CMD_CFG_DATA_NUM;
-			cmd_data = 0;
-		}
-
+		cmd_data = host->bounce_dma_addr & CMD_DATA_MASK;
 		cmd_cfg_timeout = 12;
 	} else {
 		cmd_cfg &= ~CMD_CFG_DATA_IO;
-- 
2.11.1

[PATCH 06/18] mmc: meson-gx: simplify bounce buffer setting in meson_mmc_start_cmd

[Heiner Kallweit]

If xfer_bytes is zero we can configure the chip to use the bounce
buffer as well because it won't get used anyway.
And we don't have to reset bit CMD_CFG_DATA_NUM because cmd_cfg
was zero-initialized and this bit isn't set.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/mmc/host/meson-gx-mmc.c | 10 +---------
 1 file changed, 1 insertion(+), 9 deletions(-)

diff --git a/drivers/mmc/host/meson-gx-mmc.c b/drivers/mmc/host/meson-gx-mmc.c
index c7bf0e12..423b4b17 100644
--- a/drivers/mmc/host/meson-gx-mmc.c
+++ b/drivers/mmc/host/meson-gx-mmc.c
@@ -505,15 +505,7 @@ static void meson_mmc_start_cmd(struct mmc_host *mmc, struct mmc_command *cmd)
 			cmd_cfg &= ~CMD_CFG_DATA_WR;
 		}
 
-		if (xfer_bytes > 0) {
-			cmd_cfg &= ~CMD_CFG_DATA_NUM;
-			cmd_data = host->bounce_dma_addr & CMD_DATA_MASK;
-		} else {
-			/* write data to data_addr */
-			cmd_cfg |= CMD_CFG_DATA_NUM;
-			cmd_data = 0;
-		}
-
+		cmd_data = host->bounce_dma_addr & CMD_DATA_MASK;
 		cmd_cfg_timeout = 12;
 	} else {
 		cmd_cfg &= ~CMD_CFG_DATA_IO;
-- 
2.11.1

[PATCH 07/18] mmc: meson-gx: check return value of sg_copy_[from,to]_buffer

[Heiner Kallweit]

So far it's only checked whether the requested transfer length exceeds
the bounce buffer size. Instead let's check that sg_copy_[from,to]_buffer
transferred exactly the number of requested bytes.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/mmc/host/meson-gx-mmc.c | 19 ++++++++++---------
 1 file changed, 10 insertions(+), 9 deletions(-)

diff --git a/drivers/mmc/host/meson-gx-mmc.c b/drivers/mmc/host/meson-gx-mmc.c
index 423b4b17..f3963538 100644
--- a/drivers/mmc/host/meson-gx-mmc.c
+++ b/drivers/mmc/host/meson-gx-mmc.c
@@ -495,14 +495,14 @@ static void meson_mmc_start_cmd(struct mmc_host *mmc, struct mmc_command *cmd)
 		cmd->data->bytes_xfered = 0;
 		xfer_bytes = cmd->data->blksz * cmd->data->blocks;
 		if (cmd->data->flags & MMC_DATA_WRITE) {
-			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,
-					  host->bounce_buf, xfer_bytes);
+			size_t len = sg_copy_to_buffer(cmd->data->sg,
+						       cmd->data->sg_len,
+						       host->bounce_buf,
+						       host->bounce_buf_size);
+			WARN_ON(len != xfer_bytes);
 			cmd->data->bytes_xfered = xfer_bytes;
+			cmd_cfg |= CMD_CFG_DATA_WR;
 			dma_wmb();
-		} else {
-			cmd_cfg &= ~CMD_CFG_DATA_WR;
 		}
 
 		cmd_data = host->bounce_dma_addr & CMD_DATA_MASK;
@@ -657,10 +657,11 @@ static irqreturn_t meson_mmc_irq_thread(int irq, void *dev_id)
 
 	data = cmd->data;
 	if (data && data->flags & MMC_DATA_READ) {
+		size_t len = sg_copy_from_buffer(data->sg, data->sg_len,
+						 host->bounce_buf,
+						 host->bounce_buf_size);
 		xfer_bytes = data->blksz * data->blocks;
-		WARN_ON(xfer_bytes > host->bounce_buf_size);
-		sg_copy_from_buffer(data->sg, data->sg_len,
-				    host->bounce_buf, xfer_bytes);
+		WARN_ON(len != xfer_bytes);
 		data->bytes_xfered = xfer_bytes;
 	}
 
-- 
2.11.1

[PATCH 07/18] mmc: meson-gx: check return value of sg_copy_[from,to]_buffer

[Heiner Kallweit]

So far it's only checked whether the requested transfer length exceeds
the bounce buffer size. Instead let's check that sg_copy_[from,to]_buffer
transferred exactly the number of requested bytes.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/mmc/host/meson-gx-mmc.c | 19 ++++++++++---------
 1 file changed, 10 insertions(+), 9 deletions(-)

diff --git a/drivers/mmc/host/meson-gx-mmc.c b/drivers/mmc/host/meson-gx-mmc.c
index 423b4b17..f3963538 100644
--- a/drivers/mmc/host/meson-gx-mmc.c
+++ b/drivers/mmc/host/meson-gx-mmc.c
@@ -495,14 +495,14 @@ static void meson_mmc_start_cmd(struct mmc_host *mmc, struct mmc_command *cmd)
 		cmd->data->bytes_xfered = 0;
 		xfer_bytes = cmd->data->blksz * cmd->data->blocks;
 		if (cmd->data->flags & MMC_DATA_WRITE) {
-			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,
-					  host->bounce_buf, xfer_bytes);
+			size_t len = sg_copy_to_buffer(cmd->data->sg,
+						       cmd->data->sg_len,
+						       host->bounce_buf,
+						       host->bounce_buf_size);
+			WARN_ON(len != xfer_bytes);
 			cmd->data->bytes_xfered = xfer_bytes;
+			cmd_cfg |= CMD_CFG_DATA_WR;
 			dma_wmb();
-		} else {
-			cmd_cfg &= ~CMD_CFG_DATA_WR;
 		}
 
 		cmd_data = host->bounce_dma_addr & CMD_DATA_MASK;
@@ -657,10 +657,11 @@ static irqreturn_t meson_mmc_irq_thread(int irq, void *dev_id)
 
 	data = cmd->data;
 	if (data && data->flags & MMC_DATA_READ) {
+		size_t len = sg_copy_from_buffer(data->sg, data->sg_len,
+						 host->bounce_buf,
+						 host->bounce_buf_size);
 		xfer_bytes = data->blksz * data->blocks;
-		WARN_ON(xfer_bytes > host->bounce_buf_size);
-		sg_copy_from_buffer(data->sg, data->sg_len,
-				    host->bounce_buf, xfer_bytes);
+		WARN_ON(len != xfer_bytes);
 		data->bytes_xfered = xfer_bytes;
 	}
 
-- 
2.11.1

[PATCH 08/18] mmc: meson-gx: make two functions return void

[Heiner Kallweit]

The return value of meson_mmc_request_done and meson_mmc_read_resp
isn't used, so make both functions return void.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/mmc/host/meson-gx-mmc.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/drivers/mmc/host/meson-gx-mmc.c b/drivers/mmc/host/meson-gx-mmc.c
index f3963538..a67c811e 100644
--- a/drivers/mmc/host/meson-gx-mmc.c
+++ b/drivers/mmc/host/meson-gx-mmc.c
@@ -421,7 +421,8 @@ static void meson_mmc_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
 			__func__, orig, val);
 }
 
-static int meson_mmc_request_done(struct mmc_host *mmc, struct mmc_request *mrq)
+static void meson_mmc_request_done(struct mmc_host *mmc,
+				   struct mmc_request *mrq)
 {
 	struct meson_host *host = mmc_priv(mmc);
 
@@ -430,8 +431,6 @@ static int meson_mmc_request_done(struct mmc_host *mmc, struct mmc_request *mrq)
 	host->mrq = NULL;
 	host->cmd = NULL;
 	mmc_request_done(host->mmc, mrq);
-
-	return 0;
 }
 
 static void meson_mmc_start_cmd(struct mmc_host *mmc, struct mmc_command *cmd)
@@ -537,7 +536,7 @@ static void meson_mmc_request(struct mmc_host *mmc, struct mmc_request *mrq)
 	meson_mmc_start_cmd(mmc, mrq->cmd);
 }
 
-static int meson_mmc_read_resp(struct mmc_host *mmc, struct mmc_command *cmd)
+static void meson_mmc_read_resp(struct mmc_host *mmc, struct mmc_command *cmd)
 {
 	struct meson_host *host = mmc_priv(mmc);
 
@@ -549,8 +548,6 @@ static int meson_mmc_read_resp(struct mmc_host *mmc, struct mmc_command *cmd)
 	} else if (cmd->flags & MMC_RSP_PRESENT) {
 		cmd->resp[0] = readl(host->regs + SD_EMMC_CMD_RSP);
 	}
-
-	return 0;
 }
 
 static irqreturn_t meson_mmc_irq(int irq, void *dev_id)
-- 
2.11.1

[PATCH 08/18] mmc: meson-gx: make two functions return void

[Heiner Kallweit]

The return value of meson_mmc_request_done and meson_mmc_read_resp
isn't used, so make both functions return void.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/mmc/host/meson-gx-mmc.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/drivers/mmc/host/meson-gx-mmc.c b/drivers/mmc/host/meson-gx-mmc.c
index f3963538..a67c811e 100644
--- a/drivers/mmc/host/meson-gx-mmc.c
+++ b/drivers/mmc/host/meson-gx-mmc.c
@@ -421,7 +421,8 @@ static void meson_mmc_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
 			__func__, orig, val);
 }
 
-static int meson_mmc_request_done(struct mmc_host *mmc, struct mmc_request *mrq)
+static void meson_mmc_request_done(struct mmc_host *mmc,
+				   struct mmc_request *mrq)
 {
 	struct meson_host *host = mmc_priv(mmc);
 
@@ -430,8 +431,6 @@ static int meson_mmc_request_done(struct mmc_host *mmc, struct mmc_request *mrq)
 	host->mrq = NULL;
 	host->cmd = NULL;
 	mmc_request_done(host->mmc, mrq);
-
-	return 0;
 }
 
 static void meson_mmc_start_cmd(struct mmc_host *mmc, struct mmc_command *cmd)
@@ -537,7 +536,7 @@ static void meson_mmc_request(struct mmc_host *mmc, struct mmc_request *mrq)
 	meson_mmc_start_cmd(mmc, mrq->cmd);
 }
 
-static int meson_mmc_read_resp(struct mmc_host *mmc, struct mmc_command *cmd)
+static void meson_mmc_read_resp(struct mmc_host *mmc, struct mmc_command *cmd)
 {
 	struct meson_host *host = mmc_priv(mmc);
 
@@ -549,8 +548,6 @@ static int meson_mmc_read_resp(struct mmc_host *mmc, struct mmc_command *cmd)
 	} else if (cmd->flags & MMC_RSP_PRESENT) {
 		cmd->resp[0] = readl(host->regs + SD_EMMC_CMD_RSP);
 	}
-
-	return 0;
 }
 
 static irqreturn_t meson_mmc_irq(int irq, void *dev_id)
-- 
2.11.1

[PATCH 09/18] mmc: meson-gx: change interrupt name

[Heiner Kallweit]

So far the driver name is used for the interrupt. Therefore in
/proc/interrupts we can't tell which interrupt belongs to which
SD_EMMC port.
So let's use the device name instead (what is the OF node name).

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/mmc/host/meson-gx-mmc.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/mmc/host/meson-gx-mmc.c b/drivers/mmc/host/meson-gx-mmc.c
index a67c811e..1a76c925 100644
--- a/drivers/mmc/host/meson-gx-mmc.c
+++ b/drivers/mmc/host/meson-gx-mmc.c
@@ -757,9 +757,9 @@ static int meson_mmc_probe(struct platform_device *pdev)
 	writel(IRQ_EN_MASK, host->regs + SD_EMMC_STATUS);
 	writel(IRQ_EN_MASK, host->regs + SD_EMMC_IRQ_EN);
 
-	ret = devm_request_threaded_irq(&pdev->dev, host->irq,
-					meson_mmc_irq, meson_mmc_irq_thread,
-					IRQF_SHARED, DRIVER_NAME, host);
+	ret = devm_request_threaded_irq(&pdev->dev, host->irq, meson_mmc_irq,
+					meson_mmc_irq_thread, IRQF_SHARED,
+					dev_name(&pdev->dev), host);
 	if (ret)
 		goto free_host;
 
-- 
2.11.1

[PATCH 09/18] mmc: meson-gx: change interrupt name

[Heiner Kallweit]

So far the driver name is used for the interrupt. Therefore in
/proc/interrupts we can't tell which interrupt belongs to which
SD_EMMC port.
So let's use the device name instead (what is the OF node name).

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/mmc/host/meson-gx-mmc.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/mmc/host/meson-gx-mmc.c b/drivers/mmc/host/meson-gx-mmc.c
index a67c811e..1a76c925 100644
--- a/drivers/mmc/host/meson-gx-mmc.c
+++ b/drivers/mmc/host/meson-gx-mmc.c
@@ -757,9 +757,9 @@ static int meson_mmc_probe(struct platform_device *pdev)
 	writel(IRQ_EN_MASK, host->regs + SD_EMMC_STATUS);
 	writel(IRQ_EN_MASK, host->regs + SD_EMMC_IRQ_EN);
 
-	ret = devm_request_threaded_irq(&pdev->dev, host->irq,
-					meson_mmc_irq, meson_mmc_irq_thread,
-					IRQF_SHARED, DRIVER_NAME, host);
+	ret = devm_request_threaded_irq(&pdev->dev, host->irq, meson_mmc_irq,
+					meson_mmc_irq_thread, IRQF_SHARED,
+					dev_name(&pdev->dev), host);
 	if (ret)
 		goto free_host;
 
-- 
2.11.1

[PATCH 10/18] mmc: meson-gx: remove unused members irq, ocr_mask from struct meson_host

[Heiner Kallweit]

Member ocr_mask is never used and member irq we can replace with a
local variable in meson_mmc_probe. So let's remove both members.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/mmc/host/meson-gx-mmc.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/drivers/mmc/host/meson-gx-mmc.c b/drivers/mmc/host/meson-gx-mmc.c
index 1a76c925..159ee7b4 100644
--- a/drivers/mmc/host/meson-gx-mmc.c
+++ b/drivers/mmc/host/meson-gx-mmc.c
@@ -128,8 +128,6 @@ struct meson_host {
 
 	spinlock_t lock;
 	void __iomem *regs;
-	int irq;
-	u32 ocr_mask;
 	struct clk *core_clk;
 	struct clk_mux mux;
 	struct clk *mux_clk;
@@ -696,7 +694,7 @@ static int meson_mmc_probe(struct platform_device *pdev)
 	struct resource *res;
 	struct meson_host *host;
 	struct mmc_host *mmc;
-	int ret;
+	int ret, irq;
 
 	mmc = mmc_alloc_host(sizeof(struct meson_host), &pdev->dev);
 	if (!mmc)
@@ -728,8 +726,8 @@ static int meson_mmc_probe(struct platform_device *pdev)
 		goto free_host;
 	}
 
-	host->irq = platform_get_irq(pdev, 0);
-	if (host->irq == 0) {
+	irq = platform_get_irq(pdev, 0);
+	if (!irq) {
 		dev_err(&pdev->dev, "failed to get interrupt resource.\n");
 		ret = -EINVAL;
 		goto free_host;
@@ -757,7 +755,7 @@ static int meson_mmc_probe(struct platform_device *pdev)
 	writel(IRQ_EN_MASK, host->regs + SD_EMMC_STATUS);
 	writel(IRQ_EN_MASK, host->regs + SD_EMMC_IRQ_EN);
 
-	ret = devm_request_threaded_irq(&pdev->dev, host->irq, meson_mmc_irq,
+	ret = devm_request_threaded_irq(&pdev->dev, irq, meson_mmc_irq,
 					meson_mmc_irq_thread, IRQF_SHARED,
 					dev_name(&pdev->dev), host);
 	if (ret)
-- 
2.11.1

[PATCH 10/18] mmc: meson-gx: remove unused members irq, ocr_mask from struct meson_host

[Heiner Kallweit]

Member ocr_mask is never used and member irq we can replace with a
local variable in meson_mmc_probe. So let's remove both members.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/mmc/host/meson-gx-mmc.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/drivers/mmc/host/meson-gx-mmc.c b/drivers/mmc/host/meson-gx-mmc.c
index 1a76c925..159ee7b4 100644
--- a/drivers/mmc/host/meson-gx-mmc.c
+++ b/drivers/mmc/host/meson-gx-mmc.c
@@ -128,8 +128,6 @@ struct meson_host {
 
 	spinlock_t lock;
 	void __iomem *regs;
-	int irq;
-	u32 ocr_mask;
 	struct clk *core_clk;
 	struct clk_mux mux;
 	struct clk *mux_clk;
@@ -696,7 +694,7 @@ static int meson_mmc_probe(struct platform_device *pdev)
 	struct resource *res;
 	struct meson_host *host;
 	struct mmc_host *mmc;
-	int ret;
+	int ret, irq;
 
 	mmc = mmc_alloc_host(sizeof(struct meson_host), &pdev->dev);
 	if (!mmc)
@@ -728,8 +726,8 @@ static int meson_mmc_probe(struct platform_device *pdev)
 		goto free_host;
 	}
 
-	host->irq = platform_get_irq(pdev, 0);
-	if (host->irq == 0) {
+	irq = platform_get_irq(pdev, 0);
+	if (!irq) {
 		dev_err(&pdev->dev, "failed to get interrupt resource.\n");
 		ret = -EINVAL;
 		goto free_host;
@@ -757,7 +755,7 @@ static int meson_mmc_probe(struct platform_device *pdev)
 	writel(IRQ_EN_MASK, host->regs + SD_EMMC_STATUS);
 	writel(IRQ_EN_MASK, host->regs + SD_EMMC_IRQ_EN);
 
-	ret = devm_request_threaded_irq(&pdev->dev, host->irq, meson_mmc_irq,
+	ret = devm_request_threaded_irq(&pdev->dev, irq, meson_mmc_irq,
 					meson_mmc_irq_thread, IRQF_SHARED,
 					dev_name(&pdev->dev), host);
 	if (ret)
-- 
2.11.1

[PATCH 11/18] mmc: meson-gx: remove unneeded variable in meson_mmc_clk_init

[Heiner Kallweit]

Variable mux_parent_count can be replaced with constant
MUX_CLK_NUM_PARENTS, so remove it.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/mmc/host/meson-gx-mmc.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/mmc/host/meson-gx-mmc.c b/drivers/mmc/host/meson-gx-mmc.c
index 159ee7b4..d05ffe8a 100644
--- a/drivers/mmc/host/meson-gx-mmc.c
+++ b/drivers/mmc/host/meson-gx-mmc.c
@@ -237,7 +237,6 @@ static int meson_mmc_clk_init(struct meson_host *host)
 	char clk_name[32];
 	int i, ret = 0;
 	const char *mux_parent_names[MUX_CLK_NUM_PARENTS];
-	unsigned int mux_parent_count = 0;
 	const char *clk_div_parents[1];
 	u32 clk_reg, cfg;
 
@@ -256,7 +255,6 @@ static int meson_mmc_clk_init(struct meson_host *host)
 		}
 
 		mux_parent_names[i] = __clk_get_name(host->mux_parent[i]);
-		mux_parent_count++;
 	}
 
 	/* create the mux */
@@ -265,7 +263,7 @@ static int meson_mmc_clk_init(struct meson_host *host)
 	init.ops = &clk_mux_ops;
 	init.flags = 0;
 	init.parent_names = mux_parent_names;
-	init.num_parents = mux_parent_count;
+	init.num_parents = MUX_CLK_NUM_PARENTS;
 
 	host->mux.reg = host->regs + SD_EMMC_CLOCK;
 	host->mux.shift = CLK_SRC_SHIFT;
-- 
2.11.1

[PATCH 11/18] mmc: meson-gx: remove unneeded variable in meson_mmc_clk_init

[Heiner Kallweit]

Variable mux_parent_count can be replaced with constant
MUX_CLK_NUM_PARENTS, so remove it.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/mmc/host/meson-gx-mmc.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/mmc/host/meson-gx-mmc.c b/drivers/mmc/host/meson-gx-mmc.c
index 159ee7b4..d05ffe8a 100644
--- a/drivers/mmc/host/meson-gx-mmc.c
+++ b/drivers/mmc/host/meson-gx-mmc.c
@@ -237,7 +237,6 @@ static int meson_mmc_clk_init(struct meson_host *host)
 	char clk_name[32];
 	int i, ret = 0;
 	const char *mux_parent_names[MUX_CLK_NUM_PARENTS];
-	unsigned int mux_parent_count = 0;
 	const char *clk_div_parents[1];
 	u32 clk_reg, cfg;
 
@@ -256,7 +255,6 @@ static int meson_mmc_clk_init(struct meson_host *host)
 		}
 
 		mux_parent_names[i] = __clk_get_name(host->mux_parent[i]);
-		mux_parent_count++;
 	}
 
 	/* create the mux */
@@ -265,7 +263,7 @@ static int meson_mmc_clk_init(struct meson_host *host)
 	init.ops = &clk_mux_ops;
 	init.flags = 0;
 	init.parent_names = mux_parent_names;
-	init.num_parents = mux_parent_count;
+	init.num_parents = MUX_CLK_NUM_PARENTS;
 
 	host->mux.reg = host->regs + SD_EMMC_CLOCK;
 	host->mux.shift = CLK_SRC_SHIFT;
-- 
2.11.1

[PATCH 12/18] mmc: meson-gx: remove member parent_mux from struct meson_host

[Heiner Kallweit]

Member mux_parent isn't used outside meson_mmc_clk_init. So remove it
and replace it with a local variable in meson_mmc_clk_init.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/mmc/host/meson-gx-mmc.c | 14 ++++++--------
 1 file changed, 6 insertions(+), 8 deletions(-)

diff --git a/drivers/mmc/host/meson-gx-mmc.c b/drivers/mmc/host/meson-gx-mmc.c
index d05ffe8a..74ceda75 100644
--- a/drivers/mmc/host/meson-gx-mmc.c
+++ b/drivers/mmc/host/meson-gx-mmc.c
@@ -131,7 +131,6 @@ struct meson_host {
 	struct clk *core_clk;
 	struct clk_mux mux;
 	struct clk *mux_clk;
-	struct clk *mux_parent[MUX_CLK_NUM_PARENTS];
 	unsigned long current_clock;
 
 	struct clk_divider cfg_div;
@@ -242,19 +241,18 @@ static int meson_mmc_clk_init(struct meson_host *host)
 
 	/* get the mux parents */
 	for (i = 0; i < MUX_CLK_NUM_PARENTS; i++) {
+		struct clk *clk;
 		char name[16];
 
 		snprintf(name, sizeof(name), "clkin%d", i);
-		host->mux_parent[i] = devm_clk_get(host->dev, name);
-		if (IS_ERR(host->mux_parent[i])) {
-			ret = PTR_ERR(host->mux_parent[i]);
-			if (PTR_ERR(host->mux_parent[i]) != -EPROBE_DEFER)
+		clk = devm_clk_get(host->dev, name);
+		if (IS_ERR(clk)) {
+			if (clk != ERR_PTR(-EPROBE_DEFER))
 				dev_err(host->dev, "Missing clock %s\n", name);
-			host->mux_parent[i] = NULL;
-			return ret;
+			return PTR_ERR(clk);
 		}
 
-		mux_parent_names[i] = __clk_get_name(host->mux_parent[i]);
+		mux_parent_names[i] = __clk_get_name(clk);
 	}
 
 	/* create the mux */
-- 
2.11.1

[PATCH 12/18] mmc: meson-gx: remove member parent_mux from struct meson_host

[Heiner Kallweit]

Member mux_parent isn't used outside meson_mmc_clk_init. So remove it
and replace it with a local variable in meson_mmc_clk_init.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/mmc/host/meson-gx-mmc.c | 14 ++++++--------
 1 file changed, 6 insertions(+), 8 deletions(-)

diff --git a/drivers/mmc/host/meson-gx-mmc.c b/drivers/mmc/host/meson-gx-mmc.c
index d05ffe8a..74ceda75 100644
--- a/drivers/mmc/host/meson-gx-mmc.c
+++ b/drivers/mmc/host/meson-gx-mmc.c
@@ -131,7 +131,6 @@ struct meson_host {
 	struct clk *core_clk;
 	struct clk_mux mux;
 	struct clk *mux_clk;
-	struct clk *mux_parent[MUX_CLK_NUM_PARENTS];
 	unsigned long current_clock;
 
 	struct clk_divider cfg_div;
@@ -242,19 +241,18 @@ static int meson_mmc_clk_init(struct meson_host *host)
 
 	/* get the mux parents */
 	for (i = 0; i < MUX_CLK_NUM_PARENTS; i++) {
+		struct clk *clk;
 		char name[16];
 
 		snprintf(name, sizeof(name), "clkin%d", i);
-		host->mux_parent[i] = devm_clk_get(host->dev, name);
-		if (IS_ERR(host->mux_parent[i])) {
-			ret = PTR_ERR(host->mux_parent[i]);
-			if (PTR_ERR(host->mux_parent[i]) != -EPROBE_DEFER)
+		clk = devm_clk_get(host->dev, name);
+		if (IS_ERR(clk)) {
+			if (clk != ERR_PTR(-EPROBE_DEFER))
 				dev_err(host->dev, "Missing clock %s\n", name);
-			host->mux_parent[i] = NULL;
-			return ret;
+			return PTR_ERR(clk);
 		}
 
-		mux_parent_names[i] = __clk_get_name(host->mux_parent[i]);
+		mux_parent_names[i] = __clk_get_name(clk);
 	}
 
 	/* create the mux */
-- 
2.11.1

[PATCH 13/18] mmc: meson-gx: remove unneeded meson_mmc_clk_set in meson_mmc_clk_init

[Heiner Kallweit]

I don't think this clk_disable_unprepare is right here. We do this
in meson_mmc_remove what should be sufficient.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/mmc/host/meson-gx-mmc.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/drivers/mmc/host/meson-gx-mmc.c b/drivers/mmc/host/meson-gx-mmc.c
index 74ceda75..39904fb5 100644
--- a/drivers/mmc/host/meson-gx-mmc.c
+++ b/drivers/mmc/host/meson-gx-mmc.c
@@ -315,11 +315,7 @@ static int meson_mmc_clk_init(struct meson_host *host)
 	/* Get the nearest minimum clock to 400KHz */
 	host->mmc->f_min = clk_round_rate(host->cfg_div_clk, 400000);
 
-	ret = meson_mmc_clk_set(host, host->mmc->f_min);
-	if (!ret)
-		clk_disable_unprepare(host->cfg_div_clk);
-
-	return ret;
+	return meson_mmc_clk_set(host, host->mmc->f_min);
 }
 
 static void meson_mmc_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
-- 
2.11.1

[PATCH 13/18] mmc: meson-gx: remove unneeded meson_mmc_clk_set in meson_mmc_clk_init

[Heiner Kallweit]

I don't think this clk_disable_unprepare is right here. We do this
in meson_mmc_remove what should be sufficient.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/mmc/host/meson-gx-mmc.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/drivers/mmc/host/meson-gx-mmc.c b/drivers/mmc/host/meson-gx-mmc.c
index 74ceda75..39904fb5 100644
--- a/drivers/mmc/host/meson-gx-mmc.c
+++ b/drivers/mmc/host/meson-gx-mmc.c
@@ -315,11 +315,7 @@ static int meson_mmc_clk_init(struct meson_host *host)
 	/* Get the nearest minimum clock to 400KHz */
 	host->mmc->f_min = clk_round_rate(host->cfg_div_clk, 400000);
 
-	ret = meson_mmc_clk_set(host, host->mmc->f_min);
-	if (!ret)
-		clk_disable_unprepare(host->cfg_div_clk);
-
-	return ret;
+	return meson_mmc_clk_set(host, host->mmc->f_min);
 }
 
 static void meson_mmc_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
-- 
2.11.1

[PATCH 14/18] mmc: meson-gx: remove unneeded devm_kstrdup in meson_mmc_clk_init

[Heiner Kallweit]

MMC core does a deep copy of init.name, therefore it's fully ok to
provide a local variable.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/mmc/host/meson-gx-mmc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mmc/host/meson-gx-mmc.c b/drivers/mmc/host/meson-gx-mmc.c
index 39904fb5..806554b1 100644
--- a/drivers/mmc/host/meson-gx-mmc.c
+++ b/drivers/mmc/host/meson-gx-mmc.c
@@ -276,7 +276,7 @@ static int meson_mmc_clk_init(struct meson_host *host)
 
 	/* create the divider */
 	snprintf(clk_name, sizeof(clk_name), "%s#div", dev_name(host->dev));
-	init.name = devm_kstrdup(host->dev, clk_name, GFP_KERNEL);
+	init.name = clk_name;
 	init.ops = &clk_divider_ops;
 	init.flags = CLK_SET_RATE_PARENT;
 	clk_div_parents[0] = __clk_get_name(host->mux_clk);
-- 
2.11.1

[PATCH 14/18] mmc: meson-gx: remove unneeded devm_kstrdup in meson_mmc_clk_init

[Heiner Kallweit]

MMC core does a deep copy of init.name, therefore it's fully ok to
provide a local variable.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/mmc/host/meson-gx-mmc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mmc/host/meson-gx-mmc.c b/drivers/mmc/host/meson-gx-mmc.c
index 39904fb5..806554b1 100644
--- a/drivers/mmc/host/meson-gx-mmc.c
+++ b/drivers/mmc/host/meson-gx-mmc.c
@@ -276,7 +276,7 @@ static int meson_mmc_clk_init(struct meson_host *host)
 
 	/* create the divider */
 	snprintf(clk_name, sizeof(clk_name), "%s#div", dev_name(host->dev));
-	init.name = devm_kstrdup(host->dev, clk_name, GFP_KERNEL);
+	init.name = clk_name;
 	init.ops = &clk_divider_ops;
 	init.flags = CLK_SET_RATE_PARENT;
 	clk_div_parents[0] = __clk_get_name(host->mux_clk);
-- 
2.11.1

[PATCH 15/18] mmc: meson-gx: improve initial configuration

[Heiner Kallweit]

Config values which are not changed during runtime we can set
in the probe function already.
The block size setting is overwritten later in
meson_mmc_start_cmd anyway if needed, so it doesn't harm if we
remove this setting in meson_mmc_set_ios.

In addition:
- write config register only if configuration changed
- Initialize clocks after other config registers have been initialized

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/mmc/host/meson-gx-mmc.c | 37 +++++++++++++++++++++----------------
 1 file changed, 21 insertions(+), 16 deletions(-)

diff --git a/drivers/mmc/host/meson-gx-mmc.c b/drivers/mmc/host/meson-gx-mmc.c
index 806554b1..25b46d36 100644
--- a/drivers/mmc/host/meson-gx-mmc.c
+++ b/drivers/mmc/host/meson-gx-mmc.c
@@ -385,15 +385,6 @@ static void meson_mmc_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
 	val &= ~(CFG_BUS_WIDTH_MASK << CFG_BUS_WIDTH_SHIFT);
 	val |= bus_width << CFG_BUS_WIDTH_SHIFT;
 
-	val &= ~(CFG_BLK_LEN_MASK << CFG_BLK_LEN_SHIFT);
-	val |= ilog2(SD_EMMC_CFG_BLK_SIZE) << CFG_BLK_LEN_SHIFT;
-
-	val &= ~(CFG_RESP_TIMEOUT_MASK << CFG_RESP_TIMEOUT_SHIFT);
-	val |= ilog2(SD_EMMC_CFG_RESP_TIMEOUT) << CFG_RESP_TIMEOUT_SHIFT;
-
-	val &= ~(CFG_RC_CC_MASK << CFG_RC_CC_SHIFT);
-	val |= ilog2(SD_EMMC_CFG_CMD_GAP) << CFG_RC_CC_SHIFT;
-
 	val &= ~CFG_DDR;
 	if (ios->timing == MMC_TIMING_UHS_DDR50 ||
 	    ios->timing == MMC_TIMING_MMC_DDR52 ||
@@ -404,11 +395,11 @@ static void meson_mmc_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
 	if (ios->timing == MMC_TIMING_MMC_HS400)
 		val |= CFG_CHK_DS;
 
-	writel(val, host->regs + SD_EMMC_CFG);
-
-	if (val != orig)
+	if (val != orig) {
+		writel(val, host->regs + SD_EMMC_CFG);
 		dev_dbg(host->dev, "%s: SD_EMMC_CFG: 0x%08x -> 0x%08x\n",
 			__func__, orig, val);
+	}
 }
 
 static void meson_mmc_request_done(struct mmc_host *mmc,
@@ -675,6 +666,17 @@ static int meson_mmc_get_cd(struct mmc_host *mmc)
 	return status;
 }
 
+static void meson_mmc_cfg_init(struct meson_host *host)
+{
+	u32 cfg = 0;
+
+	cfg |= ilog2(SD_EMMC_CFG_RESP_TIMEOUT) << CFG_RESP_TIMEOUT_SHIFT;
+	cfg |= ilog2(SD_EMMC_CFG_CMD_GAP) << CFG_RC_CC_SHIFT;
+	cfg |= ilog2(SD_EMMC_CFG_BLK_SIZE) << CFG_BLK_LEN_SHIFT;
+
+	writel(cfg, host->regs + SD_EMMC_CFG);
+}
+
 static const struct mmc_host_ops meson_mmc_ops = {
 	.request	= meson_mmc_request,
 	.set_ios	= meson_mmc_set_ios,
@@ -735,10 +737,6 @@ static int meson_mmc_probe(struct platform_device *pdev)
 	if (ret)
 		goto free_host;
 
-	ret = meson_mmc_clk_init(host);
-	if (ret)
-		goto free_host;
-
 	/* Stop execution */
 	writel(0, host->regs + SD_EMMC_START);
 
@@ -747,6 +745,13 @@ static int meson_mmc_probe(struct platform_device *pdev)
 	writel(IRQ_EN_MASK, host->regs + SD_EMMC_STATUS);
 	writel(IRQ_EN_MASK, host->regs + SD_EMMC_IRQ_EN);
 
+	/* set config to sane default */
+	meson_mmc_cfg_init(host);
+
+	ret = meson_mmc_clk_init(host);
+	if (ret)
+		goto free_host;
+
 	ret = devm_request_threaded_irq(&pdev->dev, irq, meson_mmc_irq,
 					meson_mmc_irq_thread, IRQF_SHARED,
 					dev_name(&pdev->dev), host);
-- 
2.11.1

[PATCH 15/18] mmc: meson-gx: improve initial configuration

[Heiner Kallweit]

Config values which are not changed during runtime we can set
in the probe function already.
The block size setting is overwritten later in
meson_mmc_start_cmd anyway if needed, so it doesn't harm if we
remove this setting in meson_mmc_set_ios.

In addition:
- write config register only if configuration changed
- Initialize clocks after other config registers have been initialized

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/mmc/host/meson-gx-mmc.c | 37 +++++++++++++++++++++----------------
 1 file changed, 21 insertions(+), 16 deletions(-)

diff --git a/drivers/mmc/host/meson-gx-mmc.c b/drivers/mmc/host/meson-gx-mmc.c
index 806554b1..25b46d36 100644
--- a/drivers/mmc/host/meson-gx-mmc.c
+++ b/drivers/mmc/host/meson-gx-mmc.c
@@ -385,15 +385,6 @@ static void meson_mmc_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
 	val &= ~(CFG_BUS_WIDTH_MASK << CFG_BUS_WIDTH_SHIFT);
 	val |= bus_width << CFG_BUS_WIDTH_SHIFT;
 
-	val &= ~(CFG_BLK_LEN_MASK << CFG_BLK_LEN_SHIFT);
-	val |= ilog2(SD_EMMC_CFG_BLK_SIZE) << CFG_BLK_LEN_SHIFT;
-
-	val &= ~(CFG_RESP_TIMEOUT_MASK << CFG_RESP_TIMEOUT_SHIFT);
-	val |= ilog2(SD_EMMC_CFG_RESP_TIMEOUT) << CFG_RESP_TIMEOUT_SHIFT;
-
-	val &= ~(CFG_RC_CC_MASK << CFG_RC_CC_SHIFT);
-	val |= ilog2(SD_EMMC_CFG_CMD_GAP) << CFG_RC_CC_SHIFT;
-
 	val &= ~CFG_DDR;
 	if (ios->timing == MMC_TIMING_UHS_DDR50 ||
 	    ios->timing == MMC_TIMING_MMC_DDR52 ||
@@ -404,11 +395,11 @@ static void meson_mmc_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
 	if (ios->timing == MMC_TIMING_MMC_HS400)
 		val |= CFG_CHK_DS;
 
-	writel(val, host->regs + SD_EMMC_CFG);
-
-	if (val != orig)
+	if (val != orig) {
+		writel(val, host->regs + SD_EMMC_CFG);
 		dev_dbg(host->dev, "%s: SD_EMMC_CFG: 0x%08x -> 0x%08x\n",
 			__func__, orig, val);
+	}
 }
 
 static void meson_mmc_request_done(struct mmc_host *mmc,
@@ -675,6 +666,17 @@ static int meson_mmc_get_cd(struct mmc_host *mmc)
 	return status;
 }
 
+static void meson_mmc_cfg_init(struct meson_host *host)
+{
+	u32 cfg = 0;
+
+	cfg |= ilog2(SD_EMMC_CFG_RESP_TIMEOUT) << CFG_RESP_TIMEOUT_SHIFT;
+	cfg |= ilog2(SD_EMMC_CFG_CMD_GAP) << CFG_RC_CC_SHIFT;
+	cfg |= ilog2(SD_EMMC_CFG_BLK_SIZE) << CFG_BLK_LEN_SHIFT;
+
+	writel(cfg, host->regs + SD_EMMC_CFG);
+}
+
 static const struct mmc_host_ops meson_mmc_ops = {
 	.request	= meson_mmc_request,
 	.set_ios	= meson_mmc_set_ios,
@@ -735,10 +737,6 @@ static int meson_mmc_probe(struct platform_device *pdev)
 	if (ret)
 		goto free_host;
 
-	ret = meson_mmc_clk_init(host);
-	if (ret)
-		goto free_host;
-
 	/* Stop execution */
 	writel(0, host->regs + SD_EMMC_START);
 
@@ -747,6 +745,13 @@ static int meson_mmc_probe(struct platform_device *pdev)
 	writel(IRQ_EN_MASK, host->regs + SD_EMMC_STATUS);
 	writel(IRQ_EN_MASK, host->regs + SD_EMMC_IRQ_EN);
 
+	/* set config to sane default */
+	meson_mmc_cfg_init(host);
+
+	ret = meson_mmc_clk_init(host);
+	if (ret)
+		goto free_host;
+
 	ret = devm_request_threaded_irq(&pdev->dev, irq, meson_mmc_irq,
 					meson_mmc_irq_thread, IRQF_SHARED,
 					dev_name(&pdev->dev), host);
-- 
2.11.1

[PATCH 16/18] mmc: meson-gx: improve response reading and sending stop command

[Heiner Kallweit]

Stopping execution at the beginning of command processing (again) is
not needed as reading SD_EMMC_CMD_RSP implicitely stops execution.
We just have to take care of the case that a command doesn't return a
response. Therefore read SD_EMMC_CMD_RSP always.

In addition we can read the response in the main irq handler already,
we don't have to do this in the threaded handler.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/mmc/host/meson-gx-mmc.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/mmc/host/meson-gx-mmc.c b/drivers/mmc/host/meson-gx-mmc.c
index 25b46d36..6721aee2 100644
--- a/drivers/mmc/host/meson-gx-mmc.c
+++ b/drivers/mmc/host/meson-gx-mmc.c
@@ -510,9 +510,6 @@ static void meson_mmc_request(struct mmc_host *mmc, struct mmc_request *mrq)
 
 	WARN_ON(host->mrq != NULL);
 
-	/* Stop execution */
-	writel(0, host->regs + SD_EMMC_START);
-
 	host->mrq = mrq;
 	meson_mmc_start_cmd(mmc, mrq->cmd);
 }
@@ -521,12 +518,16 @@ static void meson_mmc_read_resp(struct mmc_host *mmc, struct mmc_command *cmd)
 {
 	struct meson_host *host = mmc_priv(mmc);
 
+	/*
+	 * reading SD_EMMC_CMD_RSP stops processing, therefore read
+	 * this register even if command has no response
+	 */
 	if (cmd->flags & MMC_RSP_136) {
 		cmd->resp[0] = readl(host->regs + SD_EMMC_CMD_RSP3);
 		cmd->resp[1] = readl(host->regs + SD_EMMC_CMD_RSP2);
 		cmd->resp[2] = readl(host->regs + SD_EMMC_CMD_RSP1);
 		cmd->resp[3] = readl(host->regs + SD_EMMC_CMD_RSP);
-	} else if (cmd->flags & MMC_RSP_PRESENT) {
+	} else {
 		cmd->resp[0] = readl(host->regs + SD_EMMC_CMD_RSP);
 	}
 }
@@ -564,6 +565,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");
@@ -610,10 +613,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;
@@ -643,7 +644,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 (mmc_op_multi(cmd->opcode))
 		meson_mmc_start_cmd(host->mmc, data->stop);
 	else
-- 
2.11.1

[PATCH 16/18] mmc: meson-gx: improve response reading and sending stop command

[Heiner Kallweit]

Stopping execution at the beginning of command processing (again) is
not needed as reading SD_EMMC_CMD_RSP implicitely stops execution.
We just have to take care of the case that a command doesn't return a
response. Therefore read SD_EMMC_CMD_RSP always.

In addition we can read the response in the main irq handler already,
we don't have to do this in the threaded handler.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/mmc/host/meson-gx-mmc.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/mmc/host/meson-gx-mmc.c b/drivers/mmc/host/meson-gx-mmc.c
index 25b46d36..6721aee2 100644
--- a/drivers/mmc/host/meson-gx-mmc.c
+++ b/drivers/mmc/host/meson-gx-mmc.c
@@ -510,9 +510,6 @@ static void meson_mmc_request(struct mmc_host *mmc, struct mmc_request *mrq)
 
 	WARN_ON(host->mrq != NULL);
 
-	/* Stop execution */
-	writel(0, host->regs + SD_EMMC_START);
-
 	host->mrq = mrq;
 	meson_mmc_start_cmd(mmc, mrq->cmd);
 }
@@ -521,12 +518,16 @@ static void meson_mmc_read_resp(struct mmc_host *mmc, struct mmc_command *cmd)
 {
 	struct meson_host *host = mmc_priv(mmc);
 
+	/*
+	 * reading SD_EMMC_CMD_RSP stops processing, therefore read
+	 * this register even if command has no response
+	 */
 	if (cmd->flags & MMC_RSP_136) {
 		cmd->resp[0] = readl(host->regs + SD_EMMC_CMD_RSP3);
 		cmd->resp[1] = readl(host->regs + SD_EMMC_CMD_RSP2);
 		cmd->resp[2] = readl(host->regs + SD_EMMC_CMD_RSP1);
 		cmd->resp[3] = readl(host->regs + SD_EMMC_CMD_RSP);
-	} else if (cmd->flags & MMC_RSP_PRESENT) {
+	} else {
 		cmd->resp[0] = readl(host->regs + SD_EMMC_CMD_RSP);
 	}
 }
@@ -564,6 +565,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");
@@ -610,10 +613,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;
@@ -643,7 +644,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 (mmc_op_multi(cmd->opcode))
 		meson_mmc_start_cmd(host->mmc, data->stop);
 	else
-- 
2.11.1

[PATCH 17/18] mmc: meson-gx: remove member mrq from struct meson_host

[Heiner Kallweit]

Struct mmc_command includes a reference to the related mmc_request.
Therefore we don't have to store mrq separately in struct meson_host.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/mmc/host/meson-gx-mmc.c | 23 ++---------------------
 1 file changed, 2 insertions(+), 21 deletions(-)

diff --git a/drivers/mmc/host/meson-gx-mmc.c b/drivers/mmc/host/meson-gx-mmc.c
index 6721aee2..08ff29be 100644
--- a/drivers/mmc/host/meson-gx-mmc.c
+++ b/drivers/mmc/host/meson-gx-mmc.c
@@ -123,7 +123,6 @@
 struct meson_host {
 	struct	device		*dev;
 	struct	mmc_host	*mmc;
-	struct	mmc_request	*mrq;
 	struct	mmc_command	*cmd;
 
 	spinlock_t lock;
@@ -407,9 +406,6 @@ static void meson_mmc_request_done(struct mmc_host *mmc,
 {
 	struct meson_host *host = mmc_priv(mmc);
 
-	WARN_ON(host->mrq != mrq);
-
-	host->mrq = NULL;
 	host->cmd = NULL;
 	mmc_request_done(host->mmc, mrq);
 }
@@ -506,11 +502,6 @@ static void meson_mmc_start_cmd(struct mmc_host *mmc, struct mmc_command *cmd)
 
 static void meson_mmc_request(struct mmc_host *mmc, struct mmc_request *mrq)
 {
-	struct meson_host *host = mmc_priv(mmc);
-
-	WARN_ON(host->mrq != NULL);
-
-	host->mrq = mrq;
 	meson_mmc_start_cmd(mmc, mrq->cmd);
 }
 
@@ -535,7 +526,6 @@ static void meson_mmc_read_resp(struct mmc_host *mmc, struct mmc_command *cmd)
 static irqreturn_t meson_mmc_irq(int irq, void *dev_id)
 {
 	struct meson_host *host = dev_id;
-	struct mmc_request *mrq;
 	struct mmc_command *cmd;
 	u32 irq_en, status, raw_status;
 	irqreturn_t ret = IRQ_HANDLED;
@@ -545,11 +535,6 @@ static irqreturn_t meson_mmc_irq(int irq, void *dev_id)
 
 	cmd = host->cmd;
 
-	mrq = host->mrq;
-
-	if (WARN_ON(!mrq))
-		return IRQ_NONE;
-
 	if (WARN_ON(!cmd))
 		return IRQ_NONE;
 
@@ -598,7 +583,7 @@ static irqreturn_t meson_mmc_irq(int irq, void *dev_id)
 	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, mrq->stop ? 1 : 0);
+			 cmd->flags, cmd->mrq->stop ? 1 : 0);
 		if (cmd->data) {
 			struct mmc_data *data = cmd->data;
 
@@ -623,14 +608,10 @@ static irqreturn_t meson_mmc_irq(int irq, void *dev_id)
 static irqreturn_t meson_mmc_irq_thread(int irq, void *dev_id)
 {
 	struct meson_host *host = dev_id;
-	struct mmc_request *mrq = host->mrq;
 	struct mmc_command *cmd = host->cmd;
 	struct mmc_data *data;
 	unsigned int xfer_bytes;
 
-	if (WARN_ON(!mrq))
-		return IRQ_NONE;
-
 	if (WARN_ON(!cmd))
 		return IRQ_NONE;
 
@@ -647,7 +628,7 @@ static irqreturn_t meson_mmc_irq_thread(int irq, void *dev_id)
 	if (mmc_op_multi(cmd->opcode))
 		meson_mmc_start_cmd(host->mmc, data->stop);
 	else
-		meson_mmc_request_done(host->mmc, mrq);
+		meson_mmc_request_done(host->mmc, cmd->mrq);
 
 	return IRQ_HANDLED;
 }
-- 
2.11.1

[PATCH 17/18] mmc: meson-gx: remove member mrq from struct meson_host

[Heiner Kallweit]

Struct mmc_command includes a reference to the related mmc_request.
Therefore we don't have to store mrq separately in struct meson_host.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/mmc/host/meson-gx-mmc.c | 23 ++---------------------
 1 file changed, 2 insertions(+), 21 deletions(-)

diff --git a/drivers/mmc/host/meson-gx-mmc.c b/drivers/mmc/host/meson-gx-mmc.c
index 6721aee2..08ff29be 100644
--- a/drivers/mmc/host/meson-gx-mmc.c
+++ b/drivers/mmc/host/meson-gx-mmc.c
@@ -123,7 +123,6 @@
 struct meson_host {
 	struct	device		*dev;
 	struct	mmc_host	*mmc;
-	struct	mmc_request	*mrq;
 	struct	mmc_command	*cmd;
 
 	spinlock_t lock;
@@ -407,9 +406,6 @@ static void meson_mmc_request_done(struct mmc_host *mmc,
 {
 	struct meson_host *host = mmc_priv(mmc);
 
-	WARN_ON(host->mrq != mrq);
-
-	host->mrq = NULL;
 	host->cmd = NULL;
 	mmc_request_done(host->mmc, mrq);
 }
@@ -506,11 +502,6 @@ static void meson_mmc_start_cmd(struct mmc_host *mmc, struct mmc_command *cmd)
 
 static void meson_mmc_request(struct mmc_host *mmc, struct mmc_request *mrq)
 {
-	struct meson_host *host = mmc_priv(mmc);
-
-	WARN_ON(host->mrq != NULL);
-
-	host->mrq = mrq;
 	meson_mmc_start_cmd(mmc, mrq->cmd);
 }
 
@@ -535,7 +526,6 @@ static void meson_mmc_read_resp(struct mmc_host *mmc, struct mmc_command *cmd)
 static irqreturn_t meson_mmc_irq(int irq, void *dev_id)
 {
 	struct meson_host *host = dev_id;
-	struct mmc_request *mrq;
 	struct mmc_command *cmd;
 	u32 irq_en, status, raw_status;
 	irqreturn_t ret = IRQ_HANDLED;
@@ -545,11 +535,6 @@ static irqreturn_t meson_mmc_irq(int irq, void *dev_id)
 
 	cmd = host->cmd;
 
-	mrq = host->mrq;
-
-	if (WARN_ON(!mrq))
-		return IRQ_NONE;
-
 	if (WARN_ON(!cmd))
 		return IRQ_NONE;
 
@@ -598,7 +583,7 @@ static irqreturn_t meson_mmc_irq(int irq, void *dev_id)
 	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, mrq->stop ? 1 : 0);
+			 cmd->flags, cmd->mrq->stop ? 1 : 0);
 		if (cmd->data) {
 			struct mmc_data *data = cmd->data;
 
@@ -623,14 +608,10 @@ static irqreturn_t meson_mmc_irq(int irq, void *dev_id)
 static irqreturn_t meson_mmc_irq_thread(int irq, void *dev_id)
 {
 	struct meson_host *host = dev_id;
-	struct mmc_request *mrq = host->mrq;
 	struct mmc_command *cmd = host->cmd;
 	struct mmc_data *data;
 	unsigned int xfer_bytes;
 
-	if (WARN_ON(!mrq))
-		return IRQ_NONE;
-
 	if (WARN_ON(!cmd))
 		return IRQ_NONE;
 
@@ -647,7 +628,7 @@ static irqreturn_t meson_mmc_irq_thread(int irq, void *dev_id)
 	if (mmc_op_multi(cmd->opcode))
 		meson_mmc_start_cmd(host->mmc, data->stop);
 	else
-		meson_mmc_request_done(host->mmc, mrq);
+		meson_mmc_request_done(host->mmc, cmd->mrq);
 
 	return IRQ_HANDLED;
 }
-- 
2.11.1

[PATCH 18/18] mmc: meson-gx: move handling of one case from threaded handler to main irq

[Heiner Kallweit]

If cmd->data is NULL there's nothing to do in the threaded handler.
So we can handle this case in the main irq handler already.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/mmc/host/meson-gx-mmc.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/mmc/host/meson-gx-mmc.c b/drivers/mmc/host/meson-gx-mmc.c
index 08ff29be..58b0add2 100644
--- a/drivers/mmc/host/meson-gx-mmc.c
+++ b/drivers/mmc/host/meson-gx-mmc.c
@@ -578,9 +578,10 @@ 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))
-		ret = IRQ_WAKE_THREAD;
-	else  {
+	if (status & (IRQ_END_OF_CHAIN | IRQ_RESP_STATUS)) {
+		if (cmd->data)
+			ret = IRQ_WAKE_THREAD;
+	} 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);
@@ -612,11 +613,11 @@ static irqreturn_t meson_mmc_irq_thread(int irq, void *dev_id)
 	struct mmc_data *data;
 	unsigned int xfer_bytes;
 
-	if (WARN_ON(!cmd))
+	if (WARN_ON(!cmd || !cmd->data))
 		return IRQ_NONE;
 
 	data = cmd->data;
-	if (data && data->flags & MMC_DATA_READ) {
+	if (data->flags & MMC_DATA_READ) {
 		size_t len = sg_copy_from_buffer(data->sg, data->sg_len,
 						 host->bounce_buf,
 						 host->bounce_buf_size);
-- 
2.11.1

[PATCH 18/18] mmc: meson-gx: move handling of one case from threaded handler to main irq

[Heiner Kallweit]

If cmd->data is NULL there's nothing to do in the threaded handler.
So we can handle this case in the main irq handler already.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/mmc/host/meson-gx-mmc.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/mmc/host/meson-gx-mmc.c b/drivers/mmc/host/meson-gx-mmc.c
index 08ff29be..58b0add2 100644
--- a/drivers/mmc/host/meson-gx-mmc.c
+++ b/drivers/mmc/host/meson-gx-mmc.c
@@ -578,9 +578,10 @@ 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))
-		ret = IRQ_WAKE_THREAD;
-	else  {
+	if (status & (IRQ_END_OF_CHAIN | IRQ_RESP_STATUS)) {
+		if (cmd->data)
+			ret = IRQ_WAKE_THREAD;
+	} 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);
@@ -612,11 +613,11 @@ static irqreturn_t meson_mmc_irq_thread(int irq, void *dev_id)
 	struct mmc_data *data;
 	unsigned int xfer_bytes;
 
-	if (WARN_ON(!cmd))
+	if (WARN_ON(!cmd || !cmd->data))
 		return IRQ_NONE;
 
 	data = cmd->data;
-	if (data && data->flags & MMC_DATA_READ) {
+	if (data->flags & MMC_DATA_READ) {
 		size_t len = sg_copy_from_buffer(data->sg, data->sg_len,
 						 host->bounce_buf,
 						 host->bounce_buf_size);
-- 
2.11.1

Re: [PATCH 09/18] mmc: meson-gx: change interrupt name

[Ben Dooks]

On 14/02/17 20:06, Heiner Kallweit wrote:
> So far the driver name is used for the interrupt. Therefore in
> /proc/interrupts we can't tell which interrupt belongs to which
> SD_EMMC port.
> So let's use the device name instead (what is the OF node name).
>
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
> ---
>  drivers/mmc/host/meson-gx-mmc.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/mmc/host/meson-gx-mmc.c b/drivers/mmc/host/meson-gx-mmc.c
> index a67c811e..1a76c925 100644
> --- a/drivers/mmc/host/meson-gx-mmc.c
> +++ b/drivers/mmc/host/meson-gx-mmc.c
> @@ -757,9 +757,9 @@ static int meson_mmc_probe(struct platform_device *pdev)
>  	writel(IRQ_EN_MASK, host->regs + SD_EMMC_STATUS);
>  	writel(IRQ_EN_MASK, host->regs + SD_EMMC_IRQ_EN);
>
> -	ret = devm_request_threaded_irq(&pdev->dev, host->irq,
> -					meson_mmc_irq, meson_mmc_irq_thread,
> -					IRQF_SHARED, DRIVER_NAME, host);
> +	ret = devm_request_threaded_irq(&pdev->dev, host->irq, meson_mmc_irq,
> +					meson_mmc_irq_thread, IRQF_SHARED,
> +					dev_name(&pdev->dev), host);
>  	if (ret)
>  		goto free_host;
>

I am surpised that the default is not to use dev_name(dev), I wonder if
that should be something that devm_request_threaded_irq() could be
updated to do.


-- 
Ben Dooks				http://www.codethink.co.uk/
Senior Engineer				Codethink - Providing Genius

[PATCH 09/18] mmc: meson-gx: change interrupt name

[Ben Dooks]

On 14/02/17 20:06, Heiner Kallweit wrote:
> So far the driver name is used for the interrupt. Therefore in
> /proc/interrupts we can't tell which interrupt belongs to which
> SD_EMMC port.
> So let's use the device name instead (what is the OF node name).
>
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
> ---
>  drivers/mmc/host/meson-gx-mmc.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/mmc/host/meson-gx-mmc.c b/drivers/mmc/host/meson-gx-mmc.c
> index a67c811e..1a76c925 100644
> --- a/drivers/mmc/host/meson-gx-mmc.c
> +++ b/drivers/mmc/host/meson-gx-mmc.c
> @@ -757,9 +757,9 @@ static int meson_mmc_probe(struct platform_device *pdev)
>  	writel(IRQ_EN_MASK, host->regs + SD_EMMC_STATUS);
>  	writel(IRQ_EN_MASK, host->regs + SD_EMMC_IRQ_EN);
>
> -	ret = devm_request_threaded_irq(&pdev->dev, host->irq,
> -					meson_mmc_irq, meson_mmc_irq_thread,
> -					IRQF_SHARED, DRIVER_NAME, host);
> +	ret = devm_request_threaded_irq(&pdev->dev, host->irq, meson_mmc_irq,
> +					meson_mmc_irq_thread, IRQF_SHARED,
> +					dev_name(&pdev->dev), host);
>  	if (ret)
>  		goto free_host;
>

I am surpised that the default is not to use dev_name(dev), I wonder if
that should be something that devm_request_threaded_irq() could be
updated to do.


-- 
Ben Dooks				http://www.codethink.co.uk/
Senior Engineer				Codethink - Providing Genius

Re: [PATCH 02/18] mmc: meson-gx: remove code for unsupported CMD23

[Kevin Hilman]

Heiner Kallweit <hkallweit1@gmail.com> writes:

> CMD23 isn't supported on meson-gx and therefore the "set block count"
> command isn't used. So remove this dead code.

I'm admittedly a bit unfamiliar with the MMC spec and wrote this driver
mostly by looking at the vendor driver, so forgive the silly
questions...

Why isn't CMD23 supported, and should we support instead of deleting
this support?

Kevin

> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
> ---
>  drivers/mmc/host/meson-gx-mmc.c | 8 ++------
>  1 file changed, 2 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/mmc/host/meson-gx-mmc.c b/drivers/mmc/host/meson-gx-mmc.c
> index 1ad66f84..e89bdf5f 100644
> --- a/drivers/mmc/host/meson-gx-mmc.c
> +++ b/drivers/mmc/host/meson-gx-mmc.c
> @@ -555,11 +555,7 @@ static void meson_mmc_request(struct mmc_host *mmc, struct mmc_request *mrq)
>  	writel(0, host->regs + SD_EMMC_START);
>  
>  	host->mrq = mrq;
> -
> -	if (mrq->sbc)
> -		meson_mmc_start_cmd(mmc, mrq->sbc);
> -	else
> -		meson_mmc_start_cmd(mmc, mrq->cmd);
> +	meson_mmc_start_cmd(mmc, mrq->cmd);
>  }
>  
>  static int meson_mmc_read_resp(struct mmc_host *mmc, struct mmc_command *cmd)
> @@ -690,7 +686,7 @@ static irqreturn_t meson_mmc_irq_thread(int irq, void *dev_id)
>  	}
>  
>  	meson_mmc_read_resp(host->mmc, cmd);
> -	if (!data || !data->stop || mrq->sbc)
> +	if (!data || !data->stop)
>  		meson_mmc_request_done(host->mmc, mrq);
>  	else
>  		meson_mmc_start_cmd(host->mmc, data->stop);

[PATCH 02/18] mmc: meson-gx: remove code for unsupported CMD23

[Kevin Hilman]

Heiner Kallweit <hkallweit1@gmail.com> writes:

> CMD23 isn't supported on meson-gx and therefore the "set block count"
> command isn't used. So remove this dead code.

I'm admittedly a bit unfamiliar with the MMC spec and wrote this driver
mostly by looking at the vendor driver, so forgive the silly
questions...

Why isn't CMD23 supported, and should we support instead of deleting
this support?

Kevin

> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
> ---
>  drivers/mmc/host/meson-gx-mmc.c | 8 ++------
>  1 file changed, 2 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/mmc/host/meson-gx-mmc.c b/drivers/mmc/host/meson-gx-mmc.c
> index 1ad66f84..e89bdf5f 100644
> --- a/drivers/mmc/host/meson-gx-mmc.c
> +++ b/drivers/mmc/host/meson-gx-mmc.c
> @@ -555,11 +555,7 @@ static void meson_mmc_request(struct mmc_host *mmc, struct mmc_request *mrq)
>  	writel(0, host->regs + SD_EMMC_START);
>  
>  	host->mrq = mrq;
> -
> -	if (mrq->sbc)
> -		meson_mmc_start_cmd(mmc, mrq->sbc);
> -	else
> -		meson_mmc_start_cmd(mmc, mrq->cmd);
> +	meson_mmc_start_cmd(mmc, mrq->cmd);
>  }
>  
>  static int meson_mmc_read_resp(struct mmc_host *mmc, struct mmc_command *cmd)
> @@ -690,7 +686,7 @@ static irqreturn_t meson_mmc_irq_thread(int irq, void *dev_id)
>  	}
>  
>  	meson_mmc_read_resp(host->mmc, cmd);
> -	if (!data || !data->stop || mrq->sbc)
> +	if (!data || !data->stop)
>  		meson_mmc_request_done(host->mmc, mrq);
>  	else
>  		meson_mmc_start_cmd(host->mmc, data->stop);

Re: [PATCH 04/18] mmc: meson-gx: improve meson_mmc_start_cmd

[Kevin Hilman]

Heiner Kallweit <hkallweit1@gmail.com> writes:

> Remove use of unneeded members cmd_arg and cmd_resp.
> Setting SD_EMMC_CMD_RSP is only needed if CMD_CFG_RESP_NUM is set,
> so don't write this register in all other cases.
>
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>

I'm not sure I like this change.  This works now because there is only
one descriptor used, but one of the next things to work on in this
driver is taking advantage of the internal DMA capabilities, which means
having a chain of descriptorsall filled out in memory.

Kevin

> ---
>  drivers/mmc/host/meson-gx-mmc.c | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/mmc/host/meson-gx-mmc.c b/drivers/mmc/host/meson-gx-mmc.c
> index ece38b44..630e0590 100644
> --- a/drivers/mmc/host/meson-gx-mmc.c
> +++ b/drivers/mmc/host/meson-gx-mmc.c
> @@ -456,7 +456,6 @@ static void meson_mmc_start_cmd(struct mmc_host *mmc, struct mmc_command *cmd)
>  	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;
>  
>  	/* Response */
>  	if (cmd->flags & MMC_RSP_PRESENT) {
> @@ -464,7 +463,7 @@ static void meson_mmc_start_cmd(struct mmc_host *mmc, struct mmc_command *cmd)
>  		if (cmd->flags & MMC_RSP_136)
>  			desc->cmd_cfg |= CMD_CFG_RESP_128;
>  		desc->cmd_cfg |= CMD_CFG_RESP_NUM;
> -		desc->cmd_resp = 0;
> +		writel(0, host->regs + SD_EMMC_CMD_RSP);
>  
>  		if (!(cmd->flags & MMC_RSP_CRC))
>  			desc->cmd_cfg |= CMD_CFG_RESP_NOCRC;
> @@ -540,9 +539,8 @@ static void meson_mmc_start_cmd(struct mmc_host *mmc, struct mmc_command *cmd)
>  	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);
>  	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)

[PATCH 04/18] mmc: meson-gx: improve meson_mmc_start_cmd

[Kevin Hilman]

Heiner Kallweit <hkallweit1@gmail.com> writes:

> Remove use of unneeded members cmd_arg and cmd_resp.
> Setting SD_EMMC_CMD_RSP is only needed if CMD_CFG_RESP_NUM is set,
> so don't write this register in all other cases.
>
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>

I'm not sure I like this change.  This works now because there is only
one descriptor used, but one of the next things to work on in this
driver is taking advantage of the internal DMA capabilities, which means
having a chain of descriptorsall filled out in memory.

Kevin

> ---
>  drivers/mmc/host/meson-gx-mmc.c | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/mmc/host/meson-gx-mmc.c b/drivers/mmc/host/meson-gx-mmc.c
> index ece38b44..630e0590 100644
> --- a/drivers/mmc/host/meson-gx-mmc.c
> +++ b/drivers/mmc/host/meson-gx-mmc.c
> @@ -456,7 +456,6 @@ static void meson_mmc_start_cmd(struct mmc_host *mmc, struct mmc_command *cmd)
>  	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;
>  
>  	/* Response */
>  	if (cmd->flags & MMC_RSP_PRESENT) {
> @@ -464,7 +463,7 @@ static void meson_mmc_start_cmd(struct mmc_host *mmc, struct mmc_command *cmd)
>  		if (cmd->flags & MMC_RSP_136)
>  			desc->cmd_cfg |= CMD_CFG_RESP_128;
>  		desc->cmd_cfg |= CMD_CFG_RESP_NUM;
> -		desc->cmd_resp = 0;
> +		writel(0, host->regs + SD_EMMC_CMD_RSP);
>  
>  		if (!(cmd->flags & MMC_RSP_CRC))
>  			desc->cmd_cfg |= CMD_CFG_RESP_NOCRC;
> @@ -540,9 +539,8 @@ static void meson_mmc_start_cmd(struct mmc_host *mmc, struct mmc_command *cmd)
>  	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);
>  	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)

Re: [PATCH 05/18] mmc: meson-gx: eliminate struct sd_emmc_data

[Kevin Hilman]

Heiner Kallweit <hkallweit1@gmail.com> writes:

> Struct sd_emmc_data is used in meson_mmc_start_cmd only, so we can get
> rid of this struct and replace the two used members with local variables.
>
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>

For the same reasons as the previous patch, I don't like this change as
I think it will be much cleaner code if we keep this descriptor struct
and use it to fill out multiple descriptors in a chain.

Kevin

> ---
>  drivers/mmc/host/meson-gx-mmc.c | 69 +++++++++++++++++------------------------
>  1 file changed, 29 insertions(+), 40 deletions(-)
>
> diff --git a/drivers/mmc/host/meson-gx-mmc.c b/drivers/mmc/host/meson-gx-mmc.c
> index 630e0590..c7bf0e12 100644
> --- a/drivers/mmc/host/meson-gx-mmc.c
> +++ b/drivers/mmc/host/meson-gx-mmc.c
> @@ -146,12 +146,6 @@ struct meson_host {
>  	bool vqmmc_enabled;
>  };
>  
> -struct sd_emmc_desc {
> -	u32 cmd_cfg;
> -	u32 cmd_arg;
> -	u32 cmd_data;
> -	u32 cmd_resp;
> -};
>  #define CMD_CFG_LENGTH_SHIFT 0
>  #define CMD_CFG_LENGTH_MASK 0x1ff
>  #define CMD_CFG_BLOCK_MODE BIT(9)
> @@ -443,45 +437,41 @@ static int meson_mmc_request_done(struct mmc_host *mmc, struct mmc_request *mrq)
>  static void meson_mmc_start_cmd(struct mmc_host *mmc, struct mmc_command *cmd)
>  {
>  	struct meson_host *host = mmc_priv(mmc);
> -	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 */
> +	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) {
> -		desc->cmd_cfg &= ~CMD_CFG_NO_RESP;
> +		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;
> +			cmd_cfg |= CMD_CFG_RESP_128;
> +		cmd_cfg |= CMD_CFG_RESP_NUM;
>  		writel(0, host->regs + SD_EMMC_CMD_RSP);
>  
>  		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 (cmd->data) {
> -		desc->cmd_cfg |= CMD_CFG_DATA_IO;
> +		cmd_cfg |= CMD_CFG_DATA_IO;
>  		if (cmd->data->blocks > 1) {
> -			desc->cmd_cfg |= CMD_CFG_BLOCK_MODE;
> -			desc->cmd_cfg |=
> -				(cmd->data->blocks & CMD_CFG_LENGTH_MASK) <<
> -				CMD_CFG_LENGTH_SHIFT;
> +			cmd_cfg |= CMD_CFG_BLOCK_MODE;
> +			cmd_cfg |= (cmd->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);
> @@ -497,48 +487,47 @@ 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;
> +			cmd_cfg &= ~CMD_CFG_BLOCK_MODE;
> +			cmd_cfg |= (cmd->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) {
> -			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(cmd->data->sg, cmd->data->sg_len,
>  					  host->bounce_buf, xfer_bytes);
>  			cmd->data->bytes_xfered = xfer_bytes;
>  			dma_wmb();
>  		} else {
> -			desc->cmd_cfg &= ~CMD_CFG_DATA_WR;
> +			cmd_cfg &= ~CMD_CFG_DATA_WR;
>  		}
>  
>  		if (xfer_bytes > 0) {
> -			desc->cmd_cfg &= ~CMD_CFG_DATA_NUM;
> -			desc->cmd_data = host->bounce_dma_addr & CMD_DATA_MASK;
> +			cmd_cfg &= ~CMD_CFG_DATA_NUM;
> +			cmd_data = host->bounce_dma_addr & CMD_DATA_MASK;
>  		} else {
>  			/* write data to data_addr */
> -			desc->cmd_cfg |= CMD_CFG_DATA_NUM;
> -			desc->cmd_data = 0;
> +			cmd_cfg |= CMD_CFG_DATA_NUM;
> +			cmd_data = 0;
>  		}
>  
>  		cmd_cfg_timeout = 12;
>  	} else {
> -		desc->cmd_cfg &= ~CMD_CFG_DATA_IO;
> +		cmd_cfg &= ~CMD_CFG_DATA_IO;
>  		cmd_cfg_timeout = 10;
>  	}
> -	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);
> +	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);
>  	wmb(); /* ensure descriptor is written before kicked */
>  	writel(cmd->arg, host->regs + SD_EMMC_CMD_ARG);
>  }

[PATCH 05/18] mmc: meson-gx: eliminate struct sd_emmc_data

[Kevin Hilman]

Heiner Kallweit <hkallweit1@gmail.com> writes:

> Struct sd_emmc_data is used in meson_mmc_start_cmd only, so we can get
> rid of this struct and replace the two used members with local variables.
>
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>

For the same reasons as the previous patch, I don't like this change as
I think it will be much cleaner code if we keep this descriptor struct
and use it to fill out multiple descriptors in a chain.

Kevin

> ---
>  drivers/mmc/host/meson-gx-mmc.c | 69 +++++++++++++++++------------------------
>  1 file changed, 29 insertions(+), 40 deletions(-)
>
> diff --git a/drivers/mmc/host/meson-gx-mmc.c b/drivers/mmc/host/meson-gx-mmc.c
> index 630e0590..c7bf0e12 100644
> --- a/drivers/mmc/host/meson-gx-mmc.c
> +++ b/drivers/mmc/host/meson-gx-mmc.c
> @@ -146,12 +146,6 @@ struct meson_host {
>  	bool vqmmc_enabled;
>  };
>  
> -struct sd_emmc_desc {
> -	u32 cmd_cfg;
> -	u32 cmd_arg;
> -	u32 cmd_data;
> -	u32 cmd_resp;
> -};
>  #define CMD_CFG_LENGTH_SHIFT 0
>  #define CMD_CFG_LENGTH_MASK 0x1ff
>  #define CMD_CFG_BLOCK_MODE BIT(9)
> @@ -443,45 +437,41 @@ static int meson_mmc_request_done(struct mmc_host *mmc, struct mmc_request *mrq)
>  static void meson_mmc_start_cmd(struct mmc_host *mmc, struct mmc_command *cmd)
>  {
>  	struct meson_host *host = mmc_priv(mmc);
> -	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 */
> +	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) {
> -		desc->cmd_cfg &= ~CMD_CFG_NO_RESP;
> +		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;
> +			cmd_cfg |= CMD_CFG_RESP_128;
> +		cmd_cfg |= CMD_CFG_RESP_NUM;
>  		writel(0, host->regs + SD_EMMC_CMD_RSP);
>  
>  		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 (cmd->data) {
> -		desc->cmd_cfg |= CMD_CFG_DATA_IO;
> +		cmd_cfg |= CMD_CFG_DATA_IO;
>  		if (cmd->data->blocks > 1) {
> -			desc->cmd_cfg |= CMD_CFG_BLOCK_MODE;
> -			desc->cmd_cfg |=
> -				(cmd->data->blocks & CMD_CFG_LENGTH_MASK) <<
> -				CMD_CFG_LENGTH_SHIFT;
> +			cmd_cfg |= CMD_CFG_BLOCK_MODE;
> +			cmd_cfg |= (cmd->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);
> @@ -497,48 +487,47 @@ 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;
> +			cmd_cfg &= ~CMD_CFG_BLOCK_MODE;
> +			cmd_cfg |= (cmd->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) {
> -			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(cmd->data->sg, cmd->data->sg_len,
>  					  host->bounce_buf, xfer_bytes);
>  			cmd->data->bytes_xfered = xfer_bytes;
>  			dma_wmb();
>  		} else {
> -			desc->cmd_cfg &= ~CMD_CFG_DATA_WR;
> +			cmd_cfg &= ~CMD_CFG_DATA_WR;
>  		}
>  
>  		if (xfer_bytes > 0) {
> -			desc->cmd_cfg &= ~CMD_CFG_DATA_NUM;
> -			desc->cmd_data = host->bounce_dma_addr & CMD_DATA_MASK;
> +			cmd_cfg &= ~CMD_CFG_DATA_NUM;
> +			cmd_data = host->bounce_dma_addr & CMD_DATA_MASK;
>  		} else {
>  			/* write data to data_addr */
> -			desc->cmd_cfg |= CMD_CFG_DATA_NUM;
> -			desc->cmd_data = 0;
> +			cmd_cfg |= CMD_CFG_DATA_NUM;
> +			cmd_data = 0;
>  		}
>  
>  		cmd_cfg_timeout = 12;
>  	} else {
> -		desc->cmd_cfg &= ~CMD_CFG_DATA_IO;
> +		cmd_cfg &= ~CMD_CFG_DATA_IO;
>  		cmd_cfg_timeout = 10;
>  	}
> -	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);
> +	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);
>  	wmb(); /* ensure descriptor is written before kicked */
>  	writel(cmd->arg, host->regs + SD_EMMC_CMD_ARG);
>  }

Re: [PATCH 06/18] mmc: meson-gx: simplify bounce buffer setting in meson_mmc_start_cmd

[Kevin Hilman]

Heiner Kallweit <hkallweit1@gmail.com> writes:

> If xfer_bytes is zero we can configure the chip to use the bounce
> buffer as well because it won't get used anyway.
> And we don't have to reset bit CMD_CFG_DATA_NUM because cmd_cfg
> was zero-initialized and this bit isn't set.
>
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
> ---
>  drivers/mmc/host/meson-gx-mmc.c | 10 +---------
>  1 file changed, 1 insertion(+), 9 deletions(-)
>
> diff --git a/drivers/mmc/host/meson-gx-mmc.c b/drivers/mmc/host/meson-gx-mmc.c
> index c7bf0e12..423b4b17 100644
> --- a/drivers/mmc/host/meson-gx-mmc.c
> +++ b/drivers/mmc/host/meson-gx-mmc.c
> @@ -505,15 +505,7 @@ static void meson_mmc_start_cmd(struct mmc_host *mmc, struct mmc_command *cmd)
>  			cmd_cfg &= ~CMD_CFG_DATA_WR;
>  		}
>  
> -		if (xfer_bytes > 0) {
> -			cmd_cfg &= ~CMD_CFG_DATA_NUM;
> -			cmd_data = host->bounce_dma_addr & CMD_DATA_MASK;
> -		} else {
> -			/* write data to data_addr */
> -			cmd_cfg |= CMD_CFG_DATA_NUM;
> -			cmd_data = 0;
> -		}
> -
> +		cmd_data = host->bounce_dma_addr & CMD_DATA_MASK;

Could use a comment above this, mentioning (similar to the changelog)
that when CMD_CFG_DATA_NUM is set, this isn't used.

Otherwise:

Acked-by: Kevin Hilman <khilman@baylibre.com>

[PATCH 06/18] mmc: meson-gx: simplify bounce buffer setting in meson_mmc_start_cmd

[Kevin Hilman]

Heiner Kallweit <hkallweit1@gmail.com> writes:

> If xfer_bytes is zero we can configure the chip to use the bounce
> buffer as well because it won't get used anyway.
> And we don't have to reset bit CMD_CFG_DATA_NUM because cmd_cfg
> was zero-initialized and this bit isn't set.
>
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
> ---
>  drivers/mmc/host/meson-gx-mmc.c | 10 +---------
>  1 file changed, 1 insertion(+), 9 deletions(-)
>
> diff --git a/drivers/mmc/host/meson-gx-mmc.c b/drivers/mmc/host/meson-gx-mmc.c
> index c7bf0e12..423b4b17 100644
> --- a/drivers/mmc/host/meson-gx-mmc.c
> +++ b/drivers/mmc/host/meson-gx-mmc.c
> @@ -505,15 +505,7 @@ static void meson_mmc_start_cmd(struct mmc_host *mmc, struct mmc_command *cmd)
>  			cmd_cfg &= ~CMD_CFG_DATA_WR;
>  		}
>  
> -		if (xfer_bytes > 0) {
> -			cmd_cfg &= ~CMD_CFG_DATA_NUM;
> -			cmd_data = host->bounce_dma_addr & CMD_DATA_MASK;
> -		} else {
> -			/* write data to data_addr */
> -			cmd_cfg |= CMD_CFG_DATA_NUM;
> -			cmd_data = 0;
> -		}
> -
> +		cmd_data = host->bounce_dma_addr & CMD_DATA_MASK;

Could use a comment above this, mentioning (similar to the changelog)
that when CMD_CFG_DATA_NUM is set, this isn't used.

Otherwise:

Acked-by: Kevin Hilman <khilman@baylibre.com>

Re: [PATCH 07/18] mmc: meson-gx: check return value of sg_copy_[from,to]_buffer

[Kevin Hilman]

Heiner Kallweit <hkallweit1@gmail.com> writes:

> So far it's only checked whether the requested transfer length exceeds
> the bounce buffer size. Instead let's check that sg_copy_[from,to]_buffer
> transferred exactly the number of requested bytes.
>
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>

Acked-by: Kevin Hilman <khilman@baylibre.com>

> ---
>  drivers/mmc/host/meson-gx-mmc.c | 19 ++++++++++---------
>  1 file changed, 10 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/mmc/host/meson-gx-mmc.c b/drivers/mmc/host/meson-gx-mmc.c
> index 423b4b17..f3963538 100644
> --- a/drivers/mmc/host/meson-gx-mmc.c
> +++ b/drivers/mmc/host/meson-gx-mmc.c
> @@ -495,14 +495,14 @@ static void meson_mmc_start_cmd(struct mmc_host *mmc, struct mmc_command *cmd)
>  		cmd->data->bytes_xfered = 0;
>  		xfer_bytes = cmd->data->blksz * cmd->data->blocks;
>  		if (cmd->data->flags & MMC_DATA_WRITE) {
> -			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,
> -					  host->bounce_buf, xfer_bytes);
> +			size_t len = sg_copy_to_buffer(cmd->data->sg,
> +						       cmd->data->sg_len,
> +						       host->bounce_buf,
> +						       host->bounce_buf_size);
> +			WARN_ON(len != xfer_bytes);
>  			cmd->data->bytes_xfered = xfer_bytes;
> +			cmd_cfg |= CMD_CFG_DATA_WR;
>  			dma_wmb();
> -		} else {
> -			cmd_cfg &= ~CMD_CFG_DATA_WR;
>  		}
>  
>  		cmd_data = host->bounce_dma_addr & CMD_DATA_MASK;
> @@ -657,10 +657,11 @@ static irqreturn_t meson_mmc_irq_thread(int irq, void *dev_id)
>  
>  	data = cmd->data;
>  	if (data && data->flags & MMC_DATA_READ) {
> +		size_t len = sg_copy_from_buffer(data->sg, data->sg_len,
> +						 host->bounce_buf,
> +						 host->bounce_buf_size);
>  		xfer_bytes = data->blksz * data->blocks;
> -		WARN_ON(xfer_bytes > host->bounce_buf_size);
> -		sg_copy_from_buffer(data->sg, data->sg_len,
> -				    host->bounce_buf, xfer_bytes);
> +		WARN_ON(len != xfer_bytes);
>  		data->bytes_xfered = xfer_bytes;
>  	}

[PATCH 07/18] mmc: meson-gx: check return value of sg_copy_[from, to]_buffer

[Kevin Hilman]

Heiner Kallweit <hkallweit1@gmail.com> writes:

> So far it's only checked whether the requested transfer length exceeds
> the bounce buffer size. Instead let's check that sg_copy_[from,to]_buffer
> transferred exactly the number of requested bytes.
>
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>

Acked-by: Kevin Hilman <khilman@baylibre.com>

> ---
>  drivers/mmc/host/meson-gx-mmc.c | 19 ++++++++++---------
>  1 file changed, 10 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/mmc/host/meson-gx-mmc.c b/drivers/mmc/host/meson-gx-mmc.c
> index 423b4b17..f3963538 100644
> --- a/drivers/mmc/host/meson-gx-mmc.c
> +++ b/drivers/mmc/host/meson-gx-mmc.c
> @@ -495,14 +495,14 @@ static void meson_mmc_start_cmd(struct mmc_host *mmc, struct mmc_command *cmd)
>  		cmd->data->bytes_xfered = 0;
>  		xfer_bytes = cmd->data->blksz * cmd->data->blocks;
>  		if (cmd->data->flags & MMC_DATA_WRITE) {
> -			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,
> -					  host->bounce_buf, xfer_bytes);
> +			size_t len = sg_copy_to_buffer(cmd->data->sg,
> +						       cmd->data->sg_len,
> +						       host->bounce_buf,
> +						       host->bounce_buf_size);
> +			WARN_ON(len != xfer_bytes);
>  			cmd->data->bytes_xfered = xfer_bytes;
> +			cmd_cfg |= CMD_CFG_DATA_WR;
>  			dma_wmb();
> -		} else {
> -			cmd_cfg &= ~CMD_CFG_DATA_WR;
>  		}
>  
>  		cmd_data = host->bounce_dma_addr & CMD_DATA_MASK;
> @@ -657,10 +657,11 @@ static irqreturn_t meson_mmc_irq_thread(int irq, void *dev_id)
>  
>  	data = cmd->data;
>  	if (data && data->flags & MMC_DATA_READ) {
> +		size_t len = sg_copy_from_buffer(data->sg, data->sg_len,
> +						 host->bounce_buf,
> +						 host->bounce_buf_size);
>  		xfer_bytes = data->blksz * data->blocks;
> -		WARN_ON(xfer_bytes > host->bounce_buf_size);
> -		sg_copy_from_buffer(data->sg, data->sg_len,
> -				    host->bounce_buf, xfer_bytes);
> +		WARN_ON(len != xfer_bytes);
>  		data->bytes_xfered = xfer_bytes;
>  	}

Re: [PATCH 08/18] mmc: meson-gx: make two functions return void

[Kevin Hilman]

Heiner Kallweit <hkallweit1@gmail.com> writes:

> The return value of meson_mmc_request_done and meson_mmc_read_resp
> isn't used, so make both functions return void.
>
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>

Acked-by: Kevin Hilman <khilman@baylibre.com>

> ---
>  drivers/mmc/host/meson-gx-mmc.c | 9 +++------
>  1 file changed, 3 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/mmc/host/meson-gx-mmc.c b/drivers/mmc/host/meson-gx-mmc.c
> index f3963538..a67c811e 100644
> --- a/drivers/mmc/host/meson-gx-mmc.c
> +++ b/drivers/mmc/host/meson-gx-mmc.c
> @@ -421,7 +421,8 @@ static void meson_mmc_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
>  			__func__, orig, val);
>  }
>  
> -static int meson_mmc_request_done(struct mmc_host *mmc, struct mmc_request *mrq)
> +static void meson_mmc_request_done(struct mmc_host *mmc,
> +				   struct mmc_request *mrq)
>  {
>  	struct meson_host *host = mmc_priv(mmc);
>  
> @@ -430,8 +431,6 @@ static int meson_mmc_request_done(struct mmc_host *mmc, struct mmc_request *mrq)
>  	host->mrq = NULL;
>  	host->cmd = NULL;
>  	mmc_request_done(host->mmc, mrq);
> -
> -	return 0;
>  }
>  
>  static void meson_mmc_start_cmd(struct mmc_host *mmc, struct mmc_command *cmd)
> @@ -537,7 +536,7 @@ static void meson_mmc_request(struct mmc_host *mmc, struct mmc_request *mrq)
>  	meson_mmc_start_cmd(mmc, mrq->cmd);
>  }
>  
> -static int meson_mmc_read_resp(struct mmc_host *mmc, struct mmc_command *cmd)
> +static void meson_mmc_read_resp(struct mmc_host *mmc, struct mmc_command *cmd)
>  {
>  	struct meson_host *host = mmc_priv(mmc);
>  
> @@ -549,8 +548,6 @@ static int meson_mmc_read_resp(struct mmc_host *mmc, struct mmc_command *cmd)
>  	} else if (cmd->flags & MMC_RSP_PRESENT) {
>  		cmd->resp[0] = readl(host->regs + SD_EMMC_CMD_RSP);
>  	}
> -
> -	return 0;
>  }
>  
>  static irqreturn_t meson_mmc_irq(int irq, void *dev_id)

[PATCH 08/18] mmc: meson-gx: make two functions return void

[Kevin Hilman]

Heiner Kallweit <hkallweit1@gmail.com> writes:

> The return value of meson_mmc_request_done and meson_mmc_read_resp
> isn't used, so make both functions return void.
>
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>

Acked-by: Kevin Hilman <khilman@baylibre.com>

> ---
>  drivers/mmc/host/meson-gx-mmc.c | 9 +++------
>  1 file changed, 3 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/mmc/host/meson-gx-mmc.c b/drivers/mmc/host/meson-gx-mmc.c
> index f3963538..a67c811e 100644
> --- a/drivers/mmc/host/meson-gx-mmc.c
> +++ b/drivers/mmc/host/meson-gx-mmc.c
> @@ -421,7 +421,8 @@ static void meson_mmc_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
>  			__func__, orig, val);
>  }
>  
> -static int meson_mmc_request_done(struct mmc_host *mmc, struct mmc_request *mrq)
> +static void meson_mmc_request_done(struct mmc_host *mmc,
> +				   struct mmc_request *mrq)
>  {
>  	struct meson_host *host = mmc_priv(mmc);
>  
> @@ -430,8 +431,6 @@ static int meson_mmc_request_done(struct mmc_host *mmc, struct mmc_request *mrq)
>  	host->mrq = NULL;
>  	host->cmd = NULL;
>  	mmc_request_done(host->mmc, mrq);
> -
> -	return 0;
>  }
>  
>  static void meson_mmc_start_cmd(struct mmc_host *mmc, struct mmc_command *cmd)
> @@ -537,7 +536,7 @@ static void meson_mmc_request(struct mmc_host *mmc, struct mmc_request *mrq)
>  	meson_mmc_start_cmd(mmc, mrq->cmd);
>  }
>  
> -static int meson_mmc_read_resp(struct mmc_host *mmc, struct mmc_command *cmd)
> +static void meson_mmc_read_resp(struct mmc_host *mmc, struct mmc_command *cmd)
>  {
>  	struct meson_host *host = mmc_priv(mmc);
>  
> @@ -549,8 +548,6 @@ static int meson_mmc_read_resp(struct mmc_host *mmc, struct mmc_command *cmd)
>  	} else if (cmd->flags & MMC_RSP_PRESENT) {
>  		cmd->resp[0] = readl(host->regs + SD_EMMC_CMD_RSP);
>  	}
> -
> -	return 0;
>  }
>  
>  static irqreturn_t meson_mmc_irq(int irq, void *dev_id)

Re: [PATCH 09/18] mmc: meson-gx: change interrupt name

[Kevin Hilman]

Heiner Kallweit <hkallweit1@gmail.com> writes:

> So far the driver name is used for the interrupt. Therefore in
> /proc/interrupts we can't tell which interrupt belongs to which
> SD_EMMC port.
> So let's use the device name instead (what is the OF node name).
>
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>

Acked-by: Kevin Hilman <khilman@baylibre.com>

> ---
>  drivers/mmc/host/meson-gx-mmc.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/mmc/host/meson-gx-mmc.c b/drivers/mmc/host/meson-gx-mmc.c
> index a67c811e..1a76c925 100644
> --- a/drivers/mmc/host/meson-gx-mmc.c
> +++ b/drivers/mmc/host/meson-gx-mmc.c
> @@ -757,9 +757,9 @@ static int meson_mmc_probe(struct platform_device *pdev)
>  	writel(IRQ_EN_MASK, host->regs + SD_EMMC_STATUS);
>  	writel(IRQ_EN_MASK, host->regs + SD_EMMC_IRQ_EN);
>  
> -	ret = devm_request_threaded_irq(&pdev->dev, host->irq,
> -					meson_mmc_irq, meson_mmc_irq_thread,
> -					IRQF_SHARED, DRIVER_NAME, host);
> +	ret = devm_request_threaded_irq(&pdev->dev, host->irq, meson_mmc_irq,
> +					meson_mmc_irq_thread, IRQF_SHARED,
> +					dev_name(&pdev->dev), host);
>  	if (ret)
>  		goto free_host;

[PATCH 09/18] mmc: meson-gx: change interrupt name

[Kevin Hilman]

Heiner Kallweit <hkallweit1@gmail.com> writes:

> So far the driver name is used for the interrupt. Therefore in
> /proc/interrupts we can't tell which interrupt belongs to which
> SD_EMMC port.
> So let's use the device name instead (what is the OF node name).
>
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>

Acked-by: Kevin Hilman <khilman@baylibre.com>

> ---
>  drivers/mmc/host/meson-gx-mmc.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/mmc/host/meson-gx-mmc.c b/drivers/mmc/host/meson-gx-mmc.c
> index a67c811e..1a76c925 100644
> --- a/drivers/mmc/host/meson-gx-mmc.c
> +++ b/drivers/mmc/host/meson-gx-mmc.c
> @@ -757,9 +757,9 @@ static int meson_mmc_probe(struct platform_device *pdev)
>  	writel(IRQ_EN_MASK, host->regs + SD_EMMC_STATUS);
>  	writel(IRQ_EN_MASK, host->regs + SD_EMMC_IRQ_EN);
>  
> -	ret = devm_request_threaded_irq(&pdev->dev, host->irq,
> -					meson_mmc_irq, meson_mmc_irq_thread,
> -					IRQF_SHARED, DRIVER_NAME, host);
> +	ret = devm_request_threaded_irq(&pdev->dev, host->irq, meson_mmc_irq,
> +					meson_mmc_irq_thread, IRQF_SHARED,
> +					dev_name(&pdev->dev), host);
>  	if (ret)
>  		goto free_host;

Re: [PATCH 10/18] mmc: meson-gx: remove unused members irq, ocr_mask from struct meson_host

[Kevin Hilman]

Heiner Kallweit <hkallweit1@gmail.com> writes:

> Member ocr_mask is never used and member irq we can replace with a
> local variable in meson_mmc_probe. So let's remove both members.
>
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>

Acked-by: Kevin Hilman <khilman@baylibre.com>

> ---
>  drivers/mmc/host/meson-gx-mmc.c | 10 ++++------
>  1 file changed, 4 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/mmc/host/meson-gx-mmc.c b/drivers/mmc/host/meson-gx-mmc.c
> index 1a76c925..159ee7b4 100644
> --- a/drivers/mmc/host/meson-gx-mmc.c
> +++ b/drivers/mmc/host/meson-gx-mmc.c
> @@ -128,8 +128,6 @@ struct meson_host {
>  
>  	spinlock_t lock;
>  	void __iomem *regs;
> -	int irq;
> -	u32 ocr_mask;
>  	struct clk *core_clk;
>  	struct clk_mux mux;
>  	struct clk *mux_clk;
> @@ -696,7 +694,7 @@ static int meson_mmc_probe(struct platform_device *pdev)
>  	struct resource *res;
>  	struct meson_host *host;
>  	struct mmc_host *mmc;
> -	int ret;
> +	int ret, irq;
>  
>  	mmc = mmc_alloc_host(sizeof(struct meson_host), &pdev->dev);
>  	if (!mmc)
> @@ -728,8 +726,8 @@ static int meson_mmc_probe(struct platform_device *pdev)
>  		goto free_host;
>  	}
>  
> -	host->irq = platform_get_irq(pdev, 0);
> -	if (host->irq == 0) {
> +	irq = platform_get_irq(pdev, 0);
> +	if (!irq) {
>  		dev_err(&pdev->dev, "failed to get interrupt resource.\n");
>  		ret = -EINVAL;
>  		goto free_host;
> @@ -757,7 +755,7 @@ static int meson_mmc_probe(struct platform_device *pdev)
>  	writel(IRQ_EN_MASK, host->regs + SD_EMMC_STATUS);
>  	writel(IRQ_EN_MASK, host->regs + SD_EMMC_IRQ_EN);
>  
> -	ret = devm_request_threaded_irq(&pdev->dev, host->irq, meson_mmc_irq,
> +	ret = devm_request_threaded_irq(&pdev->dev, irq, meson_mmc_irq,
>  					meson_mmc_irq_thread, IRQF_SHARED,
>  					dev_name(&pdev->dev), host);
>  	if (ret)

[PATCH 10/18] mmc: meson-gx: remove unused members irq, ocr_mask from struct meson_host

[Kevin Hilman]

Heiner Kallweit <hkallweit1@gmail.com> writes:

> Member ocr_mask is never used and member irq we can replace with a
> local variable in meson_mmc_probe. So let's remove both members.
>
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>

Acked-by: Kevin Hilman <khilman@baylibre.com>

> ---
>  drivers/mmc/host/meson-gx-mmc.c | 10 ++++------
>  1 file changed, 4 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/mmc/host/meson-gx-mmc.c b/drivers/mmc/host/meson-gx-mmc.c
> index 1a76c925..159ee7b4 100644
> --- a/drivers/mmc/host/meson-gx-mmc.c
> +++ b/drivers/mmc/host/meson-gx-mmc.c
> @@ -128,8 +128,6 @@ struct meson_host {
>  
>  	spinlock_t lock;
>  	void __iomem *regs;
> -	int irq;
> -	u32 ocr_mask;
>  	struct clk *core_clk;
>  	struct clk_mux mux;
>  	struct clk *mux_clk;
> @@ -696,7 +694,7 @@ static int meson_mmc_probe(struct platform_device *pdev)
>  	struct resource *res;
>  	struct meson_host *host;
>  	struct mmc_host *mmc;
> -	int ret;
> +	int ret, irq;
>  
>  	mmc = mmc_alloc_host(sizeof(struct meson_host), &pdev->dev);
>  	if (!mmc)
> @@ -728,8 +726,8 @@ static int meson_mmc_probe(struct platform_device *pdev)
>  		goto free_host;
>  	}
>  
> -	host->irq = platform_get_irq(pdev, 0);
> -	if (host->irq == 0) {
> +	irq = platform_get_irq(pdev, 0);
> +	if (!irq) {
>  		dev_err(&pdev->dev, "failed to get interrupt resource.\n");
>  		ret = -EINVAL;
>  		goto free_host;
> @@ -757,7 +755,7 @@ static int meson_mmc_probe(struct platform_device *pdev)
>  	writel(IRQ_EN_MASK, host->regs + SD_EMMC_STATUS);
>  	writel(IRQ_EN_MASK, host->regs + SD_EMMC_IRQ_EN);
>  
> -	ret = devm_request_threaded_irq(&pdev->dev, host->irq, meson_mmc_irq,
> +	ret = devm_request_threaded_irq(&pdev->dev, irq, meson_mmc_irq,
>  					meson_mmc_irq_thread, IRQF_SHARED,
>  					dev_name(&pdev->dev), host);
>  	if (ret)

Re: [PATCH 11/18] mmc: meson-gx: remove unneeded variable in meson_mmc_clk_init

[Kevin Hilman]

Heiner Kallweit <hkallweit1@gmail.com> writes:

> Variable mux_parent_count can be replaced with constant
> MUX_CLK_NUM_PARENTS, so remove it.

Before the above sentence, add "Because the DT requires a fixed number
of mux parent clocks, ..."

> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>

With that,

Acked-by: Kevin Hilman <khilman@baylibre.com>

> ---
>  drivers/mmc/host/meson-gx-mmc.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/drivers/mmc/host/meson-gx-mmc.c b/drivers/mmc/host/meson-gx-mmc.c
> index 159ee7b4..d05ffe8a 100644
> --- a/drivers/mmc/host/meson-gx-mmc.c
> +++ b/drivers/mmc/host/meson-gx-mmc.c
> @@ -237,7 +237,6 @@ static int meson_mmc_clk_init(struct meson_host *host)
>  	char clk_name[32];
>  	int i, ret = 0;
>  	const char *mux_parent_names[MUX_CLK_NUM_PARENTS];
> -	unsigned int mux_parent_count = 0;
>  	const char *clk_div_parents[1];
>  	u32 clk_reg, cfg;
>  
> @@ -256,7 +255,6 @@ static int meson_mmc_clk_init(struct meson_host *host)
>  		}
>  
>  		mux_parent_names[i] = __clk_get_name(host->mux_parent[i]);
> -		mux_parent_count++;
>  	}
>  
>  	/* create the mux */
> @@ -265,7 +263,7 @@ static int meson_mmc_clk_init(struct meson_host *host)
>  	init.ops = &clk_mux_ops;
>  	init.flags = 0;
>  	init.parent_names = mux_parent_names;
> -	init.num_parents = mux_parent_count;
> +	init.num_parents = MUX_CLK_NUM_PARENTS;
>  
>  	host->mux.reg = host->regs + SD_EMMC_CLOCK;
>  	host->mux.shift = CLK_SRC_SHIFT;

[PATCH 11/18] mmc: meson-gx: remove unneeded variable in meson_mmc_clk_init

[Kevin Hilman]

Heiner Kallweit <hkallweit1@gmail.com> writes:

> Variable mux_parent_count can be replaced with constant
> MUX_CLK_NUM_PARENTS, so remove it.

Before the above sentence, add "Because the DT requires a fixed number
of mux parent clocks, ..."

> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>

With that,

Acked-by: Kevin Hilman <khilman@baylibre.com>

> ---
>  drivers/mmc/host/meson-gx-mmc.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/drivers/mmc/host/meson-gx-mmc.c b/drivers/mmc/host/meson-gx-mmc.c
> index 159ee7b4..d05ffe8a 100644
> --- a/drivers/mmc/host/meson-gx-mmc.c
> +++ b/drivers/mmc/host/meson-gx-mmc.c
> @@ -237,7 +237,6 @@ static int meson_mmc_clk_init(struct meson_host *host)
>  	char clk_name[32];
>  	int i, ret = 0;
>  	const char *mux_parent_names[MUX_CLK_NUM_PARENTS];
> -	unsigned int mux_parent_count = 0;
>  	const char *clk_div_parents[1];
>  	u32 clk_reg, cfg;
>  
> @@ -256,7 +255,6 @@ static int meson_mmc_clk_init(struct meson_host *host)
>  		}
>  
>  		mux_parent_names[i] = __clk_get_name(host->mux_parent[i]);
> -		mux_parent_count++;
>  	}
>  
>  	/* create the mux */
> @@ -265,7 +263,7 @@ static int meson_mmc_clk_init(struct meson_host *host)
>  	init.ops = &clk_mux_ops;
>  	init.flags = 0;
>  	init.parent_names = mux_parent_names;
> -	init.num_parents = mux_parent_count;
> +	init.num_parents = MUX_CLK_NUM_PARENTS;
>  
>  	host->mux.reg = host->regs + SD_EMMC_CLOCK;
>  	host->mux.shift = CLK_SRC_SHIFT;

Re: [PATCH 12/18] mmc: meson-gx: remove member parent_mux from struct meson_host

[Kevin Hilman]

Heiner Kallweit <hkallweit1@gmail.com> writes:

> Member mux_parent isn't used outside meson_mmc_clk_init. So remove it
> and replace it with a local variable in meson_mmc_clk_init.
>
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>

Acked-by: Kevin Hilman <khilman@baylibre.com>

> ---
>  drivers/mmc/host/meson-gx-mmc.c | 14 ++++++--------
>  1 file changed, 6 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/mmc/host/meson-gx-mmc.c b/drivers/mmc/host/meson-gx-mmc.c
> index d05ffe8a..74ceda75 100644
> --- a/drivers/mmc/host/meson-gx-mmc.c
> +++ b/drivers/mmc/host/meson-gx-mmc.c
> @@ -131,7 +131,6 @@ struct meson_host {
>  	struct clk *core_clk;
>  	struct clk_mux mux;
>  	struct clk *mux_clk;
> -	struct clk *mux_parent[MUX_CLK_NUM_PARENTS];
>  	unsigned long current_clock;
>  
>  	struct clk_divider cfg_div;
> @@ -242,19 +241,18 @@ static int meson_mmc_clk_init(struct meson_host *host)
>  
>  	/* get the mux parents */
>  	for (i = 0; i < MUX_CLK_NUM_PARENTS; i++) {
> +		struct clk *clk;
>  		char name[16];
>  
>  		snprintf(name, sizeof(name), "clkin%d", i);
> -		host->mux_parent[i] = devm_clk_get(host->dev, name);
> -		if (IS_ERR(host->mux_parent[i])) {
> -			ret = PTR_ERR(host->mux_parent[i]);
> -			if (PTR_ERR(host->mux_parent[i]) != -EPROBE_DEFER)
> +		clk = devm_clk_get(host->dev, name);
> +		if (IS_ERR(clk)) {
> +			if (clk != ERR_PTR(-EPROBE_DEFER))
>  				dev_err(host->dev, "Missing clock %s\n", name);
> -			host->mux_parent[i] = NULL;
> -			return ret;
> +			return PTR_ERR(clk);
>  		}
>  
> -		mux_parent_names[i] = __clk_get_name(host->mux_parent[i]);
> +		mux_parent_names[i] = __clk_get_name(clk);
>  	}
>  
>  	/* create the mux */

[PATCH 12/18] mmc: meson-gx: remove member parent_mux from struct meson_host

[Kevin Hilman]

Heiner Kallweit <hkallweit1@gmail.com> writes:

> Member mux_parent isn't used outside meson_mmc_clk_init. So remove it
> and replace it with a local variable in meson_mmc_clk_init.
>
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>

Acked-by: Kevin Hilman <khilman@baylibre.com>

> ---
>  drivers/mmc/host/meson-gx-mmc.c | 14 ++++++--------
>  1 file changed, 6 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/mmc/host/meson-gx-mmc.c b/drivers/mmc/host/meson-gx-mmc.c
> index d05ffe8a..74ceda75 100644
> --- a/drivers/mmc/host/meson-gx-mmc.c
> +++ b/drivers/mmc/host/meson-gx-mmc.c
> @@ -131,7 +131,6 @@ struct meson_host {
>  	struct clk *core_clk;
>  	struct clk_mux mux;
>  	struct clk *mux_clk;
> -	struct clk *mux_parent[MUX_CLK_NUM_PARENTS];
>  	unsigned long current_clock;
>  
>  	struct clk_divider cfg_div;
> @@ -242,19 +241,18 @@ static int meson_mmc_clk_init(struct meson_host *host)
>  
>  	/* get the mux parents */
>  	for (i = 0; i < MUX_CLK_NUM_PARENTS; i++) {
> +		struct clk *clk;
>  		char name[16];
>  
>  		snprintf(name, sizeof(name), "clkin%d", i);
> -		host->mux_parent[i] = devm_clk_get(host->dev, name);
> -		if (IS_ERR(host->mux_parent[i])) {
> -			ret = PTR_ERR(host->mux_parent[i]);
> -			if (PTR_ERR(host->mux_parent[i]) != -EPROBE_DEFER)
> +		clk = devm_clk_get(host->dev, name);
> +		if (IS_ERR(clk)) {
> +			if (clk != ERR_PTR(-EPROBE_DEFER))
>  				dev_err(host->dev, "Missing clock %s\n", name);
> -			host->mux_parent[i] = NULL;
> -			return ret;
> +			return PTR_ERR(clk);
>  		}
>  
> -		mux_parent_names[i] = __clk_get_name(host->mux_parent[i]);
> +		mux_parent_names[i] = __clk_get_name(clk);
>  	}
>  
>  	/* create the mux */

Re: [PATCH 13/18] mmc: meson-gx: remove unneeded meson_mmc_clk_set in meson_mmc_clk_init

[Kevin Hilman]

Heiner Kallweit <hkallweit1@gmail.com> writes:

> I don't think this clk_disable_unprepare is right here. We do this
> in meson_mmc_remove what should be sufficient.

I don't think so, because if _clk_init() fails, then _probe will fail,
and the driver's ->remove() will not be called.

Kevin

> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
> ---
>  drivers/mmc/host/meson-gx-mmc.c | 6 +-----
>  1 file changed, 1 insertion(+), 5 deletions(-)
>
> diff --git a/drivers/mmc/host/meson-gx-mmc.c b/drivers/mmc/host/meson-gx-mmc.c
> index 74ceda75..39904fb5 100644
> --- a/drivers/mmc/host/meson-gx-mmc.c
> +++ b/drivers/mmc/host/meson-gx-mmc.c
> @@ -315,11 +315,7 @@ static int meson_mmc_clk_init(struct meson_host *host)
>  	/* Get the nearest minimum clock to 400KHz */
>  	host->mmc->f_min = clk_round_rate(host->cfg_div_clk, 400000);
>  
> -	ret = meson_mmc_clk_set(host, host->mmc->f_min);
> -	if (!ret)
> -		clk_disable_unprepare(host->cfg_div_clk);
> -
> -	return ret;
> +	return meson_mmc_clk_set(host, host->mmc->f_min);
>  }
>  
>  static void meson_mmc_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)

[PATCH 13/18] mmc: meson-gx: remove unneeded meson_mmc_clk_set in meson_mmc_clk_init

[Kevin Hilman]

Heiner Kallweit <hkallweit1@gmail.com> writes:

> I don't think this clk_disable_unprepare is right here. We do this
> in meson_mmc_remove what should be sufficient.

I don't think so, because if _clk_init() fails, then _probe will fail,
and the driver's ->remove() will not be called.

Kevin

> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
> ---
>  drivers/mmc/host/meson-gx-mmc.c | 6 +-----
>  1 file changed, 1 insertion(+), 5 deletions(-)
>
> diff --git a/drivers/mmc/host/meson-gx-mmc.c b/drivers/mmc/host/meson-gx-mmc.c
> index 74ceda75..39904fb5 100644
> --- a/drivers/mmc/host/meson-gx-mmc.c
> +++ b/drivers/mmc/host/meson-gx-mmc.c
> @@ -315,11 +315,7 @@ static int meson_mmc_clk_init(struct meson_host *host)
>  	/* Get the nearest minimum clock to 400KHz */
>  	host->mmc->f_min = clk_round_rate(host->cfg_div_clk, 400000);
>  
> -	ret = meson_mmc_clk_set(host, host->mmc->f_min);
> -	if (!ret)
> -		clk_disable_unprepare(host->cfg_div_clk);
> -
> -	return ret;
> +	return meson_mmc_clk_set(host, host->mmc->f_min);
>  }
>  
>  static void meson_mmc_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)

Re: [PATCH 14/18] mmc: meson-gx: remove unneeded devm_kstrdup in meson_mmc_clk_init

[Kevin Hilman]

Heiner Kallweit <hkallweit1@gmail.com> writes:

> MMC core does a deep copy of init.name, therefore it's fully ok to

I assume you mean the clock framework, not the MMC core, right?

> provide a local variable.
>
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>

With the changelog fixed,

Acked-by: Kevin Hilman <khilman@baylibre.com>

> ---
>  drivers/mmc/host/meson-gx-mmc.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/mmc/host/meson-gx-mmc.c b/drivers/mmc/host/meson-gx-mmc.c
> index 39904fb5..806554b1 100644
> --- a/drivers/mmc/host/meson-gx-mmc.c
> +++ b/drivers/mmc/host/meson-gx-mmc.c
> @@ -276,7 +276,7 @@ static int meson_mmc_clk_init(struct meson_host *host)
>  
>  	/* create the divider */
>  	snprintf(clk_name, sizeof(clk_name), "%s#div", dev_name(host->dev));
> -	init.name = devm_kstrdup(host->dev, clk_name, GFP_KERNEL);
> +	init.name = clk_name;
>  	init.ops = &clk_divider_ops;
>  	init.flags = CLK_SET_RATE_PARENT;
>  	clk_div_parents[0] = __clk_get_name(host->mux_clk);

[PATCH 14/18] mmc: meson-gx: remove unneeded devm_kstrdup in meson_mmc_clk_init

[Kevin Hilman]

Heiner Kallweit <hkallweit1@gmail.com> writes:

> MMC core does a deep copy of init.name, therefore it's fully ok to

I assume you mean the clock framework, not the MMC core, right?

> provide a local variable.
>
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>

With the changelog fixed,

Acked-by: Kevin Hilman <khilman@baylibre.com>

> ---
>  drivers/mmc/host/meson-gx-mmc.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/mmc/host/meson-gx-mmc.c b/drivers/mmc/host/meson-gx-mmc.c
> index 39904fb5..806554b1 100644
> --- a/drivers/mmc/host/meson-gx-mmc.c
> +++ b/drivers/mmc/host/meson-gx-mmc.c
> @@ -276,7 +276,7 @@ static int meson_mmc_clk_init(struct meson_host *host)
>  
>  	/* create the divider */
>  	snprintf(clk_name, sizeof(clk_name), "%s#div", dev_name(host->dev));
> -	init.name = devm_kstrdup(host->dev, clk_name, GFP_KERNEL);
> +	init.name = clk_name;
>  	init.ops = &clk_divider_ops;
>  	init.flags = CLK_SET_RATE_PARENT;
>  	clk_div_parents[0] = __clk_get_name(host->mux_clk);

Re: [PATCH 15/18] mmc: meson-gx: improve initial configuration

[Kevin Hilman]

Heiner Kallweit <hkallweit1@gmail.com> writes:

> Config values which are not changed during runtime we can set
> in the probe function already.
> The block size setting is overwritten later in
> meson_mmc_start_cmd anyway if needed, so it doesn't harm if we
> remove this setting in meson_mmc_set_ios.
>
> In addition:
> - write config register only if configuration changed
> - Initialize clocks after other config registers have been initialized
>
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>

Acked-by: Kevin Hilman <khilman@baylibre.com>

> ---
>  drivers/mmc/host/meson-gx-mmc.c | 37 +++++++++++++++++++++----------------
>  1 file changed, 21 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/mmc/host/meson-gx-mmc.c b/drivers/mmc/host/meson-gx-mmc.c
> index 806554b1..25b46d36 100644
> --- a/drivers/mmc/host/meson-gx-mmc.c
> +++ b/drivers/mmc/host/meson-gx-mmc.c
> @@ -385,15 +385,6 @@ static void meson_mmc_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
>  	val &= ~(CFG_BUS_WIDTH_MASK << CFG_BUS_WIDTH_SHIFT);
>  	val |= bus_width << CFG_BUS_WIDTH_SHIFT;
>  
> -	val &= ~(CFG_BLK_LEN_MASK << CFG_BLK_LEN_SHIFT);
> -	val |= ilog2(SD_EMMC_CFG_BLK_SIZE) << CFG_BLK_LEN_SHIFT;
> -
> -	val &= ~(CFG_RESP_TIMEOUT_MASK << CFG_RESP_TIMEOUT_SHIFT);
> -	val |= ilog2(SD_EMMC_CFG_RESP_TIMEOUT) << CFG_RESP_TIMEOUT_SHIFT;
> -
> -	val &= ~(CFG_RC_CC_MASK << CFG_RC_CC_SHIFT);
> -	val |= ilog2(SD_EMMC_CFG_CMD_GAP) << CFG_RC_CC_SHIFT;
> -
>  	val &= ~CFG_DDR;
>  	if (ios->timing == MMC_TIMING_UHS_DDR50 ||
>  	    ios->timing == MMC_TIMING_MMC_DDR52 ||
> @@ -404,11 +395,11 @@ static void meson_mmc_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
>  	if (ios->timing == MMC_TIMING_MMC_HS400)
>  		val |= CFG_CHK_DS;
>  
> -	writel(val, host->regs + SD_EMMC_CFG);
> -
> -	if (val != orig)
> +	if (val != orig) {
> +		writel(val, host->regs + SD_EMMC_CFG);
>  		dev_dbg(host->dev, "%s: SD_EMMC_CFG: 0x%08x -> 0x%08x\n",
>  			__func__, orig, val);
> +	}
>  }
>  
>  static void meson_mmc_request_done(struct mmc_host *mmc,
> @@ -675,6 +666,17 @@ static int meson_mmc_get_cd(struct mmc_host *mmc)
>  	return status;
>  }
>  
> +static void meson_mmc_cfg_init(struct meson_host *host)
> +{
> +	u32 cfg = 0;
> +
> +	cfg |= ilog2(SD_EMMC_CFG_RESP_TIMEOUT) << CFG_RESP_TIMEOUT_SHIFT;
> +	cfg |= ilog2(SD_EMMC_CFG_CMD_GAP) << CFG_RC_CC_SHIFT;
> +	cfg |= ilog2(SD_EMMC_CFG_BLK_SIZE) << CFG_BLK_LEN_SHIFT;
> +
> +	writel(cfg, host->regs + SD_EMMC_CFG);
> +}
> +
>  static const struct mmc_host_ops meson_mmc_ops = {
>  	.request	= meson_mmc_request,
>  	.set_ios	= meson_mmc_set_ios,
> @@ -735,10 +737,6 @@ static int meson_mmc_probe(struct platform_device *pdev)
>  	if (ret)
>  		goto free_host;
>  
> -	ret = meson_mmc_clk_init(host);
> -	if (ret)
> -		goto free_host;
> -
>  	/* Stop execution */
>  	writel(0, host->regs + SD_EMMC_START);
>  
> @@ -747,6 +745,13 @@ static int meson_mmc_probe(struct platform_device *pdev)
>  	writel(IRQ_EN_MASK, host->regs + SD_EMMC_STATUS);
>  	writel(IRQ_EN_MASK, host->regs + SD_EMMC_IRQ_EN);
>  
> +	/* set config to sane default */
> +	meson_mmc_cfg_init(host);
> +
> +	ret = meson_mmc_clk_init(host);
> +	if (ret)
> +		goto free_host;
> +
>  	ret = devm_request_threaded_irq(&pdev->dev, irq, meson_mmc_irq,
>  					meson_mmc_irq_thread, IRQF_SHARED,
>  					dev_name(&pdev->dev), host);

[PATCH 15/18] mmc: meson-gx: improve initial configuration

[Kevin Hilman]

Heiner Kallweit <hkallweit1@gmail.com> writes:

> Config values which are not changed during runtime we can set
> in the probe function already.
> The block size setting is overwritten later in
> meson_mmc_start_cmd anyway if needed, so it doesn't harm if we
> remove this setting in meson_mmc_set_ios.
>
> In addition:
> - write config register only if configuration changed
> - Initialize clocks after other config registers have been initialized
>
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>

Acked-by: Kevin Hilman <khilman@baylibre.com>

> ---
>  drivers/mmc/host/meson-gx-mmc.c | 37 +++++++++++++++++++++----------------
>  1 file changed, 21 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/mmc/host/meson-gx-mmc.c b/drivers/mmc/host/meson-gx-mmc.c
> index 806554b1..25b46d36 100644
> --- a/drivers/mmc/host/meson-gx-mmc.c
> +++ b/drivers/mmc/host/meson-gx-mmc.c
> @@ -385,15 +385,6 @@ static void meson_mmc_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
>  	val &= ~(CFG_BUS_WIDTH_MASK << CFG_BUS_WIDTH_SHIFT);
>  	val |= bus_width << CFG_BUS_WIDTH_SHIFT;
>  
> -	val &= ~(CFG_BLK_LEN_MASK << CFG_BLK_LEN_SHIFT);
> -	val |= ilog2(SD_EMMC_CFG_BLK_SIZE) << CFG_BLK_LEN_SHIFT;
> -
> -	val &= ~(CFG_RESP_TIMEOUT_MASK << CFG_RESP_TIMEOUT_SHIFT);
> -	val |= ilog2(SD_EMMC_CFG_RESP_TIMEOUT) << CFG_RESP_TIMEOUT_SHIFT;
> -
> -	val &= ~(CFG_RC_CC_MASK << CFG_RC_CC_SHIFT);
> -	val |= ilog2(SD_EMMC_CFG_CMD_GAP) << CFG_RC_CC_SHIFT;
> -
>  	val &= ~CFG_DDR;
>  	if (ios->timing == MMC_TIMING_UHS_DDR50 ||
>  	    ios->timing == MMC_TIMING_MMC_DDR52 ||
> @@ -404,11 +395,11 @@ static void meson_mmc_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
>  	if (ios->timing == MMC_TIMING_MMC_HS400)
>  		val |= CFG_CHK_DS;
>  
> -	writel(val, host->regs + SD_EMMC_CFG);
> -
> -	if (val != orig)
> +	if (val != orig) {
> +		writel(val, host->regs + SD_EMMC_CFG);
>  		dev_dbg(host->dev, "%s: SD_EMMC_CFG: 0x%08x -> 0x%08x\n",
>  			__func__, orig, val);
> +	}
>  }
>  
>  static void meson_mmc_request_done(struct mmc_host *mmc,
> @@ -675,6 +666,17 @@ static int meson_mmc_get_cd(struct mmc_host *mmc)
>  	return status;
>  }
>  
> +static void meson_mmc_cfg_init(struct meson_host *host)
> +{
> +	u32 cfg = 0;
> +
> +	cfg |= ilog2(SD_EMMC_CFG_RESP_TIMEOUT) << CFG_RESP_TIMEOUT_SHIFT;
> +	cfg |= ilog2(SD_EMMC_CFG_CMD_GAP) << CFG_RC_CC_SHIFT;
> +	cfg |= ilog2(SD_EMMC_CFG_BLK_SIZE) << CFG_BLK_LEN_SHIFT;
> +
> +	writel(cfg, host->regs + SD_EMMC_CFG);
> +}
> +
>  static const struct mmc_host_ops meson_mmc_ops = {
>  	.request	= meson_mmc_request,
>  	.set_ios	= meson_mmc_set_ios,
> @@ -735,10 +737,6 @@ static int meson_mmc_probe(struct platform_device *pdev)
>  	if (ret)
>  		goto free_host;
>  
> -	ret = meson_mmc_clk_init(host);
> -	if (ret)
> -		goto free_host;
> -
>  	/* Stop execution */
>  	writel(0, host->regs + SD_EMMC_START);
>  
> @@ -747,6 +745,13 @@ static int meson_mmc_probe(struct platform_device *pdev)
>  	writel(IRQ_EN_MASK, host->regs + SD_EMMC_STATUS);
>  	writel(IRQ_EN_MASK, host->regs + SD_EMMC_IRQ_EN);
>  
> +	/* set config to sane default */
> +	meson_mmc_cfg_init(host);
> +
> +	ret = meson_mmc_clk_init(host);
> +	if (ret)
> +		goto free_host;
> +
>  	ret = devm_request_threaded_irq(&pdev->dev, irq, meson_mmc_irq,
>  					meson_mmc_irq_thread, IRQF_SHARED,
>  					dev_name(&pdev->dev), host);

Re: [PATCH 16/18] mmc: meson-gx: improve response reading and sending stop command

[Kevin Hilman]

Heiner Kallweit <hkallweit1@gmail.com> writes:

> Stopping execution at the beginning of command processing (again) is
> not needed as reading SD_EMMC_CMD_RSP implicitely stops execution.
> We just have to take care of the case that a command doesn't return a
> response. Therefore read SD_EMMC_CMD_RSP always.
>
> In addition we can read the response in the main irq handler already,
> we don't have to do this in the threaded handler.
>
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>

Acked-by: Kevin Hilman <khilman@baylibre.com>


> ---
>  drivers/mmc/host/meson-gx-mmc.c | 16 ++++++++--------
>  1 file changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/mmc/host/meson-gx-mmc.c b/drivers/mmc/host/meson-gx-mmc.c
> index 25b46d36..6721aee2 100644
> --- a/drivers/mmc/host/meson-gx-mmc.c
> +++ b/drivers/mmc/host/meson-gx-mmc.c
> @@ -510,9 +510,6 @@ static void meson_mmc_request(struct mmc_host *mmc, struct mmc_request *mrq)
>  
>  	WARN_ON(host->mrq != NULL);
>  
> -	/* Stop execution */
> -	writel(0, host->regs + SD_EMMC_START);
> -
>  	host->mrq = mrq;
>  	meson_mmc_start_cmd(mmc, mrq->cmd);
>  }
> @@ -521,12 +518,16 @@ static void meson_mmc_read_resp(struct mmc_host *mmc, struct mmc_command *cmd)
>  {
>  	struct meson_host *host = mmc_priv(mmc);
>  
> +	/*
> +	 * reading SD_EMMC_CMD_RSP stops processing, therefore read
> +	 * this register even if command has no response
> +	 */
>  	if (cmd->flags & MMC_RSP_136) {
>  		cmd->resp[0] = readl(host->regs + SD_EMMC_CMD_RSP3);
>  		cmd->resp[1] = readl(host->regs + SD_EMMC_CMD_RSP2);
>  		cmd->resp[2] = readl(host->regs + SD_EMMC_CMD_RSP1);
>  		cmd->resp[3] = readl(host->regs + SD_EMMC_CMD_RSP);
> -	} else if (cmd->flags & MMC_RSP_PRESENT) {
> +	} else {
>  		cmd->resp[0] = readl(host->regs + SD_EMMC_CMD_RSP);
>  	}
>  }
> @@ -564,6 +565,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");
> @@ -610,10 +613,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;
> @@ -643,7 +644,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 (mmc_op_multi(cmd->opcode))
>  		meson_mmc_start_cmd(host->mmc, data->stop);
>  	else

[PATCH 16/18] mmc: meson-gx: improve response reading and sending stop command

[Kevin Hilman]

Heiner Kallweit <hkallweit1@gmail.com> writes:

> Stopping execution at the beginning of command processing (again) is
> not needed as reading SD_EMMC_CMD_RSP implicitely stops execution.
> We just have to take care of the case that a command doesn't return a
> response. Therefore read SD_EMMC_CMD_RSP always.
>
> In addition we can read the response in the main irq handler already,
> we don't have to do this in the threaded handler.
>
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>

Acked-by: Kevin Hilman <khilman@baylibre.com>


> ---
>  drivers/mmc/host/meson-gx-mmc.c | 16 ++++++++--------
>  1 file changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/mmc/host/meson-gx-mmc.c b/drivers/mmc/host/meson-gx-mmc.c
> index 25b46d36..6721aee2 100644
> --- a/drivers/mmc/host/meson-gx-mmc.c
> +++ b/drivers/mmc/host/meson-gx-mmc.c
> @@ -510,9 +510,6 @@ static void meson_mmc_request(struct mmc_host *mmc, struct mmc_request *mrq)
>  
>  	WARN_ON(host->mrq != NULL);
>  
> -	/* Stop execution */
> -	writel(0, host->regs + SD_EMMC_START);
> -
>  	host->mrq = mrq;
>  	meson_mmc_start_cmd(mmc, mrq->cmd);
>  }
> @@ -521,12 +518,16 @@ static void meson_mmc_read_resp(struct mmc_host *mmc, struct mmc_command *cmd)
>  {
>  	struct meson_host *host = mmc_priv(mmc);
>  
> +	/*
> +	 * reading SD_EMMC_CMD_RSP stops processing, therefore read
> +	 * this register even if command has no response
> +	 */
>  	if (cmd->flags & MMC_RSP_136) {
>  		cmd->resp[0] = readl(host->regs + SD_EMMC_CMD_RSP3);
>  		cmd->resp[1] = readl(host->regs + SD_EMMC_CMD_RSP2);
>  		cmd->resp[2] = readl(host->regs + SD_EMMC_CMD_RSP1);
>  		cmd->resp[3] = readl(host->regs + SD_EMMC_CMD_RSP);
> -	} else if (cmd->flags & MMC_RSP_PRESENT) {
> +	} else {
>  		cmd->resp[0] = readl(host->regs + SD_EMMC_CMD_RSP);
>  	}
>  }
> @@ -564,6 +565,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");
> @@ -610,10 +613,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;
> @@ -643,7 +644,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 (mmc_op_multi(cmd->opcode))
>  		meson_mmc_start_cmd(host->mmc, data->stop);
>  	else

Re: [PATCH 14/18] mmc: meson-gx: remove unneeded devm_kstrdup in meson_mmc_clk_init

[Heiner Kallweit]

Am 15.02.2017 um 20:30 schrieb Kevin Hilman:
> Heiner Kallweit <hkallweit1@gmail.com> writes:
> 
>> MMC core does a deep copy of init.name, therefore it's fully ok to
> 
> I assume you mean the clock framework, not the MMC core, right?
> 
Uups, of course.

>> provide a local variable.
>>
>> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
> 
> With the changelog fixed,
> 
> Acked-by: Kevin Hilman <khilman@baylibre.com>
> 
>> ---
>>  drivers/mmc/host/meson-gx-mmc.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/mmc/host/meson-gx-mmc.c b/drivers/mmc/host/meson-gx-mmc.c
>> index 39904fb5..806554b1 100644
>> --- a/drivers/mmc/host/meson-gx-mmc.c
>> +++ b/drivers/mmc/host/meson-gx-mmc.c
>> @@ -276,7 +276,7 @@ static int meson_mmc_clk_init(struct meson_host *host)
>>  
>>  	/* create the divider */
>>  	snprintf(clk_name, sizeof(clk_name), "%s#div", dev_name(host->dev));
>> -	init.name = devm_kstrdup(host->dev, clk_name, GFP_KERNEL);
>> +	init.name = clk_name;
>>  	init.ops = &clk_divider_ops;
>>  	init.flags = CLK_SET_RATE_PARENT;
>>  	clk_div_parents[0] = __clk_get_name(host->mux_clk);
> 

[PATCH 14/18] mmc: meson-gx: remove unneeded devm_kstrdup in meson_mmc_clk_init

[Heiner Kallweit]

Am 15.02.2017 um 20:30 schrieb Kevin Hilman:
> Heiner Kallweit <hkallweit1@gmail.com> writes:
> 
>> MMC core does a deep copy of init.name, therefore it's fully ok to
> 
> I assume you mean the clock framework, not the MMC core, right?
> 
Uups, of course.

>> provide a local variable.
>>
>> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
> 
> With the changelog fixed,
> 
> Acked-by: Kevin Hilman <khilman@baylibre.com>
> 
>> ---
>>  drivers/mmc/host/meson-gx-mmc.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/mmc/host/meson-gx-mmc.c b/drivers/mmc/host/meson-gx-mmc.c
>> index 39904fb5..806554b1 100644
>> --- a/drivers/mmc/host/meson-gx-mmc.c
>> +++ b/drivers/mmc/host/meson-gx-mmc.c
>> @@ -276,7 +276,7 @@ static int meson_mmc_clk_init(struct meson_host *host)
>>  
>>  	/* create the divider */
>>  	snprintf(clk_name, sizeof(clk_name), "%s#div", dev_name(host->dev));
>> -	init.name = devm_kstrdup(host->dev, clk_name, GFP_KERNEL);
>> +	init.name = clk_name;
>>  	init.ops = &clk_divider_ops;
>>  	init.flags = CLK_SET_RATE_PARENT;
>>  	clk_div_parents[0] = __clk_get_name(host->mux_clk);
> 

Re: [PATCH 17/18] mmc: meson-gx: remove member mrq from struct meson_host

[Kevin Hilman]

Heiner Kallweit <hkallweit1@gmail.com> writes:

> Struct mmc_command includes a reference to the related mmc_request.
> Therefore we don't have to store mrq separately in struct meson_host.

This patch also removes a bunch of WARNs also, which shoulc be mentioned
here as well.  Those were quite handy during development, but now that
things have stabilized, I'm OK with them being removed.

> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>

With the changelog update,

Acked-by: Kevin Hilman <khilman@baylibre.com>

> ---
>  drivers/mmc/host/meson-gx-mmc.c | 23 ++---------------------
>  1 file changed, 2 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/mmc/host/meson-gx-mmc.c b/drivers/mmc/host/meson-gx-mmc.c
> index 6721aee2..08ff29be 100644
> --- a/drivers/mmc/host/meson-gx-mmc.c
> +++ b/drivers/mmc/host/meson-gx-mmc.c
> @@ -123,7 +123,6 @@
>  struct meson_host {
>  	struct	device		*dev;
>  	struct	mmc_host	*mmc;
> -	struct	mmc_request	*mrq;
>  	struct	mmc_command	*cmd;
>  
>  	spinlock_t lock;
> @@ -407,9 +406,6 @@ static void meson_mmc_request_done(struct mmc_host *mmc,
>  {
>  	struct meson_host *host = mmc_priv(mmc);
>  
> -	WARN_ON(host->mrq != mrq);
> -
> -	host->mrq = NULL;
>  	host->cmd = NULL;
>  	mmc_request_done(host->mmc, mrq);
>  }
> @@ -506,11 +502,6 @@ static void meson_mmc_start_cmd(struct mmc_host *mmc, struct mmc_command *cmd)
>  
>  static void meson_mmc_request(struct mmc_host *mmc, struct mmc_request *mrq)
>  {
> -	struct meson_host *host = mmc_priv(mmc);
> -
> -	WARN_ON(host->mrq != NULL);
> -
> -	host->mrq = mrq;
>  	meson_mmc_start_cmd(mmc, mrq->cmd);
>  }
>  
> @@ -535,7 +526,6 @@ static void meson_mmc_read_resp(struct mmc_host *mmc, struct mmc_command *cmd)
>  static irqreturn_t meson_mmc_irq(int irq, void *dev_id)
>  {
>  	struct meson_host *host = dev_id;
> -	struct mmc_request *mrq;
>  	struct mmc_command *cmd;
>  	u32 irq_en, status, raw_status;
>  	irqreturn_t ret = IRQ_HANDLED;
> @@ -545,11 +535,6 @@ static irqreturn_t meson_mmc_irq(int irq, void *dev_id)
>  
>  	cmd = host->cmd;
>  
> -	mrq = host->mrq;
> -
> -	if (WARN_ON(!mrq))
> -		return IRQ_NONE;
> -
>  	if (WARN_ON(!cmd))
>  		return IRQ_NONE;
>  
> @@ -598,7 +583,7 @@ static irqreturn_t meson_mmc_irq(int irq, void *dev_id)
>  	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, mrq->stop ? 1 : 0);
> +			 cmd->flags, cmd->mrq->stop ? 1 : 0);
>  		if (cmd->data) {
>  			struct mmc_data *data = cmd->data;
>  
> @@ -623,14 +608,10 @@ static irqreturn_t meson_mmc_irq(int irq, void *dev_id)
>  static irqreturn_t meson_mmc_irq_thread(int irq, void *dev_id)
>  {
>  	struct meson_host *host = dev_id;
> -	struct mmc_request *mrq = host->mrq;
>  	struct mmc_command *cmd = host->cmd;
>  	struct mmc_data *data;
>  	unsigned int xfer_bytes;
>  
> -	if (WARN_ON(!mrq))
> -		return IRQ_NONE;
> -
>  	if (WARN_ON(!cmd))
>  		return IRQ_NONE;
>  
> @@ -647,7 +628,7 @@ static irqreturn_t meson_mmc_irq_thread(int irq, void *dev_id)
>  	if (mmc_op_multi(cmd->opcode))
>  		meson_mmc_start_cmd(host->mmc, data->stop);
>  	else
> -		meson_mmc_request_done(host->mmc, mrq);
> +		meson_mmc_request_done(host->mmc, cmd->mrq);
>  
>  	return IRQ_HANDLED;
>  }

[PATCH 17/18] mmc: meson-gx: remove member mrq from struct meson_host

[Kevin Hilman]

Heiner Kallweit <hkallweit1@gmail.com> writes:

> Struct mmc_command includes a reference to the related mmc_request.
> Therefore we don't have to store mrq separately in struct meson_host.

This patch also removes a bunch of WARNs also, which shoulc be mentioned
here as well.  Those were quite handy during development, but now that
things have stabilized, I'm OK with them being removed.

> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>

With the changelog update,

Acked-by: Kevin Hilman <khilman@baylibre.com>

> ---
>  drivers/mmc/host/meson-gx-mmc.c | 23 ++---------------------
>  1 file changed, 2 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/mmc/host/meson-gx-mmc.c b/drivers/mmc/host/meson-gx-mmc.c
> index 6721aee2..08ff29be 100644
> --- a/drivers/mmc/host/meson-gx-mmc.c
> +++ b/drivers/mmc/host/meson-gx-mmc.c
> @@ -123,7 +123,6 @@
>  struct meson_host {
>  	struct	device		*dev;
>  	struct	mmc_host	*mmc;
> -	struct	mmc_request	*mrq;
>  	struct	mmc_command	*cmd;
>  
>  	spinlock_t lock;
> @@ -407,9 +406,6 @@ static void meson_mmc_request_done(struct mmc_host *mmc,
>  {
>  	struct meson_host *host = mmc_priv(mmc);
>  
> -	WARN_ON(host->mrq != mrq);
> -
> -	host->mrq = NULL;
>  	host->cmd = NULL;
>  	mmc_request_done(host->mmc, mrq);
>  }
> @@ -506,11 +502,6 @@ static void meson_mmc_start_cmd(struct mmc_host *mmc, struct mmc_command *cmd)
>  
>  static void meson_mmc_request(struct mmc_host *mmc, struct mmc_request *mrq)
>  {
> -	struct meson_host *host = mmc_priv(mmc);
> -
> -	WARN_ON(host->mrq != NULL);
> -
> -	host->mrq = mrq;
>  	meson_mmc_start_cmd(mmc, mrq->cmd);
>  }
>  
> @@ -535,7 +526,6 @@ static void meson_mmc_read_resp(struct mmc_host *mmc, struct mmc_command *cmd)
>  static irqreturn_t meson_mmc_irq(int irq, void *dev_id)
>  {
>  	struct meson_host *host = dev_id;
> -	struct mmc_request *mrq;
>  	struct mmc_command *cmd;
>  	u32 irq_en, status, raw_status;
>  	irqreturn_t ret = IRQ_HANDLED;
> @@ -545,11 +535,6 @@ static irqreturn_t meson_mmc_irq(int irq, void *dev_id)
>  
>  	cmd = host->cmd;
>  
> -	mrq = host->mrq;
> -
> -	if (WARN_ON(!mrq))
> -		return IRQ_NONE;
> -
>  	if (WARN_ON(!cmd))
>  		return IRQ_NONE;
>  
> @@ -598,7 +583,7 @@ static irqreturn_t meson_mmc_irq(int irq, void *dev_id)
>  	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, mrq->stop ? 1 : 0);
> +			 cmd->flags, cmd->mrq->stop ? 1 : 0);
>  		if (cmd->data) {
>  			struct mmc_data *data = cmd->data;
>  
> @@ -623,14 +608,10 @@ static irqreturn_t meson_mmc_irq(int irq, void *dev_id)
>  static irqreturn_t meson_mmc_irq_thread(int irq, void *dev_id)
>  {
>  	struct meson_host *host = dev_id;
> -	struct mmc_request *mrq = host->mrq;
>  	struct mmc_command *cmd = host->cmd;
>  	struct mmc_data *data;
>  	unsigned int xfer_bytes;
>  
> -	if (WARN_ON(!mrq))
> -		return IRQ_NONE;
> -
>  	if (WARN_ON(!cmd))
>  		return IRQ_NONE;
>  
> @@ -647,7 +628,7 @@ static irqreturn_t meson_mmc_irq_thread(int irq, void *dev_id)
>  	if (mmc_op_multi(cmd->opcode))
>  		meson_mmc_start_cmd(host->mmc, data->stop);
>  	else
> -		meson_mmc_request_done(host->mmc, mrq);
> +		meson_mmc_request_done(host->mmc, cmd->mrq);
>  
>  	return IRQ_HANDLED;
>  }

Re: [PATCH 13/18] mmc: meson-gx: remove unneeded meson_mmc_clk_set in meson_mmc_clk_init

[Heiner Kallweit]

Am 15.02.2017 um 20:27 schrieb Kevin Hilman:
> Heiner Kallweit <hkallweit1@gmail.com> writes:
> 
>> I don't think this clk_disable_unprepare is right here. We do this
>> in meson_mmc_remove what should be sufficient.
> 
> I don't think so, because if _clk_init() fails, then _probe will fail,
> and the driver's ->remove() will not be called.
> 
In general, then the condition should be "if (ret)", not "if (!ret)".
And if _clk_init() fails, then probe jumps to label free_host,
where we have exactly this clk_disable_unprepare(host->cfg_div_clk)

Heiner

> Kevin
> 
>> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
>> ---
>>  drivers/mmc/host/meson-gx-mmc.c | 6 +-----
>>  1 file changed, 1 insertion(+), 5 deletions(-)
>>
>> diff --git a/drivers/mmc/host/meson-gx-mmc.c b/drivers/mmc/host/meson-gx-mmc.c
>> index 74ceda75..39904fb5 100644
>> --- a/drivers/mmc/host/meson-gx-mmc.c
>> +++ b/drivers/mmc/host/meson-gx-mmc.c
>> @@ -315,11 +315,7 @@ static int meson_mmc_clk_init(struct meson_host *host)
>>  	/* Get the nearest minimum clock to 400KHz */
>>  	host->mmc->f_min = clk_round_rate(host->cfg_div_clk, 400000);
>>  
>> -	ret = meson_mmc_clk_set(host, host->mmc->f_min);
>> -	if (!ret)
>> -		clk_disable_unprepare(host->cfg_div_clk);
>> -
>> -	return ret;
>> +	return meson_mmc_clk_set(host, host->mmc->f_min);
>>  }
>>  
>>  static void meson_mmc_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
> 

[PATCH 13/18] mmc: meson-gx: remove unneeded meson_mmc_clk_set in meson_mmc_clk_init

[Heiner Kallweit]

Am 15.02.2017 um 20:27 schrieb Kevin Hilman:
> Heiner Kallweit <hkallweit1@gmail.com> writes:
> 
>> I don't think this clk_disable_unprepare is right here. We do this
>> in meson_mmc_remove what should be sufficient.
> 
> I don't think so, because if _clk_init() fails, then _probe will fail,
> and the driver's ->remove() will not be called.
> 
In general, then the condition should be "if (ret)", not "if (!ret)".
And if _clk_init() fails, then probe jumps to label free_host,
where we have exactly this clk_disable_unprepare(host->cfg_div_clk)

Heiner

> Kevin
> 
>> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
>> ---
>>  drivers/mmc/host/meson-gx-mmc.c | 6 +-----
>>  1 file changed, 1 insertion(+), 5 deletions(-)
>>
>> diff --git a/drivers/mmc/host/meson-gx-mmc.c b/drivers/mmc/host/meson-gx-mmc.c
>> index 74ceda75..39904fb5 100644
>> --- a/drivers/mmc/host/meson-gx-mmc.c
>> +++ b/drivers/mmc/host/meson-gx-mmc.c
>> @@ -315,11 +315,7 @@ static int meson_mmc_clk_init(struct meson_host *host)
>>  	/* Get the nearest minimum clock to 400KHz */
>>  	host->mmc->f_min = clk_round_rate(host->cfg_div_clk, 400000);
>>  
>> -	ret = meson_mmc_clk_set(host, host->mmc->f_min);
>> -	if (!ret)
>> -		clk_disable_unprepare(host->cfg_div_clk);
>> -
>> -	return ret;
>> +	return meson_mmc_clk_set(host, host->mmc->f_min);
>>  }
>>  
>>  static void meson_mmc_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
> 

Re: [PATCH 04/18] mmc: meson-gx: improve meson_mmc_start_cmd

[Heiner Kallweit]

Am 15.02.2017 um 18:04 schrieb Kevin Hilman:
> Heiner Kallweit <hkallweit1@gmail.com> writes:
> 
>> Remove use of unneeded members cmd_arg and cmd_resp.
>> Setting SD_EMMC_CMD_RSP is only needed if CMD_CFG_RESP_NUM is set,
>> so don't write this register in all other cases.
>>
>> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
> 
> I'm not sure I like this change.  This works now because there is only
> one descriptor used, but one of the next things to work on in this
> driver is taking advantage of the internal DMA capabilities, which means
> having a chain of descriptorsall filled out in memory.
> 
For testing purposes I temporarily changed the driver from passing the
descriptor in registers to passing the descriptor via DMA and it worked.

I'm not very familiar (yet) with descriptor chains and have to check
the MMC core code a little bit more ..
If we can actually benefit from it then I'd agree with you.


> Kevin
> 
>> ---
>>  drivers/mmc/host/meson-gx-mmc.c | 6 ++----
>>  1 file changed, 2 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/mmc/host/meson-gx-mmc.c b/drivers/mmc/host/meson-gx-mmc.c
>> index ece38b44..630e0590 100644
>> --- a/drivers/mmc/host/meson-gx-mmc.c
>> +++ b/drivers/mmc/host/meson-gx-mmc.c
>> @@ -456,7 +456,6 @@ static void meson_mmc_start_cmd(struct mmc_host *mmc, struct mmc_command *cmd)
>>  	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;
>>  
>>  	/* Response */
>>  	if (cmd->flags & MMC_RSP_PRESENT) {
>> @@ -464,7 +463,7 @@ static void meson_mmc_start_cmd(struct mmc_host *mmc, struct mmc_command *cmd)
>>  		if (cmd->flags & MMC_RSP_136)
>>  			desc->cmd_cfg |= CMD_CFG_RESP_128;
>>  		desc->cmd_cfg |= CMD_CFG_RESP_NUM;
>> -		desc->cmd_resp = 0;
>> +		writel(0, host->regs + SD_EMMC_CMD_RSP);
>>  
>>  		if (!(cmd->flags & MMC_RSP_CRC))
>>  			desc->cmd_cfg |= CMD_CFG_RESP_NOCRC;
>> @@ -540,9 +539,8 @@ static void meson_mmc_start_cmd(struct mmc_host *mmc, struct mmc_command *cmd)
>>  	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);
>>  	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)
> 

[PATCH 04/18] mmc: meson-gx: improve meson_mmc_start_cmd

[Heiner Kallweit]

Am 15.02.2017 um 18:04 schrieb Kevin Hilman:
> Heiner Kallweit <hkallweit1@gmail.com> writes:
> 
>> Remove use of unneeded members cmd_arg and cmd_resp.
>> Setting SD_EMMC_CMD_RSP is only needed if CMD_CFG_RESP_NUM is set,
>> so don't write this register in all other cases.
>>
>> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
> 
> I'm not sure I like this change.  This works now because there is only
> one descriptor used, but one of the next things to work on in this
> driver is taking advantage of the internal DMA capabilities, which means
> having a chain of descriptorsall filled out in memory.
> 
For testing purposes I temporarily changed the driver from passing the
descriptor in registers to passing the descriptor via DMA and it worked.

I'm not very familiar (yet) with descriptor chains and have to check
the MMC core code a little bit more ..
If we can actually benefit from it then I'd agree with you.


> Kevin
> 
>> ---
>>  drivers/mmc/host/meson-gx-mmc.c | 6 ++----
>>  1 file changed, 2 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/mmc/host/meson-gx-mmc.c b/drivers/mmc/host/meson-gx-mmc.c
>> index ece38b44..630e0590 100644
>> --- a/drivers/mmc/host/meson-gx-mmc.c
>> +++ b/drivers/mmc/host/meson-gx-mmc.c
>> @@ -456,7 +456,6 @@ static void meson_mmc_start_cmd(struct mmc_host *mmc, struct mmc_command *cmd)
>>  	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;
>>  
>>  	/* Response */
>>  	if (cmd->flags & MMC_RSP_PRESENT) {
>> @@ -464,7 +463,7 @@ static void meson_mmc_start_cmd(struct mmc_host *mmc, struct mmc_command *cmd)
>>  		if (cmd->flags & MMC_RSP_136)
>>  			desc->cmd_cfg |= CMD_CFG_RESP_128;
>>  		desc->cmd_cfg |= CMD_CFG_RESP_NUM;
>> -		desc->cmd_resp = 0;
>> +		writel(0, host->regs + SD_EMMC_CMD_RSP);
>>  
>>  		if (!(cmd->flags & MMC_RSP_CRC))
>>  			desc->cmd_cfg |= CMD_CFG_RESP_NOCRC;
>> @@ -540,9 +539,8 @@ static void meson_mmc_start_cmd(struct mmc_host *mmc, struct mmc_command *cmd)
>>  	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);
>>  	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)
> 

Re: [PATCH 09/18] mmc: meson-gx: change interrupt name

[Heiner Kallweit]

Am 15.02.2017 um 13:34 schrieb Ben Dooks:
> On 14/02/17 20:06, Heiner Kallweit wrote:
>> So far the driver name is used for the interrupt. Therefore in
>> /proc/interrupts we can't tell which interrupt belongs to which
>> SD_EMMC port.
>> So let's use the device name instead (what is the OF node name).
>>
>> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
>> ---
>>  drivers/mmc/host/meson-gx-mmc.c | 6 +++---
>>  1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/mmc/host/meson-gx-mmc.c b/drivers/mmc/host/meson-gx-mmc.c
>> index a67c811e..1a76c925 100644
>> --- a/drivers/mmc/host/meson-gx-mmc.c
>> +++ b/drivers/mmc/host/meson-gx-mmc.c
>> @@ -757,9 +757,9 @@ static int meson_mmc_probe(struct platform_device *pdev)
>>      writel(IRQ_EN_MASK, host->regs + SD_EMMC_STATUS);
>>      writel(IRQ_EN_MASK, host->regs + SD_EMMC_IRQ_EN);
>>
>> -    ret = devm_request_threaded_irq(&pdev->dev, host->irq,
>> -                    meson_mmc_irq, meson_mmc_irq_thread,
>> -                    IRQF_SHARED, DRIVER_NAME, host);
>> +    ret = devm_request_threaded_irq(&pdev->dev, host->irq, meson_mmc_irq,
>> +                    meson_mmc_irq_thread, IRQF_SHARED,
>> +                    dev_name(&pdev->dev), host);
>>      if (ret)
>>          goto free_host;
>>
> 
> I am surpised that the default is not to use dev_name(dev), I wonder if
> that should be something that devm_request_threaded_irq() could be
> updated to do.
> 
I asked myself the same question and submitted a patch:
https://lkml.org/lkml/2017/2/12/80

> 

[PATCH 09/18] mmc: meson-gx: change interrupt name

[Heiner Kallweit]

Am 15.02.2017 um 13:34 schrieb Ben Dooks:
> On 14/02/17 20:06, Heiner Kallweit wrote:
>> So far the driver name is used for the interrupt. Therefore in
>> /proc/interrupts we can't tell which interrupt belongs to which
>> SD_EMMC port.
>> So let's use the device name instead (what is the OF node name).
>>
>> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
>> ---
>>  drivers/mmc/host/meson-gx-mmc.c | 6 +++---
>>  1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/mmc/host/meson-gx-mmc.c b/drivers/mmc/host/meson-gx-mmc.c
>> index a67c811e..1a76c925 100644
>> --- a/drivers/mmc/host/meson-gx-mmc.c
>> +++ b/drivers/mmc/host/meson-gx-mmc.c
>> @@ -757,9 +757,9 @@ static int meson_mmc_probe(struct platform_device *pdev)
>>      writel(IRQ_EN_MASK, host->regs + SD_EMMC_STATUS);
>>      writel(IRQ_EN_MASK, host->regs + SD_EMMC_IRQ_EN);
>>
>> -    ret = devm_request_threaded_irq(&pdev->dev, host->irq,
>> -                    meson_mmc_irq, meson_mmc_irq_thread,
>> -                    IRQF_SHARED, DRIVER_NAME, host);
>> +    ret = devm_request_threaded_irq(&pdev->dev, host->irq, meson_mmc_irq,
>> +                    meson_mmc_irq_thread, IRQF_SHARED,
>> +                    dev_name(&pdev->dev), host);
>>      if (ret)
>>          goto free_host;
>>
> 
> I am surpised that the default is not to use dev_name(dev), I wonder if
> that should be something that devm_request_threaded_irq() could be
> updated to do.
> 
I asked myself the same question and submitted a patch:
https://lkml.org/lkml/2017/2/12/80

> 

Re: [PATCH 04/18] mmc: meson-gx: improve meson_mmc_start_cmd

[Heiner Kallweit]

Am 15.02.2017 um 20:43 schrieb Heiner Kallweit:
> Am 15.02.2017 um 18:04 schrieb Kevin Hilman:
>> Heiner Kallweit <hkallweit1@gmail.com> writes:
>>
>>> Remove use of unneeded members cmd_arg and cmd_resp.
>>> Setting SD_EMMC_CMD_RSP is only needed if CMD_CFG_RESP_NUM is set,
>>> so don't write this register in all other cases.
>>>
>>> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
>>
>> I'm not sure I like this change.  This works now because there is only
>> one descriptor used, but one of the next things to work on in this
>> driver is taking advantage of the internal DMA capabilities, which means
>> having a chain of descriptorsall filled out in memory.
>>
> For testing purposes I temporarily changed the driver from passing the
> descriptor in registers to passing the descriptor via DMA and it worked.
> 
> I'm not very familiar (yet) with descriptor chains and have to check
> the MMC core code a little bit more ..
> If we can actually benefit from it then I'd agree with you.
> 
After having had a little bit closer look at the MMC core and descriptor
chains in general:
I'm not really sure where descriptor chains would help us. SG lists in
read / write commands are linearized by the driver via
sg_copy_[to,from]_buffer already.
Beyond that I don't see any command chaining support in the core
(prerequisite for command chaining most likely would be that a
subsequent command must not depend on the response of a previous one).

Maybe somebody with more knowledge about the MMC core and MMC in general
can shed some light on this ..

Heiner
> 
>> Kevin
>>
>>> ---
>>>  drivers/mmc/host/meson-gx-mmc.c | 6 ++----
>>>  1 file changed, 2 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/drivers/mmc/host/meson-gx-mmc.c b/drivers/mmc/host/meson-gx-mmc.c
>>> index ece38b44..630e0590 100644
>>> --- a/drivers/mmc/host/meson-gx-mmc.c
>>> +++ b/drivers/mmc/host/meson-gx-mmc.c
>>> @@ -456,7 +456,6 @@ static void meson_mmc_start_cmd(struct mmc_host *mmc, struct mmc_command *cmd)
>>>  	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;
>>>  
>>>  	/* Response */
>>>  	if (cmd->flags & MMC_RSP_PRESENT) {
>>> @@ -464,7 +463,7 @@ static void meson_mmc_start_cmd(struct mmc_host *mmc, struct mmc_command *cmd)
>>>  		if (cmd->flags & MMC_RSP_136)
>>>  			desc->cmd_cfg |= CMD_CFG_RESP_128;
>>>  		desc->cmd_cfg |= CMD_CFG_RESP_NUM;
>>> -		desc->cmd_resp = 0;
>>> +		writel(0, host->regs + SD_EMMC_CMD_RSP);
>>>  
>>>  		if (!(cmd->flags & MMC_RSP_CRC))
>>>  			desc->cmd_cfg |= CMD_CFG_RESP_NOCRC;
>>> @@ -540,9 +539,8 @@ static void meson_mmc_start_cmd(struct mmc_host *mmc, struct mmc_command *cmd)
>>>  	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);
>>>  	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)
>>
> 

[PATCH 04/18] mmc: meson-gx: improve meson_mmc_start_cmd

[Heiner Kallweit]

Am 15.02.2017 um 20:43 schrieb Heiner Kallweit:
> Am 15.02.2017 um 18:04 schrieb Kevin Hilman:
>> Heiner Kallweit <hkallweit1@gmail.com> writes:
>>
>>> Remove use of unneeded members cmd_arg and cmd_resp.
>>> Setting SD_EMMC_CMD_RSP is only needed if CMD_CFG_RESP_NUM is set,
>>> so don't write this register in all other cases.
>>>
>>> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
>>
>> I'm not sure I like this change.  This works now because there is only
>> one descriptor used, but one of the next things to work on in this
>> driver is taking advantage of the internal DMA capabilities, which means
>> having a chain of descriptorsall filled out in memory.
>>
> For testing purposes I temporarily changed the driver from passing the
> descriptor in registers to passing the descriptor via DMA and it worked.
> 
> I'm not very familiar (yet) with descriptor chains and have to check
> the MMC core code a little bit more ..
> If we can actually benefit from it then I'd agree with you.
> 
After having had a little bit closer look at the MMC core and descriptor
chains in general:
I'm not really sure where descriptor chains would help us. SG lists in
read / write commands are linearized by the driver via
sg_copy_[to,from]_buffer already.
Beyond that I don't see any command chaining support in the core
(prerequisite for command chaining most likely would be that a
subsequent command must not depend on the response of a previous one).

Maybe somebody with more knowledge about the MMC core and MMC in general
can shed some light on this ..

Heiner
> 
>> Kevin
>>
>>> ---
>>>  drivers/mmc/host/meson-gx-mmc.c | 6 ++----
>>>  1 file changed, 2 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/drivers/mmc/host/meson-gx-mmc.c b/drivers/mmc/host/meson-gx-mmc.c
>>> index ece38b44..630e0590 100644
>>> --- a/drivers/mmc/host/meson-gx-mmc.c
>>> +++ b/drivers/mmc/host/meson-gx-mmc.c
>>> @@ -456,7 +456,6 @@ static void meson_mmc_start_cmd(struct mmc_host *mmc, struct mmc_command *cmd)
>>>  	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;
>>>  
>>>  	/* Response */
>>>  	if (cmd->flags & MMC_RSP_PRESENT) {
>>> @@ -464,7 +463,7 @@ static void meson_mmc_start_cmd(struct mmc_host *mmc, struct mmc_command *cmd)
>>>  		if (cmd->flags & MMC_RSP_136)
>>>  			desc->cmd_cfg |= CMD_CFG_RESP_128;
>>>  		desc->cmd_cfg |= CMD_CFG_RESP_NUM;
>>> -		desc->cmd_resp = 0;
>>> +		writel(0, host->regs + SD_EMMC_CMD_RSP);
>>>  
>>>  		if (!(cmd->flags & MMC_RSP_CRC))
>>>  			desc->cmd_cfg |= CMD_CFG_RESP_NOCRC;
>>> @@ -540,9 +539,8 @@ static void meson_mmc_start_cmd(struct mmc_host *mmc, struct mmc_command *cmd)
>>>  	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);
>>>  	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)
>>
> 

Re: [PATCH 13/18] mmc: meson-gx: remove unneeded meson_mmc_clk_set in meson_mmc_clk_init

[Kevin Hilman]

Heiner Kallweit <hkallweit1@gmail.com> writes:

> Am 15.02.2017 um 20:27 schrieb Kevin Hilman:
>> Heiner Kallweit <hkallweit1@gmail.com> writes:
>> 
>>> I don't think this clk_disable_unprepare is right here. We do this
>>> in meson_mmc_remove what should be sufficient.
>> 
>> I don't think so, because if _clk_init() fails, then _probe will fail,
>> and the driver's ->remove() will not be called.
>> 
> In general, then the condition should be "if (ret)", not "if (!ret)".
> And if _clk_init() fails, then probe jumps to label free_host,
> where we have exactly this clk_disable_unprepare(host->cfg_div_clk)

Then you need to update the changelog, which refers to remove, not the
cleanup in probe.

Kevin

>> Kevin
>> 
>>> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
>>> ---
>>>  drivers/mmc/host/meson-gx-mmc.c | 6 +-----
>>>  1 file changed, 1 insertion(+), 5 deletions(-)
>>>
>>> diff --git a/drivers/mmc/host/meson-gx-mmc.c b/drivers/mmc/host/meson-gx-mmc.c
>>> index 74ceda75..39904fb5 100644
>>> --- a/drivers/mmc/host/meson-gx-mmc.c
>>> +++ b/drivers/mmc/host/meson-gx-mmc.c
>>> @@ -315,11 +315,7 @@ static int meson_mmc_clk_init(struct meson_host *host)
>>>  	/* Get the nearest minimum clock to 400KHz */
>>>  	host->mmc->f_min = clk_round_rate(host->cfg_div_clk, 400000);
>>>  
>>> -	ret = meson_mmc_clk_set(host, host->mmc->f_min);
>>> -	if (!ret)
>>> -		clk_disable_unprepare(host->cfg_div_clk);
>>> -
>>> -	return ret;
>>> +	return meson_mmc_clk_set(host, host->mmc->f_min);
>>>  }
>>>  
>>>  static void meson_mmc_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
>> 

[PATCH 13/18] mmc: meson-gx: remove unneeded meson_mmc_clk_set in meson_mmc_clk_init

[Kevin Hilman]

Heiner Kallweit <hkallweit1@gmail.com> writes:

> Am 15.02.2017 um 20:27 schrieb Kevin Hilman:
>> Heiner Kallweit <hkallweit1@gmail.com> writes:
>> 
>>> I don't think this clk_disable_unprepare is right here. We do this
>>> in meson_mmc_remove what should be sufficient.
>> 
>> I don't think so, because if _clk_init() fails, then _probe will fail,
>> and the driver's ->remove() will not be called.
>> 
> In general, then the condition should be "if (ret)", not "if (!ret)".
> And if _clk_init() fails, then probe jumps to label free_host,
> where we have exactly this clk_disable_unprepare(host->cfg_div_clk)

Then you need to update the changelog, which refers to remove, not the
cleanup in probe.

Kevin

>> Kevin
>> 
>>> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
>>> ---
>>>  drivers/mmc/host/meson-gx-mmc.c | 6 +-----
>>>  1 file changed, 1 insertion(+), 5 deletions(-)
>>>
>>> diff --git a/drivers/mmc/host/meson-gx-mmc.c b/drivers/mmc/host/meson-gx-mmc.c
>>> index 74ceda75..39904fb5 100644
>>> --- a/drivers/mmc/host/meson-gx-mmc.c
>>> +++ b/drivers/mmc/host/meson-gx-mmc.c
>>> @@ -315,11 +315,7 @@ static int meson_mmc_clk_init(struct meson_host *host)
>>>  	/* Get the nearest minimum clock to 400KHz */
>>>  	host->mmc->f_min = clk_round_rate(host->cfg_div_clk, 400000);
>>>  
>>> -	ret = meson_mmc_clk_set(host, host->mmc->f_min);
>>> -	if (!ret)
>>> -		clk_disable_unprepare(host->cfg_div_clk);
>>> -
>>> -	return ret;
>>> +	return meson_mmc_clk_set(host, host->mmc->f_min);
>>>  }
>>>  
>>>  static void meson_mmc_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
>> 

Re: [PATCH 04/18] mmc: meson-gx: improve meson_mmc_start_cmd

[Kevin Hilman]

Heiner Kallweit <hkallweit1@gmail.com> writes:

> Am 15.02.2017 um 20:43 schrieb Heiner Kallweit:
>> Am 15.02.2017 um 18:04 schrieb Kevin Hilman:
>>> Heiner Kallweit <hkallweit1@gmail.com> writes:
>>>
>>>> Remove use of unneeded members cmd_arg and cmd_resp.
>>>> Setting SD_EMMC_CMD_RSP is only needed if CMD_CFG_RESP_NUM is set,
>>>> so don't write this register in all other cases.
>>>>
>>>> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
>>>
>>> I'm not sure I like this change.  This works now because there is only
>>> one descriptor used, but one of the next things to work on in this
>>> driver is taking advantage of the internal DMA capabilities, which means
>>> having a chain of descriptorsall filled out in memory.
>>>
>> For testing purposes I temporarily changed the driver from passing the
>> descriptor in registers to passing the descriptor via DMA and it worked.
>> 
>> I'm not very familiar (yet) with descriptor chains and have to check
>> the MMC core code a little bit more ..
>> If we can actually benefit from it then I'd agree with you.
>> 
> After having had a little bit closer look at the MMC core and descriptor
> chains in general:
> I'm not really sure where descriptor chains would help us. SG lists in
> read / write commands are linearized by the driver via
> sg_copy_[to,from]_buffer already.

Copying to the bounce buffer would be replaced by creating a descriptor
for each SG entry, and creating a descriptor chain.

> Beyond that I don't see any command chaining support in the core
> (prerequisite for command chaining most likely would be that a
> subsequent command must not depend on the response of a previous one).
>
> Maybe somebody with more knowledge about the MMC core and MMC in general
> can shed some light on this ..

I don't know if it's a feature of the core, but the Amlogic vendor
driver does the chained descriptor handling in the driver itself.

Kevin

>>> Kevin
>>>
>>>> ---
>>>>  drivers/mmc/host/meson-gx-mmc.c | 6 ++----
>>>>  1 file changed, 2 insertions(+), 4 deletions(-)
>>>>
>>>> diff --git a/drivers/mmc/host/meson-gx-mmc.c b/drivers/mmc/host/meson-gx-mmc.c
>>>> index ece38b44..630e0590 100644
>>>> --- a/drivers/mmc/host/meson-gx-mmc.c
>>>> +++ b/drivers/mmc/host/meson-gx-mmc.c
>>>> @@ -456,7 +456,6 @@ static void meson_mmc_start_cmd(struct mmc_host *mmc, struct mmc_command *cmd)
>>>>  	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;
>>>>  
>>>>  	/* Response */
>>>>  	if (cmd->flags & MMC_RSP_PRESENT) {
>>>> @@ -464,7 +463,7 @@ static void meson_mmc_start_cmd(struct mmc_host *mmc, struct mmc_command *cmd)
>>>>  		if (cmd->flags & MMC_RSP_136)
>>>>  			desc->cmd_cfg |= CMD_CFG_RESP_128;
>>>>  		desc->cmd_cfg |= CMD_CFG_RESP_NUM;
>>>> -		desc->cmd_resp = 0;
>>>> +		writel(0, host->regs + SD_EMMC_CMD_RSP);
>>>>  
>>>>  		if (!(cmd->flags & MMC_RSP_CRC))
>>>>  			desc->cmd_cfg |= CMD_CFG_RESP_NOCRC;
>>>> @@ -540,9 +539,8 @@ static void meson_mmc_start_cmd(struct mmc_host *mmc, struct mmc_command *cmd)
>>>>  	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);
>>>>  	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)
>>>
>> 

[PATCH 04/18] mmc: meson-gx: improve meson_mmc_start_cmd

[Kevin Hilman]

Heiner Kallweit <hkallweit1@gmail.com> writes:

> Am 15.02.2017 um 20:43 schrieb Heiner Kallweit:
>> Am 15.02.2017 um 18:04 schrieb Kevin Hilman:
>>> Heiner Kallweit <hkallweit1@gmail.com> writes:
>>>
>>>> Remove use of unneeded members cmd_arg and cmd_resp.
>>>> Setting SD_EMMC_CMD_RSP is only needed if CMD_CFG_RESP_NUM is set,
>>>> so don't write this register in all other cases.
>>>>
>>>> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
>>>
>>> I'm not sure I like this change.  This works now because there is only
>>> one descriptor used, but one of the next things to work on in this
>>> driver is taking advantage of the internal DMA capabilities, which means
>>> having a chain of descriptorsall filled out in memory.
>>>
>> For testing purposes I temporarily changed the driver from passing the
>> descriptor in registers to passing the descriptor via DMA and it worked.
>> 
>> I'm not very familiar (yet) with descriptor chains and have to check
>> the MMC core code a little bit more ..
>> If we can actually benefit from it then I'd agree with you.
>> 
> After having had a little bit closer look at the MMC core and descriptor
> chains in general:
> I'm not really sure where descriptor chains would help us. SG lists in
> read / write commands are linearized by the driver via
> sg_copy_[to,from]_buffer already.

Copying to the bounce buffer would be replaced by creating a descriptor
for each SG entry, and creating a descriptor chain.

> Beyond that I don't see any command chaining support in the core
> (prerequisite for command chaining most likely would be that a
> subsequent command must not depend on the response of a previous one).
>
> Maybe somebody with more knowledge about the MMC core and MMC in general
> can shed some light on this ..

I don't know if it's a feature of the core, but the Amlogic vendor
driver does the chained descriptor handling in the driver itself.

Kevin

>>> Kevin
>>>
>>>> ---
>>>>  drivers/mmc/host/meson-gx-mmc.c | 6 ++----
>>>>  1 file changed, 2 insertions(+), 4 deletions(-)
>>>>
>>>> diff --git a/drivers/mmc/host/meson-gx-mmc.c b/drivers/mmc/host/meson-gx-mmc.c
>>>> index ece38b44..630e0590 100644
>>>> --- a/drivers/mmc/host/meson-gx-mmc.c
>>>> +++ b/drivers/mmc/host/meson-gx-mmc.c
>>>> @@ -456,7 +456,6 @@ static void meson_mmc_start_cmd(struct mmc_host *mmc, struct mmc_command *cmd)
>>>>  	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;
>>>>  
>>>>  	/* Response */
>>>>  	if (cmd->flags & MMC_RSP_PRESENT) {
>>>> @@ -464,7 +463,7 @@ static void meson_mmc_start_cmd(struct mmc_host *mmc, struct mmc_command *cmd)
>>>>  		if (cmd->flags & MMC_RSP_136)
>>>>  			desc->cmd_cfg |= CMD_CFG_RESP_128;
>>>>  		desc->cmd_cfg |= CMD_CFG_RESP_NUM;
>>>> -		desc->cmd_resp = 0;
>>>> +		writel(0, host->regs + SD_EMMC_CMD_RSP);
>>>>  
>>>>  		if (!(cmd->flags & MMC_RSP_CRC))
>>>>  			desc->cmd_cfg |= CMD_CFG_RESP_NOCRC;
>>>> @@ -540,9 +539,8 @@ static void meson_mmc_start_cmd(struct mmc_host *mmc, struct mmc_command *cmd)
>>>>  	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);
>>>>  	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)
>>>
>> 

Re: [PATCH 02/18] mmc: meson-gx: remove code for unsupported CMD23

[Heiner Kallweit]

Am 15.02.2017 um 17:54 schrieb Kevin Hilman:
> Heiner Kallweit <hkallweit1@gmail.com> writes:
> 
>> CMD23 isn't supported on meson-gx and therefore the "set block count"
>> command isn't used. So remove this dead code.
> 
> I'm admittedly a bit unfamiliar with the MMC spec and wrote this driver
> mostly by looking at the vendor driver, so forgive the silly
> questions...
> 
> Why isn't CMD23 supported, and should we support instead of deleting
> this support?
> 
As far as I understand this needs to be supported by the host controller.
And setting the respective host flag to enable CMD23 mode lead to a
non-working driver when testing on my system.


> Kevin
> 
>> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
>> ---
>>  drivers/mmc/host/meson-gx-mmc.c | 8 ++------
>>  1 file changed, 2 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/mmc/host/meson-gx-mmc.c b/drivers/mmc/host/meson-gx-mmc.c
>> index 1ad66f84..e89bdf5f 100644
>> --- a/drivers/mmc/host/meson-gx-mmc.c
>> +++ b/drivers/mmc/host/meson-gx-mmc.c
>> @@ -555,11 +555,7 @@ static void meson_mmc_request(struct mmc_host *mmc, struct mmc_request *mrq)
>>  	writel(0, host->regs + SD_EMMC_START);
>>  
>>  	host->mrq = mrq;
>> -
>> -	if (mrq->sbc)
>> -		meson_mmc_start_cmd(mmc, mrq->sbc);
>> -	else
>> -		meson_mmc_start_cmd(mmc, mrq->cmd);
>> +	meson_mmc_start_cmd(mmc, mrq->cmd);
>>  }
>>  
>>  static int meson_mmc_read_resp(struct mmc_host *mmc, struct mmc_command *cmd)
>> @@ -690,7 +686,7 @@ static irqreturn_t meson_mmc_irq_thread(int irq, void *dev_id)
>>  	}
>>  
>>  	meson_mmc_read_resp(host->mmc, cmd);
>> -	if (!data || !data->stop || mrq->sbc)
>> +	if (!data || !data->stop)
>>  		meson_mmc_request_done(host->mmc, mrq);
>>  	else
>>  		meson_mmc_start_cmd(host->mmc, data->stop);
> 

[PATCH 02/18] mmc: meson-gx: remove code for unsupported CMD23

[Heiner Kallweit]

Am 15.02.2017 um 17:54 schrieb Kevin Hilman:
> Heiner Kallweit <hkallweit1@gmail.com> writes:
> 
>> CMD23 isn't supported on meson-gx and therefore the "set block count"
>> command isn't used. So remove this dead code.
> 
> I'm admittedly a bit unfamiliar with the MMC spec and wrote this driver
> mostly by looking at the vendor driver, so forgive the silly
> questions...
> 
> Why isn't CMD23 supported, and should we support instead of deleting
> this support?
> 
As far as I understand this needs to be supported by the host controller.
And setting the respective host flag to enable CMD23 mode lead to a
non-working driver when testing on my system.


> Kevin
> 
>> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
>> ---
>>  drivers/mmc/host/meson-gx-mmc.c | 8 ++------
>>  1 file changed, 2 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/mmc/host/meson-gx-mmc.c b/drivers/mmc/host/meson-gx-mmc.c
>> index 1ad66f84..e89bdf5f 100644
>> --- a/drivers/mmc/host/meson-gx-mmc.c
>> +++ b/drivers/mmc/host/meson-gx-mmc.c
>> @@ -555,11 +555,7 @@ static void meson_mmc_request(struct mmc_host *mmc, struct mmc_request *mrq)
>>  	writel(0, host->regs + SD_EMMC_START);
>>  
>>  	host->mrq = mrq;
>> -
>> -	if (mrq->sbc)
>> -		meson_mmc_start_cmd(mmc, mrq->sbc);
>> -	else
>> -		meson_mmc_start_cmd(mmc, mrq->cmd);
>> +	meson_mmc_start_cmd(mmc, mrq->cmd);
>>  }
>>  
>>  static int meson_mmc_read_resp(struct mmc_host *mmc, struct mmc_command *cmd)
>> @@ -690,7 +686,7 @@ static irqreturn_t meson_mmc_irq_thread(int irq, void *dev_id)
>>  	}
>>  
>>  	meson_mmc_read_resp(host->mmc, cmd);
>> -	if (!data || !data->stop || mrq->sbc)
>> +	if (!data || !data->stop)
>>  		meson_mmc_request_done(host->mmc, mrq);
>>  	else
>>  		meson_mmc_start_cmd(host->mmc, data->stop);
> 

Re: [PATCH 13/18] mmc: meson-gx: remove unneeded meson_mmc_clk_set in meson_mmc_clk_init

[Heiner Kallweit]

Am 16.02.2017 um 00:56 schrieb Kevin Hilman:
> Heiner Kallweit <hkallweit1@gmail.com> writes:
> 
>> Am 15.02.2017 um 20:27 schrieb Kevin Hilman:
>>> Heiner Kallweit <hkallweit1@gmail.com> writes:
>>>
>>>> I don't think this clk_disable_unprepare is right here. We do this
>>>> in meson_mmc_remove what should be sufficient.
>>>
>>> I don't think so, because if _clk_init() fails, then _probe will fail,
>>> and the driver's ->remove() will not be called.
>>>
>> In general, then the condition should be "if (ret)", not "if (!ret)".
>> And if _clk_init() fails, then probe jumps to label free_host,
>> where we have exactly this clk_disable_unprepare(host->cfg_div_clk)
> 
> Then you need to update the changelog, which refers to remove, not the
> cleanup in probe.
> 
Sure ..

> Kevin
> 
>>> Kevin
>>>
>>>> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
>>>> ---
>>>>  drivers/mmc/host/meson-gx-mmc.c | 6 +-----
>>>>  1 file changed, 1 insertion(+), 5 deletions(-)
>>>>
>>>> diff --git a/drivers/mmc/host/meson-gx-mmc.c b/drivers/mmc/host/meson-gx-mmc.c
>>>> index 74ceda75..39904fb5 100644
>>>> --- a/drivers/mmc/host/meson-gx-mmc.c
>>>> +++ b/drivers/mmc/host/meson-gx-mmc.c
>>>> @@ -315,11 +315,7 @@ static int meson_mmc_clk_init(struct meson_host *host)
>>>>  	/* Get the nearest minimum clock to 400KHz */
>>>>  	host->mmc->f_min = clk_round_rate(host->cfg_div_clk, 400000);
>>>>  
>>>> -	ret = meson_mmc_clk_set(host, host->mmc->f_min);
>>>> -	if (!ret)
>>>> -		clk_disable_unprepare(host->cfg_div_clk);
>>>> -
>>>> -	return ret;
>>>> +	return meson_mmc_clk_set(host, host->mmc->f_min);
>>>>  }
>>>>  
>>>>  static void meson_mmc_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
>>>
> 

[PATCH 13/18] mmc: meson-gx: remove unneeded meson_mmc_clk_set in meson_mmc_clk_init

[Heiner Kallweit]

Am 16.02.2017 um 00:56 schrieb Kevin Hilman:
> Heiner Kallweit <hkallweit1@gmail.com> writes:
> 
>> Am 15.02.2017 um 20:27 schrieb Kevin Hilman:
>>> Heiner Kallweit <hkallweit1@gmail.com> writes:
>>>
>>>> I don't think this clk_disable_unprepare is right here. We do this
>>>> in meson_mmc_remove what should be sufficient.
>>>
>>> I don't think so, because if _clk_init() fails, then _probe will fail,
>>> and the driver's ->remove() will not be called.
>>>
>> In general, then the condition should be "if (ret)", not "if (!ret)".
>> And if _clk_init() fails, then probe jumps to label free_host,
>> where we have exactly this clk_disable_unprepare(host->cfg_div_clk)
> 
> Then you need to update the changelog, which refers to remove, not the
> cleanup in probe.
> 
Sure ..

> Kevin
> 
>>> Kevin
>>>
>>>> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
>>>> ---
>>>>  drivers/mmc/host/meson-gx-mmc.c | 6 +-----
>>>>  1 file changed, 1 insertion(+), 5 deletions(-)
>>>>
>>>> diff --git a/drivers/mmc/host/meson-gx-mmc.c b/drivers/mmc/host/meson-gx-mmc.c
>>>> index 74ceda75..39904fb5 100644
>>>> --- a/drivers/mmc/host/meson-gx-mmc.c
>>>> +++ b/drivers/mmc/host/meson-gx-mmc.c
>>>> @@ -315,11 +315,7 @@ static int meson_mmc_clk_init(struct meson_host *host)
>>>>  	/* Get the nearest minimum clock to 400KHz */
>>>>  	host->mmc->f_min = clk_round_rate(host->cfg_div_clk, 400000);
>>>>  
>>>> -	ret = meson_mmc_clk_set(host, host->mmc->f_min);
>>>> -	if (!ret)
>>>> -		clk_disable_unprepare(host->cfg_div_clk);
>>>> -
>>>> -	return ret;
>>>> +	return meson_mmc_clk_set(host, host->mmc->f_min);
>>>>  }
>>>>  
>>>>  static void meson_mmc_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
>>>
> 

Re: [PATCH 02/18] mmc: meson-gx: remove code for unsupported CMD23

[Ulf Hansson]

On 16 February 2017 at 08:03, Heiner Kallweit <hkallweit1@gmail.com> wrote:
> Am 15.02.2017 um 17:54 schrieb Kevin Hilman:
>> Heiner Kallweit <hkallweit1@gmail.com> writes:
>>
>>> CMD23 isn't supported on meson-gx and therefore the "set block count"
>>> command isn't used. So remove this dead code.
>>
>> I'm admittedly a bit unfamiliar with the MMC spec and wrote this driver
>> mostly by looking at the vendor driver, so forgive the silly
>> questions...
>>
>> Why isn't CMD23 supported, and should we support instead of deleting
>> this support?
>>
> As far as I understand this needs to be supported by the host controller.
> And setting the respective host flag to enable CMD23 mode lead to a
> non-working driver when testing on my system.

CMD23 can be supported by the host *driver*, you don't need HW-support
to implement this.

You may for example be inspired by the mmci.c driver to understand how
to implement this. Looks for "mrq->sbc".

[...]

Kind regards
Uffe

[PATCH 02/18] mmc: meson-gx: remove code for unsupported CMD23

[Ulf Hansson]

On 16 February 2017 at 08:03, Heiner Kallweit <hkallweit1@gmail.com> wrote:
> Am 15.02.2017 um 17:54 schrieb Kevin Hilman:
>> Heiner Kallweit <hkallweit1@gmail.com> writes:
>>
>>> CMD23 isn't supported on meson-gx and therefore the "set block count"
>>> command isn't used. So remove this dead code.
>>
>> I'm admittedly a bit unfamiliar with the MMC spec and wrote this driver
>> mostly by looking at the vendor driver, so forgive the silly
>> questions...
>>
>> Why isn't CMD23 supported, and should we support instead of deleting
>> this support?
>>
> As far as I understand this needs to be supported by the host controller.
> And setting the respective host flag to enable CMD23 mode lead to a
> non-working driver when testing on my system.

CMD23 can be supported by the host *driver*, you don't need HW-support
to implement this.

You may for example be inspired by the mmci.c driver to understand how
to implement this. Looks for "mrq->sbc".

[...]

Kind regards
Uffe

Re: [PATCH 02/18] mmc: meson-gx: remove code for unsupported CMD23

[Heiner Kallweit]

Am 16.02.2017 um 09:14 schrieb Ulf Hansson:
> On 16 February 2017 at 08:03, Heiner Kallweit <hkallweit1@gmail.com> wrote:
>> Am 15.02.2017 um 17:54 schrieb Kevin Hilman:
>>> Heiner Kallweit <hkallweit1@gmail.com> writes:
>>>
>>>> CMD23 isn't supported on meson-gx and therefore the "set block count"
>>>> command isn't used. So remove this dead code.
>>>
>>> I'm admittedly a bit unfamiliar with the MMC spec and wrote this driver
>>> mostly by looking at the vendor driver, so forgive the silly
>>> questions...
>>>
>>> Why isn't CMD23 supported, and should we support instead of deleting
>>> this support?
>>>
>> As far as I understand this needs to be supported by the host controller.
>> And setting the respective host flag to enable CMD23 mode lead to a
>> non-working driver when testing on my system.
> 
> CMD23 can be supported by the host *driver*, you don't need HW-support
> to implement this.
> 
Thanks for the explanation. I think I got confused by this comment in
core/block.c: "If CMD23 is supported by card and host, [..]"
I interpreted this as "host controller needs to support this feature".

> You may for example be inspired by the mmci.c driver to understand how
> to implement this. Looks for "mrq->sbc".
> 
> [...]
> 
> Kind regards
> Uffe
> 

[PATCH 02/18] mmc: meson-gx: remove code for unsupported CMD23

[Heiner Kallweit]

Am 16.02.2017 um 09:14 schrieb Ulf Hansson:
> On 16 February 2017 at 08:03, Heiner Kallweit <hkallweit1@gmail.com> wrote:
>> Am 15.02.2017 um 17:54 schrieb Kevin Hilman:
>>> Heiner Kallweit <hkallweit1@gmail.com> writes:
>>>
>>>> CMD23 isn't supported on meson-gx and therefore the "set block count"
>>>> command isn't used. So remove this dead code.
>>>
>>> I'm admittedly a bit unfamiliar with the MMC spec and wrote this driver
>>> mostly by looking at the vendor driver, so forgive the silly
>>> questions...
>>>
>>> Why isn't CMD23 supported, and should we support instead of deleting
>>> this support?
>>>
>> As far as I understand this needs to be supported by the host controller.
>> And setting the respective host flag to enable CMD23 mode lead to a
>> non-working driver when testing on my system.
> 
> CMD23 can be supported by the host *driver*, you don't need HW-support
> to implement this.
> 
Thanks for the explanation. I think I got confused by this comment in
core/block.c: "If CMD23 is supported by card and host, [..]"
I interpreted this as "host controller needs to support this feature".

> You may for example be inspired by the mmci.c driver to understand how
> to implement this. Looks for "mrq->sbc".
> 
> [...]
> 
> Kind regards
> Uffe
> 

Re: [PATCH 02/18] mmc: meson-gx: remove code for unsupported CMD23

[Heiner Kallweit]

Am 15.02.2017 um 17:54 schrieb Kevin Hilman:
> Heiner Kallweit <hkallweit1@gmail.com> writes:
> 
>> CMD23 isn't supported on meson-gx and therefore the "set block count"
>> command isn't used. So remove this dead code.
> 
> I'm admittedly a bit unfamiliar with the MMC spec and wrote this driver
> mostly by looking at the vendor driver, so forgive the silly
> questions...
> 
> Why isn't CMD23 supported, and should we support instead of deleting
> this support?
> 
After clarification by Ulf that CMD23 is not a host feature but a host
driver feature I added this feature to the driver.
I will submit it once the smaller refactorings are applied.

Heiner

> Kevin
> 
>> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
>> ---
>>  drivers/mmc/host/meson-gx-mmc.c | 8 ++------
>>  1 file changed, 2 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/mmc/host/meson-gx-mmc.c b/drivers/mmc/host/meson-gx-mmc.c
>> index 1ad66f84..e89bdf5f 100644
>> --- a/drivers/mmc/host/meson-gx-mmc.c
>> +++ b/drivers/mmc/host/meson-gx-mmc.c
>> @@ -555,11 +555,7 @@ static void meson_mmc_request(struct mmc_host *mmc, struct mmc_request *mrq)
>>  	writel(0, host->regs + SD_EMMC_START);
>>  
>>  	host->mrq = mrq;
>> -
>> -	if (mrq->sbc)
>> -		meson_mmc_start_cmd(mmc, mrq->sbc);
>> -	else
>> -		meson_mmc_start_cmd(mmc, mrq->cmd);
>> +	meson_mmc_start_cmd(mmc, mrq->cmd);
>>  }
>>  
>>  static int meson_mmc_read_resp(struct mmc_host *mmc, struct mmc_command *cmd)
>> @@ -690,7 +686,7 @@ static irqreturn_t meson_mmc_irq_thread(int irq, void *dev_id)
>>  	}
>>  
>>  	meson_mmc_read_resp(host->mmc, cmd);
>> -	if (!data || !data->stop || mrq->sbc)
>> +	if (!data || !data->stop)
>>  		meson_mmc_request_done(host->mmc, mrq);
>>  	else
>>  		meson_mmc_start_cmd(host->mmc, data->stop);
> 

[PATCH 02/18] mmc: meson-gx: remove code for unsupported CMD23

[Heiner Kallweit]

Am 15.02.2017 um 17:54 schrieb Kevin Hilman:
> Heiner Kallweit <hkallweit1@gmail.com> writes:
> 
>> CMD23 isn't supported on meson-gx and therefore the "set block count"
>> command isn't used. So remove this dead code.
> 
> I'm admittedly a bit unfamiliar with the MMC spec and wrote this driver
> mostly by looking at the vendor driver, so forgive the silly
> questions...
> 
> Why isn't CMD23 supported, and should we support instead of deleting
> this support?
> 
After clarification by Ulf that CMD23 is not a host feature but a host
driver feature I added this feature to the driver.
I will submit it once the smaller refactorings are applied.

Heiner

> Kevin
> 
>> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
>> ---
>>  drivers/mmc/host/meson-gx-mmc.c | 8 ++------
>>  1 file changed, 2 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/mmc/host/meson-gx-mmc.c b/drivers/mmc/host/meson-gx-mmc.c
>> index 1ad66f84..e89bdf5f 100644
>> --- a/drivers/mmc/host/meson-gx-mmc.c
>> +++ b/drivers/mmc/host/meson-gx-mmc.c
>> @@ -555,11 +555,7 @@ static void meson_mmc_request(struct mmc_host *mmc, struct mmc_request *mrq)
>>  	writel(0, host->regs + SD_EMMC_START);
>>  
>>  	host->mrq = mrq;
>> -
>> -	if (mrq->sbc)
>> -		meson_mmc_start_cmd(mmc, mrq->sbc);
>> -	else
>> -		meson_mmc_start_cmd(mmc, mrq->cmd);
>> +	meson_mmc_start_cmd(mmc, mrq->cmd);
>>  }
>>  
>>  static int meson_mmc_read_resp(struct mmc_host *mmc, struct mmc_command *cmd)
>> @@ -690,7 +686,7 @@ static irqreturn_t meson_mmc_irq_thread(int irq, void *dev_id)
>>  	}
>>  
>>  	meson_mmc_read_resp(host->mmc, cmd);
>> -	if (!data || !data->stop || mrq->sbc)
>> +	if (!data || !data->stop)
>>  		meson_mmc_request_done(host->mmc, mrq);
>>  	else
>>  		meson_mmc_start_cmd(host->mmc, data->stop);
> 

Re: [PATCH 04/18] mmc: meson-gx: improve meson_mmc_start_cmd

[Heiner Kallweit]

Am 15.02.2017 um 18:04 schrieb Kevin Hilman:
> Heiner Kallweit <hkallweit1@gmail.com> writes:
> 
>> Remove use of unneeded members cmd_arg and cmd_resp.
>> Setting SD_EMMC_CMD_RSP is only needed if CMD_CFG_RESP_NUM is set,
>> so don't write this register in all other cases.
>>
>> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
> 
> I'm not sure I like this change.  This works now because there is only
> one descriptor used, but one of the next things to work on in this
> driver is taking advantage of the internal DMA capabilities, which means
> having a chain of descriptorsall filled out in memory.
> 
I implemented the descriptor chain mode and tests (together with CMD23
mode) resulted in 140 MB/s read performance (from a 128GB eMMC card).
So I will submit this extension together with the CMD23 mode extension.

Heiner

> Kevin
> 
>> ---
>>  drivers/mmc/host/meson-gx-mmc.c | 6 ++----
>>  1 file changed, 2 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/mmc/host/meson-gx-mmc.c b/drivers/mmc/host/meson-gx-mmc.c
>> index ece38b44..630e0590 100644
>> --- a/drivers/mmc/host/meson-gx-mmc.c
>> +++ b/drivers/mmc/host/meson-gx-mmc.c
>> @@ -456,7 +456,6 @@ static void meson_mmc_start_cmd(struct mmc_host *mmc, struct mmc_command *cmd)
>>  	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;
>>  
>>  	/* Response */
>>  	if (cmd->flags & MMC_RSP_PRESENT) {
>> @@ -464,7 +463,7 @@ static void meson_mmc_start_cmd(struct mmc_host *mmc, struct mmc_command *cmd)
>>  		if (cmd->flags & MMC_RSP_136)
>>  			desc->cmd_cfg |= CMD_CFG_RESP_128;
>>  		desc->cmd_cfg |= CMD_CFG_RESP_NUM;
>> -		desc->cmd_resp = 0;
>> +		writel(0, host->regs + SD_EMMC_CMD_RSP);
>>  
>>  		if (!(cmd->flags & MMC_RSP_CRC))
>>  			desc->cmd_cfg |= CMD_CFG_RESP_NOCRC;
>> @@ -540,9 +539,8 @@ static void meson_mmc_start_cmd(struct mmc_host *mmc, struct mmc_command *cmd)
>>  	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);
>>  	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)
> 

[PATCH 04/18] mmc: meson-gx: improve meson_mmc_start_cmd

[Heiner Kallweit]

Am 15.02.2017 um 18:04 schrieb Kevin Hilman:
> Heiner Kallweit <hkallweit1@gmail.com> writes:
> 
>> Remove use of unneeded members cmd_arg and cmd_resp.
>> Setting SD_EMMC_CMD_RSP is only needed if CMD_CFG_RESP_NUM is set,
>> so don't write this register in all other cases.
>>
>> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
> 
> I'm not sure I like this change.  This works now because there is only
> one descriptor used, but one of the next things to work on in this
> driver is taking advantage of the internal DMA capabilities, which means
> having a chain of descriptorsall filled out in memory.
> 
I implemented the descriptor chain mode and tests (together with CMD23
mode) resulted in 140 MB/s read performance (from a 128GB eMMC card).
So I will submit this extension together with the CMD23 mode extension.

Heiner

> Kevin
> 
>> ---
>>  drivers/mmc/host/meson-gx-mmc.c | 6 ++----
>>  1 file changed, 2 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/mmc/host/meson-gx-mmc.c b/drivers/mmc/host/meson-gx-mmc.c
>> index ece38b44..630e0590 100644
>> --- a/drivers/mmc/host/meson-gx-mmc.c
>> +++ b/drivers/mmc/host/meson-gx-mmc.c
>> @@ -456,7 +456,6 @@ static void meson_mmc_start_cmd(struct mmc_host *mmc, struct mmc_command *cmd)
>>  	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;
>>  
>>  	/* Response */
>>  	if (cmd->flags & MMC_RSP_PRESENT) {
>> @@ -464,7 +463,7 @@ static void meson_mmc_start_cmd(struct mmc_host *mmc, struct mmc_command *cmd)
>>  		if (cmd->flags & MMC_RSP_136)
>>  			desc->cmd_cfg |= CMD_CFG_RESP_128;
>>  		desc->cmd_cfg |= CMD_CFG_RESP_NUM;
>> -		desc->cmd_resp = 0;
>> +		writel(0, host->regs + SD_EMMC_CMD_RSP);
>>  
>>  		if (!(cmd->flags & MMC_RSP_CRC))
>>  			desc->cmd_cfg |= CMD_CFG_RESP_NOCRC;
>> @@ -540,9 +539,8 @@ static void meson_mmc_start_cmd(struct mmc_host *mmc, struct mmc_command *cmd)
>>  	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);
>>  	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)
>