Re: [PATCH 2/3] serial: 8250_dw: configure the Allwinner H616 UART

[Andy Shevchenko]

On Sat, Oct 24, 2026 at 10:41:55PM +0900, Chen-Yu Tsai wrote:
> On Sat, Oct 24, 2026 at 10:19 PM Andy Shevchenko
> <andriy.shevchenko@xxxxxxxxxxxxxxx> wrote:
> > On Sat, Oct 24, 2026 at 10:10:33PM +0900, Chen-Yu Tsai wrote:
> > > On Sat, Oct 24, 2026 at 12:12 AM James Hilliard
> > > <james.hilliard1@xxxxxxxxx> wrote:
> > > > On Fri, Oct 23, 2026 at 2:44 AM Andy Shevchenko
> > > > <andriy.shevchenko@xxxxxxxxxxxxxxx> wrote:
> > > > > On Fri, Oct 23, 2026 at 01:33:50AM -0600, James Hilliard wrote:

...

> > > > > >       /* Internal 8250 only */
> > > > > >       UART_PORT_AIROHA                = 124,
> > > > > >       UART_PORT_AIROHA_HS             = 125,
> > > > > > +     UART_PORT_DW_APB_256    = 126,
> > > > >
> > > > > Why? Besides the poorly chosen name, what prevents us from modifying the
> > > > > corresponding parameters at probe time by leaving the one of the existing
> > > > > types?
> > > >
> > > > The new type was primarily a way to reuse the core's FIFO configuration
> > > > table, including the RX trigger mapping. H616 uses thresholds of 1, 64,
> > > > 128 and 254 bytes, whereas PORT_16550A maps the same FCR bits to 1, 4, 8
> > > > and 14 bytes. Overriding only fifosize and tx_loadsz would leave
> > > > rx_trig_bytes reporting and selecting the wrong thresholds.
> > > >
> > > > The core already has get_rxtrig/set_rxtrig callbacks, though. I'll rework
> > > > this to retain PORT_16550A, supply the H616 FIFO parameters during setup,
> > > > and use those callbacks for the correct trigger mapping.
> > >
> > > Please consider just taking the FIFO size from the DT. On some chips
> > > not every UART has the same size FIFO, so we already need to deal with
> > > it.
> > >
> > > Also, FIFO size is a standard UART DT property, though somehow not included
> > > for dw-uart.
> >
> > FWIW, it supports all standard properties for UART.
> > Check what uart_read_port_properties() does.
> 
> Ah, that's nice. I hacked up something [1] in the past to get it to
> work, but that was probably before you added the helper. I never got
> around to splitting up the commits and sending it out.

Hmm... The helper is in the kernel for the past 2.5 years.
Maybe you rebased it and it lost original dates?

But if it's not working, we need to address that, I believe it should work out
of the box, because the same function is used across dozen of serial drivers.

It might be an issue in 8250_dw.c, but I can't tell for sure.

> [1] https://git.kernel.org/pub/scm/linux/kernel/git/wens/linux.git/commit/?id=e7e0741fa04f349ad9f94a4059e32467a30ef5d9

-- 
With Best Regards,
Andy Shevchenko