diff mbox

[U-Boot] mx53loco: Fix PMIC name

Message ID 50E48D98.9020609@boundarydevices.com
State Not Applicable
Headers show

Commit Message

Troy Kisky Jan. 2, 2013, 7:42 p.m. UTC 
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

-- 1.7.9.5

Comments

Robert Nelson Jan. 2, 2013, 7:58 p.m. UTC | #1 
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,
Fabio Estevam Jan. 2, 2013, 8:02 p.m. UTC | #2 
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
Stefano Babic Jan. 3, 2013, 8:53 a.m. UTC | #3 
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
Fabio Estevam Jan. 3, 2013, 1:25 p.m. UTC | #4 
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
Marek Vasut Jan. 3, 2013, 1:30 p.m. UTC | #5 
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
Fabio Estevam Jan. 3, 2013, 1:35 p.m. UTC | #6 
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
Marek Vasut Jan. 3, 2013, 1:49 p.m. UTC | #7 
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
Stefano Babic Jan. 3, 2013, 2:02 p.m. UTC | #8 
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
Stefano Babic Jan. 3, 2013, 2:07 p.m. UTC | #9 
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
Fabio Estevam Jan. 3, 2013, 2:30 p.m. UTC | #10 
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.
Troy Kisky Jan. 3, 2013, 6:18 p.m. UTC | #11 
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
Fabio Estevam Jan. 3, 2013, 6:26 p.m. UTC | #12 
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
diff mbox

Patch

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)