Re: [PATCH v6 4/8] serial: 8250_dw: configure the Allwinner H616 UART

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

On Mon, Nov 02, 2026 at 12:55:19PM -0700, 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.
> 
> Keep PORT_16550A and supply H616's fixed FIFO depth through platform data.
> The 8250 core derives the transmit load from that depth.
> 
> Allwinner encodes receive thresholds relative to the FIFO depth: one byte,
> quarter-full, half-full and depth minus two. Construct the RX trigger
> threshold array on the fly so other FIFO sizes can reuse the same mapping.
> For H616 the thresholds are 1, 64, 128 and 254 bytes, with the normal
> half-full FCR setting selecting 128 bytes. Use this array for both RX
> trigger callbacks and the shared bytes-to-FCR conversion.
> 
> Serialize trigger changes with termios through the port mutex. Hold a
> runtime PM reference for the register access and the UART port lock while
> updating the FCR shadow and hardware. Changing only the trigger field
> preserves the FIFO contents; a runtime resume failure leaves both the
> shadow and hardware unchanged.
> 
> Advertise automatic flow control so CRTSCTS enables MCR.AFCE, matching the
> DesignWare termios path's hardware CTS handling.
> 
> Select this configuration through the platform setup callback. H616's
> debug-divisor and RS485 control registers overlap offsets used to detect
> generic DesignWare features. Use the fixed configuration with BUSY-safe
> integer divisor programming and software RS485 handling.
> 
> Keep the UART clock enabled during termios changes with SKIP_SET_RATE.
> Its clock is a gate on the shared APB2 source rather than an independent
> baud-rate generator.

I'm not sure I followed previous discussion, but as far as I get it Ilpo
mentioned the possibility to have that FIFO trigger threshold helper to be
available on 8250_dw level (meaning dwlib). Or was it decided against this?

...

> +static int dw8250_sunxi_set_rxtrig(struct uart_port *p, unsigned char bytes)
> +{
> +	struct dw8250_data *data = to_dw8250_data(p->private_data);
> +	struct uart_8250_port *up = up_to_u8250p(p);
> +	int fcr, ret;
> +
> +	fcr = serial8250_bytes_to_fcr(data->rxtrig_bytes, bytes);
> +	if (fcr < 0)
> +		return fcr;
> +
> +	guard(mutex)(&p->state->port.mutex);
> +
> +	PM_RUNTIME_ACQUIRE_IF_ENABLED(p->dev, pm);
> +	ret = PM_RUNTIME_ACQUIRE_ERR(&pm);
> +	if (ret)
> +		return ret;
> +
> +	guard(uart_port_lock_irqsave)(p);

> +	up->fcr &= ~UART_FCR_TRIGGER_MASK;
> +	up->fcr |= fcr;

	up->fcr = (up->fcr & ~UART_FCR_TRIGGER_MASK) | fcr;

(as is also okay)?

> +	/* Changing the trigger alone preserves the FIFO contents */
> +	serial_port_out(p, UART_FCR, up->fcr);
> +
> +	return 0;
> +}

-- 
With Best Regards,
Andy Shevchenko