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

[Jiri Slaby]

On 13. 06. 23, 17:44, Arnd Bergmann wrote:
> 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.

Hmm, we are looping :).

https://lore.kernel.org/all/75375f8d-e157-a364-3da5-9c8d5b832927@xxxxxxxxxx/

regards,
--
js
suse labs