Re: [PATCH v2 08/10] serial: ambarella: add Ambarella UART driver

Linus Walleij <[email protected]> Fri, 7 Aug 2026 20:37:16 +0200
Newsgroups gmane.linux.serial,gmane.linux.drivers.devicetree,gmane.linux.ports.arm.kernel,gmane.linux.kernel,gmane.linux.kernel.clk,gmane.linux.kernel.gpio
Message-ID <CAD++jLm6HzEU1TM-c=Vtjv3FJEBecVP7TyxSgq_Q0zF-YwT6NA@mail.gmail.com>
Hi Long,

thanks for your patch!

I know this driver has a long history and has been refined inside
Ambarella for many years. However this is clearly an 8250
UART derivative:

On Thu, Aug 6, 2026 at 11:34 AM Long Zhao via B4 Relay
<[email protected]> wrote:

> +#define UART_RB_OFFSET                 0x00
> +#define UART_TH_OFFSET                 0x00
> +#define UART_DLL_OFFSET                        0x00

RB, TH, DLL sharing register 0x00.

> +#define UART_IE_OFFSET                 0x04
> +#define UART_DLH_OFFSET                        0x04

IE, DLH sharing register 0x04.

> +#define UART_II_OFFSET                 0x08
> +#define UART_FC_OFFSET                 0x08

II, FC sharing register 0x08.

Compare:

include/uapi/linux/serial_reg.h

#define UART_RX         0       /* In:  Receive buffer */
#define UART_TX         0       /* Out: Transmit buffer */
#define UART_DLL        0       /* Out: Divisor Latch Low */

#define UART_IER        1       /* Out: Interrupt Enable Register */
#define UART_DLM        1       /* Out: Divisor Latch High */

#define UART_IIR        2       /* In:  Interrupt ID Register */
#define UART_FCR        2       /* Out: FIFO Control Register */

This hardware is clearly using the original 8250/16450/16550
tricks to save address space.

The only difference is that your registers are spaced out
with a register shift of 2 so 1 becomes 0x04, 2 becomes 0x08
etc.

It's not even the first doing this, e.g. the DesignWare 8250
driver does the same thing.

This needs to be rewritten using the 8250 library in the kernel
under drivers/tty/serial/8250/8250_ambarella.c
or similar.

Probably you will get much less code to maintain and a few
features for free when you do this.

Yours,
Linus Walleij