lkml.org 
[lkml]   [2017]   [Apr]   [20]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH] RFC: serial: core: Dynamic minor support
On Thu, Apr 20, 2017 at 02:03:57PM +0200, Sjoerd Simons wrote:
> diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
> index 3fe56894974a..37014c70dd69 100644
> --- a/drivers/tty/serial/serial_core.c
> +++ b/drivers/tty/serial/serial_core.c
> @@ -44,6 +44,32 @@
> */
> static DEFINE_MUTEX(port_mutex);
>
> +#ifdef CONFIG_SERIAL_DYNAMIC_MINORS
> +/* List of uarts to allow for allocation of dynamic minor ranges*/
> +LIST_HEAD(dynamic_uarts);
> +DEFINE_MUTEX(dynamic_uarts_mutex);

Both of these should be static.

> +
> +static void
> +uart_setup_major_minor(struct uart_driver *drv)
> +{
> + int start = 0;
> + struct uart_driver *d;
> +

Why not init the list head in here too?

> + drv->major = LOW_DENSITY_UART_MAJOR;
> + mutex_lock(&dynamic_uarts_mutex);
> +
> + list_for_each_entry(d, &dynamic_uarts, dynamic_uarts) {
> + if (start + drv->nr < d->minor)
> + break;
> + start = d->minor + d->nr;
> + }
> + list_add_tail(&drv->dynamic_uarts, &d->dynamic_uarts);
> + drv->minor = start;
> +
> + mutex_unlock(&dynamic_uarts_mutex);
> +}
> +#endif
> +
> /*
> * lockdep: port->lock is initialized in two places, but we
> * want only one lock-class:
> @@ -2458,6 +2484,11 @@ int uart_register_driver(struct uart_driver *drv)
>
> BUG_ON(drv->state);
>
> +#ifdef CONFIG_SERIAL_DYNAMIC_MINORS
> + INIT_LIST_HEAD(&drv->dynamic_uarts);

That line should be in the function:

> + uart_setup_major_minor(drv);
> +#endif

And then provide a "empty" function for this if the option is not
enabled, to keep the #ifdef out of the main flow of the code.

> +
> /*
> * Maybe we should be using a slab cache for this, especially if
> * we have a large number of ports to handle.
> @@ -2527,6 +2558,13 @@ void uart_unregister_driver(struct uart_driver *drv)
> put_tty_driver(p);
> for (i = 0; i < drv->nr; i++)
> tty_port_destroy(&drv->state[i].port);
> +
> +#ifdef CONFIG_SERIAL_DYNAMIC_MINORS
> + mutex_lock(&dynamic_uarts_mutex);
> + list_del(&drv->dynamic_uarts);
> + mutex_unlock(&dynamic_uarts_mutex);
> +#endif

Make this a function, like above, and do the same thing to keep #ifdef
out of here.

> +
> kfree(drv->state);
> drv->state = NULL;
> drv->tty_driver = NULL;
> diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h
> index 58484fb35cc8..890bbefb3f90 100644
> --- 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?



> +
> #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?

Nice first try though!

thanks,

greg k-h

\
 
 \ /
  Last update: 2017-04-20 14:16    [W:0.086 / U:0.812 seconds]
©2003-2017 Jasper Spaans. hosted at Digital OceanAdvertise on this site