All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v8 0/6] UHS-I SDR-104 support for sh_mobile_sdhi
@ 2016-11-03 14:15 Simon Horman
  2016-11-03 14:15 ` [PATCH v8 1/6] mmc: core: Add helper to see if a host can be retuned Simon Horman
                   ` (6 more replies)
  0 siblings, 7 replies; 33+ messages in thread
From: Simon Horman @ 2016-11-03 14:15 UTC (permalink / raw)
  To: Wolfram Sang, Ulf Hansson
  Cc: Magnus Damm, linux-mmc, linux-renesas-soc, 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.

It is based on the next branch of the mmc tree.

To aid review the following git branch is provided:
* https:://git.kernel.org/pub/scm/linux/kernel/git/horms/renesas.git topic/sdr104-driver-v8

Overview of changes since v7:
* Correct inverted logic in sh_mobile_sdhi_hw_reset
* Correct value of SH_MOBILE_SDHI_SCC_RVSCNTL_RVSEN

Please see http://elinux.org/Tests:SD-SDHI-SDR104 for indicative tests
results.


Ai Kyuse (3):
  mmc: tmio: enhance illegal sequence handling
  mmc: tmio: Add hw reset support
  mmc: tmio: Add tuning support

Simon Horman (3):
  mmc: core: Add helper to see if a host can be retuned
  mmc: tmio: document mandatory and optional callbacks
  mmc: sh_mobile_sdhi: Add tuning support

 drivers/mmc/host/sh_mobile_sdhi.c | 265 +++++++++++++++++++++++++++++++++++++-
 drivers/mmc/host/tmio_mmc.h       |  18 ++-
 drivers/mmc/host/tmio_mmc_pio.c   |  87 ++++++++++++-
 include/linux/mmc/host.h          |   5 +
 4 files changed, 367 insertions(+), 8 deletions(-)

-- 
2.7.0.rc3.207.g0ac5344

^ permalink raw reply	[flat|nested] 33+ messages in thread

* [PATCH v8 1/6] mmc: core: Add helper to see if a host can be retuned
  2016-11-03 14:15 [PATCH v8 0/6] UHS-I SDR-104 support for sh_mobile_sdhi Simon Horman
@ 2016-11-03 14:15 ` Simon Horman
  2016-11-03 14:16 ` [PATCH v8 2/6] mmc: tmio: enhance illegal sequence handling Simon Horman
                   ` (5 subsequent siblings)
  6 siblings, 0 replies; 33+ messages in thread
From: Simon Horman @ 2016-11-03 14:15 UTC (permalink / raw)
  To: Wolfram Sang, Ulf Hansson
  Cc: Magnus Damm, linux-mmc, linux-renesas-soc, Simon Horman

This is in preparation for restoring saved tuning parameters
when resuming the TMIO driver.

Signed-off-by: Simon Horman <horms+renesas@verge.net.au>
---
v6
* New patch
---
 include/linux/mmc/host.h | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
index 0b2439441cc8..28e2f53c7c4d 100644
--- a/include/linux/mmc/host.h
+++ b/include/linux/mmc/host.h
@@ -546,6 +546,11 @@ static inline void mmc_retune_recheck(struct mmc_host *host)
 		host->retune_now = 1;
 }
 
+static inline bool mmc_can_retune(struct mmc_host *host)
+{
+	return host->can_retune == 1;
+}
+
 void mmc_retune_pause(struct mmc_host *host);
 void mmc_retune_unpause(struct mmc_host *host);
 
-- 
2.7.0.rc3.207.g0ac5344

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH v8 2/6] mmc: tmio: enhance illegal sequence handling
  2016-11-03 14:15 [PATCH v8 0/6] UHS-I SDR-104 support for sh_mobile_sdhi Simon Horman
  2016-11-03 14:15 ` [PATCH v8 1/6] mmc: core: Add helper to see if a host can be retuned Simon Horman
@ 2016-11-03 14:16 ` Simon Horman
  2016-11-03 14:16 ` [PATCH v8 3/6] mmc: tmio: document mandatory and optional callbacks Simon Horman
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 33+ messages in thread
From: Simon Horman @ 2016-11-03 14:16 UTC (permalink / raw)
  To: Wolfram Sang, Ulf Hansson
  Cc: Magnus Damm, linux-mmc, linux-renesas-soc, Ai Kyuse, Simon Horman

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

An illegal sequence command error may occur if there is a stopbit or
cmd_index error as well as a CRC error. The correct course of action
is to re-enable IRQs

An illegal sequence data error may occur if there is a CRC or stopbit
error,  or underrun. In this case set data->error correctly.

This is in preparation for enabling tuning support which relies on
differentiating between illegal sequence and other errors.

Signed-off-by: Ai Kyuse <ai.kyuse.uw@renesas.com>
[simon: broken out of a larger patch]
Signed-off-by: Simon Horman <horms+renesas@verge.net.au>
Reviewed-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
v4
* Added Ack from Wolfram Sang

v3
* No change

v2
* Broken out of a larger patch (but forgot to post)
---
 drivers/mmc/host/tmio_mmc_pio.c | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/drivers/mmc/host/tmio_mmc_pio.c b/drivers/mmc/host/tmio_mmc_pio.c
index 700567603107..0ae967087ea1 100644
--- a/drivers/mmc/host/tmio_mmc_pio.c
+++ b/drivers/mmc/host/tmio_mmc_pio.c
@@ -522,7 +522,7 @@ void tmio_mmc_do_data_irq(struct tmio_mmc_host *host)
 	schedule_work(&host->done);
 }
 
-static void tmio_mmc_data_irq(struct tmio_mmc_host *host)
+static void tmio_mmc_data_irq(struct tmio_mmc_host *host, unsigned int stat)
 {
 	struct mmc_data *data;
 	spin_lock(&host->lock);
@@ -531,6 +531,9 @@ static void tmio_mmc_data_irq(struct tmio_mmc_host *host)
 	if (!data)
 		goto out;
 
+	if (stat & TMIO_STAT_CRCFAIL || stat & TMIO_STAT_STOPBIT_ERR ||
+	    stat & TMIO_STAT_TXUNDERRUN)
+		data->error = -EILSEQ;
 	if (host->chan_tx && (data->flags & MMC_DATA_WRITE) && !host->force_pio) {
 		u32 status = sd_ctrl_read16_and_16_as_32(host, CTL_STATUS);
 		bool done = false;
@@ -579,8 +582,6 @@ static void tmio_mmc_cmd_irq(struct tmio_mmc_host *host,
 		goto out;
 	}
 
-	host->cmd = NULL;
-
 	/* This controller is sicker than the PXA one. Not only do we need to
 	 * drop the top 8 bits of the first response word, we also need to
 	 * modify the order of the response for short response command types.
@@ -600,14 +601,16 @@ static void tmio_mmc_cmd_irq(struct tmio_mmc_host *host,
 
 	if (stat & TMIO_STAT_CMDTIMEOUT)
 		cmd->error = -ETIMEDOUT;
-	else if (stat & TMIO_STAT_CRCFAIL && cmd->flags & MMC_RSP_CRC)
+	else if ((stat & TMIO_STAT_CRCFAIL && cmd->flags & MMC_RSP_CRC) ||
+		 stat & TMIO_STAT_STOPBIT_ERR ||
+		 stat & TMIO_STAT_CMD_IDX_ERR)
 		cmd->error = -EILSEQ;
 
 	/* If there is data to handle we enable data IRQs here, and
 	 * we will ultimatley finish the request in the data_end handler.
 	 * If theres no data or we encountered an error, finish now.
 	 */
-	if (host->data && !cmd->error) {
+	if (host->data && (!cmd->error || cmd->error == -EILSEQ)) {
 		if (host->data->flags & MMC_DATA_READ) {
 			if (host->force_pio || !host->chan_rx)
 				tmio_mmc_enable_mmc_irqs(host, TMIO_MASK_READOP);
@@ -668,7 +671,7 @@ static bool __tmio_mmc_sdcard_irq(struct tmio_mmc_host *host,
 	/* Data transfer completion */
 	if (ireg & TMIO_STAT_DATAEND) {
 		tmio_mmc_ack_mmc_irqs(host, TMIO_STAT_DATAEND);
-		tmio_mmc_data_irq(host);
+		tmio_mmc_data_irq(host, status);
 		return true;
 	}
 
-- 
2.7.0.rc3.207.g0ac5344

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH v8 3/6] mmc: tmio: document mandatory and optional callbacks
  2016-11-03 14:15 [PATCH v8 0/6] UHS-I SDR-104 support for sh_mobile_sdhi Simon Horman
  2016-11-03 14:15 ` [PATCH v8 1/6] mmc: core: Add helper to see if a host can be retuned Simon Horman
  2016-11-03 14:16 ` [PATCH v8 2/6] mmc: tmio: enhance illegal sequence handling Simon Horman
@ 2016-11-03 14:16 ` Simon Horman
  2016-11-03 14:16 ` [PATCH v8 4/6] mmc: tmio: Add hw reset support Simon Horman
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 33+ messages in thread
From: Simon Horman @ 2016-11-03 14:16 UTC (permalink / raw)
  To: Wolfram Sang, Ulf Hansson
  Cc: Magnus Damm, linux-mmc, linux-renesas-soc, Simon Horman

Signed-off-by: Simon Horman <horms+renesas@verge.net.au>
---
v5
* New patch
---
 drivers/mmc/host/tmio_mmc.h | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/mmc/host/tmio_mmc.h b/drivers/mmc/host/tmio_mmc.h
index 8e126afd988c..b93762950e8f 100644
--- a/drivers/mmc/host/tmio_mmc.h
+++ b/drivers/mmc/host/tmio_mmc.h
@@ -154,8 +154,10 @@ struct tmio_mmc_host {
 	bool			native_hotplug;
 	bool			sdio_irq_enabled;
 
-	int (*write16_hook)(struct tmio_mmc_host *host, int addr);
+	/* Mandatory callback */
 	int (*clk_enable)(struct tmio_mmc_host *host);
+
+	/* Optional callbacks */
 	unsigned int (*clk_update)(struct tmio_mmc_host *host,
 				   unsigned int new_clock);
 	void (*clk_disable)(struct tmio_mmc_host *host);
@@ -164,6 +166,7 @@ struct tmio_mmc_host {
 	int (*card_busy)(struct mmc_host *mmc);
 	int (*start_signal_voltage_switch)(struct mmc_host *mmc,
 					   struct mmc_ios *ios);
+	int (*write16_hook)(struct tmio_mmc_host *host, int addr);
 };
 
 struct tmio_mmc_host *tmio_mmc_host_alloc(struct platform_device *pdev);
-- 
2.7.0.rc3.207.g0ac5344

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH v8 4/6] mmc: tmio: Add hw reset support
  2016-11-03 14:15 [PATCH v8 0/6] UHS-I SDR-104 support for sh_mobile_sdhi Simon Horman
                   ` (2 preceding siblings ...)
  2016-11-03 14:16 ` [PATCH v8 3/6] mmc: tmio: document mandatory and optional callbacks Simon Horman
@ 2016-11-03 14:16 ` Simon Horman
  2016-11-03 14:16 ` [PATCH v8 5/6] mmc: tmio: Add tuning support Simon Horman
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 33+ messages in thread
From: Simon Horman @ 2016-11-03 14:16 UTC (permalink / raw)
  To: Wolfram Sang, Ulf Hansson
  Cc: Magnus Damm, linux-mmc, linux-renesas-soc, Ai Kyuse, Simon Horman

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

Add hw reset support.

Signed-off-by: Ai Kyuse <ai.kyuse.uw@renesas.com>
Signed-off-by: Simon Horman <horms+renesas@verge.net.au>
---
This is required by tuning support which will
be introduced by follow-up patches.

v6
* Rely on core to retune on reset

v5
* As suggested by Ulf Hansson
  - Broke out of a larger patch
---
 drivers/mmc/host/tmio_mmc.h     | 1 +
 drivers/mmc/host/tmio_mmc_pio.c | 9 +++++++++
 2 files changed, 10 insertions(+)

diff --git a/drivers/mmc/host/tmio_mmc.h b/drivers/mmc/host/tmio_mmc.h
index b93762950e8f..c5adf52fc049 100644
--- a/drivers/mmc/host/tmio_mmc.h
+++ b/drivers/mmc/host/tmio_mmc.h
@@ -167,6 +167,7 @@ struct tmio_mmc_host {
 	int (*start_signal_voltage_switch)(struct mmc_host *mmc,
 					   struct mmc_ios *ios);
 	int (*write16_hook)(struct tmio_mmc_host *host, int addr);
+	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 0ae967087ea1..ce6e93758576 100644
--- a/drivers/mmc/host/tmio_mmc_pio.c
+++ b/drivers/mmc/host/tmio_mmc_pio.c
@@ -759,6 +759,14 @@ 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);
+}
+
 /* Process requests from the MMC layer */
 static void tmio_mmc_request(struct mmc_host *mmc, struct mmc_request *mrq)
 {
@@ -975,6 +983,7 @@ static struct mmc_host_ops tmio_mmc_ops = {
 	.get_cd		= mmc_gpio_get_cd,
 	.enable_sdio_irq = tmio_mmc_enable_sdio_irq,
 	.multi_io_quirk	= tmio_multi_io_quirk,
+	.hw_reset	= tmio_mmc_hw_reset,
 };
 
 static int tmio_mmc_init_ocr(struct tmio_mmc_host *host)
-- 
2.7.0.rc3.207.g0ac5344

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH v8 5/6] mmc: tmio: Add tuning support
  2016-11-03 14:15 [PATCH v8 0/6] UHS-I SDR-104 support for sh_mobile_sdhi Simon Horman
                   ` (3 preceding siblings ...)
  2016-11-03 14:16 ` [PATCH v8 4/6] mmc: tmio: Add hw reset support Simon Horman
@ 2016-11-03 14:16 ` Simon Horman
  2016-11-03 14:16 ` [PATCH v8 6/6] mmc: sh_mobile_sdhi: " Simon Horman
  2016-11-10 11:45 ` [PATCH v8 0/6] UHS-I SDR-104 support for sh_mobile_sdhi Wolfram Sang
  6 siblings, 0 replies; 33+ messages in thread
From: Simon Horman @ 2016-11-03 14:16 UTC (permalink / raw)
  To: Wolfram Sang, Ulf Hansson
  Cc: Magnus Damm, linux-mmc, linux-renesas-soc, Ai Kyuse, 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>
---
v7 [Simon Horman]
* As per review by Ulf Hansson:
  - Use tmio_mmc_hw_reset() rather than mmc_hw_reset() in
    tmio_mmc_execute_tuning()
  - Do not zero host->mmc->retune_period in tmio_mmc_execute_tuning()
  - Do not perform a reset if host->select_tuning() fails in
    tmio_mmc_host_runtime_resume() retuning should be
    performed if a request subsequently fails.
* Do not implicitly trigger retuning if host->check_scc_error() returns true

v6 [Simon Horman]
* As suggested by Ulf Hansson:
  - Restore saved tuning parameters on resume

v5 [Simon Horman]
* As suggested by Ulf Hansson:
  - Move hw reset support into a separate patch
  - Use more descriptive name for callback to check for SSC error
  - Rely on core to retune in case of -EILSEQ
  - Document mandatory and optional callbacks

v4 [Simon Horman]
* As suggested by Wolfram Sang:
  - Do not perform tuning if host->select_tuning is not set:
    it seems to make little sense to do so and moreover there is currently
    no such use-case
  - Do not add mrc->sbc handling from tmio_mmc_request,
    this is a hang-over from earlier versions of this patchset which
    did not use core infrastructure for retuning
  - Tidy up local variable usage
* Correct index passed to prepare_tuning(): this seems to have
  been the last piece of resolving the timeouts during tuning puzzle
* Further cleanups to tmio_mmc_execute_tuning():
  - Ensure tap is sized proportionally to its members
  - Remove stray '*' in comment
  - Use mmc rather than host->mmc, these are equivalent but
    the former seems tidier
  - Correct inverted logic in setting tap values
* Re-introduce retuning support. This was removed in v3.

v3 [Simon Horman]
* As suggested by Kuninori Morimoto:
  - Do not add unused retuning callback to struct tmio_mmc_host
  - Change return type of prepare_tuning callback to void
  - Add tap_size parameter to select_tuning callback

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     | 12 ++++++++
 drivers/mmc/host/tmio_mmc_pio.c | 63 +++++++++++++++++++++++++++++++++++++++++
 2 files changed, 75 insertions(+)

diff --git a/drivers/mmc/host/tmio_mmc.h b/drivers/mmc/host/tmio_mmc.h
index c5adf52fc049..637581faf756 100644
--- a/drivers/mmc/host/tmio_mmc.h
+++ b/drivers/mmc/host/tmio_mmc.h
@@ -153,6 +153,7 @@ struct tmio_mmc_host {
 	struct mutex		ios_lock;	/* protect set_ios() context */
 	bool			native_hotplug;
 	bool			sdio_irq_enabled;
+	u32			scc_tappos;
 
 	/* Mandatory callback */
 	int (*clk_enable)(struct tmio_mmc_host *host);
@@ -168,6 +169,17 @@ struct tmio_mmc_host {
 					   struct mmc_ios *ios);
 	int (*write16_hook)(struct tmio_mmc_host *host, int addr);
 	void (*hw_reset)(struct tmio_mmc_host *host);
+	void (*prepare_tuning)(struct tmio_mmc_host *host, unsigned long tap);
+	bool (*check_scc_error)(struct tmio_mmc_host *host);
+
+	/* Mandatory callback for tuning to occur which is
+	 * optional for SDR50 and mandatory for SDR104 */
+	unsigned int (*init_tuning)(struct tmio_mmc_host *host);
+	int (*select_tuning)(struct tmio_mmc_host *host);
+
+	/* Tuning values: 1 for success, 0 for failure */
+	DECLARE_BITMAP(taps, BITS_PER_BYTE * sizeof(long));
+	unsigned int tap_num;
 };
 
 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 ce6e93758576..a0f05eb4f344 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>
@@ -298,6 +299,9 @@ static void tmio_mmc_finish_request(struct tmio_mmc_host *host)
 	if (mrq->cmd->error || (mrq->data && mrq->data->error))
 		tmio_mmc_abort_dma(host);
 
+	if (host->check_scc_error)
+		host->check_scc_error(host);
+
 	mmc_request_done(host->mmc, mrq);
 }
 
@@ -767,6 +771,56 @@ static void tmio_mmc_hw_reset(struct mmc_host *mmc)
 		host->hw_reset(host);
 }
 
+static int tmio_mmc_execute_tuning(struct mmc_host *mmc, u32 opcode)
+{
+	struct tmio_mmc_host *host = mmc_priv(mmc);
+	int i, ret = 0;
+
+	if (!host->tap_num) {
+		if (!host->init_tuning || !host->select_tuning)
+			/* Tuning is not supported */
+			goto out;
+
+		host->tap_num = host->init_tuning(host);
+		if (!host->tap_num)
+			/* Tuning is not supported */
+			goto out;
+	}
+
+	if (host->tap_num * 2 >= sizeof(host->taps) * BITS_PER_BYTE) {
+		dev_warn_once(&host->pdev->dev,
+		      "Too many taps, skipping tuning. Please consider "
+		      "updating size of taps field of tmio_mmc_host\n");
+		goto out;
+	}
+
+	bitmap_zero(host->taps, host->tap_num * 2);
+
+	/* Issue CMD19 twice for each tap */
+	for (i = 0; i < 2 * host->tap_num; i++) {
+		if (host->prepare_tuning)
+			host->prepare_tuning(host, i % host->tap_num);
+
+		ret = mmc_send_tuning(mmc, opcode, NULL);
+		if (ret && ret != -EILSEQ)
+			goto out;
+		if (ret == 0)
+			set_bit(i, host->taps);
+
+		mdelay(1);
+	}
+
+	ret = host->select_tuning(host);
+
+out:
+	if (ret < 0) {
+		dev_warn(&host->pdev->dev, "Tuning procedure failed\n");
+		tmio_mmc_hw_reset(mmc);
+	}
+
+	return ret;
+}
+
 /* Process requests from the MMC layer */
 static void tmio_mmc_request(struct mmc_host *mmc, struct mmc_request *mrq)
 {
@@ -984,6 +1038,7 @@ static struct mmc_host_ops tmio_mmc_ops = {
 	.enable_sdio_irq = tmio_mmc_enable_sdio_irq,
 	.multi_io_quirk	= tmio_multi_io_quirk,
 	.hw_reset	= tmio_mmc_hw_reset,
+	.execute_tuning = tmio_mmc_execute_tuning,
 };
 
 static int tmio_mmc_init_ocr(struct tmio_mmc_host *host)
@@ -1230,6 +1285,11 @@ int tmio_mmc_host_runtime_suspend(struct device *dev)
 }
 EXPORT_SYMBOL(tmio_mmc_host_runtime_suspend);
 
+static bool tmio_mmc_can_retune(struct tmio_mmc_host *host)
+{
+	return host->tap_num && mmc_can_retune(host->mmc);
+}
+
 int tmio_mmc_host_runtime_resume(struct device *dev)
 {
 	struct mmc_host *mmc = dev_get_drvdata(dev);
@@ -1243,6 +1303,9 @@ int tmio_mmc_host_runtime_resume(struct device *dev)
 
 	tmio_mmc_enable_dma(host, true);
 
+	if (tmio_mmc_can_retune(host) && host->select_tuning(host))
+		dev_warn(&host->pdev->dev, "Tuning selection failed\n");
+
 	return 0;
 }
 EXPORT_SYMBOL(tmio_mmc_host_runtime_resume);
-- 
2.7.0.rc3.207.g0ac5344

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH v8 6/6] mmc: sh_mobile_sdhi: Add tuning support
  2016-11-03 14:15 [PATCH v8 0/6] UHS-I SDR-104 support for sh_mobile_sdhi Simon Horman
                   ` (4 preceding siblings ...)
  2016-11-03 14:16 ` [PATCH v8 5/6] mmc: tmio: Add tuning support Simon Horman
@ 2016-11-03 14:16 ` Simon Horman
  2017-01-10 21:08     ` Niklas Söderlund
  2016-11-10 11:45 ` [PATCH v8 0/6] UHS-I SDR-104 support for sh_mobile_sdhi Wolfram Sang
  6 siblings, 1 reply; 33+ messages in thread
From: Simon Horman @ 2016-11-03 14:16 UTC (permalink / raw)
  To: Wolfram Sang, Ulf Hansson
  Cc: Magnus Damm, linux-mmc, linux-renesas-soc, Simon Horman, Ai Kyuse

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

Based on work by Ai Kyuse.

Cc: Ai Kyuse <ai.kyuse.uw@renesas.com>
Signed-off-by: Simon Horman <horms+renesas@verge.net.au>
---
v8 [Simon Horman]
* Correct inverted logic in sh_mobile_sdhi_hw_reset
* Correct value of SH_MOBILE_SDHI_SCC_RVSCNTL_RVSEN

v7 [Simon Horman]
* No change

v6 [Simon Horman]
* Rebase to use host->taps

v5 [Simon Horman]
* As suggested by Ulf Hansson
  - Use more descriptive name for callback to check for SSC error
* Reinstate Gen3 tuning support

v4 [Simon Horman]
As suggested by Geert Uytterhoeven:
* guard SDR104 specific portion of probe with host->mmc->caps &
  MMC_CAP_UHS_SDR104
* As suggested by Wolfram Sang:
  - Pass priv to sd_scc_{read,write}32 to save host_to_priv access
    for each call
  - Use 0x0 instead of other representations of 0 in hex
  - Use CLK_CTL_SCLKEN
  - Do not add unused SH_MOBILE_SDHI_MAX_TAP
  - Use ternary operator in sh_mobile_sdhi_select_tuning
  - Do include as yet unsupported HS200 in error string
* Reintroduce retuning support: This was removed in v3.
* Revert to algorithm in v1 patchset, on further reading of the documentation
  it appears to be correct.

v3 [Simon Horman]
* As suggested by Kuninori Morimoto:
  - Do not add unused retuning callback to struct tmio_mmc_host
  - Change return type of prepare_tuning callback to void
  - Add tap_size parameter to select_tuning callback

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
* 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

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]

Signed-off-by: Simon Horman <horms+renesas@verge.net.au>
---
 drivers/mmc/host/sh_mobile_sdhi.c | 265 +++++++++++++++++++++++++++++++++++++-
 1 file changed, 264 insertions(+), 1 deletion(-)

diff --git a/drivers/mmc/host/sh_mobile_sdhi.c b/drivers/mmc/host/sh_mobile_sdhi.c
index 49edff7fee49..15c77bc6e4ee 100644
--- a/drivers/mmc/host/sh_mobile_sdhi.c
+++ b/drivers/mmc/host/sh_mobile_sdhi.c
@@ -47,6 +47,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;
@@ -54,6 +59,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 = {
@@ -66,12 +74,35 @@ 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),
+};
+
+/* Definitions for sampling clocks */
+static struct sh_mobile_sdhi_scc rcar_gen3_scc_taps[] = {
+	{
+		.clk_rate = 0,
+		.tap = 0x00000300,
+	},
 };
 
 static const struct sh_mobile_sdhi_of_data of_rcar_gen3_compatible = {
@@ -79,6 +110,9 @@ static const struct sh_mobile_sdhi_of_data of_rcar_gen3_compatible = {
 			  TMIO_MMC_CLK_ACTUAL | TMIO_MMC_MIN_RCAR2,
 	.capabilities	= MMC_CAP_SD_HIGHSPEED | MMC_CAP_SDIO_IRQ,
 	.bus_shift	= 2,
+	.scc_offset	= 0x1000,
+	.taps		= rcar_gen3_scc_taps,
+	.taps_num	= ARRAY_SIZE(rcar_gen3_scc_taps),
 };
 
 static const struct of_device_id sh_mobile_sdhi_of_match[] = {
@@ -105,6 +139,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)
@@ -255,6 +290,201 @@ 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(0)
+/* 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,
+				struct sh_mobile_sdhi *priv, int addr)
+{
+	return readl(priv->scc_ctl + (addr << host->bus_shift));
+}
+
+static inline void sd_scc_write32(struct tmio_mmc_host *host,
+				  struct sh_mobile_sdhi *priv,
+				  int addr, u32 val)
+{
+	writel(val, priv->scc_ctl + (addr << host->bus_shift));
+}
+
+static unsigned int sh_mobile_sdhi_init_tuning(struct tmio_mmc_host *host)
+{
+	struct sh_mobile_sdhi *priv;
+
+	if (!(host->mmc->caps & MMC_CAP_UHS_SDR104))
+		return 0;
+
+	priv = host_to_priv(host);
+
+	/* set sampling clock selection range */
+	sd_scc_write32(host, priv, 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, 0x0);
+
+	sd_scc_write32(host, priv, SH_MOBILE_SDHI_SCC_DTCNTL,
+		       SH_MOBILE_SDHI_SCC_DTCNTL_TAPEN |
+		       sd_scc_read32(host, priv, SH_MOBILE_SDHI_SCC_DTCNTL));
+
+	sd_ctrl_write16(host, CTL_SD_CARD_CLK_CTL, ~CLK_CTL_SCLKEN &
+			sd_ctrl_read16(host, CTL_SD_CARD_CLK_CTL));
+
+	sd_scc_write32(host, priv, SH_MOBILE_SDHI_SCC_CKSEL,
+		       SH_MOBILE_SDHI_SCC_CKSEL_DTSEL |
+		       sd_scc_read32(host, priv, SH_MOBILE_SDHI_SCC_CKSEL));
+
+	sd_ctrl_write16(host, CTL_SD_CARD_CLK_CTL, CLK_CTL_SCLKEN |
+			sd_ctrl_read16(host, CTL_SD_CARD_CLK_CTL));
+
+	sd_scc_write32(host, priv, SH_MOBILE_SDHI_SCC_RVSCNTL,
+		       ~SH_MOBILE_SDHI_SCC_RVSCNTL_RVSEN &
+		       sd_scc_read32(host, priv, SH_MOBILE_SDHI_SCC_RVSCNTL));
+
+	sd_scc_write32(host, priv, SH_MOBILE_SDHI_SCC_DT2FF, host->scc_tappos);
+
+	/* Read TAPNUM */
+	return (sd_scc_read32(host, priv, SH_MOBILE_SDHI_SCC_DTCNTL) >>
+		SH_MOBILE_SDHI_SCC_DTCNTL_TAPNUM_SHIFT) &
+		SH_MOBILE_SDHI_SCC_DTCNTL_TAPNUM_MASK;
+}
+
+static void sh_mobile_sdhi_prepare_tuning(struct tmio_mmc_host *host,
+					 unsigned long tap)
+{
+	struct sh_mobile_sdhi *priv = host_to_priv(host);
+
+	/* Set sampling clock position */
+	sd_scc_write32(host, priv, SH_MOBILE_SDHI_SCC_TAPSET, tap);
+}
+
+#define SH_MOBILE_SDHI_MAX_TAP 3
+
+static int sh_mobile_sdhi_select_tuning(struct tmio_mmc_host *host)
+{
+	struct sh_mobile_sdhi *priv = host_to_priv(host);
+	unsigned long tap_cnt;  /* counter of tuning success */
+	unsigned long tap_set;  /* tap position */
+	unsigned long tap_start;/* start position of tuning success */
+	unsigned long tap_end;  /* end position of tuning success */
+	unsigned long ntap;     /* temporary counter of tuning success */
+	unsigned long i;
+
+	/* Clear SCC_RVSREQ */
+	sd_scc_write32(host, priv, SH_MOBILE_SDHI_SCC_RVSREQ, 0);
+
+	/*
+	 * Find the longest consecutive run of successful probes.  If that
+	 * is more than SH_MOBILE_SDHI_MAX_TAP probes long then use the
+	 * center index as the tap.
+	 */
+	tap_cnt = 0;
+	ntap = 0;
+	tap_start = 0;
+	tap_end = 0;
+	for (i = 0; i < host->tap_num * 2; i++) {
+		if (test_bit(i, host->taps))
+			ntap++;
+		else {
+			if (ntap > tap_cnt) {
+				tap_start = i - ntap;
+				tap_end = i - 1;
+				tap_cnt = ntap;
+			}
+			ntap = 0;
+		}
+	}
+
+	if (ntap > tap_cnt) {
+		tap_start = i - ntap;
+		tap_end = i - 1;
+		tap_cnt = ntap;
+	}
+
+	if (tap_cnt >= SH_MOBILE_SDHI_MAX_TAP)
+		tap_set = (tap_start + tap_end) / 2 % host->tap_num;
+	else
+		return -EIO;
+
+	/* Set SCC */
+	sd_scc_write32(host, priv, SH_MOBILE_SDHI_SCC_TAPSET, tap_set);
+
+	/* Enable auto re-tuning */
+	sd_scc_write32(host, priv, SH_MOBILE_SDHI_SCC_RVSCNTL,
+		       SH_MOBILE_SDHI_SCC_RVSCNTL_RVSEN |
+		       sd_scc_read32(host, priv, SH_MOBILE_SDHI_SCC_RVSCNTL));
+
+	return 0;
+}
+
+
+static bool sh_mobile_sdhi_check_scc_error(struct tmio_mmc_host *host)
+{
+	struct sh_mobile_sdhi *priv;
+
+	if (!(host->mmc->caps & MMC_CAP_UHS_SDR104))
+		return 0;
+
+	priv = host_to_priv(host);
+
+	/* Check SCC error */
+	if (sd_scc_read32(host, priv, SH_MOBILE_SDHI_SCC_RVSCNTL) &
+	    SH_MOBILE_SDHI_SCC_RVSCNTL_RVSEN &&
+	    sd_scc_read32(host, priv, SH_MOBILE_SDHI_SCC_RVSREQ) &
+	    SH_MOBILE_SDHI_SCC_RVSREQ_RVSERR) {
+		/* Clear SCC error */
+		sd_scc_write32(host, priv, SH_MOBILE_SDHI_SCC_RVSREQ, 0);
+		return true;
+	}
+
+	return false;
+}
+
+static void sh_mobile_sdhi_hw_reset(struct tmio_mmc_host *host)
+{
+	struct sh_mobile_sdhi *priv;
+
+	if (!(host->mmc->caps & MMC_CAP_UHS_SDR104))
+		return;
+
+	priv = host_to_priv(host);
+
+	/* Reset SCC */
+	sd_ctrl_write16(host, CTL_SD_CARD_CLK_CTL, ~CLK_CTL_SCLKEN &
+			sd_ctrl_read16(host, CTL_SD_CARD_CLK_CTL));
+
+	sd_scc_write32(host, priv, SH_MOBILE_SDHI_SCC_CKSEL,
+		       ~SH_MOBILE_SDHI_SCC_CKSEL_DTSEL &
+		       sd_scc_read32(host, priv, SH_MOBILE_SDHI_SCC_CKSEL));
+
+	sd_ctrl_write16(host, CTL_SD_CARD_CLK_CTL, CLK_CTL_SCLKEN |
+			sd_ctrl_read16(host, CTL_SD_CARD_CLK_CTL));
+
+	sd_scc_write32(host, priv, SH_MOBILE_SDHI_SCC_RVSCNTL,
+		       ~SH_MOBILE_SDHI_SCC_RVSCNTL_RVSEN &
+		       sd_scc_read32(host, priv, SH_MOBILE_SDHI_SCC_RVSCNTL));
+
+	sd_scc_write32(host, priv, SH_MOBILE_SDHI_SCC_RVSCNTL,
+		       ~SH_MOBILE_SDHI_SCC_RVSCNTL_RVSEN &
+		       sd_scc_read32(host, priv, SH_MOBILE_SDHI_SCC_RVSCNTL));
+}
+
 static int sh_mobile_sdhi_wait_idle(struct tmio_mmc_host *host)
 {
 	int timeout = 1000;
@@ -325,7 +555,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);
@@ -384,6 +614,11 @@ static int sh_mobile_sdhi_probe(struct platform_device *pdev)
 		host->card_busy	= sh_mobile_sdhi_card_busy;
 		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->check_scc_error	= sh_mobile_sdhi_check_scc_error;
+		host->hw_reset		= sh_mobile_sdhi_hw_reset;
 	}
 
 	/* Orginally registers were 16 bit apart, could be 32 or 64 nowadays */
@@ -424,6 +659,34 @@ 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;
+			const struct sh_mobile_sdhi_scc *taps;
+			bool hit = false;
+
+			of_data = of_id->data;
+			taps = of_data->taps;
+
+			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\n");
+
+			priv->scc_ctl = host->ctl + of_data->scc_offset;
+		}
+	}
+
+	i = 0;
 	while (1) {
 		irq = platform_get_irq(pdev, i);
 		if (irq < 0)
-- 
2.7.0.rc3.207.g0ac5344

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* Re: [PATCH v8 0/6] UHS-I SDR-104 support for sh_mobile_sdhi
  2016-11-03 14:15 [PATCH v8 0/6] UHS-I SDR-104 support for sh_mobile_sdhi Simon Horman
                   ` (5 preceding siblings ...)
  2016-11-03 14:16 ` [PATCH v8 6/6] mmc: sh_mobile_sdhi: " Simon Horman
@ 2016-11-10 11:45 ` Wolfram Sang
  2016-11-10 22:21   ` Ulf Hansson
  6 siblings, 1 reply; 33+ messages in thread
From: Wolfram Sang @ 2016-11-10 11:45 UTC (permalink / raw)
  To: Simon Horman
  Cc: Wolfram Sang, Ulf Hansson, Magnus Damm, linux-mmc, linux-renesas-soc

[-- Attachment #1: Type: text/plain, Size: 1197 bytes --]

On Thu, Nov 03, 2016 at 03:15:58PM +0100, 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.
> 
> It is based on the next branch of the mmc tree.
> 
> To aid review the following git branch is provided:
> * https:://git.kernel.org/pub/scm/linux/kernel/git/horms/renesas.git topic/sdr104-driver-v8
> 
> Overview of changes since v7:
> * Correct inverted logic in sh_mobile_sdhi_hw_reset
> * Correct value of SH_MOBILE_SDHI_SCC_RVSCNTL_RVSEN
> 
> Please see http://elinux.org/Tests:SD-SDHI-SDR104 for indicative tests
> results.

Successfully tested on a Lager Board with a SanDisk card and a Samsung
card which used to cause issues before.

Tested-by: Wolfram Sang <wsa+renesas@sang-engineering.com>

Ulf, I think this series is ready to go for v4.10. All issues we know of
are related to pinmuxing and will be solved elsewhere. We currently have
hacks for that so it is quite clear the issues are not caused by this
series.


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH v8 0/6] UHS-I SDR-104 support for sh_mobile_sdhi
  2016-11-10 11:45 ` [PATCH v8 0/6] UHS-I SDR-104 support for sh_mobile_sdhi Wolfram Sang
@ 2016-11-10 22:21   ` Ulf Hansson
  2016-11-11  7:20     ` Simon Horman
  0 siblings, 1 reply; 33+ messages in thread
From: Ulf Hansson @ 2016-11-10 22:21 UTC (permalink / raw)
  To: Wolfram Sang, Simon Horman
  Cc: Wolfram Sang, Magnus Damm, linux-mmc, Linux-Renesas

On 10 November 2016 at 12:45, Wolfram Sang <wsa@the-dreams.de> wrote:
> On Thu, Nov 03, 2016 at 03:15:58PM +0100, 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.
>>
>> It is based on the next branch of the mmc tree.
>>
>> To aid review the following git branch is provided:
>> * https:://git.kernel.org/pub/scm/linux/kernel/git/horms/renesas.git topic/sdr104-driver-v8
>>
>> Overview of changes since v7:
>> * Correct inverted logic in sh_mobile_sdhi_hw_reset
>> * Correct value of SH_MOBILE_SDHI_SCC_RVSCNTL_RVSEN
>>
>> Please see http://elinux.org/Tests:SD-SDHI-SDR104 for indicative tests
>> results.
>
> Successfully tested on a Lager Board with a SanDisk card and a Samsung
> card which used to cause issues before.
>
> Tested-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
>
> Ulf, I think this series is ready to go for v4.10. All issues we know of
> are related to pinmuxing and will be solved elsewhere. We currently have
> hacks for that so it is quite clear the issues are not caused by this
> series.
>

Thanks, applied for next!

I did have to fix conflict and also to resolve some checkpatch
warnings - but no worries this time!

Kind regards
Uffe

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH v8 0/6] UHS-I SDR-104 support for sh_mobile_sdhi
  2016-11-10 22:21   ` Ulf Hansson
@ 2016-11-11  7:20     ` Simon Horman
  0 siblings, 0 replies; 33+ messages in thread
From: Simon Horman @ 2016-11-11  7:20 UTC (permalink / raw)
  To: Ulf Hansson
  Cc: Wolfram Sang, Wolfram Sang, Magnus Damm, linux-mmc, Linux-Renesas

On Thu, Nov 10, 2016 at 11:21:08PM +0100, Ulf Hansson wrote:
> On 10 November 2016 at 12:45, Wolfram Sang <wsa@the-dreams.de> wrote:
> > On Thu, Nov 03, 2016 at 03:15:58PM +0100, 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.
> >>
> >> It is based on the next branch of the mmc tree.
> >>
> >> To aid review the following git branch is provided:
> >> * https:://git.kernel.org/pub/scm/linux/kernel/git/horms/renesas.git topic/sdr104-driver-v8
> >>
> >> Overview of changes since v7:
> >> * Correct inverted logic in sh_mobile_sdhi_hw_reset
> >> * Correct value of SH_MOBILE_SDHI_SCC_RVSCNTL_RVSEN
> >>
> >> Please see http://elinux.org/Tests:SD-SDHI-SDR104 for indicative tests
> >> results.
> >
> > Successfully tested on a Lager Board with a SanDisk card and a Samsung
> > card which used to cause issues before.
> >
> > Tested-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
> >
> > Ulf, I think this series is ready to go for v4.10. All issues we know of
> > are related to pinmuxing and will be solved elsewhere. We currently have
> > hacks for that so it is quite clear the issues are not caused by this
> > series.
> >
> 
> Thanks, applied for next!
> 
> I did have to fix conflict and also to resolve some checkpatch
> warnings - but no worries this time!

Thanks, sorry for letting those slip through.

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH v8 6/6] mmc: sh_mobile_sdhi: Add tuning support
  2016-11-03 14:16 ` [PATCH v8 6/6] mmc: sh_mobile_sdhi: " Simon Horman
@ 2017-01-10 21:08     ` Niklas Söderlund
  0 siblings, 0 replies; 33+ messages in thread
From: Niklas Söderlund @ 2017-01-10 21:08 UTC (permalink / raw)
  To: Simon Horman
  Cc: Wolfram Sang, Ulf Hansson, Magnus Damm, linux-mmc,
	linux-renesas-soc, Ai Kyuse

Hi Simon,

I started to se errors when I was testing DMAC+IPMMU patches on top of 
v4.10-rc1 on Koelsch.

[    2.247490] [drm] Device feb00000.display probed
[    2.254407] sh_mobile_sdhi ee100000.sd: Got CD GPIO
[    2.259535] sh_mobile_sdhi ee100000.sd: Got WP GPIO
[    2.473871] sh_mobile_sdhi ee100000.sd: mmc0 base at 0xee100000 max clock rate 195 MHz
[    2.484048] sh_mobile_sdhi ee140000.sd: Got CD GPIO
[    2.489175] sh_mobile_sdhi ee140000.sd: Got WP GPIO
[    2.703850] sh_mobile_sdhi ee140000.sd: mmc1 base at 0xee140000 max clock rate 97 MHz
[    2.714083] sh_mobile_sdhi ee160000.sd: Got CD GPIO
[    2.911847] ipmmu-vmsa e6740000.mmu: Unhandled fault: status 0x00002101 iova 0x40002000
[    2.925838] sh_mobile_sdhi ee160000.sd: mmc2 base at 0xee160000 max clock rate 97 MHz
[    2.938057] ipmmu-vmsa e6740000.mmu: Unhandled fault: status 0x00002101 iova 0x40002000
[    2.938342] asoc-simple-card sound: ak4642-hifi <-> ec500000.sound mapping ok
[    2.954859] mmc0: new ultra high speed SDR50 SDHC card at address aaaa
[    2.962604] mmcblk0: mmc0:aaaa SU04G 3.69 GiB 
[    2.971149] input: keyboard as /devices/platform/keyboard/input/input0
[    2.980044] da9063-rtc da9063-rtc: setting system clock to 2017-01-10 20:15:43 UTC (1484079343)
[    2.989814]  mmcblk0: p1
[    3.106206] Micrel KSZ8041RNLI ee700000.etherne:01: attached PHY driver [Micrel KSZ8041RNLI] (mii_bus:phy_addr=ee700000.etherne:01, irq=405)

If I boot the system without a SD card inserted the warnings are not 
printed, however when I insert the SD card they are immediately printed.  
Multiple removal/insertion of a card do not trigger additional warnings, 
only at the first insertion.

Oddly enough the error are only printed when I insert the SD card in the 
mmc0 slot. I can insert/eject the card multiple times in mmc1 and no 
error but the first insertion in mmc0 and boom. Only difference I can 
see are the clock speed between mmc0 and mmc1.

[  125.681585] ipmmu-vmsa e6740000.mmu: Unhandled fault: status 0x00002101 iova 0x40002000
[  125.698839] ipmmu-vmsa e6740000.mmu: Unhandled fault: status 0x00002101 iova 0x40002000
[  125.708228] mmc0: new ultra high speed SDR50 SDHC card at address aaaa
[  125.716394] mmcblk0: mmc0:aaaa SU04G 3.69 GiB 
[  125.737443]  mmcblk0: p1
[  305.365862] mmc0: card aaaa removed
[  307.933518] mmc0: new ultra high speed SDR50 SDHC card at address aaaa
[  307.941948] mmcblk0: mmc0:aaaa SU04G 3.69 GiB 
[  307.964353]  mmcblk0: p1
[  310.965794] mmc0: card aaaa removed
[  317.335789] mmc1: new ultra high speed SDR50 SDHC card at address aaaa
[  317.343172] mmcblk1: mmc1:aaaa SU04G 3.69 GiB 
[  317.364223]  mmcblk1: p1

Sometimes the error is reported 3 times but in most cases only 2.

I can interact fine with the card (I tried checksumming a large file and 
compared with a known good) so it's not broken. I can also interact with 
other devices using the DMAC+IPMMU without similar warnings being 
printed at all, I tested with i2c6.

If i revert this patch 06f438dd389a699d ("mmc: sh_mobile_sdhi: Add 
tuning support") the warnings go away. I have not been able to figure 
out what in the patch triggers the warnings, and I'm not sure the 
problem are with sh_mobile_sdhi. I know to little about the DMAC and 
IPMMU to rule them out as the true source. Do you have any idea what 
might cause this? I'm happy to run more tests or help out in other ways 
if I can.

To reproduce start on v4.10-rc1 and use shmobile_defconfig with a few 
additions:

CONFIG_ARM_LPAE=y
CONFIG_IPMMU_VMSA=y

And I enable IPMMU for DMAC in DT:

diff --git a/arch/arm/boot/dts/r8a7791.dtsi b/arch/arm/boot/dts/r8a7791.dtsi
index 87214668d70f..d4500d79db1d 100644
--- a/arch/arm/boot/dts/r8a7791.dtsi
+++ b/arch/arm/boot/dts/r8a7791.dtsi
@@ -325,6 +325,21 @@
                power-domains = <&sysc R8A7791_PD_ALWAYS_ON>;
                #dma-cells = <1>;
                dma-channels = <15>;
+               iommus = <&ipmmu_ds 0>,
+                        <&ipmmu_ds 1>,
+                        <&ipmmu_ds 2>,
+                        <&ipmmu_ds 3>,
+                        <&ipmmu_ds 4>,
+                        <&ipmmu_ds 5>,
+                        <&ipmmu_ds 6>,
+                        <&ipmmu_ds 7>,
+                        <&ipmmu_ds 8>,
+                        <&ipmmu_ds 9>,
+                        <&ipmmu_ds 10>,
+                        <&ipmmu_ds 11>,
+                        <&ipmmu_ds 12>,
+                        <&ipmmu_ds 13>,
+                        <&ipmmu_ds 14>;
        };
 
        dmac1: dma-controller@e6720000 {
@@ -356,6 +371,21 @@
                power-domains = <&sysc R8A7791_PD_ALWAYS_ON>;
                #dma-cells = <1>;
                dma-channels = <15>;
+               iommus = <&ipmmu_ds 15>,
+                        <&ipmmu_ds 16>,
+                        <&ipmmu_ds 17>,
+                        <&ipmmu_ds 18>,
+                        <&ipmmu_ds 19>,
+                        <&ipmmu_ds 20>,
+                        <&ipmmu_ds 21>,
+                        <&ipmmu_ds 22>,
+                        <&ipmmu_ds 23>,
+                        <&ipmmu_ds 24>,
+                        <&ipmmu_ds 25>,
+                        <&ipmmu_ds 26>,
+                        <&ipmmu_ds 27>,
+                        <&ipmmu_ds 28>,
+                        <&ipmmu_ds 29>;
        };
 
        audma0: dma-controller@ec700000 {
@@ -1688,7 +1718,7 @@
                interrupts = <GIC_SPI 198 IRQ_TYPE_LEVEL_HIGH>,
                             <GIC_SPI 199 IRQ_TYPE_LEVEL_HIGH>;
                #iommu-cells = <1>;
-               status = "disabled";
+               status = "okay";
        };
 
        ipmmu_mp: mmu@ec680000 {

On 2016-11-03 15:16:04 +0100, Simon Horman wrote:
> Add tuning support for use with SDR104 mode
> This includes adding support for the sampling clock controller (SCC).
> 
> Based on work by Ai Kyuse.
> 
> Cc: Ai Kyuse <ai.kyuse.uw@renesas.com>
> Signed-off-by: Simon Horman <horms+renesas@verge.net.au>
> ---
> v8 [Simon Horman]
> * Correct inverted logic in sh_mobile_sdhi_hw_reset
> * Correct value of SH_MOBILE_SDHI_SCC_RVSCNTL_RVSEN
> 
> v7 [Simon Horman]
> * No change
> 
> v6 [Simon Horman]
> * Rebase to use host->taps
> 
> v5 [Simon Horman]
> * As suggested by Ulf Hansson
>   - Use more descriptive name for callback to check for SSC error
> * Reinstate Gen3 tuning support
> 
> v4 [Simon Horman]
> As suggested by Geert Uytterhoeven:
> * guard SDR104 specific portion of probe with host->mmc->caps &
>   MMC_CAP_UHS_SDR104
> * As suggested by Wolfram Sang:
>   - Pass priv to sd_scc_{read,write}32 to save host_to_priv access
>     for each call
>   - Use 0x0 instead of other representations of 0 in hex
>   - Use CLK_CTL_SCLKEN
>   - Do not add unused SH_MOBILE_SDHI_MAX_TAP
>   - Use ternary operator in sh_mobile_sdhi_select_tuning
>   - Do include as yet unsupported HS200 in error string
> * Reintroduce retuning support: This was removed in v3.
> * Revert to algorithm in v1 patchset, on further reading of the documentation
>   it appears to be correct.
> 
> v3 [Simon Horman]
> * As suggested by Kuninori Morimoto:
>   - Do not add unused retuning callback to struct tmio_mmc_host
>   - Change return type of prepare_tuning callback to void
>   - Add tap_size parameter to select_tuning callback
> 
> 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
> * 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
> 
> 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]
> 
> Signed-off-by: Simon Horman <horms+renesas@verge.net.au>
> ---
>  drivers/mmc/host/sh_mobile_sdhi.c | 265 +++++++++++++++++++++++++++++++++++++-
>  1 file changed, 264 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/mmc/host/sh_mobile_sdhi.c b/drivers/mmc/host/sh_mobile_sdhi.c
> index 49edff7fee49..15c77bc6e4ee 100644
> --- a/drivers/mmc/host/sh_mobile_sdhi.c
> +++ b/drivers/mmc/host/sh_mobile_sdhi.c
> @@ -47,6 +47,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;
> @@ -54,6 +59,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 = {
> @@ -66,12 +74,35 @@ 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),
> +};
> +
> +/* Definitions for sampling clocks */
> +static struct sh_mobile_sdhi_scc rcar_gen3_scc_taps[] = {
> +	{
> +		.clk_rate = 0,
> +		.tap = 0x00000300,
> +	},
>  };
>  
>  static const struct sh_mobile_sdhi_of_data of_rcar_gen3_compatible = {
> @@ -79,6 +110,9 @@ static const struct sh_mobile_sdhi_of_data of_rcar_gen3_compatible = {
>  			  TMIO_MMC_CLK_ACTUAL | TMIO_MMC_MIN_RCAR2,
>  	.capabilities	= MMC_CAP_SD_HIGHSPEED | MMC_CAP_SDIO_IRQ,
>  	.bus_shift	= 2,
> +	.scc_offset	= 0x1000,
> +	.taps		= rcar_gen3_scc_taps,
> +	.taps_num	= ARRAY_SIZE(rcar_gen3_scc_taps),
>  };
>  
>  static const struct of_device_id sh_mobile_sdhi_of_match[] = {
> @@ -105,6 +139,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)
> @@ -255,6 +290,201 @@ 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(0)
> +/* 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,
> +				struct sh_mobile_sdhi *priv, int addr)
> +{
> +	return readl(priv->scc_ctl + (addr << host->bus_shift));
> +}
> +
> +static inline void sd_scc_write32(struct tmio_mmc_host *host,
> +				  struct sh_mobile_sdhi *priv,
> +				  int addr, u32 val)
> +{
> +	writel(val, priv->scc_ctl + (addr << host->bus_shift));
> +}
> +
> +static unsigned int sh_mobile_sdhi_init_tuning(struct tmio_mmc_host *host)
> +{
> +	struct sh_mobile_sdhi *priv;
> +
> +	if (!(host->mmc->caps & MMC_CAP_UHS_SDR104))
> +		return 0;
> +
> +	priv = host_to_priv(host);
> +
> +	/* set sampling clock selection range */
> +	sd_scc_write32(host, priv, 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, 0x0);
> +
> +	sd_scc_write32(host, priv, SH_MOBILE_SDHI_SCC_DTCNTL,
> +		       SH_MOBILE_SDHI_SCC_DTCNTL_TAPEN |
> +		       sd_scc_read32(host, priv, SH_MOBILE_SDHI_SCC_DTCNTL));
> +
> +	sd_ctrl_write16(host, CTL_SD_CARD_CLK_CTL, ~CLK_CTL_SCLKEN &
> +			sd_ctrl_read16(host, CTL_SD_CARD_CLK_CTL));
> +
> +	sd_scc_write32(host, priv, SH_MOBILE_SDHI_SCC_CKSEL,
> +		       SH_MOBILE_SDHI_SCC_CKSEL_DTSEL |
> +		       sd_scc_read32(host, priv, SH_MOBILE_SDHI_SCC_CKSEL));
> +
> +	sd_ctrl_write16(host, CTL_SD_CARD_CLK_CTL, CLK_CTL_SCLKEN |
> +			sd_ctrl_read16(host, CTL_SD_CARD_CLK_CTL));
> +
> +	sd_scc_write32(host, priv, SH_MOBILE_SDHI_SCC_RVSCNTL,
> +		       ~SH_MOBILE_SDHI_SCC_RVSCNTL_RVSEN &
> +		       sd_scc_read32(host, priv, SH_MOBILE_SDHI_SCC_RVSCNTL));
> +
> +	sd_scc_write32(host, priv, SH_MOBILE_SDHI_SCC_DT2FF, host->scc_tappos);
> +
> +	/* Read TAPNUM */
> +	return (sd_scc_read32(host, priv, SH_MOBILE_SDHI_SCC_DTCNTL) >>
> +		SH_MOBILE_SDHI_SCC_DTCNTL_TAPNUM_SHIFT) &
> +		SH_MOBILE_SDHI_SCC_DTCNTL_TAPNUM_MASK;
> +}
> +
> +static void sh_mobile_sdhi_prepare_tuning(struct tmio_mmc_host *host,
> +					 unsigned long tap)
> +{
> +	struct sh_mobile_sdhi *priv = host_to_priv(host);
> +
> +	/* Set sampling clock position */
> +	sd_scc_write32(host, priv, SH_MOBILE_SDHI_SCC_TAPSET, tap);
> +}
> +
> +#define SH_MOBILE_SDHI_MAX_TAP 3
> +
> +static int sh_mobile_sdhi_select_tuning(struct tmio_mmc_host *host)
> +{
> +	struct sh_mobile_sdhi *priv = host_to_priv(host);
> +	unsigned long tap_cnt;  /* counter of tuning success */
> +	unsigned long tap_set;  /* tap position */
> +	unsigned long tap_start;/* start position of tuning success */
> +	unsigned long tap_end;  /* end position of tuning success */
> +	unsigned long ntap;     /* temporary counter of tuning success */
> +	unsigned long i;
> +
> +	/* Clear SCC_RVSREQ */
> +	sd_scc_write32(host, priv, SH_MOBILE_SDHI_SCC_RVSREQ, 0);
> +
> +	/*
> +	 * Find the longest consecutive run of successful probes.  If that
> +	 * is more than SH_MOBILE_SDHI_MAX_TAP probes long then use the
> +	 * center index as the tap.
> +	 */
> +	tap_cnt = 0;
> +	ntap = 0;
> +	tap_start = 0;
> +	tap_end = 0;
> +	for (i = 0; i < host->tap_num * 2; i++) {
> +		if (test_bit(i, host->taps))
> +			ntap++;
> +		else {
> +			if (ntap > tap_cnt) {
> +				tap_start = i - ntap;
> +				tap_end = i - 1;
> +				tap_cnt = ntap;
> +			}
> +			ntap = 0;
> +		}
> +	}
> +
> +	if (ntap > tap_cnt) {
> +		tap_start = i - ntap;
> +		tap_end = i - 1;
> +		tap_cnt = ntap;
> +	}
> +
> +	if (tap_cnt >= SH_MOBILE_SDHI_MAX_TAP)
> +		tap_set = (tap_start + tap_end) / 2 % host->tap_num;
> +	else
> +		return -EIO;
> +
> +	/* Set SCC */
> +	sd_scc_write32(host, priv, SH_MOBILE_SDHI_SCC_TAPSET, tap_set);
> +
> +	/* Enable auto re-tuning */
> +	sd_scc_write32(host, priv, SH_MOBILE_SDHI_SCC_RVSCNTL,
> +		       SH_MOBILE_SDHI_SCC_RVSCNTL_RVSEN |
> +		       sd_scc_read32(host, priv, SH_MOBILE_SDHI_SCC_RVSCNTL));
> +
> +	return 0;
> +}
> +
> +
> +static bool sh_mobile_sdhi_check_scc_error(struct tmio_mmc_host *host)
> +{
> +	struct sh_mobile_sdhi *priv;
> +
> +	if (!(host->mmc->caps & MMC_CAP_UHS_SDR104))
> +		return 0;
> +
> +	priv = host_to_priv(host);
> +
> +	/* Check SCC error */
> +	if (sd_scc_read32(host, priv, SH_MOBILE_SDHI_SCC_RVSCNTL) &
> +	    SH_MOBILE_SDHI_SCC_RVSCNTL_RVSEN &&
> +	    sd_scc_read32(host, priv, SH_MOBILE_SDHI_SCC_RVSREQ) &
> +	    SH_MOBILE_SDHI_SCC_RVSREQ_RVSERR) {
> +		/* Clear SCC error */
> +		sd_scc_write32(host, priv, SH_MOBILE_SDHI_SCC_RVSREQ, 0);
> +		return true;
> +	}
> +
> +	return false;
> +}
> +
> +static void sh_mobile_sdhi_hw_reset(struct tmio_mmc_host *host)
> +{
> +	struct sh_mobile_sdhi *priv;
> +
> +	if (!(host->mmc->caps & MMC_CAP_UHS_SDR104))
> +		return;
> +
> +	priv = host_to_priv(host);
> +
> +	/* Reset SCC */
> +	sd_ctrl_write16(host, CTL_SD_CARD_CLK_CTL, ~CLK_CTL_SCLKEN &
> +			sd_ctrl_read16(host, CTL_SD_CARD_CLK_CTL));
> +
> +	sd_scc_write32(host, priv, SH_MOBILE_SDHI_SCC_CKSEL,
> +		       ~SH_MOBILE_SDHI_SCC_CKSEL_DTSEL &
> +		       sd_scc_read32(host, priv, SH_MOBILE_SDHI_SCC_CKSEL));
> +
> +	sd_ctrl_write16(host, CTL_SD_CARD_CLK_CTL, CLK_CTL_SCLKEN |
> +			sd_ctrl_read16(host, CTL_SD_CARD_CLK_CTL));
> +
> +	sd_scc_write32(host, priv, SH_MOBILE_SDHI_SCC_RVSCNTL,
> +		       ~SH_MOBILE_SDHI_SCC_RVSCNTL_RVSEN &
> +		       sd_scc_read32(host, priv, SH_MOBILE_SDHI_SCC_RVSCNTL));
> +
> +	sd_scc_write32(host, priv, SH_MOBILE_SDHI_SCC_RVSCNTL,
> +		       ~SH_MOBILE_SDHI_SCC_RVSCNTL_RVSEN &
> +		       sd_scc_read32(host, priv, SH_MOBILE_SDHI_SCC_RVSCNTL));
> +}
> +
>  static int sh_mobile_sdhi_wait_idle(struct tmio_mmc_host *host)
>  {
>  	int timeout = 1000;
> @@ -325,7 +555,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);
> @@ -384,6 +614,11 @@ static int sh_mobile_sdhi_probe(struct platform_device *pdev)
>  		host->card_busy	= sh_mobile_sdhi_card_busy;
>  		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->check_scc_error	= sh_mobile_sdhi_check_scc_error;
> +		host->hw_reset		= sh_mobile_sdhi_hw_reset;
>  	}
>  
>  	/* Orginally registers were 16 bit apart, could be 32 or 64 nowadays */
> @@ -424,6 +659,34 @@ 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;
> +			const struct sh_mobile_sdhi_scc *taps;
> +			bool hit = false;
> +
> +			of_data = of_id->data;
> +			taps = of_data->taps;
> +
> +			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\n");
> +
> +			priv->scc_ctl = host->ctl + of_data->scc_offset;
> +		}
> +	}
> +
> +	i = 0;
>  	while (1) {
>  		irq = platform_get_irq(pdev, i);
>  		if (irq < 0)
> -- 
> 2.7.0.rc3.207.g0ac5344
> 
> 

-- 
Regards,
Niklas S�derlund

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* Re: [PATCH v8 6/6] mmc: sh_mobile_sdhi: Add tuning support
@ 2017-01-10 21:08     ` Niklas Söderlund
  0 siblings, 0 replies; 33+ messages in thread
From: Niklas Söderlund @ 2017-01-10 21:08 UTC (permalink / raw)
  To: Simon Horman
  Cc: Wolfram Sang, Ulf Hansson, Magnus Damm, linux-mmc,
	linux-renesas-soc, Ai Kyuse

Hi Simon,

I started to se errors when I was testing DMAC+IPMMU patches on top of 
v4.10-rc1 on Koelsch.

[    2.247490] [drm] Device feb00000.display probed
[    2.254407] sh_mobile_sdhi ee100000.sd: Got CD GPIO
[    2.259535] sh_mobile_sdhi ee100000.sd: Got WP GPIO
[    2.473871] sh_mobile_sdhi ee100000.sd: mmc0 base at 0xee100000 max clock rate 195 MHz
[    2.484048] sh_mobile_sdhi ee140000.sd: Got CD GPIO
[    2.489175] sh_mobile_sdhi ee140000.sd: Got WP GPIO
[    2.703850] sh_mobile_sdhi ee140000.sd: mmc1 base at 0xee140000 max clock rate 97 MHz
[    2.714083] sh_mobile_sdhi ee160000.sd: Got CD GPIO
[    2.911847] ipmmu-vmsa e6740000.mmu: Unhandled fault: status 0x00002101 iova 0x40002000
[    2.925838] sh_mobile_sdhi ee160000.sd: mmc2 base at 0xee160000 max clock rate 97 MHz
[    2.938057] ipmmu-vmsa e6740000.mmu: Unhandled fault: status 0x00002101 iova 0x40002000
[    2.938342] asoc-simple-card sound: ak4642-hifi <-> ec500000.sound mapping ok
[    2.954859] mmc0: new ultra high speed SDR50 SDHC card at address aaaa
[    2.962604] mmcblk0: mmc0:aaaa SU04G 3.69 GiB 
[    2.971149] input: keyboard as /devices/platform/keyboard/input/input0
[    2.980044] da9063-rtc da9063-rtc: setting system clock to 2017-01-10 20:15:43 UTC (1484079343)
[    2.989814]  mmcblk0: p1
[    3.106206] Micrel KSZ8041RNLI ee700000.etherne:01: attached PHY driver [Micrel KSZ8041RNLI] (mii_bus:phy_addr=ee700000.etherne:01, irq=405)

If I boot the system without a SD card inserted the warnings are not 
printed, however when I insert the SD card they are immediately printed.  
Multiple removal/insertion of a card do not trigger additional warnings, 
only at the first insertion.

Oddly enough the error are only printed when I insert the SD card in the 
mmc0 slot. I can insert/eject the card multiple times in mmc1 and no 
error but the first insertion in mmc0 and boom. Only difference I can 
see are the clock speed between mmc0 and mmc1.

[  125.681585] ipmmu-vmsa e6740000.mmu: Unhandled fault: status 0x00002101 iova 0x40002000
[  125.698839] ipmmu-vmsa e6740000.mmu: Unhandled fault: status 0x00002101 iova 0x40002000
[  125.708228] mmc0: new ultra high speed SDR50 SDHC card at address aaaa
[  125.716394] mmcblk0: mmc0:aaaa SU04G 3.69 GiB 
[  125.737443]  mmcblk0: p1
[  305.365862] mmc0: card aaaa removed
[  307.933518] mmc0: new ultra high speed SDR50 SDHC card at address aaaa
[  307.941948] mmcblk0: mmc0:aaaa SU04G 3.69 GiB 
[  307.964353]  mmcblk0: p1
[  310.965794] mmc0: card aaaa removed
[  317.335789] mmc1: new ultra high speed SDR50 SDHC card at address aaaa
[  317.343172] mmcblk1: mmc1:aaaa SU04G 3.69 GiB 
[  317.364223]  mmcblk1: p1

Sometimes the error is reported 3 times but in most cases only 2.

I can interact fine with the card (I tried checksumming a large file and 
compared with a known good) so it's not broken. I can also interact with 
other devices using the DMAC+IPMMU without similar warnings being 
printed at all, I tested with i2c6.

If i revert this patch 06f438dd389a699d ("mmc: sh_mobile_sdhi: Add 
tuning support") the warnings go away. I have not been able to figure 
out what in the patch triggers the warnings, and I'm not sure the 
problem are with sh_mobile_sdhi. I know to little about the DMAC and 
IPMMU to rule them out as the true source. Do you have any idea what 
might cause this? I'm happy to run more tests or help out in other ways 
if I can.

To reproduce start on v4.10-rc1 and use shmobile_defconfig with a few 
additions:

CONFIG_ARM_LPAE=y
CONFIG_IPMMU_VMSA=y

And I enable IPMMU for DMAC in DT:

diff --git a/arch/arm/boot/dts/r8a7791.dtsi b/arch/arm/boot/dts/r8a7791.dtsi
index 87214668d70f..d4500d79db1d 100644
--- a/arch/arm/boot/dts/r8a7791.dtsi
+++ b/arch/arm/boot/dts/r8a7791.dtsi
@@ -325,6 +325,21 @@
                power-domains = <&sysc R8A7791_PD_ALWAYS_ON>;
                #dma-cells = <1>;
                dma-channels = <15>;
+               iommus = <&ipmmu_ds 0>,
+                        <&ipmmu_ds 1>,
+                        <&ipmmu_ds 2>,
+                        <&ipmmu_ds 3>,
+                        <&ipmmu_ds 4>,
+                        <&ipmmu_ds 5>,
+                        <&ipmmu_ds 6>,
+                        <&ipmmu_ds 7>,
+                        <&ipmmu_ds 8>,
+                        <&ipmmu_ds 9>,
+                        <&ipmmu_ds 10>,
+                        <&ipmmu_ds 11>,
+                        <&ipmmu_ds 12>,
+                        <&ipmmu_ds 13>,
+                        <&ipmmu_ds 14>;
        };
 
        dmac1: dma-controller@e6720000 {
@@ -356,6 +371,21 @@
                power-domains = <&sysc R8A7791_PD_ALWAYS_ON>;
                #dma-cells = <1>;
                dma-channels = <15>;
+               iommus = <&ipmmu_ds 15>,
+                        <&ipmmu_ds 16>,
+                        <&ipmmu_ds 17>,
+                        <&ipmmu_ds 18>,
+                        <&ipmmu_ds 19>,
+                        <&ipmmu_ds 20>,
+                        <&ipmmu_ds 21>,
+                        <&ipmmu_ds 22>,
+                        <&ipmmu_ds 23>,
+                        <&ipmmu_ds 24>,
+                        <&ipmmu_ds 25>,
+                        <&ipmmu_ds 26>,
+                        <&ipmmu_ds 27>,
+                        <&ipmmu_ds 28>,
+                        <&ipmmu_ds 29>;
        };
 
        audma0: dma-controller@ec700000 {
@@ -1688,7 +1718,7 @@
                interrupts = <GIC_SPI 198 IRQ_TYPE_LEVEL_HIGH>,
                             <GIC_SPI 199 IRQ_TYPE_LEVEL_HIGH>;
                #iommu-cells = <1>;
-               status = "disabled";
+               status = "okay";
        };
 
        ipmmu_mp: mmu@ec680000 {

On 2016-11-03 15:16:04 +0100, Simon Horman wrote:
> Add tuning support for use with SDR104 mode
> This includes adding support for the sampling clock controller (SCC).
> 
> Based on work by Ai Kyuse.
> 
> Cc: Ai Kyuse <ai.kyuse.uw@renesas.com>
> Signed-off-by: Simon Horman <horms+renesas@verge.net.au>
> ---
> v8 [Simon Horman]
> * Correct inverted logic in sh_mobile_sdhi_hw_reset
> * Correct value of SH_MOBILE_SDHI_SCC_RVSCNTL_RVSEN
> 
> v7 [Simon Horman]
> * No change
> 
> v6 [Simon Horman]
> * Rebase to use host->taps
> 
> v5 [Simon Horman]
> * As suggested by Ulf Hansson
>   - Use more descriptive name for callback to check for SSC error
> * Reinstate Gen3 tuning support
> 
> v4 [Simon Horman]
> As suggested by Geert Uytterhoeven:
> * guard SDR104 specific portion of probe with host->mmc->caps &
>   MMC_CAP_UHS_SDR104
> * As suggested by Wolfram Sang:
>   - Pass priv to sd_scc_{read,write}32 to save host_to_priv access
>     for each call
>   - Use 0x0 instead of other representations of 0 in hex
>   - Use CLK_CTL_SCLKEN
>   - Do not add unused SH_MOBILE_SDHI_MAX_TAP
>   - Use ternary operator in sh_mobile_sdhi_select_tuning
>   - Do include as yet unsupported HS200 in error string
> * Reintroduce retuning support: This was removed in v3.
> * Revert to algorithm in v1 patchset, on further reading of the documentation
>   it appears to be correct.
> 
> v3 [Simon Horman]
> * As suggested by Kuninori Morimoto:
>   - Do not add unused retuning callback to struct tmio_mmc_host
>   - Change return type of prepare_tuning callback to void
>   - Add tap_size parameter to select_tuning callback
> 
> 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
> * 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
> 
> 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]
> 
> Signed-off-by: Simon Horman <horms+renesas@verge.net.au>
> ---
>  drivers/mmc/host/sh_mobile_sdhi.c | 265 +++++++++++++++++++++++++++++++++++++-
>  1 file changed, 264 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/mmc/host/sh_mobile_sdhi.c b/drivers/mmc/host/sh_mobile_sdhi.c
> index 49edff7fee49..15c77bc6e4ee 100644
> --- a/drivers/mmc/host/sh_mobile_sdhi.c
> +++ b/drivers/mmc/host/sh_mobile_sdhi.c
> @@ -47,6 +47,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;
> @@ -54,6 +59,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 = {
> @@ -66,12 +74,35 @@ 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),
> +};
> +
> +/* Definitions for sampling clocks */
> +static struct sh_mobile_sdhi_scc rcar_gen3_scc_taps[] = {
> +	{
> +		.clk_rate = 0,
> +		.tap = 0x00000300,
> +	},
>  };
>  
>  static const struct sh_mobile_sdhi_of_data of_rcar_gen3_compatible = {
> @@ -79,6 +110,9 @@ static const struct sh_mobile_sdhi_of_data of_rcar_gen3_compatible = {
>  			  TMIO_MMC_CLK_ACTUAL | TMIO_MMC_MIN_RCAR2,
>  	.capabilities	= MMC_CAP_SD_HIGHSPEED | MMC_CAP_SDIO_IRQ,
>  	.bus_shift	= 2,
> +	.scc_offset	= 0x1000,
> +	.taps		= rcar_gen3_scc_taps,
> +	.taps_num	= ARRAY_SIZE(rcar_gen3_scc_taps),
>  };
>  
>  static const struct of_device_id sh_mobile_sdhi_of_match[] = {
> @@ -105,6 +139,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)
> @@ -255,6 +290,201 @@ 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(0)
> +/* 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,
> +				struct sh_mobile_sdhi *priv, int addr)
> +{
> +	return readl(priv->scc_ctl + (addr << host->bus_shift));
> +}
> +
> +static inline void sd_scc_write32(struct tmio_mmc_host *host,
> +				  struct sh_mobile_sdhi *priv,
> +				  int addr, u32 val)
> +{
> +	writel(val, priv->scc_ctl + (addr << host->bus_shift));
> +}
> +
> +static unsigned int sh_mobile_sdhi_init_tuning(struct tmio_mmc_host *host)
> +{
> +	struct sh_mobile_sdhi *priv;
> +
> +	if (!(host->mmc->caps & MMC_CAP_UHS_SDR104))
> +		return 0;
> +
> +	priv = host_to_priv(host);
> +
> +	/* set sampling clock selection range */
> +	sd_scc_write32(host, priv, 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, 0x0);
> +
> +	sd_scc_write32(host, priv, SH_MOBILE_SDHI_SCC_DTCNTL,
> +		       SH_MOBILE_SDHI_SCC_DTCNTL_TAPEN |
> +		       sd_scc_read32(host, priv, SH_MOBILE_SDHI_SCC_DTCNTL));
> +
> +	sd_ctrl_write16(host, CTL_SD_CARD_CLK_CTL, ~CLK_CTL_SCLKEN &
> +			sd_ctrl_read16(host, CTL_SD_CARD_CLK_CTL));
> +
> +	sd_scc_write32(host, priv, SH_MOBILE_SDHI_SCC_CKSEL,
> +		       SH_MOBILE_SDHI_SCC_CKSEL_DTSEL |
> +		       sd_scc_read32(host, priv, SH_MOBILE_SDHI_SCC_CKSEL));
> +
> +	sd_ctrl_write16(host, CTL_SD_CARD_CLK_CTL, CLK_CTL_SCLKEN |
> +			sd_ctrl_read16(host, CTL_SD_CARD_CLK_CTL));
> +
> +	sd_scc_write32(host, priv, SH_MOBILE_SDHI_SCC_RVSCNTL,
> +		       ~SH_MOBILE_SDHI_SCC_RVSCNTL_RVSEN &
> +		       sd_scc_read32(host, priv, SH_MOBILE_SDHI_SCC_RVSCNTL));
> +
> +	sd_scc_write32(host, priv, SH_MOBILE_SDHI_SCC_DT2FF, host->scc_tappos);
> +
> +	/* Read TAPNUM */
> +	return (sd_scc_read32(host, priv, SH_MOBILE_SDHI_SCC_DTCNTL) >>
> +		SH_MOBILE_SDHI_SCC_DTCNTL_TAPNUM_SHIFT) &
> +		SH_MOBILE_SDHI_SCC_DTCNTL_TAPNUM_MASK;
> +}
> +
> +static void sh_mobile_sdhi_prepare_tuning(struct tmio_mmc_host *host,
> +					 unsigned long tap)
> +{
> +	struct sh_mobile_sdhi *priv = host_to_priv(host);
> +
> +	/* Set sampling clock position */
> +	sd_scc_write32(host, priv, SH_MOBILE_SDHI_SCC_TAPSET, tap);
> +}
> +
> +#define SH_MOBILE_SDHI_MAX_TAP 3
> +
> +static int sh_mobile_sdhi_select_tuning(struct tmio_mmc_host *host)
> +{
> +	struct sh_mobile_sdhi *priv = host_to_priv(host);
> +	unsigned long tap_cnt;  /* counter of tuning success */
> +	unsigned long tap_set;  /* tap position */
> +	unsigned long tap_start;/* start position of tuning success */
> +	unsigned long tap_end;  /* end position of tuning success */
> +	unsigned long ntap;     /* temporary counter of tuning success */
> +	unsigned long i;
> +
> +	/* Clear SCC_RVSREQ */
> +	sd_scc_write32(host, priv, SH_MOBILE_SDHI_SCC_RVSREQ, 0);
> +
> +	/*
> +	 * Find the longest consecutive run of successful probes.  If that
> +	 * is more than SH_MOBILE_SDHI_MAX_TAP probes long then use the
> +	 * center index as the tap.
> +	 */
> +	tap_cnt = 0;
> +	ntap = 0;
> +	tap_start = 0;
> +	tap_end = 0;
> +	for (i = 0; i < host->tap_num * 2; i++) {
> +		if (test_bit(i, host->taps))
> +			ntap++;
> +		else {
> +			if (ntap > tap_cnt) {
> +				tap_start = i - ntap;
> +				tap_end = i - 1;
> +				tap_cnt = ntap;
> +			}
> +			ntap = 0;
> +		}
> +	}
> +
> +	if (ntap > tap_cnt) {
> +		tap_start = i - ntap;
> +		tap_end = i - 1;
> +		tap_cnt = ntap;
> +	}
> +
> +	if (tap_cnt >= SH_MOBILE_SDHI_MAX_TAP)
> +		tap_set = (tap_start + tap_end) / 2 % host->tap_num;
> +	else
> +		return -EIO;
> +
> +	/* Set SCC */
> +	sd_scc_write32(host, priv, SH_MOBILE_SDHI_SCC_TAPSET, tap_set);
> +
> +	/* Enable auto re-tuning */
> +	sd_scc_write32(host, priv, SH_MOBILE_SDHI_SCC_RVSCNTL,
> +		       SH_MOBILE_SDHI_SCC_RVSCNTL_RVSEN |
> +		       sd_scc_read32(host, priv, SH_MOBILE_SDHI_SCC_RVSCNTL));
> +
> +	return 0;
> +}
> +
> +
> +static bool sh_mobile_sdhi_check_scc_error(struct tmio_mmc_host *host)
> +{
> +	struct sh_mobile_sdhi *priv;
> +
> +	if (!(host->mmc->caps & MMC_CAP_UHS_SDR104))
> +		return 0;
> +
> +	priv = host_to_priv(host);
> +
> +	/* Check SCC error */
> +	if (sd_scc_read32(host, priv, SH_MOBILE_SDHI_SCC_RVSCNTL) &
> +	    SH_MOBILE_SDHI_SCC_RVSCNTL_RVSEN &&
> +	    sd_scc_read32(host, priv, SH_MOBILE_SDHI_SCC_RVSREQ) &
> +	    SH_MOBILE_SDHI_SCC_RVSREQ_RVSERR) {
> +		/* Clear SCC error */
> +		sd_scc_write32(host, priv, SH_MOBILE_SDHI_SCC_RVSREQ, 0);
> +		return true;
> +	}
> +
> +	return false;
> +}
> +
> +static void sh_mobile_sdhi_hw_reset(struct tmio_mmc_host *host)
> +{
> +	struct sh_mobile_sdhi *priv;
> +
> +	if (!(host->mmc->caps & MMC_CAP_UHS_SDR104))
> +		return;
> +
> +	priv = host_to_priv(host);
> +
> +	/* Reset SCC */
> +	sd_ctrl_write16(host, CTL_SD_CARD_CLK_CTL, ~CLK_CTL_SCLKEN &
> +			sd_ctrl_read16(host, CTL_SD_CARD_CLK_CTL));
> +
> +	sd_scc_write32(host, priv, SH_MOBILE_SDHI_SCC_CKSEL,
> +		       ~SH_MOBILE_SDHI_SCC_CKSEL_DTSEL &
> +		       sd_scc_read32(host, priv, SH_MOBILE_SDHI_SCC_CKSEL));
> +
> +	sd_ctrl_write16(host, CTL_SD_CARD_CLK_CTL, CLK_CTL_SCLKEN |
> +			sd_ctrl_read16(host, CTL_SD_CARD_CLK_CTL));
> +
> +	sd_scc_write32(host, priv, SH_MOBILE_SDHI_SCC_RVSCNTL,
> +		       ~SH_MOBILE_SDHI_SCC_RVSCNTL_RVSEN &
> +		       sd_scc_read32(host, priv, SH_MOBILE_SDHI_SCC_RVSCNTL));
> +
> +	sd_scc_write32(host, priv, SH_MOBILE_SDHI_SCC_RVSCNTL,
> +		       ~SH_MOBILE_SDHI_SCC_RVSCNTL_RVSEN &
> +		       sd_scc_read32(host, priv, SH_MOBILE_SDHI_SCC_RVSCNTL));
> +}
> +
>  static int sh_mobile_sdhi_wait_idle(struct tmio_mmc_host *host)
>  {
>  	int timeout = 1000;
> @@ -325,7 +555,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);
> @@ -384,6 +614,11 @@ static int sh_mobile_sdhi_probe(struct platform_device *pdev)
>  		host->card_busy	= sh_mobile_sdhi_card_busy;
>  		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->check_scc_error	= sh_mobile_sdhi_check_scc_error;
> +		host->hw_reset		= sh_mobile_sdhi_hw_reset;
>  	}
>  
>  	/* Orginally registers were 16 bit apart, could be 32 or 64 nowadays */
> @@ -424,6 +659,34 @@ 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;
> +			const struct sh_mobile_sdhi_scc *taps;
> +			bool hit = false;
> +
> +			of_data = of_id->data;
> +			taps = of_data->taps;
> +
> +			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\n");
> +
> +			priv->scc_ctl = host->ctl + of_data->scc_offset;
> +		}
> +	}
> +
> +	i = 0;
>  	while (1) {
>  		irq = platform_get_irq(pdev, i);
>  		if (irq < 0)
> -- 
> 2.7.0.rc3.207.g0ac5344
> 
> 

-- 
Regards,
Niklas Söderlund

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* Re: [PATCH v8 6/6] mmc: sh_mobile_sdhi: Add tuning support
  2017-01-10 21:08     ` Niklas Söderlund
  (?)
@ 2017-01-10 22:30     ` Wolfram Sang
  2017-01-11  8:35         ` Niklas Söderlund
  -1 siblings, 1 reply; 33+ messages in thread
From: Wolfram Sang @ 2017-01-10 22:30 UTC (permalink / raw)
  To: Niklas Söderlund
  Cc: Simon Horman, Wolfram Sang, Ulf Hansson, Magnus Damm, linux-mmc,
	linux-renesas-soc, Ai Kyuse

[-- Attachment #1: Type: text/plain, Size: 1230 bytes --]


> Oddly enough the error are only printed when I insert the SD card in the 
> mmc0 slot. I can insert/eject the card multiple times in mmc1 and no 
> error but the first insertion in mmc0 and boom. Only difference I can 
> see are the clock speed between mmc0 and mmc1.

Can you try this patch?

From: Wolfram Sang <wsa+renesas@sang-engineering.com>
Date: Sun, 13 Nov 2016 11:10:09 +0100
Subject: [PATCH] pinctrl: pfc: r8a7795: WIP: hardcode TDSEL value

Otherwise, AC-180M won't get probed with SDR50 and EMMY-W1 has more
tuning errors with SDR104.

Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
 drivers/pinctrl/sh-pfc/pfc-r8a7795.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/pinctrl/sh-pfc/pfc-r8a7795.c b/drivers/pinctrl/sh-pfc/pfc-r8a7795.c
index 3f58bfd676ce94..3e3f7585efe8b3 100644
--- a/drivers/pinctrl/sh-pfc/pfc-r8a7795.c
+++ b/drivers/pinctrl/sh-pfc/pfc-r8a7795.c
@@ -5416,6 +5416,9 @@ static int r8a7795_pinmux_init(struct sh_pfc *pfc)
 		pr_info("%s: R-Car H3 >= ES2.0\n", __func__);
 		// FIXME Fixup r8a7795_pinmux_info for ES2.0
 	}
+
+#define TDSEL 0xe60603c0
+	sh_pfc_write_reg(pfc, TDSEL, 32, 0xc3);
 	return 0;
 }
 
-- 
2.10.2



[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* Re: [PATCH v8 6/6] mmc: sh_mobile_sdhi: Add tuning support
  2017-01-10 22:30     ` Wolfram Sang
@ 2017-01-11  8:35         ` Niklas Söderlund
  0 siblings, 0 replies; 33+ messages in thread
From: Niklas Söderlund @ 2017-01-11  8:35 UTC (permalink / raw)
  To: Wolfram Sang
  Cc: Simon Horman, Wolfram Sang, Ulf Hansson, Magnus Damm, linux-mmc,
	linux-renesas-soc, Ai Kyuse

Hi Wolfram,

Thanks for your feedback.

On 2017-01-10 23:30:43 +0100, Wolfram Sang wrote:
> 
> > Oddly enough the error are only printed when I insert the SD card in the 
> > mmc0 slot. I can insert/eject the card multiple times in mmc1 and no 
> > error but the first insertion in mmc0 and boom. Only difference I can 
> > see are the clock speed between mmc0 and mmc1.
> 
> Can you try this patch?
> 
> From: Wolfram Sang <wsa+renesas@sang-engineering.com>
> Date: Sun, 13 Nov 2016 11:10:09 +0100
> Subject: [PATCH] pinctrl: pfc: r8a7795: WIP: hardcode TDSEL value
> 
> Otherwise, AC-180M won't get probed with SDR50 and EMMY-W1 has more
> tuning errors with SDR104.
> 
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
> ---
>  drivers/pinctrl/sh-pfc/pfc-r8a7795.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/pinctrl/sh-pfc/pfc-r8a7795.c b/drivers/pinctrl/sh-pfc/pfc-r8a7795.c

I'm doing my tests on Koelsch so I'm afraid setting the TDSEL in 
pfc-r8a7795.c won't do much ;-) Nevertheless I tried to mimic the patch 
for Koelsch but found the documentation lacking and where unable to do 
so. That is I found a TDSEL register but no documentation of its 
content.

I could try to run the test on r8a7795 but my understanding is that the 
IPMMU is not 100% OK on ES1.0. That is at least why I stopped testing on 
it a while back. Have this been addressed in a recent firmware I could 
upgrade to?

> index 3f58bfd676ce94..3e3f7585efe8b3 100644
> --- a/drivers/pinctrl/sh-pfc/pfc-r8a7795.c
> +++ b/drivers/pinctrl/sh-pfc/pfc-r8a7795.c
> @@ -5416,6 +5416,9 @@ static int r8a7795_pinmux_init(struct sh_pfc *pfc)
>  		pr_info("%s: R-Car H3 >= ES2.0\n", __func__);
>  		// FIXME Fixup r8a7795_pinmux_info for ES2.0
>  	}
> +
> +#define TDSEL 0xe60603c0
> +	sh_pfc_write_reg(pfc, TDSEL, 32, 0xc3);
>  	return 0;
>  }
>  
> -- 
> 2.10.2
> 
> 



-- 
Regards,
Niklas S�derlund

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH v8 6/6] mmc: sh_mobile_sdhi: Add tuning support
@ 2017-01-11  8:35         ` Niklas Söderlund
  0 siblings, 0 replies; 33+ messages in thread
From: Niklas Söderlund @ 2017-01-11  8:35 UTC (permalink / raw)
  To: Wolfram Sang
  Cc: Simon Horman, Wolfram Sang, Ulf Hansson, Magnus Damm, linux-mmc,
	linux-renesas-soc, Ai Kyuse

Hi Wolfram,

Thanks for your feedback.

On 2017-01-10 23:30:43 +0100, Wolfram Sang wrote:
> 
> > Oddly enough the error are only printed when I insert the SD card in the 
> > mmc0 slot. I can insert/eject the card multiple times in mmc1 and no 
> > error but the first insertion in mmc0 and boom. Only difference I can 
> > see are the clock speed between mmc0 and mmc1.
> 
> Can you try this patch?
> 
> From: Wolfram Sang <wsa+renesas@sang-engineering.com>
> Date: Sun, 13 Nov 2016 11:10:09 +0100
> Subject: [PATCH] pinctrl: pfc: r8a7795: WIP: hardcode TDSEL value
> 
> Otherwise, AC-180M won't get probed with SDR50 and EMMY-W1 has more
> tuning errors with SDR104.
> 
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
> ---
>  drivers/pinctrl/sh-pfc/pfc-r8a7795.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/pinctrl/sh-pfc/pfc-r8a7795.c b/drivers/pinctrl/sh-pfc/pfc-r8a7795.c

I'm doing my tests on Koelsch so I'm afraid setting the TDSEL in 
pfc-r8a7795.c won't do much ;-) Nevertheless I tried to mimic the patch 
for Koelsch but found the documentation lacking and where unable to do 
so. That is I found a TDSEL register but no documentation of its 
content.

I could try to run the test on r8a7795 but my understanding is that the 
IPMMU is not 100% OK on ES1.0. That is at least why I stopped testing on 
it a while back. Have this been addressed in a recent firmware I could 
upgrade to?

> index 3f58bfd676ce94..3e3f7585efe8b3 100644
> --- a/drivers/pinctrl/sh-pfc/pfc-r8a7795.c
> +++ b/drivers/pinctrl/sh-pfc/pfc-r8a7795.c
> @@ -5416,6 +5416,9 @@ static int r8a7795_pinmux_init(struct sh_pfc *pfc)
>  		pr_info("%s: R-Car H3 >= ES2.0\n", __func__);
>  		// FIXME Fixup r8a7795_pinmux_info for ES2.0
>  	}
> +
> +#define TDSEL 0xe60603c0
> +	sh_pfc_write_reg(pfc, TDSEL, 32, 0xc3);
>  	return 0;
>  }
>  
> -- 
> 2.10.2
> 
> 



-- 
Regards,
Niklas Söderlund

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH v8 6/6] mmc: sh_mobile_sdhi: Add tuning support
  2017-01-11  8:35         ` Niklas Söderlund
  (?)
@ 2017-01-11  8:42         ` Geert Uytterhoeven
  2017-01-11  9:17             ` Niklas Söderlund
  -1 siblings, 1 reply; 33+ messages in thread
From: Geert Uytterhoeven @ 2017-01-11  8:42 UTC (permalink / raw)
  To: Niklas Söderlund
  Cc: Wolfram Sang, Simon Horman, Wolfram Sang, Ulf Hansson,
	Magnus Damm, Linux MMC List, Linux-Renesas, Ai Kyuse

Hi Niklas,

On Wed, Jan 11, 2017 at 9:35 AM, Niklas Söderlund
<niklas.soderlund@ragnatech.se> wrote:
> On 2017-01-10 23:30:43 +0100, Wolfram Sang wrote:
>> > Oddly enough the error are only printed when I insert the SD card in the
>> > mmc0 slot. I can insert/eject the card multiple times in mmc1 and no
>> > error but the first insertion in mmc0 and boom. Only difference I can
>> > see are the clock speed between mmc0 and mmc1.
>>
>> Can you try this patch?
>>
>> From: Wolfram Sang <wsa+renesas@sang-engineering.com>
>> Date: Sun, 13 Nov 2016 11:10:09 +0100
>> Subject: [PATCH] pinctrl: pfc: r8a7795: WIP: hardcode TDSEL value
>>
>> Otherwise, AC-180M won't get probed with SDR50 and EMMY-W1 has more
>> tuning errors with SDR104.
>>
>> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
>> ---
>>  drivers/pinctrl/sh-pfc/pfc-r8a7795.c | 3 +++
>>  1 file changed, 3 insertions(+)
>>
>> diff --git a/drivers/pinctrl/sh-pfc/pfc-r8a7795.c b/drivers/pinctrl/sh-pfc/pfc-r8a7795.c
>
> I'm doing my tests on Koelsch so I'm afraid setting the TDSEL in
> pfc-r8a7795.c won't do much ;-) Nevertheless I tried to mimic the patch
> for Koelsch but found the documentation lacking and where unable to do
> so. That is I found a TDSEL register but no documentation of its
> content.

The datasheet does mention which bits are for SD[023]_CLK.
Actual bit patterns should be the same as on r8a7795.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH v8 6/6] mmc: sh_mobile_sdhi: Add tuning support
  2017-01-11  8:42         ` Geert Uytterhoeven
@ 2017-01-11  9:17             ` Niklas Söderlund
  0 siblings, 0 replies; 33+ messages in thread
From: Niklas Söderlund @ 2017-01-11  9:17 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Wolfram Sang, Simon Horman, Wolfram Sang, Ulf Hansson,
	Magnus Damm, Linux MMC List, Linux-Renesas, Ai Kyuse

Hi Geert,

Thanks for your feedback.

On 2017-01-11 09:42:09 +0100, Geert Uytterhoeven wrote:
> Hi Niklas,
> 
> On Wed, Jan 11, 2017 at 9:35 AM, Niklas S�derlund
> <niklas.soderlund@ragnatech.se> wrote:
> > On 2017-01-10 23:30:43 +0100, Wolfram Sang wrote:
> >> > Oddly enough the error are only printed when I insert the SD card in the
> >> > mmc0 slot. I can insert/eject the card multiple times in mmc1 and no
> >> > error but the first insertion in mmc0 and boom. Only difference I can
> >> > see are the clock speed between mmc0 and mmc1.
> >>
> >> Can you try this patch?
> >>
> >> From: Wolfram Sang <wsa+renesas@sang-engineering.com>
> >> Date: Sun, 13 Nov 2016 11:10:09 +0100
> >> Subject: [PATCH] pinctrl: pfc: r8a7795: WIP: hardcode TDSEL value
> >>
> >> Otherwise, AC-180M won't get probed with SDR50 and EMMY-W1 has more
> >> tuning errors with SDR104.
> >>
> >> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
> >> ---
> >>  drivers/pinctrl/sh-pfc/pfc-r8a7795.c | 3 +++
> >>  1 file changed, 3 insertions(+)
> >>
> >> diff --git a/drivers/pinctrl/sh-pfc/pfc-r8a7795.c b/drivers/pinctrl/sh-pfc/pfc-r8a7795.c
> >
> > I'm doing my tests on Koelsch so I'm afraid setting the TDSEL in
> > pfc-r8a7795.c won't do much ;-) Nevertheless I tried to mimic the patch
> > for Koelsch but found the documentation lacking and where unable to do
> > so. That is I found a TDSEL register but no documentation of its
> > content.
> 
> The datasheet does mention which bits are for SD[023]_CLK.
> Actual bit patterns should be the same as on r8a7795.

Thanks for the information about the bit patterns. I tried using the 
same bit pattern on SD[023]_CLK but the error persisted :-( 

> 
> Gr{oetje,eeting}s,
> 
>                         Geert
> 
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
> 
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
>                                 -- Linus Torvalds

-- 
Regards,
Niklas S�derlund

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH v8 6/6] mmc: sh_mobile_sdhi: Add tuning support
@ 2017-01-11  9:17             ` Niklas Söderlund
  0 siblings, 0 replies; 33+ messages in thread
From: Niklas Söderlund @ 2017-01-11  9:17 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Wolfram Sang, Simon Horman, Wolfram Sang, Ulf Hansson,
	Magnus Damm, Linux MMC List, Linux-Renesas, Ai Kyuse

Hi Geert,

Thanks for your feedback.

On 2017-01-11 09:42:09 +0100, Geert Uytterhoeven wrote:
> Hi Niklas,
> 
> On Wed, Jan 11, 2017 at 9:35 AM, Niklas Söderlund
> <niklas.soderlund@ragnatech.se> wrote:
> > On 2017-01-10 23:30:43 +0100, Wolfram Sang wrote:
> >> > Oddly enough the error are only printed when I insert the SD card in the
> >> > mmc0 slot. I can insert/eject the card multiple times in mmc1 and no
> >> > error but the first insertion in mmc0 and boom. Only difference I can
> >> > see are the clock speed between mmc0 and mmc1.
> >>
> >> Can you try this patch?
> >>
> >> From: Wolfram Sang <wsa+renesas@sang-engineering.com>
> >> Date: Sun, 13 Nov 2016 11:10:09 +0100
> >> Subject: [PATCH] pinctrl: pfc: r8a7795: WIP: hardcode TDSEL value
> >>
> >> Otherwise, AC-180M won't get probed with SDR50 and EMMY-W1 has more
> >> tuning errors with SDR104.
> >>
> >> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
> >> ---
> >>  drivers/pinctrl/sh-pfc/pfc-r8a7795.c | 3 +++
> >>  1 file changed, 3 insertions(+)
> >>
> >> diff --git a/drivers/pinctrl/sh-pfc/pfc-r8a7795.c b/drivers/pinctrl/sh-pfc/pfc-r8a7795.c
> >
> > I'm doing my tests on Koelsch so I'm afraid setting the TDSEL in
> > pfc-r8a7795.c won't do much ;-) Nevertheless I tried to mimic the patch
> > for Koelsch but found the documentation lacking and where unable to do
> > so. That is I found a TDSEL register but no documentation of its
> > content.
> 
> The datasheet does mention which bits are for SD[023]_CLK.
> Actual bit patterns should be the same as on r8a7795.

Thanks for the information about the bit patterns. I tried using the 
same bit pattern on SD[023]_CLK but the error persisted :-( 

> 
> Gr{oetje,eeting}s,
> 
>                         Geert
> 
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
> 
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
>                                 -- Linus Torvalds

-- 
Regards,
Niklas Söderlund

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH v8 6/6] mmc: sh_mobile_sdhi: Add tuning support
  2017-01-11  8:35         ` Niklas Söderlund
  (?)
  (?)
@ 2017-01-11 15:05         ` Wolfram Sang
  2017-01-11 15:57             ` Niklas Söderlund
  -1 siblings, 1 reply; 33+ messages in thread
From: Wolfram Sang @ 2017-01-11 15:05 UTC (permalink / raw)
  To: Niklas Söderlund
  Cc: Simon Horman, Wolfram Sang, Ulf Hansson, Magnus Damm, linux-mmc,
	linux-renesas-soc, Ai Kyuse

[-- Attachment #1: Type: text/plain, Size: 744 bytes --]

Hi Niklas,

> I'm doing my tests on Koelsch so I'm afraid setting the TDSEL in 
> pfc-r8a7795.c won't do much ;-) Nevertheless I tried to mimic the patch 
> for Koelsch but found the documentation lacking and where unable to do 
> so. That is I found a TDSEL register but no documentation of its 
> content.

Sorry, that slipped through. Actually, I sent a patch for H2:

https://patchwork.kernel.org/patch/9434323/

I wanted to do some TDSEL testing on other Gen2 boards in Brussels,
provided that people could bring some. So, if you could bring that
Koelsch board, that would be really helpful.

I assume that you created a similar patch to the above for Koelsch and
the issue still remained? :(

Regards,

   Wolfram


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH v8 6/6] mmc: sh_mobile_sdhi: Add tuning support
  2017-01-11 15:05         ` Wolfram Sang
@ 2017-01-11 15:57             ` Niklas Söderlund
  0 siblings, 0 replies; 33+ messages in thread
From: Niklas Söderlund @ 2017-01-11 15:57 UTC (permalink / raw)
  To: Wolfram Sang
  Cc: Simon Horman, Wolfram Sang, Ulf Hansson, Magnus Damm, linux-mmc,
	linux-renesas-soc, Ai Kyuse

Hi Wolfram,

On 2017-01-11 16:05:21 +0100, Wolfram Sang wrote:
> Hi Niklas,
> 
> > I'm doing my tests on Koelsch so I'm afraid setting the TDSEL in 
> > pfc-r8a7795.c won't do much ;-) Nevertheless I tried to mimic the patch 
> > for Koelsch but found the documentation lacking and where unable to do 
> > so. That is I found a TDSEL register but no documentation of its 
> > content.
> 
> Sorry, that slipped through. Actually, I sent a patch for H2:
> 
> https://patchwork.kernel.org/patch/9434323/
> 
> I wanted to do some TDSEL testing on other Gen2 boards in Brussels,
> provided that people could bring some. So, if you could bring that
> Koelsch board, that would be really helpful.

Sure, can you or someone else bring a power source? I plan to not check 
luggage so everything to conserve space helps :-)

> 
> I assume that you created a similar patch to the above for Koelsch and
> the issue still remained? :(

Yes I did with the help from Geert, I have now also tried your patch.  
Unfortunately the result is the same and the warning is printed. I hope 
to be able to spend more time later this week with the tuning patch to 
try and figure this out.

> 
> Regards,
> 
>    Wolfram
> 



-- 
Regards,
Niklas S�derlund

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH v8 6/6] mmc: sh_mobile_sdhi: Add tuning support
@ 2017-01-11 15:57             ` Niklas Söderlund
  0 siblings, 0 replies; 33+ messages in thread
From: Niklas Söderlund @ 2017-01-11 15:57 UTC (permalink / raw)
  To: Wolfram Sang
  Cc: Simon Horman, Wolfram Sang, Ulf Hansson, Magnus Damm, linux-mmc,
	linux-renesas-soc, Ai Kyuse

Hi Wolfram,

On 2017-01-11 16:05:21 +0100, Wolfram Sang wrote:
> Hi Niklas,
> 
> > I'm doing my tests on Koelsch so I'm afraid setting the TDSEL in 
> > pfc-r8a7795.c won't do much ;-) Nevertheless I tried to mimic the patch 
> > for Koelsch but found the documentation lacking and where unable to do 
> > so. That is I found a TDSEL register but no documentation of its 
> > content.
> 
> Sorry, that slipped through. Actually, I sent a patch for H2:
> 
> https://patchwork.kernel.org/patch/9434323/
> 
> I wanted to do some TDSEL testing on other Gen2 boards in Brussels,
> provided that people could bring some. So, if you could bring that
> Koelsch board, that would be really helpful.

Sure, can you or someone else bring a power source? I plan to not check 
luggage so everything to conserve space helps :-)

> 
> I assume that you created a similar patch to the above for Koelsch and
> the issue still remained? :(

Yes I did with the help from Geert, I have now also tried your patch.  
Unfortunately the result is the same and the warning is printed. I hope 
to be able to spend more time later this week with the tuning patch to 
try and figure this out.

> 
> Regards,
> 
>    Wolfram
> 



-- 
Regards,
Niklas Söderlund

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH v8 6/6] mmc: sh_mobile_sdhi: Add tuning support
  2017-01-11 15:57             ` Niklas Söderlund
  (?)
@ 2017-01-11 15:59             ` Geert Uytterhoeven
  2017-01-11 17:34               ` Wolfram Sang
  -1 siblings, 1 reply; 33+ messages in thread
From: Geert Uytterhoeven @ 2017-01-11 15:59 UTC (permalink / raw)
  To: Niklas Söderlund
  Cc: Wolfram Sang, Simon Horman, Wolfram Sang, Ulf Hansson,
	Magnus Damm, Linux MMC List, Linux-Renesas, Ai Kyuse

On Wed, Jan 11, 2017 at 4:57 PM, Niklas Söderlund
<niklas.soderlund@ragnatech.se> wrote:
> On 2017-01-11 16:05:21 +0100, Wolfram Sang wrote:
>> I wanted to do some TDSEL testing on other Gen2 boards in Brussels,
>> provided that people could bring some. So, if you could bring that
>> Koelsch board, that would be really helpful.
>
> Sure, can you or someone else bring a power source? I plan to not check
> luggage so everything to conserve space helps :-)

I also have a koelsch, so no need to take one on a plane ;-)

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH v8 6/6] mmc: sh_mobile_sdhi: Add tuning support
  2017-01-11 15:59             ` Geert Uytterhoeven
@ 2017-01-11 17:34               ` Wolfram Sang
  2017-01-12 11:17                   ` Niklas Söderlund
  0 siblings, 1 reply; 33+ messages in thread
From: Wolfram Sang @ 2017-01-11 17:34 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Niklas Söderlund, Simon Horman, Wolfram Sang, Ulf Hansson,
	Magnus Damm, Linux MMC List, Linux-Renesas, Ai Kyuse

[-- Attachment #1: Type: text/plain, Size: 185 bytes --]


> I also have a koelsch, so no need to take one on a plane ;-)

I thought yours was so heavily hooked up that it was a bit cumbersome to
bring it somewhere. Happy to be wrong here :)


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH v8 6/6] mmc: sh_mobile_sdhi: Add tuning support
  2017-01-11 17:34               ` Wolfram Sang
@ 2017-01-12 11:17                   ` Niklas Söderlund
  0 siblings, 0 replies; 33+ messages in thread
From: Niklas Söderlund @ 2017-01-12 11:17 UTC (permalink / raw)
  To: Wolfram Sang
  Cc: Geert Uytterhoeven, Simon Horman, Wolfram Sang, Ulf Hansson,
	Magnus Damm, Linux MMC List, Linux-Renesas, Ai Kyuse

On 2017-01-11 18:34:32 +0100, Wolfram Sang wrote:
> 
> > I also have a koelsch, so no need to take one on a plane ;-)
> 
> I thought yours was so heavily hooked up that it was a bit cumbersome to
> bring it somewhere. Happy to be wrong here :)
> 

To be super clear, Geert you can bring your Koelsch and Wolfram you only 
want to do testing on one Koelsch board and not as many as you can get 
your hands on?

If so I be happy to leave my board at home and not have to take it on 
the plane, please confirm so there are no misunderstandings :-)

-- 
Regards,
Niklas S�derlund

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH v8 6/6] mmc: sh_mobile_sdhi: Add tuning support
@ 2017-01-12 11:17                   ` Niklas Söderlund
  0 siblings, 0 replies; 33+ messages in thread
From: Niklas Söderlund @ 2017-01-12 11:17 UTC (permalink / raw)
  To: Wolfram Sang
  Cc: Geert Uytterhoeven, Simon Horman, Wolfram Sang, Ulf Hansson,
	Magnus Damm, Linux MMC List, Linux-Renesas, Ai Kyuse

On 2017-01-11 18:34:32 +0100, Wolfram Sang wrote:
> 
> > I also have a koelsch, so no need to take one on a plane ;-)
> 
> I thought yours was so heavily hooked up that it was a bit cumbersome to
> bring it somewhere. Happy to be wrong here :)
> 

To be super clear, Geert you can bring your Koelsch and Wolfram you only 
want to do testing on one Koelsch board and not as many as you can get 
your hands on?

If so I be happy to leave my board at home and not have to take it on 
the plane, please confirm so there are no misunderstandings :-)

-- 
Regards,
Niklas Söderlund

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH v8 6/6] mmc: sh_mobile_sdhi: Add tuning support
  2017-01-12 11:17                   ` Niklas Söderlund
  (?)
@ 2017-01-12 11:23                   ` Geert Uytterhoeven
  2017-01-12 13:03                     ` Wolfram Sang
  -1 siblings, 1 reply; 33+ messages in thread
From: Geert Uytterhoeven @ 2017-01-12 11:23 UTC (permalink / raw)
  To: Niklas Söderlund
  Cc: Wolfram Sang, Simon Horman, Wolfram Sang, Ulf Hansson,
	Magnus Damm, Linux MMC List, Linux-Renesas, Ai Kyuse

On Thu, Jan 12, 2017 at 12:17 PM, Niklas Söderlund
<niklas.soderlund@ragnatech.se> wrote:
> On 2017-01-11 18:34:32 +0100, Wolfram Sang wrote:
>> > I also have a koelsch, so no need to take one on a plane ;-)
>>
>> I thought yours was so heavily hooked up that it was a bit cumbersome to
>> bring it somewhere. Happy to be wrong here :)
>
> To be super clear, Geert you can bring your Koelsch and Wolfram you only
> want to do testing on one Koelsch board and not as many as you can get
> your hands on?
>
> If so I be happy to leave my board at home and not have to take it on
> the plane, please confirm so there are no misunderstandings :-)

I'll bring my Koelsch.

AFAIK, Wolfram is interested in "as many SD cards", not "as many Koelsch
boards".

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH v8 6/6] mmc: sh_mobile_sdhi: Add tuning support
  2017-01-12 11:23                   ` Geert Uytterhoeven
@ 2017-01-12 13:03                     ` Wolfram Sang
  2017-01-17 11:29                         ` Niklas Söderlund
  0 siblings, 1 reply; 33+ messages in thread
From: Wolfram Sang @ 2017-01-12 13:03 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Niklas Söderlund, Simon Horman, Wolfram Sang, Ulf Hansson,
	Magnus Damm, Linux MMC List, Linux-Renesas, Ai Kyuse

[-- Attachment #1: Type: text/plain, Size: 793 bytes --]


> I'll bring my Koelsch.

Great. I *think* one Koelsch will do, but if it is not too much of a
problem, double-checking with a second board won't hurt. So, since Geert
will probably bring all necessary cables and supplies, maybe you can
pack the board nonetheless? But having one Koelsch already is really
good!

> AFAIK, Wolfram is interested in "as many SD cards", not "as many Koelsch
> boards".

1st: as many Gen2 boards as possible (one per Gen2 SoC version would be
awesome!) Gose, Alt anyone?

For those, I'll bring my SanDisk card which causes issues when ejecting
on all my boards. This is my main target.

2nd: as many SD cards as possible

This is a secondary target, but trying to find other issues/other type
of cards with same issues will be helpful, too.

Thanks,

   Wolfram


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH v8 6/6] mmc: sh_mobile_sdhi: Add tuning support
  2017-01-12 13:03                     ` Wolfram Sang
@ 2017-01-17 11:29                         ` Niklas Söderlund
  0 siblings, 0 replies; 33+ messages in thread
From: Niklas Söderlund @ 2017-01-17 11:29 UTC (permalink / raw)
  To: Wolfram Sang
  Cc: Geert Uytterhoeven, Simon Horman, Wolfram Sang, Ulf Hansson,
	Magnus Damm, Linux MMC List, Linux-Renesas, Ai Kyuse

On 2017-01-12 14:03:24 +0100, Wolfram Sang wrote:
> 
> > I'll bring my Koelsch.
> 
> Great. I *think* one Koelsch will do, but if it is not too much of a
> problem, double-checking with a second board won't hurt. So, since Geert
> will probably bring all necessary cables and supplies, maybe you can
> pack the board nonetheless? But having one Koelsch already is really
> good!

If it fits in my luggage I will bring it then (I think it will), but I 
won't throw out my laptop to make room for it then since Geert will 
bring his :-)

> 
> > AFAIK, Wolfram is interested in "as many SD cards", not "as many Koelsch
> > boards".
> 
> 1st: as many Gen2 boards as possible (one per Gen2 SoC version would be
> awesome!) Gose, Alt anyone?
> 
> For those, I'll bring my SanDisk card which causes issues when ejecting
> on all my boards. This is my main target.
> 
> 2nd: as many SD cards as possible
> 
> This is a secondary target, but trying to find other issues/other type
> of cards with same issues will be helpful, too.
> 
> Thanks,
> 
>    Wolfram
> 



-- 
Regards,
Niklas S�derlund

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH v8 6/6] mmc: sh_mobile_sdhi: Add tuning support
@ 2017-01-17 11:29                         ` Niklas Söderlund
  0 siblings, 0 replies; 33+ messages in thread
From: Niklas Söderlund @ 2017-01-17 11:29 UTC (permalink / raw)
  To: Wolfram Sang
  Cc: Geert Uytterhoeven, Simon Horman, Wolfram Sang, Ulf Hansson,
	Magnus Damm, Linux MMC List, Linux-Renesas, Ai Kyuse

On 2017-01-12 14:03:24 +0100, Wolfram Sang wrote:
> 
> > I'll bring my Koelsch.
> 
> Great. I *think* one Koelsch will do, but if it is not too much of a
> problem, double-checking with a second board won't hurt. So, since Geert
> will probably bring all necessary cables and supplies, maybe you can
> pack the board nonetheless? But having one Koelsch already is really
> good!

If it fits in my luggage I will bring it then (I think it will), but I 
won't throw out my laptop to make room for it then since Geert will 
bring his :-)

> 
> > AFAIK, Wolfram is interested in "as many SD cards", not "as many Koelsch
> > boards".
> 
> 1st: as many Gen2 boards as possible (one per Gen2 SoC version would be
> awesome!) Gose, Alt anyone?
> 
> For those, I'll bring my SanDisk card which causes issues when ejecting
> on all my boards. This is my main target.
> 
> 2nd: as many SD cards as possible
> 
> This is a secondary target, but trying to find other issues/other type
> of cards with same issues will be helpful, too.
> 
> Thanks,
> 
>    Wolfram
> 



-- 
Regards,
Niklas Söderlund

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH v8 6/6] mmc: sh_mobile_sdhi: Add tuning support
  2017-01-10 21:08     ` Niklas Söderlund
  (?)
  (?)
@ 2017-01-23 11:16     ` Wolfram Sang
  2017-01-31 20:56         ` Niklas Söderlund
  -1 siblings, 1 reply; 33+ messages in thread
From: Wolfram Sang @ 2017-01-23 11:16 UTC (permalink / raw)
  To: Niklas Söderlund
  Cc: Simon Horman, Wolfram Sang, Ulf Hansson, Magnus Damm, linux-mmc,
	linux-renesas-soc, Ai Kyuse

[-- Attachment #1: Type: text/plain, Size: 1876 bytes --]

Hi Niklas,

so I scratched my head around this a little more... I don't have an
explanation, but want to highlight some interesting points:

> [    2.954859] mmc0: new ultra high speed SDR50 SDHC card at address aaaa

So, this is an SDR50 card. The patch in questions adds tuning support
which is only needed for SDR104. The whole tuning procedure is not (or
at least should not be) exercised.

> Oddly enough the error are only printed when I insert the SD card in the 
> mmc0 slot. I can insert/eject the card multiple times in mmc1 and no 
> error but the first insertion in mmc0 and boom. Only difference I can 
> see are the clock speed between mmc0 and mmc1.

That actually makes sense. mmc0 is SDR104 capable and has the SCC unit
which is needed for tuning (note the larger register space in the dtsi).
The other mmc cores do only SDR50 and do not have an SCC. Because your
warning is about a broken pointer, I wonder if it is about accessing
SCC? Maybe in hw_reset? Wild guess, though. On the other hand, the
register ranges in the dtsi look ok, so I'd assume the IPMMU should have
all the info it needs? But I don't really know...

First thing to try: please remove the property "sd-uhs-sdr104;" from
your Koelsch DTS. I'd expect this makes the warnings go away. If so,
readd the property and instrument if sh_mobile_sdhi_hw_reset() gets
called. One outcome might be that the printouts might be tied to the
warnings.

> I can interact fine with the card (I tried checksumming a large file and 
> compared with a known good) so it's not broken. I can also interact with 
> other devices using the DMAC+IPMMU without similar warnings being 
> printed at all, I tested with i2c6.

That is relieving and also makes sense in the way that nothing in this
patch should be needed to get an SDR50 card running.

Regards,

   Wolfram


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH v8 6/6] mmc: sh_mobile_sdhi: Add tuning support
  2017-01-10 21:08     ` Niklas Söderlund
                       ` (2 preceding siblings ...)
  (?)
@ 2017-01-26  9:58     ` Simon Horman
  -1 siblings, 0 replies; 33+ messages in thread
From: Simon Horman @ 2017-01-26  9:58 UTC (permalink / raw)
  To: Niklas Söderlund
  Cc: Wolfram Sang, Ulf Hansson, Magnus Damm, linux-mmc,
	linux-renesas-soc, Ai Kyuse

On Tue, Jan 10, 2017 at 10:08:48PM +0100, Niklas Söderlund wrote:
> Hi Simon,
> 
> I started to se errors when I was testing DMAC+IPMMU patches on top of 
> v4.10-rc1 on Koelsch.

There has been some discussion in this thread already. I would like to
provide some more information in case it is useful.

The 3.5.0 BSP appears to contain several that may be relevant to this
discussion:

383c4437846d mmc: sh_mobile_sdhi: Add detecting a change point of data to SCC tuning
1823812e0937 mmc: tmio: Add detecting a change point of data to SCC tunin
03935e9182d9 mmc: tmio: Fix tuning flow
c711db03349c mmc: sh_mobile_sdhi: Fix sampling clock position selecting
2838a2ff8ca7 mmc: tmio: fix soft lockup on CMD12 for R-Car SDHI


Of these, so far I have looked into "mmc: tmio: Fix tuning flow".
It seems to do several things:

- Ensure tuning initialisation is called for each tuning procedure:
  this seems a correct fix for a bug added by me
- Do not terminate tuning on error:
  this is not my reading of the documentation but may well be correct
- Reset more:
  I am least sure about this as it does not seem to have any explanation
  in the changelog

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH v8 6/6] mmc: sh_mobile_sdhi: Add tuning support
  2017-01-23 11:16     ` Wolfram Sang
@ 2017-01-31 20:56         ` Niklas Söderlund
  0 siblings, 0 replies; 33+ messages in thread
From: Niklas Söderlund @ 2017-01-31 20:56 UTC (permalink / raw)
  To: Wolfram Sang
  Cc: Simon Horman, Wolfram Sang, Ulf Hansson, Magnus Damm, linux-mmc,
	linux-renesas-soc, Ai Kyuse

Hi Wolfram and Simon,

On 2017-01-23 12:16:39 +0100, Wolfram Sang wrote:
> Hi Niklas,
> 
> so I scratched my head around this a little more... I don't have an
> explanation, but want to highlight some interesting points:
> 
> > [    2.954859] mmc0: new ultra high speed SDR50 SDHC card at address aaaa
> 
> So, this is an SDR50 card. The patch in questions adds tuning support
> which is only needed for SDR104. The whole tuning procedure is not (or
> at least should not be) exercised.
> 
> > Oddly enough the error are only printed when I insert the SD card in the 
> > mmc0 slot. I can insert/eject the card multiple times in mmc1 and no 
> > error but the first insertion in mmc0 and boom. Only difference I can 
> > see are the clock speed between mmc0 and mmc1.
> 
> That actually makes sense. mmc0 is SDR104 capable and has the SCC unit
> which is needed for tuning (note the larger register space in the dtsi).
> The other mmc cores do only SDR50 and do not have an SCC. Because your
> warning is about a broken pointer, I wonder if it is about accessing
> SCC? Maybe in hw_reset? Wild guess, though. On the other hand, the
> register ranges in the dtsi look ok, so I'd assume the IPMMU should have
> all the info it needs? But I don't really know...

Thanks for the explanation, now I understand why it only happens on 
mmc0.

> 
> First thing to try: please remove the property "sd-uhs-sdr104;" from
> your Koelsch DTS. I'd expect this makes the warnings go away. If so,
> readd the property and instrument if sh_mobile_sdhi_hw_reset() gets
> called. One outcome might be that the printouts might be tied to the
> warnings.

I played around a bit with what you suggested above and here are my 
observations:

1. Removing "sd-uhs-sdr104;" from my Koelsch DTS do indeed make the 
   warning go away.

2. sh_mobile_sdhi_hw_reset() is called each time I insert the card. The 
   warning is as previously stated triggered on the first insertion.

3. If I turn the sh_mobile_sdhi_hw_reset() into a noop function (just 
   return without doing anything) the warning gets printed each time the 
   card is inserted.

As it turns out sh_mobile_sdhi_hw_reset() have something to do with the 
warnings and was a good point for me to start digging at.

The warning originates from mmc_send_tuning() inside a loop in
tmio_mmc_execute_tuning():

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

            ret = mmc_send_tuning(mmc, opcode, NULL);
            if (ret && ret != -EILSEQ)
                    goto out;
            if (ret == 0)
                    set_bit(i, host->taps);

            mdelay(1);
    }

The warning is printed for me on loop iteration 7 and 15, which if I 
understand things correctly is for the same tap (7)?

If I track mmc_send_tuning() it looks like t starts a command 
(__mmc_start_req()) and then waits for it to finish 
(mmc_wait_for_req_done()) and it is in between these calls our warning 
is printed.

If i dig a bit deeper starting at __mmc_start_req() I end up where I 
think the command which generates the warning is issued to the hardware 
in tmio_mmc_start_command(). Simplified call graph:

tmio_mmc_execute_tuning()
  mmc_send_tuning()
    mmc_wait_for_req()
      __mmc_start_req()
        ...
          ...
            tmio_mmc_start_command()
      mmc_wait_for_req_done()

At the very end of  tmio_mmc_start_command() the command is fired to the 
hardware and it's right after that the warning is printed. With a bit 
creative msleep() and pr_info() I think I can confirm this like this:

   /* Fire off the command */
   sd_ctrl_write32_as_16_and_16(host, CTL_ARG_REG, cmd->arg);
   pr_info("%s 1\n", __func__);
   msleep(1000);
   pr_info("%s 2\n", __func__);
   sd_ctrl_write16(host, CTL_SD_CMD, c);
   pr_info("%s 3\n", __func__);
   msleep(1000);
   pr_info("%s 4\n", __func__);

Which gives the output for the loop itteration 7 and 15 in 
tmio_mmc_execute_tuning():

[  136.046594] tmio_mmc_start_command 1
[  137.116454] tmio_mmc_start_command 2
[  137.122399] tmio_mmc_start_command 3
[  137.122461] ipmmu-vmsa e6740000.mmu: Unhandled fault: status 0x00002101 iova 0x40002000
[  138.156454] tmio_mmc_start_command 4

Arriving at this I feel my knowledge reached its limit and would like to 
hear what you think about my findings and possible fix? First of is this 
a possible place for the warning to originate?

Why do it only happen for tap 7? Tap 0-6 are fine. I did double check 
the TAPNUM from sh_mobile_sdhi_init_tuning() return 8. And that the 
TAPNUMS used in sh_mobile_sdhi_prepare_tuning() is in the range 0-7.  
This should as far as I understand the docs be correct?

I do however have a patch that makes the warnings go away :-) But I 
would like to hear what you have to say about the above and about the 
patch itself before I send it of as a real patch, my knowledge about 
this driver is rudimentary at best and the patch is just a product of 
what I learnt while investigating this issue.

diff --git a/drivers/mmc/host/tmio_mmc_pio.c b/drivers/mmc/host/tmio_mmc_pio.c
index 2064fa1a5bf11f9a..1483902a1e323adb 100644
--- a/drivers/mmc/host/tmio_mmc_pio.c
+++ b/drivers/mmc/host/tmio_mmc_pio.c
@@ -824,6 +824,9 @@ static int tmio_mmc_execute_tuning(struct mmc_host *mmc, u32 opcode)
 
        bitmap_zero(host->taps, host->tap_num * 2);
 
+       /* Reset hardware before tuning */
+       tmio_mmc_hw_reset(mmc);
+
        /* Issue CMD19 twice for each tap */
        for (i = 0; i < 2 * host->tap_num; i++) {
                if (host->prepare_tuning)

Let me know if you think this is a suitable fix and I will resend it as 
a proper patch. Whit this applied the warnings are no more and I can 
properly interact with the card, I do however only have a SDR50 card to 
try with.

> 
> > I can interact fine with the card (I tried checksumming a large file and 
> > compared with a known good) so it's not broken. I can also interact with 
> > other devices using the DMAC+IPMMU without similar warnings being 
> > printed at all, I tested with i2c6.
> 
> That is relieving and also makes sense in the way that nothing in this
> patch should be needed to get an SDR50 card running.
> 
> Regards,
> 
>    Wolfram
> 


-- 
Regards,
Niklas S�derlund

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* Re: [PATCH v8 6/6] mmc: sh_mobile_sdhi: Add tuning support
@ 2017-01-31 20:56         ` Niklas Söderlund
  0 siblings, 0 replies; 33+ messages in thread
From: Niklas Söderlund @ 2017-01-31 20:56 UTC (permalink / raw)
  To: Wolfram Sang
  Cc: Simon Horman, Wolfram Sang, Ulf Hansson, Magnus Damm, linux-mmc,
	linux-renesas-soc, Ai Kyuse

Hi Wolfram and Simon,

On 2017-01-23 12:16:39 +0100, Wolfram Sang wrote:
> Hi Niklas,
> 
> so I scratched my head around this a little more... I don't have an
> explanation, but want to highlight some interesting points:
> 
> > [    2.954859] mmc0: new ultra high speed SDR50 SDHC card at address aaaa
> 
> So, this is an SDR50 card. The patch in questions adds tuning support
> which is only needed for SDR104. The whole tuning procedure is not (or
> at least should not be) exercised.
> 
> > Oddly enough the error are only printed when I insert the SD card in the 
> > mmc0 slot. I can insert/eject the card multiple times in mmc1 and no 
> > error but the first insertion in mmc0 and boom. Only difference I can 
> > see are the clock speed between mmc0 and mmc1.
> 
> That actually makes sense. mmc0 is SDR104 capable and has the SCC unit
> which is needed for tuning (note the larger register space in the dtsi).
> The other mmc cores do only SDR50 and do not have an SCC. Because your
> warning is about a broken pointer, I wonder if it is about accessing
> SCC? Maybe in hw_reset? Wild guess, though. On the other hand, the
> register ranges in the dtsi look ok, so I'd assume the IPMMU should have
> all the info it needs? But I don't really know...

Thanks for the explanation, now I understand why it only happens on 
mmc0.

> 
> First thing to try: please remove the property "sd-uhs-sdr104;" from
> your Koelsch DTS. I'd expect this makes the warnings go away. If so,
> readd the property and instrument if sh_mobile_sdhi_hw_reset() gets
> called. One outcome might be that the printouts might be tied to the
> warnings.

I played around a bit with what you suggested above and here are my 
observations:

1. Removing "sd-uhs-sdr104;" from my Koelsch DTS do indeed make the 
   warning go away.

2. sh_mobile_sdhi_hw_reset() is called each time I insert the card. The 
   warning is as previously stated triggered on the first insertion.

3. If I turn the sh_mobile_sdhi_hw_reset() into a noop function (just 
   return without doing anything) the warning gets printed each time the 
   card is inserted.

As it turns out sh_mobile_sdhi_hw_reset() have something to do with the 
warnings and was a good point for me to start digging at.

The warning originates from mmc_send_tuning() inside a loop in
tmio_mmc_execute_tuning():

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

            ret = mmc_send_tuning(mmc, opcode, NULL);
            if (ret && ret != -EILSEQ)
                    goto out;
            if (ret == 0)
                    set_bit(i, host->taps);

            mdelay(1);
    }

The warning is printed for me on loop iteration 7 and 15, which if I 
understand things correctly is for the same tap (7)?

If I track mmc_send_tuning() it looks like t starts a command 
(__mmc_start_req()) and then waits for it to finish 
(mmc_wait_for_req_done()) and it is in between these calls our warning 
is printed.

If i dig a bit deeper starting at __mmc_start_req() I end up where I 
think the command which generates the warning is issued to the hardware 
in tmio_mmc_start_command(). Simplified call graph:

tmio_mmc_execute_tuning()
  mmc_send_tuning()
    mmc_wait_for_req()
      __mmc_start_req()
        ...
          ...
            tmio_mmc_start_command()
      mmc_wait_for_req_done()

At the very end of  tmio_mmc_start_command() the command is fired to the 
hardware and it's right after that the warning is printed. With a bit 
creative msleep() and pr_info() I think I can confirm this like this:

   /* Fire off the command */
   sd_ctrl_write32_as_16_and_16(host, CTL_ARG_REG, cmd->arg);
   pr_info("%s 1\n", __func__);
   msleep(1000);
   pr_info("%s 2\n", __func__);
   sd_ctrl_write16(host, CTL_SD_CMD, c);
   pr_info("%s 3\n", __func__);
   msleep(1000);
   pr_info("%s 4\n", __func__);

Which gives the output for the loop itteration 7 and 15 in 
tmio_mmc_execute_tuning():

[  136.046594] tmio_mmc_start_command 1
[  137.116454] tmio_mmc_start_command 2
[  137.122399] tmio_mmc_start_command 3
[  137.122461] ipmmu-vmsa e6740000.mmu: Unhandled fault: status 0x00002101 iova 0x40002000
[  138.156454] tmio_mmc_start_command 4

Arriving at this I feel my knowledge reached its limit and would like to 
hear what you think about my findings and possible fix? First of is this 
a possible place for the warning to originate?

Why do it only happen for tap 7? Tap 0-6 are fine. I did double check 
the TAPNUM from sh_mobile_sdhi_init_tuning() return 8. And that the 
TAPNUMS used in sh_mobile_sdhi_prepare_tuning() is in the range 0-7.  
This should as far as I understand the docs be correct?

I do however have a patch that makes the warnings go away :-) But I 
would like to hear what you have to say about the above and about the 
patch itself before I send it of as a real patch, my knowledge about 
this driver is rudimentary at best and the patch is just a product of 
what I learnt while investigating this issue.

diff --git a/drivers/mmc/host/tmio_mmc_pio.c b/drivers/mmc/host/tmio_mmc_pio.c
index 2064fa1a5bf11f9a..1483902a1e323adb 100644
--- a/drivers/mmc/host/tmio_mmc_pio.c
+++ b/drivers/mmc/host/tmio_mmc_pio.c
@@ -824,6 +824,9 @@ static int tmio_mmc_execute_tuning(struct mmc_host *mmc, u32 opcode)
 
        bitmap_zero(host->taps, host->tap_num * 2);
 
+       /* Reset hardware before tuning */
+       tmio_mmc_hw_reset(mmc);
+
        /* Issue CMD19 twice for each tap */
        for (i = 0; i < 2 * host->tap_num; i++) {
                if (host->prepare_tuning)

Let me know if you think this is a suitable fix and I will resend it as 
a proper patch. Whit this applied the warnings are no more and I can 
properly interact with the card, I do however only have a SDR50 card to 
try with.

> 
> > I can interact fine with the card (I tried checksumming a large file and 
> > compared with a known good) so it's not broken. I can also interact with 
> > other devices using the DMAC+IPMMU without similar warnings being 
> > printed at all, I tested with i2c6.
> 
> That is relieving and also makes sense in the way that nothing in this
> patch should be needed to get an SDR50 card running.
> 
> Regards,
> 
>    Wolfram
> 


-- 
Regards,
Niklas Söderlund

^ permalink raw reply related	[flat|nested] 33+ messages in thread

end of thread, other threads:[~2017-01-31 20:56 UTC | newest]

Thread overview: 33+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-11-03 14:15 [PATCH v8 0/6] UHS-I SDR-104 support for sh_mobile_sdhi Simon Horman
2016-11-03 14:15 ` [PATCH v8 1/6] mmc: core: Add helper to see if a host can be retuned Simon Horman
2016-11-03 14:16 ` [PATCH v8 2/6] mmc: tmio: enhance illegal sequence handling Simon Horman
2016-11-03 14:16 ` [PATCH v8 3/6] mmc: tmio: document mandatory and optional callbacks Simon Horman
2016-11-03 14:16 ` [PATCH v8 4/6] mmc: tmio: Add hw reset support Simon Horman
2016-11-03 14:16 ` [PATCH v8 5/6] mmc: tmio: Add tuning support Simon Horman
2016-11-03 14:16 ` [PATCH v8 6/6] mmc: sh_mobile_sdhi: " Simon Horman
2017-01-10 21:08   ` Niklas Söderlund
2017-01-10 21:08     ` Niklas Söderlund
2017-01-10 22:30     ` Wolfram Sang
2017-01-11  8:35       ` Niklas Söderlund
2017-01-11  8:35         ` Niklas Söderlund
2017-01-11  8:42         ` Geert Uytterhoeven
2017-01-11  9:17           ` Niklas Söderlund
2017-01-11  9:17             ` Niklas Söderlund
2017-01-11 15:05         ` Wolfram Sang
2017-01-11 15:57           ` Niklas Söderlund
2017-01-11 15:57             ` Niklas Söderlund
2017-01-11 15:59             ` Geert Uytterhoeven
2017-01-11 17:34               ` Wolfram Sang
2017-01-12 11:17                 ` Niklas Söderlund
2017-01-12 11:17                   ` Niklas Söderlund
2017-01-12 11:23                   ` Geert Uytterhoeven
2017-01-12 13:03                     ` Wolfram Sang
2017-01-17 11:29                       ` Niklas Söderlund
2017-01-17 11:29                         ` Niklas Söderlund
2017-01-23 11:16     ` Wolfram Sang
2017-01-31 20:56       ` Niklas Söderlund
2017-01-31 20:56         ` Niklas Söderlund
2017-01-26  9:58     ` Simon Horman
2016-11-10 11:45 ` [PATCH v8 0/6] UHS-I SDR-104 support for sh_mobile_sdhi Wolfram Sang
2016-11-10 22:21   ` Ulf Hansson
2016-11-11  7:20     ` Simon Horman

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.