Re: [PATCH v4 5/5] drivers/tty/serial: add ESP32S3 ACM device driver

[Greg Kroah-Hartman <gregkh@linuxfoundation.org>]

On Thu, Oct 05, 2023 at 02:21:55PM -0700, Max Filippov wrote:
> On Thu, Oct 5, 2023 at 11:57 AM Greg Kroah-Hartman
> > > > > --- /dev/null
> > > > > +++ b/drivers/tty/serial/esp32_acm.c
> > > > > @@ -0,0 +1,459 @@
> > > > > +// SPDX-License-Identifier: GPL-2.0-or-later
> > > >
> > > > Why "or later"?  I have to ask, sorry.
> > >
> > > I don't really have a preference here. Is there a reason to choose
> > > GPL-2.0 only for a new code?
> >
> > It's your call, you need to pick that, but I can provide recommendations
> > if you want :)
> 
> Please do?

If you only want your code being used in Linux, then stick with
"GPL-2.0".  If you want your code to be able to be copied into other
GPLv3 licensed code bodies (like Hurd and others), then license it as
"GPL-2.0-or-later".  Your call.

> > > > And no copyright information?  That's fine, but be sure your company's
> > > > lawyers are ok with it...
> > >
> > > There's no company behind this, just myself.
> >
> > Great, it's your copyright, be proud, put it on there!
> 
> If I don't have to I'd rather not. This is just a piece of meaningless noise.

You already own the copyright by virtue of creating the code (you can't
give it away), so might as well put your mark on it to make it more
noticable.  But it's not required, if you don't want to, no one can
force you, again, your call.

> > > > > --- a/include/uapi/linux/serial_core.h
> > > > > +++ b/include/uapi/linux/serial_core.h
> > > > > @@ -248,4 +248,7 @@
> > > > >  /* Espressif ESP32 UART */
> > > > >  #define PORT_ESP32UART       124
> > > > >
> > > > > +/* Espressif ESP32 ACM */
> > > > > +#define PORT_ESP32ACM        125
> > > >
> > > > Why are these defines needed?  What in userspace is going to require
> > > > them?  If nothing, please do not add them.
> > >
> > > I don't understand what the alternatives are. The comment for the
> > > uart_ops::config_port() callback says that port->type should be set
> > > to the type of the port found, and I see that almost every serial driver
> > > defines a unique PORT_* for that.
> >
> > Yes, but not all do.  If you don't need to do anything special, it can
> > just claim to be a normal device, we've had threads about this on the
> > list before.  If you don't need to determine in userspace from the tty
> > connection what device it is, just use the default one instead.
> 
> Ok, it looks like having
> 
> #define PORT_ESP32ACM (-1)
> 
> in the driver source should be ok? I've tested that it works.

Hah, I like that hack.  But why not just use PORT_UNKNOWN?

thanks,

greg k-h