On Tue, May 9, 2023, at 14:25, Ilpo Järvinen wrote: > On Tue, 9 May 2023, Arnd Bergmann wrote: >> On Tue, May 9, 2023, at 12:17, Ilpo Järvinen wrote: >> > On Mon, 8 May 2023, Jacky Huang wrote: >> >> + >> >> +#define UART_NR 17 >> >> + >> >> +#define UART_REG_RBR 0x00 >> >> +#define UART_REG_THR 0x00 >> >> +#define UART_REG_IER 0x04 >> >> +#define UART_REG_FCR 0x08 >> >> +#define UART_REG_LCR 0x0C >> >> +#define UART_REG_MCR 0x10 >> > >> > These duplicate include/uapi/linux/serial_reg.h ones, use the std ones >> > directly. >> > >> > Setup regshift too and use it in serial_in. >> >> I think this came up in previous reviews, but it turned out that >> only the first six registers are compatible, while the later >> ones are all different, and it's not 8250 compatible. > > So use the normal name for compatible ones and HW specific names for the > others? > > It might not be compatible in everything but surely 8250 influence is > visible here and there. I'd rename all of them and share nothing. I had the same thought as you when I first looked at the driver, and thought of how we merged the omap uart into 8250 for this reason, but after I found a datasheet for this one, my impression was that it's a much more distant cousin of 8250 than the others, There is clearly some family lineage, but there are differences everywhere, and I don't think it was designed by extending a 8250 compatible hardware block with extra features, but rather built from scratch (sigh) based only loosely on a register description but then extending it with no intent of retaining compatibility. Arnd