Hi David, Thanks a lot for your review! About the test cases, please tell me where should I add the case code? I've done some local tests with debug log, not sure whether these case are good enough: 1. -p 1 -a 4 orig_size: 10171 S: 0, p: 1, a: 4, padlen: 0 S: 0, p: 1, a: 4, padlen: 0 S: 0, p: 1, a: 4, padlen: 1 S: 0, p: 1, a: 4, padlen: 1 S: 0, p: 1, a: 4, padlen: 1 final_size: 10172 2. -p 102 -a 4 orig_size: 10171 S: 0, p: 102, a: 4, padlen: 0 S: 0, p: 102, a: 4, padlen: 0 S: 0, p: 102, a: 4, padlen: 102 S: 0, p: 102, a: 4, padlen: 102 S: 0, p: 102, a: 4, padlen: 105 final_size: 10276 3. -S 10000 -a 4 orig_size: 10171 S: 10000, p: 0, a: 4, padlen: 0 Warning: blob size 10171 >= minimum size 10000 S: 10000, p: 0, a: 4, padlen: 0 S: 10000, p: 0, a: 4, padlen: 0 S: 10000, p: 0, a: 4, padlen: 0 S: 10000, p: 0, a: 4, padlen: 1 final_size: 10172 4. -S 99999 -a 4 orig_size: 10171 S: 99999, p: 0, a: 4, padlen: 0 S: 99999, p: 0, a: 4, padlen: 89828 S: 99999, p: 0, a: 4, padlen: 89828 S: 99999, p: 0, a: 4, padlen: 89828 S: 99999, p: 0, a: 4, padlen: 89829 final_size: 100000 5. -a 4 orig_size: 10171 S: 0, p: 0, a: 4, padlen: 0 S: 0, p: 0, a: 4, padlen: 0 S: 0, p: 0, a: 4, padlen: 0 S: 0, p: 0, a: 4, padlen: 0 S: 0, p: 0, a: 4, padlen: 1 final_size: 10172 Best Regards Tim Wang(王艇艇) -----Original Message----- From: David Gibson [mailto:david@xxxxxxxxxxxxxxxxxxxxx] Sent: Tuesday, July 12, 2016 7:00 PM To: Wang Tim（王艇艇） Cc: devicetree-compiler@xxxxxxxxxxxxxxx; wtt_usst@xxxxxxx Subject: Re: [PATCH v5] Implement the -a option to pad dtb aligned On Thu, Jul 07, 2016 at 06:07:08PM +0800, Tim Wang wrote: > There is one condition that need cat the dtb files into one dtb.img > which can support several boards use same SoC platform. > > And the original dtb file size is not aligned to any base. > This may cause "Synchronous Abort" when load from a unligned address > on some SoC machine, such as ARM. > > So this patch implement the -a <aligned number> option to pad zero at > the end of dtb files and make the dtb size aligned to <aligned > number>. > > Then, the aligned dtbs can cat together and load without "Synchronous > Abort". > > Signed-off-by: Tim Wang <timwang@xxxxxxxxxxxx> Alright. The code looks good now. But.. it needs some testcases. > --- > dtc.c | 17 ++++++++++++++++- > dtc.h | 1 + > flattree.c | 17 +++++++++++++---- > 3 files changed, 30 insertions(+), 5 deletions(-) > > diff --git a/dtc.c b/dtc.c > index 5fa23c4..9dcf640 100644 > --- a/dtc.c > +++ b/dtc.c > @@ -30,8 +30,14 @@ int quiet; /* Level of quietness */ > int reservenum; /* Number of memory reservation slots */ > int minsize; /* Minimum blob size */ > int padsize; /* Additional padding to blob */ > +int alignsize; /* Additional padding to blob accroding to the alignsize */ > int phandle_format = PHANDLE_BOTH; /* Use linux,phandle or phandle properties */ > > +static int is_power_of_2(int x) > +{ > + return (x > 0) && ((x & (x - 1)) == 0); } > + > static void fill_fullpaths(struct node *tree, const char *prefix) { > struct node *child; > @@ -53,7 +59,7 @@ static void fill_fullpaths(struct node *tree, const char *prefix) > #define FDT_VERSION(version) _FDT_VERSION(version) > #define _FDT_VERSION(version) #version > static const char usage_synopsis[] = "dtc [options] <input file>"; > -static const char usage_short_opts[] = > "qI:O:o:V:d:R:S:p:fb:i:H:sW:E:hv"; > +static const char usage_short_opts[] = > +"qI:O:o:V:d:R:S:p:a:fb:i:H:sW:E:hv"; > static struct option const usage_long_opts[] = { > {"quiet", no_argument, NULL, 'q'}, > {"in-format", a_argument, NULL, 'I'}, > @@ -64,6 +70,7 @@ static struct option const usage_long_opts[] = { > {"reserve", a_argument, NULL, 'R'}, > {"space", a_argument, NULL, 'S'}, > {"pad", a_argument, NULL, 'p'}, > + {"align", a_argument, NULL, 'a'}, > {"boot-cpu", a_argument, NULL, 'b'}, > {"force", no_argument, NULL, 'f'}, > {"include", a_argument, NULL, 'i'}, > @@ -91,6 +98,7 @@ static const char * const usage_opts_help[] = { > "\n\tMake space for <number> reserve map entries (for dtb and asm output)", > "\n\tMake the blob at least <bytes> long (extra space)", > "\n\tAdd padding to the blob of <bytes> long (extra space)", > + "\n\tMake the blob align to the <bytes> (extra space)", > "\n\tSet the physical boot cpu", > "\n\tTry to produce output even if the input tree has errors", > "\n\tAdd a path to search for include files", @@ -169,6 +177,7 @@ > int main(int argc, char *argv[]) > reservenum = 0; > minsize = 0; > padsize = 0; > + alignsize = 0; > > while ((opt = util_getopt_long()) != EOF) { > switch (opt) { > @@ -196,6 +205,12 @@ int main(int argc, char *argv[]) > case 'p': > padsize = strtol(optarg, NULL, 0); > break; > + case 'a': > + alignsize = strtol(optarg, NULL, 0); > + if (!is_power_of_2(alignsize)) > + die("Invalid argument \"%d\" to -a option\n", > + optarg); > + break; > case 'f': > force = true; > break; > diff --git a/dtc.h b/dtc.h > index 56212c8..32009bc 100644 > --- a/dtc.h > +++ b/dtc.h > @@ -53,6 +53,7 @@ extern int quiet; /* Level of quietness */ > extern int reservenum; /* Number of memory reservation slots */ > extern int minsize; /* Minimum blob size */ > extern int padsize; /* Additional padding to blob */ > +extern int alignsize; /* Additional padding to blob accroding to the alignsize */ > extern int phandle_format; /* Use linux,phandle or phandle properties */ > > #define PHANDLE_LEGACY 0x1 > diff --git a/flattree.c b/flattree.c > index ec14954..01052c2 100644 > --- a/flattree.c > +++ b/flattree.c > @@ -398,15 +398,22 @@ void dt_to_blob(FILE *f, struct boot_info *bi, int version) > */ > if (minsize > 0) { > padlen = minsize - fdt32_to_cpu(fdt.totalsize); > - if ((padlen < 0) && (quiet < 1)) > - fprintf(stderr, > - "Warning: blob size %d >= minimum size %d\n", > - fdt32_to_cpu(fdt.totalsize), minsize); > + if (padlen < 0) { > + padlen = 0; > + if (quiet < 1) > + fprintf(stderr, > + "Warning: blob size %d >= minimum size %d\n", > + fdt32_to_cpu(fdt.totalsize), minsize); > + } > } > > if (padsize > 0) > padlen = padsize; > > + if (alignsize > 0) > + padlen = ALIGN(fdt32_to_cpu(fdt.totalsize) + padlen, alignsize) > + - fdt32_to_cpu(fdt.totalsize); > + > if (padlen > 0) { > int tsize = fdt32_to_cpu(fdt.totalsize); > tsize += padlen; > @@ -572,6 +579,8 @@ void dt_to_asm(FILE *f, struct boot_info *bi, int version) > if (padsize > 0) { > fprintf(f, "\t.space\t%d, 0\n", padsize); > } > + if (alignsize > 0) > + asm_emit_align(f, alignsize); > emit_label(f, symprefix, "blob_abs_end"); > > data_free(strbuf); -- 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 ��.n��������+%������w��{.n����z�{��z��^n�r������&��z�ޗ�zf���h���~����������_��+v���)ߣ�
