Re: [tegrarcm PATCH 2/2] tegrarcm: Add support for loading MTS

[Thierry Reding <thierry.reding-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>]

[-- Attachment #1: Type: text/plain, Size: 2443 bytes --]

On Wed, Mar 11, 2015 at 03:37:44PM +0000, Stephen Warren wrote:
> Thierry Reding wrote at Wednesday, March 11, 2015 4:46 AM:
> > On Tue, Jan 13, 2015 at 04:20:02PM -0800, Jimmy Zhang wrote:
> > > From: Allen Martin <amartin-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
> > >
> > > The Denver CPU in Tegra132 requires microcode to be loaded before CPU
> > > initialization.  There are two microcode files required, "preboot" MTS
> > > and MTS proper.  Add support for loading MTS from either the binary
> > > versions checked in or from the command line.
> 
> > > @@ -117,6 +131,16 @@ static void usage(char *progname)
> > >  	fprintf(stderr, "\t\tminiloader\n");
> > >  	fprintf(stderr, "\t--miniloader_entry=<mlentry>\n");
> > >  	fprintf(stderr, "\t\tSpecify the entry point for the miniloader\n");
> > > +	fprintf(stderr, "\t--preboot=pbfile\n");
> > > +	fprintf(stderr, "\t\tRead the preboot mts ucode from file instead of using built-in\n");
> > > +	fprintf(stderr, "\t\tpreboot mts\n");
> > > +	fprintf(stderr, "\t--preboot_entry=<pbentry>\n");
> > 
> > Can we please change this to --preboot-entry...
> > 
> > > +	fprintf(stderr, "\t\tSpecify the entry point for the preboot mts ucode\n");
> > > +	fprintf(stderr, "\t--mts=mtsfile\n");
> > > +	fprintf(stderr, "\t\tRead the mts ucode from file instead of using built-in\n");
> > > +	fprintf(stderr, "\t\tmts\n");
> > > +	fprintf(stderr, "\t--mts_entry=<mtsentry>\n");
> > 
> > and this to --mts-entry? I know we've used underscores in the past, but
> > they are very unconventional in command-line options and I mistype them
> > every time.
> > 
> > I have local patches to convert the existing command-line options from
> > underscores to dashes as well. I'll send those out after these patches
> > have been applied.
> 
> I think it makes sense to accept dashes instead of underscores, but we
> shouldn't remove support for the existing cmdline options, so that the
> tool remains backwards-compatible with any scripts anyone has, and
> their muscle memory.

Of course. My local patches simply add a variant of the option with a
dash while keeping the original with the underscore. Not sure we should
proliferate the practice of supporting underscores in new options. I
suppose consistency would be an argument, but I'd rather have us use
dashed variants consistently and only keep the underscore ones for
backwards-compatibility.

Thierry

[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]