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