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

[Chen-Yu Tsai]

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:
> > > H616 has 256-byte transmit and receive FIFOs, but its optional DesignWare
> > > component parameter register reads as zero. Generic 16550 probing leaves
> > > the port using the 16-byte FIFO configuration and its trigger-level table.
> > > DMA attachment also depends on a FIFO size being available during probe,
> > > so these ports do not reach the existing 8250 DMA path.
> > >
> > > Use a fixed port configuration for the H616 compatible, with a 256-byte
> > > transmit load and the documented receive thresholds of 1, 64, 128 and 254
> > > bytes. Select the quarter-full receive trigger by default.
> > >
> > > Skip optional-register probing for this fixed configuration. H616 has
> > > debug-divisor and RS485 control registers at the offsets used to detect
> > > DesignWare hardware RS485 and fractional-divisor support. Probing these
> > > offsets writes unrelated registers and can select incompatible callbacks.
> > > Keep the BUSY-safe integer divisor hook and software RS485 handling.
> > >
> > > Use the existing SKIP_SET_RATE quirk to keep the UART clock enabled during
> > > termios changes. Its clock is a gate on the shared APB2 source, rather than
> > > an independent baud-rate generator.
> > >
> > > Attach DMA from the fixed configuration before registering the port. The
> > > 8250 core installs its FIFO defaults during registration, after the probe's
> > > FIFO-size test. Configure single-byte RX and TX bursts because the
> > > peripheral request can correspond to just one character. The existing
> > > startup path requests the firmware-described channels and falls back to
> > > PIO when they are unavailable or the port is a kernel console.
> >
> > ...
> >
> > >       /* 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.


ChenYu

> >
> > ...
> >
> > >  struct dw8250_platform_data {
> > >       u8 usr_reg;
> > >       u32 cpr_value;
> > > +     /* Use a fixed configuration instead of probing optional registers. */
> > > +     unsigned int type;
> > > +     unsigned int dma_maxburst;
> >
> > This sounds like a hack.  Why do we need type here? The address of platform
> > data is enough, no?
>
> The type member selected the new table entry and also identified the
> fixed-configuration path. The matched platform-data pointer can select
> the H616 setup directly, including bypassing the incompatible optional
> register probes. I'll remove the type member and use that instead.
>
> >
> > >       unsigned int quirks;
> >
> > --
> > With Best Regards,
> > Andy Shevchenko
> >
> >
>