[U-Boot] [PATCH 0/4] rockchip: allow rk3188 to boot from mmc devices
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(-)
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
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
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!
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" }, { } };
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
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!
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;
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
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!
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>;
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
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!
participants (2)
-
Heiko Stuebner -
Philipp Tomsich