[U-Boot] [RFC PATCH 0/2] i.MX28 SoC pending patches
Those patches are waiting in my tree for some time and are part of my work i.MX233 support. Am trying to send small parts that are ready for merging and that do not depends on i.MX233 SoC to work.
Otavio Salvador (2): i.MX28: Check if we are using a valid VBUS when initializing power i.MX28: use a clear name for DDR2 initialization
arch/arm/cpu/arm926ejs/mx28/spl_mem_init.c | 12 ++++++------ arch/arm/cpu/arm926ejs/mx28/spl_power_init.c | 21 ++++++++++++++------- 2 files changed, 20 insertions(+), 13 deletions(-)
Signed-off-by: Otavio Salvador otavio@ossystems.com.br --- arch/arm/cpu/arm926ejs/mx28/spl_power_init.c | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-)
diff --git a/arch/arm/cpu/arm926ejs/mx28/spl_power_init.c b/arch/arm/cpu/arm926ejs/mx28/spl_power_init.c index 4b09b0c..c8a35af 100644 --- a/arch/arm/cpu/arm926ejs/mx28/spl_power_init.c +++ b/arch/arm/cpu/arm926ejs/mx28/spl_power_init.c @@ -564,6 +564,15 @@ void mx28_batt_boot(void) 0x8 << POWER_5VCTRL_CHARGE_4P2_ILIMIT_OFFSET); }
+static int mx28_valid_vbus(void) +{ + struct mx28_power_regs *power_regs = + (struct mx28_power_regs *)MXS_POWER_BASE; + + /* iMX23 uses POWER_STS_VBUSVALID_STATUS at same offset */ + return readl(&power_regs->hw_power_sts) & POWER_STS_VBUSVALID0_STATUS; +} + void mx28_handle_5v_conflict(void) { struct mx28_power_regs *power_regs = @@ -581,7 +590,7 @@ void mx28_handle_5v_conflict(void) break; }
- if (tmp & POWER_STS_VDD5V_GT_VDDIO) { + if (mx28_valid_vbus() && (tmp & POWER_STS_VDD5V_GT_VDDIO)) { mx28_boot_valid_5v(); break; } else { @@ -601,17 +610,15 @@ void mx28_5v_boot(void) struct mx28_power_regs *power_regs = (struct mx28_power_regs *)MXS_POWER_BASE;
- /* - * NOTE: In original IMX-Bootlets, this also checks for VBUSVALID, - * but their implementation always returns 1 so we omit it here. - */ - if (readl(&power_regs->hw_power_sts) & POWER_STS_VDD5V_GT_VDDIO) { + if (mx28_valid_vbus() && + (readl(&power_regs->hw_power_sts) & POWER_STS_VDD5V_GT_VDDIO)) { mx28_boot_valid_5v(); return; }
early_delay(1000); - if (readl(&power_regs->hw_power_sts) & POWER_STS_VDD5V_GT_VDDIO) { + if (mx28_valid_vbus() && + (readl(&power_regs->hw_power_sts) & POWER_STS_VDD5V_GT_VDDIO)) { mx28_boot_valid_5v(); return; }
Dear Otavio Salvador,
[...]
if (tmp & POWER_STS_VDD5V_GT_VDDIO) {
if (mx28_valid_vbus() && (tmp & POWER_STS_VDD5V_GT_VDDIO)) {
So if the board isn't supplied by USB VBUS, this will fail now?
[...]
Best regards, Marek Vasut
On Mon, Jul 16, 2012 at 5:42 PM, Marek Vasut marex@denx.de wrote:
Dear Otavio Salvador,
[...]
if (tmp & POWER_STS_VDD5V_GT_VDDIO) {
if (mx28_valid_vbus() && (tmp & POWER_STS_VDD5V_GT_VDDIO)) {So if the board isn't supplied by USB VBUS, this will fail now?
I sent a v2 of it, but with a minor change in short description.
The mx28 prefix has been added to the initialization data and function so it is clear by which SoC it is used as i.MX233 will have a specific one.
Signed-off-by: Otavio Salvador otavio@ossystems.com.br --- arch/arm/cpu/arm926ejs/mx28/spl_mem_init.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/arch/arm/cpu/arm926ejs/mx28/spl_mem_init.c b/arch/arm/cpu/arm926ejs/mx28/spl_mem_init.c index e17a4d7..7317a97 100644 --- a/arch/arm/cpu/arm926ejs/mx28/spl_mem_init.c +++ b/arch/arm/cpu/arm926ejs/mx28/spl_mem_init.c @@ -31,7 +31,7 @@
#include "mx28_init.h"
-uint32_t dram_vals[] = { +uint32_t mx28_dram_vals[] = { 0x00000000, 0x00000000, 0x00000000, 0x00000000, 0x00000000, 0x00000000, 0x00000000, 0x00000000, 0x00000000, 0x00000000, 0x00000000, 0x00000000, @@ -88,14 +88,14 @@ void __mx28_adjust_memory_params(uint32_t *dram_vals) void mx28_adjust_memory_params(uint32_t *dram_vals) __attribute__((weak, alias("__mx28_adjust_memory_params")));
-void init_m28_200mhz_ddr2(void) +void init_mx28_200mhz_ddr2(void) { int i;
- mx28_adjust_memory_params(dram_vals); + mx28_adjust_memory_params(mx28_dram_vals);
- for (i = 0; i < ARRAY_SIZE(dram_vals); i++) - writel(dram_vals[i], MXS_DRAM_BASE + (4 * i)); + for (i = 0; i < ARRAY_SIZE(mx28_dram_vals); i++) + writel(mx28_dram_vals[i], MXS_DRAM_BASE + (4 * i)); }
void mx28_mem_init_clock(void) @@ -230,7 +230,7 @@ void mx28_mem_init(void) /* Clear START bit from DRAM_CTL16 */ clrbits_le32(MXS_DRAM_BASE + 0x40, 1);
- init_m28_200mhz_ddr2(); + init_mx28_200mhz_ddr2();
/* Clear SREFRESH bit from DRAM_CTL17 */ clrbits_le32(MXS_DRAM_BASE + 0x44, 1);
Dear Otavio Salvador,
The mx28 prefix has been added to the initialization data and function so it is clear by which SoC it is used as i.MX233 will have a specific one.
Signed-off-by: Otavio Salvador otavio@ossystems.com.br
Acked-by: Marek Vasut marex@denx.de
[...]
Best regards, Marek Vasut
On 16/07/2012 22:39, Otavio Salvador wrote:
The mx28 prefix has been added to the initialization data and function so it is clear by which SoC it is used as i.MX233 will have a specific one.
Signed-off-by: Otavio Salvador otavio@ossystems.com.br
Hi Otavio,
arch/arm/cpu/arm926ejs/mx28/spl_mem_init.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/arch/arm/cpu/arm926ejs/mx28/spl_mem_init.c b/arch/arm/cpu/arm926ejs/mx28/spl_mem_init.c index e17a4d7..7317a97 100644 --- a/arch/arm/cpu/arm926ejs/mx28/spl_mem_init.c +++ b/arch/arm/cpu/arm926ejs/mx28/spl_mem_init.c @@ -31,7 +31,7 @@
#include "mx28_init.h"
-uint32_t dram_vals[] = { +uint32_t mx28_dram_vals[] = { 0x00000000, 0x00000000, 0x00000000, 0x00000000, 0x00000000, 0x00000000, 0x00000000, 0x00000000, 0x00000000, 0x00000000, 0x00000000, 0x00000000, @@ -88,14 +88,14 @@ void __mx28_adjust_memory_params(uint32_t *dram_vals) void mx28_adjust_memory_params(uint32_t *dram_vals) __attribute__((weak, alias("__mx28_adjust_memory_params")));
Simply a general remark - I think we can discuss further when first patches forMX23 will be submitted. However, I think is a bad idea to specialize functions to a specific SOC. If a driver needs such as function, we will introduce some nasty #ifdef in driver code only to select which function must be called. And we duplicate this problem in all drivers.
Instead of that, we could use the same general name. Where do we put mx23 code ? If we put it together with mx28 code (I mean in the same directory) we could extract specific SOC function from general files and use the mechanism provided by Makefile to compile the right one, for example:
COBJS-$(CONFIG_MX28) += spl_mem_mx28.o COBJS-$(CONFIG_MX23) += spl_mem_mx23.o
and spl_mem_init.c contains general code. Drivers call always the same function. Of course, this is only an example. At the moment, most mx28 function have the mx28_ prefix, but this is acceptable because up now the mx28 is the only one in this architecture.
Best regards, Stefano Babic
On Tue, Jul 17, 2012 at 6:15 AM, Stefano Babic sbabic@denx.de wrote:
Simply a general remark - I think we can discuss further when first patches for MX23 will be submitted. However, I think is a bad idea to specialize functions to a specific SOC. If a driver needs such as function, we will introduce some nasty #ifdef in driver code only to select which function must be called. And we duplicate this problem in all drivers.
Instead of that, we could use the same general name. Where do we put mx23 code ? If we put it together with mx28 code (I mean in the same directory) we could extract specific SOC function from general files and use the mechanism provided by Makefile to compile the right one, for example:
COBJS-$(CONFIG_MX28) += spl_mem_mx28.o COBJS-$(CONFIG_MX23) += spl_mem_mx23.o
and spl_mem_init.c contains general code. Drivers call always the same function. Of course, this is only an example. At the moment, most mx28 function have the mx28_ prefix, but this is acceptable because up now the mx28 is the only one in this architecture.
Agreed; I will check my current work in progress code and rework it this way. However this prefix change is highly desired as it is callmed m28 (looking as m28evk specific) instead of mx28.
On 17/07/2012 14:16, Otavio Salvador wrote:
On Tue, Jul 17, 2012 at 6:15 AM, Stefano Babic sbabic@denx.de wrote:
Simply a general remark - I think we can discuss further when first patches for MX23 will be submitted. However, I think is a bad idea to specialize functions to a specific SOC. If a driver needs such as function, we will introduce some nasty #ifdef in driver code only to select which function must be called. And we duplicate this problem in all drivers.
Instead of that, we could use the same general name. Where do we put mx23 code ? If we put it together with mx28 code (I mean in the same directory) we could extract specific SOC function from general files and use the mechanism provided by Makefile to compile the right one, for example:
COBJS-$(CONFIG_MX28) += spl_mem_mx28.o COBJS-$(CONFIG_MX23) += spl_mem_mx23.o
and spl_mem_init.c contains general code. Drivers call always the same function. Of course, this is only an example. At the moment, most mx28 function have the mx28_ prefix, but this is acceptable because up now the mx28 is the only one in this architecture.
Agreed; I will check my current work in progress code and rework it this way. However this prefix change is highly desired as it is callmed m28 (looking as m28evk specific) instead of mx28.
Yes, mx28 is a better name - if a prefix, it should be a SOC prefix, not a board name.
But I noted : -uint32_t dram_vals[] = { +uint32_t mx28_dram_vals[] = { 0x00000000, 0x00000000, 0x00000000, 0x00000000, 0x00000000, 0x00000000, 0x00000000, 0x00000000, 0x00000000, 0x00000000, 0x00000000, 0x00000000,
why is this structure not static ?
Best regards, Stefano Babic
On Tue, Jul 17, 2012 at 9:41 AM, Stefano Babic sbabic@denx.de wrote:
Agreed; I will check my current work in progress code and rework it this way. However this prefix change is highly desired as it is callmed m28 (looking as m28evk specific) instead of mx28.
Yes, mx28 is a better name - if a prefix, it should be a SOC prefix, not a board name.
But I noted : -uint32_t dram_vals[] = { +uint32_t mx28_dram_vals[] = { 0x00000000, 0x00000000, 0x00000000, 0x00000000, 0x00000000, 0x00000000, 0x00000000, 0x00000000, 0x00000000, 0x00000000, 0x00000000, 0x00000000,
why is this structure not static ?
Indeed; I will send a new patch for it.
participants (3)
-
Marek Vasut -
Otavio Salvador -
Stefano Babic