lkml.org 
[lkml]   [2017]   [Mar]   [7]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    SubjectRe: [PATCH v3 3/3] printk: fix double printing with earlycon
    From
    Date


    On 03/06/2017 03:59 PM, Sergey Senozhatsky wrote:
    > On (03/03/17 18:49), Aleksey Makarov wrote:
    > [..]
    >> +static enum { CONSOLE_MATCH, CONSOLE_MATCH_RETURN, CONSOLE_MATCH_NEXT }
    >> +match_console(struct console *newcon, struct console_cmdline *c)
    >
    > that enum in function return is interesting :)
    > can we make it less hackish?

    We probably can, but I can not figure out how to do that.
    Suggestions will be appreciated.
    We should signal 3 different outcomes.
    I thought that using standard errnos is not quite desciptive.

    >> + if (!newcon->match ||
    >> + newcon->match(newcon, c->name, c->index, c->options) != 0) {
    >> + /* default matching */
    >> + BUILD_BUG_ON(sizeof(c->name) != sizeof(newcon->name));
    >> + if (strcmp(c->name, newcon->name) != 0)
    >> + return CONSOLE_MATCH_NEXT;
    >> + if (newcon->index >= 0 && newcon->index != c->index)
    >> + return CONSOLE_MATCH_NEXT;
    >
    > who is checking CONSOLE_MATCH_NEXT?

    The caller checks two other values, this one goes under the default case.
    Should I check it explicitly?

    >> + if (newcon->index < 0)
    >> + newcon->index = c->index;
    >> +
    >> + if (_braille_register_console(newcon, c))
    >> + return CONSOLE_MATCH_RETURN;
    >> +
    >> + if (newcon->setup &&
    >> + newcon->setup(newcon, c->options) != 0)
    >> + return CONSOLE_MATCH;
    >> + }
    >> +
    >> + newcon->flags |= CON_ENABLED;
    >> + return CONSOLE_MATCH;
    >> +}
    >> +
    >> /*
    >> * The console driver calls this routine during kernel initialization
    >> * to register the console printing procedure with printk() and to
    >> @@ -2454,40 +2480,50 @@ void register_console(struct console *newcon)
    >> }
    >>
    >> /*
    >> + * Check if this console was set as preferred by command line parameters
    >> + * or by call to add_preferred_console(). There may be several entries
    >> + * in the console_cmdline array matching with the same console so we
    >> + * can not just use the first match. Instead check the entry pointed
    >> + * by preferred_console and then all other entries.
    >> + */
    >> + if (preferred_console >= 0) {
    >> + switch (match_console(newcon,
    >> + console_cmdline + preferred_console)) {
    >> + case CONSOLE_MATCH:
    >> + if (newcon->flags | CON_ENABLED) {
    >
    > newcon->flags & CON_ENABLED ?

    Sure. Thank you.

    >> + newcon->flags |= CON_CONSDEV;
    >> + has_preferred = true;
    >> + }
    >> + goto match;
    >> + case CONSOLE_MATCH_RETURN:
    >> + return;
    >> + default:
    >> + break;
    >> + }
    >> + }
    >> +
    >> + /*
    >> * See if this console matches one we selected on
    >> * the command line.
    >> */
    >> for (i = 0, c = console_cmdline;
    >> i < MAX_CMDLINECONSOLES && c->name[0];
    >> i++, c++) {
    >> - if (!newcon->match ||
    >> - newcon->match(newcon, c->name, c->index, c->options) != 0) {
    >> - /* default matching */
    >> - BUILD_BUG_ON(sizeof(c->name) != sizeof(newcon->name));
    >> - if (strcmp(c->name, newcon->name) != 0)
    >> - continue;
    >> - if (newcon->index >= 0 &&
    >> - newcon->index != c->index)
    >> - continue;
    >> - if (newcon->index < 0)
    >> - newcon->index = c->index;
    >> -
    >> - if (_braille_register_console(newcon, c))
    >> - return;
    >>
    >> - if (newcon->setup &&
    >> - newcon->setup(newcon, c->options) != 0)
    >> - break;
    >> - }
    >> + if (preferred_console == i)
    >> + continue;
    >>
    >> - newcon->flags |= CON_ENABLED;
    >> - if (i == preferred_console) {
    >> - newcon->flags |= CON_CONSDEV;
    >> - has_preferred = true;
    >> + switch (match_console(newcon, c)) {
    >> + case CONSOLE_MATCH:
    >> + goto match;
    >> + case CONSOLE_MATCH_RETURN:
    >> + return;
    >> + default:
    >> + break;
    >
    > sorry, it was a rather long for me today. need to look more at this.
    > for what is now CONSOLE_MATCH_NEXT we used to have continue,

    CONSOLE_MATCH is for the case when the console matches against the description,
    CONSOLE_MATCH_NEXT - it does not, we should try next,
    CONSOLE_MATCH_RETURN - we should return as the braille console is registered now.

    > and now we break out of the loop?

    `goto match` or just when we have seen all the entries

    Thank you
    Aleksey Makarov

    >
    > -ss
    > --
    > To unsubscribe from this list: send the line "unsubscribe linux-serial" in
    > the body of a message to majordomo@vger.kernel.org
    > More majordomo info at http://vger.kernel.org/majordomo-info.html
    >

    \
     
     \ /
      Last update: 2017-03-07 16:05    [W:3.458 / U:0.120 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site