All of lore.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.