Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Feedback-ID: i56a14606:Fastmail
User-Agent: Cyrus-JMAP/3.9.0-alpha0-492-g08e3be04ba-fm-20230607.003-g08e3be04
Mime-Version: 1.0
Message-Id: <35e768ad-7f15-48a4-9c38-09570026cf71@app.fastmail.com>
In-Reply-To: <2023061356-matchbook-footwear-d142@gregkh>
References: <20230612025355.547871-1-ychuang570808@gmail.com>
 <20230612025355.547871-2-ychuang570808@gmail.com>
 <2023061338-lunchbox-snorkel-e6a9@gregkh>
 <f8eb6114-8248-8886-b301-c2886e50e016@gmail.com>
 <2023061356-matchbook-footwear-d142@gregkh>
Date:   Tue, 13 Jun 2023 17:44:23 +0200
From:   "Arnd Bergmann" <arnd@arndb.de>
To:     "Greg Kroah-Hartman" <gregkh@linuxfoundation.org>,
        "Jacky Huang" <ychuang570808@gmail.com>
Cc:     "Rob Herring" <robh+dt@kernel.org>,
        krzysztof.kozlowski+dt@linaro.org, "Lee Jones" <lee@kernel.org>,
        "Michael Turquette" <mturquette@baylibre.com>,
        "Stephen Boyd" <sboyd@kernel.org>,
        "Philipp Zabel" <p.zabel@pengutronix.de>,
        "Jiri Slaby" <jirislaby@kernel.org>,
        "Tomer Maimon" <tmaimon77@gmail.com>,
        "Catalin Marinas" <catalin.marinas@arm.com>,
        "Will Deacon" <will@kernel.org>, devicetree@vger.kernel.org,
        linux-clk@vger.kernel.org, linux-kernel@vger.kernel.org,
        linux-arm-kernel@lists.infradead.org, linux-serial@vger.kernel.org,
        soc@kernel.org, schung@nuvoton.com, mjchen@nuvoton.com,
        "Jacky Huang" <ychuang3@nuvoton.com>
Subject: Re: [PATCH v14 1/1] tty: serial: Add Nuvoton ma35d1 serial driver support
Content-Type: text/plain;charset=utf-8
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

On Tue, Jun 13, 2023, at 16:49, Greg KH wrote:
> On Tue, Jun 13, 2023 at 06:58:32PM +0800, Jacky Huang wrote:
>>=20
>> On 2023/6/13 =E4=B8=8B=E5=8D=88 06:28, Greg KH wrote:
>> > On Mon, Jun 12, 2023 at 02:53:55AM +0000, Jacky Huang wrote:
>> > > From: Jacky Huang <ychuang3@nuvoton.com>
>> > >=20
>> > > 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 :)
>> >=20
>> > > --- 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?
>> >=20
>> > thanks,
>> >=20
>> > greg k-h
>>=20
>> 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_c=
ore.h.
>
> I really really want to get rid of this list, as it's a UAPI that no o=
ne
> 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=20
  non-uapi version.

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

       Arnd