Re: [PATCH v3 3/3] printk: fix double printing with earlycon

[Sergey Senozhatsky]

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?

> +	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?

> +		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  ?

> +				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, and now
we break out of the loop?

	-ss