Re: [U-Boot] [PATCH V4 01/11] imximage: mx53 needs transfer length a multiple of 512
On 28/11/2012 22:35, Wolfgang Denk wrote: Dear Troy Kisky, In message 50b67c99.8080...@boundarydevices.com you wrote: But the reason I didn't include common.h is because of the target specific files that it also includes. Would you mind if I moved Why would these hurt? They don't anywhere else. Personally, I think that mkimage as generic tool should not include common.h. Doing that, it does not allow to compile mkimage without running config, and let's think that we need a different mkimage for each target, and that is not true. This will break also support from distros, because their packages (for example, u-boot-tools, uboot-mkimage under Ubuntu) are compiled without configuring u-boot - and I think it is correct. IMHO we are discussing about a single macro. We can let it in mkimage as in patch and move it in a general file only if we will have a use case with a bunch of macros. 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: sba...@denx.de = ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH V4 01/11] imximage: mx53 needs transfer length a multiple of 512
On 29/11/2012 06:28, Wolfgang Denk wrote: Dear Troy Kisky, In message 50b6cb79.4030...@boundarydevices.com you wrote: Would you like to see the Linux way of ALIGN, or ROUND? Do you align some buffer or similar, or do you round (up) a size? Now, back to the other topic you raised. Should I apply the bug work-around for all version 2 headers, or find a way to distinguish mx53/mx6? I cannot tell. Stefano, what do you think? Well, I am thinking about which are the real benefits. If we always round up the size to 512 bytes for V2 header, *maybe* we constrain the i.MX6 to load some bytes more, but it is the only drawback. And this if the i.MX6 does not suffer of the same problem found on i.MX53. On the other side, having two different versions of V2 header is confusing. It is then undocumented by Freescale, and maybe it is possible to find a note in some errata. Having the same interface without special hacking for each SOC overcomes the increment in the footprint for the i.MX6 (in worst case 511 bytes - and not a lot compared to current size of U-Boot for MX5/MX6, usually several hundred of KB). 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: sba...@denx.de = ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH V4 01/11] imximage: mx53 needs transfer length a multiple of 512
Dear Troy Kisky, In message 1354066303-29762-2-git-send-email-troy.ki...@boundarydevices.com you wrote: The mx53 ROM will truncate the length at a multiple of 512. Transferring too much is not a problem, so round up. What about other SoCs using the same code? +#define ALIGN(a, b) (((a) + (b) - 1) ~((b) - 1)) NAK. This macro is mis-named; it has nothing to do with alignment - you write yourself: round up. And you don't have to re-invent the wheel. Please use the existing macros for this purpose. Best regards, Wolfgang Denk -- DENX Software Engineering GmbH, MD: Wolfgang Denk Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de If something is different, it's either better or worse, and usually both.- Larry Wall ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH V4 01/11] imximage: mx53 needs transfer length a multiple of 512
-Original Message-
From: Troy Kisky [mailto:troy.ki...@boundarydevices.com]
Sent: Wednesday, November 28, 2012 9:32 AM
To: sba...@denx.de
Cc: dirk.be...@googlemail.com; u-boot@lists.denx.de; Liu Hui-R64343;
feste...@gmail.com; Troy Kisky
Subject: [PATCH V4 01/11] imximage: mx53 needs transfer length a multiple
of 512
The mx53 ROM will truncate the length at a multiple of 512.
Transferring too much is not a problem, so round up.
Problem reported by Stefano Babic.
Signed-off-by: Troy Kisky troy.ki...@boundarydevices.com
Acked-by: Jason Liu r64...@freescale.com
---
tools/imximage.c | 10 +-
1 file changed, 9 insertions(+), 1 deletion(-)
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 mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH V4 01/11] imximage: mx53 needs transfer length a multiple of 512
On 11/28/2012 2:27 AM, Wolfgang Denk wrote: Dear Troy Kisky, In message 1354066303-29762-2-git-send-email-troy.ki...@boundarydevices.com you wrote: The mx53 ROM will truncate the length at a multiple of 512. Transferring too much is not a problem, so round up. What about other SoCs using the same code? +#define ALIGN(a, b)(((a) + (b) - 1) ~((b) - 1)) NAK. This macro is mis-named; it has nothing to do with alignment - you write yourself: round up. And you don't have to re-invent the wheel. Please use the existing macros for this purpose. Best regards, Wolfgang Denk Oddly enough, I originally called it ROUND_UP. But then I saw these lines in include/common.h #define ALIGN(x,a) __ALIGN_MASK((x),(typeof(x))(a)-1) #define __ALIGN_MASK(x,mask)(((x)+(mask))~(mask)) So, I deleted my definition of ROUND_UP and used ALIGN. But imximage.c did not automatically include common.h. Instead of trying to include common.h and all the files it pulled in, I added the ALIGN definition. Troy ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH V4 01/11] imximage: mx53 needs transfer length a multiple of 512
On 11/28/2012 2:27 AM, Wolfgang Denk wrote: Dear Troy Kisky, In message 1354066303-29762-2-git-send-email-troy.ki...@boundarydevices.com you wrote: The mx53 ROM will truncate the length at a multiple of 512. Transferring too much is not a problem, so round up. What about other SoCs using the same code? It would be easy to add a version 2 header test, but that would not distinguish between mx53 and mx6. Should I create a version 2bug header that I can specify in mx53's config file? Thanks Troy ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH V4 01/11] imximage: mx53 needs transfer length a multiple of 512
Dear Troy Kisky, In message 50b65583.1070...@boundarydevices.com you wrote: Oddly enough, I originally called it ROUND_UP. But then I saw these lines in include/common.h And why didn't you find (and use) ROUND() in include/common.h ? Best regards, Wolfgang Denk -- DENX Software Engineering GmbH, MD: Wolfgang Denk Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de My challenge to the goto-less programmer is to recode tcp_input() without any gotos ... without any loss of efficiency (there has to be a catch). - W. R. Stevens ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH V4 01/11] imximage: mx53 needs transfer length a multiple of 512
On 11/28/2012 1:25 PM, Wolfgang Denk wrote: Dear Troy Kisky, In message 50b65583.1070...@boundarydevices.com you wrote: Oddly enough, I originally called it ROUND_UP. But then I saw these lines in include/common.h And why didn't you find (and use) ROUND() in include/common.h ? Best regards, Wolfgang Denk I did also find ROUND, so I checked to see what Linux did. Linux does not have ROUND, but it does have ALIGN. But I personally prefer ROUND, or even better ROUND_UP. I just wanted to use the most common form. u-boot seems to use ROUND in config files and ALIGN in .c files But the reason I didn't include common.h is because of the target specific files that it also includes. Would you mind if I moved _ #define ARRAY_SIZE(x) (sizeof(x) / sizeof((x)[0])) #define ROUND(a,b) (((a) + (b) - 1) ~((b) - 1)) #define DIV_ROUND(n,d) (((n) + ((d)/2)) / (d)) #define DIV_ROUND_UP(n,d) (((n) + (d) - 1) / (d)) #define roundup(x, y) x) + ((y) - 1)) / (y)) * (y)) #define ALIGN(x,a) __ALIGN_MASK((x),(typeof(x))(a)-1) #define __ALIGN_MASK(x,mask)(((x)+(mask))~(mask)) - from common.h to a new file common_macro.h and included common_macro.h instead? Perhaps you have a better alternative? Thanks Troy ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH V4 01/11] imximage: mx53 needs transfer length a multiple of 512
Dear Troy Kisky, In message 50b67c99.8080...@boundarydevices.com you wrote: But the reason I didn't include common.h is because of the target specific files that it also includes. Would you mind if I moved Why would these hurt? They don't anywhere else. and included common_macro.h instead? I see no benefit for that. Best regards, Wolfgang Denk -- DENX Software Engineering GmbH, MD: Wolfgang Denk Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de History is only a confused heap of facts. -- Philip Earl of Chesterfield ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH V4 01/11] imximage: mx53 needs transfer length a multiple of 512
On 11/28/2012 2:35 PM, Wolfgang Denk wrote: Dear Troy Kisky, In message 50b67c99.8080...@boundarydevices.com you wrote: But the reason I didn't include common.h is because of the target specific files that it also includes. Would you mind if I moved Why would these hurt? They don't anywhere else. I'm not saying that including common.h wouldn't work. I'm saying that it seems wrong to include target specific include files in an executable that should generate the same code regardless of the target selected. I really don't care enough to argue. I just want you to understand why I did it the way I did. It wasn't because I was crazy, or lazy. We just hold different priorities. Would you like to see the Linux way of ALIGN, or ROUND? Now, back to the other topic you raised. Should I apply the bug work-around for all version 2 headers, or find a way to distinguish mx53/mx6? Thanks Troy ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH V4 01/11] imximage: mx53 needs transfer length a multiple of 512
Dear Troy Kisky, In message 50b6cb79.4030...@boundarydevices.com you wrote: Would you like to see the Linux way of ALIGN, or ROUND? Do you align some buffer or similar, or do you round (up) a size? Now, back to the other topic you raised. Should I apply the bug work-around for all version 2 headers, or find a way to distinguish mx53/mx6? I cannot tell. Stefano, what do you think? Best regards, Wolfgang Denk -- DENX Software Engineering GmbH, MD: Wolfgang Denk Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de Don't think; let the machine do it for you!- E. C. Berkeley ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] [PATCH V4 01/11] imximage: mx53 needs transfer length a multiple of 512
The mx53 ROM will truncate the length at a multiple of 512.
Transferring too much is not a problem, so round up.
Problem reported by Stefano Babic.
Signed-off-by: Troy Kisky troy.ki...@boundarydevices.com
---
tools/imximage.c | 10 +-
1 file changed, 9 insertions(+), 1 deletion(-)
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 mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
