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

On Tue,  4 Aug 2026 10:57:53 +0200, Uwe Kleine-König (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önig (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() calls
> serial_txx9_register_ports() with the created platform device and so
> up->dev == &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 and
> remove the useless respectively wrong functions.
> 
> Fixes: 0970769aceb9 ("[SERIAL] serial_txx9 driver update")
> Signed-off-by: Uwe Kleine-König (The Capable Hub) <[email protected]>
> ---
>  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/serial_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 line)
>  	struct uart_port *uart = &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 = 0;
>  	uart->type = PORT_UNKNOWN;
>  	uart->iobase = 0;
> @@ -1019,52 +1020,6 @@ static void serial_txx9_unregister_port(int line)
>  	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 = dev_get_platdata(&dev->dev);
> -	struct uart_port port;
> -	int ret, i;
> -
> -	memset(&port, 0, sizeof(struct uart_port));
> -	for (i = 0; p && p->uartclk != 0; p++, i++) {
> -		port.iobase	= p->iobase;
> -		port.membase	= p->membase;
> -		port.irq	= p->irq;
> -		port.uartclk	= p->uartclk;
> -		port.iotype	= p->iotype;
> -		port.flags	= p->flags;
> -		port.mapbase	= p->mapbase;
> -		port.dev	= &dev->dev;
> -		port.has_sysrq	= IS_ENABLED(CONFIG_SERIAL_TXX9_CONSOLE);
> -		ret = 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 = 0; i < UART_NR; i++) {
> -		struct uart_port *up = &serial_txx9_ports[i];
> -
> -		if (up->dev == &dev->dev)
> -			serial_txx9_unregister_port(i);
> -	}
> -}
> -
>  #ifdef CONFIG_PM
>  static int serial_txx9_suspend(struct platform_device *dev, pm_message_t state)
>  {
> @@ -1096,8 +1051,6 @@ static int serial_txx9_resume(struct platform_device *dev)
>  #endif
>  
>  static struct platform_driver serial_txx9_plat_driver = {
> -	.probe		= serial_txx9_probe,
> -	.remove		= serial_txx9_remove,
>  #ifdef CONFIG_PM
>  	.suspend	= serial_txx9_suspend,
>  	.resume		= 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 = 0; i < UART_NR; i++) {
> -		struct uart_port *up = &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
>