Re: [PATCH v1 1/1] serial: 8250: Move CE4100 quirks to a module under 8250 driver

[Andy Shevchenko <andriy.shevchenko@linux.intel.com>]

On Mon, Jun 30, 2025 at 01:27:10PM +0200, Jiri Slaby wrote:
> On 27. 06. 25, 20:25, Andy Shevchenko wrote:
> > There is inconvenient for maintainers and maintainership to have
> > some quirks under architectural code. Move it to the specific quirk
> > file like other 8250-compatible drivers do.
> > 
> > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> 
> Nice.
> 
> Reviewed-by: Jiri Slaby <jirislaby@kernel.org>

Thanks, but it seems a bit late :-)

> Just two nits (one suggestion actually) below. Ignore if you won't resubmit
> for some other reasons...

Sure, see above.

...

> > +static inline void sdv_serial_fixup(void) {};
> 
> Superfluous ;.

Indeed. But I don't want to update just this (as it now may require
a separate change for x86).

...

> > -static u32 ce4100_mem_serial_in(struct uart_port *p, unsigned int offset)
> > -{
> > -	u32 ret, ier, lsr;
> > -
> > -	if (offset != UART_IIR)
> > -		return mem_serial_in(p, offset);
> > -
> > -	offset <<= p->regshift;
> > -
> > -	ret = readl(p->membase + offset);
> 
> Just noticed: why the two above lines are not one:
> ret = mem_serial_in()?
> 
> Or in fact the whole function intro:
> ret = mem_serial_in(p, offset);
> if (offset != UART_IIR)
>   return ret;

Hmm... Sounds like an equivalent. I probably can send a followup for this
suggestion.

-- 
With Best Regards,
Andy Shevchenko