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 > =