[U-Boot] [PATCH] mx53loco: Fix PMIC name

[U-Boot] [PATCH] mx53loco: Fix PMIC name

[Fabio Estevam]

commit c73368150 (pmic: Extend PMIC framework to support multiple instances 
of PMIC devices) has incorrectly passed the PMIC name under the FSL PMIC case.

Fix that by passing "FSL_PMIC" as the parameter of pmic_get.

Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>
---
 board/freescale/mx53loco/mx53loco.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/board/freescale/mx53loco/mx53loco.c b/board/freescale/mx53loco/mx53loco.c
index 81c511c..2c8cb7a 100644
--- a/board/freescale/mx53loco/mx53loco.c
+++ b/board/freescale/mx53loco/mx53loco.c
@@ -374,7 +374,7 @@ static int power_init(void)
 		if (retval)
 			return retval;
 
-		p = pmic_get("DIALOG_PMIC");
+		p = pmic_get("FSL_PMIC");
 		if (!p)
 			return -ENODEV;
 
-- 
1.7.9.5

[U-Boot] [PATCH] mx53loco: Fix PMIC name

[Stefano Babic]

On 11/12/2012 17:36, Fabio Estevam wrote:
> commit c73368150 (pmic: Extend PMIC framework to support multiple instances 
> of PMIC devices) has incorrectly passed the PMIC name under the FSL PMIC case.
> 
> Fix that by passing "FSL_PMIC" as the parameter of pmic_get.
> 
> Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>
> ---
>  board/freescale/mx53loco/mx53loco.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/board/freescale/mx53loco/mx53loco.c b/board/freescale/mx53loco/mx53loco.c
> index 81c511c..2c8cb7a 100644
> --- a/board/freescale/mx53loco/mx53loco.c
> +++ b/board/freescale/mx53loco/mx53loco.c
> @@ -374,7 +374,7 @@ static int power_init(void)
>  		if (retval)
>  			return retval;
>  
> -		p = pmic_get("DIALOG_PMIC");
> +		p = pmic_get("FSL_PMIC");
>  		if (!p)
>  			return -ENODEV;
>  
> 

Applied to u-boot-imx, thanks.

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] [PATCH] mx53loco: Fix PMIC name

[Robert Nelson]

On Tue, Dec 11, 2012 at 10:36 AM, Fabio Estevam
<fabio.estevam@freescale.com> wrote:
> commit c73368150 (pmic: Extend PMIC framework to support multiple instances
> of PMIC devices) has incorrectly passed the PMIC name under the FSL PMIC case.
>
> Fix that by passing "FSL_PMIC" as the parameter of pmic_get.
>
> Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>
> ---
>  board/freescale/mx53loco/mx53loco.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/board/freescale/mx53loco/mx53loco.c b/board/freescale/mx53loco/mx53loco.c
> index 81c511c..2c8cb7a 100644
> --- a/board/freescale/mx53loco/mx53loco.c
> +++ b/board/freescale/mx53loco/mx53loco.c
> @@ -374,7 +374,7 @@ static int power_init(void)
>                 if (retval)
>                         return retval;
>
> -               p = pmic_get("DIALOG_PMIC");
> +               p = pmic_get("FSL_PMIC");
>                 if (!p)
>                         return -ENODEV;
>
> --
> 1.7.9.5

Hi Fabio,

It looks like we need one more fixup after commit c73368150 on the
first run/older (non R, Dialog, bug wired extra Capacitor) version of
this board..  Do you happen to have any in your lab?

U-Boot 2013.01-rc2-dirty (Dec 20 2012 - 15:55:01)

Board: MX53 LOCO
I2C:   ready
DRAM:  1 GiB
pmic_alloc: No available memory for allocation!
pmic_init: POWER allocation error!
CPU:   Freescale i.MX53 family rev2.0 at 800 MHz
Reset cause: POR
MMC:   FSL_SDHC: 0, FSL_SDHC: 1
*** Warning - bad CRC, using default environment

(and it just keeps on resetting)

It looks to be failing after calloc...
http://git.denx.de/?p=u-boot.git;a=blob;f=drivers/power/power_core.c#l100

 100         p = calloc(sizeof(*p), 1);
 101         if (!p) {
 102                 printf("%s: No available memory for
allocation!\n", __func__);
 103                 return NULL;
 104         }

Regards,

-- 
Robert Nelson
http://www.rcn-ee.com/

[U-Boot] [PATCH] mx53loco: Fix PMIC name

[Fabio Estevam]

Hi Robert,

On Thu, Dec 20, 2012 at 8:04 PM, Robert Nelson <robertcnelson@gmail.com> wrote:

> Hi Fabio,
>
> It looks like we need one more fixup after commit c73368150 on the
> first run/older (non R, Dialog, bug wired extra Capacitor) version of
> this board..  Do you happen to have any in your lab?

Thanks for reporting this issue. I will try to get access to mx53qsb
dialog next week to investigate this further.

Regards,

Fabio Estevam

[U-Boot] [PATCH] mx53loco: Fix PMIC name

[Fabio Estevam]

Robert,

On Thu, Dec 20, 2012 at 8:04 PM, Robert Nelson <robertcnelson@gmail.com> wrote:

> Hi Fabio,
>
> It looks like we need one more fixup after commit c73368150 on the
> first run/older (non R, Dialog, bug wired extra Capacitor) version of
> this board..  Do you happen to have any in your lab?
>
> U-Boot 2013.01-rc2-dirty (Dec 20 2012 - 15:55:01)
>
> Board: MX53 LOCO
> I2C:   ready
> DRAM:  1 GiB
> pmic_alloc: No available memory for allocation!
> pmic_init: POWER allocation error!
> CPU:   Freescale i.MX53 family rev2.0 at 800 MHz
> Reset cause: POR
> MMC:   FSL_SDHC: 0, FSL_SDHC: 1
> *** Warning - bad CRC, using default environment
>
> (and it just keeps on resetting)
>
> It looks to be failing after calloc...
> http://git.denx.de/?p=u-boot.git;a=blob;f=drivers/power/power_core.c#l100
>
>  100         p = calloc(sizeof(*p), 1);
>  101         if (!p) {
>  102                 printf("%s: No available memory for
> allocation!\n", __func__);
>  103                 return NULL;
>  104         }

I can reproduce this issue using top of head U-boot tree.

It seems indeed to be caused by:

Author: ?ukasz Majewski <l.majewski@samsung.com>
Date:   Tue Nov 13 03:21:55 2012 +0000
commit c7336815078ff3745e3130aeff35991e3e98e61e
Author: ?ukasz Majewski <l.majewski@samsung.com>
Date:   Tue Nov 13 03:21:55 2012 +0000

    pmic: Extend PMIC framework to support multiple instances of PMIC devices

    The PMIC framework has been extended to support multiple instances of
    the variety of devices responsible for power management.
    This change allows supporting of e.g. fuel gauge, charger, MUIC (Micro USB
    Interface Circuit).
    Power related includes have been moved to ./include/power directory.
    This is a first of a series of patches - in the future "pmic" will be
    replaced with "power".

    Two important issues:
    1. The PMIC needs to be initialized just after malloc is configured
    2. It uses list to hold information about available PMIC devices

    Signed-off-by: Lukasz Majewski <l.majewski@samsung.com>
    Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
    Cc: Stefano Babic <sbabic@denx.de>

I am Ccing Lukasz/Stefano in case they have any suggestion.

With the new PMIC framework: is it safe to call the PMIC API from
board_init function?

Regards,

Fabio Estevam

[U-Boot] [PATCH] mx53loco: Fix PMIC name

[Fabio Estevam]

On Wed, Dec 26, 2012 at 1:02 PM, Fabio Estevam <festevam@gmail.com> wrote:

> With the new PMIC framework: is it safe to call the PMIC API from
> board_init function?

Calling it from board_late_init fixes the issue for me.

Will send the patch soon.

Regards,

Fabio Estevam

[U-Boot] [PATCH] mx53loco: Fix PMIC name

[Fabio Estevam]

Robert,

On Wed, Dec 26, 2012 at 1:07 PM, Fabio Estevam <festevam@gmail.com> wrote:
> On Wed, Dec 26, 2012 at 1:02 PM, Fabio Estevam <festevam@gmail.com> wrote:
>
>> With the new PMIC framework: is it safe to call the PMIC API from
>> board_init function?
>
> Calling it from board_late_init fixes the issue for me.
>
> Will send the patch soon.

Can you try the patch below?

board/freescale/mx53loco/mx53loco.c |   10 ++++++++--
 include/configs/mx53loco.h          |    1 +
 2 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/board/freescale/mx53loco/mx53loco.c
b/board/freescale/mx53loco/mx53loco.c
index 2c8cb7a..63a4f8b 100644
--- a/board/freescale/mx53loco/mx53loco.c
+++ b/board/freescale/mx53loco/mx53loco.c
@@ -462,12 +462,18 @@ int board_init(void)

 	mxc_set_sata_internal_clock();
 	setup_iomux_i2c();
+
+	lcd_enable();
+
+	return 0;
+}
+
+int board_late_init(void)
+{
 	if (!power_init())
 		clock_1GHz();
 	print_cpuinfo();

-	lcd_enable();
-
 	return 0;
 }

diff --git a/include/configs/mx53loco.h b/include/configs/mx53loco.h
index e30502b..c4181bd 100644
--- a/include/configs/mx53loco.h
+++ b/include/configs/mx53loco.h
@@ -39,6 +39,7 @@
 #define CONFIG_SYS_MALLOC_LEN		(10 * 1024 * 1024)

 #define CONFIG_BOARD_EARLY_INIT_F
+#define CONFIG_BOARD_LATE_INIT
 #define CONFIG_MXC_GPIO
 #define CONFIG_REVISION_TAG

-- 
1.7.9.5


It boots now, but I get resets now:

U-Boot 2013.01-rc2-00172-gf8cfcf1-dirty (Dec 26 2012 - 13:13:28)

Board: MX53 LOCO
I2C:   ready
DRAM:  1 GiB
MMC:   FSL_SDHC: 0, FSL_SDHC: 1
In:    serial
Out:   serial
Err:   serial
CPU:   Freescale i.MX53 family rev2.1 at 1000 MHz
Reset cause: WDOG
Net:   FEC
Warning: FEC using MAC address from net device

Hit any key to stop autoboot:  0
data abort

    MAYBE you should read doc/README.arm-unaligned-accesses

pc : [<aff72220>]          lr : [<aff721fc>]
sp : af565e20  ip : af566918     fp : 00000000
r10: 00000003  r9 : affabb5b     r8 : af565f58
r7 : 00000000  r6 : 36747fff     r5 : af5668e8  r4 : 36747fff
r3 : af5668ec  r2 : af5668eb     r1 : 00000000  r0 : af5668e8
Flags: NzcV  IRQs off  FIQs off  Mode SVC_32
Resetting CPU ...

resetting ...


U-Boot 2013.01-rc2-00172-gf8cfcf1-dirty (Dec 26 2012 - 13:13:28)

Board: MX53 LOCO
I2C:   ready
DRAM:  1 GiB


but it looks like a separate issue?

Regards,

Fabio Estevam

[U-Boot] [PATCH] mx53loco: Fix PMIC name

[Robert Nelson]

Hi Fabio,

On Wed, Dec 26, 2012 at 9:21 AM, Fabio Estevam <festevam@gmail.com> wrote:
> Robert,
>
> On Wed, Dec 26, 2012 at 1:07 PM, Fabio Estevam <festevam@gmail.com> wrote:
>> On Wed, Dec 26, 2012 at 1:02 PM, Fabio Estevam <festevam@gmail.com> wrote:
>>
>>> With the new PMIC framework: is it safe to call the PMIC API from
>>> board_init function?
>>
>> Calling it from board_late_init fixes the issue for me.
>>
>> Will send the patch soon.
>
> Can you try the patch below?

Thanks for taking a look at this..

>
> board/freescale/mx53loco/mx53loco.c |   10 ++++++++--
>  include/configs/mx53loco.h          |    1 +
>  2 files changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/board/freescale/mx53loco/mx53loco.c
> b/board/freescale/mx53loco/mx53loco.c
> index 2c8cb7a..63a4f8b 100644
> --- a/board/freescale/mx53loco/mx53loco.c
> +++ b/board/freescale/mx53loco/mx53loco.c
> @@ -462,12 +462,18 @@ int board_init(void)
>
>         mxc_set_sata_internal_clock();
>         setup_iomux_i2c();
> +
> +       lcd_enable();
> +
> +       return 0;
> +}
> +
> +int board_late_init(void)
> +{
>         if (!power_init())
>                 clock_1GHz();
>         print_cpuinfo();
>
> -       lcd_enable();
> -
>         return 0;
>  }
>
> diff --git a/include/configs/mx53loco.h b/include/configs/mx53loco.h
> index e30502b..c4181bd 100644
> --- a/include/configs/mx53loco.h
> +++ b/include/configs/mx53loco.h
> @@ -39,6 +39,7 @@
>  #define CONFIG_SYS_MALLOC_LEN          (10 * 1024 * 1024)
>
>  #define CONFIG_BOARD_EARLY_INIT_F
> +#define CONFIG_BOARD_LATE_INIT

Ah, i was missing this when heading down the "board_late_init" road on friday..

>  #define CONFIG_MXC_GPIO
>  #define CONFIG_REVISION_TAG
>
> --
> 1.7.9.5
>
>
> It boots now, but I get resets now:
>
> U-Boot 2013.01-rc2-00172-gf8cfcf1-dirty (Dec 26 2012 - 13:13:28)
>
> Board: MX53 LOCO
> I2C:   ready
> DRAM:  1 GiB
> MMC:   FSL_SDHC: 0, FSL_SDHC: 1
> In:    serial
> Out:   serial
> Err:   serial
> CPU:   Freescale i.MX53 family rev2.1 at 1000 MHz
> Reset cause: WDOG
> Net:   FEC
> Warning: FEC using MAC address from net device

Interesting, I'm still only getting this far on the old Dialog board..

U-Boot 2013.01-rc2-00173-gd781d95-dirty (Dec 26 2012 - 09:42:43)

Board: MX53 LOCO
I2C:   ready
DRAM:  1 GiB

I'm going to start printf'ing:
http://git.denx.de/?p=u-boot.git;a=blob;f=drivers/power/power_dialog.c
 as we aren't getting the memory error, so it should be atleast past
that...


> Hit any key to stop autoboot:  0
> data abort
>
>     MAYBE you should read doc/README.arm-unaligned-accesses
>
> pc : [<aff72220>]          lr : [<aff721fc>]
> sp : af565e20  ip : af566918     fp : 00000000
> r10: 00000003  r9 : affabb5b     r8 : af565f58
> r7 : 00000000  r6 : 36747fff     r5 : af5668e8  r4 : 36747fff
> r3 : af5668ec  r2 : af5668eb     r1 : 00000000  r0 : af5668e8
> Flags: NzcV  IRQs off  FIQs off  Mode SVC_32
> Resetting CPU ...
>
> resetting ...

I've reverted this just to double check (no change for me...):
http://git.denx.de/?p=u-boot.git;a=commit;h=28e5ac2d974547bde0c72aa0c1d66fd22c6ef3ad


> U-Boot 2013.01-rc2-00172-gf8cfcf1-dirty (Dec 26 2012 - 13:13:28)
>
> Board: MX53 LOCO
> I2C:   ready
> DRAM:  1 GiB
>
>
> but it looks like a separate issue?

Regards,

-- 
Robert Nelson
http://www.rcn-ee.com/

[U-Boot] [PATCH] mx53loco: Fix PMIC name

[Fabio Estevam]

On Wed, Dec 26, 2012 at 1:51 PM, Robert Nelson <robertcnelson@gmail.com> wrote:

> Interesting, I'm still only getting this far on the old Dialog board..
>
> U-Boot 2013.01-rc2-00173-gd781d95-dirty (Dec 26 2012 - 09:42:43)
>
> Board: MX53 LOCO
> I2C:   ready
> DRAM:  1 GiB

I tested with 2 mx53loco boards here: one with FSL PMIC and one with Dialog.

I recall we had 2 versions of boards with Dialog PMIC, but at least
with the one I have here it boots fine now after applying the patch I
just sent.

If you have any news, just let us know.

Thanks,

Fabio Estevam

[U-Boot] [PATCH] mx53loco: Fix PMIC name

[Robert Nelson]

On Wed, Dec 26, 2012 at 9:54 AM, Fabio Estevam <festevam@gmail.com> wrote:
> On Wed, Dec 26, 2012 at 1:51 PM, Robert Nelson <robertcnelson@gmail.com> wrote:
>
>> Interesting, I'm still only getting this far on the old Dialog board..
>>
>> U-Boot 2013.01-rc2-00173-gd781d95-dirty (Dec 26 2012 - 09:42:43)
>>
>> Board: MX53 LOCO
>> I2C:   ready
>> DRAM:  1 GiB
>
> I tested with 2 mx53loco boards here: one with FSL PMIC and one with Dialog.
>
> I recall we had 2 versions of boards with Dialog PMIC, but at least
> with the one I have here it boots fine now after applying the patch I
> just sent.

Okay, good to hear it works with the Dialog PMIC you have, as that
should cover a majority of the Dialog based boards customers actually
have. As the one I have here was one of the very first ones and i know
something changed later in production...

Regards,

-- 
Robert Nelson
http://www.rcn-ee.com/

[U-Boot] [PATCH] mx53loco: Fix PMIC name

[Fabio Estevam]

On Wed, Dec 26, 2012 at 2:04 PM, Robert Nelson <robertcnelson@gmail.com> wrote:

> Okay, good to hear it works with the Dialog PMIC you have, as that
> should cover a majority of the Dialog based boards customers actually
> have. As the one I have here was one of the very first ones and i know
> something changed later in production...

Yes, but not happy to see a regression with your board and also the
resets that started to happen now.

Which toolchain did you use? I am using gcc 4.6.3. If you have a
chance to try the same toolchain to make sure we got the same results,
that would be nice.

Regards,

Fabio Estevam

[U-Boot] [PATCH] mx53loco: Fix PMIC name

[Robert Nelson]

On Wed, Dec 26, 2012 at 10:50 AM, Fabio Estevam <festevam@gmail.com> wrote:
> On Wed, Dec 26, 2012 at 2:04 PM, Robert Nelson <robertcnelson@gmail.com> wrote:
>
>> Okay, good to hear it works with the Dialog PMIC you have, as that
>> should cover a majority of the Dialog based boards customers actually
>> have. As the one I have here was one of the very first ones and i know
>> something changed later in production...
>
> Yes, but not happy to see a regression with your board and also the
> resets that started to happen now.

I'll keep debugging it here, did reverting this commit, help with the resets?
http://git.denx.de/?p=u-boot.git;a=commit;h=28e5ac2d974547bde0c72aa0c1d66fd22c6ef3ad

> Which toolchain did you use? I am using gcc 4.6.3. If you have a
> chance to try the same toolchain to make sure we got the same results,
> that would be nice.

That build was with Linaro's 4.7.1 2012.04 release, from:
https://launchpad.net/linaro-toolchain-binaries/

./arm-linux-gnueabi-gcc --version
arm-linux-gnueabi-gcc (crosstool-NG linaro-1.13.1-2012.04-20120426 -
Linaro GCC 2012.04) 4.7.1 20120402 (prerelease)

Regards,

-- 
Robert Nelson
http://www.rcn-ee.com/

[U-Boot] [PATCH] mx53loco: Fix PMIC name

[Fabio Estevam]

On Wed, Dec 26, 2012 at 3:08 PM, Robert Nelson <robertcnelson@gmail.com> wrote:

> I'll keep debugging it here, did reverting this commit, help with the resets?
> http://git.denx.de/?p=u-boot.git;a=commit;h=28e5ac2d974547bde0c72aa0c1d66fd22c6ef3ad

Tried reverting it and still see the data-aborts.

I am also testing on my mx53loco board with the mc34708 pmic and I see
that it hangs on boot about 10% of the attempts:
U-Boot 2013.01-rc2-00172-gf8cfcf1-dirty (Dec 26 2012 - 15:15:27)

Board: MX53 LOCO
I2C:   ready
DRAM:  1 GiB
MMC:   FSL_SDHC: 0, FSL_SDHC: 1

This did not happen prior to commit c73368150 (pmic: Extend PMIC
framework to support multiple instances
of PMIC devices).

Regards,

Fabio Estevam

[U-Boot] [PATCH] mx53loco: Fix PMIC name

[Robert Nelson]

On Wed, Dec 26, 2012 at 11:21 AM, Fabio Estevam <festevam@gmail.com> wrote:
> On Wed, Dec 26, 2012 at 3:08 PM, Robert Nelson <robertcnelson@gmail.com> wrote:
>
>> I'll keep debugging it here, did reverting this commit, help with the resets?
>> http://git.denx.de/?p=u-boot.git;a=commit;h=28e5ac2d974547bde0c72aa0c1d66fd22c6ef3ad
>
> Tried reverting it and still see the data-aborts.
>
> I am also testing on my mx53loco board with the mc34708 pmic and I see
> that it hangs on boot about 10% of the attempts:
> U-Boot 2013.01-rc2-00172-gf8cfcf1-dirty (Dec 26 2012 - 15:15:27)
>
> Board: MX53 LOCO
> I2C:   ready
> DRAM:  1 GiB
> MMC:   FSL_SDHC: 0, FSL_SDHC: 1
>
> This did not happen prior to commit c73368150 (pmic: Extend PMIC
> framework to support multiple instances
> of PMIC devices).

Okay, now we are on the same page, upgraded to:
 "arm-linux-gnueabihf-gcc (crosstool-NG
linaro-1.13.1-4.7-2012.12-20121214 - Linaro GCC 2012.12) 4.7.3
20121205 (prerelease)"..

With your Dialog patch, it is properly setting 1Ghz, and just keeps
rebooting.. :)

U-Boot 2013.01-rc2-00173-ga0d04f3 (Dec 26 2012 - 11:47:34)

Board: MX53 LOCO
I2C:   ready
DRAM:  1 GiB
MMC:   FSL_SDHC: 0, FSL_SDHC: 1
*** Warning - bad CRC, using default environment
In:    serial
Out:   serial
Err:   serial
CPU:   Freescale i.MX53 family rev2.0 at 1000 MHz
Reset cause: WDOG
Net:   FEC
Warning: FEC using MAC address from net device

Hit any key to stop autoboot:  0

data abort
    MAYBE you should read doc/README.arm-unaligned-accesses
pc : [<aff72250>]	   lr : [<aff72228>]
sp : af565d68  ip : fffffff4	 fp : 00000000
r10: 00000003  r9 : affab8d9	 r8 : af565f58
r7 : 00000000  r6 : e5bc345d	 r5 : af6df330  r4 : e5bc345d
r3 : af6df334  r2 : af6df333	 r1 : 00000000  r0 : af6df330
Flags: nzCv  IRQs off  FIQs off  Mode SVC_32

Resetting CPU ...
resetting ...

U-Boot 2013.01-rc2-00173-ga0d04f3 (Dec 26 2012 - 11:47:34)

Board: MX53 LOCO
I2C:   ready
DRAM:  1 GiB
MMC:   FSL_SDHC: 0, FSL_SDHC: 1
*** Warning - bad CRC, using default environment
In:    serial
Out:   serial
Err:   serial
CPU:   Freescale i.MX53 family rev2.0 at 1000 MHz

Reset cause: WDOG

Regards,

-- 
Robert Nelson
http://www.rcn-ee.com/

[U-Boot] [PATCH] mx53loco: Fix PMIC name

[Robert Nelson]

On Wed, Dec 26, 2012 at 11:51 AM, Robert Nelson <robertcnelson@gmail.com> wrote:
> On Wed, Dec 26, 2012 at 11:21 AM, Fabio Estevam <festevam@gmail.com> wrote:
>> On Wed, Dec 26, 2012 at 3:08 PM, Robert Nelson <robertcnelson@gmail.com> wrote:
>>
>>> I'll keep debugging it here, did reverting this commit, help with the resets?
>>> http://git.denx.de/?p=u-boot.git;a=commit;h=28e5ac2d974547bde0c72aa0c1d66fd22c6ef3ad
>>
>> Tried reverting it and still see the data-aborts.
>>
>> I am also testing on my mx53loco board with the mc34708 pmic and I see
>> that it hangs on boot about 10% of the attempts:
>> U-Boot 2013.01-rc2-00172-gf8cfcf1-dirty (Dec 26 2012 - 15:15:27)
>>
>> Board: MX53 LOCO
>> I2C:   ready
>> DRAM:  1 GiB
>> MMC:   FSL_SDHC: 0, FSL_SDHC: 1
>>
>> This did not happen prior to commit c73368150 (pmic: Extend PMIC
>> framework to support multiple instances
>> of PMIC devices).
>
> Okay, now we are on the same page, upgraded to:
>  "arm-linux-gnueabihf-gcc (crosstool-NG
> linaro-1.13.1-4.7-2012.12-20121214 - Linaro GCC 2012.12) 4.7.3
> 20121205 (prerelease)"..
>
> With your Dialog patch, it is properly setting 1Ghz, and just keeps
> rebooting.. :)
>
> U-Boot 2013.01-rc2-00173-ga0d04f3 (Dec 26 2012 - 11:47:34)
>
> Board: MX53 LOCO
> I2C:   ready
> DRAM:  1 GiB
> MMC:   FSL_SDHC: 0, FSL_SDHC: 1
> *** Warning - bad CRC, using default environment
> In:    serial
> Out:   serial
> Err:   serial
> CPU:   Freescale i.MX53 family rev2.0 at 1000 MHz
> Reset cause: WDOG
> Net:   FEC
> Warning: FEC using MAC address from net device
>
> Hit any key to stop autoboot:  0
>
> data abort
>     MAYBE you should read doc/README.arm-unaligned-accesses
> pc : [<aff72250>]          lr : [<aff72228>]
> sp : af565d68  ip : fffffff4     fp : 00000000
> r10: 00000003  r9 : affab8d9     r8 : af565f58
> r7 : 00000000  r6 : e5bc345d     r5 : af6df330  r4 : e5bc345d
> r3 : af6df334  r2 : af6df333     r1 : 00000000  r0 : af6df330
> Flags: nzCv  IRQs off  FIQs off  Mode SVC_32
>
> Resetting CPU ...
> resetting ...

Okay success on another bisect run to find what was causing the Data
Abort with linaro's gcc toolchain..
gcc version 4.7.3 20121205 (prerelease) (crosstool-NG
linaro-1.13.1-4.7-2012.12-20121214 - Linaro GCC 2012.12)

Reverting, Troy's last 4 imximage changes:

ab857f2613be39274d0870768720067ff9d859c0 (imximage: make
set_imx_hdr_v1/v2 easier to read)
ad0826dcd81ba6cf4cc93a4ca8337e1bc676c238 (imximage: change parameters
to set_imx_hdr)
243319825fa6e79885d57b6b78e72f3fe10ef69c (imximage: delay setting of image size)
348ca8efb79652d2b259fbd0ed6d317a77a8cbb9 (imximage: fix size of image
to load.) (this was the first bad bisect, the previous 3 are needed
for a clean revert)

Along with Fabio's "mx53loco: Call PMIC related functions from
board_late_init()"

The Dialog based mx53loco is no operating correctly with linaro's toolchain...

U-Boot 2013.01-rc2-00176-gbc67175-dirty (Jan 02 2013 - 12:14:51)

Board: MX53 LOCO
I2C:   ready
DRAM:  1 GiB
MMC:   FSL_SDHC: 0, FSL_SDHC: 1
*** Warning - bad CRC, using default environment

In:    serial
Out:   serial
Err:   serial
CPU:   Freescale i.MX53 family rev2.0 at 1000 MHz
Reset cause: POR
Net:   FEC
Warning: FEC using MAC address from net device

Hit any key to stop autoboot:  0
mmc0 is current device
** Unrecognized filesystem type **
** Unrecognized filesystem type **
Booting from net ...
BOOTP broadcast 1

Regards,

-- 
Robert Nelson
http://www.rcn-ee.com/

[U-Boot] [PATCH] mx53loco: Fix PMIC name

[Fabio Estevam]

On Wed, Jan 2, 2013 at 4:27 PM, Robert Nelson <robertcnelson@gmail.com> wrote:

> Okay success on another bisect run to find what was causing the Data
> Abort with linaro's gcc toolchain..
> gcc version 4.7.3 20121205 (prerelease) (crosstool-NG
> linaro-1.13.1-4.7-2012.12-20121214 - Linaro GCC 2012.12)
>
> Reverting, Troy's last 4 imximage changes:
>
> ab857f2613be39274d0870768720067ff9d859c0 (imximage: make
> set_imx_hdr_v1/v2 easier to read)
> ad0826dcd81ba6cf4cc93a4ca8337e1bc676c238 (imximage: change parameters
> to set_imx_hdr)
> 243319825fa6e79885d57b6b78e72f3fe10ef69c (imximage: delay setting of image size)
> 348ca8efb79652d2b259fbd0ed6d317a77a8cbb9 (imximage: fix size of image
> to load.) (this was the first bad bisect, the previous 3 are needed
> for a clean revert)
>
> Along with Fabio's "mx53loco: Call PMIC related functions from
> board_late_init()"
>
> The Dialog based mx53loco is no operating correctly with linaro's toolchain...
>
> U-Boot 2013.01-rc2-00176-gbc67175-dirty (Jan 02 2013 - 12:14:51)
>
> Board: MX53 LOCO
> I2C:   ready
> DRAM:  1 GiB
> MMC:   FSL_SDHC: 0, FSL_SDHC: 1
> *** Warning - bad CRC, using default environment
>
> In:    serial
> Out:   serial
> Err:   serial
> CPU:   Freescale i.MX53 family rev2.0 at 1000 MHz
> Reset cause: POR
> Net:   FEC
> Warning: FEC using MAC address from net device
>
> Hit any key to stop autoboot:  0
> mmc0 is current device
> ** Unrecognized filesystem type **
> ** Unrecognized filesystem type **
> Booting from net ...
> BOOTP broadcast 1

Thanks for bisecting, Robert.

Troy/Stefano,

Any suggestions about this? It would be really nice if we could keep
mx53loco functional when using gcc 4.6.2.

Regards,

Fabio Estevam

[U-Boot] [PATCH] mx53loco: Fix PMIC name

[Troy Kisky]

On 1/2/2013 11:33 AM, Fabio Estevam wrote:
> Thanks for bisecting, Robert. Troy/Stefano, Any suggestions about 
> this? It would be really nice if we could keep mx53loco functional 
> when using gcc 4.6.2. Regards, Fabio Estevam 
Could you see if this patch makes any difference? It still needs cleaned 
up some before mainline
[PATCH V4 01/11] imximage: mx53 needs transfer length a multiple of 512

diff --git a/tools/imximage.c b/tools/imximage.c
index 63f88b6..7e54e97 100644
--- a/tools/imximage.c
+++ b/tools/imximage.c
@@ -494,6 +494,8 @@ static void imximage_print_header(const void *ptr)
  	}
  }
  
+#define ALIGN(a, b)	(((a) + (b) - 1) & ~((b) - 1))
+
  static void imximage_set_header(void *ptr, struct stat *sbuf, int ifd,
  				struct mkimage_params *params)
  {
@@ -515,7 +517,13 @@ static void imximage_set_header(void *ptr, struct stat *sbuf, int ifd,
  
  	/* Set the imx header */
  	(*set_imx_hdr)(imxhdr, dcd_len, params->ep, imxhdr->flash_offset);
-	*header_size_ptr = sbuf->st_size + imxhdr->flash_offset;
+	/*
+	 * ROM bug alert
+	 * mx53 only loads 512 byte multiples.
+	 * The remaining fraction of a block bytes would
+	 * not be loaded.
+	 */
+	*header_size_ptr = ALIGN(sbuf->st_size + imxhdr->flash_offset, 512);
  }
  
  int imximage_check_params(struct mkimage_params *params)
-- 1.7.9.5

[U-Boot] [PATCH] mx53loco: Fix PMIC name

[Robert Nelson]

On Wed, Jan 2, 2013 at 1:42 PM, Troy Kisky
<troy.kisky@boundarydevices.com> wrote:
> On 1/2/2013 11:33 AM, Fabio Estevam wrote:
>>
>> Thanks for bisecting, Robert. Troy/Stefano, Any suggestions about this? It
>> would be really nice if we could keep mx53loco functional when using gcc
>> 4.6.2. Regards, Fabio Estevam
>
> Could you see if this patch makes any difference? It still needs cleaned up
> some before mainline
> [PATCH V4 01/11] imximage: mx53 needs transfer length a multiple of 512
>
> diff --git a/tools/imximage.c b/tools/imximage.c
> index 63f88b6..7e54e97 100644
> --- a/tools/imximage.c
> +++ b/tools/imximage.c
> @@ -494,6 +494,8 @@ static void imximage_print_header(const void *ptr)
>         }
>  }
>  +#define ALIGN(a, b)   (((a) + (b) - 1) & ~((b) - 1))
> +
>  static void imximage_set_header(void *ptr, struct stat *sbuf, int ifd,
>                                 struct mkimage_params *params)
>  {
> @@ -515,7 +517,13 @@ static void imximage_set_header(void *ptr, struct stat
> *sbuf, int ifd,
>         /* Set the imx header */
>         (*set_imx_hdr)(imxhdr, dcd_len, params->ep, imxhdr->flash_offset);
> -       *header_size_ptr = sbuf->st_size + imxhdr->flash_offset;
> +       /*
> +        * ROM bug alert
> +        * mx53 only loads 512 byte multiples.
> +        * The remaining fraction of a block bytes would
> +        * not be loaded.
> +        */
> +       *header_size_ptr = ALIGN(sbuf->st_size + imxhdr->flash_offset, 512);
>  }
>   int imximage_check_params(struct mkimage_params *params)
> -- 1.7.9.5

Sweet! Thanks Troy,

That fixes it on my Dialog based mx53loco with linaro's 4.7.3 gcc toolchain...

Regards,

-- 
Robert Nelson
http://www.rcn-ee.com/

[U-Boot] [PATCH] mx53loco: Fix PMIC name

[Fabio Estevam]

On Wed, Jan 2, 2013 at 5:58 PM, Robert Nelson <robertcnelson@gmail.com> wrote:

> Sweet! Thanks Troy,
>
> That fixes it on my Dialog based mx53loco with linaro's 4.7.3 gcc toolchain...

Excellent, Troy!

Fixes the issue for me with gcc 4.6.2 as well.

It would be really nice to have this one applied for 2013.01.

Thanks,

Fabio Estevam

[U-Boot] [PATCH] mx53loco: Fix PMIC name

[Stefano Babic]

On 02/01/2013 21:02, Fabio Estevam wrote:
> On Wed, Jan 2, 2013 at 5:58 PM, Robert Nelson <robertcnelson@gmail.com> wrote:
> 
>> Sweet! Thanks Troy,
>>
>> That fixes it on my Dialog based mx53loco with linaro's 4.7.3 gcc toolchain...
> 
> Excellent, Troy!
> 
> Fixes the issue for me with gcc 4.6.2 as well.
> 
> It would be really nice to have this one applied for 2013.01.

Hi all,

the patch was only blocked for Wolfgang's comment regarding the
introduced ALIGN() macro. I think the other doubts regarding issues on
other Socs (i.e., MX6) can be considered closed as well. IMHO it is
enough to rename the macro to ROUND_UP (as Troy have already proposed),
and then I will apply to i.MX tree.

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] [PATCH] mx53loco: Fix PMIC name

[Fabio Estevam]

Hi Stefano,

On Thu, Jan 3, 2013 at 6:53 AM, Stefano Babic <sbabic@denx.de> wrote:

> Hi all,
>
> the patch was only blocked for Wolfgang's comment regarding the
> introduced ALIGN() macro. I think the other doubts regarding issues on
> other Socs (i.e., MX6) can be considered closed as well. IMHO it is
> enough to rename the macro to ROUND_UP (as Troy have already proposed),
> and then I will apply to i.MX tree.

I tried using DIV_ROUND_UP definition from common.h and the board does not boot.

I rename it to ROUND, which matches the definition in common.h.

Just sent the patch.

Regards,

Fabio Estevam

[U-Boot] [PATCH] mx53loco: Fix PMIC name

[Marek Vasut]

Dear Fabio Estevam,

> Hi Stefano,
> 
> On Thu, Jan 3, 2013 at 6:53 AM, Stefano Babic <sbabic@denx.de> wrote:
> > Hi all,
> > 
> > the patch was only blocked for Wolfgang's comment regarding the
> > introduced ALIGN() macro. I think the other doubts regarding issues on
> > other Socs (i.e., MX6) can be considered closed as well. IMHO it is
> > enough to rename the macro to ROUND_UP (as Troy have already proposed),
> > and then I will apply to i.MX tree.
> 
> I tried using DIV_ROUND_UP definition from common.h and the board does not
> boot.
> 
> I rename it to ROUND, which matches the definition in common.h.

Ugh, why can't the stuff from common.h be used ?

> Just sent the patch.
> 
> Regards,
> 
> Fabio Estevam

Best regards,
Marek Vasut

[U-Boot] [PATCH] mx53loco: Fix PMIC name

[Fabio Estevam]

On Thu, Jan 3, 2013 at 11:30 AM, Marek Vasut <marex@denx.de> wrote:

> Ugh, why can't the stuff from common.h be used ?

Yes, I tried it initially, but it throws a lot of errors that I am not
sure that they can be easily fixed:

....
gcc -Wall -Wstrict-prototypes -O2 -fomit-frame-pointer -idirafter
/home/fabio/denx/u-boot-imx/include -idirafter
/home/fabio/denx/u-boot-imx/include2 -idirafter
/home/fabio/denx/u-boot-imx/include -I
/home/fabio/denx/u-boot-imx/lib/libfdt -I
/home/fabio/denx/u-boot-imx/tools -DCONFIG_SYS_TEXT_BASE=0x77800000
-DUSE_HOSTCC -D__KERNEL_STRICT_NAMES   -o imximage.o imximage.c -c
make[1]: Leaving directory `/home/fabio/denx/u-boot-imx/arch/arm/cpu/armv7'
make[2]: Entering directory `/home/fabio/denx/u-boot-imx/tools/kernel-doc'
make[2]: Nothing to be done for `all'.
make[2]: Leaving directory `/home/fabio/denx/u-boot-imx/tools/kernel-doc'
In file included from /home/fabio/denx/u-boot-imx/include/common.h:39:0,
                 from imximage.c:34:
/home/fabio/denx/u-boot-imx/include/linux/bitops.h: In function
?generic_set_bit?:
/home/fabio/denx/u-boot-imx/include/linux/bitops.h:141:23: error:
?BITS_PER_LONG? undeclared (first use in this function)
/home/fabio/denx/u-boot-imx/include/linux/bitops.h:141:23: note: each
undeclared identifier is reported only once for each function it
appears in
/home/fabio/denx/u-boot-imx/include/linux/bitops.h: In function
?generic_clear_bit?:
/home/fabio/denx/u-boot-imx/include/linux/bitops.h:149:23: error:
?BITS_PER_LONG? undeclared (first use in this function)
In file included from /home/fabio/denx/u-boot-imx/include/common.h:174:0,
                 from imximage.c:34:
/home/fabio/denx/u-boot-imx/include/asm/global_data.h: At top level:
/home/fabio/denx/u-boot-imx/include/asm/global_data.h:69:2: error:
unknown type name ?phys_size_t?
In file included from imximage.c:34:0:
/home/fabio/denx/u-boot-imx/include/common.h:270:1: error: unknown
type name ?phys_size_t?
/home/fabio/denx/u-boot-imx/include/common.h:311:1: error: unknown
type name ?u8?
/home/fabio/denx/u-boot-imx/include/common.h:328:14: error: unknown
type name ?cmd_tbl_t?
/home/fabio/denx/u-boot-imx/include/common.h:331:19: error: unknown
type name ?cmd_tbl_t?
/home/fabio/denx/u-boot-imx/include/common.h:334:17: error: unknown
type name ?cmd_tbl_t?
/home/fabio/denx/u-boot-imx/include/common.h:344:5: error: conflicting
types for ?setenv?
/usr/include/stdlib.h:585:12: note: previous declaration of ?setenv? was here
/home/fabio/denx/u-boot-imx/include/common.h:562:1: error: unknown
type name ?u32?
In file included from imximage.c:34:0:
/home/fabio/denx/u-boot-imx/include/common.h:787:5: error: conflicting
types for ?_IO_getc?
/usr/include/libio.h:462:12: note: previous declaration of ?_IO_getc? was here
/home/fabio/denx/u-boot-imx/include/common.h:791:23: error: macro
"putc" requires 2 arguments, but only 1 given
/home/fabio/denx/u-boot-imx/include/common.h:791:6: error: ?putc?
redeclared as different kind of symbol
/home/fabio/denx/u-boot-imx/include/common.h:792:6: error: conflicting
types for ?puts?
/home/fabio/denx/u-boot-imx/include/common.h:810:5: error: conflicting
types for ?fprintf?
/home/fabio/denx/u-boot-imx/include/common.h:812:6: error: conflicting
types for ?fputs?
/home/fabio/denx/u-boot-imx/include/common.h:813:6: error: conflicting
types for ?fputc?
/home/fabio/denx/u-boot-imx/include/common.h:815:5: error: conflicting
types for ?fgetc?
/usr/include/stdio.h:537:12: note: previous declaration of ?fgetc? was here
In file included from /home/fabio/denx/u-boot-imx/include/common.h:825:0,
                 from imximage.c:34:
/home/fabio/denx/u-boot-imx/include/net.h:42:1: error: unknown type name ?u32?
/home/fabio/denx/u-boot-imx/include/net.h:632:1: error: unknown type name ?u8?
/home/fabio/denx/u-boot-imx/include/net.h:644:1: error: unknown type name ?u8?
/home/fabio/denx/u-boot-imx/include/net.h:655:1: error: unknown type name ?u8?
/home/fabio/denx/u-boot-imx/include/net.h:670:1: error: unknown type name ?u8?
make[1]: *** [imximage.o] Error 1
make[1]: *** Waiting for unfinished jobs....
make[1]: Leaving directory `/home/fabio/denx/u-boot-imx/tools'
make: *** [tools] Error 2

Regards,

Fabio Estevam

[U-Boot] [PATCH] mx53loco: Fix PMIC name

[Marek Vasut]

Dear Fabio Estevam,

> On Thu, Jan 3, 2013 at 11:30 AM, Marek Vasut <marex@denx.de> wrote:
> > Ugh, why can't the stuff from common.h be used ?
> 
> Yes, I tried it initially, but it throws a lot of errors that I am not
> sure that they can be easily fixed:

Only with #include <common.h>  ?

> Regards,
> 
> Fabio Estevam

Best regards,
Marek Vasut

[U-Boot] [PATCH] mx53loco: Fix PMIC name

[Stefano Babic]

On 03/01/2013 14:25, Fabio Estevam wrote:
> Hi Stefano,
> 
> On Thu, Jan 3, 2013 at 6:53 AM, Stefano Babic <sbabic@denx.de> wrote:
> 
>> Hi all,
>>
>> the patch was only blocked for Wolfgang's comment regarding the
>> introduced ALIGN() macro. I think the other doubts regarding issues on
>> other Socs (i.e., MX6) can be considered closed as well. IMHO it is
>> enough to rename the macro to ROUND_UP (as Troy have already proposed),
>> and then I will apply to i.MX tree.
> 
> I tried using DIV_ROUND_UP definition from common.h and the board does not boot.
> 
> I rename it to ROUND, which matches the definition in common.h.

I agree with this solution - I do not like that common.h is included,
because _config must run and tools should be independent from the
selected board.


> 
> Just sent the patch.

Then I apply it !

Thanks,
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] [PATCH] mx53loco: Fix PMIC name

[Stefano Babic]

On 03/01/2013 14:49, Marek Vasut wrote:
> Dear Fabio Estevam,
> 

Hi,

>> On Thu, Jan 3, 2013 at 11:30 AM, Marek Vasut <marex@denx.de> wrote:
>>> Ugh, why can't the stuff from common.h be used ?
>>
>> Yes, I tried it initially, but it throws a lot of errors that I am not
>> sure that they can be easily fixed:
> 
> Only with #include <common.h>  ?

I will remark a previous comment - even if including common.h seems a
good idea to avoid duplications, it makes tools like mkimage to depend
on the selected board, because <board>_config must run. Even if this is
not a problem for us u-boot developers, it becomes an issue when these
tools are included in distros (like u-boot-tools in Ubuntu) and cannot
be packaged.

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] [PATCH] mx53loco: Fix PMIC name

[Fabio Estevam]

On Thu, Jan 3, 2013 at 11:49 AM, Marek Vasut <marex@denx.de> wrote:
> Dear Fabio Estevam,
>
>> On Thu, Jan 3, 2013 at 11:30 AM, Marek Vasut <marex@denx.de> wrote:
>> > Ugh, why can't the stuff from common.h be used ?
>>
>> Yes, I tried it initially, but it throws a lot of errors that I am not
>> sure that they can be easily fixed:
>
> Only with #include <common.h>  ?

Yes, correct.

[U-Boot] [PATCH] mx53loco: Fix PMIC name

[Troy Kisky]

On 1/3/2013 7:07 AM, Stefano Babic wrote:
> On 03/01/2013 14:49, Marek Vasut wrote:
>> Dear Fabio Estevam,
>>
> Hi,
>
>>> On Thu, Jan 3, 2013 at 11:30 AM, Marek Vasut <marex@denx.de> wrote:
>>>> Ugh, why can't the stuff from common.h be used ?
>>> Yes, I tried it initially, but it throws a lot of errors that I am not
>>> sure that they can be easily fixed:
>> Only with #include <common.h>  ?
> I will remark a previous comment - even if including common.h seems a
> good idea to avoid duplications, it makes tools like mkimage to depend
> on the selected board, because <board>_config must run. Even if this is
> not a problem for us u-boot developers, it becomes an issue when these
> tools are included in distros (like u-boot-tools in Ubuntu) and cannot
> be packaged.
>
> Best regards,
> Stefano Babic
>
It would be good to include this in commit log if not already committed.


Thanks
Troy

[U-Boot] [PATCH] mx53loco: Fix PMIC name

[Fabio Estevam]

On Thu, Jan 3, 2013 at 4:18 PM, Troy Kisky
<troy.kisky@boundarydevices.com> wrote:

> It would be good to include this in commit log if not already committed.

Ok, just sent a v2 with such comment.

Regards,

Fabio Estevam