Re: [PATCH 5/4] mkimage: update man page and -h output
On 07/11/2023 08.30, Rasmus Villemoes wrote: > I'll send a revert to Tom for the prematurely applied fixup. Oh, I see that's already done. Good. Rasmus
Re: [PATCH 5/4] mkimage: update man page and -h output
On 07/11/2023 01.46, Sean Anderson wrote: > On 10/13/23 14:30, Rasmus Villemoes wrote: >> On 12/10/2023 04.17, Sean Anderson wrote: >> >>> I was hoping you would respond to my most-recent email regarding this >>> series. >>> In particular: >>> >>> | Why does mkimage have to do this? Can't you just use truncate or, in a >>> | binman context, align-size? >> >> In both cases, that just affects the size of the file, but does not >> affect the totalsize field in the fdt header. > > Use `dtc -a` then. Not possible, when the generator is binman which calls mkimage which calls dtc. If I was using dtc directly, sure. Anyway, I currently have more important things to deal with than this. Simon, please drop the series. I'll send a revert to Tom for the prematurely applied fixup. Rasmus
Re: [PATCH 5/4] mkimage: update man page and -h output
On 10/13/23 14:30, Rasmus Villemoes wrote: On 12/10/2023 04.17, Sean Anderson wrote: I was hoping you would respond to my most-recent email regarding this series. In particular: | Why does mkimage have to do this? Can't you just use truncate or, in a | binman context, align-size? In both cases, that just affects the size of the file, but does not affect the totalsize field in the fdt header. Use `dtc -a` then. Wrt. binman, just as I was somewhat misled by the short mkimage -h output into thinking that this already worked as I expected, binman's fit,align documentation can also be read that way - and the _implementation_ certainly currently unconditionally translates a fit,align property into a -B argument to mkimage. So if we don't want mkimage to support a -B by itself, binman documentation and code would probably need another patch to reject that as well. Yeah, that's a good idea. --Sean
Re: [PATCH 5/4] mkimage: update man page and -h output
On 12/10/2023 04.17, Sean Anderson wrote: > I was hoping you would respond to my most-recent email regarding this > series. > In particular: > > | Why does mkimage have to do this? Can't you just use truncate or, in a > | binman context, align-size? In both cases, that just affects the size of the file, but does not affect the totalsize field in the fdt header. Wrt. binman, just as I was somewhat misled by the short mkimage -h output into thinking that this already worked as I expected, binman's fit,align documentation can also be read that way - and the _implementation_ certainly currently unconditionally translates a fit,align property into a -B argument to mkimage. So if we don't want mkimage to support a -B by itself, binman documentation and code would probably need another patch to reject that as well. Rasmus
Re: [PATCH 5/4] mkimage: update man page and -h output
On Wed, Oct 11, 2023 at 10:17:50PM -0400, Sean Anderson wrote: > Hi Rasmus, > > On 10/11/23 15:07, Rasmus Villemoes wrote: > > On 11/10/2023 20.37, Tom Rini wrote: > > > On Thu, Sep 28, 2023 at 10:02:57AM +0200, Rasmus Villemoes wrote: > > > > > > > The man page correctly said that -B was ignored without -E, while the > > > > `mkimage -h` output suggested otherwise. Now that -B can actually be > > > > used by itself, update the man page. > > > > > > > > While at it, also amend the `mkimage -h` line to mention the > > > > connection with -E. > > > > > > > > The FDT header is a fixed 40 bytes, so its size cannot (and is not) > > > > modified, while its alignment is a property of the address in RAM one > > > > loads the FIT to, so not something mkimage can affect in any way. (In > > > > the file itself, the header is of course at offset 0, which has all > > > > possible alignments already.) > > > > > > > > Reported-by: Sean Anderson > > > > Signed-off-by: Rasmus Villemoes > > > > Reviewed-by: Simon Glass > > > > > > Applied to u-boot/master, thanks! > > > > > > > Thanks, but I'm afraid that was premature. > > > > The original series which this was a fixup/followup for hasn't been > > applied, and Sean had reservations. I'm leaving it to Simon or Tom or > > whoever has final say to decide if they should eventually go in, but it > > would probably be good to get a verdict soonish (it really shouldn't be > > too controversial), and if it's a no, this should just be reverted. > > I was hoping you would respond to my most-recent email regarding this series. > In particular: > > | Why does mkimage have to do this? Can't you just use truncate or, in a > | binman context, align-size? > > Presumably you have some reason for wanting this in mkimage rather than using > existing tooling. I think part of the fun of trying to figure out how to have the dtb in u-boot be properly (8 byte) aligned was that existing command wise, something portable is tricky? -- Tom signature.asc Description: PGP signature
Re: [PATCH 5/4] mkimage: update man page and -h output
Hi, On Wed, 11 Oct 2023 at 12:33, Tom Rini wrote: > > On Wed, Oct 11, 2023 at 09:07:11PM +0200, Rasmus Villemoes wrote: > > On 11/10/2023 20.37, Tom Rini wrote: > > > On Thu, Sep 28, 2023 at 10:02:57AM +0200, Rasmus Villemoes wrote: > > > > > >> The man page correctly said that -B was ignored without -E, while the > > >> `mkimage -h` output suggested otherwise. Now that -B can actually be > > >> used by itself, update the man page. > > >> > > >> While at it, also amend the `mkimage -h` line to mention the > > >> connection with -E. > > >> > > >> The FDT header is a fixed 40 bytes, so its size cannot (and is not) > > >> modified, while its alignment is a property of the address in RAM one > > >> loads the FIT to, so not something mkimage can affect in any way. (In > > >> the file itself, the header is of course at offset 0, which has all > > >> possible alignments already.) > > >> > > >> Reported-by: Sean Anderson > > >> Signed-off-by: Rasmus Villemoes > > >> Reviewed-by: Simon Glass > > > > > > Applied to u-boot/master, thanks! > > > > > > > Thanks, but I'm afraid that was premature. > > > > The original series which this was a fixup/followup for hasn't been > > applied, and Sean had reservations. I'm leaving it to Simon or Tom or > > whoever has final say to decide if they should eventually go in, but it > > would probably be good to get a verdict soonish (it really shouldn't be > > too controversial), and if it's a no, this should just be reverted. > > Ugh, thanks. I'll push the revert with the next round of changes. My intent was to apply it, but I haven't got to it yet. Regards, Simon
Re: [PATCH 5/4] mkimage: update man page and -h output
Hi Rasmus, On 10/11/23 15:07, Rasmus Villemoes wrote: On 11/10/2023 20.37, Tom Rini wrote: On Thu, Sep 28, 2023 at 10:02:57AM +0200, Rasmus Villemoes wrote: The man page correctly said that -B was ignored without -E, while the `mkimage -h` output suggested otherwise. Now that -B can actually be used by itself, update the man page. While at it, also amend the `mkimage -h` line to mention the connection with -E. The FDT header is a fixed 40 bytes, so its size cannot (and is not) modified, while its alignment is a property of the address in RAM one loads the FIT to, so not something mkimage can affect in any way. (In the file itself, the header is of course at offset 0, which has all possible alignments already.) Reported-by: Sean Anderson Signed-off-by: Rasmus Villemoes Reviewed-by: Simon Glass Applied to u-boot/master, thanks! Thanks, but I'm afraid that was premature. The original series which this was a fixup/followup for hasn't been applied, and Sean had reservations. I'm leaving it to Simon or Tom or whoever has final say to decide if they should eventually go in, but it would probably be good to get a verdict soonish (it really shouldn't be too controversial), and if it's a no, this should just be reverted. I was hoping you would respond to my most-recent email regarding this series. In particular: | Why does mkimage have to do this? Can't you just use truncate or, in a | binman context, align-size? Presumably you have some reason for wanting this in mkimage rather than using existing tooling. --Sean
Re: [PATCH 5/4] mkimage: update man page and -h output
On Wed, Oct 11, 2023 at 09:07:11PM +0200, Rasmus Villemoes wrote: > On 11/10/2023 20.37, Tom Rini wrote: > > On Thu, Sep 28, 2023 at 10:02:57AM +0200, Rasmus Villemoes wrote: > > > >> The man page correctly said that -B was ignored without -E, while the > >> `mkimage -h` output suggested otherwise. Now that -B can actually be > >> used by itself, update the man page. > >> > >> While at it, also amend the `mkimage -h` line to mention the > >> connection with -E. > >> > >> The FDT header is a fixed 40 bytes, so its size cannot (and is not) > >> modified, while its alignment is a property of the address in RAM one > >> loads the FIT to, so not something mkimage can affect in any way. (In > >> the file itself, the header is of course at offset 0, which has all > >> possible alignments already.) > >> > >> Reported-by: Sean Anderson > >> Signed-off-by: Rasmus Villemoes > >> Reviewed-by: Simon Glass > > > > Applied to u-boot/master, thanks! > > > > Thanks, but I'm afraid that was premature. > > The original series which this was a fixup/followup for hasn't been > applied, and Sean had reservations. I'm leaving it to Simon or Tom or > whoever has final say to decide if they should eventually go in, but it > would probably be good to get a verdict soonish (it really shouldn't be > too controversial), and if it's a no, this should just be reverted. Ugh, thanks. I'll push the revert with the next round of changes. -- Tom signature.asc Description: PGP signature
Re: [PATCH 5/4] mkimage: update man page and -h output
On 11/10/2023 20.37, Tom Rini wrote: > On Thu, Sep 28, 2023 at 10:02:57AM +0200, Rasmus Villemoes wrote: > >> The man page correctly said that -B was ignored without -E, while the >> `mkimage -h` output suggested otherwise. Now that -B can actually be >> used by itself, update the man page. >> >> While at it, also amend the `mkimage -h` line to mention the >> connection with -E. >> >> The FDT header is a fixed 40 bytes, so its size cannot (and is not) >> modified, while its alignment is a property of the address in RAM one >> loads the FIT to, so not something mkimage can affect in any way. (In >> the file itself, the header is of course at offset 0, which has all >> possible alignments already.) >> >> Reported-by: Sean Anderson >> Signed-off-by: Rasmus Villemoes >> Reviewed-by: Simon Glass > > Applied to u-boot/master, thanks! > Thanks, but I'm afraid that was premature. The original series which this was a fixup/followup for hasn't been applied, and Sean had reservations. I'm leaving it to Simon or Tom or whoever has final say to decide if they should eventually go in, but it would probably be good to get a verdict soonish (it really shouldn't be too controversial), and if it's a no, this should just be reverted. Rasmus
Re: [PATCH 5/4] mkimage: update man page and -h output
On Thu, Sep 28, 2023 at 10:02:57AM +0200, Rasmus Villemoes wrote: > The man page correctly said that -B was ignored without -E, while the > `mkimage -h` output suggested otherwise. Now that -B can actually be > used by itself, update the man page. > > While at it, also amend the `mkimage -h` line to mention the > connection with -E. > > The FDT header is a fixed 40 bytes, so its size cannot (and is not) > modified, while its alignment is a property of the address in RAM one > loads the FIT to, so not something mkimage can affect in any way. (In > the file itself, the header is of course at offset 0, which has all > possible alignments already.) > > Reported-by: Sean Anderson > Signed-off-by: Rasmus Villemoes > Reviewed-by: Simon Glass Applied to u-boot/master, thanks! -- Tom signature.asc Description: PGP signature
Re: [PATCH 5/4] mkimage: update man page and -h output
On Thu, 28 Sept 2023 at 02:03, Rasmus Villemoes wrote: > > The man page correctly said that -B was ignored without -E, while the > `mkimage -h` output suggested otherwise. Now that -B can actually be > used by itself, update the man page. > > While at it, also amend the `mkimage -h` line to mention the > connection with -E. > > The FDT header is a fixed 40 bytes, so its size cannot (and is not) > modified, while its alignment is a property of the address in RAM one > loads the FIT to, so not something mkimage can affect in any way. (In > the file itself, the header is of course at offset 0, which has all > possible alignments already.) > > Reported-by: Sean Anderson > Signed-off-by: Rasmus Villemoes > --- > doc/mkimage.1 | 6 -- > tools/mkimage.c | 2 +- > 2 files changed, 5 insertions(+), 3 deletions(-) Reviewed-by: Simon Glass Strange that your patch says 5/4
[PATCH 5/4] mkimage: update man page and -h output
The man page correctly said that -B was ignored without -E, while the `mkimage -h` output suggested otherwise. Now that -B can actually be used by itself, update the man page. While at it, also amend the `mkimage -h` line to mention the connection with -E. The FDT header is a fixed 40 bytes, so its size cannot (and is not) modified, while its alignment is a property of the address in RAM one loads the FIT to, so not something mkimage can affect in any way. (In the file itself, the header is of course at offset 0, which has all possible alignments already.) Reported-by: Sean Anderson Signed-off-by: Rasmus Villemoes --- doc/mkimage.1 | 6 -- tools/mkimage.c | 2 +- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/doc/mkimage.1 b/doc/mkimage.1 index 76c7859bb0..1a4bc25936 100644 --- a/doc/mkimage.1 +++ b/doc/mkimage.1 @@ -281,8 +281,10 @@ properties. A \(oqdata-offset\(cq of 0 indicates that it starts in the first .BI \-B " alignment" .TQ .BI \-\-alignment " alignment" -The alignment, in hexadecimal, that external data will be aligned to. This -option only has an effect when \-E is specified. +The alignment, in hexadecimal, that the FDT structure will be aligned +to. With +.BR \-E , +also specifies the alignment for the external data. . .TP .BI \-p " external-position" diff --git a/tools/mkimage.c b/tools/mkimage.c index 6dfe3e1d42..a5979fa6fd 100644 --- a/tools/mkimage.c +++ b/tools/mkimage.c @@ -112,7 +112,7 @@ static void usage(const char *msg) " -f => input filename for FIT source\n" " -i => input filename for ramdisk file\n" " -E => place data outside of the FIT structure\n" - " -B => align size in hex for FIT structure and header\n" + " -B => align size in hex for FIT structure and, with -E, for the external data\n" " -b => append the device tree binary to the FIT\n" " -t => update the timestamp in the FIT\n"); #ifdef CONFIG_FIT_SIGNATURE -- 2.37.2
