Re: [PATCH v10 10/10] tty: serial: Add Nuvoton ma35d1 serial driver support

Jacky Huang
2023-05-10T01:27:03Z

Dear Arnd and Ilpo,

Thank you for the comments.

On 2023/5/9 下午 08:32, Arnd Bergmann wrote:
> 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

Yes, the design of this UART IP is indeed incompatible with the 8250, but it
does imitate the 8250 in some register and register bit field naming, and
even in usage definitions, which can easily lead to misunderstandings.

In order to distinguish it from the 8250 and make it clear that it has
nothing
to do with the 8250, I hope you can agree with me not to use the existing
register and bit field definitions of the 8250 in this driver.

In fact, this UART design has been used for more than 15 years and is used
in our M0/M23/M4, ARM7/ARM9 MCUs and MPUs. The MA35 series will also
continue to use this design. I will add the MA35_ prefix to all
registers and bit
fields, and make the modifications suggested by Ilpo that are unrelated to
this 8250 issue.

Best Regards,
Jacky Huang