Re: [PATCH v14 1/1] tty: serial: Add Nuvoton ma35d1 serial driver support - Greg Kroah-Hartman

From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: Arnd Bergmann <arnd@arndb.de>
Cc: Jacky Huang <ychuang570808@gmail.com>,
	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
Date: Thu, 15 Jun 2023 17:00:21 +0200	[thread overview]
Message-ID: <2023061500-tipper-tightwad-8843@gregkh> (raw)
In-Reply-To: <502240f7-2cac-4fe6-9e27-f9861db3666d@app.fastmail.com>

On Thu, Jun 15, 2023 at 04:01:55PM +0200, Arnd Bergmann wrote:
> On Thu, Jun 15, 2023, at 12: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:
> >> 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?
> 
> The "default" value is 0, which translates to PORT_UNKNOWN, and the
> serial core code prevents this from working. I think Jacky tried
> to use this the last one or two times you commented on it, and
> it did not work.

Ah, thanks, that makes sense.

> Setting it to a plain '1' as Jacky suggested in his reply is the
> same as PORT_8250, which may or may not be a good choice here.

Odds are it would be fine :)

> Since the number is exported to userspace in serial_struct,
> it might be better to pick a new constant such as
> 
> #define PORT_SERIAL_GENERIC (-1)
> 
> in order to be less ambiguous. It's a signed integer, so -1
> would work here this would clearly be a special value, or
> another option might be to use 255 as something that is
> slightly less special but still recognizable as something
> that may have a special meaning.

A new constant would be good, 255 is nice, and then we can move everyone
to use it unless they can specifically show a reason why it will not
work for them.

I think originally this was used to do device-specific ioctls, right?
That shouldn't be happening anymore, hopefully...

thanks,

greg k-h