On Fri, Aug 23, 2013 at 01:05:11PM -0600, Stephen Warren wrote:
> On 08/22/2013 09:46 PM, Penny Chiu wrote:
> > Add the Tegra124 chip support to cbootimage. User can use "-t124" as
> > option to parse .cfg and generate BCT/image for Tegra124.
> 
> This looks fine to me at a quick glance. Just one small comment below:
> 
> > diff --git a/src/cbootimage.c b/src/cbootimage.c
> 
> >     printf("Usage: cbootimage [options] configfile imagename\n");
> >     printf("    options:\n");
> > -   printf("    -h, --help, -?        Display this message.\n");
> > -   printf("    -d, --debug           Output debugging information.\n");
> > -   printf("    -gbct                 Generate the new bct file.\n");
> > -   printf("    -o<ODM_DATA>          Specify the odm_data(in hex).\n");
> > -   printf("    [-t20|-t30|-t114]     Select one of the possible target 
> > devices,\n");
> > -   printf("                          -t20 if unspecified.\n");
> > -   printf("    configfile            File with configuration 
> > information\n");
> > -   printf("    imagename             Output image name\n");
> > +   printf("    -h, --help, -?              Display this message.\n");
> > +   printf("    -d, --debug                 Output debugging 
> > information.\n");
> > +   printf("    -gbct                       Generate the new bct file.\n");
> > +   printf("    -o<ODM_DATA>                Specify the odm_data(in 
> > hex).\n");
> > +   printf("    [-t20|-t30|-t114|-t124]     Select one of the possible 
> > target devices,\n");
> > +   printf("                                -t20 if unspecified.\n");
> > +   printf("    configfile                  File with configuration 
> > information\n");
> > +   printf("    imagename                   Output image name\n");
> >  }
> 
> To avoid continually re-formatting that text when adding new SoCs, why
> not start the description of the -tNN options on a separate line, and
> leave the indentation as-is for all the unmodified lines. i.e.:
> 
> >> +  printf("    [-t20|-t30|-t114|-t124]\n");
> >> +  printf("                          Select one of the possible target 
> >> devices,\n");
> >> +  printf("                          -t20 if unspecified.\n");

Perhaps in the long run it would be better to turn this into something
like:

        printf("    -t, --target SOC      Select target device. Must be one 
of:\n");
        printf("                          20, 30, 114, 124 (default: 20)\n");

Thierry

Attachment: pgpdrK68ByU0c.pgp
Description: PGP signature

Reply via email to