On Mon, May 06, 2024 at 04:41:19PM +0200, Hans de Goede wrote: > On 5/5/24 3:07 PM, Weifeng Liu wrote: ... > If a serdev_device_driver is already loaded for a serdev_tty_port when it > gets registered by tty_port_register_device_attr_serdev() then that > driver's probe() method will be called immediately. > > The serdev_device_driver's probe() method should then be able to call > serdev_device_open() successfully, but because UPF_DEAD is still dead > serdev_device_open() will fail with -ENXIO in this scenario: > > serdev_device_open() > ctrl->ops->open() /* this callback being ttyport_open() */ > tty->ops->open() /* this callback being uart_open() */ > tty_port_open() > port->ops->activate() /* this callback being uart_port_activate() */ > Find bit UPF_DEAD is set in uport->flags and fail with errno -ENXIO. > > Fix this be clearing UPF_DEAD before tty_port_register_device_attr_serdev() > note this only moves up the UPD_DEAD clearing a small bit, before: > > tty_port_register_device_attr_serdev(); > mutex_unlock(&tty_port.mutex); > uart_port.flags &= ~UPF_DEAD; > mutex_unlock(&port_mutex); > > after: > > uart_port.flags &= ~UPF_DEAD; > tty_port_register_device_attr_serdev(); > mutex_unlock(&tty_port.mutex); > mutex_unlock(&port_mutex); > Reported-by: Weifeng Liu <weifeng.liu.z@xxxxxxxxx> > Closes: https://lore.kernel.org/platform-driver-x86/20240505130800.2546640-1-weifeng.liu.z@xxxxxxxxx/ > Cc: Maximilian Luz <luzmaximilian@xxxxxxxxx> > Cc: Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx> Can you move Cc after the cutter '---' line? The patch makes sense to me, FWIW, Reviewed-by: Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx> but Cc'ed Tony just in case. > Signed-off-by: Hans de Goede <hdegoede@xxxxxxxxxx> > --- > drivers/tty/serial/serial_core.c | 6 ++++-- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c > index ff85ebd3a007..d9424fe6513b 100644 > --- a/drivers/tty/serial/serial_core.c > +++ b/drivers/tty/serial/serial_core.c > @@ -3196,6 +3196,9 @@ static int serial_core_add_one_port(struct uart_driver *drv, struct uart_port *u > if (uport->attr_group) > uport->tty_groups[1] = uport->attr_group; > > + /* Ensure serdev drivers can call serdev_device_open() right away */ > + uport->flags &= ~UPF_DEAD; > + > /* > * Register the port whether it's detected or not. This allows > * setserial to be used to alter this port's parameters. > @@ -3206,6 +3209,7 @@ static int serial_core_add_one_port(struct uart_driver *drv, struct uart_port *u > if (!IS_ERR(tty_dev)) { > device_set_wakeup_capable(tty_dev, 1); > } else { > + uport->flags |= UPF_DEAD; > dev_err(uport->dev, "Cannot register tty device on line %d\n", > uport->line); > } > @@ -3411,8 +3415,6 @@ int serial_core_register_port(struct uart_driver *drv, struct uart_port *port) > if (ret) > goto err_unregister_port_dev; > > - port->flags &= ~UPF_DEAD; > - > mutex_unlock(&port_mutex); > > return 0; -- With Best Regards, Andy Shevchenko
