Implement the -R option and add a -S option.
PATCH 1/1 Implement the -R option and add a -S option.
Implement the -R <number> option to add memory reserve slots. Add a -S <size> option makes the blob at least this number of bytes.
--- dtc.c | 19 ++++++++++++++++--- dtc.h | 6 ++++-- flattree.c | 22 ++++++++++++++++++++-- 3 files changed, 40 insertions(+), 7 deletions(-)
dtc.c +16 -3
@@ -21,6 +21,13 @@#include "dtc.h" #include "srcpos.h" +/* +* Command line options +*/ +int quiet;/* Level of quietness */ +int reservenum;/* Number of memory reservation slots */ +int minsize;/* Minimum blob size */ +char *join_path (char *path,char *name) {int lenp =strlen (path);
@@ -85,6 +92,8 @@ static void usage(void)fprintf (stderr,"\t\tBlob version to produce, defaults to %d (relevant for dtb\n\t\tand asm output only)\n" ,OF_DEFAULT_VERSION );fprintf (stderr,"\t-R <number>\n" );fprintf (stderr,"\t\tMake space for <number> reserve map entries (relevant for \n\t\tdtb and asm output only)\n" ); +fprintf (stderr,"\t-S <bytes>\n" ); +fprintf (stderr,"\t\tMake the blob at least <bytes> long (extra space)\n" );fprintf (stderr,"\t-b <number>\n" );fprintf (stderr,"\t\tSet the physical boot cpu\n" );fprintf (stderr,"\t-f\n" );
@@ -104,12 +113,13 @@ int main(int argc, char *argv[])FILE *inf =NULL ;FILE *outf =NULL ;int outversion =OF_DEFAULT_VERSION ; -int reservenum =1 ;int boot_cpuid_phys =0xfeedbeef ; - quiet =0 ; + quiet =0 ; + reservenum =0 ; + minsize =0 ; -while ((opt =getopt (argc, argv,"hI:O:o:V:R:fqb:" )) !=EOF ) { +while ((opt =getopt (argc, argv,"hI:O:o:V:R:S:fqb:" )) !=EOF ) {switch (opt) {case 'I' : inform = optarg;
@@ -126,6 +136,9 @@ int main(int argc, char *argv[])case 'R' : reservenum =strtol (optarg,NULL ,0 );break ; +case 'S' : + minsize =strtol (optarg,NULL ,0 ); +break ;case 'f' : force =1 ;break ;
dtc.h +4 -2
@@ -37,9 +37,11 @@#include "flat_dt.h" /* -* Level of quietness +* Command line options */ -int quiet; +extern int quiet;/* Level of quietness */ +extern int reservenum;/* Number of memory reservation slots */ +extern int minsize;/* Minimum blob size */ static inline void die (char * str, ...) {
flattree.c +20 -2
@@ -295,10 +295,18 @@ static struct data flatten_reserve_list(struct reserve_info *reservelist, {struct reserve_info *re;struct data d = empty_data; +static struct reserve_entry null_re = {0 ,0 }; +int j;for (re = reservelist; re; re = re->next) { d =data_append_re (d, &re->re); } +/* +* Add additional reserved slots if the user asked for them. +*/ +for (j =0 ; j < reservenum; j++) { + d =data_append_re (d, &null_re); + }return d; }
@@ -324,8 +332,18 @@ static void make_bph(struct boot_param_header *bph, bph->off_dt_struct =cpu_to_be32 (reserve_off + reservesize); bph->off_dt_strings =cpu_to_be32 (reserve_off + reservesize + dtsize); - bph->totalsize =cpu_to_be32 (reserve_off + reservesize - + dtsize + strsize); + bph->totalsize = reserve_off + reservesize + dtsize + strsize; +if (minsize >0 ) { +if (bph->totalsize >= minsize) { +if (quiet <1 ) +fprintf (stderr, +"Warning: blob size %d >= minimum size %d\n" , + bph->totalsize, minsize); + + }else + bph->totalsize = minsize; + } + bph->totalsize =cpu_to_be32 (bph->totalsize);if (vi->flags &FTF_BOOTCPUID ) bph->boot_cpuid_phys =cpu_to_be32 (boot_cpuid_phys);
Signature · 1 line
1.4.4.4
Jon Loeliger
AppliedSo, like, the other day Jerry Van Baren mumbled:
Quoted 4 lines
Implement the -R <number> option to add memory reserve slots. Add a -S <size> option makes the blob at least this number of bytes.
Signed-off-by Gerald Van Baren <[email protected]>
Applied.
Thanks,
jdl
Quoted 10 lines · Jon Loeliger
So, like, the other day Jerry Van Baren mumbled:
Quoted 4 lines
Implement the -R <number> option to add memory reserve slots. Add a -S <size> option makes the blob at least this number of bytes.
Signed-off-by Gerald Van Baren <[email protected]>Applied.
Thanks,
jdl
Thanks, Jon. That makes the new u-boot "fdt" command tremendously easier to use because you can now allocate extra space in the blob at compile time rather than having to reallocate space using the u-boot "fdt" command.
Note to u-boot "fdt" command users:
* Pull the latest dtc source from Jon's repo
<http://www.jdl.com/git_repos/>
* When you compile your dts, use the new -S parameter to create some extra space (e.g. -S 0x2000). If you don't want to guess how big your current blob is, run a test compile with -S 1 - you will get a warning (don't use -q ;-) that will tell you how big your blob is. Re-run with a reasonable -S.
* The -R to make extra mem reserve slots may be useful too.
* Make sure you *don't* specify version 16 (some legacy recipes do that). The latest version is 17 and dtc now Does The Right Thing without needing the version specified. Do *not* use -V.
Best regards,
gvb
Quoted 2 lines · Jerry Van Baren
Implement the -R <number> option to add memory reserve slots. Add a -S <size> option makes the blob at least this number of bytes.
Wouldn't it be better to just specify the amount of extra space, instead of the minimum total space? That way, you only need to know what you intend to add, not how much is already there.
-Scott
Quoted 9 lines · Scott Wood
Quoted 2 lines · Jerry Van Baren
Implement the -R <number> option to add memory reserve slots. Add a -S <size> option makes the blob at least this number of bytes.
Wouldn't it be better to just specify the amount of extra space, instead of the minimum total space? That way, you only need to know what you intend to add, not how much is already there.
-Scott
I thought briefly about this, but decided to implement a fixed size so that someone could allocate, say, 8K of memory in their memory map (likely flash) and know their blob would fit.
Maybe we need a little -s option to say "add -s bytes".
Jon suggested a --stats option to print out the important statistics. That would also be a good enhancement.
gvb
Quoted 9 lines · Jerry Van Baren
Quoted 8 lines · Scott Wood
Quoted 2 lines · Jerry Van Baren
Implement the -R <number> option to add memory reserve slots. Add a -S <size> option makes the blob at least this number of bytes.
Wouldn't it be better to just specify the amount of extra space, instead of the minimum total space? That way, you only need to know what you intend to add, not how much is already there.
Quoted 7 lines
Quoted 1 line
-Scott
I thought briefly about this, but decided to implement a fixed size so that someone could allocate, say, 8K of memory in their memory map (likely flash) and know their blob would fit.
Maybe we need a little -s option to say "add -s bytes".
I think having both options would be a good idea. It would also be nice to have options to do this from the dts file (something similar to /memreserve/).
Quoted 2 lines
Jon suggested a --stats option to print out the important statistics. That would also be a good enhancement.
Ok. How would you envisage this working?
I've thought for some time that it would be a good idea to add an "info" output mode. In that mode instead of outputting a converted device tree, it would give various bits of info on the input tree. This would include things like the header field debugging information that's currently output as pseudo-error messages when using dtb input.
I'm not sure to what extent your "--stats" idea would overlap with that.
Signature · 4 lines
David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson
Quoted 22 lines · David Gibson
Quoted 4 lines · Jerry Van Baren
Quoted 3 lines · Scott Wood
Quoted 2 lines · Jerry Van Baren
Implement the -R <number> option to add memory reserve slots. Add a -S <size> option makes the blob at least this number of bytes.
Quoted 8 lines
Quoted 5 lines
Wouldn't it be better to just specify the amount of extra space, instead of the minimum total space? That way, you only need to know what you intend to add, not how much is already there.
-Scott
I thought briefly about this, but decided to implement a fixed size so that someone could allocate, say, 8K of memory in their memory map (likely flash) and know their blob would fit.
Maybe we need a little -s option to say "add -s bytes".
I think having both options would be a good idea. It would also be nice to have options to do this from the dts file (something similar to /memreserve/).
Quoted 1 line
Jon suggested a --stats option to print out the important statistics. That would also be a good enhancement.
Ok. How would you envisage this working?
Quoted 8 lines
I've thought for some time that it would be a good idea to add an "info" output mode. In that mode instead of outputting a converted device tree, it would give various bits of info on the input tree. This would include things like the header field debugging information that's currently output as pseudo-error messages when using dtb input.
I'm not sure to what extent your "--stats" idea would overlap with that.
Hi David,
Well, that was Jon's suggestion/idea so I have not thought about it much. It would probably overlap 100% with your --info idea, with the exception that Jon's suggestion would also put out the blob.
Since the dtc puts out the blob to sdtout, this actually is problematic.
I presume that is why you suggest inhibiting output. Using stderr
would sorta bypass that problem, but is not ideal (IMHO).
Is there any reason we don't have a -o option to direct the compiled blob output to a file? Since the usual use is to write to a file, it would free up stdout for (info/stats) outputs.
On an unrelated related note, I don't believe my -R additions are actually putting out additional reserve map slots (easiest to see using the asm format output). I'm still trying to understand why not, it seemed pretty straight-forward. When I implemented it, I was looking at hexdumps of the dtb binary format and looking at the header and thought I had it working... using it with my u-boot mods shows no extra reserved slots. I'm looking into where I went wrong.
Best regards,
gvb
Jerry Van Baren wrote:
[snip]
Quoted 1 line
On an unrelated related note, I don't believe my -R additions are actually putting out additional reserve map slots (easiest to see using the asm format output). I'm still trying to understand why not, it seemed pretty straight-forward. When I implemented it, I was looking at hexdumps of the dtb binary format and looking at the header and thought I had it working... using it with my u-boot mods shows no extra reserved slots. I'm looking into where I went wrong.
OK, I didn't add a reserve additional slots output in the asm emission code, so that was a pretty simple fix. I'll provide a patch after a bit more testing.
gvb
Quoted 32 lines · Jerry Van Baren
Quoted 31 lines · David Gibson
Quoted 8 lines · Jerry Van Baren
Quoted 7 lines · Scott Wood
Quoted 2 lines · Jerry Van Baren
Implement the -R <number> option to add memory reserve slots. Add a -S <size> option makes the blob at least this number of bytes.
Wouldn't it be better to just specify the amount of extra space, instead of the minimum total space? That way, you only need to know what you intend to add, not how much is already there.
Quoted 6 lines
Quoted 1 line
-Scott
I thought briefly about this, but decided to implement a fixed size so that someone could allocate, say, 8K of memory in their memory map (likely flash) and know their blob would fit.
Maybe we need a little -s option to say "add -s bytes".
I think having both options would be a good idea. It would also be nice to have options to do this from the dts file (something similar to /memreserve/).
Quoted 2 lines
Jon suggested a --stats option to print out the important statistics. That would also be a good enhancement.
Ok. How would you envisage this working?
I've thought for some time that it would be a good idea to add an "info" output mode. In that mode instead of outputting a converted device tree, it would give various bits of info on the input tree. This would include things like the header field debugging information that's currently output as pseudo-error messages when using dtb input.
Quoted 8 lines
Quoted 2 lines
I'm not sure to what extent your "--stats" idea would overlap with that.
Hi David,
Well, that was Jon's suggestion/idea so I have not thought about it much. It would probably overlap 100% with your --info idea, with the exception that Jon's suggestion would also put out the blob.
Well, my idea wasn't a new --info but rather "-O info", if that distinction makes sense.
Quoted 3 lines
Since the dtc puts out the blob to sdtout, this actually is problematic.
I presume that is why you suggest inhibiting output. Using stderr
would sorta bypass that problem, but is not ideal (IMHO).
Yes, that's more or less precisely my reasoning.
Quoted 3 lines
Is there any reason we don't have a -o option to direct the compiled blob output to a file? Since the usual use is to write to a file, it would free up stdout for (info/stats) outputs.
We do have a -o option...
Quoted 7 lines
On an unrelated related note, I don't believe my -R additions are actually putting out additional reserve map slots (easiest to see using the asm format output). I'm still trying to understand why not, it seemed pretty straight-forward. When I implemented it, I was looking at hexdumps of the dtb binary format and looking at the header and thought I had it working... using it with my u-boot mods shows no extra reserved slots. I'm looking into where I went wrong.
Be careful to check the actual offsets. Bear in mind that objdump may elide zero words. Also bear in mind that the only way a reader of the device tree has of counting the number of reserve entries is stepping through until it hits the terminating (0,0), so the extra entries will just look like an early termination of the list. In this sense -R doesn't add "extra slots", but just ensures that there is space after the reserve map to add more entries.
Signature · 4 lines
David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson
Quoted 1 line · David Gibson
On Sat, Apr 14, 2007 at 08:58:49AM -0400, Jerry Van Baren wrote:
[snip]
Quoted 9 lines
Quoted 1 line
On an unrelated related note, I don't believe my -R additions are actually putting out additional reserve map slots (easiest to see using the asm format output). I'm still trying to understand why not, it seemed pretty straight-forward. When I implemented it, I was looking at hexdumps of the dtb binary format and looking at the header and thought I had it working... using it with my u-boot mods shows no extra reserved slots. I'm looking into where I went wrong.
Be careful to check the actual offsets. Bear in mind that objdump may elide zero words. Also bear in mind that the only way a reader of the device tree has of counting the number of reserve entries is stepping through until it hits the terminating (0,0), so the extra entries will just look like an early termination of the list. In this sense -R doesn't add "extra slots", but just ensures that there is space after the reserve map to add more entries.
Hi David,
It actually is OK, went back and verified it. I needed to modify the asm output to implement the -R extra slots which was confusing me (patch that you didn't like the calloc in ;-).
The calloc change was actually window dressing, I was just being paranoid about a binary blob that is bigger than its contents (has extra space). The unused space is unused, so it doen't really matter if it is non-zero. I'll roll a new version.
gvb
Quoted 23 lines · Jerry Van Baren
Quoted 1 line · David Gibson
On Sat, Apr 14, 2007 at 08:58:49AM -0400, Jerry Van Baren wrote:
[snip]
Quoted 8 lines
Quoted 7 lines
On an unrelated related note, I don't believe my -R additions are actually putting out additional reserve map slots (easiest to see using the asm format output). I'm still trying to understand why not, it seemed pretty straight-forward. When I implemented it, I was looking at hexdumps of the dtb binary format and looking at the header and thought I had it working... using it with my u-boot mods shows no extra reserved slots. I'm looking into where I went wrong.
Quoted 7 lines
Be careful to check the actual offsets. Bear in mind that objdump may elide zero words. Also bear in mind that the only way a reader of the device tree has of counting the number of reserve entries is stepping through until it hits the terminating (0,0), so the extra entries will just look like an early termination of the list. In this sense -R doesn't add "extra slots", but just ensures that there is space after the reserve map to add more entries.
Hi David,
Quoted 8 lines
It actually is OK, went back and verified it. I needed to modify the asm output to implement the -R extra slots which was confusing me (patch that you didn't like the calloc in ;-).
The calloc change was actually window dressing, I was just being paranoid about a binary blob that is bigger than its contents (has extra space). The unused space is unused, so it doen't really matter if it is non-zero. I'll roll a new version.
Oh, I think zeroing it is correct, I'd just prefer it was done at the callsite, rather than within xmalloc(). Strictly speaking you're right, what's in the extra area shouldn't matter, but I don't like the idea of potentially leaking random memory contents to file, which I think could happen without a memset(). Because dtc is unpriveleged it shouldn't actually matter, but it's ugly and a bad habit to establish.
Signature · 4 lines
David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson
Quoted 17 lines · David Gibson
Quoted 16 lines · Jerry Van Baren
Quoted 1 line · David Gibson
On Sat, Apr 14, 2007 at 08:58:49AM -0400, Jerry Van Baren wrote:
[snip]
Quoted 8 lines
Quoted 1 line
On an unrelated related note, I don't believe my -R additions are actually putting out additional reserve map slots (easiest to see using the asm format output). I'm still trying to understand why not, it seemed pretty straight-forward. When I implemented it, I was looking at hexdumps of the dtb binary format and looking at the header and thought I had it working... using it with my u-boot mods shows no extra reserved slots. I'm looking into where I went wrong.
Be careful to check the actual offsets. Bear in mind that objdump may elide zero words. Also bear in mind that the only way a reader of the device tree has of counting the number of reserve entries is stepping through until it hits the terminating (0,0), so the extra entries will just look like an early termination of the list. In this sense -R doesn't add "extra slots", but just ensures that there is space after the reserve map to add more entries.
Hi David,
It actually is OK, went back and verified it. I needed to modify the asm output to implement the -R extra slots which was confusing me (patch that you didn't like the calloc in ;-).
Quoted 8 lines
Quoted 1 line
The calloc change was actually window dressing, I was just being paranoid about a binary blob that is bigger than its contents (has extra space). The unused space is unused, so it doen't really matter if it is non-zero. I'll roll a new version.
Oh, I think zeroing it is correct, I'd just prefer it was done at the callsite, rather than within
xmalloc(). Strictly speaking you're right, what's in the extra area shouldn't matter, but I don't like the idea of potentially leaking random memory contents to file, which I think could happen without amemset(). Because dtc is unpriveleged it shouldn't actually matter, but it's ugly and a bad habit to establish.
Where I got concerned was a conversion of a .dtb input into a .dts or .asm output (or even a .dtb output). I've run it and verified that the extra space (which is potentially random garbage) is not copied to the output - the size in the header is what it is and the data output is only the actual data (no major surprise there).
Generating a text output format actually strips the extra size/data because dtc turns it back into a symbolic form and padding is outside of the symbolic form, if you follow my drift (there isn't a way to represent extra blob padding in a dts format).
gvb