Re: [PATCH v3 0/2] Defer probing of SAM if serdev device is not ready

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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






[Index of Archives]     [Kernel Newbies]     [Security]     [Netfilter]     [Bugtraq]     [Linux PPP]     [Linux FS]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Video 4 Linux]     [Linmodem]     [Device Mapper]     [Linux Kernel for ARM]

  Powered by Linux