Re: [PATCH 2/9] serial: core: use uart_iotype_*() to simplify code

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

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

On Mon, 27 Apr 2026, Hugo Villeneuve wrote:

> On Fri, 24 Apr 2026 11:30:46 -0400
> Hugo Villeneuve <hugo@hugovil.com> wrote:
> 
> > On Fri, 24 Apr 2026 14:13:02 +0300 (EEST)
> > Ilpo Järvinen <ilpo.jarvinen@linux.intel.com> wrote:
> > 
> > > On Thu, 23 Apr 2026, Hugo Villeneuve wrote:
> > > 
> > > > From: Hugo Villeneuve <hvilleneuve@dimonoff.com>
> > > > 
> > > > Make use of new functions uart_iotype_mmio() and uart_iotype_legacy_io()
> > > > to simplify and improve code readability.
> > > > 
> > > > Signed-off-by: Hugo Villeneuve <hvilleneuve@dimonoff.com>
> > > > ---
> > > >  drivers/tty/serial/serial_core.c | 24 +++++++++---------------
> > > >  1 file changed, 9 insertions(+), 15 deletions(-)
> > > > 
> > > > diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
> > > > index b1cf45a8fc854cd97e349ff077d83b42e3ef8b16..fc273f8f9e75de89dca1ac1aca3589567bcf8a18 100644
> > > > --- a/drivers/tty/serial/serial_core.c
> > > > +++ b/drivers/tty/serial/serial_core.c
> > > > @@ -3221,23 +3221,17 @@ bool uart_match_port(const struct uart_port *port1,
> > > >  {
> > > >  	if (port1->iotype != port2->iotype)
> > > >  		return false;
> > > > -
> > > > -	switch (port1->iotype) {
> > > > -	case UPIO_PORT:
> > > > -		return port1->iobase == port2->iobase;
> > > > -	case UPIO_HUB6:
> > > > -		return port1->iobase == port2->iobase &&
> > > > -		       port1->hub6   == port2->hub6;
> > > > -	case UPIO_MEM:
> > > > -	case UPIO_MEM16:
> > > > -	case UPIO_MEM32:
> > > > -	case UPIO_MEM32BE:
> > > > -	case UPIO_AU:
> > > > -	case UPIO_TSI:
> > > > +	else if (uart_iotype_mmio(port1->iotype))
> > > >  		return port1->mapbase == port2->mapbase;
> > > > -	default:
> > > > +	else if (uart_iotype_legacy_io(port1->iotype))
> > > > +		if (port1->iobase != port2->iobase)
> > > > +			return false;
> > > > +		else if (port1->iotype == UPIO_HUB6 && port1->hub6 != port2->hub6)
> > > 
> > > I suggest placing the entire hub6 related check (including the iobase 
> > > check) into own function in 8250_hub6.c and add a stub for the case when 
> > > hub6 code is not even built into kernel into 8250.h.
> > 
> > Neat. I will make a separate commit for that in V2. This
> > simplifies uart_match_port() even more...
> 
> I implemented it, the only thing I am not sure about is the include of
> "8250/8250.h" in serial_core.c?

I'd just include it and add comment before it that it's included because 
of UPIO_HUB6 port matching.

The more generic alternative would be to make the matcher a callback but 
that feels overkill for some legacy iotype trick, IMO.

-- 
 i.