[PATCH v3 1/10] mmc: meson-gx: simplify bounce buffer setting in meson_mmc_start_cmd

[PATCH v3 1/10] mmc: meson-gx: simplify bounce buffer setting in meson_mmc_start_cmd

[Heiner Kallweit]

Core ensures that there are no commands with cmd->data being set and
nothing to transfer. 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>
Acked-by: Kevin Hilman <khilman@baylibre.com>
---
v2:
- adjusted commit message because branch xfer_bytes == 0 is never reached
- added acked-by
v3:
- no changes
---
 drivers/mmc/host/meson-gx-mmc.c | 9 +--------
 1 file changed, 1 insertion(+), 8 deletions(-)

diff --git a/drivers/mmc/host/meson-gx-mmc.c b/drivers/mmc/host/meson-gx-mmc.c
index 5a959783..07a7399c 100644
--- a/drivers/mmc/host/meson-gx-mmc.c
+++ b/drivers/mmc/host/meson-gx-mmc.c
@@ -516,14 +516,7 @@ static void meson_mmc_start_cmd(struct mmc_host *mmc, struct mmc_command *cmd)
 			desc->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;
-		} else {
-			/* write data to data_addr */
-			desc->cmd_cfg |= CMD_CFG_DATA_NUM;
-			desc->cmd_data = 0;
-		}
+		desc->cmd_data = host->bounce_dma_addr & CMD_DATA_MASK;
 
 		cmd_cfg_timeout = 12;
 	} else {
-- 
2.11.1

[PATCH v3 1/10] mmc: meson-gx: simplify bounce buffer setting in meson_mmc_start_cmd

[Heiner Kallweit]

Core ensures that there are no commands with cmd->data being set and
nothing to transfer. 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>
Acked-by: Kevin Hilman <khilman@baylibre.com>
---
v2:
- adjusted commit message because branch xfer_bytes == 0 is never reached
- added acked-by
v3:
- no changes
---
 drivers/mmc/host/meson-gx-mmc.c | 9 +--------
 1 file changed, 1 insertion(+), 8 deletions(-)

diff --git a/drivers/mmc/host/meson-gx-mmc.c b/drivers/mmc/host/meson-gx-mmc.c
index 5a959783..07a7399c 100644
--- a/drivers/mmc/host/meson-gx-mmc.c
+++ b/drivers/mmc/host/meson-gx-mmc.c
@@ -516,14 +516,7 @@ static void meson_mmc_start_cmd(struct mmc_host *mmc, struct mmc_command *cmd)
 			desc->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;
-		} else {
-			/* write data to data_addr */
-			desc->cmd_cfg |= CMD_CFG_DATA_NUM;
-			desc->cmd_data = 0;
-		}
+		desc->cmd_data = host->bounce_dma_addr & CMD_DATA_MASK;
 
 		cmd_cfg_timeout = 12;
 	} else {
-- 
2.11.1

[PATCH v3 2/10] 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>
Acked-by: Kevin Hilman <khilman@baylibre.com>
---
v2:
- added acked-by
v3:
- no changes
---
 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 07a7399c..b99461ff 100644
--- a/drivers/mmc/host/meson-gx-mmc.c
+++ b/drivers/mmc/host/meson-gx-mmc.c
@@ -426,7 +426,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);
 
@@ -435,8 +436,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)
@@ -554,7 +553,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);
 
@@ -566,8 +565,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 v3 2/10] 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>
Acked-by: Kevin Hilman <khilman@baylibre.com>
---
v2:
- added acked-by
v3:
- no changes
---
 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 07a7399c..b99461ff 100644
--- a/drivers/mmc/host/meson-gx-mmc.c
+++ b/drivers/mmc/host/meson-gx-mmc.c
@@ -426,7 +426,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);
 
@@ -435,8 +436,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)
@@ -554,7 +553,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);
 
@@ -566,8 +565,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 v3 3/10] 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>
Acked-by: Kevin Hilman <khilman@baylibre.com>
---
v2:
- added acked-by
v3:
- no changes
---
 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 b99461ff..b9de3d47 100644
--- a/drivers/mmc/host/meson-gx-mmc.c
+++ b/drivers/mmc/host/meson-gx-mmc.c
@@ -127,8 +127,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;
@@ -712,7 +710,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)
@@ -744,8 +742,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;
@@ -773,9 +771,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, irq, meson_mmc_irq,
+					meson_mmc_irq_thread, IRQF_SHARED,
+					DRIVER_NAME, host);
 	if (ret)
 		goto free_host;
 
-- 
2.11.1

[PATCH v3 3/10] 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>
Acked-by: Kevin Hilman <khilman@baylibre.com>
---
v2:
- added acked-by
v3:
- no changes
---
 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 b99461ff..b9de3d47 100644
--- a/drivers/mmc/host/meson-gx-mmc.c
+++ b/drivers/mmc/host/meson-gx-mmc.c
@@ -127,8 +127,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;
@@ -712,7 +710,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)
@@ -744,8 +742,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;
@@ -773,9 +771,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, irq, meson_mmc_irq,
+					meson_mmc_irq_thread, IRQF_SHARED,
+					DRIVER_NAME, host);
 	if (ret)
 		goto free_host;
 
-- 
2.11.1

[PATCH v3 4/10] mmc: meson-gx: remove unneeded variable in meson_mmc_clk_init

[Heiner Kallweit]

Because the DT requires a fixed number of mux parent clocks,
variable mux_parent_count can be replaced with constant
MUX_CLK_NUM_PARENTS, so remove it.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
Acked-by: Kevin Hilman <khilman@baylibre.com>
---
v2:
- adjusted commit message
- added acked-by
v3:
- no changes
---
 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 b9de3d47..ea2db3ce 100644
--- a/drivers/mmc/host/meson-gx-mmc.c
+++ b/drivers/mmc/host/meson-gx-mmc.c
@@ -242,7 +242,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;
 
@@ -261,7 +260,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 */
@@ -270,7 +268,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 v3 4/10] mmc: meson-gx: remove unneeded variable in meson_mmc_clk_init

[Heiner Kallweit]

Because the DT requires a fixed number of mux parent clocks,
variable mux_parent_count can be replaced with constant
MUX_CLK_NUM_PARENTS, so remove it.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
Acked-by: Kevin Hilman <khilman@baylibre.com>
---
v2:
- adjusted commit message
- added acked-by
v3:
- no changes
---
 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 b9de3d47..ea2db3ce 100644
--- a/drivers/mmc/host/meson-gx-mmc.c
+++ b/drivers/mmc/host/meson-gx-mmc.c
@@ -242,7 +242,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;
 
@@ -261,7 +260,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 */
@@ -270,7 +268,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 v3 5/10] 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>
Acked-by: Kevin Hilman <khilman@baylibre.com>
---
v2:
- added acked-by
v3:
- no changes
---
 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 ea2db3ce..68e76fa8 100644
--- a/drivers/mmc/host/meson-gx-mmc.c
+++ b/drivers/mmc/host/meson-gx-mmc.c
@@ -130,7 +130,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;
@@ -247,19 +246,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 v3 5/10] 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>
Acked-by: Kevin Hilman <khilman@baylibre.com>
---
v2:
- added acked-by
v3:
- no changes
---
 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 ea2db3ce..68e76fa8 100644
--- a/drivers/mmc/host/meson-gx-mmc.c
+++ b/drivers/mmc/host/meson-gx-mmc.c
@@ -130,7 +130,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;
@@ -247,19 +246,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 v3 6/10] mmc: meson-gx: fix error path in meson_mmc_clk_init / meson_mmc_probe

[Heiner Kallweit]

The condition should be "if (ret)" as the disable/unprepare is
supposed to be executed if the previous command fails.
In addition adjust the error path in probe to properly deal
with the case that cfg_div_clk can be registered successfully
but enable/prepare fails.
In this case we shouldn't call clk_disable_unprepare.

Reported-by: Michał Zegan <webczat_200@poczta.onet.pl>
Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
v2:
- extended commit message
v3:
- adjust error path in probe
---
 drivers/mmc/host/meson-gx-mmc.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/mmc/host/meson-gx-mmc.c b/drivers/mmc/host/meson-gx-mmc.c
index 68e76fa8..002e4aac 100644
--- a/drivers/mmc/host/meson-gx-mmc.c
+++ b/drivers/mmc/host/meson-gx-mmc.c
@@ -321,7 +321,7 @@ static int meson_mmc_clk_init(struct meson_host *host)
 	host->mmc->f_min = clk_round_rate(host->cfg_div_clk, 400000);
 
 	ret = meson_mmc_clk_set(host, host->mmc->f_min);
-	if (!ret)
+	if (ret)
 		clk_disable_unprepare(host->cfg_div_clk);
 
 	return ret;
@@ -771,7 +771,7 @@ static int meson_mmc_probe(struct platform_device *pdev)
 					meson_mmc_irq_thread, IRQF_SHARED,
 					DRIVER_NAME, host);
 	if (ret)
-		goto free_host;
+		goto err_div_clk;
 
 	mmc->max_blk_count = CMD_CFG_LENGTH_MASK;
 	mmc->max_req_size = mmc->max_blk_count * mmc->max_blk_size;
@@ -784,7 +784,7 @@ static int meson_mmc_probe(struct platform_device *pdev)
 	if (host->bounce_buf == NULL) {
 		dev_err(host->dev, "Unable to map allocate DMA bounce buffer.\n");
 		ret = -ENOMEM;
-		goto free_host;
+		goto err_div_clk;
 	}
 
 	mmc->ops = &meson_mmc_ops;
@@ -792,8 +792,9 @@ static int meson_mmc_probe(struct platform_device *pdev)
 
 	return 0;
 
-free_host:
+err_div_clk:
 	clk_disable_unprepare(host->cfg_div_clk);
+free_host:
 	clk_disable_unprepare(host->core_clk);
 	mmc_free_host(mmc);
 	return ret;
-- 
2.11.1

[PATCH v3 6/10] mmc: meson-gx: fix error path in meson_mmc_clk_init / meson_mmc_probe

[Heiner Kallweit]

The condition should be "if (ret)" as the disable/unprepare is
supposed to be executed if the previous command fails.
In addition adjust the error path in probe to properly deal
with the case that cfg_div_clk can be registered successfully
but enable/prepare fails.
In this case we shouldn't call clk_disable_unprepare.

Reported-by: Micha? Zegan <webczat_200@poczta.onet.pl>
Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
v2:
- extended commit message
v3:
- adjust error path in probe
---
 drivers/mmc/host/meson-gx-mmc.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/mmc/host/meson-gx-mmc.c b/drivers/mmc/host/meson-gx-mmc.c
index 68e76fa8..002e4aac 100644
--- a/drivers/mmc/host/meson-gx-mmc.c
+++ b/drivers/mmc/host/meson-gx-mmc.c
@@ -321,7 +321,7 @@ static int meson_mmc_clk_init(struct meson_host *host)
 	host->mmc->f_min = clk_round_rate(host->cfg_div_clk, 400000);
 
 	ret = meson_mmc_clk_set(host, host->mmc->f_min);
-	if (!ret)
+	if (ret)
 		clk_disable_unprepare(host->cfg_div_clk);
 
 	return ret;
@@ -771,7 +771,7 @@ static int meson_mmc_probe(struct platform_device *pdev)
 					meson_mmc_irq_thread, IRQF_SHARED,
 					DRIVER_NAME, host);
 	if (ret)
-		goto free_host;
+		goto err_div_clk;
 
 	mmc->max_blk_count = CMD_CFG_LENGTH_MASK;
 	mmc->max_req_size = mmc->max_blk_count * mmc->max_blk_size;
@@ -784,7 +784,7 @@ static int meson_mmc_probe(struct platform_device *pdev)
 	if (host->bounce_buf == NULL) {
 		dev_err(host->dev, "Unable to map allocate DMA bounce buffer.\n");
 		ret = -ENOMEM;
-		goto free_host;
+		goto err_div_clk;
 	}
 
 	mmc->ops = &meson_mmc_ops;
@@ -792,8 +792,9 @@ static int meson_mmc_probe(struct platform_device *pdev)
 
 	return 0;
 
-free_host:
+err_div_clk:
 	clk_disable_unprepare(host->cfg_div_clk);
+free_host:
 	clk_disable_unprepare(host->core_clk);
 	mmc_free_host(mmc);
 	return ret;
-- 
2.11.1

[PATCH v3 7/10] mmc: meson-gx: remove unneeded devm_kstrdup in meson_mmc_clk_init

[Heiner Kallweit]

CLK 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>
Acked-by: Kevin Hilman <khilman@baylibre.com>
---
v2:
- fixed commit message
- added acked-by
v3:
- no changes
---
 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 002e4aac..684cc088 100644
--- a/drivers/mmc/host/meson-gx-mmc.c
+++ b/drivers/mmc/host/meson-gx-mmc.c
@@ -281,7 +281,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 v3 7/10] mmc: meson-gx: remove unneeded devm_kstrdup in meson_mmc_clk_init

[Heiner Kallweit]

CLK 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>
Acked-by: Kevin Hilman <khilman@baylibre.com>
---
v2:
- fixed commit message
- added acked-by
v3:
- no changes
---
 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 002e4aac..684cc088 100644
--- a/drivers/mmc/host/meson-gx-mmc.c
+++ b/drivers/mmc/host/meson-gx-mmc.c
@@ -281,7 +281,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 v3 8/10] 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>
Acked-by: Kevin Hilman <khilman@baylibre.com>
---
v2:
- added acked-by
v3:
- rebased
---
 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 684cc088..e904b0a5 100644
--- a/drivers/mmc/host/meson-gx-mmc.c
+++ b/drivers/mmc/host/meson-gx-mmc.c
@@ -394,15 +394,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 ||
@@ -413,11 +404,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,
@@ -695,6 +686,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,
@@ -755,10 +757,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);
 
@@ -767,6 +765,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,
 					DRIVER_NAME, host);
-- 
2.11.1

[PATCH v3 8/10] 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>
Acked-by: Kevin Hilman <khilman@baylibre.com>
---
v2:
- added acked-by
v3:
- rebased
---
 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 684cc088..e904b0a5 100644
--- a/drivers/mmc/host/meson-gx-mmc.c
+++ b/drivers/mmc/host/meson-gx-mmc.c
@@ -394,15 +394,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 ||
@@ -413,11 +404,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,
@@ -695,6 +686,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,
@@ -755,10 +757,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);
 
@@ -767,6 +765,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,
 					DRIVER_NAME, host);
-- 
2.11.1

[PATCH v3 9/10] 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.
And we can remove some now unneeded WARN_ON's.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
Acked-by: Kevin Hilman <khilman@baylibre.com>
---
v2:
- extended commit message
- added acked-by
v3:
- rebased
---
 drivers/mmc/host/meson-gx-mmc.c | 24 +++---------------------
 1 file changed, 3 insertions(+), 21 deletions(-)

diff --git a/drivers/mmc/host/meson-gx-mmc.c b/drivers/mmc/host/meson-gx-mmc.c
index e904b0a5..5281014e 100644
--- a/drivers/mmc/host/meson-gx-mmc.c
+++ b/drivers/mmc/host/meson-gx-mmc.c
@@ -122,7 +122,6 @@
 struct meson_host {
 	struct	device		*dev;
 	struct	mmc_host	*mmc;
-	struct	mmc_request	*mrq;
 	struct	mmc_command	*cmd;
 
 	spinlock_t lock;
@@ -416,9 +415,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);
 }
@@ -525,13 +521,9 @@ 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);
-
 	/* Stop execution */
 	writel(0, host->regs + SD_EMMC_START);
 
-	host->mrq = mrq;
-
 	if (mrq->sbc)
 		meson_mmc_start_cmd(mmc, mrq->sbc);
 	else
@@ -555,7 +547,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;
@@ -565,11 +556,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;
 
@@ -616,7 +602,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;
 
@@ -643,14 +629,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;
 
@@ -664,8 +646,8 @@ 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)
-		meson_mmc_request_done(host->mmc, mrq);
+	if (!data || !data->stop || cmd->mrq->sbc)
+		meson_mmc_request_done(host->mmc, cmd->mrq);
 	else
 		meson_mmc_start_cmd(host->mmc, data->stop);
 
-- 
2.11.1

[PATCH v3 9/10] 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.
And we can remove some now unneeded WARN_ON's.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
Acked-by: Kevin Hilman <khilman@baylibre.com>
---
v2:
- extended commit message
- added acked-by
v3:
- rebased
---
 drivers/mmc/host/meson-gx-mmc.c | 24 +++---------------------
 1 file changed, 3 insertions(+), 21 deletions(-)

diff --git a/drivers/mmc/host/meson-gx-mmc.c b/drivers/mmc/host/meson-gx-mmc.c
index e904b0a5..5281014e 100644
--- a/drivers/mmc/host/meson-gx-mmc.c
+++ b/drivers/mmc/host/meson-gx-mmc.c
@@ -122,7 +122,6 @@
 struct meson_host {
 	struct	device		*dev;
 	struct	mmc_host	*mmc;
-	struct	mmc_request	*mrq;
 	struct	mmc_command	*cmd;
 
 	spinlock_t lock;
@@ -416,9 +415,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);
 }
@@ -525,13 +521,9 @@ 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);
-
 	/* Stop execution */
 	writel(0, host->regs + SD_EMMC_START);
 
-	host->mrq = mrq;
-
 	if (mrq->sbc)
 		meson_mmc_start_cmd(mmc, mrq->sbc);
 	else
@@ -555,7 +547,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;
@@ -565,11 +556,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;
 
@@ -616,7 +602,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;
 
@@ -643,14 +629,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;
 
@@ -664,8 +646,8 @@ 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)
-		meson_mmc_request_done(host->mmc, mrq);
+	if (!data || !data->stop || cmd->mrq->sbc)
+		meson_mmc_request_done(host->mmc, cmd->mrq);
 	else
 		meson_mmc_start_cmd(host->mmc, data->stop);
 
-- 
2.11.1

[PATCH v3 10/10] mmc: meson-gx: replace magic timeout numbers with constants

[Heiner Kallweit]

Replace timeout magic numbers with proper constants.

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

diff --git a/drivers/mmc/host/meson-gx-mmc.c b/drivers/mmc/host/meson-gx-mmc.c
index 5281014e..f244fcf9 100644
--- a/drivers/mmc/host/meson-gx-mmc.c
+++ b/drivers/mmc/host/meson-gx-mmc.c
@@ -116,6 +116,8 @@
 
 #define SD_EMMC_CFG_BLK_SIZE 512 /* internal buffer max: 512 bytes */
 #define SD_EMMC_CFG_RESP_TIMEOUT 256 /* in clock cycles */
+#define SD_EMMC_CMD_TIMEOUT 1024 /* in ms */
+#define SD_EMMC_CMD_TIMEOUT_DATA 4096 /* in ms */
 #define SD_EMMC_CFG_CMD_GAP 16 /* in clock cycles */
 #define MUX_CLK_NUM_PARENTS 2
 
@@ -498,10 +500,10 @@ static void meson_mmc_start_cmd(struct mmc_host *mmc, struct mmc_command *cmd)
 
 		desc->cmd_data = host->bounce_dma_addr & CMD_DATA_MASK;
 
-		cmd_cfg_timeout = 12;
+		cmd_cfg_timeout = ilog2(SD_EMMC_CMD_TIMEOUT_DATA);
 	} else {
 		desc->cmd_cfg &= ~CMD_CFG_DATA_IO;
-		cmd_cfg_timeout = 10;
+		cmd_cfg_timeout = ilog2(SD_EMMC_CMD_TIMEOUT);
 	}
 	desc->cmd_cfg |= (cmd_cfg_timeout & CMD_CFG_TIMEOUT_MASK) <<
 		CMD_CFG_TIMEOUT_SHIFT;
-- 
2.11.1

[PATCH v3 10/10] mmc: meson-gx: replace magic timeout numbers with constants

[Heiner Kallweit]

Replace timeout magic numbers with proper constants.

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

diff --git a/drivers/mmc/host/meson-gx-mmc.c b/drivers/mmc/host/meson-gx-mmc.c
index 5281014e..f244fcf9 100644
--- a/drivers/mmc/host/meson-gx-mmc.c
+++ b/drivers/mmc/host/meson-gx-mmc.c
@@ -116,6 +116,8 @@
 
 #define SD_EMMC_CFG_BLK_SIZE 512 /* internal buffer max: 512 bytes */
 #define SD_EMMC_CFG_RESP_TIMEOUT 256 /* in clock cycles */
+#define SD_EMMC_CMD_TIMEOUT 1024 /* in ms */
+#define SD_EMMC_CMD_TIMEOUT_DATA 4096 /* in ms */
 #define SD_EMMC_CFG_CMD_GAP 16 /* in clock cycles */
 #define MUX_CLK_NUM_PARENTS 2
 
@@ -498,10 +500,10 @@ static void meson_mmc_start_cmd(struct mmc_host *mmc, struct mmc_command *cmd)
 
 		desc->cmd_data = host->bounce_dma_addr & CMD_DATA_MASK;
 
-		cmd_cfg_timeout = 12;
+		cmd_cfg_timeout = ilog2(SD_EMMC_CMD_TIMEOUT_DATA);
 	} else {
 		desc->cmd_cfg &= ~CMD_CFG_DATA_IO;
-		cmd_cfg_timeout = 10;
+		cmd_cfg_timeout = ilog2(SD_EMMC_CMD_TIMEOUT);
 	}
 	desc->cmd_cfg |= (cmd_cfg_timeout & CMD_CFG_TIMEOUT_MASK) <<
 		CMD_CFG_TIMEOUT_SHIFT;
-- 
2.11.1

Re: [PATCH v3 6/10] mmc: meson-gx: fix error path in meson_mmc_clk_init / meson_mmc_probe

[Michał Zegan]

[-- Attachment #1.1: Type: text/plain, Size: 2278 bytes --]



W dniu 18.02.2017 o 14:19, Heiner Kallweit pisze:
> The condition should be "if (ret)" as the disable/unprepare is
> supposed to be executed if the previous command fails.
> In addition adjust the error path in probe to properly deal
> with the case that cfg_div_clk can be registered successfully
> but enable/prepare fails.
> In this case we shouldn't call clk_disable_unprepare.
> 
> Reported-by: Michał Zegan <webczat_200@poczta.onet.pl>
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
> ---
> v2:
> - extended commit message
> v3:
> - adjust error path in probe
> ---
>  drivers/mmc/host/meson-gx-mmc.c | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/mmc/host/meson-gx-mmc.c b/drivers/mmc/host/meson-gx-mmc.c
> index 68e76fa8..002e4aac 100644
> --- a/drivers/mmc/host/meson-gx-mmc.c
> +++ b/drivers/mmc/host/meson-gx-mmc.c
> @@ -321,7 +321,7 @@ static int meson_mmc_clk_init(struct meson_host *host)
>  	host->mmc->f_min = clk_round_rate(host->cfg_div_clk, 400000);
>  
>  	ret = meson_mmc_clk_set(host, host->mmc->f_min);
> -	if (!ret)
> +	if (ret)
>  		clk_disable_unprepare(host->cfg_div_clk);
>  
>  	return ret;
> @@ -771,7 +771,7 @@ static int meson_mmc_probe(struct platform_device *pdev)
>  					meson_mmc_irq_thread, IRQF_SHARED,
>  					DRIVER_NAME, host);
>  	if (ret)
> -		goto free_host;
> +		goto err_div_clk;
>  
>  	mmc->max_blk_count = CMD_CFG_LENGTH_MASK;
>  	mmc->max_req_size = mmc->max_blk_count * mmc->max_blk_size;
> @@ -784,7 +784,7 @@ static int meson_mmc_probe(struct platform_device *pdev)
>  	if (host->bounce_buf == NULL) {
>  		dev_err(host->dev, "Unable to map allocate DMA bounce buffer.\n");
>  		ret = -ENOMEM;
> -		goto free_host;
> +		goto err_div_clk;
>  	}
>  
>  	mmc->ops = &meson_mmc_ops;
> @@ -792,8 +792,9 @@ static int meson_mmc_probe(struct platform_device *pdev)
>  
>  	return 0;
>  
> -free_host:
> +err_div_clk:
>  	clk_disable_unprepare(host->cfg_div_clk);
> +free_host:
>  	clk_disable_unprepare(host->core_clk);
>  	mmc_free_host(mmc);
>  	return ret;
> 
Doesn't the same problem exist with a core clock? It also could error
when being enabled, and it will be disabled even if not enabled before...


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 525 bytes --]

[PATCH v3 6/10] mmc: meson-gx: fix error path in meson_mmc_clk_init / meson_mmc_probe

[Michał Zegan]

W dniu 18.02.2017 o 14:19, Heiner Kallweit pisze:
> The condition should be "if (ret)" as the disable/unprepare is
> supposed to be executed if the previous command fails.
> In addition adjust the error path in probe to properly deal
> with the case that cfg_div_clk can be registered successfully
> but enable/prepare fails.
> In this case we shouldn't call clk_disable_unprepare.
> 
> Reported-by: Micha? Zegan <webczat_200@poczta.onet.pl>
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
> ---
> v2:
> - extended commit message
> v3:
> - adjust error path in probe
> ---
>  drivers/mmc/host/meson-gx-mmc.c | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/mmc/host/meson-gx-mmc.c b/drivers/mmc/host/meson-gx-mmc.c
> index 68e76fa8..002e4aac 100644
> --- a/drivers/mmc/host/meson-gx-mmc.c
> +++ b/drivers/mmc/host/meson-gx-mmc.c
> @@ -321,7 +321,7 @@ static int meson_mmc_clk_init(struct meson_host *host)
>  	host->mmc->f_min = clk_round_rate(host->cfg_div_clk, 400000);
>  
>  	ret = meson_mmc_clk_set(host, host->mmc->f_min);
> -	if (!ret)
> +	if (ret)
>  		clk_disable_unprepare(host->cfg_div_clk);
>  
>  	return ret;
> @@ -771,7 +771,7 @@ static int meson_mmc_probe(struct platform_device *pdev)
>  					meson_mmc_irq_thread, IRQF_SHARED,
>  					DRIVER_NAME, host);
>  	if (ret)
> -		goto free_host;
> +		goto err_div_clk;
>  
>  	mmc->max_blk_count = CMD_CFG_LENGTH_MASK;
>  	mmc->max_req_size = mmc->max_blk_count * mmc->max_blk_size;
> @@ -784,7 +784,7 @@ static int meson_mmc_probe(struct platform_device *pdev)
>  	if (host->bounce_buf == NULL) {
>  		dev_err(host->dev, "Unable to map allocate DMA bounce buffer.\n");
>  		ret = -ENOMEM;
> -		goto free_host;
> +		goto err_div_clk;
>  	}
>  
>  	mmc->ops = &meson_mmc_ops;
> @@ -792,8 +792,9 @@ static int meson_mmc_probe(struct platform_device *pdev)
>  
>  	return 0;
>  
> -free_host:
> +err_div_clk:
>  	clk_disable_unprepare(host->cfg_div_clk);
> +free_host:
>  	clk_disable_unprepare(host->core_clk);
>  	mmc_free_host(mmc);
>  	return ret;
> 
Doesn't the same problem exist with a core clock? It also could error
when being enabled, and it will be disabled even if not enabled before...

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 525 bytes
Desc: OpenPGP digital signature
URL: <http://lists.infradead.org/pipermail/linux-amlogic/attachments/20170218/7ebfe0b4/attachment.sig>

Re: [PATCH v3 6/10] mmc: meson-gx: fix error path in meson_mmc_clk_init / meson_mmc_probe

[Heiner Kallweit]

Am 18.02.2017 um 14:57 schrieb Michał Zegan:
> 
> 
> W dniu 18.02.2017 o 14:19, Heiner Kallweit pisze:
>> The condition should be "if (ret)" as the disable/unprepare is
>> supposed to be executed if the previous command fails.
>> In addition adjust the error path in probe to properly deal
>> with the case that cfg_div_clk can be registered successfully
>> but enable/prepare fails.
>> In this case we shouldn't call clk_disable_unprepare.
>>
>> Reported-by: Michał Zegan <webczat_200@poczta.onet.pl>
>> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
>> ---
>> v2:
>> - extended commit message
>> v3:
>> - adjust error path in probe
>> ---
>>  drivers/mmc/host/meson-gx-mmc.c | 9 +++++----
>>  1 file changed, 5 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/mmc/host/meson-gx-mmc.c b/drivers/mmc/host/meson-gx-mmc.c
>> index 68e76fa8..002e4aac 100644
>> --- a/drivers/mmc/host/meson-gx-mmc.c
>> +++ b/drivers/mmc/host/meson-gx-mmc.c
>> @@ -321,7 +321,7 @@ static int meson_mmc_clk_init(struct meson_host *host)
>>  	host->mmc->f_min = clk_round_rate(host->cfg_div_clk, 400000);
>>  
>>  	ret = meson_mmc_clk_set(host, host->mmc->f_min);
>> -	if (!ret)
>> +	if (ret)
>>  		clk_disable_unprepare(host->cfg_div_clk);
>>  
>>  	return ret;
>> @@ -771,7 +771,7 @@ static int meson_mmc_probe(struct platform_device *pdev)
>>  					meson_mmc_irq_thread, IRQF_SHARED,
>>  					DRIVER_NAME, host);
>>  	if (ret)
>> -		goto free_host;
>> +		goto err_div_clk;
>>  
>>  	mmc->max_blk_count = CMD_CFG_LENGTH_MASK;
>>  	mmc->max_req_size = mmc->max_blk_count * mmc->max_blk_size;
>> @@ -784,7 +784,7 @@ static int meson_mmc_probe(struct platform_device *pdev)
>>  	if (host->bounce_buf == NULL) {
>>  		dev_err(host->dev, "Unable to map allocate DMA bounce buffer.\n");
>>  		ret = -ENOMEM;
>> -		goto free_host;
>> +		goto err_div_clk;
>>  	}
>>  
>>  	mmc->ops = &meson_mmc_ops;
>> @@ -792,8 +792,9 @@ static int meson_mmc_probe(struct platform_device *pdev)
>>  
>>  	return 0;
>>  
>> -free_host:
>> +err_div_clk:
>>  	clk_disable_unprepare(host->cfg_div_clk);
>> +free_host:
>>  	clk_disable_unprepare(host->core_clk);
>>  	mmc_free_host(mmc);
>>  	return ret;
>>
> Doesn't the same problem exist with a core clock? It also could error
> when being enabled, and it will be disabled even if not enabled before...
> 
Yes, there it's basically the same issue. Feel free to submit a patch.

[PATCH v3 6/10] mmc: meson-gx: fix error path in meson_mmc_clk_init / meson_mmc_probe

[Heiner Kallweit]

Am 18.02.2017 um 14:57 schrieb Micha? Zegan:
> 
> 
> W dniu 18.02.2017 o 14:19, Heiner Kallweit pisze:
>> The condition should be "if (ret)" as the disable/unprepare is
>> supposed to be executed if the previous command fails.
>> In addition adjust the error path in probe to properly deal
>> with the case that cfg_div_clk can be registered successfully
>> but enable/prepare fails.
>> In this case we shouldn't call clk_disable_unprepare.
>>
>> Reported-by: Micha? Zegan <webczat_200@poczta.onet.pl>
>> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
>> ---
>> v2:
>> - extended commit message
>> v3:
>> - adjust error path in probe
>> ---
>>  drivers/mmc/host/meson-gx-mmc.c | 9 +++++----
>>  1 file changed, 5 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/mmc/host/meson-gx-mmc.c b/drivers/mmc/host/meson-gx-mmc.c
>> index 68e76fa8..002e4aac 100644
>> --- a/drivers/mmc/host/meson-gx-mmc.c
>> +++ b/drivers/mmc/host/meson-gx-mmc.c
>> @@ -321,7 +321,7 @@ static int meson_mmc_clk_init(struct meson_host *host)
>>  	host->mmc->f_min = clk_round_rate(host->cfg_div_clk, 400000);
>>  
>>  	ret = meson_mmc_clk_set(host, host->mmc->f_min);
>> -	if (!ret)
>> +	if (ret)
>>  		clk_disable_unprepare(host->cfg_div_clk);
>>  
>>  	return ret;
>> @@ -771,7 +771,7 @@ static int meson_mmc_probe(struct platform_device *pdev)
>>  					meson_mmc_irq_thread, IRQF_SHARED,
>>  					DRIVER_NAME, host);
>>  	if (ret)
>> -		goto free_host;
>> +		goto err_div_clk;
>>  
>>  	mmc->max_blk_count = CMD_CFG_LENGTH_MASK;
>>  	mmc->max_req_size = mmc->max_blk_count * mmc->max_blk_size;
>> @@ -784,7 +784,7 @@ static int meson_mmc_probe(struct platform_device *pdev)
>>  	if (host->bounce_buf == NULL) {
>>  		dev_err(host->dev, "Unable to map allocate DMA bounce buffer.\n");
>>  		ret = -ENOMEM;
>> -		goto free_host;
>> +		goto err_div_clk;
>>  	}
>>  
>>  	mmc->ops = &meson_mmc_ops;
>> @@ -792,8 +792,9 @@ static int meson_mmc_probe(struct platform_device *pdev)
>>  
>>  	return 0;
>>  
>> -free_host:
>> +err_div_clk:
>>  	clk_disable_unprepare(host->cfg_div_clk);
>> +free_host:
>>  	clk_disable_unprepare(host->core_clk);
>>  	mmc_free_host(mmc);
>>  	return ret;
>>
> Doesn't the same problem exist with a core clock? It also could error
> when being enabled, and it will be disabled even if not enabled before...
> 
Yes, there it's basically the same issue. Feel free to submit a patch.

Re: [PATCH v3 6/10] mmc: meson-gx: fix error path in meson_mmc_clk_init / meson_mmc_probe

[Michał Zegan]

[-- Attachment #1.1: Type: text/plain, Size: 2723 bytes --]



W dniu 18.02.2017 o 16:22, Heiner Kallweit pisze:
> Am 18.02.2017 um 14:57 schrieb Michał Zegan:
>>
>>
>> W dniu 18.02.2017 o 14:19, Heiner Kallweit pisze:
>>> The condition should be "if (ret)" as the disable/unprepare is
>>> supposed to be executed if the previous command fails.
>>> In addition adjust the error path in probe to properly deal
>>> with the case that cfg_div_clk can be registered successfully
>>> but enable/prepare fails.
>>> In this case we shouldn't call clk_disable_unprepare.
>>>
>>> Reported-by: Michał Zegan <webczat_200@poczta.onet.pl>
>>> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
>>> ---
>>> v2:
>>> - extended commit message
>>> v3:
>>> - adjust error path in probe
>>> ---
>>>  drivers/mmc/host/meson-gx-mmc.c | 9 +++++----
>>>  1 file changed, 5 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/drivers/mmc/host/meson-gx-mmc.c b/drivers/mmc/host/meson-gx-mmc.c
>>> index 68e76fa8..002e4aac 100644
>>> --- a/drivers/mmc/host/meson-gx-mmc.c
>>> +++ b/drivers/mmc/host/meson-gx-mmc.c
>>> @@ -321,7 +321,7 @@ static int meson_mmc_clk_init(struct meson_host *host)
>>>  	host->mmc->f_min = clk_round_rate(host->cfg_div_clk, 400000);
>>>  
>>>  	ret = meson_mmc_clk_set(host, host->mmc->f_min);
>>> -	if (!ret)
>>> +	if (ret)
>>>  		clk_disable_unprepare(host->cfg_div_clk);
>>>  
>>>  	return ret;
>>> @@ -771,7 +771,7 @@ static int meson_mmc_probe(struct platform_device *pdev)
>>>  					meson_mmc_irq_thread, IRQF_SHARED,
>>>  					DRIVER_NAME, host);
>>>  	if (ret)
>>> -		goto free_host;
>>> +		goto err_div_clk;
>>>  
>>>  	mmc->max_blk_count = CMD_CFG_LENGTH_MASK;
>>>  	mmc->max_req_size = mmc->max_blk_count * mmc->max_blk_size;
>>> @@ -784,7 +784,7 @@ static int meson_mmc_probe(struct platform_device *pdev)
>>>  	if (host->bounce_buf == NULL) {
>>>  		dev_err(host->dev, "Unable to map allocate DMA bounce buffer.\n");
>>>  		ret = -ENOMEM;
>>> -		goto free_host;
>>> +		goto err_div_clk;
>>>  	}
>>>  
>>>  	mmc->ops = &meson_mmc_ops;
>>> @@ -792,8 +792,9 @@ static int meson_mmc_probe(struct platform_device *pdev)
>>>  
>>>  	return 0;
>>>  
>>> -free_host:
>>> +err_div_clk:
>>>  	clk_disable_unprepare(host->cfg_div_clk);
>>> +free_host:
>>>  	clk_disable_unprepare(host->core_clk);
>>>  	mmc_free_host(mmc);
>>>  	return ret;
>>>
>> Doesn't the same problem exist with a core clock? It also could error
>> when being enabled, and it will be disabled even if not enabled before...
>>
> Yes, there it's basically the same issue. Feel free to submit a patch.
> 
Okay, I will submit a patch based on this series to fix that too, thanks.
Reviewed-by: Michał Zegan <webczat@webczatnet.pl>


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 525 bytes --]

[PATCH v3 6/10] mmc: meson-gx: fix error path in meson_mmc_clk_init / meson_mmc_probe

[Michał Zegan]

W dniu 18.02.2017 o 16:22, Heiner Kallweit pisze:
> Am 18.02.2017 um 14:57 schrieb Micha? Zegan:
>>
>>
>> W dniu 18.02.2017 o 14:19, Heiner Kallweit pisze:
>>> The condition should be "if (ret)" as the disable/unprepare is
>>> supposed to be executed if the previous command fails.
>>> In addition adjust the error path in probe to properly deal
>>> with the case that cfg_div_clk can be registered successfully
>>> but enable/prepare fails.
>>> In this case we shouldn't call clk_disable_unprepare.
>>>
>>> Reported-by: Micha? Zegan <webczat_200@poczta.onet.pl>
>>> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
>>> ---
>>> v2:
>>> - extended commit message
>>> v3:
>>> - adjust error path in probe
>>> ---
>>>  drivers/mmc/host/meson-gx-mmc.c | 9 +++++----
>>>  1 file changed, 5 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/drivers/mmc/host/meson-gx-mmc.c b/drivers/mmc/host/meson-gx-mmc.c
>>> index 68e76fa8..002e4aac 100644
>>> --- a/drivers/mmc/host/meson-gx-mmc.c
>>> +++ b/drivers/mmc/host/meson-gx-mmc.c
>>> @@ -321,7 +321,7 @@ static int meson_mmc_clk_init(struct meson_host *host)
>>>  	host->mmc->f_min = clk_round_rate(host->cfg_div_clk, 400000);
>>>  
>>>  	ret = meson_mmc_clk_set(host, host->mmc->f_min);
>>> -	if (!ret)
>>> +	if (ret)
>>>  		clk_disable_unprepare(host->cfg_div_clk);
>>>  
>>>  	return ret;
>>> @@ -771,7 +771,7 @@ static int meson_mmc_probe(struct platform_device *pdev)
>>>  					meson_mmc_irq_thread, IRQF_SHARED,
>>>  					DRIVER_NAME, host);
>>>  	if (ret)
>>> -		goto free_host;
>>> +		goto err_div_clk;
>>>  
>>>  	mmc->max_blk_count = CMD_CFG_LENGTH_MASK;
>>>  	mmc->max_req_size = mmc->max_blk_count * mmc->max_blk_size;
>>> @@ -784,7 +784,7 @@ static int meson_mmc_probe(struct platform_device *pdev)
>>>  	if (host->bounce_buf == NULL) {
>>>  		dev_err(host->dev, "Unable to map allocate DMA bounce buffer.\n");
>>>  		ret = -ENOMEM;
>>> -		goto free_host;
>>> +		goto err_div_clk;
>>>  	}
>>>  
>>>  	mmc->ops = &meson_mmc_ops;
>>> @@ -792,8 +792,9 @@ static int meson_mmc_probe(struct platform_device *pdev)
>>>  
>>>  	return 0;
>>>  
>>> -free_host:
>>> +err_div_clk:
>>>  	clk_disable_unprepare(host->cfg_div_clk);
>>> +free_host:
>>>  	clk_disable_unprepare(host->core_clk);
>>>  	mmc_free_host(mmc);
>>>  	return ret;
>>>
>> Doesn't the same problem exist with a core clock? It also could error
>> when being enabled, and it will be disabled even if not enabled before...
>>
> Yes, there it's basically the same issue. Feel free to submit a patch.
> 
Okay, I will submit a patch based on this series to fix that too, thanks.
Reviewed-by: Micha? Zegan <webczat@webczatnet.pl>

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 525 bytes
Desc: OpenPGP digital signature
URL: <http://lists.infradead.org/pipermail/linux-amlogic/attachments/20170218/8d1750e3/attachment.sig>

Re: [PATCH v3 6/10] mmc: meson-gx: fix error path in meson_mmc_clk_init / meson_mmc_probe

[Kevin Hilman]

Heiner Kallweit <hkallweit1@gmail.com> writes:

> The condition should be "if (ret)" as the disable/unprepare is
> supposed to be executed if the previous command fails.
> In addition adjust the error path in probe to properly deal
> with the case that cfg_div_clk can be registered successfully
> but enable/prepare fails.
> In this case we shouldn't call clk_disable_unprepare.
>
> Reported-by: Michał Zegan <webczat_200@poczta.onet.pl>
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>

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

> ---
> v2:
> - extended commit message
> v3:
> - adjust error path in probe
> ---
>  drivers/mmc/host/meson-gx-mmc.c | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/mmc/host/meson-gx-mmc.c b/drivers/mmc/host/meson-gx-mmc.c
> index 68e76fa8..002e4aac 100644
> --- a/drivers/mmc/host/meson-gx-mmc.c
> +++ b/drivers/mmc/host/meson-gx-mmc.c
> @@ -321,7 +321,7 @@ static int meson_mmc_clk_init(struct meson_host *host)
>  	host->mmc->f_min = clk_round_rate(host->cfg_div_clk, 400000);
>  
>  	ret = meson_mmc_clk_set(host, host->mmc->f_min);
> -	if (!ret)
> +	if (ret)
>  		clk_disable_unprepare(host->cfg_div_clk);
>  
>  	return ret;
> @@ -771,7 +771,7 @@ static int meson_mmc_probe(struct platform_device *pdev)
>  					meson_mmc_irq_thread, IRQF_SHARED,
>  					DRIVER_NAME, host);
>  	if (ret)
> -		goto free_host;
> +		goto err_div_clk;
>  
>  	mmc->max_blk_count = CMD_CFG_LENGTH_MASK;
>  	mmc->max_req_size = mmc->max_blk_count * mmc->max_blk_size;
> @@ -784,7 +784,7 @@ static int meson_mmc_probe(struct platform_device *pdev)
>  	if (host->bounce_buf == NULL) {
>  		dev_err(host->dev, "Unable to map allocate DMA bounce buffer.\n");
>  		ret = -ENOMEM;
> -		goto free_host;
> +		goto err_div_clk;
>  	}
>  
>  	mmc->ops = &meson_mmc_ops;
> @@ -792,8 +792,9 @@ static int meson_mmc_probe(struct platform_device *pdev)
>  
>  	return 0;
>  
> -free_host:
> +err_div_clk:
>  	clk_disable_unprepare(host->cfg_div_clk);
> +free_host:
>  	clk_disable_unprepare(host->core_clk);
>  	mmc_free_host(mmc);
>  	return ret;

[PATCH v3 6/10] mmc: meson-gx: fix error path in meson_mmc_clk_init / meson_mmc_probe

[Kevin Hilman]

Heiner Kallweit <hkallweit1@gmail.com> writes:

> The condition should be "if (ret)" as the disable/unprepare is
> supposed to be executed if the previous command fails.
> In addition adjust the error path in probe to properly deal
> with the case that cfg_div_clk can be registered successfully
> but enable/prepare fails.
> In this case we shouldn't call clk_disable_unprepare.
>
> Reported-by: Micha? Zegan <webczat_200@poczta.onet.pl>
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>

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

> ---
> v2:
> - extended commit message
> v3:
> - adjust error path in probe
> ---
>  drivers/mmc/host/meson-gx-mmc.c | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/mmc/host/meson-gx-mmc.c b/drivers/mmc/host/meson-gx-mmc.c
> index 68e76fa8..002e4aac 100644
> --- a/drivers/mmc/host/meson-gx-mmc.c
> +++ b/drivers/mmc/host/meson-gx-mmc.c
> @@ -321,7 +321,7 @@ static int meson_mmc_clk_init(struct meson_host *host)
>  	host->mmc->f_min = clk_round_rate(host->cfg_div_clk, 400000);
>  
>  	ret = meson_mmc_clk_set(host, host->mmc->f_min);
> -	if (!ret)
> +	if (ret)
>  		clk_disable_unprepare(host->cfg_div_clk);
>  
>  	return ret;
> @@ -771,7 +771,7 @@ static int meson_mmc_probe(struct platform_device *pdev)
>  					meson_mmc_irq_thread, IRQF_SHARED,
>  					DRIVER_NAME, host);
>  	if (ret)
> -		goto free_host;
> +		goto err_div_clk;
>  
>  	mmc->max_blk_count = CMD_CFG_LENGTH_MASK;
>  	mmc->max_req_size = mmc->max_blk_count * mmc->max_blk_size;
> @@ -784,7 +784,7 @@ static int meson_mmc_probe(struct platform_device *pdev)
>  	if (host->bounce_buf == NULL) {
>  		dev_err(host->dev, "Unable to map allocate DMA bounce buffer.\n");
>  		ret = -ENOMEM;
> -		goto free_host;
> +		goto err_div_clk;
>  	}
>  
>  	mmc->ops = &meson_mmc_ops;
> @@ -792,8 +792,9 @@ static int meson_mmc_probe(struct platform_device *pdev)
>  
>  	return 0;
>  
> -free_host:
> +err_div_clk:
>  	clk_disable_unprepare(host->cfg_div_clk);
> +free_host:
>  	clk_disable_unprepare(host->core_clk);
>  	mmc_free_host(mmc);
>  	return ret;

Re: [PATCH v3 10/10] mmc: meson-gx: replace magic timeout numbers with constants

[Kevin Hilman]

Heiner Kallweit <hkallweit1@gmail.com> writes:

> Replace timeout magic numbers with proper constants.
>
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>

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

> ---
> v3:
> - added to patch series
> ---
>  drivers/mmc/host/meson-gx-mmc.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/mmc/host/meson-gx-mmc.c b/drivers/mmc/host/meson-gx-mmc.c
> index 5281014e..f244fcf9 100644
> --- a/drivers/mmc/host/meson-gx-mmc.c
> +++ b/drivers/mmc/host/meson-gx-mmc.c
> @@ -116,6 +116,8 @@
>  
>  #define SD_EMMC_CFG_BLK_SIZE 512 /* internal buffer max: 512 bytes */
>  #define SD_EMMC_CFG_RESP_TIMEOUT 256 /* in clock cycles */
> +#define SD_EMMC_CMD_TIMEOUT 1024 /* in ms */
> +#define SD_EMMC_CMD_TIMEOUT_DATA 4096 /* in ms */
>  #define SD_EMMC_CFG_CMD_GAP 16 /* in clock cycles */
>  #define MUX_CLK_NUM_PARENTS 2
>  
> @@ -498,10 +500,10 @@ static void meson_mmc_start_cmd(struct mmc_host *mmc, struct mmc_command *cmd)
>  
>  		desc->cmd_data = host->bounce_dma_addr & CMD_DATA_MASK;
>  
> -		cmd_cfg_timeout = 12;
> +		cmd_cfg_timeout = ilog2(SD_EMMC_CMD_TIMEOUT_DATA);
>  	} else {
>  		desc->cmd_cfg &= ~CMD_CFG_DATA_IO;
> -		cmd_cfg_timeout = 10;
> +		cmd_cfg_timeout = ilog2(SD_EMMC_CMD_TIMEOUT);
>  	}
>  	desc->cmd_cfg |= (cmd_cfg_timeout & CMD_CFG_TIMEOUT_MASK) <<
>  		CMD_CFG_TIMEOUT_SHIFT;

[PATCH v3 10/10] mmc: meson-gx: replace magic timeout numbers with constants

[Kevin Hilman]

Heiner Kallweit <hkallweit1@gmail.com> writes:

> Replace timeout magic numbers with proper constants.
>
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>

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

> ---
> v3:
> - added to patch series
> ---
>  drivers/mmc/host/meson-gx-mmc.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/mmc/host/meson-gx-mmc.c b/drivers/mmc/host/meson-gx-mmc.c
> index 5281014e..f244fcf9 100644
> --- a/drivers/mmc/host/meson-gx-mmc.c
> +++ b/drivers/mmc/host/meson-gx-mmc.c
> @@ -116,6 +116,8 @@
>  
>  #define SD_EMMC_CFG_BLK_SIZE 512 /* internal buffer max: 512 bytes */
>  #define SD_EMMC_CFG_RESP_TIMEOUT 256 /* in clock cycles */
> +#define SD_EMMC_CMD_TIMEOUT 1024 /* in ms */
> +#define SD_EMMC_CMD_TIMEOUT_DATA 4096 /* in ms */
>  #define SD_EMMC_CFG_CMD_GAP 16 /* in clock cycles */
>  #define MUX_CLK_NUM_PARENTS 2
>  
> @@ -498,10 +500,10 @@ static void meson_mmc_start_cmd(struct mmc_host *mmc, struct mmc_command *cmd)
>  
>  		desc->cmd_data = host->bounce_dma_addr & CMD_DATA_MASK;
>  
> -		cmd_cfg_timeout = 12;
> +		cmd_cfg_timeout = ilog2(SD_EMMC_CMD_TIMEOUT_DATA);
>  	} else {
>  		desc->cmd_cfg &= ~CMD_CFG_DATA_IO;
> -		cmd_cfg_timeout = 10;
> +		cmd_cfg_timeout = ilog2(SD_EMMC_CMD_TIMEOUT);
>  	}
>  	desc->cmd_cfg |= (cmd_cfg_timeout & CMD_CFG_TIMEOUT_MASK) <<
>  		CMD_CFG_TIMEOUT_SHIFT;

Re: [PATCH v3 1/10] mmc: meson-gx: simplify bounce buffer setting in meson_mmc_start_cmd

[Kevin Hilman]

Hi Heinar,

Heiner Kallweit <hkallweit1@gmail.com> writes:

> Core ensures that there are no commands with cmd->data being set and
> nothing to transfer. 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>
> Acked-by: Kevin Hilman <khilman@baylibre.com>

Could you send this series one more time, collecting the remaining tags
and including the patches from Michał Zegan?

I think the MMC maintainers would appreciate one final batch with
everything collected together, and including a cover letter.

Thanks for all your great work on improving this driver,

Kevin

[PATCH v3 1/10] mmc: meson-gx: simplify bounce buffer setting in meson_mmc_start_cmd

[Kevin Hilman]

Hi Heinar,

Heiner Kallweit <hkallweit1@gmail.com> writes:

> Core ensures that there are no commands with cmd->data being set and
> nothing to transfer. 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>
> Acked-by: Kevin Hilman <khilman@baylibre.com>

Could you send this series one more time, collecting the remaining tags
and including the patches from Micha? Zegan?

I think the MMC maintainers would appreciate one final batch with
everything collected together, and including a cover letter.

Thanks for all your great work on improving this driver,

Kevin

Re: [PATCH v3 1/10] mmc: meson-gx: simplify bounce buffer setting in meson_mmc_start_cmd

[Heiner Kallweit]

Am 01.03.2017 um 19:09 schrieb Kevin Hilman:
> Hi Heinar,
> 
> Heiner Kallweit <hkallweit1@gmail.com> writes:
> 
>> Core ensures that there are no commands with cmd->data being set and
>> nothing to transfer. 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>
>> Acked-by: Kevin Hilman <khilman@baylibre.com>
> 
> Could you send this series one more time, collecting the remaining tags
> and including the patches from Michał Zegan?
> 
Patchwork tracks all the tags coming in for a patch, there it's visible
that you acked my 10 patches + the two from Michal.
So I would leave it up to Ulf whether he prefers to have the series
re-sent (with just the few remaining acks as changes).

Heiner

> I think the MMC maintainers would appreciate one final batch with
> everything collected together, and including a cover letter.
> 
> Thanks for all your great work on improving this driver,
> 
> Kevin
> 

[PATCH v3 1/10] mmc: meson-gx: simplify bounce buffer setting in meson_mmc_start_cmd

[Heiner Kallweit]

Am 01.03.2017 um 19:09 schrieb Kevin Hilman:
> Hi Heinar,
> 
> Heiner Kallweit <hkallweit1@gmail.com> writes:
> 
>> Core ensures that there are no commands with cmd->data being set and
>> nothing to transfer. 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>
>> Acked-by: Kevin Hilman <khilman@baylibre.com>
> 
> Could you send this series one more time, collecting the remaining tags
> and including the patches from Micha? Zegan?
> 
Patchwork tracks all the tags coming in for a patch, there it's visible
that you acked my 10 patches + the two from Michal.
So I would leave it up to Ulf whether he prefers to have the series
re-sent (with just the few remaining acks as changes).

Heiner

> I think the MMC maintainers would appreciate one final batch with
> everything collected together, and including a cover letter.
> 
> Thanks for all your great work on improving this driver,
> 
> Kevin
> 

Re: [PATCH v3 1/10] mmc: meson-gx: simplify bounce buffer setting in meson_mmc_start_cmd

[Kevin Hilman]

Heiner Kallweit <hkallweit1@gmail.com> writes:

> Am 01.03.2017 um 19:09 schrieb Kevin Hilman:
>> Hi Heinar,
>> 
>> Heiner Kallweit <hkallweit1@gmail.com> writes:
>> 
>>> Core ensures that there are no commands with cmd->data being set and
>>> nothing to transfer. 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>
>>> Acked-by: Kevin Hilman <khilman@baylibre.com>
>> 
>> Could you send this series one more time, collecting the remaining tags
>> and including the patches from Michał Zegan?
>> 
> Patchwork tracks all the tags coming in for a patch, there it's visible
> that you acked my 10 patches + the two from Michal.

Yes, I'm well aware of how patchwork works, yet my request still stands.

> So I would leave it up to Ulf whether he prefers to have the series
> re-sent (with just the few remaining acks as changes).

That's your choice, but some friendly advice from a kernel maintainer:
making things as easy as possible for maintainers/reviewers greatly
improves speed your code will get merged.

Kevin

[PATCH v3 1/10] mmc: meson-gx: simplify bounce buffer setting in meson_mmc_start_cmd

[Kevin Hilman]

Heiner Kallweit <hkallweit1@gmail.com> writes:

> Am 01.03.2017 um 19:09 schrieb Kevin Hilman:
>> Hi Heinar,
>> 
>> Heiner Kallweit <hkallweit1@gmail.com> writes:
>> 
>>> Core ensures that there are no commands with cmd->data being set and
>>> nothing to transfer. 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>
>>> Acked-by: Kevin Hilman <khilman@baylibre.com>
>> 
>> Could you send this series one more time, collecting the remaining tags
>> and including the patches from Micha? Zegan?
>> 
> Patchwork tracks all the tags coming in for a patch, there it's visible
> that you acked my 10 patches + the two from Michal.

Yes, I'm well aware of how patchwork works, yet my request still stands.

> So I would leave it up to Ulf whether he prefers to have the series
> re-sent (with just the few remaining acks as changes).

That's your choice, but some friendly advice from a kernel maintainer:
making things as easy as possible for maintainers/reviewers greatly
improves speed your code will get merged.

Kevin