Re: [PATCH v1 3/6] serial: 8250: Make uart_match_port() a 8250 specific function

["Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>]

[-- Attachment #1: Type: text/plain, Size: 5462 bytes --]

On Fri, 31 Jul 2026, Uwe Kleine-König wrote:

> The 8250 driver conglomerate is the only user of this function and in

Please write these such that they don't depend on the shortlog. Here "this 
function" lacks any context without assuming one has first read the 
shortlog.

You have similar problem with "this driver" in patch 2.

-- 
 i.

> fact it already is 8250 specific as it uses hub6_match_port() which is
> defined in drivers/tty/serial/8250/8250_hub6.c unless the dummy
> implementation is in use.
> 
> Signed-off-by: Uwe Kleine-König <ukleinek@kernel.org>
> ---
>  Documentation/driver-api/serial/driver.rst |  2 +-
>  drivers/tty/serial/8250/8250_core.c        | 27 +++++++++++++++++++++-
>  drivers/tty/serial/serial_core.c           | 27 ----------------------
>  include/linux/serial_core.h                |  2 --
>  4 files changed, 27 insertions(+), 31 deletions(-)
> 
> diff --git a/Documentation/driver-api/serial/driver.rst b/Documentation/driver-api/serial/driver.rst
> index c1db6a1a67c4..975b0ba53e98 100644
> --- a/Documentation/driver-api/serial/driver.rst
> +++ b/Documentation/driver-api/serial/driver.rst
> @@ -70,7 +70,7 @@ Other functions
>  
>  .. kernel-doc:: drivers/tty/serial/serial_core.c
>     :identifiers: uart_update_timeout uart_get_baud_rate uart_get_divisor
> -           uart_match_port uart_write_wakeup uart_register_driver
> +           uart_write_wakeup uart_register_driver
>             uart_unregister_driver uart_suspend_port uart_resume_port
>             uart_add_one_port uart_remove_one_port uart_console_write
>             uart_parse_earlycon uart_parse_options uart_set_options
> diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c
> index c0e8a4efbdcc..6059b84d8364 100644
> --- a/drivers/tty/serial/8250/8250_core.c
> +++ b/drivers/tty/serial/8250/8250_core.c
> @@ -630,6 +630,31 @@ EXPORT_SYMBOL(serial8250_resume_port);
>   */
>  static DEFINE_MUTEX(serial_mutex);
>  
> +/**
> + * serial8250_match_port - are the two ports equivalent?
> + * @port1: first port
> + * @port2: second port
> + *
> + * This utility function can be used to determine whether two uart_port
> + * structures describe the same port.
> + */
> +static bool serial8250_match_port(const struct uart_port *port1,
> +				  const struct uart_port *port2)
> +{
> +	if (port1->iotype != port2->iotype)
> +		return false;
> +	else if (port1->iotype == UPIO_PORT)
> +		return port1->iobase == port2->iobase;
> +	else if (port1->iotype == UPIO_HUB6)
> +		return hub6_match_port(port1, port2);
> +	else if (uart_iotype_mmio(port1->iotype))
> +		return port1->mapbase == port2->mapbase;
> +	else if (port1->iotype == UPIO_BUS)
> +		return true;
> +	else
> +		return false;
> +}
> +
>  static struct uart_8250_port *serial8250_find_match_or_unused(const struct uart_port *port)
>  {
>  	int i;
> @@ -638,7 +663,7 @@ static struct uart_8250_port *serial8250_find_match_or_unused(const struct uart_
>  	 * First, find a port entry which matches.
>  	 */
>  	for (i = 0; i < nr_uarts; i++)
> -		if (uart_match_port(&serial8250_ports[i].port, port))
> +		if (serial8250_match_port(&serial8250_ports[i].port, port))
>  			return &serial8250_ports[i];
>  
>  	/* try line number first if still available */
> diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
> index edd1e7be2a5c..8a6c53a9996e 100644
> --- a/drivers/tty/serial/serial_core.c
> +++ b/drivers/tty/serial/serial_core.c
> @@ -33,7 +33,6 @@
>  #include <linux/uaccess.h>
>  
>  #include "serial_base.h"
> -#include "8250/8250.h" /* For hub6_match_port() */
>  
>  /*
>   * This is used to lock changes in serial line configuration.
> @@ -3229,32 +3228,6 @@ static void serial_core_remove_one_port(struct uart_driver *drv,
>  	state->uart_port = NULL;
>  }
>  
> -/**
> - * uart_match_port - are the two ports equivalent?
> - * @port1: first port
> - * @port2: second port
> - *
> - * This utility function can be used to determine whether two uart_port
> - * structures describe the same port.
> - */
> -bool uart_match_port(const struct uart_port *port1,
> -		const struct uart_port *port2)
> -{
> -	if (port1->iotype != port2->iotype)
> -		return false;
> -	else if (port1->iotype == UPIO_PORT)
> -		return port1->iobase == port2->iobase;
> -	else if (port1->iotype == UPIO_HUB6)
> -		return hub6_match_port(port1, port2);
> -	else if (uart_iotype_mmio(port1->iotype))
> -		return port1->mapbase == port2->mapbase;
> -	else if (port1->iotype == UPIO_BUS)
> -		return true;
> -	else
> -		return false;
> -}
> -EXPORT_SYMBOL(uart_match_port);
> -
>  static struct serial_ctrl_device *
>  serial_core_get_ctrl_dev(struct serial_port_device *port_dev)
>  {
> diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h
> index c4cc4f66af4b..1659805e2233 100644
> --- a/include/linux/serial_core.h
> +++ b/include/linux/serial_core.h
> @@ -1137,8 +1137,6 @@ int uart_add_one_port(struct uart_driver *reg, struct uart_port *port);
>  void uart_remove_one_port(struct uart_driver *reg, struct uart_port *port);
>  int uart_read_port_properties(struct uart_port *port);
>  int uart_read_and_validate_port_properties(struct uart_port *port);
> -bool uart_match_port(const struct uart_port *port1,
> -		const struct uart_port *port2);
>  
>  /*
>   * Power Management
>