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

Greg Kroah-Hartman gregkh at linuxfoundation.org
Thu Jun 15 08:00:21 PDT 2023


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



More information about the linux-arm-kernel mailing list