linux-kernel - Re: [PATCH 6/6] tty: serial: amba-pl011: Parse bits option as 5, 6, 7 or 8 in _get

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 31 Oct 2023 10:35:29 +0100
From:   Théo Lebrun <theo.lebrun@...tlin.com>
To:     "Hugo Villeneuve" <hugo@...ovil.com>
Cc:     "Russell King" <linux@...linux.org.uk>,
        "Greg Kroah-Hartman" <gregkh@...uxfoundation.org>,
        "Jiri Slaby" <jirislaby@...nel.org>,
        <linux-kernel@...r.kernel.org>, <linux-serial@...r.kernel.org>,
        "Linus Walleij" <linus.walleij@...aro.org>,
        "Gregory CLEMENT" <gregory.clement@...tlin.com>,
        "Alexandre Belloni" <alexandre.belloni@...tlin.com>,
        "Thomas Petazzoni" <thomas.petazzoni@...tlin.com>,
        "Vladimir Kondratiev" <vladimir.kondratiev@...ileye.com>,
        "Tawfik Bayouk" <tawfik.bayouk@...ileye.com>
Subject: Re: [PATCH 6/6] tty: serial: amba-pl011: Parse bits option as 5, 6,
 7 or 8 in _get_options

Hello,

On Thu Oct 26, 2023 at 4:53 PM CEST, Hugo Villeneuve wrote:
> On Thu, 26 Oct 2023 12:41:23 +0200
> Théo Lebrun <theo.lebrun@...tlin.com> wrote:
>
> Hi,
> I would change the commit title to better indicate that you add support
> for bits 5 and 6, which was missing.
>
> Maybe "Add support for 5 and 6 bits in..." ?
>
> > pl011_console_get_options() gets called to retrieve currently configured
> > options from the registers. Previously, LCRH_TX.WLEN was being parsed
>
> It took me some time to understand your explanation :) Maybe change
> to:
>
> "Previously, only 7 or 8 bits were supported."
>
> > as either 7 or 8 (fallback). Hardware supports values from 5 to 8
>
> Add bits:
>
> "5 to 8 bits..."
>
> And indicate that this patch adds support for 5 and 6 bits.

I agree the whole commit message is unclear. Let's rewrite it. What do
you think of the following:

   tty: serial: amba-pl011: Allow parsing word length of 5/6 bits at console setup

   If no options are given at console setup, we parse hardware register
   LCRH_TX.WLEN for bits per word. We compare the value to the 7 bits
   value (UART01x_LCRH_WLEN_7). If the hardware is configured for 5, 6
   or 8 bits per word, we fallback to 8 bits.

   Change that behavior to parse the whole range available: from 5 to 8
   bits per word.

Note that we don't add support for 5/6 bits, we only update the parsing
of the regs (if no options are passed at setup) to reflect the current
hardware config. The behavior will be different only if the inherited
value (from reset/bootloader) is 5 or 6: previously we guessed 8 bits
word length, now we guess the right value.

What's your opinion on this new commit message?

Thanks!

--
Théo Lebrun, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com