From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-ed1-f41.google.com (mail-ed1-f41.google.com [209.85.208.41]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 02B6B41C6A for ; Tue, 5 May 2026 17:48:01 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.208.41 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1778003283; cv=none; b=NxvoxoMdQpdPbPUFlE5gHd1gel/kxOMQv6pOlmS4jEnUOnd+0rkdywcAyjG1u4/9fRYWbISWiZudP+GEd1xOMAzQgmCCHsWmT6GyyI+tFn836giz8HyHvpy3IBE4rfwmp+tRCpkDyEyDZL5nJimE10CGH2BQtD7hEVeMzuZgvvU= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1778003283; c=relaxed/simple; bh=BHIypYZws6LsxbPE8niDqH5hDCiygd/jcyX7BXAeuZI=; h=Message-ID:Subject:From:To:Cc:Date:In-Reply-To:References: Content-Type:MIME-Version; b=ZyfD/TpGS8Pc9lw1mdW0I5/gUftOen+7tWBz0zAarY0S4nQAQeZCbWd+K+1MXFQ24Qpa5/F6xEPQ4YOA6hHSuAlHaKOVRrXf2Lh30nQE6zFL5PruRcAobynlay6l6iFJOvDUdb5kdUVupWGiJP62MO0tiVWbyKD+J1oFczEe9lc= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=suse.com; spf=pass smtp.mailfrom=suse.com; dkim=pass (2048-bit key) header.d=suse.com header.i=@suse.com header.b=fxMCd9U/; arc=none smtp.client-ip=209.85.208.41 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=suse.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=suse.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=suse.com header.i=@suse.com header.b="fxMCd9U/" Received: by mail-ed1-f41.google.com with SMTP id 4fb4d7f45d1cf-671c24f23b1so8527687a12.0 for ; Tue, 05 May 2026 10:48:01 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.com; s=google; t=1778003280; x=1778608080; darn=vger.kernel.org; h=mime-version:user-agent:content-transfer-encoding:autocrypt :references:in-reply-to:date:cc:to:from:subject:message-id:from:to :cc:subject:date:message-id:reply-to; bh=YF8cC+WD8/dt88I/zzzRNrhURLCXBQrCmTjbnx5Fng0=; b=fxMCd9U/RTH5CmLTzh9ndzU6o+gFCRvrC65qHxqCmWO+lEwcaT7Iy74RCg7JwFXv0w p+1pO3yVaOWFisnebk+zZmbJYES2cnZP01GhqAvkZH/baPMHJCTdJla/P1EEi9UzxbHo 8gaWZA01T7x1EBeYMQmQy3al0Y8zJLMUd3YLnPxB+0bmIcE/aHqitqR7od09pJ0zyT1m F8ySPil4kqebeb2trKQqeHNso342Ds+CUwMn9ebW/ScEtPQ+vRPTzqmCdVReMfIi1Ykk 3kjMqDGhxrPNJnuAItTRlg2B3CcbIRZUI6xJanIJ1Lk34nydhyJVIuu9RMU0SeLoJ/vG DVrA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1778003280; x=1778608080; h=mime-version:user-agent:content-transfer-encoding:autocrypt :references:in-reply-to:date:cc:to:from:subject:message-id:x-gm-gg :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=YF8cC+WD8/dt88I/zzzRNrhURLCXBQrCmTjbnx5Fng0=; b=P2kgAMI/5nMOFQ82DD9HRhEg1d9t2TU5i2FcjkeFhItKrFu39ze74VYlamWfexzrxI 17b4uKO+7MNV+EGz96u8xODGkIYNwyYsxe8yK2/IUZgE0WAVorJAj2Hb8ah3Pgy8D2/O jr8xgu1nQLOlmK2YGp6x1WeEDsPLiycQN+R+6g320Wz5sYB+HrEJOkPgyDw9bnnH0qzB WU7lZpnzX66nX0+NoGMWxeSyyY84vAjaAnm+2+ZnkaUAxjuIJrc/+o05GcX2gDO519V7 JjkIl7PBz32cFNY+dyql4V2Bbyb3/rLZrHTTFWFWu8UVjMWKmsIXibocVt3lttdoffto Z9gQ== X-Forwarded-Encrypted: i=1; AFNElJ9U/o5nZZjwzK2PIUxn4RLsltU7a/EB4txYEEEvo8YPggUlnFPjDYrWnrLdk3FRjOl0FceIn4jQJwrR76I=@vger.kernel.org X-Gm-Message-State: AOJu0YzmX0u1h+tyhMJOQFqqT5Xw4dj1nWCMSLnLz9HtAl75/QCOJDrO Fw+oySH7aXP/Z9ayBGUKv0Q4aYkrgTceYVcyU+bWFtzrSTl/serRePt8WkkaYSKn8Rs2wOgO9KX +Kn9Heqk= X-Gm-Gg: AeBDiet5PMCKvZgMMJd/faQAed6hhyCsXOgk0CdmMRZl2yEVPi1WISurjhnwli9K9UJ w6RE1vtTnlZhM3gn0GyoAPqbzh1LTIoM6bO6L35YKFIdVXO1gIVD/w27NkoQMXA4rjjWuMclHbT mDoYTF8+LaQdXeqDR2eiDt12cAjwcaLyKN3MBsmN5RiUaSDr+79vyjlb3vgER5Bmd0QNz09wESA mlU6AE5ls6FDhGUWUk+rVZx++0ljIwJmqf3VB7cemAvEAcRc8SPW7Jbpw5OCSBO9qT+09gBMsuq KXxTKiYTdBhE6A/HRDnPeYaTBxd6SwVXsmlHCv5tEoKFv1MKSC2cEvsfX1/MentLRnQe22X1OwN Y3nMXKrhpKxJDqOGXLwWS+whKV6+SaIfvqvYS5Knq5XHiD/li0NEM13IagUk7xRjaPaGN7/rN0d OPXrJxNSV70ltAMt3LrU6yP+gllQl8ku0kxp4LQ86pO+EXmhc+RsM2E7ARw4pPlKkPRMz/fALpO hFLonltxv0MA56W X-Received: by 2002:a05:6402:510b:b0:67c:5745:ba25 with SMTP id 4fb4d7f45d1cf-67c5745cc0bmr6383895a12.0.1778003280078; Tue, 05 May 2026 10:48:00 -0700 (PDT) Received: from ?IPv6:2804:1bc4:224:7800:585c:db3a:fcb:e21f? ([2804:1bc4:224:7800:585c:db3a:fcb:e21f]) by smtp.gmail.com with ESMTPSA id 4fb4d7f45d1cf-67cd91a4746sm618070a12.15.2026.05.05.10.47.57 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 05 May 2026 10:47:59 -0700 (PDT) Message-ID: <8d9bc86a72279902e107b567b0cb79bda4019fd0.camel@suse.com> Subject: Re: [PATCH v2 3/9] printk: Separate code for adding/updating preferred console metadata From: Marcos Paulo de Souza To: Petr Mladek , John Ogness Cc: Sergey Senozhatsky , Steven Rostedt , Chris Down , linux-kernel@vger.kernel.org Date: Tue, 05 May 2026 14:47:53 -0300 In-Reply-To: <20260423130015.85175-4-pmladek@suse.com> References: <20260423130015.85175-1-pmladek@suse.com> <20260423130015.85175-4-pmladek@suse.com> Autocrypt: addr=mpdesouza@suse.com; prefer-encrypt=mutual; keydata=mDMEZ/0YqhYJKwYBBAHaRw8BAQdA4JZz0FED+JD5eKlhkNyjDrp6lAGmgR3LPTduPYGPT Km0Kk1hcmNvcyBQYXVsbyBkZSBTb3V6YSA8bXBkZXNvdXphQHN1c2UuY29tPoiTBBMWCgA7FiEE2g gC66iLbhUsCBoBemssEuRpLLUFAmf9GKoCGwMFCwkIBwICIgIGFQoJCAsCBBYCAwECHgcCF4AACgk QemssEuRpLLWGxwD/S1I0bjp462FlKb81DikrOfWbeJ0FOJP44eRzmn20HmEBALBZIMrfIH2dJ5eM GO8seNG8sYiP6JfRjl7Hyqca6YsE Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable User-Agent: Evolution 3.60.1 (by Flathub.org) Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 On Thu, 2026-04-23 at 15:00 +0200, Petr Mladek wrote: > The logic for adding or updating a preferred console is currently > duplicated within __add_preferred_console(), making the code > difficult > to follow and prone to consistency issues. >=20 > Introduce update_preferred_console() to centralize the initialization > and updating of struct preferred_console entries. This refactoring > explicitly defines and enforces the following rules: >=20 > 1. Console names and/or indexes are not set when a console is > preferred > =C2=A0=C2=A0 via devname; these are resolved later during device matching= . > 2. Console names are only added alongside a valid index. > 3. Only matching entries are updated. > 4. Console and Braille options are never cleared. They are updated > =C2=A0=C2=A0 only via the command line. > 5. The global 'preferred_dev_console' index and > 'console_set_on_cmdline' > =C2=A0=C2=A0 flag are updated consistently. >=20 > Additionally, rename braille_set_options() to > braille_update_options() > to better reflect its conditional behavior. >=20 > Behavior change: >=20 > The original code never updated the preferred console options > when it was preferred more times, e.g. via the command line > and/or some platform specific code, e.g. SPCR or device tree. >=20 > The new code explicitly allows to update the console options > when they are preferred over the command line. >=20 > It mostly worked even before but only because the command line was > processed early enough before handling SPCR, device tree, or other > platform specific init code. >=20 > The main behavior change is when the same console is preferred more > times on the command line. Newly, the later or Braille variant > wins. It is a more common and expected behavior. Sorry for the late review. I just wanted to say that the improvement is really great: documentation and code reorganization really improve the code. It was too confusing and rather problematic to follow up. Acked-by: Marcos Paulo de Souza >=20 > Signed-off-by: Petr Mladek > --- > =C2=A0kernel/printk/braille.h |=C2=A0=C2=A0 7 +- > =C2=A0kernel/printk/printk.c=C2=A0 | 149 ++++++++++++++++++++++++++++++--= ------ > -- > =C2=A02 files changed, 118 insertions(+), 38 deletions(-) >=20 > diff --git a/kernel/printk/braille.h b/kernel/printk/braille.h > index 55cd3178a17a..0bdac303f8b1 100644 > --- a/kernel/printk/braille.h > +++ b/kernel/printk/braille.h > @@ -5,9 +5,10 @@ > =C2=A0#ifdef CONFIG_A11Y_BRAILLE_CONSOLE > =C2=A0 > =C2=A0static inline void > -braille_set_options(struct preferred_console *pc, char *brl_options) > +braille_update_options(struct preferred_console *pc, char > *brl_options) > =C2=A0{ > - pc->brl_options =3D brl_options; > + if (brl_options) > + pc->brl_options =3D brl_options; > =C2=A0} > =C2=A0 > =C2=A0/* > @@ -29,7 +30,7 @@ _braille_unregister_console(struct console > *console); > =C2=A0#else > =C2=A0 > =C2=A0static inline void > -braille_set_options(struct preferred_console *pc, char *brl_options) > +braille_update_options(struct preferred_console *pc, char > *brl_options) > =C2=A0{ > =C2=A0} > =C2=A0 > diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c > index 13c98285892b..d251bf8e104f 100644 > --- a/kernel/printk/printk.c > +++ b/kernel/printk/printk.c > @@ -2544,18 +2544,119 @@ asmlinkage __visible void early_printk(const > char *fmt, ...) > =C2=A0} > =C2=A0#endif > =C2=A0 > -static void set_user_specified(struct preferred_console *pc, bool > user_specified) > +/** update_preferred_console - Update a given entry in the > preferred_consoles[] > + * table. > + * @i: index of the entry in @preferred_consoles table which should > get updated. > + * @name: The name of the preferred console driver. > + * @idx: Preferred console index, e.g. port number. > + * @devname: The name of the preferred physical device. > + * @options: Options used when setting up the console driver. > + * @brl_options: Options used when setting up the console driver > + * as a braille console. > + * @user_specified: True if preferred via the kernel command line. > + * > + * The function ensures that the given values are consistent. Also > + * it updates some global variables which are used to make the right > + * decisions in register_console(). > + * > + * Rules: > + * > + *=C2=A0=C2=A0 1. Either @name and valid @idx OR @devname and @idx=3D-1 = are > allowed. > + * Note that a valid @name and @idx will get assigned later > when > + * @devname matches during the device initialization. > + *=C2=A0=C2=A0 2. Specify @brl_options if the console should be enabled = as > + * a Braille console [*] > + *=C2=A0=C2=A0 3. Only matching entries can be updated. > + *=C2=A0=C2=A0 4. @options passed via the command line are used when the= same > + * console is preferred also by some platform-specific code. > + * > + * [*] Braille console is using the mechanism for registering > consoles > + *=C2=A0=C2=A0=C2=A0=C2=A0 but it is very special. It is primarily used = for user > interaction > + *=C2=A0=C2=A0=C2=A0=C2=A0 with the system. It neither gets printk() mes= sages nor is > associated > + *=C2=A0=C2=A0=C2=A0=C2=A0 with /dev/console. > + */ > +static int update_preferred_console(unsigned int i, > + =C2=A0=C2=A0=C2=A0 const char *name, const short > idx, > + =C2=A0=C2=A0=C2=A0 const char *devname, char > *options, > + =C2=A0=C2=A0=C2=A0 char *brl_options, bool > user_specified) > =C2=A0{ > - if (!user_specified) > - return; > + struct preferred_console *pc; > + > + if (i >=3D MAX_PREFERRED_CONSOLES) > + return -E2BIG; > + > + pc =3D &preferred_consoles[i]; > + > + if (!name && !devname) > + return -EINVAL; > + > + if (devname) { > + /* > + * A valid console name and index will get assigned > when > + * a matching device gets registered. > + */ > + if (name) { > + pr_err("Adding a preferred console devname > with a hard-coded console name: %s, %s\n", > + =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 devname, name); > + return -EINVAL; > + } > + if (pc->name[0]) { > + pr_err("Updating a preferred console entry > with an already assigned console name via devname: %s, %s\n", > + =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 devname, pc->name); > + return -EINVAL; > + } > + if (idx !=3D -1) { > + pr_err("Adding a preferred console devname > with a hard-coded index: %s, %d\n", > + =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 devname, idx); > + return -EINVAL; > + } > + > + if (!pc->devname[0]) { > + strscpy(pc->devname, devname); > + pc->index =3D idx; > + } else if (strcmp(pc->devname, devname) !=3D 0) { > + pr_err("Updating a preferred console with an > invalid devname: %s vs. %s\n", > + =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 pc->devname, devname); > + return -EINVAL; > + } > + } > + > + if (name) { > + /* A console name must be defined with a valid > index. */ > + if (idx < 0) { > + pr_err("Adding a preferred console with an > invalid index: %s, %d\n", > + =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 name, idx); > + return -EINVAL; > + } > + > + if (!pc->name[0]) { > + strscpy(pc->name, name); > + pc->index =3D idx; > + } else if (strcmp(pc->name, name) !=3D 0 || pc->index > !=3D idx) { > + pr_err("Updating a preferred console with an > invalid name or index: %s%d vs. %s%d\n", > + =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 pc->name, pc->index, name, idx); > + return -EINVAL; > + } > + } > + > + if (!pc->options || (user_specified && options)) > + pc->options =3D options; > + > + braille_update_options(pc, brl_options); > + > + if (!brl_options) > + preferred_dev_console =3D i; > =C2=A0 > =C2=A0 /* > =C2=A0 * @pc console was defined by the user on the command line. > =C2=A0 * Do not clear when added twice also by SPCR or the device > tree. > =C2=A0 */ > - pc->user_specified =3D true; > - /* At least one console defined by the user on the command > line. */ > - console_set_on_cmdline =3D 1; > + if (user_specified) { > + pc->user_specified =3D true; > + console_set_on_cmdline =3D 1; > + } > + > + return 0; > =C2=A0} > =C2=A0 > =C2=A0static int __add_preferred_console(const char *name, const short > idx, > @@ -2563,19 +2664,11 @@ static int __add_preferred_console(const char > *name, const short idx, > =C2=A0 =C2=A0=C2=A0 char *brl_options, bool > user_specified) > =C2=A0{ > =C2=A0 struct preferred_console *pc; > - int i; > + unsigned int i; > =C2=A0 > =C2=A0 if (!name && !devname) > =C2=A0 return -EINVAL; > =C2=A0 > - /* > - * We use a signed short index for struct console for device > drivers to > - * indicate a not yet assigned index or port. However, a > negative index > - * value is not valid when the console name and index are > defined on > - * the command line. > - */ > - if (name && idx < 0) > - return -EINVAL; > =C2=A0 > =C2=A0 /* > =C2=A0 * See if this tty is not yet registered, and > @@ -2584,28 +2677,14 @@ static int __add_preferred_console(const char > *name, const short idx, > =C2=A0 for (i =3D 0, pc =3D preferred_consoles; > =C2=A0 =C2=A0=C2=A0=C2=A0=C2=A0 i < MAX_PREFERRED_CONSOLES && (pc->name[0= ] || pc- > >devname[0]); > =C2=A0 =C2=A0=C2=A0=C2=A0=C2=A0 i++, pc++) { > - if ((name && strcmp(pc->name, name) =3D=3D 0 && pc- > >index =3D=3D idx) || > - =C2=A0=C2=A0=C2=A0 (devname && strcmp(pc->devname, devname) =3D=3D 0)) > { > - if (!brl_options) > - preferred_dev_console =3D i; > - set_user_specified(pc, user_specified); > - return 0; > - } > + if (name && strcmp(pc->name, name) =3D=3D 0 && pc->index > =3D=3D idx) > + break; > + if (devname && strcmp(pc->devname, devname) =3D=3D 0) > + break; > =C2=A0 } > - if (i =3D=3D MAX_PREFERRED_CONSOLES) > - return -E2BIG; > - if (!brl_options) > - preferred_dev_console =3D i; > - if (name) > - strscpy(pc->name, name); > - if (devname) > - strscpy(pc->devname, devname); > - pc->options =3D options; > - set_user_specified(pc, user_specified); > - braille_set_options(pc, brl_options); > =C2=A0 > - pc->index =3D idx; > - return 0; > + return update_preferred_console(i, name, idx, devname, > options, > + brl_options, > user_specified); > =C2=A0} > =C2=A0 > =C2=A0static int __init console_msg_format_setup(char *str)