[U-Boot] [PATCH v2 0/3] rk3288: veyron: Enable SDMMC when booting from SPI

From: Carlo Caione carlo@endlessm.com
These patches toghether with the previously submitted patch [0] enable the chromebook veyron jerry to use the SDMMC interface when U-Boot is not chainloaded by depthcharge but booted directly from SPI.
[0] https://marc.info/?l=u-boot&m=152836928803742&w=2
Changelog:
V2: - Add Reviewed-by - Expand comment on PATCH 2/3
Carlo Caione (3): rk3288: veyron: Init boot-on regulators rk3288: Disable JTAG function from sdmmc0 IO rockchip: veyron: Set vcc33_sd regulator value
arch/arm/mach-rockchip/rk3288-board.c | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+)

From: Carlo Caione carlo@endlessm.com
Use regulators_enable_boot_on() to init all the regulators with regulator-boot-on property.
Signed-off-by: Carlo Caione carlo@endlessm.com Reviewed-by: Simon Glass sjg@chromium.org --- arch/arm/mach-rockchip/rk3288-board.c | 6 ++++++ 1 file changed, 6 insertions(+)
diff --git a/arch/arm/mach-rockchip/rk3288-board.c b/arch/arm/mach-rockchip/rk3288-board.c index 8c128d4f94..0365793009 100644 --- a/arch/arm/mach-rockchip/rk3288-board.c +++ b/arch/arm/mach-rockchip/rk3288-board.c @@ -122,6 +122,12 @@ static int veyron_init(void) if (IS_ERR_VALUE(ret)) return ret;
+ ret = regulators_enable_boot_on(false); + if (ret) { + debug("%s: Cannot enable boot on regulators\n", __func__); + return ret; + } + return 0; } #endif

On 11 Jun 2018, at 10:08, Carlo Caione carlo@caione.org wrote:
From: Carlo Caione carlo@endlessm.com
Use regulators_enable_boot_on() to init all the regulators with regulator-boot-on property.
Signed-off-by: Carlo Caione carlo@endlessm.com Reviewed-by: Simon Glass sjg@chromium.org
Reviewed-by: Philipp Tomsich philipp.tomsich@theobroma-systems.com Acked-by: Philipp Tomsich philipp.tomsich@theobroma-systems.com

From: Carlo Caione carlo@endlessm.com
The GRF_SOC_CON0.grf_force_jtag bit is automatically set at boot and it is preventing the SDMMC to work correctly. Disable the JTAG function on the assumption that a working SD has higher priority over JTAG.
Signed-off-by: Carlo Caione carlo@endlessm.com Reviewed-by: Simon Glass sjg@chromium.org --- arch/arm/mach-rockchip/rk3288-board.c | 7 +++++++ 1 file changed, 7 insertions(+)
diff --git a/arch/arm/mach-rockchip/rk3288-board.c b/arch/arm/mach-rockchip/rk3288-board.c index 0365793009..bf24d8e074 100644 --- a/arch/arm/mach-rockchip/rk3288-board.c +++ b/arch/arm/mach-rockchip/rk3288-board.c @@ -307,6 +307,7 @@ U_BOOT_CMD( "" );
+#define GRF_SOC_CON0 0xff770244 #define GRF_SOC_CON2 0xff77024c
int board_early_init_f(void) @@ -339,5 +340,11 @@ int board_early_init_f(void) } rk_setreg(GRF_SOC_CON2, 1 << 0);
+ /* + * Disable JTAG on sdmmc0 IO. The SDMMC won't work until this bit is + * cleared + */ + rk_clrreg(GRF_SOC_CON0, 1 << 12); + return 0; }

On 11 Jun 2018, at 10:08, Carlo Caione carlo@caione.org wrote:
From: Carlo Caione carlo@endlessm.com
The GRF_SOC_CON0.grf_force_jtag bit is automatically set at boot and it is preventing the SDMMC to work correctly. Disable the JTAG function on the assumption that a working SD has higher priority over JTAG.
Signed-off-by: Carlo Caione carlo@endlessm.com Reviewed-by: Simon Glass sjg@chromium.org
Reviewed-by: Philipp Tomsich philipp.tomsich@theobroma-systems.com Acked-by: Philipp Tomsich philipp.tomsich@theobroma-systems.com
See below for a nitpick ...
arch/arm/mach-rockchip/rk3288-board.c | 7 +++++++ 1 file changed, 7 insertions(+)
diff --git a/arch/arm/mach-rockchip/rk3288-board.c b/arch/arm/mach-rockchip/rk3288-board.c index 0365793009..bf24d8e074 100644 --- a/arch/arm/mach-rockchip/rk3288-board.c +++ b/arch/arm/mach-rockchip/rk3288-board.c @@ -307,6 +307,7 @@ U_BOOT_CMD( "" );
+#define GRF_SOC_CON0 0xff770244 #define GRF_SOC_CON2 0xff77024c
Could you convert these to ‘const uintptr_t GRF_SOC_CON0 = …’ ? The compiler will generate the same code for a const as if it’s a define, but we’ll have full type-safety.
int board_early_init_f(void) @@ -339,5 +340,11 @@ int board_early_init_f(void) } rk_setreg(GRF_SOC_CON2, 1 << 0);
- /*
* Disable JTAG on sdmmc0 IO. The SDMMC won't work until this bit is
* cleared
*/
- rk_clrreg(GRF_SOC_CON0, 1 << 12);
- return 0;
}
2.17.1

On Mon, 2018-06-11 at 10:41 +0200, Dr. Philipp Tomsich wrote:
On 11 Jun 2018, at 10:08, Carlo Caione carlo@caione.org wrote:
/cut
+#define GRF_SOC_CON0 0xff770244 #define GRF_SOC_CON2 0xff77024c
Could you convert these to ‘const uintptr_t GRF_SOC_CON0 = …’ ? The compiler will generate the same code for a const as if it’s a define, but we’ll have full type-safety.
Yeah, no problem. But if this is ok with you in v3 I'm going to leave this patch as is and adding one more patch to convert both the lines at the same time.
Thanks.

On 11 Jun 2018, at 19:08, Carlo Caione carlo@endlessm.com wrote:
On Mon, 2018-06-11 at 10:41 +0200, Dr. Philipp Tomsich wrote:
On 11 Jun 2018, at 10:08, Carlo Caione carlo@caione.org wrote:
/cut
+#define GRF_SOC_CON0 0xff770244 #define GRF_SOC_CON2 0xff77024c
Could you convert these to ‘const uintptr_t GRF_SOC_CON0 = …’ ? The compiler will generate the same code for a const as if it’s a define, but we’ll have full type-safety.
Yeah, no problem. But if this is ok with you in v3 I'm going to leave this patch as is and adding one more patch to convert both the lines at the same time.
Sure. Add another patch and I’ll squash it when applying.
—Philipp.

From: Carlo Caione carlo@endlessm.com
On the veyron board the vcc33_sd regulator is used as vmmc-supply for the SD card. This regulator is powered in the MMC core during power on but its value is never actually set.
In the veyron platform the reset value for the LDO output is 1.8V while the standard (min and max) value for this regulator defined in the DTS is 3.3V. When the MMC core enable the regulator without setting its value, the output is automatically set to 1.8V instead of 3.3V.
With this patch we preemptively set the value to 3.3V.
Signed-off-by: Carlo Caione carlo@endlessm.com Reviewed-by: Simon Glass sjg@chromium.org --- arch/arm/mach-rockchip/rk3288-board.c | 10 ++++++++++ 1 file changed, 10 insertions(+)
diff --git a/arch/arm/mach-rockchip/rk3288-board.c b/arch/arm/mach-rockchip/rk3288-board.c index bf24d8e074..0e83c0a947 100644 --- a/arch/arm/mach-rockchip/rk3288-board.c +++ b/arch/arm/mach-rockchip/rk3288-board.c @@ -122,6 +122,16 @@ static int veyron_init(void) if (IS_ERR_VALUE(ret)) return ret;
+ ret = regulator_get_by_platname("vcc33_sd", &dev); + if (ret) { + debug("Cannot get regulator name\n"); + return ret; + } + + ret = regulator_set_value(dev, 3300000); + if (ret) + return ret; + ret = regulators_enable_boot_on(false); if (ret) { debug("%s: Cannot enable boot on regulators\n", __func__);

On 11 Jun 2018, at 10:08, Carlo Caione carlo@caione.org wrote:
From: Carlo Caione carlo@endlessm.com
On the veyron board the vcc33_sd regulator is used as vmmc-supply for the SD card. This regulator is powered in the MMC core during power on but its value is never actually set.
In the veyron platform the reset value for the LDO output is 1.8V while the standard (min and max) value for this regulator defined in the DTS is 3.3V. When the MMC core enable the regulator without setting its value, the output is automatically set to 1.8V instead of 3.3V.
With this patch we preemptively set the value to 3.3V.
Signed-off-by: Carlo Caione carlo@endlessm.com Reviewed-by: Simon Glass sjg@chromium.org
Reviewed-by: Philipp Tomsich philipp.tomsich@theobroma-systems.com Acked-by: Philipp Tomsich philipp.tomsich@theobroma-systems.com
participants (3)
-
Carlo Caione
-
Carlo Caione
-
Dr. Philipp Tomsich