Re: [PATCH v1 6/6] x86/boot: Support nocfg parameter for earlyprintk

[Andy Shevchenko <andriy.shevchenko@linux.intel.com>]

On Tue, 2018-01-16 at 04:12 +0100, Ingo Molnar wrote:
> * Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
> 
> > @@ -133,12 +135,16 @@ static void parse_earlyprintk(void)
> >  		if (arg[pos] == ',')
> >  			pos++;
> >  
> > -		baud = simple_strtoull(arg + pos, &e, 0);
> > -		if (baud == 0 || arg + pos == e)
> > -			baud = DEFAULT_BAUD;
> > +		if (strncmp(arg + pos, "nocfg", 5)) {
> > +			baud = simple_strtoull(arg + pos, &e, 0);
> > +			if (baud == 0 || arg + pos == e)
> > +				baud = DEFAULT_BAUD;
> > +		} else {
> > +			configure = false;
> > +		}
> >  	}
> >  
> > -	early_serial_init(port, baud);
> > +	early_serial_init(port, baud, configure);
> >  }
> >  
> >  #define BASE_BAUD (1843200/16)
> > @@ -162,6 +168,7 @@ static void parse_console_uart8250(void)
> >  	char optstr[64], *options;
> >  	int baud = DEFAULT_BAUD;
> >  	unsigned long port = 0;
> > +	bool configure = true;
> >  
> >  	/*
> >  	 * console=uart8250,io,0x3f8,115200n8
> > @@ -179,12 +186,16 @@ static void parse_console_uart8250(void)
> >  	else
> >  		return;
> >  
> > -	if (options && (options[0] == ','))
> > -		baud = simple_strtoull(options + 1, &options, 0);
> > -	else
> > +	if (options[0] == ',') {
> > +		if (strncmp(options + 1, "nocfg", 5))
> > +			baud = simple_strtoull(options + 1,
> > &options, 0);
> > +		else
> > +			configure = false;
> > +	} else {
> >  		baud = probe_baud(port);
> 
> These code patters seem very similar - could a common function be
> factored out, to 
> simplify future changes (such as the one done here)?

Need to think about. Moreoever, arch/x86/kernel/early_print.c contains
even more duplication, though I understand why it's split to different
folders.

And on top of that we have earlycon (which indeed would be more
preferable solution). Perhaps, instead of playing with earlyprintk at
boot stage we might parse earlycon option that more flexible?

P.S. In any choice at least patch 1 (and maybe patch 2) would be needed.

-- 
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy