[U-Boot] [RFC PATCH 0/2] i.MX28 SoC pending patches

[U-Boot] [RFC PATCH 0/2] i.MX28 SoC pending patches

[Otavio Salvador]

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(-)

-- 
1.7.10.4

[U-Boot] [RFC PATCH 1/2] i.MX28: Check if we are using a valid VBUS when initializing power

[Otavio Salvador]

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@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;
 	}
-- 
1.7.10.4

[U-Boot] [RFC PATCH 2/2] i.MX28: use a clear name for DDR2 initialization

[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>
---
 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);
-- 
1.7.10.4

[U-Boot] [RFC PATCH 1/2] i.MX28: Check if we are using a valid VBUS when initializing power

[Marek Vasut]

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

[U-Boot] [RFC PATCH 2/2] i.MX28: use a clear name for DDR2 initialization

[Marek Vasut]

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

[U-Boot] [RFC PATCH 1/2] i.MX28: Check if we are using a valid VBUS when initializing power

[Otavio Salvador]

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.

-- 
Otavio Salvador                             O.S. Systems
E-mail: otavio at ossystems.com.br  http://www.ossystems.com.br
Mobile: +55 53 9981-7854              http://projetos.ossystems.com.br

[U-Boot] [RFC PATCH 2/2] i.MX28: use a clear name for DDR2 initialization

[Stefano Babic]

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

-- 
=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic at denx.de
=====================================================================

[U-Boot] [RFC PATCH 2/2] i.MX28: use a clear name for DDR2 initialization

[Otavio Salvador]

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.

-- 
Otavio Salvador                             O.S. Systems
E-mail: otavio at ossystems.com.br  http://www.ossystems.com.br
Mobile: +55 53 9981-7854              http://projetos.ossystems.com.br

[U-Boot] [RFC PATCH 2/2] i.MX28: use a clear name for DDR2 initialization

[Stefano Babic]

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

-- 
=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic at denx.de
=====================================================================

[U-Boot] [RFC PATCH 2/2] i.MX28: use a clear name for DDR2 initialization

[Otavio Salvador]

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.

-- 
Otavio Salvador                             O.S. Systems
E-mail: otavio at ossystems.com.br  http://www.ossystems.com.br
Mobile: +55 53 9981-7854              http://projetos.ossystems.com.br