Messages in this thread |  | | Date | Tue, 16 Jan 2018 16:53:18 +0100 | From | Ingo Molnar <> | Subject | Re: [PATCH v1 6/6] x86/boot: Support nocfg parameter for earlyprintk |
| |
* Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
> 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.
I'm fine with your current approach - and earlyprintk is preferred by many kernel developers. Was just wondering how hard it would be to create a common parser - and whether that's desirable at all (it might not be).
Thanks,
Ingo
|  |