[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