On 08/26/2013 01:47 AM, Thierry Reding wrote: > 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"); I don't like the idea of "--target 20", since "20" isn't an SoC name; it's "tegra20" or just perhaps "t20". I'd be fine with a patch that allowed "-SOC" or "--target SOC", with SOC being "t20", "t30", ... -- To unsubscribe from this list: send the line "unsubscribe linux-tegra" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html