Re: [PATCH v14 1/1] tty: serial: Add Nuvoton ma35d1 serial driver support

[Arnd Bergmann]

On Tue, Jun 13, 2023, at 16:49, Greg KH wrote:
> On Tue, Jun 13, 2023 at 06:58:32PM +0800, Jacky Huang wrote:
>> 
>> On 2023/6/13 下午 06:28, Greg KH wrote:
>> > On Mon, Jun 12, 2023 at 02:53:55AM +0000, Jacky Huang wrote:
>> > > From: Jacky Huang <ychuang3@xxxxxxxxxxx>
>> > > 
>> > > This adds UART and console driver for Nuvoton ma35d1 Soc.
>> > > It supports full-duplex communication, FIFO control, and
>> > > hardware flow control.
>> > You get a full 72 columns for your changelog :)
>> > 
>> > > --- a/include/uapi/linux/serial_core.h
>> > > +++ b/include/uapi/linux/serial_core.h
>> > > @@ -279,4 +279,7 @@
>> > >   /* Sunplus UART */
>> > >   #define PORT_SUNPLUS	123
>> > > +/* Nuvoton MA35 SoC */
>> > > +#define PORT_MA35	124
>> > > +
>> > Why is this change needed?  What userspace code is going to rely on it?
>> > 
>> > thanks,
>> > 
>> > greg k-h
>> 
>> Because the serial driver requires a port->type, and almost all serial
>> drivers defined their port type here. We follow the practice of most serial
>> drivers here.
>> If we don't do it this way, we would have to directly assign a value to
>> port->type. However, such modifications were questioned in the past,
>> which is why we changed it back to defining the port type in serial_core.h.
>
> I really really want to get rid of this list, as it's a UAPI that no one
> uses.  So please don't use it, it doesn't help anything, and while the
> serial driver might require it, it doesn't actually do anything with
> that field, right?  So why don't we just set all of the values to the
> same one?

I don't see how Jacky can come up with a patch to do this correctly
without more specific guidance to what exactly you are looking for,
after the last 123 people that added support for a new port got
that merged.

I checked debian codesearch and found only three obscure packages that
accidentally include this header instead of including linux/serial.h,
a couple of lists of all kernel headers, and none that include it on
purpose. I agree that this header should really not exist in uapi,
but the question is what exactly to do about it.

Possible changes would be:

- add a special value PORT_* constant other than PORT_UNKNOWN that
  can be used by serial drivers instead of a unique value, and
  ensure that the serial core can handle drivers using it.

- move all values used by the 8250 driver from serial_core.h
  to serial.h, as this driver actually uses the constants.

- Move the remaining contents of uapi/linux/serial.h into the 
  non-uapi version.

- Change all drivers that only reference a single PORT_*
  value to use the generic one.

       Arnd