Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S965780AbdDTMo2 convert rfc822-to-8bit (ORCPT ); Thu, 20 Apr 2017 08:44:28 -0400 Received: from bhuna.collabora.co.uk ([46.235.227.227]:33826 "EHLO bhuna.collabora.co.uk" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S943529AbdDTMoX (ORCPT ); Thu, 20 Apr 2017 08:44:23 -0400 Message-ID: <1492692258.27393.8.camel@collabora.co.uk> Subject: Re: [PATCH] RFC: serial: core: Dynamic minor support From: Sjoerd Simons To: Greg Kroah-Hartman Cc: linux-serial@vger.kernel.org, Geert Uytterhoeven , linux-kernel@vger.kernel.org, Jiri Slaby Date: Thu, 20 Apr 2017 14:44:18 +0200 In-Reply-To: <20170420121544.GA1131@kroah.com> References: <20170420120357.18317-1-sjoerd.simons@collabora.co.uk> <20170420121544.GA1131@kroah.com> Organization: Collabora Ltd. Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: 8BIT X-Mailer: Evolution 3.22.6-1 Mime-Version: 1.0 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1498 Lines: 48 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 > >  #include > >   > > +#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. > > > + > >  #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. -- Sjoerd Simons Collabora Ltd.