Re: [PATCH 1/4] serial: vt8500: Find console ports in the serial core state

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Low] serial: vt8500: direct access to private uart_driver.state field
--

commit 97ad19d6dd8fe51bf33a0dd201b9aa1860b0702f
Author: Alexey Charkov <alchark@gmail.com>

serial: vt8500: Find console ports in the serial core state

This commit removes the driver's vt8500_uart_ports[] array and looks up
the console port via vt8500_uart_driver.state[co->index].uart_port in
vt8500_console_write() and vt8500_console_setup().

> diff --git a/drivers/tty/serial/vt8500_serial.c b/drivers/tty/serial/vt8500_serial.c
> index a7f167a734160..6c53e108756a8 100644
> --- a/drivers/tty/serial/vt8500_serial.c
> +++ b/drivers/tty/serial/vt8500_serial.c
> @@ -444,7 +444,6 @@ static int vt8500_verify_port(struct uart_port *port,
>  	return 0;
>  }
>  
> -static struct vt8500_port *vt8500_uart_ports[VT8500_MAX_PORTS];
>  static struct uart_driver vt8500_uart_driver;
>  
>  #ifdef CONFIG_SERIAL_VT8500_CONSOLE
> @@ -472,28 +471,27 @@ static void vt8500_console_putchar(struct uart_port *port, unsigned char c)
>  static void vt8500_console_write(struct console *co, const char *s,
>  			      unsigned int count)
>  {
> -	struct vt8500_port *vt8500_port = vt8500_uart_ports[co->index];
> +	struct uart_port *port = vt8500_uart_driver.state[co->index].uart_port;
>  	unsigned long ier;
>  
>  	BUG_ON(co->index < 0 || co->index >= vt8500_uart_driver.nr);
>  
> -	ier = vt8500_read(&vt8500_port->uart, VT8500_URIER);
> -	vt8500_write(&vt8500_port->uart, VT8500_URIER, 0);
> +	ier = vt8500_read(port, VT8500_URIER);
> +	vt8500_write(port, VT8500_URIER, 0);
>  
> -	uart_console_write(&vt8500_port->uart, s, count,
> -			   vt8500_console_putchar);
> +	uart_console_write(port, s, count, vt8500_console_putchar);
>  
>  	/*
>  	 *	Finally, wait for transmitter to become empty
>  	 *	and switch back to FIFO
>  	 */
> -	wait_for_xmitr(&vt8500_port->uart);
> -	vt8500_write(&vt8500_port->uart, VT8500_URIER, ier);
> +	wait_for_xmitr(port);
> +	vt8500_write(port, VT8500_URIER, ier);
>  }
>  
>  static int __init vt8500_console_setup(struct console *co, char *options)
>  {
> -	struct vt8500_port *vt8500_port;
> +	struct uart_port *port;
>  	int baud = 9600;
>  	int bits = 8;
>  	int parity = 'n';
> @@ -502,16 +500,15 @@ static int __init vt8500_console_setup(struct console *co, char *options)
>  	if (unlikely(co->index >= vt8500_uart_driver.nr || co->index < 0))
>  		return -ENXIO;
>  
> -	vt8500_port = vt8500_uart_ports[co->index];
> +	port = vt8500_uart_driver.state[co->index].uart_port;

[Severity: Low]
Should vt8500_console_write() and vt8500_console_setup() reach into
vt8500_uart_driver.state directly?

include/linux/serial_core.h:struct uart_driver {
    ...
	/*
	 * these are private; the low level driver should not
	 * touch these; they should be initialised to NULL
	 */
	struct uart_state	*state;
	struct tty_driver	*tty_driver;
};

struct uart_driver::state is marked private to the serial core and not
meant to be accessed directly by low-level UART drivers.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261111-vt8500-serial-cleanup-v1-0-b33594a25f87@gmail.com?part=1