public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: Sjoerd Simons <sjoerd.simons@collabora.co.uk>
Cc: linux-serial@vger.kernel.org,
	Geert Uytterhoeven <geert@linux-m68k.org>,
	linux-kernel@vger.kernel.org, Jiri Slaby <jslaby@suse.com>
Subject: Re: [PATCH] RFC: serial: core: Dynamic minor support
Date: Thu, 20 Apr 2017 14:54:16 +0200	[thread overview]
Message-ID: <20170420125416.GA5659@kroah.com> (raw)
In-Reply-To: <1492692258.27393.8.camel@collabora.co.uk>

On Thu, Apr 20, 2017 at 02:44:18PM +0200, Sjoerd Simons wrote:
> On Thu, 2017-04-20 at 14:15 +0200, Greg Kroah-Hartman wrote:
> > On Thu, Apr 20, 2017 at 02:03:57PM +0200, Sjoerd Simons wrote:
> > > --- a/include/linux/serial_core.h
> > > +++ b/include/linux/serial_core.h
> > > @@ -31,6 +31,8 @@
> > >  #include <linux/sysrq.h>
> > >  #include <uapi/linux/serial_core.h>
> > >  
> > > +#define LOW_DENSITY_UART_MAJOR 204
> > 
> > Where are you stealing this from?
> 
> Heh, 204 is defined as the "Low-density serial ports" in
> devices.txt. As documented in the commit message, i've repurposed that
> for dynamic minors (if configured). Maybe it's better to request a new
> major for this purpose? But then again, then just means 204 will go
> unused when the option is on so...
> 
> Lots of drivers do have it as a hard-coded number, seemed sane to put
> it a bit more central for some potential later cleanup in other
> drivers.

Ah, no, that's fine, didn't realize it wasn't already defined somewhere
already.

> > > +
> > >  #ifdef CONFIG_SERIAL_CORE_CONSOLE
> > >  #define uart_console(port) \
> > >  	((port)->cons && (port)->cons->index == (port)->line)
> > > @@ -313,6 +315,10 @@ struct uart_driver {
> > >  	 */
> > >  	struct uart_state	*state;
> > >  	struct tty_driver	*tty_driver;
> > > +
> > > +#ifdef CONFIG_SERIAL_DYNAMIC_MINORS
> > > +	struct list_head	dynamic_uarts;
> > > +#endif
> > 
> > Why not just always have this?
> 
> Trying to save a few bytes if the option is unused; Maybe overdoing it
> :)
> 
> > Nice first try though!
> 
> Thanks, If there are no big comments onthe general approach i'll respin
> without RFC soonish addressing your other comments.

Wait, can you use an idr for this instead of rolling your own search
logic for a series?  I think the usb-serial core did this same sort of
functionality in the drivers/usb/serial/usb-serial.c:allocate_minors()
function using an idr.  Try that instead here as well.

thanks,

greg k-h

  reply	other threads:[~2017-04-20 12:54 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-04-20 12:03 [PATCH] RFC: serial: core: Dynamic minor support Sjoerd Simons
2017-04-20 12:15 ` Greg Kroah-Hartman
2017-04-20 12:44   ` Sjoerd Simons
2017-04-20 12:54     ` Greg Kroah-Hartman [this message]
2017-04-25 19:43 ` Alan Cox

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20170420125416.GA5659@kroah.com \
    --to=gregkh@linuxfoundation.org \
    --cc=geert@linux-m68k.org \
    --cc=jslaby@suse.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-serial@vger.kernel.org \
    --cc=sjoerd.simons@collabora.co.uk \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox