[PATCH 0/3] mmc: dw_mmc: Generic clock phase tuning that works with rk3288 (possibly more!)

[PATCH 0/3] mmc: dw_mmc: Generic clock phase tuning that works with rk3288 (possibly more!)

[Alexandru M Stan]

This series implements most of what's required to have tuning work on rk3288. A
lot of this is inspired by the exynos tuning code, but generalized to work for
any number of phases. At one point I realized that the tuning is not rockchip
specific so I changed it to work for any dw_mmc device.

Rockchip specific stuff:
"rk3288: Add drive/sample clocks" needs "ARM: dts: rockchip: set dw_mmc max-freq
150Mhz" to apply cleanly. So this series is based on heiko's github repository:
https://github.com/mmind/linux-rockchip/tree/wip/v3.20-armsoc/dts

The missing DTO interrupt is still a problem in DWMMC, at least on rockchip. So
if you want to test this you might want to grab "[PATCH v3] mmc: dw_mmc: add
quirk for broken data transfer over scheme" and my change in the replies there.
Warning: it still causes problems on some cards.

And of course one would have to enable the high speed functionality, something
like this would suffice:
https://chromium-review.googlesource.com/#/c/227199/34/arch/arm/boot/dts/rk3288-veyron.dtsi


Alexandru M Stan (3):
  mmc: dw_mmc: dt-binding: Add tuning related things
  mmc: dw_mmc: Generic MMC tuning with the clock phase framework
  ARM: dts: rk3288: Add drive/sample clocks for dw_mmc devices

 .../devicetree/bindings/mmc/synopsys-dw-mshc.txt   |  14 +-
 arch/arm/boot/dts/rk3288.dtsi                      |  20 ++-
 drivers/mmc/host/dw_mmc.c                          | 189 +++++++++++++++++++++
 include/linux/mmc/dw_mmc.h                         |   3 +
 4 files changed, 214 insertions(+), 12 deletions(-)

-- 
2.2.0.rc0.207.ga3a616c

[PATCH 1/3] mmc: dw_mmc: dt-binding: Add tuning related things

[Alexandru M Stan]

Add ciu_drv, ciu_sample clocks and default-sample-phase. This will later be used
by tuning code.

We do not touch ciu_drive (and by extension define default-drive-phase). Drive
phase is mostly used to define minimum hold times, while one could write some
code to determine what phase meets the minimum hold time (ex 10 degrees) this
will not work with the current clock phase framework (which floors angles, so
we'll get 0 deg, and there's no way to know what resolution the floors happen
at). We assume that the default drive angles set by the hardware are good enough.

Signed-off-by: Alexandru M Stan <amstan@chromium.org>
Suggested-by: Heiko Stuebner <heiko@sntech.de>
Suggested-by: Doug Anderson <dianders@chromium.org>
---
 Documentation/devicetree/bindings/mmc/synopsys-dw-mshc.txt | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/Documentation/devicetree/bindings/mmc/synopsys-dw-mshc.txt b/Documentation/devicetree/bindings/mmc/synopsys-dw-mshc.txt
index 346c609..5edadc2 100644
--- a/Documentation/devicetree/bindings/mmc/synopsys-dw-mshc.txt
+++ b/Documentation/devicetree/bindings/mmc/synopsys-dw-mshc.txt
@@ -42,11 +42,13 @@ Optional properties:
 * clocks: from common clock binding: handle to biu and ciu clocks for the
   bus interface unit clock and the card interface unit clock.
 
-* clock-names: from common clock binding: Shall be "biu" and "ciu".
-  If the biu clock is missing we'll simply skip enabling it.  If the
-  ciu clock is missing we'll just assume that the clock is running at
+* clock-names: from common clock binding: Shall be "biu", "ciu", "ciu_drv" and
+  "ciu_sample".  If the biu clock is missing we'll simply skip enabling it.
+  If the ciu clock is missing we'll just assume that the clock is running at
   clock-frequency.  It is an error to omit both the ciu clock and the
-  clock-frequency.
+  clock-frequency.  "ciu_drv" and "ciu_sample" are used to control the clock
+  phases, "ciu_sample" is required for tuning high speed modes (if no other
+  custom tuning method is defined).
 
 * clock-frequency: should be the frequency (in Hz) of the ciu clock.  If this
   is specified and the ciu clock is specified then we'll try to set the ciu
@@ -75,6 +77,10 @@ Optional properties:
 * vmmc-supply: The phandle to the regulator to use for vmmc.  If this is
   specified we'll defer probe until we can find this regulator.
 
+* default-sample-phase: The default phase to set ciu_sample at probing, low
+  speeds or in case where all phases work at tuning time. If not specified
+  0 deg will be used.
+
 Aliases:
 
 - All the MSHC controller nodes should be represented in the aliases node using
-- 
2.2.0.rc0.207.ga3a616c

[PATCH 2/3] mmc: dw_mmc: Generic MMC tuning with the clock phase framework

[Alexandru M Stan]

This algorithm will try 5 degree increments, since there's no way to tell
what resolution the underlying phase code uses. As an added bonus, doing many
tunings yields better results since some tests are run more than once(ex: if the
underlying driver uses 45 degree increments, the tuning code will try the same
angle more than once).

It will then construct a list of good phase ranges (even ranges that cross
360/0), will pick the biggest range then it will set the sample_clk to the
middle of that range.

We do not touch ciu_drive (and by extension define default-drive-phase). Drive
phase is mostly used to define minimum hold times, while one could write some
code to determine what phase meets the minimum hold time (ex 10 degrees) this
will not work with the current clock phase framework (which floors angles, so
we'll get 0 deg, and there's no way to know what resolution the floors happen
at). We assume that the default drive angles set by the hardware are good enough.

If a device has device specific code (like exynos) then that will still take
precedence, otherwise this new code will execute. If the device wants to tune,
but has no sample_clk defined we'll return EIO with an error message.

Signed-off-by: Alexandru M Stan <amstan@chromium.org>
Suggested-by: Heiko Stuebner <heiko@sntech.de>
Suggested-by: Doug Anderson <dianders@chromium.org>
---
 drivers/mmc/host/dw_mmc.c  | 189 +++++++++++++++++++++++++++++++++++++++++++++
 include/linux/mmc/dw_mmc.h |   3 +
 2 files changed, 192 insertions(+)

diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index 69f0cc6..b59c221 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -984,6 +984,12 @@ static void dw_mci_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
 	if (drv_data && drv_data->set_ios)
 		drv_data->set_ios(slot->host, ios);
 
+	/* Make sure we use phases which we can enumerate with */
+	if (!IS_ERR(slot->host->sample_clk)) {
+		clk_set_phase(slot->host->sample_clk,
+			      slot->host->default_sample_phase);
+	}
+
 	/* Slot specific timing and width adjustment */
 	dw_mci_setup_bus(slot, false);
 
@@ -1187,6 +1193,174 @@ static void dw_mci_enable_sdio_irq(struct mmc_host *mmc, int enb)
 	}
 }
 
+static int dw_mci_tuning_test(struct dw_mci_slot *slot, u32 opcode,
+			       struct dw_mci_tuning_data *tuning_data,
+			       u8 *blk_test)
+{
+	struct dw_mci *host = slot->host;
+	struct mmc_host *mmc = slot->mmc;
+	const u8 *blk_pattern = tuning_data->blk_pattern;
+	unsigned int blksz = tuning_data->blksz;
+	struct mmc_request mrq = { NULL };
+	struct mmc_command cmd = {0};
+	struct mmc_command stop = {0};
+	struct mmc_data data = {0};
+	struct scatterlist sg;
+
+	memset(blk_test, 0, blksz);
+
+	cmd.opcode = opcode;
+	cmd.arg = 0;
+	cmd.flags = MMC_RSP_R1 | MMC_CMD_ADTC;
+
+	stop.opcode = MMC_STOP_TRANSMISSION;
+	stop.arg = 0;
+	stop.flags = MMC_RSP_R1B | MMC_CMD_AC;
+
+	data.blksz = blksz;
+	data.blocks = 1;
+	data.flags = MMC_DATA_READ;
+	data.sg = &sg;
+	data.sg_len = 1;
+
+	sg_init_one(&sg, blk_test, blksz);
+	mrq.cmd = &cmd;
+	mrq.stop = &stop;
+	mrq.data = &data;
+	host->mrq = &mrq;
+
+	mci_writel(host, TMOUT, ~0);
+
+	mmc_wait_for_req(mmc, &mrq);
+
+	if (!cmd.error && !data.error) {
+		if (!memcmp(blk_pattern, blk_test, blksz))
+			return 0;
+		return -EIO;
+	} else {
+		dev_dbg(host->dev,
+			"Tuning error: cmd.error:%d, data.error:%d\n",
+			cmd.error, data.error);
+		if (cmd.error)
+			return cmd.error;
+		else
+			return data.error;
+	}
+}
+
+static int dw_mci_execute_generic_tuning(struct dw_mci_slot *slot, u32 opcode,
+					 struct dw_mci_tuning_data *tuning_data)
+{
+	struct dw_mci *host = slot->host;
+	unsigned int blksz = tuning_data->blksz;
+	u8 *blk_test;
+	int ret = 0;
+	int i;
+	bool v, prev_v = 0, first_v;
+	struct range_t {
+		int start;
+		int end; /* inclusive */
+	};
+	struct range_t *ranges;
+	unsigned int range_count = 0;
+	int longest_range_len = -1;
+	int longest_range = -1;
+	int middle_phase;
+	const int PHASE_INCREMENT = 5;
+	const int NUM_PHASES = 360 / PHASE_INCREMENT;
+
+	if (IS_ERR(host->sample_clk)) {
+		dev_err(host->dev, "Tuning clock (sample_clk) not defined.\n");
+		return -EIO;
+	}
+
+	blk_test = kmalloc(blksz, GFP_KERNEL);
+	if (!blk_test)
+		return -ENOMEM;
+
+	ranges = kmalloc(((NUM_PHASES / 2 + 1) * sizeof(ranges)), GFP_KERNEL);
+	if (!blk_test) {
+		ret = -ENOMEM;
+		goto free_blk_test;
+	}
+
+	/* Try each phase and extract good ranges */
+	for (i = 0; i < NUM_PHASES; i++) {
+		clk_set_phase(host->sample_clk, i * PHASE_INCREMENT);
+
+		v = !dw_mci_tuning_test(slot, opcode, tuning_data, blk_test);
+
+		if ((!prev_v) && v) {
+			range_count++;
+			ranges[range_count-1].start = i;
+		}
+		if (v) {
+			ranges[range_count-1].end = i;
+		}
+
+		if (i == 0)
+			first_v = v;
+
+		prev_v = v;
+	}
+
+	if (range_count == 0) {
+		dev_info(host->dev, "All phases bad!");
+		ret = -EIO;
+		goto free;
+	}
+
+	/* wrap around case, merge the end points */
+	if ((range_count > 1) && first_v && v) {
+		ranges[0].start = ranges[range_count-1].start;
+		range_count--;
+	}
+
+	if ((ranges[0].start == 0) && (ranges[0].end == NUM_PHASES - 1)) {
+		clk_set_phase(host->sample_clk, host->default_sample_phase);
+		dev_info(host->dev, "All phases work, using default phase %d.",
+			host->default_sample_phase);
+		goto free;
+	}
+
+	/* Find the longest range */
+	for (i = 0; i < range_count; i++) {
+		int len = (ranges[i].end - ranges[i].start + 1);
+		if (len < 0)
+			len += NUM_PHASES;
+
+		if (longest_range_len < len) {
+			longest_range_len = len;
+			longest_range = i;
+		}
+
+		dev_dbg(host->dev, "Good phase range %d-%d (%d len)\n",
+			ranges[i].start * PHASE_INCREMENT,
+			ranges[i].end * PHASE_INCREMENT,
+			len
+		);
+	}
+
+	dev_dbg(host->dev, "Best phase range %d-%d (%d len)\n",
+		ranges[longest_range].start * PHASE_INCREMENT,
+		ranges[longest_range].end * PHASE_INCREMENT,
+		longest_range_len
+	);
+
+	middle_phase = ranges[longest_range].start + longest_range_len / 2;
+	middle_phase %= NUM_PHASES;
+	dev_info(host->dev, "Successfully tuned phase to %d\n",
+		middle_phase * PHASE_INCREMENT);
+
+	clk_set_phase(host->sample_clk, middle_phase * PHASE_INCREMENT);
+
+free:
+	kfree(ranges);
+free_blk_test:
+	kfree(blk_test);
+	return ret;
+}
+
 static int dw_mci_execute_tuning(struct mmc_host *mmc, u32 opcode)
 {
 	struct dw_mci_slot *slot = mmc_priv(mmc);
@@ -1216,6 +1390,8 @@ static int dw_mci_execute_tuning(struct mmc_host *mmc, u32 opcode)
 
 	if (drv_data && drv_data->execute_tuning)
 		err = drv_data->execute_tuning(slot, opcode, &tuning_data);
+	else
+		err = dw_mci_execute_generic_tuning(slot, opcode, &tuning_data);
 	return err;
 }
 
@@ -2492,6 +2668,11 @@ static struct dw_mci_board *dw_mci_parse_dt(struct dw_mci *host)
 	if (!of_property_read_u32(np, "clock-frequency", &clock_frequency))
 		pdata->bus_hz = clock_frequency;
 
+	if (of_property_read_u32(np, "default-sample-phase",
+					&host->default_sample_phase)) {
+		host->default_sample_phase = 0;
+	}
+
 	if (drv_data && drv_data->parse_dt) {
 		ret = drv_data->parse_dt(host);
 		if (ret)
@@ -2564,6 +2745,14 @@ int dw_mci_probe(struct dw_mci *host)
 		host->bus_hz = clk_get_rate(host->ciu_clk);
 	}
 
+	host->drv_clk = devm_clk_get(host->dev, "ciu_drv");
+	if (IS_ERR(host->drv_clk))
+		dev_dbg(host->dev, "ciu_drv not available\n");
+
+	host->sample_clk = devm_clk_get(host->dev, "ciu_sample");
+	if (IS_ERR(host->sample_clk))
+		dev_dbg(host->dev, "ciu_sample not available\n");
+
 	if (!host->bus_hz) {
 		dev_err(host->dev,
 			"Platform data must supply bus speed\n");
diff --git a/include/linux/mmc/dw_mmc.h b/include/linux/mmc/dw_mmc.h
index 0013669..335e2f3 100644
--- a/include/linux/mmc/dw_mmc.h
+++ b/include/linux/mmc/dw_mmc.h
@@ -172,7 +172,10 @@ struct dw_mci {
 	void			*priv;
 	struct clk		*biu_clk;
 	struct clk		*ciu_clk;
+	struct clk		*drv_clk;
+	struct clk		*sample_clk;
 	struct dw_mci_slot	*slot[MAX_MCI_SLOTS];
+	int			default_sample_phase;
 
 	/* FIFO push and pull */
 	int			fifo_depth;
-- 
2.2.0.rc0.207.ga3a616c

[PATCH 3/3] ARM: dts: rk3288: Add drive/sample clocks for dw_mmc devices

[Alexandru M Stan]

The drive/sample clocks can be phase shifted.  The drive clock
could be used in a future patch to adjust hold times.  The sample
clock is used for tuning.

Signed-off-by: Alexandru M Stan <amstan@chromium.org>
Signed-off-by: Doug Anderson <dianders@chromium.org>
Suggested-by: Heiko Stuebner <heiko@sntech.de>
Suggested-by: Doug Anderson <dianders@chromium.org>
---
 arch/arm/boot/dts/rk3288.dtsi | 20 ++++++++++++--------
 1 file changed, 12 insertions(+), 8 deletions(-)

diff --git a/arch/arm/boot/dts/rk3288.dtsi b/arch/arm/boot/dts/rk3288.dtsi
index 9c35a1d..49e498e 100644
--- a/arch/arm/boot/dts/rk3288.dtsi
+++ b/arch/arm/boot/dts/rk3288.dtsi
@@ -150,8 +150,9 @@
 	sdmmc: dwmmc at ff0c0000 {
 		compatible = "rockchip,rk3288-dw-mshc";
 		clock-freq-min-max = <400000 150000000>;
-		clocks = <&cru HCLK_SDMMC>, <&cru SCLK_SDMMC>;
-		clock-names = "biu", "ciu";
+		clocks = <&cru HCLK_SDMMC>, <&cru SCLK_SDMMC>,
+			 <&cru SCLK_SDMMC_DRV>, <&cru SCLK_SDMMC_SAMPLE>;
+		clock-names = "biu", "ciu", "ciu_drv", "ciu_sample";
 		fifo-depth = <0x100>;
 		interrupts = <GIC_SPI 32 IRQ_TYPE_LEVEL_HIGH>;
 		reg = <0xff0c0000 0x4000>;
@@ -161,8 +162,9 @@
 	sdio0: dwmmc at ff0d0000 {
 		compatible = "rockchip,rk3288-dw-mshc";
 		clock-freq-min-max = <400000 150000000>;
-		clocks = <&cru HCLK_SDIO0>, <&cru SCLK_SDIO0>;
-		clock-names = "biu", "ciu";
+		clocks = <&cru HCLK_SDIO0>, <&cru SCLK_SDIO0>,
+			 <&cru SCLK_SDIO0_DRV>, <&cru SCLK_SDIO0_SAMPLE>;
+		clock-names = "biu", "ciu", "ciu_drv", "ciu_sample";
 		fifo-depth = <0x100>;
 		interrupts = <GIC_SPI 33 IRQ_TYPE_LEVEL_HIGH>;
 		reg = <0xff0d0000 0x4000>;
@@ -172,8 +174,9 @@
 	sdio1: dwmmc at ff0e0000 {
 		compatible = "rockchip,rk3288-dw-mshc";
 		clock-freq-min-max = <400000 150000000>;
-		clocks = <&cru HCLK_SDIO1>, <&cru SCLK_SDIO1>;
-		clock-names = "biu", "ciu";
+		clocks = <&cru HCLK_SDIO1>, <&cru SCLK_SDIO1>,
+			 <&cru SCLK_SDIO1_DRV>, <&cru SCLK_SDIO1_SAMPLE>;
+		clock-names = "biu", "ciu", "ciu_drv", "ciu_sample";
 		fifo-depth = <0x100>;
 		interrupts = <GIC_SPI 34 IRQ_TYPE_LEVEL_HIGH>;
 		reg = <0xff0e0000 0x4000>;
@@ -183,8 +186,9 @@
 	emmc: dwmmc at ff0f0000 {
 		compatible = "rockchip,rk3288-dw-mshc";
 		clock-freq-min-max = <400000 150000000>;
-		clocks = <&cru HCLK_EMMC>, <&cru SCLK_EMMC>;
-		clock-names = "biu", "ciu";
+		clocks = <&cru HCLK_EMMC>, <&cru SCLK_EMMC>,
+			 <&cru SCLK_EMMC_DRV>, <&cru SCLK_EMMC_SAMPLE>;
+		clock-names = "biu", "ciu", "ciu_drv", "ciu_sample";
 		fifo-depth = <0x100>;
 		interrupts = <GIC_SPI 35 IRQ_TYPE_LEVEL_HIGH>;
 		reg = <0xff0f0000 0x4000>;
-- 
2.2.0.rc0.207.ga3a616c

[PATCH 2/3] mmc: dw_mmc: Generic MMC tuning with the clock phase framework

[Doug Anderson]

Alex,

On Thu, Dec 18, 2014 at 3:01 PM, Alexandru M Stan <amstan@chromium.org> wrote:
> +       blk_test = kmalloc(blksz, GFP_KERNEL);
> +       if (!blk_test)
> +               return -ENOMEM;
> +
> +       ranges = kmalloc(((NUM_PHASES / 2 + 1) * sizeof(ranges)), GFP_KERNEL);

sizeof(*ranges)


Other than that, this looks good to me and you can add my reviewed-by
when the above is fixed.

[PATCH 3/3] ARM: dts: rk3288: Add drive/sample clocks for dw_mmc devices

[Doug Anderson]

Alex,

On Thu, Dec 18, 2014 at 3:01 PM, Alexandru M Stan <amstan@chromium.org> wrote:
> The drive/sample clocks can be phase shifted.  The drive clock
> could be used in a future patch to adjust hold times.  The sample
> clock is used for tuning.
>
> Signed-off-by: Alexandru M Stan <amstan@chromium.org>
> Signed-off-by: Doug Anderson <dianders@chromium.org>
> Suggested-by: Heiko Stuebner <heiko@sntech.de>
> Suggested-by: Doug Anderson <dianders@chromium.org>
> ---
>  arch/arm/boot/dts/rk3288.dtsi | 20 ++++++++++++--------
>  1 file changed, 12 insertions(+), 8 deletions(-)

I'm not sure I really need a sign-off-by for this since I don't think
I did much besides reupload it once or something.  You can add my
reviewed by instead:

Reviewed-by: Doug Anderson <dianders@chromium.org>

[PATCH 1/3] mmc: dw_mmc: dt-binding: Add tuning related things

[Doug Anderson]

Alex,

On Thu, Dec 18, 2014 at 3:01 PM, Alexandru M Stan <amstan@chromium.org> wrote:
> Add ciu_drv, ciu_sample clocks and default-sample-phase. This will later be used
> by tuning code.
>
> We do not touch ciu_drive (and by extension define default-drive-phase). Drive
> phase is mostly used to define minimum hold times, while one could write some
> code to determine what phase meets the minimum hold time (ex 10 degrees) this
> will not work with the current clock phase framework (which floors angles, so
> we'll get 0 deg, and there's no way to know what resolution the floors happen
> at). We assume that the default drive angles set by the hardware are good enough.
>
> Signed-off-by: Alexandru M Stan <amstan@chromium.org>
> Suggested-by: Heiko Stuebner <heiko@sntech.de>
> Suggested-by: Doug Anderson <dianders@chromium.org>
> ---
>  Documentation/devicetree/bindings/mmc/synopsys-dw-mshc.txt | 14 ++++++++++----
>  1 file changed, 10 insertions(+), 4 deletions(-)

Reviewed-by: Doug Anderson <dianders@chromium.org>

[PATCH 2/3] mmc: dw_mmc: Generic MMC tuning with the clock phase framework

[Ulf Hansson]

On 19 December 2014 at 00:01, Alexandru M Stan <amstan@chromium.org> wrote:
> This algorithm will try 5 degree increments, since there's no way to tell
> what resolution the underlying phase code uses. As an added bonus, doing many
> tunings yields better results since some tests are run more than once(ex: if the
> underlying driver uses 45 degree increments, the tuning code will try the same
> angle more than once).
>
> It will then construct a list of good phase ranges (even ranges that cross
> 360/0), will pick the biggest range then it will set the sample_clk to the
> middle of that range.
>
> We do not touch ciu_drive (and by extension define default-drive-phase). Drive
> phase is mostly used to define minimum hold times, while one could write some
> code to determine what phase meets the minimum hold time (ex 10 degrees) this
> will not work with the current clock phase framework (which floors angles, so
> we'll get 0 deg, and there's no way to know what resolution the floors happen
> at). We assume that the default drive angles set by the hardware are good enough.
>
> If a device has device specific code (like exynos) then that will still take
> precedence, otherwise this new code will execute. If the device wants to tune,
> but has no sample_clk defined we'll return EIO with an error message.
>
> Signed-off-by: Alexandru M Stan <amstan@chromium.org>
> Suggested-by: Heiko Stuebner <heiko@sntech.de>
> Suggested-by: Doug Anderson <dianders@chromium.org>
> ---
>  drivers/mmc/host/dw_mmc.c  | 189 +++++++++++++++++++++++++++++++++++++++++++++
>  include/linux/mmc/dw_mmc.h |   3 +
>  2 files changed, 192 insertions(+)
>
> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
> index 69f0cc6..b59c221 100644
> --- a/drivers/mmc/host/dw_mmc.c
> +++ b/drivers/mmc/host/dw_mmc.c
> @@ -984,6 +984,12 @@ static void dw_mci_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
>         if (drv_data && drv_data->set_ios)
>                 drv_data->set_ios(slot->host, ios);
>
> +       /* Make sure we use phases which we can enumerate with */
> +       if (!IS_ERR(slot->host->sample_clk)) {
> +               clk_set_phase(slot->host->sample_clk,
> +                             slot->host->default_sample_phase);
> +       }
> +
>         /* Slot specific timing and width adjustment */
>         dw_mci_setup_bus(slot, false);
>
> @@ -1187,6 +1193,174 @@ static void dw_mci_enable_sdio_irq(struct mmc_host *mmc, int enb)
>         }
>  }
>
> +static int dw_mci_tuning_test(struct dw_mci_slot *slot, u32 opcode,
> +                              struct dw_mci_tuning_data *tuning_data,
> +                              u8 *blk_test)
> +{
> +       struct dw_mci *host = slot->host;
> +       struct mmc_host *mmc = slot->mmc;
> +       const u8 *blk_pattern = tuning_data->blk_pattern;
> +       unsigned int blksz = tuning_data->blksz;
> +       struct mmc_request mrq = { NULL };
> +       struct mmc_command cmd = {0};
> +       struct mmc_command stop = {0};
> +       struct mmc_data data = {0};
> +       struct scatterlist sg;
> +
> +       memset(blk_test, 0, blksz);
> +
> +       cmd.opcode = opcode;
> +       cmd.arg = 0;
> +       cmd.flags = MMC_RSP_R1 | MMC_CMD_ADTC;
> +
> +       stop.opcode = MMC_STOP_TRANSMISSION;
> +       stop.arg = 0;
> +       stop.flags = MMC_RSP_R1B | MMC_CMD_AC;
> +
> +       data.blksz = blksz;
> +       data.blocks = 1;
> +       data.flags = MMC_DATA_READ;
> +       data.sg = &sg;
> +       data.sg_len = 1;
> +
> +       sg_init_one(&sg, blk_test, blksz);
> +       mrq.cmd = &cmd;
> +       mrq.stop = &stop;
> +       mrq.data = &data;
> +       host->mrq = &mrq;
> +
> +       mci_writel(host, TMOUT, ~0);
> +
> +       mmc_wait_for_req(mmc, &mrq);
> +
> +       if (!cmd.error && !data.error) {
> +               if (!memcmp(blk_pattern, blk_test, blksz))
> +                       return 0;
> +               return -EIO;
> +       } else {
> +               dev_dbg(host->dev,
> +                       "Tuning error: cmd.error:%d, data.error:%d\n",
> +                       cmd.error, data.error);
> +               if (cmd.error)
> +                       return cmd.error;
> +               else
> +                       return data.error;
> +       }

This function should be simplified using mmc_send_tuning().

Now, dw_mmc seems to have some issues that it needs the tuning command
to have a stop command sent as well. That's not according to the eMMC
spec.

Please have a look at this patch:
http://www.spinics.net/lists/linux-mmc/msg29807.html

Kind regards
Uffe

> +}
> +
> +static int dw_mci_execute_generic_tuning(struct dw_mci_slot *slot, u32 opcode,
> +                                        struct dw_mci_tuning_data *tuning_data)
> +{
> +       struct dw_mci *host = slot->host;
> +       unsigned int blksz = tuning_data->blksz;
> +       u8 *blk_test;
> +       int ret = 0;
> +       int i;
> +       bool v, prev_v = 0, first_v;
> +       struct range_t {
> +               int start;
> +               int end; /* inclusive */
> +       };
> +       struct range_t *ranges;
> +       unsigned int range_count = 0;
> +       int longest_range_len = -1;
> +       int longest_range = -1;
> +       int middle_phase;
> +       const int PHASE_INCREMENT = 5;
> +       const int NUM_PHASES = 360 / PHASE_INCREMENT;
> +
> +       if (IS_ERR(host->sample_clk)) {
> +               dev_err(host->dev, "Tuning clock (sample_clk) not defined.\n");
> +               return -EIO;
> +       }
> +
> +       blk_test = kmalloc(blksz, GFP_KERNEL);
> +       if (!blk_test)
> +               return -ENOMEM;
> +
> +       ranges = kmalloc(((NUM_PHASES / 2 + 1) * sizeof(ranges)), GFP_KERNEL);
> +       if (!blk_test) {
> +               ret = -ENOMEM;
> +               goto free_blk_test;
> +       }
> +
> +       /* Try each phase and extract good ranges */
> +       for (i = 0; i < NUM_PHASES; i++) {
> +               clk_set_phase(host->sample_clk, i * PHASE_INCREMENT);
> +
> +               v = !dw_mci_tuning_test(slot, opcode, tuning_data, blk_test);
> +
> +               if ((!prev_v) && v) {
> +                       range_count++;
> +                       ranges[range_count-1].start = i;
> +               }
> +               if (v) {
> +                       ranges[range_count-1].end = i;
> +               }
> +
> +               if (i == 0)
> +                       first_v = v;
> +
> +               prev_v = v;
> +       }
> +
> +       if (range_count == 0) {
> +               dev_info(host->dev, "All phases bad!");
> +               ret = -EIO;
> +               goto free;
> +       }
> +
> +       /* wrap around case, merge the end points */
> +       if ((range_count > 1) && first_v && v) {
> +               ranges[0].start = ranges[range_count-1].start;
> +               range_count--;
> +       }
> +
> +       if ((ranges[0].start == 0) && (ranges[0].end == NUM_PHASES - 1)) {
> +               clk_set_phase(host->sample_clk, host->default_sample_phase);
> +               dev_info(host->dev, "All phases work, using default phase %d.",
> +                       host->default_sample_phase);
> +               goto free;
> +       }
> +
> +       /* Find the longest range */
> +       for (i = 0; i < range_count; i++) {
> +               int len = (ranges[i].end - ranges[i].start + 1);
> +               if (len < 0)
> +                       len += NUM_PHASES;
> +
> +               if (longest_range_len < len) {
> +                       longest_range_len = len;
> +                       longest_range = i;
> +               }
> +
> +               dev_dbg(host->dev, "Good phase range %d-%d (%d len)\n",
> +                       ranges[i].start * PHASE_INCREMENT,
> +                       ranges[i].end * PHASE_INCREMENT,
> +                       len
> +               );
> +       }
> +
> +       dev_dbg(host->dev, "Best phase range %d-%d (%d len)\n",
> +               ranges[longest_range].start * PHASE_INCREMENT,
> +               ranges[longest_range].end * PHASE_INCREMENT,
> +               longest_range_len
> +       );
> +
> +       middle_phase = ranges[longest_range].start + longest_range_len / 2;
> +       middle_phase %= NUM_PHASES;
> +       dev_info(host->dev, "Successfully tuned phase to %d\n",
> +               middle_phase * PHASE_INCREMENT);
> +
> +       clk_set_phase(host->sample_clk, middle_phase * PHASE_INCREMENT);
> +
> +free:
> +       kfree(ranges);
> +free_blk_test:
> +       kfree(blk_test);
> +       return ret;
> +}
> +
>  static int dw_mci_execute_tuning(struct mmc_host *mmc, u32 opcode)
>  {
>         struct dw_mci_slot *slot = mmc_priv(mmc);
> @@ -1216,6 +1390,8 @@ static int dw_mci_execute_tuning(struct mmc_host *mmc, u32 opcode)
>
>         if (drv_data && drv_data->execute_tuning)
>                 err = drv_data->execute_tuning(slot, opcode, &tuning_data);
> +       else
> +               err = dw_mci_execute_generic_tuning(slot, opcode, &tuning_data);
>         return err;
>  }
>
> @@ -2492,6 +2668,11 @@ static struct dw_mci_board *dw_mci_parse_dt(struct dw_mci *host)
>         if (!of_property_read_u32(np, "clock-frequency", &clock_frequency))
>                 pdata->bus_hz = clock_frequency;
>
> +       if (of_property_read_u32(np, "default-sample-phase",
> +                                       &host->default_sample_phase)) {
> +               host->default_sample_phase = 0;
> +       }
> +
>         if (drv_data && drv_data->parse_dt) {
>                 ret = drv_data->parse_dt(host);
>                 if (ret)
> @@ -2564,6 +2745,14 @@ int dw_mci_probe(struct dw_mci *host)
>                 host->bus_hz = clk_get_rate(host->ciu_clk);
>         }
>
> +       host->drv_clk = devm_clk_get(host->dev, "ciu_drv");
> +       if (IS_ERR(host->drv_clk))
> +               dev_dbg(host->dev, "ciu_drv not available\n");
> +
> +       host->sample_clk = devm_clk_get(host->dev, "ciu_sample");
> +       if (IS_ERR(host->sample_clk))
> +               dev_dbg(host->dev, "ciu_sample not available\n");
> +
>         if (!host->bus_hz) {
>                 dev_err(host->dev,
>                         "Platform data must supply bus speed\n");
> diff --git a/include/linux/mmc/dw_mmc.h b/include/linux/mmc/dw_mmc.h
> index 0013669..335e2f3 100644
> --- a/include/linux/mmc/dw_mmc.h
> +++ b/include/linux/mmc/dw_mmc.h
> @@ -172,7 +172,10 @@ struct dw_mci {
>         void                    *priv;
>         struct clk              *biu_clk;
>         struct clk              *ciu_clk;
> +       struct clk              *drv_clk;
> +       struct clk              *sample_clk;
>         struct dw_mci_slot      *slot[MAX_MCI_SLOTS];
> +       int                     default_sample_phase;
>
>         /* FIFO push and pull */
>         int                     fifo_depth;
> --
> 2.2.0.rc0.207.ga3a616c
>

[PATCH 2/3] mmc: dw_mmc: Generic MMC tuning with the clock phase framework

[Stephen Boyd]

On 12/18/2014 04:20 PM, Doug Anderson wrote:
> Alex,
>
> On Thu, Dec 18, 2014 at 3:01 PM, Alexandru M Stan <amstan@chromium.org> wrote:
>> +       blk_test = kmalloc(blksz, GFP_KERNEL);
>> +       if (!blk_test)
>> +               return -ENOMEM;
>> +
>> +       ranges = kmalloc(((NUM_PHASES / 2 + 1) * sizeof(ranges)), GFP_KERNEL);
> sizeof(*ranges)
>
>
> Other than that, this looks good to me and you can add my reviewed-by
> when the above is fixed.
>
>

Might want to use kmalloc_array() too.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project