[PATCH 00/14] mmc: meson-gx: driver fixups and upgrade

[PATCH 00/14] mmc: meson-gx: driver fixups and upgrade

[Jerome Brunet]

The patchset features several bugfixes, rework and upgrade for the
meson-gx MMC driver.

The main goal is to improve readability and enable new high speed
modes, such as eMMC DDR52 and sdcard UHS modes up to SDR50 (100Mhz)

While full speed SDR104 is stable with most cards, a few seems to
require an even more precise tuning. For this, we'll probably have
to implement per-line delay calibration.

This series has been tested on gxbb-p200, gxbb-nanopi-k2 and
gxl-s905x-libretech

Jerome Brunet (14):
  mmc: meson-gx: fix mux mask definition
  mmc: meson-gx: remove CLK_DIVIDER_ALLOW_ZERO clock flag
  mmc: meson-gx: clean up some constants
  mmc: meson-gx: use _irqsave variant of spinlock
  mmc: meson-gx: cfg init overwrite values
  mmc: meson-gx: rework set_ios function
  mmc: meson-gx: rework clk_set function
  mmc: meson-gx: rework clock init function
  mmc: meson-gx: simplify interrupt handler
  mmc: meson-gx: implement card_busy callback
  mmc: meson-gx: rework tuning function
  mmc: meson-gx: fix dual data rate mode frequencies
  mmc: meson-gx: work around clk-stop issue
  mmc: meson-gx: implement voltage switch callback

 drivers/mmc/host/meson-gx-mmc.c | 611 ++++++++++++++++++++++++++--------------
 1 file changed, 396 insertions(+), 215 deletions(-)

-- 
2.9.4

[PATCH 01/14] mmc: meson-gx: fix mux mask definition

[Jerome Brunet]

CCF generic mux will shift the mask using the value defined in shift
Define the mask accordingly

Signed-off-by: Jerome Brunet <jbrunet@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 de962c2d5e00..4217287923d4 100644
--- a/drivers/mmc/host/meson-gx-mmc.c
+++ b/drivers/mmc/host/meson-gx-mmc.c
@@ -366,7 +366,7 @@ static int meson_mmc_clk_init(struct meson_host *host)
 	init.num_parents = MUX_CLK_NUM_PARENTS;
 	host->mux.reg = host->regs + SD_EMMC_CLOCK;
 	host->mux.shift = __bf_shf(CLK_SRC_MASK);
-	host->mux.mask = CLK_SRC_MASK;
+	host->mux.mask = CLK_SRC_MASK >> host->mux.shift;
 	host->mux.flags = 0;
 	host->mux.table = NULL;
 	host->mux.hw.init = &init;
-- 
2.9.4

[PATCH 02/14] mmc: meson-gx: remove CLK_DIVIDER_ALLOW_ZERO clock flag

[Jerome Brunet]

Remove CLK_DIVIDER_ALLOW_ZERO. This flag means that a 1 based divider
with a 0 value will behave as a bypass clock

The mmc divider does not behave like this, a 0 value disables the clock
Remove this flag so CCF never allows a 0 value on this clock

Fixes: 51c5d8447bd7 ("MMC: meson: initial support for GX platforms")
Signed-off-by: Jerome Brunet <jbrunet@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 4217287923d4..d480a8052a06 100644
--- a/drivers/mmc/host/meson-gx-mmc.c
+++ b/drivers/mmc/host/meson-gx-mmc.c
@@ -389,7 +389,7 @@ static int meson_mmc_clk_init(struct meson_host *host)
 	host->cfg_div.width = __builtin_popcountl(CLK_DIV_MASK);
 	host->cfg_div.hw.init = &init;
 	host->cfg_div.flags = CLK_DIVIDER_ONE_BASED |
-		CLK_DIVIDER_ROUND_CLOSEST | CLK_DIVIDER_ALLOW_ZERO;
+		CLK_DIVIDER_ROUND_CLOSEST;
 
 	host->cfg_div_clk = devm_clk_register(host->dev, &host->cfg_div.hw);
 	if (WARN_ON(PTR_ERR_OR_ZERO(host->cfg_div_clk)))
-- 
2.9.4

[PATCH 03/14] mmc: meson-gx: clean up some constants

[Jerome Brunet]

Remove useless clock rate defines. These should not be defined but
equested from the clock framework.

Also correct typo on the DELAY register

Signed-off-by: Jerome Brunet <jbrunet@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 d480a8052a06..8a74a048db88 100644
--- a/drivers/mmc/host/meson-gx-mmc.c
+++ b/drivers/mmc/host/meson-gx-mmc.c
@@ -45,9 +45,7 @@
 #define   CLK_DIV_MAX 63
 #define   CLK_SRC_MASK GENMASK(7, 6)
 #define   CLK_SRC_XTAL 0   /* external crystal */
-#define   CLK_SRC_XTAL_RATE 24000000
 #define   CLK_SRC_PLL 1    /* FCLK_DIV2 */
-#define   CLK_SRC_PLL_RATE 1000000000
 #define   CLK_CORE_PHASE_MASK GENMASK(9, 8)
 #define   CLK_TX_PHASE_MASK GENMASK(11, 10)
 #define   CLK_RX_PHASE_MASK GENMASK(13, 12)
@@ -57,7 +55,7 @@
 #define   CLK_PHASE_270 3
 #define   CLK_ALWAYS_ON BIT(24)
 
-#define SD_EMMC_DElAY 0x4
+#define SD_EMMC_DELAY 0x4
 #define SD_EMMC_ADJUST 0x8
 #define SD_EMMC_CALOUT 0x10
 #define SD_EMMC_START 0x40
-- 
2.9.4

[PATCH 04/14] mmc: meson-gx: use _irqsave variant of spinlock

[Jerome Brunet]

spinlock used in interrupt handler should use the _irqsave variant

Fixes: 51c5d8447bd7 ("MMC: meson: initial support for GX platforms")
Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
---
 drivers/mmc/host/meson-gx-mmc.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/mmc/host/meson-gx-mmc.c b/drivers/mmc/host/meson-gx-mmc.c
index 8a74a048db88..a399fbd415f4 100644
--- a/drivers/mmc/host/meson-gx-mmc.c
+++ b/drivers/mmc/host/meson-gx-mmc.c
@@ -727,6 +727,7 @@ static irqreturn_t meson_mmc_irq(int irq, void *dev_id)
 	struct mmc_command *cmd;
 	struct mmc_data *data;
 	u32 irq_en, status, raw_status;
+	unsigned long flag;
 	irqreturn_t ret = IRQ_HANDLED;
 
 	if (WARN_ON(!host))
@@ -739,7 +740,7 @@ static irqreturn_t meson_mmc_irq(int irq, void *dev_id)
 
 	data = cmd->data;
 
-	spin_lock(&host->lock);
+	spin_lock_irqsave(&host->lock, flag);
 	irq_en = readl(host->regs + SD_EMMC_IRQ_EN);
 	raw_status = readl(host->regs + SD_EMMC_STATUS);
 	status = raw_status & irq_en;
@@ -806,7 +807,7 @@ static irqreturn_t meson_mmc_irq(int irq, void *dev_id)
 	if (ret == IRQ_HANDLED)
 		meson_mmc_request_done(host->mmc, cmd->mrq);
 
-	spin_unlock(&host->lock);
+	spin_unlock_irqrestore(&host->lock, flag);
 	return ret;
 }
 
-- 
2.9.4

[PATCH 05/14] mmc: meson-gx: cfg init overwrite values

[Jerome Brunet]

cfg init function overwrite values set in the clk init function
Remove the cfg pokes from the clk init. Actually, trying to use
the CLK_AUTO, like initially tried in clk_init, would break
the card initialization

BEWARE not to poke the cfg register while the divider value in clk
register is 0. It crashes the SoC.

Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
---
 drivers/mmc/host/meson-gx-mmc.c | 14 ++++----------
 1 file changed, 4 insertions(+), 10 deletions(-)

diff --git a/drivers/mmc/host/meson-gx-mmc.c b/drivers/mmc/host/meson-gx-mmc.c
index a399fbd415f4..61668891b4fc 100644
--- a/drivers/mmc/host/meson-gx-mmc.c
+++ b/drivers/mmc/host/meson-gx-mmc.c
@@ -337,7 +337,7 @@ static int meson_mmc_clk_init(struct meson_host *host)
 	int i, ret = 0;
 	const char *mux_parent_names[MUX_CLK_NUM_PARENTS];
 	const char *clk_div_parents[1];
-	u32 clk_reg, cfg;
+	u32 clk_reg;
 
 	/* get the mux parents */
 	for (i = 0; i < MUX_CLK_NUM_PARENTS; i++) {
@@ -403,12 +403,6 @@ static int meson_mmc_clk_init(struct meson_host *host)
 	clk_reg &= ~CLK_ALWAYS_ON;
 	writel(clk_reg, host->regs + SD_EMMC_CLOCK);
 
-	/* Ensure clock starts in "auto" mode, not "always on" */
-	cfg = readl(host->regs + SD_EMMC_CFG);
-	cfg &= ~CFG_CLK_ALWAYS_ON;
-	cfg |= CFG_AUTO_CLK;
-	writel(cfg, host->regs + SD_EMMC_CFG);
-
 	ret = clk_prepare_enable(host->cfg_div_clk);
 	if (ret)
 		return ret;
@@ -958,6 +952,9 @@ static int meson_mmc_probe(struct platform_device *pdev)
 	if (ret)
 		goto err_core_clk;
 
+	/* set config to sane default */
+	meson_mmc_cfg_init(host);
+
 	/* Stop execution */
 	writel(0, host->regs + SD_EMMC_START);
 
@@ -966,9 +963,6 @@ 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 = devm_request_threaded_irq(&pdev->dev, irq, meson_mmc_irq,
 					meson_mmc_irq_thread, IRQF_SHARED,
 					NULL, host);
-- 
2.9.4

[PATCH 06/14] mmc: meson-gx: rework set_ios function

[Jerome Brunet]

Remove conditional write of cfg register. Warn if set_clk fails for some
reason. Consistently use host->dev instead of mixing with mmc_dev(mmc)

Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
---
 drivers/mmc/host/meson-gx-mmc.c | 22 +++++++++-------------
 1 file changed, 9 insertions(+), 13 deletions(-)

diff --git a/drivers/mmc/host/meson-gx-mmc.c b/drivers/mmc/host/meson-gx-mmc.c
index 61668891b4fc..18fff28025d8 100644
--- a/drivers/mmc/host/meson-gx-mmc.c
+++ b/drivers/mmc/host/meson-gx-mmc.c
@@ -445,8 +445,8 @@ static void meson_mmc_set_tuning_params(struct mmc_host *mmc)
 static void meson_mmc_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
 {
 	struct meson_host *host = mmc_priv(mmc);
-	u32 bus_width;
-	u32 val, orig;
+	u32 bus_width, val;
+	int err;
 
 	/*
 	 * GPIO regulator, only controls switching between 1v8 and
@@ -474,7 +474,7 @@ static void meson_mmc_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
 			int ret = regulator_enable(mmc->supply.vqmmc);
 
 			if (ret < 0)
-				dev_err(mmc_dev(mmc),
+				dev_err(host->dev,
 					"failed to enable vqmmc regulator\n");
 			else
 				host->vqmmc_enabled = true;
@@ -483,9 +483,6 @@ static void meson_mmc_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
 		break;
 	}
 
-
-	meson_mmc_clk_set(host, ios->clock);
-
 	/* Bus width */
 	switch (ios->bus_width) {
 	case MMC_BUS_WIDTH_1:
@@ -504,8 +501,6 @@ static void meson_mmc_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
 	}
 
 	val = readl(host->regs + SD_EMMC_CFG);
-	orig = val;
-
 	val &= ~CFG_BUS_WIDTH_MASK;
 	val |= FIELD_PREP(CFG_BUS_WIDTH_MASK, bus_width);
 
@@ -519,11 +514,12 @@ 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;
 
-	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);
-	}
+	err = meson_mmc_clk_set(host, ios->clock);
+	if (err)
+		dev_err(host->dev, "Failed to set clock: %d\n,", err);
+
+	writel(val, host->regs + SD_EMMC_CFG);
+	dev_dbg(host->dev, "SD_EMMC_CFG:  0x%08x\n", val);
 }
 
 static void meson_mmc_request_done(struct mmc_host *mmc,
-- 
2.9.4

[PATCH 07/14] mmc: meson-gx: rework clk_set function

[Jerome Brunet]

Clean-up clk_set function to prepare the next changes (DDR and clk-stop)

Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
---
 drivers/mmc/host/meson-gx-mmc.c | 30 +++++++++---------------------
 1 file changed, 9 insertions(+), 21 deletions(-)

diff --git a/drivers/mmc/host/meson-gx-mmc.c b/drivers/mmc/host/meson-gx-mmc.c
index 18fff28025d8..8f9ba5190c18 100644
--- a/drivers/mmc/host/meson-gx-mmc.c
+++ b/drivers/mmc/host/meson-gx-mmc.c
@@ -139,7 +139,7 @@ struct meson_host {
 	struct clk *core_clk;
 	struct clk_mux mux;
 	struct clk *mux_clk;
-	unsigned long current_clock;
+	unsigned long req_rate;
 
 	struct clk_divider cfg_div;
 	struct clk *cfg_div_clk;
@@ -275,29 +275,18 @@ static int meson_mmc_clk_set(struct meson_host *host, unsigned long clk_rate)
 	int ret;
 	u32 cfg;
 
-	if (clk_rate) {
-		if (WARN_ON(clk_rate > mmc->f_max))
-			clk_rate = mmc->f_max;
-		else if (WARN_ON(clk_rate < mmc->f_min))
-			clk_rate = mmc->f_min;
-	}
-
-	if (clk_rate == host->current_clock)
+	/* Same request - bail-out */
+	if (host->req_rate == clk_rate)
 		return 0;
 
 	/* stop clock */
 	cfg = readl(host->regs + SD_EMMC_CFG);
-	if (!(cfg & CFG_STOP_CLOCK)) {
-		cfg |= CFG_STOP_CLOCK;
-		writel(cfg, host->regs + SD_EMMC_CFG);
-	}
-
-	dev_dbg(host->dev, "change clock rate %u -> %lu\n",
-		mmc->actual_clock, clk_rate);
+	cfg |= CFG_STOP_CLOCK;
+	writel(cfg, host->regs + SD_EMMC_CFG);
+	host->req_rate = 0;
 
 	if (!clk_rate) {
 		mmc->actual_clock = 0;
-		host->current_clock = 0;
 		/* return with clock being stopped */
 		return 0;
 	}
@@ -309,13 +298,12 @@ static int meson_mmc_clk_set(struct meson_host *host, unsigned long clk_rate)
 		return ret;
 	}
 
+	host->req_rate = clk_rate;
 	mmc->actual_clock = clk_get_rate(host->cfg_div_clk);
-	host->current_clock = clk_rate;
 
+	dev_dbg(host->dev, "clk rate: %u Hz\n", mmc->actual_clock);
 	if (clk_rate != mmc->actual_clock)
-		dev_dbg(host->dev,
-			"divider requested rate %lu != actual rate %u\n",
-			clk_rate, mmc->actual_clock);
+		dev_dbg(host->dev, "requested rate was %lu\n", clk_rate);
 
 	/* (re)start clock */
 	cfg = readl(host->regs + SD_EMMC_CFG);
-- 
2.9.4

[PATCH 08/14] mmc: meson-gx: rework clock init function

[Jerome Brunet]

Perform basic initialisation of the clk register before providing it to
the CCF.

Thanks to devm, carrying the clock structure around after init is not
necessary. Rework the function to remove these from the controller host
data.

Finally, set initial mmc clock rate before enabling it, simplifying the
exit condition.

Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
---
 drivers/mmc/host/meson-gx-mmc.c | 101 +++++++++++++++++++---------------------
 1 file changed, 49 insertions(+), 52 deletions(-)

diff --git a/drivers/mmc/host/meson-gx-mmc.c b/drivers/mmc/host/meson-gx-mmc.c
index 8f9ba5190c18..4cc7d6530536 100644
--- a/drivers/mmc/host/meson-gx-mmc.c
+++ b/drivers/mmc/host/meson-gx-mmc.c
@@ -42,10 +42,7 @@
 
 #define SD_EMMC_CLOCK 0x0
 #define   CLK_DIV_MASK GENMASK(5, 0)
-#define   CLK_DIV_MAX 63
 #define   CLK_SRC_MASK GENMASK(7, 6)
-#define   CLK_SRC_XTAL 0   /* external crystal */
-#define   CLK_SRC_PLL 1    /* FCLK_DIV2 */
 #define   CLK_CORE_PHASE_MASK GENMASK(9, 8)
 #define   CLK_TX_PHASE_MASK GENMASK(11, 10)
 #define   CLK_RX_PHASE_MASK GENMASK(13, 12)
@@ -137,13 +134,9 @@ struct meson_host {
 	spinlock_t lock;
 	void __iomem *regs;
 	struct clk *core_clk;
-	struct clk_mux mux;
-	struct clk *mux_clk;
+	struct clk *signal_clk;
 	unsigned long req_rate;
 
-	struct clk_divider cfg_div;
-	struct clk *cfg_div_clk;
-
 	unsigned int bounce_buf_size;
 	void *bounce_buf;
 	dma_addr_t bounce_dma_addr;
@@ -291,7 +284,7 @@ static int meson_mmc_clk_set(struct meson_host *host, unsigned long clk_rate)
 		return 0;
 	}
 
-	ret = clk_set_rate(host->cfg_div_clk, clk_rate);
+	ret = clk_set_rate(host->signal_clk, clk_rate);
 	if (ret) {
 		dev_err(host->dev, "Unable to set cfg_div_clk to %lu. ret=%d\n",
 			clk_rate, ret);
@@ -299,7 +292,7 @@ static int meson_mmc_clk_set(struct meson_host *host, unsigned long clk_rate)
 	}
 
 	host->req_rate = clk_rate;
-	mmc->actual_clock = clk_get_rate(host->cfg_div_clk);
+	mmc->actual_clock = clk_get_rate(host->signal_clk);
 
 	dev_dbg(host->dev, "clk rate: %u Hz\n", mmc->actual_clock);
 	if (clk_rate != mmc->actual_clock)
@@ -321,10 +314,13 @@ static int meson_mmc_clk_set(struct meson_host *host, unsigned long clk_rate)
 static int meson_mmc_clk_init(struct meson_host *host)
 {
 	struct clk_init_data init;
+	struct clk_mux *mux;
+	struct clk_divider *div;
+	struct clk *clk;
 	char clk_name[32];
 	int i, ret = 0;
 	const char *mux_parent_names[MUX_CLK_NUM_PARENTS];
-	const char *clk_div_parents[1];
+	const char *clk_parent[1];
 	u32 clk_reg;
 
 	/* get the mux parents */
@@ -343,6 +339,19 @@ static int meson_mmc_clk_init(struct meson_host *host)
 		mux_parent_names[i] = __clk_get_name(clk);
 	}
 
+	/* init SD_EMMC_CLOCK to sane defaults w/min clock rate */
+	clk_reg = 0;
+	clk_reg |= FIELD_PREP(CLK_CORE_PHASE_MASK, host->tp.core_phase);
+	clk_reg |= FIELD_PREP(CLK_TX_PHASE_MASK, host->tp.tx_phase);
+	clk_reg |= FIELD_PREP(CLK_RX_PHASE_MASK, host->tp.rx_phase);
+	clk_reg |= CLK_DIV_MASK;
+	clk_reg |= CLK_ALWAYS_ON;
+	writel(clk_reg, host->regs + SD_EMMC_CLOCK);
+
+	/* create the mux */
+	mux = devm_kzalloc(host->dev, sizeof(mux), GFP_KERNEL);
+	if (!mux)
+		return -ENOMEM;
 	/* create the mux */
 	snprintf(clk_name, sizeof(clk_name), "%s#mux", dev_name(host->dev));
 	init.name = clk_name;
@@ -350,59 +359,47 @@ static int meson_mmc_clk_init(struct meson_host *host)
 	init.flags = 0;
 	init.parent_names = mux_parent_names;
 	init.num_parents = MUX_CLK_NUM_PARENTS;
-	host->mux.reg = host->regs + SD_EMMC_CLOCK;
-	host->mux.shift = __bf_shf(CLK_SRC_MASK);
-	host->mux.mask = CLK_SRC_MASK >> host->mux.shift;
-	host->mux.flags = 0;
-	host->mux.table = NULL;
-	host->mux.hw.init = &init;
 
-	host->mux_clk = devm_clk_register(host->dev, &host->mux.hw);
-	if (WARN_ON(IS_ERR(host->mux_clk)))
-		return PTR_ERR(host->mux_clk);
+	mux->reg = host->regs + SD_EMMC_CLOCK;
+	mux->shift = __bf_shf(CLK_SRC_MASK);
+	mux->mask = CLK_SRC_MASK >> mux->shift;
+	mux->hw.init = &init;
+
+	clk = devm_clk_register(host->dev, &mux->hw);
+	if (WARN_ON(IS_ERR(clk)))
+		return PTR_ERR(clk);
 
 	/* create the divider */
+	div = devm_kzalloc(host->dev, sizeof(div), GFP_KERNEL);
+	if (!div)
+		return -ENOMEM;
+
 	snprintf(clk_name, sizeof(clk_name), "%s#div", dev_name(host->dev));
 	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);
-	init.parent_names = clk_div_parents;
-	init.num_parents = ARRAY_SIZE(clk_div_parents);
+	clk_parent[0] = __clk_get_name(clk);
+	init.parent_names = clk_parent;
+	init.num_parents = 1;
 
-	host->cfg_div.reg = host->regs + SD_EMMC_CLOCK;
-	host->cfg_div.shift = __bf_shf(CLK_DIV_MASK);
-	host->cfg_div.width = __builtin_popcountl(CLK_DIV_MASK);
-	host->cfg_div.hw.init = &init;
-	host->cfg_div.flags = CLK_DIVIDER_ONE_BASED |
-		CLK_DIVIDER_ROUND_CLOSEST;
+	div->reg = host->regs + SD_EMMC_CLOCK;
+	div->shift = __bf_shf(CLK_DIV_MASK);
+	div->width = __builtin_popcountl(CLK_DIV_MASK);
+	div->hw.init = &init;
+	div->flags = (CLK_DIVIDER_ONE_BASED |
+		      CLK_DIVIDER_ROUND_CLOSEST);
 
-	host->cfg_div_clk = devm_clk_register(host->dev, &host->cfg_div.hw);
-	if (WARN_ON(PTR_ERR_OR_ZERO(host->cfg_div_clk)))
-		return PTR_ERR(host->cfg_div_clk);
+	host->signal_clk = devm_clk_register(host->dev, &div->hw);
+	if (WARN_ON(PTR_ERR_OR_ZERO(host->signal_clk)))
+		return PTR_ERR(host->signal_clk);
 
 	/* init SD_EMMC_CLOCK to sane defaults w/min clock rate */
-	clk_reg = 0;
-	clk_reg |= FIELD_PREP(CLK_CORE_PHASE_MASK, host->tp.core_phase);
-	clk_reg |= FIELD_PREP(CLK_TX_PHASE_MASK, host->tp.tx_phase);
-	clk_reg |= FIELD_PREP(CLK_RX_PHASE_MASK, host->tp.rx_phase);
-	clk_reg |= FIELD_PREP(CLK_SRC_MASK, CLK_SRC_XTAL);
-	clk_reg |= FIELD_PREP(CLK_DIV_MASK, CLK_DIV_MAX);
-	clk_reg &= ~CLK_ALWAYS_ON;
-	writel(clk_reg, host->regs + SD_EMMC_CLOCK);
-
-	ret = clk_prepare_enable(host->cfg_div_clk);
+	host->mmc->f_min = clk_round_rate(host->signal_clk, 400000);
+	ret = clk_set_rate(host->signal_clk, host->mmc->f_min);
 	if (ret)
 		return ret;
 
-	/* 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 clk_prepare_enable(host->signal_clk);
 }
 
 static void meson_mmc_set_tuning_params(struct mmc_host *mmc)
@@ -987,7 +984,7 @@ static int meson_mmc_probe(struct platform_device *pdev)
 	dma_free_coherent(host->dev, host->bounce_buf_size,
 			  host->bounce_buf, host->bounce_dma_addr);
 err_div_clk:
-	clk_disable_unprepare(host->cfg_div_clk);
+	clk_disable_unprepare(host->signal_clk);
 err_core_clk:
 	clk_disable_unprepare(host->core_clk);
 free_host:
@@ -1009,7 +1006,7 @@ static int meson_mmc_remove(struct platform_device *pdev)
 	dma_free_coherent(host->dev, host->bounce_buf_size,
 			  host->bounce_buf, host->bounce_dma_addr);
 
-	clk_disable_unprepare(host->cfg_div_clk);
+	clk_disable_unprepare(host->signal_clk);
 	clk_disable_unprepare(host->core_clk);
 
 	mmc_free_host(host->mmc);
-- 
2.9.4

[PATCH 09/14] mmc: meson-gx: simplify interrupt handler

[Jerome Brunet]

No functional change, just improve interrupt handler readability

Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
---
 drivers/mmc/host/meson-gx-mmc.c | 93 +++++++++++++++++------------------------
 1 file changed, 39 insertions(+), 54 deletions(-)

diff --git a/drivers/mmc/host/meson-gx-mmc.c b/drivers/mmc/host/meson-gx-mmc.c
index 4cc7d6530536..d876b80db27d 100644
--- a/drivers/mmc/host/meson-gx-mmc.c
+++ b/drivers/mmc/host/meson-gx-mmc.c
@@ -78,16 +78,22 @@
 #define   STATUS_BUSY BIT(31)
 
 #define SD_EMMC_IRQ_EN 0x4c
-#define   IRQ_EN_MASK GENMASK(13, 0)
 #define   IRQ_RXD_ERR_MASK GENMASK(7, 0)
 #define   IRQ_TXD_ERR BIT(8)
 #define   IRQ_DESC_ERR BIT(9)
 #define   IRQ_RESP_ERR BIT(10)
+#define   IRQ_CRC_ERR \
+	(IRQ_RXD_ERR_MASK | IRQ_TXD_ERR | IRQ_DESC_ERR | IRQ_RESP_ERR)
 #define   IRQ_RESP_TIMEOUT BIT(11)
 #define   IRQ_DESC_TIMEOUT BIT(12)
+#define   IRQ_TIMEOUTS \
+	(IRQ_RESP_TIMEOUT | IRQ_DESC_TIMEOUT)
 #define   IRQ_END_OF_CHAIN BIT(13)
 #define   IRQ_RESP_STATUS BIT(14)
 #define   IRQ_SDIO BIT(15)
+#define   IRQ_EN_MASK \
+	(IRQ_CRC_ERR | IRQ_TIMEOUTS | IRQ_END_OF_CHAIN | IRQ_RESP_STATUS |\
+	 IRQ_SDIO)
 
 #define SD_EMMC_CMD_CFG 0x50
 #define SD_EMMC_CMD_ARG 0x54
@@ -703,57 +709,40 @@ static irqreturn_t meson_mmc_irq(int irq, void *dev_id)
 	struct mmc_data *data;
 	u32 irq_en, status, raw_status;
 	unsigned long flag;
-	irqreturn_t ret = IRQ_HANDLED;
+	irqreturn_t ret = IRQ_NONE;
 
-	if (WARN_ON(!host))
+	if (WARN_ON(!host) || WARN_ON(!host->cmd))
 		return IRQ_NONE;
 
-	cmd = host->cmd;
-
-	if (WARN_ON(!cmd))
-		return IRQ_NONE;
+	spin_lock_irqsave(&host->lock, flag);
 
+	cmd = host->cmd;
 	data = cmd->data;
-
-	spin_lock_irqsave(&host->lock, flag);
 	irq_en = readl(host->regs + SD_EMMC_IRQ_EN);
 	raw_status = readl(host->regs + SD_EMMC_STATUS);
 	status = raw_status & irq_en;
 
-	if (!status) {
-		dev_warn(host->dev, "Spurious IRQ! status=0x%08x, irq_en=0x%08x\n",
-			 raw_status, irq_en);
-		ret = IRQ_NONE;
-		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");
-		cmd->error = -EILSEQ;
-	}
-	if (status & IRQ_TXD_ERR) {
-		dev_dbg(host->dev, "Unhandled IRQ: TXD error\n");
-		cmd->error = -EILSEQ;
-	}
-	if (status & IRQ_DESC_ERR)
-		dev_dbg(host->dev, "Unhandled IRQ: Descriptor error\n");
-	if (status & IRQ_RESP_ERR) {
-		dev_dbg(host->dev, "Unhandled IRQ: Response error\n");
+	if (status & IRQ_CRC_ERR) {
+		dev_dbg(host->dev, "CRC Error - status 0x%08x\n", status);
 		cmd->error = -EILSEQ;
+		ret = IRQ_HANDLED;
+		goto out;
 	}
-	if (status & IRQ_RESP_TIMEOUT) {
-		dev_dbg(host->dev, "Unhandled IRQ: Response timeout\n");
+
+	if (status & IRQ_TIMEOUTS) {
+		dev_dbg(host->dev, "Timeout - status 0x%08x\n", status);
 		cmd->error = -ETIMEDOUT;
+		ret = IRQ_HANDLED;
+		goto out;
 	}
-	if (status & IRQ_DESC_TIMEOUT) {
-		dev_dbg(host->dev, "Unhandled IRQ: Descriptor timeout\n");
-		cmd->error = -ETIMEDOUT;
+
+	meson_mmc_read_resp(host->mmc, cmd);
+
+	if (status & IRQ_SDIO) {
+		dev_dbg(host->dev, "IRQ: SDIO TODO.\n");
+		ret = IRQ_HANDLED;
 	}
-	if (status & IRQ_SDIO)
-		dev_dbg(host->dev, "Unhandled IRQ: SDIO.\n");
 
 	if (status & (IRQ_END_OF_CHAIN | IRQ_RESP_STATUS)) {
 		if (data && !cmd->error)
@@ -761,26 +750,20 @@ static irqreturn_t meson_mmc_irq(int irq, void *dev_id)
 		if (meson_mmc_bounce_buf_read(data) ||
 		    meson_mmc_get_next_command(cmd))
 			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);
-		if (cmd->data) {
-			struct mmc_data *data = cmd->data;
-
-			dev_warn(host->dev, "\tblksz %u blocks %u flags 0x%08x (%s%s)",
-				 data->blksz, data->blocks, data->flags,
-				 data->flags & MMC_DATA_WRITE ? "write" : "",
-				 data->flags & MMC_DATA_READ ? "read" : "");
-		}
+		else
+			ret = IRQ_HANDLED;
 	}
 
 out:
-	/* ack all (enabled) interrupts */
-	writel(status, host->regs + SD_EMMC_STATUS);
+	/* ack all enabled interrupts */
+	writel(irq_en, host->regs + SD_EMMC_STATUS);
 
 	if (ret == IRQ_HANDLED)
 		meson_mmc_request_done(host->mmc, cmd->mrq);
+	else if (ret == IRQ_NONE)
+		dev_warn(host->dev,
+			 "Unexpected IRQ! status=0x%08x, irq_en=0x%08x\n",
+			 raw_status, irq_en);
 
 	spin_unlock_irqrestore(&host->lock, flag);
 	return ret;
@@ -939,10 +922,12 @@ static int meson_mmc_probe(struct platform_device *pdev)
 	/* Stop execution */
 	writel(0, host->regs + SD_EMMC_START);
 
-	/* clear, ack, enable all interrupts */
+	/* clear, ack and enable interrupts */
 	writel(0, host->regs + SD_EMMC_IRQ_EN);
-	writel(IRQ_EN_MASK, host->regs + SD_EMMC_STATUS);
-	writel(IRQ_EN_MASK, host->regs + SD_EMMC_IRQ_EN);
+	writel(IRQ_CRC_ERR | IRQ_TIMEOUTS | IRQ_END_OF_CHAIN,
+	       host->regs + SD_EMMC_STATUS);
+	writel(IRQ_CRC_ERR | IRQ_TIMEOUTS | IRQ_END_OF_CHAIN,
+	       host->regs + SD_EMMC_IRQ_EN);
 
 	ret = devm_request_threaded_irq(&pdev->dev, irq, meson_mmc_irq,
 					meson_mmc_irq_thread, IRQF_SHARED,
-- 
2.9.4

[PATCH 10/14] mmc: meson-gx: implement card_busy callback

[Jerome Brunet]

The card_busy callback is important to then add the voltage switch
callback as it allow to verify that the card is done dealing with
the voltage switch

Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
---
 drivers/mmc/host/meson-gx-mmc.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/drivers/mmc/host/meson-gx-mmc.c b/drivers/mmc/host/meson-gx-mmc.c
index d876b80db27d..33ab341a8b33 100644
--- a/drivers/mmc/host/meson-gx-mmc.c
+++ b/drivers/mmc/host/meson-gx-mmc.c
@@ -76,6 +76,7 @@
 
 #define SD_EMMC_STATUS 0x48
 #define   STATUS_BUSY BIT(31)
+#define   STATUS_DATI GENMASK(23, 16)
 
 #define SD_EMMC_IRQ_EN 0x4c
 #define   IRQ_RXD_ERR_MASK GENMASK(7, 0)
@@ -845,6 +846,17 @@ static void meson_mmc_cfg_init(struct meson_host *host)
 	writel(cfg, host->regs + SD_EMMC_CFG);
 }
 
+static int meson_mmc_card_busy(struct mmc_host *mmc)
+{
+	struct meson_host *host = mmc_priv(mmc);
+	u32 regval;
+
+	regval = readl(host->regs + SD_EMMC_STATUS);
+
+	/* We are only interrested in lines 0 to 3, so mask the other ones */
+	return !(FIELD_GET(STATUS_DATI, regval) & 0xf);
+}
+
 static const struct mmc_host_ops meson_mmc_ops = {
 	.request	= meson_mmc_request,
 	.set_ios	= meson_mmc_set_ios,
@@ -852,6 +864,7 @@ static const struct mmc_host_ops meson_mmc_ops = {
 	.pre_req	= meson_mmc_pre_req,
 	.post_req	= meson_mmc_post_req,
 	.execute_tuning = meson_mmc_execute_tuning,
+	.card_busy	= meson_mmc_card_busy,
 };
 
 static int meson_mmc_probe(struct platform_device *pdev)
-- 
2.9.4

[PATCH 11/14] mmc: meson-gx: rework tuning function

[Jerome Brunet]

Rework tuning function of the rx phase, with the addition of rx delay.
This allow a more fine setting of the rx phase, which allows to use
new modes such as SDR50.

Also, use 270 degree Tx phase as it make eMMC DDR52 mode functional on
the libretech-cc, without any regression so far.

Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
---
 drivers/mmc/host/meson-gx-mmc.c | 215 ++++++++++++++++++++++++++++++----------
 1 file changed, 160 insertions(+), 55 deletions(-)

diff --git a/drivers/mmc/host/meson-gx-mmc.c b/drivers/mmc/host/meson-gx-mmc.c
index 33ab341a8b33..560de8faea50 100644
--- a/drivers/mmc/host/meson-gx-mmc.c
+++ b/drivers/mmc/host/meson-gx-mmc.c
@@ -50,6 +50,13 @@
 #define   CLK_PHASE_90 1
 #define   CLK_PHASE_180 2
 #define   CLK_PHASE_270 3
+#define   CLK_PHASE_NUM 4
+#define   CLK_TX_DELAY_MASK GENMASK(19, 16)
+#define   CLK_RX_DELAY_MASK GENMASK(23, 20)
+#define   CLK_DELAY_STEP_PS 200
+#define   CLK_PHASE_STEP 30
+#define   CLK_PHASE_POINT_NUM (360 / CLK_PHASE_STEP)
+#define   CLK_DELAY_MAX 0xf
 #define   CLK_ALWAYS_ON BIT(24)
 
 #define SD_EMMC_DELAY 0x4
@@ -120,12 +127,6 @@
 
 #define MUX_CLK_NUM_PARENTS 2
 
-struct meson_tuning_params {
-	u8 core_phase;
-	u8 tx_phase;
-	u8 rx_phase;
-};
-
 struct sd_emmc_desc {
 	u32 cmd_cfg;
 	u32 cmd_arg;
@@ -150,7 +151,6 @@ struct meson_host {
 	struct sd_emmc_desc *descs;
 	dma_addr_t descs_dma_addr;
 
-	struct meson_tuning_params tp;
 	bool vqmmc_enabled;
 };
 
@@ -269,6 +269,35 @@ static void meson_mmc_post_req(struct mmc_host *mmc, struct mmc_request *mrq,
 			     mmc_get_dma_dir(data));
 }
 
+static void meson_mmc_clk_phase_dflt(struct meson_host *host)
+{
+	u32 val;
+
+	val = readl(host->regs + SD_EMMC_CLOCK);
+	val &= ~(CLK_CORE_PHASE_MASK | CLK_TX_PHASE_MASK | CLK_RX_PHASE_MASK |
+		 CLK_TX_DELAY_MASK | CLK_RX_DELAY_MASK);
+
+	/*
+	 * Set phases : These values are mostly the datasheet recommended ones
+	 * except for the Tx phase. Datasheet recommends 180 but DDR52 mode
+	 * does not work with that value. 270 works just fine, w/o any
+	 * regression on the other modes so far.
+	 *
+	 * At this point, cylcing on the Tx phase in the tuning function does
+	 * not seems necessary. We may add it later on, if some mode high speed
+	 * mode needs different Tx phase.
+	 */
+	val |= FIELD_PREP(CLK_CORE_PHASE_MASK, CLK_PHASE_180);
+	val |= FIELD_PREP(CLK_TX_PHASE_MASK, CLK_PHASE_270);
+	val |= FIELD_PREP(CLK_RX_PHASE_MASK, CLK_PHASE_0);
+
+	/* Reset delays */
+	val |= FIELD_PREP(CLK_TX_DELAY_MASK, 0);
+	val |= FIELD_PREP(CLK_RX_DELAY_MASK, 0);
+
+	writel(val, host->regs + SD_EMMC_CLOCK);
+}
+
 static int meson_mmc_clk_set(struct meson_host *host, unsigned long clk_rate)
 {
 	struct mmc_host *mmc = host->mmc;
@@ -348,9 +377,6 @@ static int meson_mmc_clk_init(struct meson_host *host)
 
 	/* init SD_EMMC_CLOCK to sane defaults w/min clock rate */
 	clk_reg = 0;
-	clk_reg |= FIELD_PREP(CLK_CORE_PHASE_MASK, host->tp.core_phase);
-	clk_reg |= FIELD_PREP(CLK_TX_PHASE_MASK, host->tp.tx_phase);
-	clk_reg |= FIELD_PREP(CLK_RX_PHASE_MASK, host->tp.rx_phase);
 	clk_reg |= CLK_DIV_MASK;
 	clk_reg |= CLK_ALWAYS_ON;
 	writel(clk_reg, host->regs + SD_EMMC_CLOCK);
@@ -409,31 +435,6 @@ static int meson_mmc_clk_init(struct meson_host *host)
 	return clk_prepare_enable(host->signal_clk);
 }
 
-static void meson_mmc_set_tuning_params(struct mmc_host *mmc)
-{
-	struct meson_host *host = mmc_priv(mmc);
-	u32 regval;
-
-	/* stop clock */
-	regval = readl(host->regs + SD_EMMC_CFG);
-	regval |= CFG_STOP_CLOCK;
-	writel(regval, host->regs + SD_EMMC_CFG);
-
-	regval = readl(host->regs + SD_EMMC_CLOCK);
-	regval &= ~CLK_CORE_PHASE_MASK;
-	regval |= FIELD_PREP(CLK_CORE_PHASE_MASK, host->tp.core_phase);
-	regval &= ~CLK_TX_PHASE_MASK;
-	regval |= FIELD_PREP(CLK_TX_PHASE_MASK, host->tp.tx_phase);
-	regval &= ~CLK_RX_PHASE_MASK;
-	regval |= FIELD_PREP(CLK_RX_PHASE_MASK, host->tp.rx_phase);
-	writel(regval, host->regs + SD_EMMC_CLOCK);
-
-	/* start clock */
-	regval = readl(host->regs + SD_EMMC_CFG);
-	regval &= ~CFG_STOP_CLOCK;
-	writel(regval, host->regs + SD_EMMC_CFG);
-}
-
 static void meson_mmc_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
 {
 	struct meson_host *host = mmc_priv(mmc);
@@ -472,6 +473,7 @@ static void meson_mmc_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
 				host->vqmmc_enabled = true;
 		}
 
+		meson_mmc_clk_phase_dflt(host);
 		break;
 	}
 
@@ -797,27 +799,134 @@ static irqreturn_t meson_mmc_irq_thread(int irq, void *dev_id)
 	return IRQ_HANDLED;
 }
 
+static void meson_mmc_apply_rx_phase_delay(struct meson_host *host,
+					   unsigned int phase,
+					   unsigned int delay)
+{
+	u32 val;
+
+	val = readl(host->regs + SD_EMMC_CLOCK);
+	val &= ~(CLK_RX_PHASE_MASK | CLK_RX_DELAY_MASK);
+	val |= FIELD_PREP(CLK_RX_PHASE_MASK, phase);
+	val |= FIELD_PREP(CLK_RX_DELAY_MASK, delay);
+	writel(val, host->regs + SD_EMMC_CLOCK);
+}
+
+
+static void meson_mmc_set_rx_phase(struct meson_host *host,
+				   unsigned long period_ps,
+				   unsigned int phase)
+{
+	uint64_t p;
+	unsigned long r, d;
+
+	/*
+	 * First compute the phase index (p), the remainder (r) is the part
+	 * we'll try to acheive using 200 ps delays (d).
+	 */
+	p = (phase % 360);
+	r = do_div(p, (360 / CLK_PHASE_NUM));
+	d = DIV_ROUND_CLOSEST((r * period_ps), (CLK_DELAY_STEP_PS * 360));
+
+	if (d > CLK_DELAY_MAX)
+		d = CLK_DELAY_MAX;
+
+	meson_mmc_apply_rx_phase_delay(host, p, d);
+}
+
+static void meson_mmc_shift_map(unsigned long *map, unsigned long shift)
+{
+	DECLARE_BITMAP(left, CLK_PHASE_POINT_NUM);
+	DECLARE_BITMAP(right, CLK_PHASE_POINT_NUM);
+
+	/*
+	 * shift the bitmap right and reintroduce the dropped bits on the left
+	 * of the bitmap
+	 */
+	bitmap_shift_right(right, map, shift, CLK_PHASE_POINT_NUM);
+	bitmap_shift_left(left, map, CLK_PHASE_POINT_NUM - shift,
+			  CLK_PHASE_POINT_NUM);
+	bitmap_or(map, left, right, CLK_PHASE_POINT_NUM);
+}
+
+static void meson_mmc_find_next_region(unsigned long *map,
+				       unsigned long *start,
+				       unsigned long *stop)
+{
+	*start = find_next_bit(map, CLK_PHASE_POINT_NUM, *start);
+	*stop = find_next_zero_bit(map, CLK_PHASE_POINT_NUM, *start);
+}
+
+static int meson_mmc_find_tuning_point(unsigned long *test)
+{
+	unsigned long shift, stop, offset = 0, start = 0, size = 0;
+
+	/* Get the all good/all bad situation out the way */
+	if (bitmap_full(test, CLK_PHASE_POINT_NUM))
+		return 0; /* All points are good so point 0 will do */
+	else if (bitmap_empty(test, CLK_PHASE_POINT_NUM))
+		return -EINVAL; /* No successful tuning point */
+
+	/*
+	 * Now we know there is a least one region find. Make sure it does
+	 * not wrap by the shifting the bitmap if necessary
+	 */
+	shift = find_first_zero_bit(test, CLK_PHASE_POINT_NUM);
+	if (shift != 0)
+		meson_mmc_shift_map(test, shift);
+
+	while (start < CLK_PHASE_POINT_NUM) {
+		meson_mmc_find_next_region(test, &start, &stop);
+
+		if ((stop - start) > size) {
+			offset = start;
+			size = stop - start;
+		}
+
+		start = stop;
+	}
+
+	/* Get the center point of the region */
+	offset += (size / 2);
+
+	/* Shift the result back */
+	offset = (offset + shift) % CLK_PHASE_POINT_NUM;
+
+	return offset;
+}
+
 static int meson_mmc_execute_tuning(struct mmc_host *mmc, u32 opcode)
 {
 	struct meson_host *host = mmc_priv(mmc);
-	struct meson_tuning_params tp_old = host->tp;
-	int ret = -EINVAL, i, cmd_error;
-
-	dev_info(mmc_dev(mmc), "(re)tuning...\n");
-
-	for (i = CLK_PHASE_0; i <= CLK_PHASE_270; i++) {
-		host->tp.rx_phase = i;
-		/* exclude the active parameter set if retuning */
-		if (!memcmp(&tp_old, &host->tp, sizeof(tp_old)) &&
-		    mmc->doing_retune)
-			continue;
-		meson_mmc_set_tuning_params(mmc);
-		ret = mmc_send_tuning(mmc, opcode, &cmd_error);
-		if (!ret)
-			break;
+	int point, cmd_error;
+	unsigned long period_ps;
+	DECLARE_BITMAP(test, CLK_PHASE_POINT_NUM);
+
+	dev_dbg(mmc_dev(mmc), "(re)tuning rx phase/delay...\n");
+	meson_mmc_clk_phase_dflt(host);
+
+	/* Get period */
+	period_ps = DIV_ROUND_UP((unsigned long)NSEC_PER_SEC * 1000,
+				 clk_get_rate(host->signal_clk));
+
+	/* Explore tuning points */
+	for (point = 0; point < CLK_PHASE_POINT_NUM; point++) {
+		meson_mmc_set_rx_phase(host, period_ps, point * CLK_PHASE_STEP);
+		if (!mmc_send_tuning(mmc, opcode, &cmd_error))
+			set_bit(point, test);
 	}
 
-	return ret;
+	/* Find the optimal tuning point and apply it */
+	point = meson_mmc_find_tuning_point(test);
+	if (point < 0) {
+		dev_warn(mmc_dev(mmc), "phase/delay tuning failed\n");
+		return point;
+	}
+
+	dev_dbg(mmc_dev(mmc), "tuning successful: point %d\n", point);
+	meson_mmc_set_rx_phase(host, period_ps, point * CLK_PHASE_STEP);
+
+	return 0;
 }
 
 /*
@@ -921,10 +1030,6 @@ static int meson_mmc_probe(struct platform_device *pdev)
 	if (ret)
 		goto free_host;
 
-	host->tp.core_phase = CLK_PHASE_180;
-	host->tp.tx_phase = CLK_PHASE_0;
-	host->tp.rx_phase = CLK_PHASE_0;
-
 	ret = meson_mmc_clk_init(host);
 	if (ret)
 		goto err_core_clk;
-- 
2.9.4

[PATCH 12/14] mmc: meson-gx: fix dual data rate mode frequencies

[Jerome Brunet]

In DDR modes, meson mmc controller requires an input rate twice as fast
as the output rate

Fixes: 51c5d8447bd7 ("MMC: meson: initial support for GX platforms")
Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
---
 drivers/mmc/host/meson-gx-mmc.c | 41 +++++++++++++++++++++++++++++------------
 1 file changed, 29 insertions(+), 12 deletions(-)

diff --git a/drivers/mmc/host/meson-gx-mmc.c b/drivers/mmc/host/meson-gx-mmc.c
index 560de8faea50..f973278a3f8d 100644
--- a/drivers/mmc/host/meson-gx-mmc.c
+++ b/drivers/mmc/host/meson-gx-mmc.c
@@ -298,14 +298,29 @@ static void meson_mmc_clk_phase_dflt(struct meson_host *host)
 	writel(val, host->regs + SD_EMMC_CLOCK);
 }
 
-static int meson_mmc_clk_set(struct meson_host *host, unsigned long clk_rate)
+static bool meson_mmc_timing_is_ddr(struct mmc_ios *ios)
+{
+	if (ios->timing == MMC_TIMING_MMC_DDR52 ||
+	    ios->timing == MMC_TIMING_UHS_DDR50 ||
+	    ios->timing == MMC_TIMING_MMC_HS400)
+		return true;
+
+	return false;
+}
+
+static int meson_mmc_clk_set(struct meson_host *host, struct mmc_ios *ios)
 {
 	struct mmc_host *mmc = host->mmc;
+	unsigned long rate = ios->clock;
 	int ret;
 	u32 cfg;
 
+	/* DDR modes require higher module clock */
+	if (meson_mmc_timing_is_ddr(ios))
+		rate <<= 1;
+
 	/* Same request - bail-out */
-	if (host->req_rate == clk_rate)
+	if (host->req_rate == rate)
 		return 0;
 
 	/* stop clock */
@@ -314,25 +329,29 @@ static int meson_mmc_clk_set(struct meson_host *host, unsigned long clk_rate)
 	writel(cfg, host->regs + SD_EMMC_CFG);
 	host->req_rate = 0;
 
-	if (!clk_rate) {
+	if (!rate) {
 		mmc->actual_clock = 0;
 		/* return with clock being stopped */
 		return 0;
 	}
 
-	ret = clk_set_rate(host->signal_clk, clk_rate);
+	ret = clk_set_rate(host->signal_clk, rate);
 	if (ret) {
 		dev_err(host->dev, "Unable to set cfg_div_clk to %lu. ret=%d\n",
-			clk_rate, ret);
+			rate, ret);
 		return ret;
 	}
 
-	host->req_rate = clk_rate;
+	host->req_rate = rate;
 	mmc->actual_clock = clk_get_rate(host->signal_clk);
 
+	/* We should report the real output frequency of the controller */
+	if (meson_mmc_timing_is_ddr(ios))
+		mmc->actual_clock >>= 1;
+
 	dev_dbg(host->dev, "clk rate: %u Hz\n", mmc->actual_clock);
-	if (clk_rate != mmc->actual_clock)
-		dev_dbg(host->dev, "requested rate was %lu\n", clk_rate);
+	if (ios->clock != mmc->actual_clock)
+		dev_dbg(host->dev, "requested rate was %u\n", ios->clock);
 
 	/* (re)start clock */
 	cfg = readl(host->regs + SD_EMMC_CFG);
@@ -499,16 +518,14 @@ static void meson_mmc_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
 	val |= FIELD_PREP(CFG_BUS_WIDTH_MASK, bus_width);
 
 	val &= ~CFG_DDR;
-	if (ios->timing == MMC_TIMING_UHS_DDR50 ||
-	    ios->timing == MMC_TIMING_MMC_DDR52 ||
-	    ios->timing == MMC_TIMING_MMC_HS400)
+	if (meson_mmc_timing_is_ddr(ios))
 		val |= CFG_DDR;
 
 	val &= ~CFG_CHK_DS;
 	if (ios->timing == MMC_TIMING_MMC_HS400)
 		val |= CFG_CHK_DS;
 
-	err = meson_mmc_clk_set(host, ios->clock);
+	err = meson_mmc_clk_set(host, ios);
 	if (err)
 		dev_err(host->dev, "Failed to set clock: %d\n,", err);
 
-- 
2.9.4

[PATCH 13/14] mmc: meson-gx: work around clk-stop issue

[Jerome Brunet]

It seems that the signal clock is also used and required, somehow, by
the controller it self.

It is shown during init, when writing to CFG while the divider is set
to 0 will crash the SoC. During voltage switch, the controller may crash
and the card may then fail to exit busy state if the clock is stopped.

To avoid this, it is best to keep the clock running for the controller,
except during rate change. However, we still need to be able to gate
the clock out of the SoC. Let's use the pinmux for this, and fallback
to gpio mode (pulled-down) when we need to gate the clock

Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
---
 drivers/mmc/host/meson-gx-mmc.c | 74 +++++++++++++++++++++++++++++++++++++----
 1 file changed, 68 insertions(+), 6 deletions(-)

diff --git a/drivers/mmc/host/meson-gx-mmc.c b/drivers/mmc/host/meson-gx-mmc.c
index f973278a3f8d..e991c9e452ab 100644
--- a/drivers/mmc/host/meson-gx-mmc.c
+++ b/drivers/mmc/host/meson-gx-mmc.c
@@ -145,6 +145,10 @@ struct meson_host {
 	struct clk *signal_clk;
 	unsigned long req_rate;
 
+	struct pinctrl *pinctrl;
+	struct pinctrl_state *pins_default;
+	struct pinctrl_state *pins_clk_gate;
+
 	unsigned int bounce_buf_size;
 	void *bounce_buf;
 	dma_addr_t bounce_dma_addr;
@@ -308,6 +312,42 @@ static bool meson_mmc_timing_is_ddr(struct mmc_ios *ios)
 	return false;
 }
 
+/*
+ * Gating the clock on this controller is tricky.  It seems the signal clock
+ * is also used by the controller.  It may crash during some operation if the
+ * clock is stopped.  The safest thing to do, whenever possible, is to keep
+ * clock running at stop it at the pad using the pinmux.
+ */
+static void meson_mmc_clk_gate(struct meson_host *host)
+{
+	u32 cfg;
+
+	if (host->pins_clk_gate) {
+		pinctrl_select_state(host->pinctrl, host->pins_clk_gate);
+	} else {
+		/*
+		 * If the pinmux is not provided - default to the classic and
+		 * unsafe method
+		 */
+		cfg = readl(host->regs + SD_EMMC_CFG);
+		cfg |= CFG_STOP_CLOCK;
+		writel(cfg, host->regs + SD_EMMC_CFG);
+	}
+}
+
+static void meson_mmc_clk_ungate(struct meson_host *host)
+{
+	u32 cfg;
+
+	if (host->pins_clk_gate)
+		pinctrl_select_state(host->pinctrl, host->pins_default);
+
+	/* Make sure the clock is not stopped in the controller */
+	cfg = readl(host->regs + SD_EMMC_CFG);
+	cfg &= ~CFG_STOP_CLOCK;
+	writel(cfg, host->regs + SD_EMMC_CFG);
+}
+
 static int meson_mmc_clk_set(struct meson_host *host, struct mmc_ios *ios)
 {
 	struct mmc_host *mmc = host->mmc;
@@ -324,9 +364,7 @@ static int meson_mmc_clk_set(struct meson_host *host, struct mmc_ios *ios)
 		return 0;
 
 	/* stop clock */
-	cfg = readl(host->regs + SD_EMMC_CFG);
-	cfg |= CFG_STOP_CLOCK;
-	writel(cfg, host->regs + SD_EMMC_CFG);
+	meson_mmc_clk_gate(host);
 	host->req_rate = 0;
 
 	if (!rate) {
@@ -335,6 +373,11 @@ static int meson_mmc_clk_set(struct meson_host *host, struct mmc_ios *ios)
 		return 0;
 	}
 
+	/* Stop the clock during rate change to avoid glitches */
+	cfg = readl(host->regs + SD_EMMC_CFG);
+	cfg |= CFG_STOP_CLOCK;
+	writel(cfg, host->regs + SD_EMMC_CFG);
+
 	ret = clk_set_rate(host->signal_clk, rate);
 	if (ret) {
 		dev_err(host->dev, "Unable to set cfg_div_clk to %lu. ret=%d\n",
@@ -354,9 +397,7 @@ static int meson_mmc_clk_set(struct meson_host *host, struct mmc_ios *ios)
 		dev_dbg(host->dev, "requested rate was %u\n", ios->clock);
 
 	/* (re)start clock */
-	cfg = readl(host->regs + SD_EMMC_CFG);
-	cfg &= ~CFG_STOP_CLOCK;
-	writel(cfg, host->regs + SD_EMMC_CFG);
+	meson_mmc_clk_ungate(host);
 
 	return 0;
 }
@@ -1037,6 +1078,27 @@ static int meson_mmc_probe(struct platform_device *pdev)
 		goto free_host;
 	}
 
+	host->pinctrl = devm_pinctrl_get(&pdev->dev);
+	if (IS_ERR(host->pinctrl)) {
+		ret = PTR_ERR(host->pinctrl);
+		goto free_host;
+	}
+
+	host->pins_default = pinctrl_lookup_state(host->pinctrl,
+						  PINCTRL_STATE_DEFAULT);
+	if (IS_ERR(host->pins_default)) {
+		ret = PTR_ERR(host->pins_default);
+		goto free_host;
+	}
+
+	host->pins_clk_gate = pinctrl_lookup_state(host->pinctrl,
+					     "clk-gate");
+	if (IS_ERR(host->pins_clk_gate)) {
+		dev_warn(&pdev->dev,
+			 "can't get clk-gate pinctrl, using clk_stop bit\n");
+		host->pins_clk_gate = NULL;
+	}
+
 	host->core_clk = devm_clk_get(&pdev->dev, "core");
 	if (IS_ERR(host->core_clk)) {
 		ret = PTR_ERR(host->core_clk);
-- 
2.9.4

[PATCH 14/14] mmc: meson-gx: implement voltage switch callback

[Jerome Brunet]

Implement voltage switch callback (shamelessly copied from sunxi mmc
driver). This allow, with the appropriate tuning function, to use
SD ultra high speed modes.

Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
---
 drivers/mmc/host/meson-gx-mmc.c | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/drivers/mmc/host/meson-gx-mmc.c b/drivers/mmc/host/meson-gx-mmc.c
index e991c9e452ab..de2d7b8a9d97 100644
--- a/drivers/mmc/host/meson-gx-mmc.c
+++ b/drivers/mmc/host/meson-gx-mmc.c
@@ -38,6 +38,8 @@
 #include <linux/interrupt.h>
 #include <linux/bitfield.h>
 
+#include <linux/delay.h>
+
 #define DRIVER_NAME "meson-gx-mmc"
 
 #define SD_EMMC_CLOCK 0x0
@@ -1024,6 +1026,28 @@ static int meson_mmc_card_busy(struct mmc_host *mmc)
 	return !(FIELD_GET(STATUS_DATI, regval) & 0xf);
 }
 
+static int meson_mmc_voltage_switch(struct mmc_host *mmc, struct mmc_ios *ios)
+{
+	/* vqmmc regulator is available */
+	if (!IS_ERR(mmc->supply.vqmmc)) {
+		/*
+		 * The usual amlogic setup uses a GPIO to switch from one
+		 * regulator to the other. While the voltage ramp up is
+		 * pretty fast, care must be taken when switching from 3.3v
+		 * to 1.8v. Please make sure the regulator framework is aware
+		 * of your own regulator constraints
+		 */
+		return mmc_regulator_set_vqmmc(mmc, ios);
+
+	}
+
+	/* no vqmmc regulator, assume fixed regulator at 3/3.3V */
+	if (ios->signal_voltage == MMC_SIGNAL_VOLTAGE_330)
+		return 0;
+
+	return -EINVAL;
+}
+
 static const struct mmc_host_ops meson_mmc_ops = {
 	.request	= meson_mmc_request,
 	.set_ios	= meson_mmc_set_ios,
@@ -1032,6 +1056,7 @@ static const struct mmc_host_ops meson_mmc_ops = {
 	.post_req	= meson_mmc_post_req,
 	.execute_tuning = meson_mmc_execute_tuning,
 	.card_busy	= meson_mmc_card_busy,
+	.start_signal_voltage_switch = meson_mmc_voltage_switch,
 };
 
 static int meson_mmc_probe(struct platform_device *pdev)
-- 
2.9.4

Re: [PATCH 00/14] mmc: meson-gx: driver fixups and upgrade

[Jerome Brunet]

On Fri, 2017-08-04 at 19:43 +0200, Jerome Brunet wrote:
> The patchset features several bugfixes, rework and upgrade for the
> meson-gx MMC driver.
> 
> The main goal is to improve readability and enable new high speed
> modes, such as eMMC DDR52 and sdcard UHS modes up to SDR50 (100Mhz)
> 
> While full speed SDR104 is stable with most cards, a few seems to
> require an even more precise tuning. For this, we'll probably have
> to implement per-line delay calibration.
> 
> This series has been tested on gxbb-p200, gxbb-nanopi-k2 and
> gxl-s905x-libretech
> 

Ulf, Kevin,

Please ignore this series. I'll resend it soon

I've discovered new things around the tuning capabilities of the SoC.
I'll probably rework the tuning function again.

Sorry for the noise

Regards
Jerome

> Jerome Brunet (14):
>   mmc: meson-gx: fix mux mask definition
>   mmc: meson-gx: remove CLK_DIVIDER_ALLOW_ZERO clock flag
>   mmc: meson-gx: clean up some constants
>   mmc: meson-gx: use _irqsave variant of spinlock
>   mmc: meson-gx: cfg init overwrite values
>   mmc: meson-gx: rework set_ios function
>   mmc: meson-gx: rework clk_set function
>   mmc: meson-gx: rework clock init function
>   mmc: meson-gx: simplify interrupt handler
>   mmc: meson-gx: implement card_busy callback
>   mmc: meson-gx: rework tuning function
>   mmc: meson-gx: fix dual data rate mode frequencies
>   mmc: meson-gx: work around clk-stop issue
>   mmc: meson-gx: implement voltage switch callback
> 
>  drivers/mmc/host/meson-gx-mmc.c | 611 ++++++++++++++++++++++++++-------------
> -
>  1 file changed, 396 insertions(+), 215 deletions(-)
> 

Re: [PATCH 03/14] mmc: meson-gx: clean up some constants

[Kevin Hilman]

Jerome Brunet <jbrunet@baylibre.com> writes:

> Remove useless clock rate defines. These should not be defined but

To be more precise, they're also unused, so maybe s/useless/unused/ ?

> equested from the clock framework.

s/equested/requested/

> Also correct typo on the DELAY register
>
> Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>

Otherwise,

Reviewed-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 d480a8052a06..8a74a048db88 100644
> --- a/drivers/mmc/host/meson-gx-mmc.c
> +++ b/drivers/mmc/host/meson-gx-mmc.c
> @@ -45,9 +45,7 @@
>  #define   CLK_DIV_MAX 63
>  #define   CLK_SRC_MASK GENMASK(7, 6)
>  #define   CLK_SRC_XTAL 0   /* external crystal */
> -#define   CLK_SRC_XTAL_RATE 24000000
>  #define   CLK_SRC_PLL 1    /* FCLK_DIV2 */
> -#define   CLK_SRC_PLL_RATE 1000000000
>  #define   CLK_CORE_PHASE_MASK GENMASK(9, 8)
>  #define   CLK_TX_PHASE_MASK GENMASK(11, 10)
>  #define   CLK_RX_PHASE_MASK GENMASK(13, 12)
> @@ -57,7 +55,7 @@
>  #define   CLK_PHASE_270 3
>  #define   CLK_ALWAYS_ON BIT(24)
>  
> -#define SD_EMMC_DElAY 0x4
> +#define SD_EMMC_DELAY 0x4
>  #define SD_EMMC_ADJUST 0x8
>  #define SD_EMMC_CALOUT 0x10
>  #define SD_EMMC_START 0x40

Re: [PATCH 08/14] mmc: meson-gx: rework clock init function

[Kevin Hilman]

Jerome Brunet <jbrunet@baylibre.com> writes:

> Perform basic initialisation of the clk register before providing it to
> the CCF.
>
> Thanks to devm, carrying the clock structure around after init is not
> necessary. Rework the function to remove these from the controller host
> data.
>
> Finally, set initial mmc clock rate before enabling it, simplifying the
> exit condition.
>
> Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
> ---
>  drivers/mmc/host/meson-gx-mmc.c | 101 +++++++++++++++++++---------------------
>  1 file changed, 49 insertions(+), 52 deletions(-)
>
> diff --git a/drivers/mmc/host/meson-gx-mmc.c b/drivers/mmc/host/meson-gx-mmc.c
> index 8f9ba5190c18..4cc7d6530536 100644
> --- a/drivers/mmc/host/meson-gx-mmc.c
> +++ b/drivers/mmc/host/meson-gx-mmc.c
> @@ -42,10 +42,7 @@
>  
>  #define SD_EMMC_CLOCK 0x0
>  #define   CLK_DIV_MASK GENMASK(5, 0)
> -#define   CLK_DIV_MAX 63
>  #define   CLK_SRC_MASK GENMASK(7, 6)
> -#define   CLK_SRC_XTAL 0   /* external crystal */
> -#define   CLK_SRC_PLL 1    /* FCLK_DIV2 */
>  #define   CLK_CORE_PHASE_MASK GENMASK(9, 8)
>  #define   CLK_TX_PHASE_MASK GENMASK(11, 10)
>  #define   CLK_RX_PHASE_MASK GENMASK(13, 12)
> @@ -137,13 +134,9 @@ struct meson_host {
>  	spinlock_t lock;
>  	void __iomem *regs;
>  	struct clk *core_clk;
> -	struct clk_mux mux;
> -	struct clk *mux_clk;
> +	struct clk *signal_clk;
>  	unsigned long req_rate;
>  
> -	struct clk_divider cfg_div;
> -	struct clk *cfg_div_clk;
> -
>  	unsigned int bounce_buf_size;
>  	void *bounce_buf;
>  	dma_addr_t bounce_dma_addr;
> @@ -291,7 +284,7 @@ static int meson_mmc_clk_set(struct meson_host *host, unsigned long clk_rate)
>  		return 0;
>  	}
>  
> -	ret = clk_set_rate(host->cfg_div_clk, clk_rate);
> +	ret = clk_set_rate(host->signal_clk, clk_rate);

minor nit: where does the name "signal" come from?  I called this
"div_clk" because it's the output of the divider right before the
sd/emmc IP block.   Admittedly, that's not a great name either, and I'm
not too picky about the naming, just curious...

Looking at the diagram we have since I initially wrote the driver, this
is more commonly referred to as device_clk.

Anyways, if you're going to rename...

[...]

>  static void meson_mmc_set_tuning_params(struct mmc_host *mmc)
> @@ -987,7 +984,7 @@ static int meson_mmc_probe(struct platform_device *pdev)
>  	dma_free_coherent(host->dev, host->bounce_buf_size,
>  			  host->bounce_buf, host->bounce_dma_addr);
>  err_div_clk:

... probably should rename this too.

Otherwise,

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

Kevin

Re: [PATCH 13/14] mmc: meson-gx: work around clk-stop issue

[Kevin Hilman]

Jerome Brunet <jbrunet@baylibre.com> writes:

> It seems that the signal clock is also used and required, somehow, by
> the controller it self.
>
> It is shown during init, when writing to CFG while the divider is set
> to 0 will crash the SoC. During voltage switch, the controller may crash
> and the card may then fail to exit busy state if the clock is stopped.
>
> To avoid this, it is best to keep the clock running for the controller,
> except during rate change. However, we still need to be able to gate
> the clock out of the SoC. Let's use the pinmux for this, and fallback
> to gpio mode (pulled-down) when we need to gate the clock
>
> Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>

Curious "feature" of the IP, but the solution looks good to me.

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

Kevin

Re: [PATCH 00/14] mmc: meson-gx: driver fixups and upgrade

[Kevin Hilman]

Jerome Brunet <jbrunet@baylibre.com> writes:

> On Fri, 2017-08-04 at 19:43 +0200, Jerome Brunet wrote:
>> The patchset features several bugfixes, rework and upgrade for the
>> meson-gx MMC driver.
>> 
>> The main goal is to improve readability and enable new high speed
>> modes, such as eMMC DDR52 and sdcard UHS modes up to SDR50 (100Mhz)
>> 
>> While full speed SDR104 is stable with most cards, a few seems to
>> require an even more precise tuning. For this, we'll probably have
>> to implement per-line delay calibration.
>> 
>> This series has been tested on gxbb-p200, gxbb-nanopi-k2 and
>> gxl-s905x-libretech
>> 
>
> Ulf, Kevin,
>
> Please ignore this series. I'll resend it soon
>
> I've discovered new things around the tuning capabilities of the SoC.
> I'll probably rework the tuning function again.
>
> Sorry for the noise

No worries, I had a couple very minor comments on some individual
patches. Other than that, LGTM.

Feel free to add:

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

to anything that doesn't need major rework.

Also, You might want to separate out the fixups/cleanups from the
voltage/clock changes stuff.

Kevin

Re: [PATCH 03/14] mmc: meson-gx: clean up some constants

[Jerome Brunet]

On Mon, 2017-08-07 at 14:06 -0700, Kevin Hilman wrote:
> Jerome Brunet <jbrunet@baylibre.com> writes:
> 
> > Remove useless clock rate defines. These should not be defined but
> 
> To be more precise, they're also unused, so maybe s/useless/unused/ ?
> 
> > equested from the clock framework.
> 
> s/equested/requested/
> 

Thx Kevin !

> > Also correct typo on the DELAY register
> > 
> > Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
> 
> Otherwise,
> 
> Reviewed-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 d480a8052a06..8a74a048db88 100644
> > --- a/drivers/mmc/host/meson-gx-mmc.c
> > +++ b/drivers/mmc/host/meson-gx-mmc.c
> > @@ -45,9 +45,7 @@
> >  #define   CLK_DIV_MAX 63
> >  #define   CLK_SRC_MASK GENMASK(7, 6)
> >  #define   CLK_SRC_XTAL 0   /* external crystal */
> > -#define   CLK_SRC_XTAL_RATE 24000000
> >  #define   CLK_SRC_PLL 1    /* FCLK_DIV2 */
> > -#define   CLK_SRC_PLL_RATE 1000000000
> >  #define   CLK_CORE_PHASE_MASK GENMASK(9, 8)
> >  #define   CLK_TX_PHASE_MASK GENMASK(11, 10)
> >  #define   CLK_RX_PHASE_MASK GENMASK(13, 12)
> > @@ -57,7 +55,7 @@
> >  #define   CLK_PHASE_270 3
> >  #define   CLK_ALWAYS_ON BIT(24)
> >  
> > -#define SD_EMMC_DElAY 0x4
> > +#define SD_EMMC_DELAY 0x4
> >  #define SD_EMMC_ADJUST 0x8
> >  #define SD_EMMC_CALOUT 0x10
> >  #define SD_EMMC_START 0x40

Re: [PATCH 08/14] mmc: meson-gx: rework clock init function

[Jerome Brunet]

On Mon, 2017-08-07 at 14:34 -0700, Kevin Hilman wrote:
> Jerome Brunet <jbrunet@baylibre.com> writes:
> 
> > Perform basic initialisation of the clk register before providing it to
> > the CCF.
> > 
> > Thanks to devm, carrying the clock structure around after init is not
> > necessary. Rework the function to remove these from the controller host
> > data.
> > 
> > Finally, set initial mmc clock rate before enabling it, simplifying the
> > exit condition.
> > 
> > Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
> > ---
> >  drivers/mmc/host/meson-gx-mmc.c | 101 +++++++++++++++++++----------------
> > -----
> >  1 file changed, 49 insertions(+), 52 deletions(-)
> > 
> > diff --git a/drivers/mmc/host/meson-gx-mmc.c b/drivers/mmc/host/meson-gx-
> > mmc.c
> > index 8f9ba5190c18..4cc7d6530536 100644
> > --- a/drivers/mmc/host/meson-gx-mmc.c
> > +++ b/drivers/mmc/host/meson-gx-mmc.c
> > @@ -42,10 +42,7 @@
> >  
> >  #define SD_EMMC_CLOCK 0x0
> >  #define   CLK_DIV_MASK GENMASK(5, 0)
> > -#define   CLK_DIV_MAX 63
> >  #define   CLK_SRC_MASK GENMASK(7, 6)
> > -#define   CLK_SRC_XTAL 0   /* external crystal */
> > -#define   CLK_SRC_PLL 1    /* FCLK_DIV2 */
> >  #define   CLK_CORE_PHASE_MASK GENMASK(9, 8)
> >  #define   CLK_TX_PHASE_MASK GENMASK(11, 10)
> >  #define   CLK_RX_PHASE_MASK GENMASK(13, 12)
> > @@ -137,13 +134,9 @@ struct meson_host {
> >  	spinlock_t lock;
> >  	void __iomem *regs;
> >  	struct clk *core_clk;
> > -	struct clk_mux mux;
> > -	struct clk *mux_clk;
> > +	struct clk *signal_clk;
> >  	unsigned long req_rate;
> >  
> > -	struct clk_divider cfg_div;
> > -	struct clk *cfg_div_clk;
> > -
> >  	unsigned int bounce_buf_size;
> >  	void *bounce_buf;
> >  	dma_addr_t bounce_dma_addr;
> > @@ -291,7 +284,7 @@ static int meson_mmc_clk_set(struct meson_host *host,
> > unsigned long clk_rate)
> >  		return 0;
> >  	}
> >  
> > -	ret = clk_set_rate(host->cfg_div_clk, clk_rate);
> > +	ret = clk_set_rate(host->signal_clk, clk_rate);
> 
> minor nit: where does the name "signal" come from?  I called this
> "div_clk" because it's the output of the divider right before the
> sd/emmc IP block. 

Actually, no before but inside.

> Admittedly, that's not a great name either, and I'm
> not too picky about the naming, just curious...

Well, I thought div_clk was not great name too ... since the clock is actually
the one used for the mmc signal (as opposed to the clock gate named "core") I
thought it was ok.

In the v2, I added the handling of clock phase through CCF. With this change,
div_clk does not make sense anymore.

According to the datasheet, It should be named "core_clk" (it is the clock
coming out of the CORE_PHASE setting), unfortunately we already used this name
for the clock gate ... changing it would mean changing the clock names in the
bindings as well, which would probably be very confusing.

In the v2, I named it mmc_clk ... probably not great either :( If you have
another idea, I'm happy to sed it.

> 
> Looking at the diagram we have since I initially wrote the driver, this
> is more commonly referred to as device_clk.
> 
> Anyways, if you're going to rename...
> 
> [...]
> 
> >  static void meson_mmc_set_tuning_params(struct mmc_host *mmc)
> > @@ -987,7 +984,7 @@ static int meson_mmc_probe(struct platform_device *pdev)
> >  	dma_free_coherent(host->dev, host->bounce_buf_size,
> >  			  host->bounce_buf, host->bounce_dma_addr);
> >  err_div_clk:
> 
> ... probably should rename this too.

Indeed !

> 
> Otherwise,
> 
> Reviewed-by: Kevin Hilman <khilman@baylibre.com>
> 
> Kevin