[PATCH v2 0/6] arm64: allwinner: a64: Enable MMC support

[PATCH v2 0/6] arm64: allwinner: a64: Enable MMC support

[Maxime Ripard]

Hi,

Here is a second attempt at getting the MMC controllers running, the first
having been done by Andre.

This has been tested on a board with one SDIO device (a Marvell WiFi chip)
and a Kingston eMMC with 1.8V IOs.

For SDIO, the HS DDR mode works just fine. There's a little bit of work to
get to SDR104, but that will come eventually.

For the eMMC, HS200 with the voltage switch works. HS400 doesn't at the
moment, but since it's significantly more complex, and at the same time
Allwinner recommends to limit its frequency to 100MHz, this doesn't have
any benefits. If there's any at some point, this can be added later.

Let me know what you think,
Maxime

Andre Przywara (1):
  arm64: allwinner: a64: Add MMC nodes

Maxime Ripard (5):
  mmc: sunxi: Always set signal delay to 0 for A64
  mmc: sunxi: Enable the new timings for the A64 MMC controllers
  mmc: sunxi: Add EMMC (MMC2) controller compatible
  arm64: allwinner: a64: Add MMC pinctrl nodes
  arm64: allwinner: a64: Increase the MMC max frequency

 arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi | 67 ++++++++++++++++++++-
 drivers/mmc/host/sunxi-mmc.c                  | 63 +++++++++----------
 2 files changed, 97 insertions(+), 33 deletions(-)

base-commit: f9ca9b952ee139fbb9cd4d354a33f440bc1049cd
-- 
git-series 0.8.11

[PATCH v2 1/6] mmc: sunxi: Always set signal delay to 0 for A64

[Maxime Ripard]

Experience have shown that the using the  autocalibration could severely
degrade the performances of the MMC bus.

Allwinner is using in its BSP a delay set to 0 for all the modes but HS400.
Remove the calibration code for now, and add comments to document our
findings.

Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
 drivers/mmc/host/sunxi-mmc.c | 50 ++++++++++++-------------------------
 1 file changed, 17 insertions(+), 33 deletions(-)

diff --git a/drivers/mmc/host/sunxi-mmc.c b/drivers/mmc/host/sunxi-mmc.c
index b1d1303389a7..ea9552a0d820 100644
--- a/drivers/mmc/host/sunxi-mmc.c
+++ b/drivers/mmc/host/sunxi-mmc.c
@@ -683,41 +683,19 @@ static int sunxi_mmc_oclk_onoff(struct sunxi_mmc_host *host, u32 oclk_en)
 
 static int sunxi_mmc_calibrate(struct sunxi_mmc_host *host, int reg_off)
 {
-	u32 reg = readl(host->reg_base + reg_off);
-	u32 delay;
-	unsigned long timeout;
-
 	if (!host->cfg->can_calibrate)
 		return 0;
 
-	reg &= ~(SDXC_CAL_DL_MASK << SDXC_CAL_DL_SW_SHIFT);
-	reg &= ~SDXC_CAL_DL_SW_EN;
-
-	writel(reg | SDXC_CAL_START, host->reg_base + reg_off);
-
-	dev_dbg(mmc_dev(host->mmc), "calibration started\n");
-
-	timeout = jiffies + HZ * SDXC_CAL_TIMEOUT;
-
-	while (!((reg = readl(host->reg_base + reg_off)) & SDXC_CAL_DONE)) {
-		if (time_before(jiffies, timeout))
-			cpu_relax();
-		else {
-			reg &= ~SDXC_CAL_START;
-			writel(reg, host->reg_base + reg_off);
-
-			return -ETIMEDOUT;
-		}
-	}
-
-	delay = (reg >> SDXC_CAL_DL_SHIFT) & SDXC_CAL_DL_MASK;
-
-	reg &= ~SDXC_CAL_START;
-	reg |= (delay << SDXC_CAL_DL_SW_SHIFT) | SDXC_CAL_DL_SW_EN;
-
-	writel(reg, host->reg_base + reg_off);
-
-	dev_dbg(mmc_dev(host->mmc), "calibration ended, reg is 0x%x\n", reg);
+	/*
+	 * FIXME:
+	 * This is not clear how the calibration is supposed to work
+	 * yet. The best rate have been obtained by simply setting the
+	 * delay to 0, as Allwinner does in its BSP.
+	 *
+	 * The only mode that doesn't have such a delay is HS400, that
+	 * is in itself a TODO.
+	 */
+	writel(SDXC_CAL_DL_SW_EN, host->reg_base + reg_off);
 
 	return 0;
 }
@@ -806,7 +784,13 @@ static int sunxi_mmc_clk_set_rate(struct sunxi_mmc_host *host,
 	if (ret)
 		return ret;
 
-	/* TODO: enable calibrate on sdc2 SDXC_REG_DS_DL_REG of A64 */
+	/*
+	 * FIXME:
+	 *
+	 * In HS400 we'll also need to calibrate the data strobe
+	 * signal. This should only happen on the MMC2 controller (at
+	 * least on the A64 and older SoCs).
+	 */
 
 	return sunxi_mmc_oclk_onoff(host, 1);
 }
-- 
git-series 0.8.11

[PATCH v2 2/6] mmc: sunxi: Enable the new timings for the A64 MMC controllers

[Maxime Ripard]

The A64 MMC controllers need to set a "new timings" bit when a new rate is
set.

The actual meaning of that bit is not clear yet, but not setting it leads
to some corner-case issues, like the CMD53 failing, which is used to
implement SDIO packet aggregation.

Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
 drivers/mmc/host/sunxi-mmc.c | 6 ++++++
 1 file changed, 6 insertions(+), 0 deletions(-)

diff --git a/drivers/mmc/host/sunxi-mmc.c b/drivers/mmc/host/sunxi-mmc.c
index ea9552a0d820..9a860bcac154 100644
--- a/drivers/mmc/host/sunxi-mmc.c
+++ b/drivers/mmc/host/sunxi-mmc.c
@@ -253,6 +253,8 @@ struct sunxi_mmc_cfg {
 
 	/* does the IP block support autocalibration? */
 	bool can_calibrate;
+
+	bool needs_new_timings;
 };
 
 struct sunxi_mmc_host {
@@ -776,6 +778,9 @@ static int sunxi_mmc_clk_set_rate(struct sunxi_mmc_host *host,
 	}
 	mmc_writel(host, REG_CLKCR, rval);
 
+	if (host->cfg->needs_new_timings)
+		mmc_writel(host, REG_SD_NTSR, SDXC_2X_TIMING_MODE);
+
 	ret = sunxi_mmc_clk_set_phase(host, ios, rate);
 	if (ret)
 		return ret;
@@ -1073,6 +1078,7 @@ static const struct sunxi_mmc_cfg sun50i_a64_cfg = {
 	.idma_des_size_bits = 16,
 	.clk_delays = NULL,
 	.can_calibrate = true,
+	.needs_new_timings = true,
 };
 
 static const struct of_device_id sunxi_mmc_of_match[] = {
-- 
git-series 0.8.11

[PATCH v2 3/6] mmc: sunxi: Add EMMC (MMC2) controller compatible

[Maxime Ripard]

The MMC2 controller on the A64 is kind of a special beast.

While the general controller design is the same than the other MMC
controllers in the SoC, it also has a bunch of features and changes that
prevent it to be driven in the same way.

It has for example a different bus width limit, a different maximum
frequency, and, for some reason, the maximum buffer size of a DMA
descriptor.

Add a new compatible specifically for this controller.

Acked-by: Rob Herring <robh@kernel.org>
Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
 drivers/mmc/host/sunxi-mmc.c | 7 +++++++
 1 file changed, 7 insertions(+), 0 deletions(-)

diff --git a/drivers/mmc/host/sunxi-mmc.c b/drivers/mmc/host/sunxi-mmc.c
index 9a860bcac154..bdcc87c9d8b8 100644
--- a/drivers/mmc/host/sunxi-mmc.c
+++ b/drivers/mmc/host/sunxi-mmc.c
@@ -1081,12 +1081,19 @@ static const struct sunxi_mmc_cfg sun50i_a64_cfg = {
 	.needs_new_timings = true,
 };
 
+static const struct sunxi_mmc_cfg sun50i_a64_emmc_cfg = {
+	.idma_des_size_bits = 13,
+	.clk_delays = NULL,
+	.can_calibrate = true,
+};
+
 static const struct of_device_id sunxi_mmc_of_match[] = {
 	{ .compatible = "allwinner,sun4i-a10-mmc", .data = &sun4i_a10_cfg },
 	{ .compatible = "allwinner,sun5i-a13-mmc", .data = &sun5i_a13_cfg },
 	{ .compatible = "allwinner,sun7i-a20-mmc", .data = &sun7i_a20_cfg },
 	{ .compatible = "allwinner,sun9i-a80-mmc", .data = &sun9i_a80_cfg },
 	{ .compatible = "allwinner,sun50i-a64-mmc", .data = &sun50i_a64_cfg },
+	{ .compatible = "allwinner,sun50i-a64-emmc", .data = &sun50i_a64_emmc_cfg },
 	{ /* sentinel */ }
 };
 MODULE_DEVICE_TABLE(of, sunxi_mmc_of_match);
-- 
git-series 0.8.11

[PATCH v2 4/6] arm64: allwinner: a64: Add MMC nodes

[Maxime Ripard]

From: Andre Przywara <andre.przywara@arm.com>

The A64 has 3 MMC controllers, one of them being especially targeted to
eMMC. Among other things, it has a data strobe signal and a 8 bits data
width.

The two other are more usual controllers that will have a 4 bits width at
most and no data strobe signal, which limits it to more usual SD or MMC
peripherals.

Signed-off-by: Andre Przywara <andre.przywara@arm.com>
Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
 arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi | 39 ++++++++++++++++++++-
 1 file changed, 39 insertions(+), 0 deletions(-)

diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
index 99b6bb1e141c..143e9706438f 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
+++ b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
@@ -299,6 +299,45 @@
 			#size-cells = <0>;
 		};
 
+		mmc0: mmc@1c0f000 {
+			compatible = "allwinner,sun50i-a64-mmc";
+			reg = <0x01c0f000 0x1000>;
+			clocks = <&ccu CLK_BUS_MMC0>, <&ccu CLK_MMC0>;
+			clock-names = "ahb", "mmc";
+			resets = <&ccu RST_BUS_MMC0>;
+			reset-names = "ahb";
+			interrupts = <GIC_SPI 60 IRQ_TYPE_LEVEL_HIGH>;
+			status = "disabled";
+			#address-cells = <1>;
+			#size-cells = <0>;
+		};
+
+		mmc1: mmc@1c10000 {
+			compatible = "allwinner,sun50i-a64-mmc";
+			reg = <0x01c10000 0x1000>;
+			clocks = <&ccu CLK_BUS_MMC1>, <&ccu CLK_MMC1>;
+			clock-names = "ahb", "mmc";
+			resets = <&ccu RST_BUS_MMC1>;
+			reset-names = "ahb";
+			interrupts = <GIC_SPI 61 IRQ_TYPE_LEVEL_HIGH>;
+			status = "disabled";
+			#address-cells = <1>;
+			#size-cells = <0>;
+		};
+
+		mmc2: mmc@1c11000 {
+			compatible = "allwinner,sun50i-a64-emmc";
+			reg = <0x01c11000 0x1000>;
+			clocks = <&ccu CLK_BUS_MMC2>, <&ccu CLK_MMC2>;
+			clock-names = "ahb", "mmc";
+			resets = <&ccu RST_BUS_MMC2>;
+			reset-names = "ahb";
+			interrupts = <GIC_SPI 62 IRQ_TYPE_LEVEL_HIGH>;
+			status = "disabled";
+			#address-cells = <1>;
+			#size-cells = <0>;
+		};
+
 		gic: interrupt-controller@1c81000 {
 			compatible = "arm,gic-400";
 			reg = <0x01c81000 0x1000>,
-- 
git-series 0.8.11

[PATCH v2 5/6] arm64: allwinner: a64: Add MMC pinctrl nodes

[Maxime Ripard]

The A64 only has a single set of pins for each MMC controller. Since we
already have boards that require all of them, let's add them to the DTSI.

Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
 arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi | 25 ++++++++++++++++++++-
 1 file changed, 25 insertions(+), 0 deletions(-)

diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
index 143e9706438f..8e149498e096 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
+++ b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
@@ -205,6 +205,31 @@
 				function = "i2c1";
 			};
 
+			mmc0_pins: mmc0-pins {
+				pins = "PF0", "PF1", "PF2", "PF3",
+				       "PF4", "PF5";
+				function = "mmc0";
+				drive-strength = <30>;
+				bias-pull-up;
+			};
+
+			mmc1_pins: mmc1-pins {
+				pins = "PG0", "PG1", "PG2", "PG3",
+				       "PG4", "PG5";
+				function = "mmc1";
+				drive-strength = <30>;
+				bias-pull-up;
+			};
+
+			mmc2_pins: mmc2-pins {
+				pins = "PC1", "PC5", "PC6", "PC8", "PC9",
+				       "PC10","PC11", "PC12", "PC13",
+				       "PC14", "PC15", "PC16";
+				function = "mmc2";
+				drive-strength = <30>;
+				bias-pull-up;
+			};
+
 			uart0_pins_a: uart0@0 {
 				pins = "PB8", "PB9";
 				function = "uart0";
-- 
git-series 0.8.11

[PATCH v2 6/6] arm64: allwinner: a64: Increase the MMC max frequency

[Maxime Ripard]

All the controllers can have a maximum frequency of 200MHz.

Since older SoCs cannot go that high, we cannot change the default maximum
frequency, but fortunately for us we have a property for that in the DT.

This also has the side effect of allowing to use the MMC HS200 mode for the
boards that support it (with either 1.2v or 1.8v IOs).

Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
 arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi | 3 +++
 1 file changed, 3 insertions(+), 0 deletions(-)

diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
index 8e149498e096..f46ae965cf5b 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
+++ b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
@@ -332,6 +332,7 @@
 			resets = <&ccu RST_BUS_MMC0>;
 			reset-names = "ahb";
 			interrupts = <GIC_SPI 60 IRQ_TYPE_LEVEL_HIGH>;
+			max-frequency = <200000000>;
 			status = "disabled";
 			#address-cells = <1>;
 			#size-cells = <0>;
@@ -345,6 +346,7 @@
 			resets = <&ccu RST_BUS_MMC1>;
 			reset-names = "ahb";
 			interrupts = <GIC_SPI 61 IRQ_TYPE_LEVEL_HIGH>;
+			max-frequency = <200000000>;
 			status = "disabled";
 			#address-cells = <1>;
 			#size-cells = <0>;
@@ -358,6 +360,7 @@
 			resets = <&ccu RST_BUS_MMC2>;
 			reset-names = "ahb";
 			interrupts = <GIC_SPI 62 IRQ_TYPE_LEVEL_HIGH>;
+			max-frequency = <200000000>;
 			status = "disabled";
 			#address-cells = <1>;
 			#size-cells = <0>;
-- 
git-series 0.8.11

Re: [PATCH v2 6/6] arm64: allwinner: a64: Increase the MMC max frequency

[Chen-Yu Tsai]

On Tue, Jan 10, 2017 at 12:46 AM, Maxime Ripard
<maxime.ripard@free-electrons.com> wrote:
> All the controllers can have a maximum frequency of 200MHz.
>
> Since older SoCs cannot go that high, we cannot change the default maximum
> frequency, but fortunately for us we have a property for that in the DT.
>
> This also has the side effect of allowing to use the MMC HS200 mode for the
> boards that support it (with either 1.2v or 1.8v IOs).
>
> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> ---
>  arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi | 3 +++
>  1 file changed, 3 insertions(+), 0 deletions(-)
>
> diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
> index 8e149498e096..f46ae965cf5b 100644
> --- a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
> +++ b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
> @@ -332,6 +332,7 @@
>                         resets = <&ccu RST_BUS_MMC0>;
>                         reset-names = "ahb";
>                         interrupts = <GIC_SPI 60 IRQ_TYPE_LEVEL_HIGH>;
> +                       max-frequency = <200000000>;

You also have to set one of MMC_CAP2_HS200* in the driver,
or mmc-hs200-1_8v or mmc-hs200-1_2v in the device tree to
actually use HS200, right?

ChenYu

>                         status = "disabled";
>                         #address-cells = <1>;
>                         #size-cells = <0>;
> @@ -345,6 +346,7 @@
>                         resets = <&ccu RST_BUS_MMC1>;
>                         reset-names = "ahb";
>                         interrupts = <GIC_SPI 61 IRQ_TYPE_LEVEL_HIGH>;
> +                       max-frequency = <200000000>;
>                         status = "disabled";
>                         #address-cells = <1>;
>                         #size-cells = <0>;
> @@ -358,6 +360,7 @@
>                         resets = <&ccu RST_BUS_MMC2>;
>                         reset-names = "ahb";
>                         interrupts = <GIC_SPI 62 IRQ_TYPE_LEVEL_HIGH>;
> +                       max-frequency = <200000000>;
>                         status = "disabled";
>                         #address-cells = <1>;
>                         #size-cells = <0>;
> --
> git-series 0.8.11

Re: [PATCH v2 1/6] mmc: sunxi: Always set signal delay to 0 for A64

[André Przywara]

On 09/01/17 16:46, Maxime Ripard wrote:
> Experience have shown that the using the  autocalibration could severely
> degrade the performances of the MMC bus.
> 
> Allwinner is using in its BSP a delay set to 0 for all the modes but HS400.
> Remove the calibration code for now, and add comments to document our
> findings.
> 
> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> ---
>  drivers/mmc/host/sunxi-mmc.c | 50 ++++++++++++-------------------------
>  1 file changed, 17 insertions(+), 33 deletions(-)
> 
> diff --git a/drivers/mmc/host/sunxi-mmc.c b/drivers/mmc/host/sunxi-mmc.c
> index b1d1303389a7..ea9552a0d820 100644
> --- a/drivers/mmc/host/sunxi-mmc.c
> +++ b/drivers/mmc/host/sunxi-mmc.c
> @@ -683,41 +683,19 @@ static int sunxi_mmc_oclk_onoff(struct sunxi_mmc_host *host, u32 oclk_en)
>  
>  static int sunxi_mmc_calibrate(struct sunxi_mmc_host *host, int reg_off)
>  {
> -	u32 reg = readl(host->reg_base + reg_off);
> -	u32 delay;
> -	unsigned long timeout;
> -
>  	if (!host->cfg->can_calibrate)
>  		return 0;
>  
> -	reg &= ~(SDXC_CAL_DL_MASK << SDXC_CAL_DL_SW_SHIFT);
> -	reg &= ~SDXC_CAL_DL_SW_EN;
> -
> -	writel(reg | SDXC_CAL_START, host->reg_base + reg_off);
> -
> -	dev_dbg(mmc_dev(host->mmc), "calibration started\n");
> -
> -	timeout = jiffies + HZ * SDXC_CAL_TIMEOUT;
> -
> -	while (!((reg = readl(host->reg_base + reg_off)) & SDXC_CAL_DONE)) {
> -		if (time_before(jiffies, timeout))
> -			cpu_relax();
> -		else {
> -			reg &= ~SDXC_CAL_START;
> -			writel(reg, host->reg_base + reg_off);
> -
> -			return -ETIMEDOUT;
> -		}
> -	}
> -
> -	delay = (reg >> SDXC_CAL_DL_SHIFT) & SDXC_CAL_DL_MASK;
> -
> -	reg &= ~SDXC_CAL_START;
> -	reg |= (delay << SDXC_CAL_DL_SW_SHIFT) | SDXC_CAL_DL_SW_EN;
> -
> -	writel(reg, host->reg_base + reg_off);
> -
> -	dev_dbg(mmc_dev(host->mmc), "calibration ended, reg is 0x%x\n", reg);
> +	/*
> +	 * FIXME:
> +	 * This is not clear how the calibration is supposed to work
> +	 * yet. The best rate have been obtained by simply setting the
> +	 * delay to 0, as Allwinner does in its BSP.
> +	 *
> +	 * The only mode that doesn't have such a delay is HS400, that
> +	 * is in itself a TODO.
> +	 */
> +	writel(SDXC_CAL_DL_SW_EN, host->reg_base + reg_off);
>  
>  	return 0;
>  }
> @@ -806,7 +784,13 @@ static int sunxi_mmc_clk_set_rate(struct sunxi_mmc_host *host,
>  	if (ret)
>  		return ret;
>  
> -	/* TODO: enable calibrate on sdc2 SDXC_REG_DS_DL_REG of A64 */
> +	/*
> +	 * FIXME:
> +	 *
> +	 * In HS400 we'll also need to calibrate the data strobe
> +	 * signal. This should only happen on the MMC2 controller (at
> +	 * least on the A64 and older SoCs).

Which older SoCs have this calibration register and a DS signal?
Is that supposed to mean "other" SoCs?

Other than that:

Reviewed-by: Andre Przywara <andre.przywara@arm.com>

Cheers,
Andre.

Re: [PATCH v2 5/6] arm64: allwinner: a64: Add MMC pinctrl nodes

[André Przywara]

On 09/01/17 16:46, Maxime Ripard wrote:
> The A64 only has a single set of pins for each MMC controller. Since we
> already have boards that require all of them, let's add them to the DTSI.

This matches my reworked version from the previous series, so:

Reviewed-by: Andre Przywara <andre.przywara@arm.com>

Cheers,
Andre.

> 
> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> ---
>  arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi | 25 ++++++++++++++++++++-
>  1 file changed, 25 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
> index 143e9706438f..8e149498e096 100644
> --- a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
> +++ b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
> @@ -205,6 +205,31 @@
>  				function = "i2c1";
>  			};
>  
> +			mmc0_pins: mmc0-pins {
> +				pins = "PF0", "PF1", "PF2", "PF3",
> +				       "PF4", "PF5";
> +				function = "mmc0";
> +				drive-strength = <30>;
> +				bias-pull-up;
> +			};
> +
> +			mmc1_pins: mmc1-pins {
> +				pins = "PG0", "PG1", "PG2", "PG3",
> +				       "PG4", "PG5";
> +				function = "mmc1";
> +				drive-strength = <30>;
> +				bias-pull-up;
> +			};
> +
> +			mmc2_pins: mmc2-pins {
> +				pins = "PC1", "PC5", "PC6", "PC8", "PC9",
> +				       "PC10","PC11", "PC12", "PC13",
> +				       "PC14", "PC15", "PC16";
> +				function = "mmc2";
> +				drive-strength = <30>;
> +				bias-pull-up;
> +			};
> +
>  			uart0_pins_a: uart0@0 {
>  				pins = "PB8", "PB9";
>  				function = "uart0";
> 

Re: [PATCH v2 6/6] arm64: allwinner: a64: Increase the MMC max frequency

[Maxime Ripard]

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

Hi,

On Tue, Jan 10, 2017 at 01:01:20AM +0800, Chen-Yu Tsai wrote:
> On Tue, Jan 10, 2017 at 12:46 AM, Maxime Ripard
> <maxime.ripard@free-electrons.com> wrote:
> > All the controllers can have a maximum frequency of 200MHz.
> >
> > Since older SoCs cannot go that high, we cannot change the default maximum
> > frequency, but fortunately for us we have a property for that in the DT.
> >
> > This also has the side effect of allowing to use the MMC HS200 mode for the
> > boards that support it (with either 1.2v or 1.8v IOs).
> >
> > Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> > ---
> >  arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi | 3 +++
> >  1 file changed, 3 insertions(+), 0 deletions(-)
> >
> > diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
> > index 8e149498e096..f46ae965cf5b 100644
> > --- a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
> > +++ b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
> > @@ -332,6 +332,7 @@
> >                         resets = <&ccu RST_BUS_MMC0>;
> >                         reset-names = "ahb";
> >                         interrupts = <GIC_SPI 60 IRQ_TYPE_LEVEL_HIGH>;
> > +                       max-frequency = <200000000>;
> 
> You also have to set one of MMC_CAP2_HS200* in the driver,
> or mmc-hs200-1_8v or mmc-hs200-1_2v in the device tree to
> actually use HS200, right?

Yes, but that requires a board with 1.8V IOs to work properly, which
not all board use, so it's probably best to enable it in the board
DTS.

Thanks!
Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

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

Re: [PATCH v2 6/6] arm64: allwinner: a64: Increase the MMC max frequency

[Chen-Yu Tsai]

On Wed, Jan 11, 2017 at 3:15 AM, Maxime Ripard
<maxime.ripard@free-electrons.com> wrote:
> Hi,
>
> On Tue, Jan 10, 2017 at 01:01:20AM +0800, Chen-Yu Tsai wrote:
>> On Tue, Jan 10, 2017 at 12:46 AM, Maxime Ripard
>> <maxime.ripard@free-electrons.com> wrote:
>> > All the controllers can have a maximum frequency of 200MHz.
>> >
>> > Since older SoCs cannot go that high, we cannot change the default maximum
>> > frequency, but fortunately for us we have a property for that in the DT.
>> >
>> > This also has the side effect of allowing to use the MMC HS200 mode for the
>> > boards that support it (with either 1.2v or 1.8v IOs).
>> >
>> > Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
>> > ---
>> >  arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi | 3 +++
>> >  1 file changed, 3 insertions(+), 0 deletions(-)
>> >
>> > diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
>> > index 8e149498e096..f46ae965cf5b 100644
>> > --- a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
>> > +++ b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
>> > @@ -332,6 +332,7 @@
>> >                         resets = <&ccu RST_BUS_MMC0>;
>> >                         reset-names = "ahb";
>> >                         interrupts = <GIC_SPI 60 IRQ_TYPE_LEVEL_HIGH>;
>> > +                       max-frequency = <200000000>;
>>
>> You also have to set one of MMC_CAP2_HS200* in the driver,
>> or mmc-hs200-1_8v or mmc-hs200-1_2v in the device tree to
>> actually use HS200, right?
>
> Yes, but that requires a board with 1.8V IOs to work properly, which
> not all board use, so it's probably best to enable it in the board
> DTS.

It's limited by the vqmmc regulator. Either way the host controller
supports it right?

ChenYu

Re: [PATCH v2 6/6] arm64: allwinner: a64: Increase the MMC max frequency

[Maxime Ripard]

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

Hi,

On Wed, Jan 11, 2017 at 02:44:00PM +0800, Chen-Yu Tsai wrote:
> On Wed, Jan 11, 2017 at 3:15 AM, Maxime Ripard
> <maxime.ripard@free-electrons.com> wrote:
> > Hi,
> >
> > On Tue, Jan 10, 2017 at 01:01:20AM +0800, Chen-Yu Tsai wrote:
> >> On Tue, Jan 10, 2017 at 12:46 AM, Maxime Ripard
> >> <maxime.ripard@free-electrons.com> wrote:
> >> > All the controllers can have a maximum frequency of 200MHz.
> >> >
> >> > Since older SoCs cannot go that high, we cannot change the default maximum
> >> > frequency, but fortunately for us we have a property for that in the DT.
> >> >
> >> > This also has the side effect of allowing to use the MMC HS200 mode for the
> >> > boards that support it (with either 1.2v or 1.8v IOs).
> >> >
> >> > Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> >> > ---
> >> >  arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi | 3 +++
> >> >  1 file changed, 3 insertions(+), 0 deletions(-)
> >> >
> >> > diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
> >> > index 8e149498e096..f46ae965cf5b 100644
> >> > --- a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
> >> > +++ b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
> >> > @@ -332,6 +332,7 @@
> >> >                         resets = <&ccu RST_BUS_MMC0>;
> >> >                         reset-names = "ahb";
> >> >                         interrupts = <GIC_SPI 60 IRQ_TYPE_LEVEL_HIGH>;
> >> > +                       max-frequency = <200000000>;
> >>
> >> You also have to set one of MMC_CAP2_HS200* in the driver,
> >> or mmc-hs200-1_8v or mmc-hs200-1_2v in the device tree to
> >> actually use HS200, right?
> >
> > Yes, but that requires a board with 1.8V IOs to work properly, which
> > not all board use, so it's probably best to enable it in the board
> > DTS.
> 
> It's limited by the vqmmc regulator. Either way the host controller
> supports it right?

Yes, but if the card supports HS200 with 1.8V, and the controller
reports it too, the core will pick that mode and will try to switch to
it, which in turn will fail, making the card initialisation fail as
well.

We basically have two choices: either we ask all the boards that
support it to set mmc-hs200-1_8v in their DTS, or we set it in the
DTSI and we have the boards set no-1-8-v in their DTS if they do not
support those modes.

The first case is just the more convenient, because so far we've only
seen one non-upstream (yet) board that supports 1.8V IOs. All the
other do not.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

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

Re: [PATCH v2 1/6] mmc: sunxi: Always set signal delay to 0 for A64

[Maxime Ripard]

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

On Tue, Jan 10, 2017 at 12:30:41AM +0000, André Przywara wrote:
> On 09/01/17 16:46, Maxime Ripard wrote:
> > Experience have shown that the using the  autocalibration could severely
> > degrade the performances of the MMC bus.
> > 
> > Allwinner is using in its BSP a delay set to 0 for all the modes but HS400.
> > Remove the calibration code for now, and add comments to document our
> > findings.
> > 
> > Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> > ---
> >  drivers/mmc/host/sunxi-mmc.c | 50 ++++++++++++-------------------------
> >  1 file changed, 17 insertions(+), 33 deletions(-)
> > 
> > diff --git a/drivers/mmc/host/sunxi-mmc.c b/drivers/mmc/host/sunxi-mmc.c
> > index b1d1303389a7..ea9552a0d820 100644
> > --- a/drivers/mmc/host/sunxi-mmc.c
> > +++ b/drivers/mmc/host/sunxi-mmc.c
> > @@ -683,41 +683,19 @@ static int sunxi_mmc_oclk_onoff(struct sunxi_mmc_host *host, u32 oclk_en)
> >  
> >  static int sunxi_mmc_calibrate(struct sunxi_mmc_host *host, int reg_off)
> >  {
> > -	u32 reg = readl(host->reg_base + reg_off);
> > -	u32 delay;
> > -	unsigned long timeout;
> > -
> >  	if (!host->cfg->can_calibrate)
> >  		return 0;
> >  
> > -	reg &= ~(SDXC_CAL_DL_MASK << SDXC_CAL_DL_SW_SHIFT);
> > -	reg &= ~SDXC_CAL_DL_SW_EN;
> > -
> > -	writel(reg | SDXC_CAL_START, host->reg_base + reg_off);
> > -
> > -	dev_dbg(mmc_dev(host->mmc), "calibration started\n");
> > -
> > -	timeout = jiffies + HZ * SDXC_CAL_TIMEOUT;
> > -
> > -	while (!((reg = readl(host->reg_base + reg_off)) & SDXC_CAL_DONE)) {
> > -		if (time_before(jiffies, timeout))
> > -			cpu_relax();
> > -		else {
> > -			reg &= ~SDXC_CAL_START;
> > -			writel(reg, host->reg_base + reg_off);
> > -
> > -			return -ETIMEDOUT;
> > -		}
> > -	}
> > -
> > -	delay = (reg >> SDXC_CAL_DL_SHIFT) & SDXC_CAL_DL_MASK;
> > -
> > -	reg &= ~SDXC_CAL_START;
> > -	reg |= (delay << SDXC_CAL_DL_SW_SHIFT) | SDXC_CAL_DL_SW_EN;
> > -
> > -	writel(reg, host->reg_base + reg_off);
> > -
> > -	dev_dbg(mmc_dev(host->mmc), "calibration ended, reg is 0x%x\n", reg);
> > +	/*
> > +	 * FIXME:
> > +	 * This is not clear how the calibration is supposed to work
> > +	 * yet. The best rate have been obtained by simply setting the
> > +	 * delay to 0, as Allwinner does in its BSP.
> > +	 *
> > +	 * The only mode that doesn't have such a delay is HS400, that
> > +	 * is in itself a TODO.
> > +	 */
> > +	writel(SDXC_CAL_DL_SW_EN, host->reg_base + reg_off);
> >  
> >  	return 0;
> >  }
> > @@ -806,7 +784,13 @@ static int sunxi_mmc_clk_set_rate(struct sunxi_mmc_host *host,
> >  	if (ret)
> >  		return ret;
> >  
> > -	/* TODO: enable calibrate on sdc2 SDXC_REG_DS_DL_REG of A64 */
> > +	/*
> > +	 * FIXME:
> > +	 *
> > +	 * In HS400 we'll also need to calibrate the data strobe
> > +	 * signal. This should only happen on the MMC2 controller (at
> > +	 * least on the A64 and older SoCs).
> 
> Which older SoCs have this calibration register and a DS signal?
> Is that supposed to mean "other" SoCs?

That was supposed to mean that newer (than A64) SoCs might have that
calibration on other controllers than MMC2. But you're right that it
actually applies only to A64 anyway, I'll remove the and older part.

> Other than that:
> 
> Reviewed-by: Andre Przywara <andre.przywara@arm.com>

Thanks!
Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

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