On 2023/6/15 下午 06:19, Greg Kroah-Hartman wrote:
On Tue, Jun 13, 2023 at 05:44:23PM +0200, 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 keep complaining about this, when I notice it. Just use the "default"
port type in the serial driver and don't add a new type here and it
should just work, right?
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.
Why do we need a special constant?
- move all values used by the 8250 driver from serial_core.h
to serial.h, as this driver actually uses the constants.
Makes sense.
- 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.
I think this is the best thing to do.
thanks,
greg k-h
I will remove the definition of PORT_MA35 without modifying serial_core.h.
However, we still need to assign a value to port->type. So, can we follow
the approach used in the LiteUART driver and directly assign port->type = 1?
Best regards,
Jacky Huang
