From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751339AbdCYKfl (ORCPT ); Sat, 25 Mar 2017 06:35:41 -0400 Received: from lelnx194.ext.ti.com ([198.47.27.80]:50074 "EHLO lelnx194.ext.ti.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750766AbdCYKfi (ORCPT ); Sat, 25 Mar 2017 06:35:38 -0400 Subject: Re: [PATCH v2 1/2] tty: serial_core: Add name field to uart_port struct To: Andy Shevchenko References: <20170324052800.11526-1-vigneshr@ti.com> <20170324052800.11526-2-vigneshr@ti.com> CC: Greg Kroah-Hartman , Jiri Slaby , Jisheng Zhang , "linux-serial@vger.kernel.org" , linux-arm Mailing List , "linux-kernel@vger.kernel.org" From: Vignesh R Message-ID: Date: Sat, 25 Mar 2017 16:03:31 +0530 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:45.0) Gecko/20100101 Thunderbird/45.8.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 3/24/2017 5:16 PM, Andy Shevchenko wrote: > On Fri, Mar 24, 2017 at 7:27 AM, Vignesh R 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 > > Just in case, have you checked if there any possible scenarios where > memory will be leaked? I ran kmemleak scan after opening and closing a serial port and did not see any memleak report. Also uport->tty_groups is allocated and deallocated in the same functions uport->name. > >> Signed-off-by: Vignesh R >> --- >> >> 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 >> > > > From mboxrd@z Thu Jan 1 00:00:00 1970 From: Vignesh R Subject: Re: [PATCH v2 1/2] tty: serial_core: Add name field to uart_port struct Date: Sat, 25 Mar 2017 16:03:31 +0530 Message-ID: References: <20170324052800.11526-1-vigneshr@ti.com> <20170324052800.11526-2-vigneshr@ti.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=m.gmane.org@lists.infradead.org To: Andy Shevchenko Cc: Jisheng Zhang , Greg Kroah-Hartman , "linux-kernel@vger.kernel.org" , "linux-serial@vger.kernel.org" , Jiri Slaby , linux-arm Mailing List List-Id: linux-serial@vger.kernel.org On 3/24/2017 5:16 PM, Andy Shevchenko wrote: > On Fri, Mar 24, 2017 at 7:27 AM, Vignesh R 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 > > Just in case, have you checked if there any possible scenarios where > memory will be leaked? I ran kmemleak scan after opening and closing a serial port and did not see any memleak report. Also uport->tty_groups is allocated and deallocated in the same functions uport->name. > >> Signed-off-by: Vignesh R >> --- >> >> 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 >> > > > From mboxrd@z Thu Jan 1 00:00:00 1970 From: vigneshr@ti.com (Vignesh R) Date: Sat, 25 Mar 2017 16:03:31 +0530 Subject: [PATCH v2 1/2] tty: serial_core: Add name field to uart_port struct In-Reply-To: References: <20170324052800.11526-1-vigneshr@ti.com> <20170324052800.11526-2-vigneshr@ti.com> Message-ID: To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org On 3/24/2017 5:16 PM, Andy Shevchenko wrote: > On Fri, Mar 24, 2017 at 7:27 AM, Vignesh R 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 > > Just in case, have you checked if there any possible scenarios where > memory will be leaked? I ran kmemleak scan after opening and closing a serial port and did not see any memleak report. Also uport->tty_groups is allocated and deallocated in the same functions uport->name. > >> Signed-off-by: Vignesh R >> --- >> >> 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 >> > > >