[PATCH 00/29] tty: cleanup no. 99

[PATCH 00/29] tty: cleanup no. 99

[Jiri Slaby (SUSE)]

Hi,

this is (again) a series of cleanup in tty. I am trying to rework
tty+serial to avoid limitations of devices (so called NR_UART or
tty_alloc_driver()'s first parameter). And the below popped up while
crawling through the code. So this is only a prep cleanup.

* many tty flags are now enums
* many functions were improved for readability
* quite a few unused or old code dropped

In particular, the runtime behaviour of the kernel before and after the
changes is supposed to be bug to bug compatible (except moxa's ioctl
and ISA evils dropped). That is, noone should notice.

Cc: Alex Elder <elder@kernel.org>
Cc: Andrew Lunn <andrew+netdev@lunn.ch>
Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: David Lin <dtwlin@gmail.com>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Eric Dumazet <edumazet@google.com>
Cc: greybus-dev@lists.linaro.org
Cc: "Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>
Cc: Jakub Kicinski <kuba@kernel.org>
Cc: Johan Hovold <johan@kernel.org>
Cc: linux-alpha@vger.kernel.org
Cc: linux-staging@lists.linux.dev
Cc: Matt Turner <mattst88@gmail.com>
Cc: netdev@vger.kernel.org
Cc: Paolo Abeni <pabeni@redhat.com>
Cc: Richard Henderson <richard.henderson@linaro.org>
Cc: Rob Herring <robh@kernel.org>
Cc: sparclinux@vger.kernel.org

Jiri Slaby (SUSE) (29):
  tty: convert "TTY Struct Flags" to an enum
  tty: audit: do not use N_TTY_BUF_SIZE
  tty: caif: do not use N_TTY_BUF_SIZE
  tty: move N_TTY_BUF_SIZE to n_tty
  tty: n_tty: use uint for space returned by tty_write_room()
  tty: n_tty: simplify process_output()
  tty: n_tty: clean up process_output_block()
  tty: n_tty: drop n_tty_trace()
  tty: n_tty: extract n_tty_continue_cookie() from n_tty_read()
  tty: n_tty: extract n_tty_wait_for_input()
  tty: n_tty: move more_to_be_read to the end of n_tty_read()
  tty: tty_driver: move TTY macros to the top
  tty: tty_driver: convert "TTY Driver Flags" to an enum
  tty: tty_driver: document both {,__}tty_alloc_driver() properly
  tty: tty_driver: introduce TTY driver sub/types enums
  tty: serdev: drop serdev_controller_ops::write_room()
  tty: moxa: drop version dump to logs
  tty: moxa: drop ISA support
  tty: moxa: carve out special ioctls and extra tty_port
  tty: srmcons: fix retval from srmcons_init()
  tty: staging/greybus: pass tty_driver flags to tty_alloc_driver()
  tty: sunsu: drop serial_{in,out}p()
  tty: sunsu: remove unused serial_icr_read()
  serial: remove redundant tty_port_link_device()
  serial: pass struct uart_state to uart_line_info()
  serial: 8250: use serial_in/out() helpers
  serial: 8250_rsa: simplify rsa8250_{request/release}_resource()
  serial: 8250_port: do not use goto for UPQ_NO_TXEN_TEST code flow
  serial: 8250_port: simplify serial8250_request_std_resource()

 Documentation/driver-api/tty/tty_driver.rst |   4 +-
 Documentation/driver-api/tty/tty_struct.rst |   2 +-
 arch/alpha/kernel/srmcons.c                 |  62 ++---
 drivers/net/caif/caif_serial.c              |   2 +-
 drivers/staging/greybus/uart.c              |   4 +-
 drivers/tty/Kconfig                         |   2 +-
 drivers/tty/moxa.c                          | 251 +-------------------
 drivers/tty/n_tty.c                         | 212 ++++++++---------
 drivers/tty/serdev/core.c                   |  11 -
 drivers/tty/serdev/serdev-ttyport.c         |   9 -
 drivers/tty/serial/8250/8250_dw.c           |  17 +-
 drivers/tty/serial/8250/8250_fsl.c          |   8 +-
 drivers/tty/serial/8250/8250_omap.c         |   2 +-
 drivers/tty/serial/8250/8250_port.c         |  59 ++---
 drivers/tty/serial/8250/8250_rsa.c          |  21 +-
 drivers/tty/serial/serial_core.c            |   6 +-
 drivers/tty/serial/sunsu.c                  | 178 ++++++--------
 drivers/tty/tty_audit.c                     |  10 +-
 drivers/tty/tty_io.c                        |   8 +-
 include/linux/serdev.h                      |   6 -
 include/linux/tty.h                         |  53 +++--
 include/linux/tty_driver.h                  | 180 +++++++-------
 22 files changed, 410 insertions(+), 697 deletions(-)

-- 
2.48.1

[PATCH 20/29] tty: srmcons: fix retval from srmcons_init()

[Jiri Slaby (SUSE)]

The value returned from srmcons_init() was -ENODEV for over 2 decades.
But it does not matter, given device_initcall() ignores retvals.

But to be honest, return 0 in case the tty driver was registered
properly.

To do that, the condition is inverted and a short path taken in case of
error.

err_free_drv is introduced as it will be used from more places later.

Signed-off-by: Jiri Slaby (SUSE) <jirislaby@kernel.org>
Cc: Richard Henderson <richard.henderson@linaro.org>
Cc: Matt Turner <mattst88@gmail.com>
Cc: linux-alpha@vger.kernel.org
---
 arch/alpha/kernel/srmcons.c | 62 ++++++++++++++++++++-----------------
 1 file changed, 33 insertions(+), 29 deletions(-)

diff --git a/arch/alpha/kernel/srmcons.c b/arch/alpha/kernel/srmcons.c
index 3e61073f4b30..b9cd364e814e 100644
--- a/arch/alpha/kernel/srmcons.c
+++ b/arch/alpha/kernel/srmcons.c
@@ -196,40 +196,44 @@ static const struct tty_operations srmcons_ops = {
 static int __init
 srmcons_init(void)
 {
+	struct tty_driver *driver;
+	int err;
+
 	timer_setup(&srmcons_singleton.timer, srmcons_receive_chars, 0);
-	if (srm_is_registered_console) {
-		struct tty_driver *driver;
-		int err;
-
-		driver = tty_alloc_driver(MAX_SRM_CONSOLE_DEVICES, 0);
-		if (IS_ERR(driver))
-			return PTR_ERR(driver);
-
-		tty_port_init(&srmcons_singleton.port);
-
-		driver->driver_name = "srm";
-		driver->name = "srm";
-		driver->major = 0; 	/* dynamic */
-		driver->minor_start = 0;
-		driver->type = TTY_DRIVER_TYPE_SYSTEM;
-		driver->subtype = SYSTEM_TYPE_SYSCONS;
-		driver->init_termios = tty_std_termios;
-		tty_set_operations(driver, &srmcons_ops);
-		tty_port_link_device(&srmcons_singleton.port, driver, 0);
-		err = tty_register_driver(driver);
-		if (err) {
-			tty_driver_kref_put(driver);
-			tty_port_destroy(&srmcons_singleton.port);
-			return err;
-		}
-		srmcons_driver = driver;
-	}
 
-	return -ENODEV;
+	if (!srm_is_registered_console)
+		return -ENODEV;
+
+	driver = tty_alloc_driver(MAX_SRM_CONSOLE_DEVICES, 0);
+	if (IS_ERR(driver))
+		return PTR_ERR(driver);
+
+	tty_port_init(&srmcons_singleton.port);
+
+	driver->driver_name = "srm";
+	driver->name = "srm";
+	driver->major = 0;	/* dynamic */
+	driver->minor_start = 0;
+	driver->type = TTY_DRIVER_TYPE_SYSTEM;
+	driver->subtype = SYSTEM_TYPE_SYSCONS;
+	driver->init_termios = tty_std_termios;
+	tty_set_operations(driver, &srmcons_ops);
+	tty_port_link_device(&srmcons_singleton.port, driver, 0);
+	err = tty_register_driver(driver);
+	if (err)
+		goto err_free_drv;
+
+	srmcons_driver = driver;
+
+	return 0;
+err_free_drv:
+	tty_driver_kref_put(driver);
+	tty_port_destroy(&srmcons_singleton.port);
+
+	return err;
 }
 device_initcall(srmcons_init);
 
-\f
 /*
  * The console driver
  */
-- 
2.48.1

Re: [PATCH 20/29] tty: srmcons: fix retval from srmcons_init()

[Magnus Lindholm]

I've applied and verified this patch on an Alphaserver ES40 with
serial console.

Regarding the future use of label err_free_drv, is the intention to
use it to break out early if tty_alloc_driver() fails?


Tested-by: Magnus Lindholm <linmag7@gmail.com>




On Thu, Feb 20, 2025 at 12:22 PM Jiri Slaby (SUSE) <jirislaby@kernel.org> wrote:
>
> The value returned from srmcons_init() was -ENODEV for over 2 decades.
> But it does not matter, given device_initcall() ignores retvals.
>
> But to be honest, return 0 in case the tty driver was registered
> properly.
>
> To do that, the condition is inverted and a short path taken in case of
> error.
>
> err_free_drv is introduced as it will be used from more places later.
>
> Signed-off-by: Jiri Slaby (SUSE) <jirislaby@kernel.org>
> Cc: Richard Henderson <richard.henderson@linaro.org>
> Cc: Matt Turner <mattst88@gmail.com>
> Cc: linux-alpha@vger.kernel.org
> ---
>  arch/alpha/kernel/srmcons.c | 62 ++++++++++++++++++++-----------------
>  1 file changed, 33 insertions(+), 29 deletions(-)
>
> diff --git a/arch/alpha/kernel/srmcons.c b/arch/alpha/kernel/srmcons.c
> index 3e61073f4b30..b9cd364e814e 100644
> --- a/arch/alpha/kernel/srmcons.c
> +++ b/arch/alpha/kernel/srmcons.c
> @@ -196,40 +196,44 @@ static const struct tty_operations srmcons_ops = {
>  static int __init
>  srmcons_init(void)
>  {
> +       struct tty_driver *driver;
> +       int err;
> +
>         timer_setup(&srmcons_singleton.timer, srmcons_receive_chars, 0);
> -       if (srm_is_registered_console) {
> -               struct tty_driver *driver;
> -               int err;
> -
> -               driver = tty_alloc_driver(MAX_SRM_CONSOLE_DEVICES, 0);
> -               if (IS_ERR(driver))
> -                       return PTR_ERR(driver);
> -
> -               tty_port_init(&srmcons_singleton.port);
> -
> -               driver->driver_name = "srm";
> -               driver->name = "srm";
> -               driver->major = 0;      /* dynamic */
> -               driver->minor_start = 0;
> -               driver->type = TTY_DRIVER_TYPE_SYSTEM;
> -               driver->subtype = SYSTEM_TYPE_SYSCONS;
> -               driver->init_termios = tty_std_termios;
> -               tty_set_operations(driver, &srmcons_ops);
> -               tty_port_link_device(&srmcons_singleton.port, driver, 0);
> -               err = tty_register_driver(driver);
> -               if (err) {
> -                       tty_driver_kref_put(driver);
> -                       tty_port_destroy(&srmcons_singleton.port);
> -                       return err;
> -               }
> -               srmcons_driver = driver;
> -       }
>
> -       return -ENODEV;
> +       if (!srm_is_registered_console)
> +               return -ENODEV;
> +
> +       driver = tty_alloc_driver(MAX_SRM_CONSOLE_DEVICES, 0);
> +       if (IS_ERR(driver))
> +               return PTR_ERR(driver);
> +
> +       tty_port_init(&srmcons_singleton.port);
> +
> +       driver->driver_name = "srm";
> +       driver->name = "srm";
> +       driver->major = 0;      /* dynamic */
> +       driver->minor_start = 0;
> +       driver->type = TTY_DRIVER_TYPE_SYSTEM;
> +       driver->subtype = SYSTEM_TYPE_SYSCONS;
> +       driver->init_termios = tty_std_termios;
> +       tty_set_operations(driver, &srmcons_ops);
> +       tty_port_link_device(&srmcons_singleton.port, driver, 0);
> +       err = tty_register_driver(driver);
> +       if (err)
> +               goto err_free_drv;
> +
> +       srmcons_driver = driver;
> +
> +       return 0;
> +err_free_drv:
> +       tty_driver_kref_put(driver);
> +       tty_port_destroy(&srmcons_singleton.port);
> +
> +       return err;
>  }
>  device_initcall(srmcons_init);
>
> -
>  /*
>   * The console driver
>   */
> --
> 2.48.1
>
>

Re: [PATCH 20/29] tty: srmcons: fix retval from srmcons_init()

[Jiri Slaby]

On 20. 02. 25, 22:48, Magnus Lindholm wrote:
> I've applied and verified this patch on an Alphaserver ES40 with
> serial console.
> 
> Regarding the future use of label err_free_drv, is the intention to
> use it to break out early if tty_alloc_driver() fails?

...

>> +       if (!srm_is_registered_console)
>> +               return -ENODEV;
>> +
>> +       driver = tty_alloc_driver(MAX_SRM_CONSOLE_DEVICES, 0);
>> +       if (IS_ERR(driver))
>> +               return PTR_ERR(driver);
>> +
>> +       tty_port_init(&srmcons_singleton.port);
>> +
>> +       driver->driver_name = "srm";
>> +       driver->name = "srm";
>> +       driver->major = 0;      /* dynamic */
>> +       driver->minor_start = 0;
>> +       driver->type = TTY_DRIVER_TYPE_SYSTEM;
>> +       driver->subtype = SYSTEM_TYPE_SYSCONS;
>> +       driver->init_termios = tty_std_termios;
>> +       tty_set_operations(driver, &srmcons_ops);
>> +       tty_port_link_device(&srmcons_singleton.port, driver, 0);

I plan on removing tty_port_link_device() as it was a temporary aid. 
Yay, for 13 years! (commit 2cb4ca0208722).

>> +       err = tty_register_driver(driver);
>> +       if (err)
>> +               goto err_free_drv;

Instead, the idea is to properly tty_port_register_device() after 
tty_register_driver() instead. And that can fail. So is fail path reuses 
err_free_drv (and adds tty_unregister_driver() on top).

>> +
>> +       srmcons_driver = driver;
>> +
>> +       return 0;
>> +err_free_drv:
>> +       tty_driver_kref_put(driver);
>> +       tty_port_destroy(&srmcons_singleton.port);
>> +
>> +       return err;

thanks,
-- 
js
suse labs

Re: [PATCH 00/29] tty: cleanup no. 99

[Greg KH]

On Thu, Feb 20, 2025 at 12:15:37PM +0100, Jiri Slaby (SUSE) wrote:
> Hi,
> 
> this is (again) a series of cleanup in tty. I am trying to rework
> tty+serial to avoid limitations of devices (so called NR_UART or
> tty_alloc_driver()'s first parameter). And the below popped up while
> crawling through the code. So this is only a prep cleanup.
> 
> * many tty flags are now enums
> * many functions were improved for readability
> * quite a few unused or old code dropped
> 
> In particular, the runtime behaviour of the kernel before and after the
> changes is supposed to be bug to bug compatible (except moxa's ioctl
> and ISA evils dropped). That is, noone should notice.

Were you going to do a new respin of this, or do you want me to take
this as-is and you will send a follow-up ones for the commented-on
changes?

thanks,

greg k-h

Re: [PATCH 00/29] tty: cleanup no. 99

[Jiri Slaby]

On 17. 03. 25, 5:28, Greg KH wrote:
> On Thu, Feb 20, 2025 at 12:15:37PM +0100, Jiri Slaby (SUSE) wrote:
>> Hi,
>>
>> this is (again) a series of cleanup in tty. I am trying to rework
>> tty+serial to avoid limitations of devices (so called NR_UART or
>> tty_alloc_driver()'s first parameter). And the below popped up while
>> crawling through the code. So this is only a prep cleanup.
>>
>> * many tty flags are now enums
>> * many functions were improved for readability
>> * quite a few unused or old code dropped
>>
>> In particular, the runtime behaviour of the kernel before and after the
>> changes is supposed to be bug to bug compatible (except moxa's ioctl
>> and ISA evils dropped). That is, noone should notice.
> 
> Were you going to do a new respin of this, or do you want me to take
> this as-is and you will send a follow-up ones for the commented-on
> changes?

I planned to send a v2 on Fri, but did not make it. I will today.

thanks,
-- 
js
suse labs

Re: [PATCH 00/29] tty: cleanup no. 99

[Greg KH]

On Mon, Mar 17, 2025 at 05:59:01AM +0100, Jiri Slaby wrote:
> On 17. 03. 25, 5:28, Greg KH wrote:
> > On Thu, Feb 20, 2025 at 12:15:37PM +0100, Jiri Slaby (SUSE) wrote:
> > > Hi,
> > > 
> > > this is (again) a series of cleanup in tty. I am trying to rework
> > > tty+serial to avoid limitations of devices (so called NR_UART or
> > > tty_alloc_driver()'s first parameter). And the below popped up while
> > > crawling through the code. So this is only a prep cleanup.
> > > 
> > > * many tty flags are now enums
> > > * many functions were improved for readability
> > > * quite a few unused or old code dropped
> > > 
> > > In particular, the runtime behaviour of the kernel before and after the
> > > changes is supposed to be bug to bug compatible (except moxa's ioctl
> > > and ISA evils dropped). That is, noone should notice.
> > 
> > Were you going to do a new respin of this, or do you want me to take
> > this as-is and you will send a follow-up ones for the commented-on
> > changes?
> 
> I planned to send a v2 on Fri, but did not make it. I will today.

Not a problem, just wanted to make sure I didn't drop these on my side
accidentally.

thanks,

greg k-h