Received-SPF: pass (google.com: domain of linux-kernel+bounces-150104-linux.lists.archive=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) client-ip=139.178.88.99;
Date: Thu, 18 Apr 2024 14:53:43 +0200
From: Greg KH <gregkh@linuxfoundation.org>
To: Esa Laakso <fidelix.laakso@gmail.com>
Cc: jirislaby@kernel.org, linux-serial@vger.kernel.org,
	kari.argillander@gmail.com, linux-kernel@vger.kernel.org,
	Esa Laakso <esa.laakso@fidelix.com>
Subject: Re: [PATCH RFC] pty: Add parity enabling routine
Message-ID: <2024041840-roundworm-sedate-9167@gregkh>
References: <20240418124349.26289-1-esa.laakso@fidelix.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20240418124349.26289-1-esa.laakso@fidelix.com>

On Thu, Apr 18, 2024 at 03:43:49PM +0300, Esa Laakso wrote:
> There are some cases where parity selection is required for passing
> it forward to a virtualized terminal. In this sepcific use-case, we
> want to use pty to send and receive serial data to a serial
> multiplexer. By using a pty, we avoid writing a custom tty driver.
> 
> There is very little evidence on the reasoning on why this option is
> hard-coded to be disabled. AFAIK it has been as such since 1996. With
> the lack of information about why this is, and based on the fact there
> are other similar fields that are not hard-coded, it is considered safe
> to enable this option.
> 
> Still, in order not to be too intrusive about the change, add it only on
> the condition that the termios flag `EXTPROC` is turned on. This way
> there is very little chance it will cause any unintended problems in any
> other implementation.

You need to document that EXTPROC thing somewhere, otherwise someone is
going to ask about this in 20 years and be confused :)

> Signed-off-by: Esa Laakso <esa.laakso@fidelix.com>
> Signed-off-by: Esa Laakso <fidelix.laakso@gmail.com>

Do not sign off on something twice, use your real email address only
once, that's all that is needed.

> ---
> 
> We are looking for some assistance on this patch, or just a green light
> to submit it, if it is good to go.
> 
> We would need to know if the change is valid in context of pty, as
> there seems to be fairly little information about the reasoning behind
> the hard-coded values.
> 
> Our guess is that they have been as such forever and nobody has had a
> reason to change them. We have a reason to change them, and we would
> like to know if it is safe to do so and possibly contribute in the
> process. Either by just using the patch for ourselves or submitting
> it here.
> 
> For the record, this is my first patch submitted upstream. Please let
> me know if something is incorrect or missing.
> ---
> 
>  drivers/tty/pty.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/tty/pty.c b/drivers/tty/pty.c
> index 07394fdaf522..e2d9718dcea0 100644
> --- a/drivers/tty/pty.c
> +++ b/drivers/tty/pty.c
> @@ -267,7 +267,9 @@ static void pty_set_termios(struct tty_struct *tty,
>  		}
>  	}
>  
> -	tty->termios.c_cflag &= ~(CSIZE | PARENB);
> +	tty->termios.c_cflag &= ~(CSIZE);
> +	if (!L_EXTPROC(tty))
> +		tty->termios.c_cflag &= ~(PARENB);

Some description of how you tested this in the changelog would also be
good, and a comment here too as to what you are doing.

thanks,

greg k-h