[PATCH 0/2] serial: Add name field to uart_port

[PATCH 0/2] serial: Add name field to uart_port

[Vignesh R]

This series adds name attribute to uart_port struct inorder to store
name of the tty port that will help in identify different uart
instances.
This patch series is based on discussion here[1].


[1] https://www.spinics.net/lists/arm-kernel/msg569188.html

Vignesh R (2):
  tty: serial_core: Add name field to uart_port struct
  serial: 8250: 8250_core: Fix irq name for 8250 serial irq

 drivers/tty/serial/8250/8250_core.c | 2 +-
 drivers/tty/serial/serial_core.c    | 2 ++
 include/linux/serial_core.h         | 1 +
 3 files changed, 4 insertions(+), 1 deletion(-)

-- 
2.11.0

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

[Vignesh R]

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().

Signed-off-by: Vignesh R <vigneshr@ti.com>
---
 drivers/tty/serial/serial_core.c | 2 ++
 include/linux/serial_core.h      | 1 +
 2 files changed, 3 insertions(+)

diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
index 0fb3f7cce62a..8b0fff74b190 100644
--- a/drivers/tty/serial/serial_core.c
+++ b/drivers/tty/serial/serial_core.c
@@ -2744,6 +2744,8 @@ 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;
+	snprintf(uport->name, sizeof(uport->name), "%s%d", drv->dev_name,
+		 drv->tty_driver->name_base + uport->line);
 
 	/*
 	 * If this port is a console, then the spinlock is already
diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h
index 58484fb35cc8..821f97ce3ae2 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[20];		/* 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

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

[Andy Shevchenko]

On Tue, Mar 21, 2017 at 3:36 PM, Vignesh R <vigneshr@ti.com> 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

uart -> 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

irq(s) -> IRQ(s)

> uart_add_one_port().

> +       snprintf(uport->name, sizeof(uport->name), "%s%d", drv->dev_name,
> +                drv->tty_driver->name_base + uport->line);

This is fragile. dev_name is defined as pointer, so, it would be any length.
So, better to use [devm_]kasprintf().

-- 
With Best Regards,
Andy Shevchenko

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

[Vignesh R]

On Thursday 23 March 2017 12:15 AM, Andy Shevchenko wrote:
> On Tue, Mar 21, 2017 at 3:36 PM, Vignesh R <vigneshr@ti.com> 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
> 
> uart -> 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
> 
> irq(s) -> IRQ(s)
> 
>> uart_add_one_port().
> 
>> +       snprintf(uport->name, sizeof(uport->name), "%s%d", drv->dev_name,
>> +                drv->tty_driver->name_base + uport->line);
> 
> This is fragile. dev_name is defined as pointer, so, it would be any length.
> So, better to use [devm_]kasprintf().
> 

Ok, will you use kasprintf() (and corresponding kfree()) as device is
not yet allocated at this stage to use devm_*.

-- 
Regards
Vignesh

[PATCH 2/2] serial: 8250: 8250_core: Fix irq name for 8250 serial irq

[Vignesh R]

Using dev_name() as irq name during request_irq() might be misleading in
case of serial over PCI. Therefore identify serial port irqs using
uart_port's name field. This will help mapping irqs to appropriate
ttySN(where N is the serial port index) instances.

Signed-off-by: Vignesh R <vigneshr@ti.com>
---
 drivers/tty/serial/8250/8250_core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c
index f83b69f30987..48a07e2f617f 100644
--- a/drivers/tty/serial/8250/8250_core.c
+++ b/drivers/tty/serial/8250/8250_core.c
@@ -218,7 +218,7 @@ static int serial_link_irq_chain(struct uart_8250_port *up)
 		spin_unlock_irq(&i->lock);
 		irq_flags |= up->port.irqflags;
 		ret = request_irq(up->port.irq, serial8250_interrupt,
-				  irq_flags, dev_name(up->port.dev), i);
+				  irq_flags, up->port.name, i);
 		if (ret < 0)
 			serial_do_unlink(i, up);
 	}
-- 
2.11.0

[PATCH 2/2] serial: 8250: 8250_core: Fix irq name for 8250 serial irq

[Andy Shevchenko]

On Tue, Mar 21, 2017 at 3:36 PM, Vignesh R <vigneshr@ti.com> wrote:
> Using dev_name() as irq name during request_irq() might be misleading in
> case of serial over PCI. Therefore identify serial port irqs using
> uart_port's name field. This will help mapping irqs to appropriate
> ttySN(where N is the serial port index) instances.
>

Based on the information in name member of uart_port it looks best
approach to me.

Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>

> Signed-off-by: Vignesh R <vigneshr@ti.com>
> ---
>  drivers/tty/serial/8250/8250_core.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c
> index f83b69f30987..48a07e2f617f 100644
> --- a/drivers/tty/serial/8250/8250_core.c
> +++ b/drivers/tty/serial/8250/8250_core.c
> @@ -218,7 +218,7 @@ static int serial_link_irq_chain(struct uart_8250_port *up)
>                 spin_unlock_irq(&i->lock);
>                 irq_flags |= up->port.irqflags;
>                 ret = request_irq(up->port.irq, serial8250_interrupt,
> -                                 irq_flags, dev_name(up->port.dev), i);
> +                                 irq_flags, up->port.name, i);
>                 if (ret < 0)
>                         serial_do_unlink(i, up);
>         }
> --
> 2.11.0
>



-- 
With Best Regards,
Andy Shevchenko

[PATCH 0/2] serial: Add name field to uart_port

[Andy Shevchenko]

On Tue, Mar 21, 2017 at 3:36 PM, Vignesh R <vigneshr@ti.com> wrote:
> This series adds name attribute to uart_port struct inorder to store
> name of the tty port that will help in identify different uart
> instances.
> This patch series is based on discussion here[1].

Please, use versioning for the series. This AFAIU should be v3, and
next (which is expecting based on comments) would be v4.

> [1] https://www.spinics.net/lists/arm-kernel/msg569188.html

-- 
With Best Regards,
Andy Shevchenko

[PATCH 0/2] serial: Add name field to uart_port

[Vignesh R]

On Thursday 23 March 2017 12:16 AM, Andy Shevchenko wrote:
> On Tue, Mar 21, 2017 at 3:36 PM, Vignesh R <vigneshr@ti.com> wrote:
>> This series adds name attribute to uart_port struct inorder to store
>> name of the tty port that will help in identify different uart
>> instances.
>> This patch series is based on discussion here[1].
> 
> Please, use versioning for the series. This AFAIU should be v3, and
> next (which is expecting based on comments) would be v4.

The very first patch I submitted was accepted. So I did not make the
previous patch as v2 as it was an incremental fix. Since, this series is
very different from previous patch, I chose not to label this as v3.
Anyways, will make the next version as v4.

Thanks for the review!


-- 
Regards
Vignesh

[PATCH 0/2] serial: Add name field to uart_port

[Andy Shevchenko]

On Thu, Mar 23, 2017 at 2:22 PM, Vignesh R <vigneshr@ti.com> wrote:
> On Thursday 23 March 2017 12:16 AM, Andy Shevchenko wrote:
>> On Tue, Mar 21, 2017 at 3:36 PM, Vignesh R <vigneshr@ti.com> wrote:
>>> This series adds name attribute to uart_port struct inorder to store
>>> name of the tty port that will help in identify different uart
>>> instances.
>>> This patch series is based on discussion here[1].
>>
>> Please, use versioning for the series. This AFAIU should be v3, and
>> next (which is expecting based on comments) would be v4.
>
> The very first patch I submitted was accepted. So I did not make the
> previous patch as v2 as it was an incremental fix. Since, this series is
> very different from previous patch, I chose not to label this as v3.

Fair enough.

> Anyways, will make the next version as v4.

No need, then it makes sense to mark it as v2.

-- 
With Best Regards,
Andy Shevchenko