Re: [PATCH v2 1/8] serial: txx9: Drop noop probe function and dangerous remove callback

Atsushi Nemoto <[email protected]> Tue, 04 Aug 2026 22:52:36 +0900 (JST)
Newsgroups gmane.linux.serial,gmane.linux.kernel
Message-ID <[email protected]>
On Tue,  4 Aug 2026 10:57:53 +0200, Uwe Kleine-K=F6nig (The Capable Hub=
) <[email protected]> wrote:
> The only platform device that can be bound to the driver is allocated=
 by
> the driver itself. That device doesn't have platdata, so the probe
> function only consists of a loop that is never run and thus can be
> dropped without loss of functionality.

The noop loop might came from old 8250 driver I referenced at that time=
.=

Now I think it can be removed safely.
Thank you for this cleanup.

Reviewed-by: Atsushi Nemoto <[email protected]>

---
Atsushi Nemoto

On Tue,  4 Aug 2026 10:57:53 +0200, Uwe Kleine-K=F6nig (The Capable Hub=
) <[email protected]> wrote:
> The only platform device that can be bound to the driver is allocated=
 by
> the driver itself. That device doesn't have platdata, so the probe
> function only consists of a loop that is never run and thus can be
> dropped without loss of functionality.
> =

> The remove function then should better not undo things that were not
> done in the first place. That happens because serial_txx9_init() call=
s
> serial_txx9_register_ports() with the created platform device and so
> up->dev =3D=3D &dev->dev evaluates to true and uart_remove_one_port()=
 is
> called twice for each port.
> =

> So let serial_txx9_exit() undo exactly what serial_txx9_init() does a=
nd
> remove the useless respectively wrong functions.
> =

> Fixes: 0970769aceb9 ("[SERIAL] serial_txx9 driver update")
> Signed-off-by: Uwe Kleine-K=F6nig (The Capable Hub) <u.kleine-koenig@=
baylibre.com>
> ---
>  drivers/tty/serial/serial_txx9.c | 55 ++----------------------------=
--
>  1 file changed, 3 insertions(+), 52 deletions(-)
> =

> diff --git a/drivers/tty/serial/serial_txx9.c b/drivers/tty/serial/se=
rial_txx9.c
> index 4ae9a45c8e3a..ef56979321b9 100644
> --- a/drivers/tty/serial/serial_txx9.c
> +++ b/drivers/tty/serial/serial_txx9.c
> @@ -1009,7 +1009,8 @@ static void serial_txx9_unregister_port(int lin=
e)
>  	struct uart_port *uart =3D &serial_txx9_ports[line];
>  =

>  	mutex_lock(&serial_txx9_mutex);
> -	uart_remove_one_port(&serial_txx9_reg, uart);
> +	if (uart->iobase || uart->mapbase)
> +		uart_remove_one_port(&serial_txx9_reg, uart);
>  	uart->flags =3D 0;
>  	uart->type =3D PORT_UNKNOWN;
>  	uart->iobase =3D 0;
> @@ -1019,52 +1020,6 @@ static void serial_txx9_unregister_port(int li=
ne)
>  	mutex_unlock(&serial_txx9_mutex);
>  }
>  =

> -/*
> - * Register a set of serial devices attached to a platform device.
> - */
> -static int serial_txx9_probe(struct platform_device *dev)
> -{
> -	struct uart_port *p =3D dev_get_platdata(&dev->dev);
> -	struct uart_port port;
> -	int ret, i;
> -
> -	memset(&port, 0, sizeof(struct uart_port));
> -	for (i =3D 0; p && p->uartclk !=3D 0; p++, i++) {
> -		port.iobase	=3D p->iobase;
> -		port.membase	=3D p->membase;
> -		port.irq	=3D p->irq;
> -		port.uartclk	=3D p->uartclk;
> -		port.iotype	=3D p->iotype;
> -		port.flags	=3D p->flags;
> -		port.mapbase	=3D p->mapbase;
> -		port.dev	=3D &dev->dev;
> -		port.has_sysrq	=3D IS_ENABLED(CONFIG_SERIAL_TXX9_CONSOLE);
> -		ret =3D serial_txx9_register_port(&port);
> -		if (ret < 0) {
> -			dev_err(&dev->dev, "unable to register port at index %d "
> -				"(IO%lx MEM%llx IRQ%d): %d\n", i,
> -				p->iobase, (unsigned long long)p->mapbase,
> -				p->irq, ret);
> -		}
> -	}
> -	return 0;
> -}
> -
> -/*
> - * Remove serial ports registered against a platform device.
> - */
> -static void serial_txx9_remove(struct platform_device *dev)
> -{
> -	int i;
> -
> -	for (i =3D 0; i < UART_NR; i++) {
> -		struct uart_port *up =3D &serial_txx9_ports[i];
> -
> -		if (up->dev =3D=3D &dev->dev)
> -			serial_txx9_unregister_port(i);
> -	}
> -}
> -
>  #ifdef CONFIG_PM
>  static int serial_txx9_suspend(struct platform_device *dev, pm_messa=
ge_t state)
>  {
> @@ -1096,8 +1051,6 @@ static int serial_txx9_resume(struct platform_d=
evice *dev)
>  #endif
>  =

>  static struct platform_driver serial_txx9_plat_driver =3D {
> -	.probe		=3D serial_txx9_probe,
> -	.remove		=3D serial_txx9_remove,
>  #ifdef CONFIG_PM
>  	.suspend	=3D serial_txx9_suspend,
>  	.resume		=3D serial_txx9_resume,
> @@ -1251,9 +1204,7 @@ static void __exit serial_txx9_exit(void)
>  	platform_driver_unregister(&serial_txx9_plat_driver);
>  	platform_device_unregister(serial_txx9_plat_devs);
>  	for (i =3D 0; i < UART_NR; i++) {
> -		struct uart_port *up =3D &serial_txx9_ports[i];
> -		if (up->iobase || up->mapbase)
> -			uart_remove_one_port(&serial_txx9_reg, up);
> +		serial_txx9_unregister_port(i);
>  	}
>  =

>  	uart_unregister_driver(&serial_txx9_reg);
> -- =

> 2.55.0.11.g153666a7d9bb
> =