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

[Long Zhao]

Hi Linus,

Thanks for pointing this out. I will rework the UART support on top of
the 8250 core. I will first evaluate whether 8250_dw can support the
hardware directly with reg-shift = 2, and add only a small Ambarella
wrapper if any SoC-specific quirks remain.

Best regards,
Long Zhao

Ambarella Shanghai Co., Ltd
-----Original Message-----
From: Linus Walleij <linusw@xxxxxxxxxx> 
Sent: Saturday, August 8, 2026 02:37
To: Long Zhao <longzhao@xxxxxxxxxxxxx>
Cc: Rob Herring <robh@xxxxxxxxxx>; Krzysztof Kozlowski <krzk+dt@xxxxxxxxxx>; Conor Dooley <conor+dt@xxxxxxxxxx>; Michael Turquette <mturquette@xxxxxxxxxxxx>; Stephen Boyd <sboyd@xxxxxxxxxx>; Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>; Jiri Slaby <jirislaby@xxxxxxxxxx>; Catalin Marinas <catalin.marinas@xxxxxxx>; Will Deacon <will@xxxxxxxxxx>; Arnd Bergmann <arnd@xxxxxxxx>; Alexandre Belloni <alexandre.belloni@xxxxxxxxxxx>; devicetree@xxxxxxxxxxxxxxx; linux-arm-kernel@xxxxxxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; linux-clk@xxxxxxxxxxxxxxx; linux-gpio@xxxxxxxxxxxxxxx; linux-serial@xxxxxxxxxxxxxxx; soc@xxxxxxxxxxxxxxx
Subject: [EXT] Re: [PATCH v2 08/10] serial: ambarella: add Ambarella UART driver

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 <devnull+longzhao.ambarella.com@xxxxxxxxxx> 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

######################################################################
This EXTERNAL email has been scanned by Proofpoint Email Protect service.

**********************************************************************
This email and attachments contain Ambarella Proprietary and/or Confidential Information and is intended solely for the use of the individual(s) to whom it is addressed. Any unauthorized review, use, disclosure, distribute, copy, or print is prohibited. If you are not an intended recipient, please contact the sender by reply email and destroy all copies of the original message. Thank you.