Re: [PATCH 20/29] tty: srmcons: fix retval from srmcons_init()
From: Magnus Lindholm
Date: Thu Feb 20 2025 - 16:49:21 EST
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@xxxxxxxxx>
On Thu, Feb 20, 2025 at 12:22 PM Jiri Slaby (SUSE) <jirislaby@xxxxxxxxxx> 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@xxxxxxxxxx>
> Cc: Richard Henderson <richard.henderson@xxxxxxxxxx>
> Cc: Matt Turner <mattst88@xxxxxxxxx>
> Cc: linux-alpha@xxxxxxxxxxxxxxx
> ---
> 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
>
>