[U-Boot] [PATCH 0/4] rockchip: allow rk3188 to boot from mmc devices

[U-Boot] [PATCH 0/4] rockchip: allow rk3188 to boot from mmc devices

[Heiko Stuebner]

With this series I can now sucessfully boot from a sd-card, with
even big kernels being read sucessfully.

Patch 1 is not strictly necessary for that but was a result of me
investigating the mmc hang which even hung the whole uboot.

Heiko Stuebner (4):
  mmc: dw_mmc: check fifo status with a timeout in fifo mode
  rockchip: dwmmc: add rk2928-dw-mshc compatible
  rockchip: rk3188: add u-boot-specific mmc properties
  rockchip: rk3188: explicitly set vcc_sd0 pin to gpio on
    rk3188-radxarock

 arch/arm/dts/rk3188-radxarock-u-boot.dtsi | 15 +++++++++++
 arch/arm/dts/rk3188-radxarock.dts         |  8 ++++++
 drivers/mmc/dw_mmc.c                      | 32 +++++++++++++++++++++--
 drivers/mmc/rockchip_dw_mmc.c             |  1 +
 include/dwmmc.h                           |  2 ++
 5 files changed, 56 insertions(+), 2 deletions(-)

-- 
2.18.0

[U-Boot] [PATCH 1/4] mmc: dw_mmc: check fifo status with a timeout in fifo mode

[Heiko Stuebner]

While trying to enable the dw_mmc on rk3188 I managed to confuse
and hang the dw_mmc controller into not delivering further data.
The fifo state never became ready and the driver was iterating in
the while loop reading 0-byte packets forever.

So inspired by how other implementations handle this, check the fifo-
state beforhand and add a timeout to catch any glaring fifo issues
without hanging uboot altogether.

Signed-off-by: Heiko Stuebner <heiko@sntech.de>
---
 drivers/mmc/dw_mmc.c | 32 ++++++++++++++++++++++++++++++--
 include/dwmmc.h      |  2 ++
 2 files changed, 32 insertions(+), 2 deletions(-)

diff --git a/drivers/mmc/dw_mmc.c b/drivers/mmc/dw_mmc.c
index 13180fc0d6..3c702b3ed8 100644
--- a/drivers/mmc/dw_mmc.c
+++ b/drivers/mmc/dw_mmc.c
@@ -92,6 +92,24 @@ static void dwmci_prepare_data(struct dwmci_host *host,
 	dwmci_writel(host, DWMCI_BYTCNT, data->blocksize * data->blocks);
 }
 
+static int dwmci_fifo_ready(struct dwmci_host *host, u32 bit, u32 *len)
+{
+	u32 timeout = 20000;
+
+	*len = dwmci_readl(host, DWMCI_STATUS);
+	while (--timeout && (*len & bit)) {
+		udelay(200);
+		*len = dwmci_readl(host, DWMCI_STATUS);
+	}
+
+	if (!timeout) {
+		debug("%s: FIFO underflow timeout\n", __func__);
+		return -ETIMEDOUT;
+	}
+
+	return 0;
+}
+
 static int dwmci_data_transfer(struct dwmci_host *host, struct mmc_data *data)
 {
 	int ret = 0;
@@ -122,7 +140,12 @@ static int dwmci_data_transfer(struct dwmci_host *host, struct mmc_data *data)
 			if (data->flags == MMC_DATA_READ &&
 			    (mask & DWMCI_INTMSK_RXDR)) {
 				while (size) {
-					len = dwmci_readl(host, DWMCI_STATUS);
+					ret = dwmci_fifo_ready(host,
+							DWMCI_FIFO_EMPTY,
+							&len);
+					if (ret < 0)
+						break;
+
 					len = (len >> DWMCI_FIFO_SHIFT) &
 						    DWMCI_FIFO_MASK;
 					len = min(size, len);
@@ -136,7 +159,12 @@ static int dwmci_data_transfer(struct dwmci_host *host, struct mmc_data *data)
 			} else if (data->flags == MMC_DATA_WRITE &&
 				   (mask & DWMCI_INTMSK_TXDR)) {
 				while (size) {
-					len = dwmci_readl(host, DWMCI_STATUS);
+					ret = dwmci_fifo_ready(host,
+							DWMCI_FIFO_FULL,
+							&len);
+					if (ret < 0)
+						break;
+
 					len = fifo_depth - ((len >>
 						   DWMCI_FIFO_SHIFT) &
 						   DWMCI_FIFO_MASK);
diff --git a/include/dwmmc.h b/include/dwmmc.h
index bc1d6e3abb..0f9d51b557 100644
--- a/include/dwmmc.h
+++ b/include/dwmmc.h
@@ -103,6 +103,8 @@
 #define DWMCI_CTYPE_8BIT	(1 << 16)
 
 /* Status Register */
+#define DWMCI_FIFO_EMPTY	(1 << 2)
+#define DWMCI_FIFO_FULL		(1 << 3)
 #define DWMCI_BUSY		(1 << 9)
 #define DWMCI_FIFO_MASK		0x1fff
 #define DWMCI_FIFO_SHIFT	17
-- 
2.18.0

[U-Boot] [PATCH 2/4] rockchip: dwmmc: add rk2928-dw-mshc compatible

[Heiko Stuebner]

The rk3188 works nicely with the rockchip mmc driver, so we just need
to add the different compatible for it - as used in the Linux kernel.

Signed-off-by: Heiko Stuebner <heiko@sntech.de>
---
 drivers/mmc/rockchip_dw_mmc.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/mmc/rockchip_dw_mmc.c b/drivers/mmc/rockchip_dw_mmc.c
index f54d95a881..bf2d83a52c 100644
--- a/drivers/mmc/rockchip_dw_mmc.c
+++ b/drivers/mmc/rockchip_dw_mmc.c
@@ -156,6 +156,7 @@ static int rockchip_dwmmc_bind(struct udevice *dev)
 }
 
 static const struct udevice_id rockchip_dwmmc_ids[] = {
+	{ .compatible = "rockchip,rk2928-dw-mshc" },
 	{ .compatible = "rockchip,rk3288-dw-mshc" },
 	{ }
 };
-- 
2.18.0

[U-Boot] [PATCH 3/4] rockchip: rk3188: add u-boot-specific mmc properties

[Heiko Stuebner]

The dwmmc controllers on rk3188 do not have idma support, so need to
use the fifo-mode and it my tests they became confused and stopped
working if the frequency was to high.

While I only tested in somewhat bigger steps, 32MHz for example
hung the controller, while reducing it to 16MHz worked just fine
and is reasonably fast to load a kernel from mmc.

Signed-off-by: Heiko Stuebner <heiko@sntech.de>
---
 arch/arm/dts/rk3188-radxarock-u-boot.dtsi | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/arch/arm/dts/rk3188-radxarock-u-boot.dtsi b/arch/arm/dts/rk3188-radxarock-u-boot.dtsi
index 013535abcd..1bb5408592 100644
--- a/arch/arm/dts/rk3188-radxarock-u-boot.dtsi
+++ b/arch/arm/dts/rk3188-radxarock-u-boot.dtsi
@@ -11,6 +11,21 @@
 	u-boot,dm-spl;
 };
 
+&mmc0 {
+	fifo-mode;
+	max-frequency = <16000000>;
+};
+
+&mmc1 {
+	fifo-mode;
+	max-frequency = <16000000>;
+};
+
+&emmc {
+	fifo-mode;
+	max-frequency = <16000000>;
+};
+
 &uart2 {
 	status = "okay";
 	u-boot,dm-spl;
-- 
2.18.0

[U-Boot] [PATCH 4/4] rockchip: rk3188: explicitly set vcc_sd0 pin to gpio on rk3188-radxarock

[Heiko Stuebner]

It is good practice to make the setting of gpio-pinctrls explicitly in the
devicetree, and in this case even necessary.
Rockchip boards start with iomux settings set to gpio for most pins and
while the linux pinctrl driver also implicitly sets the gpio function if
a pin is requested as gpio that is not necessarily true for other drivers.

The issue in question stems from uboot, where the sdmmc_pwr pin is set
to function 1 (sdmmc-power) by the bootrom when reading the 1st-stage
loader. The regulator controlled by the pin is active-low though, so
when the dwmmc hw-block sets its enabled bit, it actually disables the
regulator. By changing the pin back to gpio we fix that behaviour.

[picked from the identical linux patch
https://patchwork.kernel.org/patch/10609253/]
Signed-off-by: Heiko Stuebner <heiko@sntech.de>
---
 arch/arm/dts/rk3188-radxarock.dts | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/arch/arm/dts/rk3188-radxarock.dts b/arch/arm/dts/rk3188-radxarock.dts
index ac931e14af..61367126ba 100644
--- a/arch/arm/dts/rk3188-radxarock.dts
+++ b/arch/arm/dts/rk3188-radxarock.dts
@@ -104,6 +104,8 @@
 		regulator-min-microvolt = <3300000>;
 		regulator-max-microvolt = <3300000>;
 		gpio = <&gpio3 1 GPIO_ACTIVE_LOW>;
+		pinctrl-names = "default";
+		pinctrl-0 = <&sdmmc_pwr>;
 		startup-delay-us = <100000>;
 		vin-supply = <&vcc_io>;
 	};
@@ -334,6 +336,12 @@
 		};
 	};
 
+	sd0 {
+		sdmmc_pwr: sdmmc-pwr {
+			rockchip,pins = <RK_GPIO3 1 RK_FUNC_GPIO &pcfg_pull_none>;
+		};
+	};
+
 	usb {
 		host_vbus_drv: host-vbus-drv {
 			rockchip,pins = <0 3 RK_FUNC_GPIO &pcfg_pull_none>;
-- 
2.18.0

[U-Boot] [PATCH 4/4] rockchip: rk3188: explicitly set vcc_sd0 pin to gpio on rk3188-radxarock

[Philipp Tomsich]

> On 21.09.2018, at 10:59, Heiko Stuebner <heiko@sntech.de> wrote:
> 
> It is good practice to make the setting of gpio-pinctrls explicitly in the
> devicetree, and in this case even necessary.
> Rockchip boards start with iomux settings set to gpio for most pins and
> while the linux pinctrl driver also implicitly sets the gpio function if
> a pin is requested as gpio that is not necessarily true for other drivers.
> 
> The issue in question stems from uboot, where the sdmmc_pwr pin is set
> to function 1 (sdmmc-power) by the bootrom when reading the 1st-stage
> loader. The regulator controlled by the pin is active-low though, so
> when the dwmmc hw-block sets its enabled bit, it actually disables the
> regulator. By changing the pin back to gpio we fix that behaviour.
> 
> [picked from the identical linux patch
> https://patchwork.kernel.org/patch/10609253/]
> Signed-off-by: Heiko Stuebner <heiko@sntech.de>

Reviewed-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
Acked-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>

[U-Boot] [PATCH 3/4] rockchip: rk3188: add u-boot-specific mmc properties

[Philipp Tomsich]

On 21.09.2018, at 10:59, Heiko Stuebner <heiko@sntech.de> wrote:
> 
> The dwmmc controllers on rk3188 do not have idma support, so need to
> use the fifo-mode and it my tests they became confused and stopped
> working if the frequency was to high.
> 
> While I only tested in somewhat bigger steps, 32MHz for example
> hung the controller, while reducing it to 16MHz worked just fine
> and is reasonably fast to load a kernel from mmc.
> 
> Signed-off-by: Heiko Stuebner <heiko@sntech.de>

Reviewed-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
Acked-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>

[U-Boot] [PATCH 2/4] rockchip: dwmmc: add rk2928-dw-mshc compatible

[Philipp Tomsich]

> On 21.09.2018, at 10:59, Heiko Stuebner <heiko@sntech.de> wrote:
> 
> The rk3188 works nicely with the rockchip mmc driver, so we just need
> to add the different compatible for it - as used in the Linux kernel.
> 
> Signed-off-by: Heiko Stuebner <heiko@sntech.de>

Reviewed-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
Acked-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>

[U-Boot] [PATCH 1/4] mmc: dw_mmc: check fifo status with a timeout in fifo mode

[Philipp Tomsich]

> On 21.09.2018, at 10:59, Heiko Stuebner <heiko@sntech.de> wrote:
> 
> While trying to enable the dw_mmc on rk3188 I managed to confuse
> and hang the dw_mmc controller into not delivering further data.
> The fifo state never became ready and the driver was iterating in
> the while loop reading 0-byte packets forever.
> 
> So inspired by how other implementations handle this, check the fifo-
> state beforhand and add a timeout to catch any glaring fifo issues
> without hanging uboot altogether.
> 
> Signed-off-by: Heiko Stuebner <heiko@sntech.de>

Reviewed-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
Acked-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>

[U-Boot] [U-Boot, 1/4] mmc: dw_mmc: check fifo status with a timeout in fifo mode

[Philipp Tomsich]

> While trying to enable the dw_mmc on rk3188 I managed to confuse
> and hang the dw_mmc controller into not delivering further data.
> The fifo state never became ready and the driver was iterating in
> the while loop reading 0-byte packets forever.
> 
> So inspired by how other implementations handle this, check the fifo-
> state beforhand and add a timeout to catch any glaring fifo issues
> without hanging uboot altogether.
> 
> Signed-off-by: Heiko Stuebner <heiko@sntech.de>
> Reviewed-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
> Acked-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
> ---
>  drivers/mmc/dw_mmc.c | 32 ++++++++++++++++++++++++++++++--
>  include/dwmmc.h      |  2 ++
>  2 files changed, 32 insertions(+), 2 deletions(-)
> 

Applied to u-boot-rockchip, thanks!

[U-Boot] [U-Boot, 2/4] rockchip: dwmmc: add rk2928-dw-mshc compatible

[Philipp Tomsich]

> The rk3188 works nicely with the rockchip mmc driver, so we just need
> to add the different compatible for it - as used in the Linux kernel.
> 
> Signed-off-by: Heiko Stuebner <heiko@sntech.de>
> Reviewed-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
> Acked-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
> ---
>  drivers/mmc/rockchip_dw_mmc.c | 1 +
>  1 file changed, 1 insertion(+)
> 

Applied to u-boot-rockchip, thanks!

[U-Boot] [U-Boot, 4/4] rockchip: rk3188: explicitly set vcc_sd0 pin to gpio on rk3188-radxarock

[Philipp Tomsich]

> It is good practice to make the setting of gpio-pinctrls explicitly in the
> devicetree, and in this case even necessary.
> Rockchip boards start with iomux settings set to gpio for most pins and
> while the linux pinctrl driver also implicitly sets the gpio function if
> a pin is requested as gpio that is not necessarily true for other drivers.
> 
> The issue in question stems from uboot, where the sdmmc_pwr pin is set
> to function 1 (sdmmc-power) by the bootrom when reading the 1st-stage
> loader. The regulator controlled by the pin is active-low though, so
> when the dwmmc hw-block sets its enabled bit, it actually disables the
> regulator. By changing the pin back to gpio we fix that behaviour.
> 
> [picked from the identical linux patch
> https://patchwork.kernel.org/patch/10609253/]
> Signed-off-by: Heiko Stuebner <heiko@sntech.de>
> Reviewed-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
> Acked-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
> ---
>  arch/arm/dts/rk3188-radxarock.dts | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 

Applied to u-boot-rockchip, thanks!

[U-Boot] [U-Boot, 3/4] rockchip: rk3188: add u-boot-specific mmc properties

[Philipp Tomsich]

> The dwmmc controllers on rk3188 do not have idma support, so need to
> use the fifo-mode and it my tests they became confused and stopped
> working if the frequency was to high.
> 
> While I only tested in somewhat bigger steps, 32MHz for example
> hung the controller, while reducing it to 16MHz worked just fine
> and is reasonably fast to load a kernel from mmc.
> 
> Signed-off-by: Heiko Stuebner <heiko@sntech.de>
> Reviewed-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
> Acked-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
> ---
>  arch/arm/dts/rk3188-radxarock-u-boot.dtsi | 15 +++++++++++++++
>  1 file changed, 15 insertions(+)
> 

Applied to u-boot-rockchip, thanks!