Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.35 as permitted sender) client-ip=23.128.96.35;
Date:   Tue, 31 Oct 2023 11:22:46 +0000
From:   "Russell King (Oracle)" <linux@armlinux.org.uk>
To:     =?iso-8859-1?Q?Th=E9o?= Lebrun <theo.lebrun@bootlin.com>
Cc:     Hugo Villeneuve <hugo@hugovil.com>,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        Jiri Slaby <jirislaby@kernel.org>,
        linux-kernel@vger.kernel.org, linux-serial@vger.kernel.org,
        Linus Walleij <linus.walleij@linaro.org>,
        Gregory CLEMENT <gregory.clement@bootlin.com>,
        Alexandre Belloni <alexandre.belloni@bootlin.com>,
        Thomas Petazzoni <thomas.petazzoni@bootlin.com>,
        Vladimir Kondratiev <vladimir.kondratiev@mobileye.com>,
        Tawfik Bayouk <tawfik.bayouk@mobileye.com>
Subject: Re: [PATCH 6/6] tty: serial: amba-pl011: Parse bits option as 5, 6,
 7 or 8 in _get_options
Message-ID: <ZUDjhpQKgUgqVeBh@shell.armlinux.org.uk>
References: <20231026-mbly-uart-v1-0-9258eea297d3@bootlin.com>
 <20231026-mbly-uart-v1-6-9258eea297d3@bootlin.com>
 <20231026105329.0ee9603563202bd2157a7d27@hugovil.com>
 <CWMITJ9VX9IP.1WPQCX981VRDE@tleb-bootlin-xps13-01>
 <ZUDS5UpWlo+DUZc4@shell.armlinux.org.uk>
 <CWMKPFZ9LOVD.2756QU9AP6U3W@tleb-bootlin-xps13-01>
MIME-Version: 1.0
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <CWMKPFZ9LOVD.2756QU9AP6U3W@tleb-bootlin-xps13-01>
Sender: Russell King (Oracle) <linux@armlinux.org.uk>
Precedence: bulk

On Tue, Oct 31, 2023 at 12:04:11PM +0100, Th?o Lebrun wrote:
> Hello,
> 
> On Tue Oct 31, 2023 at 11:11 AM CET, Russell King (Oracle) wrote:
> > There is no point in supporting 5 or 6 bits for console usage. Think
> > about it. What values are going to be sent over the console? It'll be
> > ASCII, which requires at _least_ 7-bit. 6-bit would turn alpha
> > characters into control characters, punctuation and numbers. 5-bit
> > would be all control characters.
> >
> > So there's no point trying to do anything with 5 or 6 bits per byte,
> > and I decided we might as well take that as an error (or maybe a
> > case that the hardware has not been setup) and default to 8 bits per
> > byte.
> 
> I see your point. Two things come to mind:
> 
>  - I added this parsing of 5/6 bits to be symmetrical with
>    pl011_set_termios that handles 5/6 properly. Should pl011_set_termios
>    be modified then?

Why should it? Note that I said above about _console_ usage which is
what you were referring to - the early code that sets up the console
by either reading the current settings (so that we can transparently
use the UART when its handed over already setup by a boot loader).

This is completely different to what happens once the kernel is running.
Userspace might very well have a reason to set 5 or 6 bits if it wants
to communicate with a device that uses those sizes.

However, such a device won't be a console for the reasons I outlined
above (it will truncate the ASCII characters turning console messages
into garbage.)

> If you decide to keep the current behavior, I'd be down to adding a
> comment to explicit this choice in pl011_console_get_options.

Well, honestly I don't think it needs a comment _if_ one thinks about
what these sizes mean for what is supposed to be a console displaying
ASCII characters. It feels to me like pointing out the obvious, and
would be on the level of teaching people how to suck eggs... but then
again, maybe there are times when people need to be taught how to
suck eggs...

So yes, add a comment if you think it's a good idea, but should that
comment be replicated in almost every driver or should it be documented
elsewhere?

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!