Re: [PATCH v2 1/2] tty: serial_core: Add name field to uart_port struct

[Andy Shevchenko]

On Fri, Mar 24, 2017 at 7:27 AM, Vignesh R <vigneshr@xxxxxx> wrote:
> Introduce a field to store name of uart_port that can be used to easily
> identify UART port instances on a system that has more than one UART
> instance. The name is of the form ttyXN(eg. ttyS0, ttyAMA0,..) where N
> is number that particular UART instance.
> This field will be useful when printing debug info for a particular port
> or in register IRQs with unique IRQ name. Port name is populated during
> uart_add_one_port().
>

Looks good to me:
Reviewed-by: Andy Shevchenko <andy.shevchenko@xxxxxxxxx>

Just in case, have you checked if there any possible scenarios where
memory will be leaked?

> Signed-off-by: Vignesh R <vigneshr@xxxxxx>
> ---
>
> v2:
> use kasprintf() instead of snprintf()
> Fix up commit message.
>
>  drivers/tty/serial/serial_core.c | 7 +++++++
>  include/linux/serial_core.h      | 1 +
>  2 files changed, 8 insertions(+)
>
> diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
> index 0fb3f7cce62a..f5572e28d16a 100644
> --- a/drivers/tty/serial/serial_core.c
> +++ b/drivers/tty/serial/serial_core.c
> @@ -2744,6 +2744,12 @@ int uart_add_one_port(struct uart_driver *drv, struct uart_port *uport)
>         state->pm_state = UART_PM_STATE_UNDEFINED;
>         uport->cons = drv->cons;
>         uport->minor = drv->tty_driver->minor_start + uport->line;
> +       uport->name = kasprintf(GFP_KERNEL, "%s%d", drv->dev_name,
> +                               drv->tty_driver->name_base + uport->line);
> +       if (!uport->name) {
> +               ret = -ENOMEM;
> +               goto out;
> +       }
>
>         /*
>          * If this port is a console, then the spinlock is already
> @@ -2861,6 +2867,7 @@ int uart_remove_one_port(struct uart_driver *drv, struct uart_port *uport)
>         if (uport->type != PORT_UNKNOWN && uport->ops->release_port)
>                 uport->ops->release_port(uport);
>         kfree(uport->tty_groups);
> +       kfree(uport->name);
>
>         /*
>          * Indicate that there isn't a port here anymore.
> diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h
> index 58484fb35cc8..60530678c633 100644
> --- a/include/linux/serial_core.h
> +++ b/include/linux/serial_core.h
> @@ -247,6 +247,7 @@ struct uart_port {
>         unsigned char           suspended;
>         unsigned char           irq_wake;
>         unsigned char           unused[2];
> +       char                    *name;                  /* port name */
>         struct attribute_group  *attr_group;            /* port specific attributes */
>         const struct attribute_group **tty_groups;      /* all attributes (serial core use only) */
>         struct serial_rs485     rs485;
> --
> 2.11.0
>



-- 
With Best Regards,
Andy Shevchenko