[PATCH/RFC v2 0/3] UHS-I SDR-104 support for sh_mobile_sdhi

[PATCH/RFC v2 0/3] UHS-I SDR-104 support for sh_mobile_sdhi

[Simon Horman]

Hi,

this series is based on work by Ai Kyuse to add UHS-I SDR-104 support for
sh_mobile_sdhi. It builds on work by Shinobu Uehara, Rob Taylor, William
Towle and Ian Molton, Ben Hutchings, Wolfram Sang and others to add UHS-I
SDR-50 support to the same driver.

This series enables UHS-I SDR-104 on the r8a7790/Lager where
UHS-I SDR-50 is already enabled.

It is based on a merge of:
* The next branch of the mmc tree  
* The renesas-next-20160427v2-v4.6-rc1 tag of the renesas tree
* The sh-pfc-for-v4.7-tag1 tag of the renesas-drivers tree

The first two mmc driver patches are targeted at the next branch of the mmc
tree. The last DT patch is targeted at the devel branch of the renesas tree.

To aid review this series is provided in the _temporary_ topic/sdr104-v2 branch
of the renesas tree.


I received somewhat extensive review of v1 of this series and I have
attempted to rework, correspondingly extensively, these patches to reflect
that review. I apologise if I missed anything noted in v1. And given the
extent of the changes since then I fully expect to need to follow-up with v3.

A pleasant side effect of the above mentioned changes is
that v2 has roughly 40% fewer lines of code than v1 :)


Please see "[PATCH/RFC 0/3] UHS-I SDR-104 support for sh_mobile_sdhi" for
indicative test results: I do not believe the numbers have not changed
in a material way since then.


Ai Kyuse (2):
  mmc: tmio: Add tuning support
  mmc: sh_mobile_sdhi: Add tuning support

Simon Horman (1):
  ARM: dts: r8a7790: lager: Enable UHS-I SDR-104

 arch/arm/boot/dts/r8a7790-lager.dts |   1 +
 drivers/mmc/host/sh_mobile_sdhi.c   | 216 +++++++++++++++++++++++++++++++++++-
 drivers/mmc/host/tmio_mmc.h         |   6 +
 drivers/mmc/host/tmio_mmc_pio.c     |  85 +++++++++++++-
 4 files changed, 306 insertions(+), 2 deletions(-)

-- 
2.1.4

[PATCH/RFC v2 1/3] mmc: tmio: Add tuning support

[Simon Horman]

From: Ai Kyuse <ai.kyuse.uw@renesas.com>

Add tuning support for use with SDR104 mode

Signed-off-by: Ai Kyuse <ai.kyuse.uw@renesas.com>
Signed-off-by: Simon Horman <horms+renesas@verge.net.au>
---
v2 [Simon Horman]
* As suggested by Kuninori Morimoto
  - Actually remove unnecessary TMIO_MMC_HAS_UHS_SCC define
* As suggested by Wolfram Sang
  - Rely on core to call tuning. This simplifies things somewhat.
  - Use mmc_send_tuning()
    - A side affect of this appears to be that we now see some recoverable
      errors logged during tuning. These are typically corrected by
      subsequent tuning. It is the logging that is the apparent side effect
      of this change.
      e.g.
      sh_mobile_sdhi ee100000.sd: timeout waiting for hardware interrupt (CMD19)
      sh_mobile_sdhi ee100000.sd: Tuning procedure failed
* Use bool rather than unsigned long to pass test status
  to select_tuning() callback
* Do not retune if init_tuning callback is not present or
  indicates that there are no taps present
* Retune on hardware reset

v1 [Simon Horman]
* Omit start_signal_voltage_switch and tmio_mmc_card_busy changes which are
  already present in mainline in a different form
* Return num from init_tuning rather than passing an extra parameter
  to hold the return value
* Only call host->init_tuning if it is non-NULL
* Place tmio_mmc_execute_tuning() such that no new forward declarations are
  required
* Remove unused TMIO_MMC_HAS_UHS_SCC define

v0 [Ai Kyuse]
---
 drivers/mmc/host/tmio_mmc.h     |  6 +++
 drivers/mmc/host/tmio_mmc_pio.c | 85 ++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 90 insertions(+), 1 deletion(-)

diff --git a/drivers/mmc/host/tmio_mmc.h b/drivers/mmc/host/tmio_mmc.h
index 1aac2ad8edf2..b2168370af22 100644
--- a/drivers/mmc/host/tmio_mmc.h
+++ b/drivers/mmc/host/tmio_mmc.h
@@ -150,6 +150,7 @@ struct tmio_mmc_host {
 	struct mutex		ios_lock;	/* protect set_ios() context */
 	bool			native_hotplug;
 	bool			sdio_irq_enabled;
+	u32			scc_tappos;
 
 	int (*write16_hook)(struct tmio_mmc_host *host, int addr);
 	int (*clk_enable)(struct tmio_mmc_host *host);
@@ -160,6 +161,11 @@ struct tmio_mmc_host {
 			      unsigned int direction, int blk_size);
 	int (*start_signal_voltage_switch)(struct mmc_host *mmc,
 					   struct mmc_ios *ios);
+	unsigned int (*init_tuning)(struct tmio_mmc_host *host);
+	int (*prepare_tuning)(struct tmio_mmc_host *host, unsigned long tap);
+	int (*select_tuning)(struct tmio_mmc_host *host, bool *tap);
+	bool (*retuning)(struct tmio_mmc_host *host);
+	void (*hw_reset)(struct tmio_mmc_host *host);
 };
 
 struct tmio_mmc_host *tmio_mmc_host_alloc(struct platform_device *pdev);
diff --git a/drivers/mmc/host/tmio_mmc_pio.c b/drivers/mmc/host/tmio_mmc_pio.c
index 12dc0440a487..e20f6522c9f9 100644
--- a/drivers/mmc/host/tmio_mmc_pio.c
+++ b/drivers/mmc/host/tmio_mmc_pio.c
@@ -36,6 +36,7 @@
 #include <linux/io.h>
 #include <linux/irq.h>
 #include <linux/mfd/tmio.h>
+#include <linux/mmc/card.h>
 #include <linux/mmc/host.h>
 #include <linux/mmc/mmc.h>
 #include <linux/mmc/slot-gpio.h>
@@ -363,7 +364,8 @@ static int tmio_mmc_start_command(struct tmio_mmc_host *host, struct mmc_command
 			 * multiple block transfer
 			 */
 			if ((host->pdata->flags & TMIO_MMC_HAVE_CMD12_CTRL) &&
-			    (cmd->opcode == SD_IO_RW_EXTENDED))
+			    ((cmd->opcode == SD_IO_RW_EXTENDED) ||
+			     host->mrq->sbc))
 				c |= NO_CMD12_ISSUE;
 		}
 		if (data->flags & MMC_DATA_READ)
@@ -756,6 +758,74 @@ static int tmio_mmc_start_data(struct tmio_mmc_host *host,
 	return 0;
 }
 
+static void tmio_mmc_hw_reset(struct mmc_host *mmc)
+{
+	struct tmio_mmc_host *host = mmc_priv(mmc);
+
+	if (host->hw_reset)
+		host->hw_reset(host);
+
+	mmc_retune_timer_stop(host->mmc);
+	mmc_retune_needed(host->mmc);
+}
+
+static int tmio_mmc_execute_tuning(struct mmc_host *mmc, u32 opcode)
+{
+	struct tmio_mmc_host *host = mmc_priv(mmc);
+	unsigned int num = 0;
+	int i, ret = 0;
+	bool *tap;
+
+	if (host->init_tuning)
+		num = host->init_tuning(host);
+	if (!num) {
+		/* Tuning is not supported */
+		ret = 0;
+		goto out;
+	}
+
+	tap = kmalloc(num * 2, GFP_KERNEL);
+	if (tap == NULL) {
+		ret = -ENOMEM;
+		goto out;
+	}
+
+	/*
+	 * Issue CMD19 twice for each tap
+	 */
+	for (i = 0; i < 2 * num; i++) {
+		int err;
+
+		if (host->prepare_tuning)
+			host->prepare_tuning(host, i);
+
+		err = mmc_send_tuning(host->mmc, opcode, NULL);
+		if (err && err != -EILSEQ) {
+			ret = err;
+			goto err_free;
+		}
+		tap[i] = (err == 0);
+
+		mdelay(1);
+	}
+
+	if (host->select_tuning)
+		ret = host->select_tuning(host, tap);
+
+err_free:
+	kfree(tap);
+out:
+	if (ret < 0) {
+		dev_warn(&host->pdev->dev, "Tuning procedure failed\n");
+		tmio_mmc_hw_reset(mmc);
+	} else {
+		host->mmc->retune_period = 0;
+	}
+
+	return ret;
+
+}
+
 /* Process requests from the MMC layer */
 static void tmio_mmc_request(struct mmc_host *mmc, struct mmc_request *mrq)
 {
@@ -781,6 +851,14 @@ static void tmio_mmc_request(struct mmc_host *mmc, struct mmc_request *mrq)
 
 	spin_unlock_irqrestore(&host->lock, flags);
 
+	if (mrq->sbc) {
+		ret = tmio_mmc_start_command(host, mrq->sbc);
+		if (ret)
+			goto fail;
+		host->last_req_ts = jiffies;
+		host->mrq = mrq;
+	}
+
 	if (mrq->data) {
 		ret = tmio_mmc_start_data(host, mrq->data);
 		if (ret)
@@ -978,6 +1056,8 @@ static struct mmc_host_ops tmio_mmc_ops = {
 	.enable_sdio_irq = tmio_mmc_enable_sdio_irq,
 	.card_busy	= tmio_mmc_card_busy,
 	.multi_io_quirk	= tmio_multi_io_quirk,
+	.execute_tuning = tmio_mmc_execute_tuning,
+	.hw_reset	= tmio_mmc_hw_reset,
 };
 
 static int tmio_mmc_init_ocr(struct tmio_mmc_host *host)
@@ -1202,6 +1282,9 @@ int tmio_mmc_host_runtime_suspend(struct device *dev)
 	struct mmc_host *mmc = dev_get_drvdata(dev);
 	struct tmio_mmc_host *host = mmc_priv(mmc);
 
+	mmc_retune_timer_stop(host->mmc);
+	mmc_retune_needed(host->mmc);
+
 	tmio_mmc_disable_mmc_irqs(host, TMIO_MASK_ALL);
 
 	if (host->clk_cache)
-- 
2.1.4

[PATCH/RFC v2 2/3] mmc: sh_mobile_sdhi: Add tuning support

[Simon Horman]

From: Ai Kyuse <ai.kyuse.uw@renesas.com>

Add tuning support for use with SDR104 mode
This includes adding support for the sampling clock controller (SCC).

Signed-off-by: Ai Kyuse <ai.kyuse.uw@renesas.com>
Signed-off-by: Simon Horman <horms+renesas@verge.net.au>
---
v3 [Simon Horman]
  - Use bool instead of long for taps parameter of
    sh_mobile_sdhi_select_tuning()
  - Return 0 from sh_mobile_sdhi_init_tuning() if the
    SDR104 capability is not set and thus tuning should not
    be performed because it is not supported by the hardware

v2 [Simon Horman]
* As suggested by Kuninori Morimoto
  - Use host->mmc->caps & MMC_CAP_UHS_SDR104 instead of
    pdata->flags & TMIO_MMC_HAS_UHS_SCC to avoid needing the
    MMC_CAP_UHS_SDR104 flag at all.
    N.B: Morimoto-san suggested using but this flag is not actually
    set there in by current probe come.
  - Simplify logic in sh_mobile_sdhi_inquiry_tuning
* As suggested by Wolfram Sang
  - Use clk_rate instead of clk for field in struct sh_mobile_sdhi_scc
  - Remove inquiry_tuning callback which is now unnecessary as calling
    of tuning is handled by the core
  - Removed unused sh_mobile_sdhi_set_clk_div callback
  - Save sci_base address rather than calculating it on each read and write
* Update selection logic in sh_mobile_sdhi_select_tuning to match spec

v1 [Simon Horman]
* Rebase
* Always use value of 0x8 for TAPNUM field of DTCNTL register
  rather than reading value from DT property. There does not
  seem to be a need to expose this in DT at this point.
* Do not include tmio_mmc_start_signal_voltage_switch changes which
  are already in mainline in a different form
* Do not add renesas,clk-rate property as the max-frequency property, which
  is now present in mainline, seems to provide the needed rate
* Omit Gen3 specific changes
* Do not provide renesas,mmc-scc-tappos DT property.
  Instead, always use taps provided in driver.
* Do not parse sd-uhs-sdr50 and sd-uhs-sdr104 properties.
  This is handled by the core.

v0 [Ai Kyuse]
---
 drivers/mmc/host/sh_mobile_sdhi.c | 216 +++++++++++++++++++++++++++++++++++++-
 1 file changed, 215 insertions(+), 1 deletion(-)

diff --git a/drivers/mmc/host/sh_mobile_sdhi.c b/drivers/mmc/host/sh_mobile_sdhi.c
index 5309c73be1f0..f79e1f804bc4 100644
--- a/drivers/mmc/host/sh_mobile_sdhi.c
+++ b/drivers/mmc/host/sh_mobile_sdhi.c
@@ -41,6 +41,11 @@
 
 #define host_to_priv(host) container_of((host)->pdata, struct sh_mobile_sdhi, mmc_data)
 
+struct sh_mobile_sdhi_scc {
+	unsigned long clk_rate;	/* clock rate for SDR104 */
+	u32 tap;		/* sampling clock position for SDR104 */
+};
+
 struct sh_mobile_sdhi_of_data {
 	unsigned long tmio_flags;
 	unsigned long capabilities;
@@ -48,6 +53,9 @@ struct sh_mobile_sdhi_of_data {
 	enum dma_slave_buswidth dma_buswidth;
 	dma_addr_t dma_rx_offset;
 	unsigned bus_shift;
+	int scc_offset;
+	struct sh_mobile_sdhi_scc *taps;
+	int taps_num;
 };
 
 static const struct sh_mobile_sdhi_of_data of_default_cfg = {
@@ -60,12 +68,27 @@ static const struct sh_mobile_sdhi_of_data of_rcar_gen1_compatible = {
 	.capabilities	= MMC_CAP_SD_HIGHSPEED | MMC_CAP_SDIO_IRQ,
 };
 
+/* Definitions for sampling clocks */
+static struct sh_mobile_sdhi_scc rcar_gen2_scc_taps[] = {
+	{
+		.clk_rate = 156000000,
+		.tap = 0x00000703,
+	},
+	{
+		.clk_rate = 0,
+		.tap = 0x00000300,
+	},
+};
+
 static const struct sh_mobile_sdhi_of_data of_rcar_gen2_compatible = {
 	.tmio_flags	= TMIO_MMC_HAS_IDLE_WAIT | TMIO_MMC_WRPROTECT_DISABLE |
 			  TMIO_MMC_CLK_ACTUAL | TMIO_MMC_MIN_RCAR2,
 	.capabilities	= MMC_CAP_SD_HIGHSPEED | MMC_CAP_SDIO_IRQ,
 	.dma_buswidth	= DMA_SLAVE_BUSWIDTH_4_BYTES,
 	.dma_rx_offset	= 0x2000,
+	.scc_offset	= 0x0300,
+	.taps		= rcar_gen2_scc_taps,
+	.taps_num	= ARRAY_SIZE(rcar_gen2_scc_taps),
 };
 
 static const struct sh_mobile_sdhi_of_data of_rcar_gen3_compatible = {
@@ -98,6 +121,7 @@ struct sh_mobile_sdhi {
 	struct tmio_mmc_dma dma_priv;
 	struct pinctrl *pinctrl;
 	struct pinctrl_state *pins_default, *pins_uhs;
+	void __iomem *scc_ctl;
 };
 
 static void sh_mobile_sdhi_sdbuf_width(struct tmio_mmc_host *host, int width)
@@ -241,6 +265,167 @@ static int sh_mobile_sdhi_start_signal_voltage_switch(struct mmc_host *mmc,
 	return pinctrl_select_state(priv->pinctrl, pin_state);
 }
 
+/* SCC registers */
+#define SH_MOBILE_SDHI_SCC_DTCNTL	0x000
+#define SH_MOBILE_SDHI_SCC_TAPSET	0x002
+#define SH_MOBILE_SDHI_SCC_DT2FF	0x004
+#define SH_MOBILE_SDHI_SCC_CKSEL	0x006
+#define SH_MOBILE_SDHI_SCC_RVSCNTL	0x008
+#define SH_MOBILE_SDHI_SCC_RVSREQ	0x00A
+
+/* Definitions for values the SH_MOBILE_SDHI_SCC_DTCNTL register */
+#define SH_MOBILE_SDHI_SCC_DTCNTL_TAPEN		BIT(0)
+#define SH_MOBILE_SDHI_SCC_DTCNTL_TAPNUM_SHIFT	16
+#define SH_MOBILE_SDHI_SCC_DTCNTL_TAPNUM_MASK	0xff
+
+/* Definitions for values the SH_MOBILE_SDHI_SCC_CKSEL register */
+#define SH_MOBILE_SDHI_SCC_CKSEL_DTSEL		BIT(0)
+/* Definitions for values the SH_MOBILE_SDHI_SCC_RVSCNTL register */
+#define SH_MOBILE_SDHI_SCC_RVSCNTL_RVSEN	BIT(1)
+/* Definitions for values the SH_MOBILE_SDHI_SCC_RVSREQ register */
+#define SH_MOBILE_SDHI_SCC_RVSREQ_RVSERR	BIT(2)
+
+static inline u32 sd_scc_read32(struct tmio_mmc_host *host, int addr)
+{
+	return readl(host_to_priv(host)->scc_ctl + (addr << host->bus_shift));
+}
+
+static inline void sd_scc_write32(struct tmio_mmc_host *host, int addr, u32 val)
+{
+	writel(val, host_to_priv(host)->scc_ctl + (addr << host->bus_shift));
+}
+
+static unsigned int sh_mobile_sdhi_init_tuning(struct tmio_mmc_host *host)
+{
+	if (!(host->mmc->caps & MMC_CAP_UHS_SDR104))
+		return 0;
+
+	/* set sampling clock selection range */
+	sd_scc_write32(host, SH_MOBILE_SDHI_SCC_DTCNTL,
+			0x8 << SH_MOBILE_SDHI_SCC_DTCNTL_TAPNUM_SHIFT);
+
+	/* Initialize SCC */
+	sd_ctrl_write32_as_16_and_16(host, CTL_STATUS, 0x00000000);
+
+	sd_scc_write32(host, SH_MOBILE_SDHI_SCC_DTCNTL,
+		SH_MOBILE_SDHI_SCC_DTCNTL_TAPEN |
+		sd_scc_read32(host, SH_MOBILE_SDHI_SCC_DTCNTL));
+
+	sd_ctrl_write16(host, CTL_SD_CARD_CLK_CTL, ~0x0100 &
+		sd_ctrl_read16(host, CTL_SD_CARD_CLK_CTL));
+
+	sd_scc_write32(host, SH_MOBILE_SDHI_SCC_CKSEL,
+		SH_MOBILE_SDHI_SCC_CKSEL_DTSEL |
+		sd_scc_read32(host, SH_MOBILE_SDHI_SCC_CKSEL));
+
+	sd_ctrl_write16(host, CTL_SD_CARD_CLK_CTL, 0x0100 |
+		sd_ctrl_read16(host, CTL_SD_CARD_CLK_CTL));
+
+	sd_scc_write32(host, SH_MOBILE_SDHI_SCC_RVSCNTL,
+		~SH_MOBILE_SDHI_SCC_RVSCNTL_RVSEN &
+		sd_scc_read32(host, SH_MOBILE_SDHI_SCC_RVSCNTL));
+
+	sd_scc_write32(host, SH_MOBILE_SDHI_SCC_DT2FF, host->scc_tappos);
+
+	/* Read TAPNUM */
+	return (sd_scc_read32(host, SH_MOBILE_SDHI_SCC_DTCNTL) >>
+		SH_MOBILE_SDHI_SCC_DTCNTL_TAPNUM_SHIFT) &
+		SH_MOBILE_SDHI_SCC_DTCNTL_TAPNUM_MASK;
+}
+
+static int sh_mobile_sdhi_prepare_tuning(struct tmio_mmc_host *host,
+					 unsigned long tap)
+{
+	/* Set sampling clock position */
+	sd_scc_write32(host, SH_MOBILE_SDHI_SCC_TAPSET, tap);
+
+	return 0;
+}
+
+#define SH_MOBILE_SDHI_MAX_TAP	3
+
+static int sh_mobile_sdhi_select_tuning(struct tmio_mmc_host *host, bool *tap)
+{
+	unsigned long tap_num, i;
+	int ok_count;
+
+	/* Clear SCC_RVSREQ */
+	sd_scc_write32(host, SH_MOBILE_SDHI_SCC_RVSREQ, 0);
+
+	/* Select SCC */
+	tap_num = (sd_scc_read32(host, SH_MOBILE_SDHI_SCC_DTCNTL) >>
+		   SH_MOBILE_SDHI_SCC_DTCNTL_TAPNUM_SHIFT) &
+		SH_MOBILE_SDHI_SCC_DTCNTL_TAPNUM_MASK;
+
+	/*
+	 * Select clock where three consecutive bock reads succeeded.
+	 *
+	 * There may be multiple occurrences of three successive reads
+	 * and selecting any of them is correct. Here the first one is
+	 * selected.
+	 */
+	ok_count = 0;
+	for (i = 0; i < 2 * tap_num; i++) {
+		if (tap[i])
+			ok_count++;
+		else
+			ok_count = 0;
+		if (ok_count == 3)
+			break;
+	}
+
+	if (ok_count != 3)
+		return -EIO;
+
+	/* Set SCC */
+	sd_scc_write32(host, SH_MOBILE_SDHI_SCC_TAPSET, i);
+
+	/* Enable auto re-tuning */
+	sd_scc_write32(host, SH_MOBILE_SDHI_SCC_RVSCNTL,
+		SH_MOBILE_SDHI_SCC_RVSCNTL_RVSEN |
+		sd_scc_read32(host, SH_MOBILE_SDHI_SCC_RVSCNTL));
+
+	return 0;
+}
+
+static bool sh_mobile_sdhi_retuning(struct tmio_mmc_host *host)
+{
+	/* Check SCC error */
+	if (sd_scc_read32(host, SH_MOBILE_SDHI_SCC_RVSCNTL) &
+	    SH_MOBILE_SDHI_SCC_RVSCNTL_RVSEN &&
+	    sd_scc_read32(host, SH_MOBILE_SDHI_SCC_RVSREQ) &
+	    SH_MOBILE_SDHI_SCC_RVSREQ_RVSERR) {
+		/* Clear SCC error */
+		sd_scc_write32(host, SH_MOBILE_SDHI_SCC_RVSREQ, 0);
+		return true;
+	}
+	return false;
+}
+
+static void sh_mobile_sdhi_hw_reset(struct tmio_mmc_host *host)
+{
+	if (host->mmc->caps & MMC_CAP_UHS_SDR104) {
+		/* Reset SCC */
+		sd_ctrl_write16(host, CTL_SD_CARD_CLK_CTL, ~0x0100 &
+			sd_ctrl_read16(host, CTL_SD_CARD_CLK_CTL));
+
+		sd_scc_write32(host, SH_MOBILE_SDHI_SCC_CKSEL,
+			~SH_MOBILE_SDHI_SCC_CKSEL_DTSEL &
+			sd_scc_read32(host, SH_MOBILE_SDHI_SCC_CKSEL));
+
+		sd_ctrl_write16(host, CTL_SD_CARD_CLK_CTL, 0x0100 |
+			sd_ctrl_read16(host, CTL_SD_CARD_CLK_CTL));
+
+		sd_scc_write32(host, SH_MOBILE_SDHI_SCC_RVSCNTL,
+			~SH_MOBILE_SDHI_SCC_RVSCNTL_RVSEN &
+			sd_scc_read32(host, SH_MOBILE_SDHI_SCC_RVSCNTL));
+
+		sd_scc_write32(host, SH_MOBILE_SDHI_SCC_RVSCNTL,
+			~SH_MOBILE_SDHI_SCC_RVSCNTL_RVSEN &
+			sd_scc_read32(host, SH_MOBILE_SDHI_SCC_RVSCNTL));
+	}
+}
+
 static int sh_mobile_sdhi_wait_idle(struct tmio_mmc_host *host)
 {
 	int timeout = 1000;
@@ -311,7 +496,7 @@ static int sh_mobile_sdhi_probe(struct platform_device *pdev)
 	struct tmio_mmc_data *mmd = pdev->dev.platform_data;
 	struct tmio_mmc_host *host;
 	struct resource *res;
-	int irq, ret, i = 0;
+	int irq, ret, i;
 	struct tmio_mmc_dma *dma_priv;
 
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
@@ -364,6 +549,11 @@ static int sh_mobile_sdhi_probe(struct platform_device *pdev)
 	host->clk_disable	= sh_mobile_sdhi_clk_disable;
 	host->multi_io_quirk	= sh_mobile_sdhi_multi_io_quirk;
 	host->start_signal_voltage_switch = sh_mobile_sdhi_start_signal_voltage_switch;
+	host->init_tuning	= sh_mobile_sdhi_init_tuning;
+	host->prepare_tuning	= sh_mobile_sdhi_prepare_tuning;
+	host->select_tuning	= sh_mobile_sdhi_select_tuning;
+	host->retuning		= sh_mobile_sdhi_retuning;
+	host->hw_reset		= sh_mobile_sdhi_hw_reset;
 
 	/* Orginally registers were 16 bit apart, could be 32 or 64 nowadays */
 	if (!host->bus_shift && resource_size(res) > 0x100) /* old way to determine the shift */
@@ -403,6 +593,30 @@ static int sh_mobile_sdhi_probe(struct platform_device *pdev)
 	if (ret < 0)
 		goto efree;
 
+	if (host->mmc->caps & MMC_CAP_UHS_SDR104)
+		host->mmc->caps |= MMC_CAP_HW_RESET;
+
+	if (of_id && of_id->data) {
+		const struct sh_mobile_sdhi_of_data *of_data = of_id->data;
+		const struct sh_mobile_sdhi_scc *taps = of_data->taps;
+		bool hit = false;
+
+		for (i = 0; i < of_data->taps_num; i++) {
+			if (taps[i].clk_rate == 0 ||
+			    taps[i].clk_rate == host->mmc->f_max) {
+				host->scc_tappos = taps->tap;
+				hit = true;
+				break;
+			}
+		}
+
+		if (!hit)
+			dev_warn(&host->pdev->dev, "Unknown clock rate for SDR104 and HS200\n");
+
+		priv->scc_ctl = host->ctl + of_data->scc_offset;
+	}
+
+	i = 0;
 	while (1) {
 		irq = platform_get_irq(pdev, i);
 		if (irq < 0)
-- 
2.1.4

[PATCH/RFC v2 3/3] ARM: dts: r8a7790: lager: Enable UHS-I SDR-104

[Simon Horman]

Add the sd-uhs-sdr104 property to SDHI0.

Signed-off-by: Simon Horman <horms+renesas@verge.net.au>
---
 arch/arm/boot/dts/r8a7790-lager.dts | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/boot/dts/r8a7790-lager.dts b/arch/arm/boot/dts/r8a7790-lager.dts
index 749ba02b6a53..05d1ff7acee2 100644
--- a/arch/arm/boot/dts/r8a7790-lager.dts
+++ b/arch/arm/boot/dts/r8a7790-lager.dts
@@ -559,6 +559,7 @@
 	vqmmc-supply = <&vccq_sdhi0>;
 	cd-gpios = <&gpio3 6 GPIO_ACTIVE_LOW>;
 	sd-uhs-sdr50;
+	sd-uhs-sdr104;
 	status = "okay";
 };
 
-- 
2.1.4

Re: [PATCH/RFC v2 1/3] mmc: tmio: Add tuning support

[Kuninori Morimoto]

Hi Simon

Thank you for your patch.
Picky comment from me :)

> From: Ai Kyuse <ai.kyuse.uw@renesas.com>
> 
> Add tuning support for use with SDR104 mode
> 
> Signed-off-by: Ai Kyuse <ai.kyuse.uw@renesas.com>
> Signed-off-by: Simon Horman <horms+renesas@verge.net.au>
> ---
(snip)
> @@ -160,6 +161,11 @@ struct tmio_mmc_host {
>  			      unsigned int direction, int blk_size);
>  	int (*start_signal_voltage_switch)(struct mmc_host *mmc,
>  					   struct mmc_ios *ios);
> +	unsigned int (*init_tuning)(struct tmio_mmc_host *host);
> +	int (*prepare_tuning)(struct tmio_mmc_host *host, unsigned long tap);
> +	int (*select_tuning)(struct tmio_mmc_host *host, bool *tap);
> +	bool (*retuning)(struct tmio_mmc_host *host);
> +	void (*hw_reset)(struct tmio_mmc_host *host);
>  };

There is no (*retuning) code ?

> +static int tmio_mmc_execute_tuning(struct mmc_host *mmc, u32 opcode)
> +{
> +	struct tmio_mmc_host *host = mmc_priv(mmc);
> +	unsigned int num = 0;
> +	int i, ret = 0;
> +	bool *tap;
> +
> +	if (host->init_tuning)
> +		num = host->init_tuning(host);
> +	if (!num) {
> +		/* Tuning is not supported */
> +		ret = 0;
> +		goto out;
> +	}
> +
> +	tap = kmalloc(num * 2, GFP_KERNEL);
> +	if (tap == NULL) {
> +		ret = -ENOMEM;
> +		goto out;
> +	}

	if (!tap) {
		...
	}

> +	/*
> +	 * Issue CMD19 twice for each tap
> +	 */
> +	for (i = 0; i < 2 * num; i++) {
> +		int err;
> +
> +		if (host->prepare_tuning)
> +			host->prepare_tuning(host, i);

you want to check return value here ?
	int (*prepare_tuning)(xxx);

> +		err = mmc_send_tuning(host->mmc, opcode, NULL);
> +		if (err && err != -EILSEQ) {
> +			ret = err;
> +			goto err_free;
> +		}
> +		tap[i] = (err == 0);
> +
> +		mdelay(1);
> +	}
> +
> +	if (host->select_tuning)
> +		ret = host->select_tuning(host, tap);

Here, "tap" was alloc:ed array, but there is no array_size parameter.
A littile bit strange for me..., but I'm not sure


Best regards
---
Kuninori Morimoto

Re: [PATCH/RFC v2 1/3] mmc: tmio: Add tuning support

[Kuninori Morimoto]

Hi Simon

Thank you for your patch.
Picky comment from me :)

> From: Ai Kyuse <ai.kyuse.uw@renesas.com>
> 
> Add tuning support for use with SDR104 mode
> 
> Signed-off-by: Ai Kyuse <ai.kyuse.uw@renesas.com>
> Signed-off-by: Simon Horman <horms+renesas@verge.net.au>
> ---
(snip)
> @@ -160,6 +161,11 @@ struct tmio_mmc_host {
>  			      unsigned int direction, int blk_size);
>  	int (*start_signal_voltage_switch)(struct mmc_host *mmc,
>  					   struct mmc_ios *ios);
> +	unsigned int (*init_tuning)(struct tmio_mmc_host *host);
> +	int (*prepare_tuning)(struct tmio_mmc_host *host, unsigned long tap);
> +	int (*select_tuning)(struct tmio_mmc_host *host, bool *tap);
> +	bool (*retuning)(struct tmio_mmc_host *host);
> +	void (*hw_reset)(struct tmio_mmc_host *host);
>  };

There is no (*retuning) code ?

> +static int tmio_mmc_execute_tuning(struct mmc_host *mmc, u32 opcode)
> +{
> +	struct tmio_mmc_host *host = mmc_priv(mmc);
> +	unsigned int num = 0;
> +	int i, ret = 0;
> +	bool *tap;
> +
> +	if (host->init_tuning)
> +		num = host->init_tuning(host);
> +	if (!num) {
> +		/* Tuning is not supported */
> +		ret = 0;
> +		goto out;
> +	}
> +
> +	tap = kmalloc(num * 2, GFP_KERNEL);
> +	if (tap == NULL) {
> +		ret = -ENOMEM;
> +		goto out;
> +	}

	if (!tap) {
		...
	}

> +	/*
> +	 * Issue CMD19 twice for each tap
> +	 */
> +	for (i = 0; i < 2 * num; i++) {
> +		int err;
> +
> +		if (host->prepare_tuning)
> +			host->prepare_tuning(host, i);

you want to check return value here ?
	int (*prepare_tuning)(xxx);

> +		err = mmc_send_tuning(host->mmc, opcode, NULL);
> +		if (err && err != -EILSEQ) {
> +			ret = err;
> +			goto err_free;
> +		}
> +		tap[i] = (err == 0);
> +
> +		mdelay(1);
> +	}
> +
> +	if (host->select_tuning)
> +		ret = host->select_tuning(host, tap);

Here, "tap" was alloc:ed array, but there is no array_size parameter.
A littile bit strange for me..., but I'm not sure


Best regards
---
Kuninori Morimoto

Re: [PATCH/RFC v2 2/3] mmc: sh_mobile_sdhi: Add tuning support

[Kuninori Morimoto]

Hi Simon

> From: Ai Kyuse <ai.kyuse.uw@renesas.com>
> 
> Add tuning support for use with SDR104 mode
> This includes adding support for the sampling clock controller (SCC).
> 
> Signed-off-by: Ai Kyuse <ai.kyuse.uw@renesas.com>
> Signed-off-by: Simon Horman <horms+renesas@verge.net.au>
> ---
(snip)
> +static int sh_mobile_sdhi_select_tuning(struct tmio_mmc_host *host, bool *tap)
> +{
> +	unsigned long tap_num, i;
> +	int ok_count;
> +
> +	/* Clear SCC_RVSREQ */
> +	sd_scc_write32(host, SH_MOBILE_SDHI_SCC_RVSREQ, 0);
> +
> +	/* Select SCC */
> +	tap_num = (sd_scc_read32(host, SH_MOBILE_SDHI_SCC_DTCNTL) >>
> +		   SH_MOBILE_SDHI_SCC_DTCNTL_TAPNUM_SHIFT) &
> +		SH_MOBILE_SDHI_SCC_DTCNTL_TAPNUM_MASK;
> +
> +	/*
> +	 * Select clock where three consecutive bock reads succeeded.
> +	 *
> +	 * There may be multiple occurrences of three successive reads
> +	 * and selecting any of them is correct. Here the first one is
> +	 * selected.
> +	 */
> +	ok_count = 0;
> +	for (i = 0; i < 2 * tap_num; i++) {
> +		if (tap[i])
> +			ok_count++;
> +		else
> +			ok_count = 0;
> +		if (ok_count == 3)
> +			break;
> +	}

As I pointed on [1/3] patch, having "tap_array_size" on this function,
and check tap_num <-> tap_array_size is nice idea IMO.

Re: [PATCH/RFC v2 2/3] mmc: sh_mobile_sdhi: Add tuning support

[Kuninori Morimoto]

Hi Simon

> From: Ai Kyuse <ai.kyuse.uw@renesas.com>
> 
> Add tuning support for use with SDR104 mode
> This includes adding support for the sampling clock controller (SCC).
> 
> Signed-off-by: Ai Kyuse <ai.kyuse.uw@renesas.com>
> Signed-off-by: Simon Horman <horms+renesas@verge.net.au>
> ---
(snip)
> +static int sh_mobile_sdhi_select_tuning(struct tmio_mmc_host *host, bool *tap)
> +{
> +	unsigned long tap_num, i;
> +	int ok_count;
> +
> +	/* Clear SCC_RVSREQ */
> +	sd_scc_write32(host, SH_MOBILE_SDHI_SCC_RVSREQ, 0);
> +
> +	/* Select SCC */
> +	tap_num = (sd_scc_read32(host, SH_MOBILE_SDHI_SCC_DTCNTL) >>
> +		   SH_MOBILE_SDHI_SCC_DTCNTL_TAPNUM_SHIFT) &
> +		SH_MOBILE_SDHI_SCC_DTCNTL_TAPNUM_MASK;
> +
> +	/*
> +	 * Select clock where three consecutive bock reads succeeded.
> +	 *
> +	 * There may be multiple occurrences of three successive reads
> +	 * and selecting any of them is correct. Here the first one is
> +	 * selected.
> +	 */
> +	ok_count = 0;
> +	for (i = 0; i < 2 * tap_num; i++) {
> +		if (tap[i])
> +			ok_count++;
> +		else
> +			ok_count = 0;
> +		if (ok_count == 3)
> +			break;
> +	}

As I pointed on [1/3] patch, having "tap_array_size" on this function,
and check tap_num <-> tap_array_size is nice idea IMO.

Re: [PATCH/RFC v2 1/3] mmc: tmio: Add tuning support

[Simon Horman]

On Mon, May 23, 2016 at 12:53:59AM +0000, Kuninori Morimoto wrote:
> 
> Hi Simon
> 
> Thank you for your patch.
> Picky comment from me :)

Thanks for noticing these things and sorry for not having
done so myself.

> > From: Ai Kyuse <ai.kyuse.uw@renesas.com>
> > 
> > Add tuning support for use with SDR104 mode
> > 
> > Signed-off-by: Ai Kyuse <ai.kyuse.uw@renesas.com>
> > Signed-off-by: Simon Horman <horms+renesas@verge.net.au>
> > ---
> (snip)
> > @@ -160,6 +161,11 @@ struct tmio_mmc_host {
> >  			      unsigned int direction, int blk_size);
> >  	int (*start_signal_voltage_switch)(struct mmc_host *mmc,
> >  					   struct mmc_ios *ios);
> > +	unsigned int (*init_tuning)(struct tmio_mmc_host *host);
> > +	int (*prepare_tuning)(struct tmio_mmc_host *host, unsigned long tap);
> > +	int (*select_tuning)(struct tmio_mmc_host *host, bool *tap);
> > +	bool (*retuning)(struct tmio_mmc_host *host);
> > +	void (*hw_reset)(struct tmio_mmc_host *host);
> >  };
> 
> There is no (*retuning) code ?

Thanks, I think returning should simply be removed.

> > +static int tmio_mmc_execute_tuning(struct mmc_host *mmc, u32 opcode)
> > +{
> > +	struct tmio_mmc_host *host = mmc_priv(mmc);
> > +	unsigned int num = 0;
> > +	int i, ret = 0;
> > +	bool *tap;
> > +
> > +	if (host->init_tuning)
> > +		num = host->init_tuning(host);
> > +	if (!num) {
> > +		/* Tuning is not supported */
> > +		ret = 0;
> > +		goto out;
> > +	}
> > +
> > +	tap = kmalloc(num * 2, GFP_KERNEL);
> > +	if (tap == NULL) {
> > +		ret = -ENOMEM;
> > +		goto out;
> > +	}
> 
> 	if (!tap) {
> 		...
> 	}

ok

> > +	/*
> > +	 * Issue CMD19 twice for each tap
> > +	 */
> > +	for (i = 0; i < 2 * num; i++) {
> > +		int err;
> > +
> > +		if (host->prepare_tuning)
> > +			host->prepare_tuning(host, i);
> 
> you want to check return value here ?
> 	int (*prepare_tuning)(xxx);

The implementation of prepare_tuning doesn't seem to ever return an error.
Perhaps it would be best to just change the type to:

	void (*prepare_tuning)(xxx);

> > +		err = mmc_send_tuning(host->mmc, opcode, NULL);
> > +		if (err && err != -EILSEQ) {
> > +			ret = err;
> > +			goto err_free;
> > +		}
> > +		tap[i] = (err == 0);
> > +
> > +		mdelay(1);
> > +	}
> > +
> > +	if (host->select_tuning)
> > +		ret = host->select_tuning(host, tap);
> 
> Here, "tap" was alloc:ed array, but there is no array_size parameter.
> A littile bit strange for me..., but I'm not sure

Right. I think its safe. But its not very clean.
I will add a size parameter.

Re: [PATCH/RFC v2 2/3] mmc: sh_mobile_sdhi: Add tuning support

[Simon Horman]

On Mon, May 23, 2016 at 12:57:41AM +0000, Kuninori Morimoto wrote:
> 
> Hi Simon
> 
> > From: Ai Kyuse <ai.kyuse.uw@renesas.com>
> > 
> > Add tuning support for use with SDR104 mode
> > This includes adding support for the sampling clock controller (SCC).
> > 
> > Signed-off-by: Ai Kyuse <ai.kyuse.uw@renesas.com>
> > Signed-off-by: Simon Horman <horms+renesas@verge.net.au>
> > ---
> (snip)
> > +static int sh_mobile_sdhi_select_tuning(struct tmio_mmc_host *host, bool *tap)
> > +{
> > +	unsigned long tap_num, i;
> > +	int ok_count;
> > +
> > +	/* Clear SCC_RVSREQ */
> > +	sd_scc_write32(host, SH_MOBILE_SDHI_SCC_RVSREQ, 0);
> > +
> > +	/* Select SCC */
> > +	tap_num = (sd_scc_read32(host, SH_MOBILE_SDHI_SCC_DTCNTL) >>
> > +		   SH_MOBILE_SDHI_SCC_DTCNTL_TAPNUM_SHIFT) &
> > +		SH_MOBILE_SDHI_SCC_DTCNTL_TAPNUM_MASK;
> > +
> > +	/*
> > +	 * Select clock where three consecutive bock reads succeeded.
> > +	 *
> > +	 * There may be multiple occurrences of three successive reads
> > +	 * and selecting any of them is correct. Here the first one is
> > +	 * selected.
> > +	 */
> > +	ok_count = 0;
> > +	for (i = 0; i < 2 * tap_num; i++) {
> > +		if (tap[i])
> > +			ok_count++;
> > +		else
> > +			ok_count = 0;
> > +		if (ok_count == 3)
> > +			break;
> > +	}
> 
> As I pointed on [1/3] patch, having "tap_array_size" on this function,
> and check tap_num <-> tap_array_size is nice idea IMO.

We could also provide the array parameter and
not read tap_num using sd_scc_read32() here at all.
Which do you feel is best?

Re: [PATCH/RFC v2 2/3] mmc: sh_mobile_sdhi: Add tuning support

[Kuninori Morimoto]

Hi Simon

> > > +static int sh_mobile_sdhi_select_tuning(struct tmio_mmc_host *host, bool *tap)
> > > +{
> > > +	unsigned long tap_num, i;
> > > +	int ok_count;
> > > +
> > > +	/* Clear SCC_RVSREQ */
> > > +	sd_scc_write32(host, SH_MOBILE_SDHI_SCC_RVSREQ, 0);
> > > +
> > > +	/* Select SCC */
> > > +	tap_num = (sd_scc_read32(host, SH_MOBILE_SDHI_SCC_DTCNTL) >>
> > > +		   SH_MOBILE_SDHI_SCC_DTCNTL_TAPNUM_SHIFT) &
> > > +		SH_MOBILE_SDHI_SCC_DTCNTL_TAPNUM_MASK;
> > > +
> > > +	/*
> > > +	 * Select clock where three consecutive bock reads succeeded.
> > > +	 *
> > > +	 * There may be multiple occurrences of three successive reads
> > > +	 * and selecting any of them is correct. Here the first one is
> > > +	 * selected.
> > > +	 */
> > > +	ok_count = 0;
> > > +	for (i = 0; i < 2 * tap_num; i++) {
> > > +		if (tap[i])
> > > +			ok_count++;
> > > +		else
> > > +			ok_count = 0;
> > > +		if (ok_count == 3)
> > > +			break;
> > > +	}
> > 
> > As I pointed on [1/3] patch, having "tap_array_size" on this function,
> > and check tap_num <-> tap_array_size is nice idea IMO.
> 
> We could also provide the array parameter and
> not read tap_num using sd_scc_read32() here at all.
> Which do you feel is best?

Not sure, but getting array_size as parameter is very normal for me.
This sd_scc_read32() is using SH_MOBILE_xxx (Renesas specific register).
So, we can use it as sanitary check ?
I don't care about it, its up to you :)

Re: [PATCH/RFC v2 2/3] mmc: sh_mobile_sdhi: Add tuning support

[Kuninori Morimoto]

Hi Simon

> > > +static int sh_mobile_sdhi_select_tuning(struct tmio_mmc_host *host, bool *tap)
> > > +{
> > > +	unsigned long tap_num, i;
> > > +	int ok_count;
> > > +
> > > +	/* Clear SCC_RVSREQ */
> > > +	sd_scc_write32(host, SH_MOBILE_SDHI_SCC_RVSREQ, 0);
> > > +
> > > +	/* Select SCC */
> > > +	tap_num = (sd_scc_read32(host, SH_MOBILE_SDHI_SCC_DTCNTL) >>
> > > +		   SH_MOBILE_SDHI_SCC_DTCNTL_TAPNUM_SHIFT) &
> > > +		SH_MOBILE_SDHI_SCC_DTCNTL_TAPNUM_MASK;
> > > +
> > > +	/*
> > > +	 * Select clock where three consecutive bock reads succeeded.
> > > +	 *
> > > +	 * There may be multiple occurrences of three successive reads
> > > +	 * and selecting any of them is correct. Here the first one is
> > > +	 * selected.
> > > +	 */
> > > +	ok_count = 0;
> > > +	for (i = 0; i < 2 * tap_num; i++) {
> > > +		if (tap[i])
> > > +			ok_count++;
> > > +		else
> > > +			ok_count = 0;
> > > +		if (ok_count == 3)
> > > +			break;
> > > +	}
> > 
> > As I pointed on [1/3] patch, having "tap_array_size" on this function,
> > and check tap_num <-> tap_array_size is nice idea IMO.
> 
> We could also provide the array parameter and
> not read tap_num using sd_scc_read32() here at all.
> Which do you feel is best?

Not sure, but getting array_size as parameter is very normal for me.
This sd_scc_read32() is using SH_MOBILE_xxx (Renesas specific register).
So, we can use it as sanitary check ?
I don't care about it, its up to you :)

Re: [PATCH/RFC v2 0/3] UHS-I SDR-104 support for sh_mobile_sdhi

[Simon Horman]

On Fri, May 20, 2016 at 04:02:20PM +0900, Simon Horman wrote:
> 
> Hi,
> 
> this series is based on work by Ai Kyuse to add UHS-I SDR-104 support for
> sh_mobile_sdhi. It builds on work by Shinobu Uehara, Rob Taylor, William
> Towle and Ian Molton, Ben Hutchings, Wolfram Sang and others to add UHS-I
> SDR-50 support to the same driver.
> 
> This series enables UHS-I SDR-104 on the r8a7790/Lager where
> UHS-I SDR-50 is already enabled.
> 
> It is based on a merge of:
> * The next branch of the mmc tree  
> * The renesas-next-20160427v2-v4.6-rc1 tag of the renesas tree
> * The sh-pfc-for-v4.7-tag1 tag of the renesas-drivers tree
> 
> The first two mmc driver patches are targeted at the next branch of the mmc
> tree. The last DT patch is targeted at the devel branch of the renesas tree.
> 
> To aid review this series is provided in the _temporary_ topic/sdr104-v2 branch
> of the renesas tree.
> 
> 
> I received somewhat extensive review of v1 of this series and I have
> attempted to rework, correspondingly extensively, these patches to reflect
> that review. I apologise if I missed anything noted in v1. And given the
> extent of the changes since then I fully expect to need to follow-up with v3.
> 
> A pleasant side effect of the above mentioned changes is
> that v2 has roughly 40% fewer lines of code than v1 :)
> 
> 
> Please see "[PATCH/RFC 0/3] UHS-I SDR-104 support for sh_mobile_sdhi" for
> indicative test results: I do not believe the numbers have not changed
> in a material way since then.

I accidently omitted one patch from this series, some irq updates for tmio.
I will post it as part of v3. It is already available at the branch
mentioned above.

> Ai Kyuse (2):
>   mmc: tmio: Add tuning support
>   mmc: sh_mobile_sdhi: Add tuning support
> 
> Simon Horman (1):
>   ARM: dts: r8a7790: lager: Enable UHS-I SDR-104
> 
>  arch/arm/boot/dts/r8a7790-lager.dts |   1 +
>  drivers/mmc/host/sh_mobile_sdhi.c   | 216 +++++++++++++++++++++++++++++++++++-
>  drivers/mmc/host/tmio_mmc.h         |   6 +
>  drivers/mmc/host/tmio_mmc_pio.c     |  85 +++++++++++++-
>  4 files changed, 306 insertions(+), 2 deletions(-)
> 
> -- 
> 2.1.4
>