From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754948AbdCTMez (ORCPT ); Mon, 20 Mar 2017 08:34:55 -0400 Received: from pandora.armlinux.org.uk ([78.32.30.218]:33090 "EHLO pandora.armlinux.org.uk" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754771AbdCTMcd (ORCPT ); Mon, 20 Mar 2017 08:32:33 -0400 Date: Mon, 20 Mar 2017 12:32:04 +0000 From: Russell King - ARM Linux To: Andy Shevchenko Cc: Robin Murphy , Jisheng Zhang , Vignesh R , Greg Kroah-Hartman , "linux-kernel@vger.kernel.org" , "linux-serial@vger.kernel.org" , Jiri Slaby , linux-arm Mailing List Subject: Re: [PATCH] serial: 8250: 8250_core: Fix irq name for 8250 serial irq Message-ID: <20170320123204.GI21222@n2100.armlinux.org.uk> References: <20170316122653.19930-1-vigneshr@ti.com> <20170316133632.GV21222@n2100.armlinux.org.uk> <9a6dcf55-0ace-e617-73bc-0472a939b36c@arm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, Mar 16, 2017 at 10:58:54PM +0200, Andy Shevchenko wrote: > On Thu, Mar 16, 2017 at 4:55 PM, Robin Murphy wrote: > > On 16/03/17 13:36, Russell King - ARM Linux wrote: > >> On Thu, Mar 16, 2017 at 05:56:53PM +0530, Vignesh R wrote: > >>> Using dev_name() as irq name during request_irq() might be misleading in > >>> case of serial over PCI. Therefore use a better alternative name for > >>> identifying serial port irqs as "serial" appended with serial_index of > >>> the port. This ensures that "serial" string is always present in irq > >>> name while port index will help in distinguishing b/w different ports. > >> > >> Wouldn't it be better to use the device name (iow, ttySx) rather than > >> "serialx" ? > >> > >> Maybe a helper function in serial_core.c to format the device name into > >> a supplied string, which can be re-used elsewhere, eg, uart_report_port() > >> and uart_suspend_port(). IOW: > >> > >> const char *uart_port_name(char *buf, size_t n, struct uart_driver *drv, > >> struct uart_port *port) > >> { > >> snprintf(buf, n, "%s%d", drv->dev_name, > >> drv->tty_driver->name_base + port->line); > >> > >> return buf; > >> } > >> > >> which means you can do this: > >> > >> char name[16]; > >> > >> request_irq(..., uart_port_name(name, sizeof(name), driver, port), ...) > >> > >> which also avoids the allocation. > > > > ...and makes 'cat /proc/interrupts' particularly fun later: > > > > 8: 0 GICv2 72 Level � �h ����V! > > > > Unless a suitably long-lived string already exists somewhere else in the > > serial core, the allocation is unavoidable, although kasprintf() (or its > > devm_ variant) might make matters a little simpler. > > What prevents us to create a field in uart_port (uart8250_port?) where > we put the uart_port_name() for future use as long as uart_port is > alive? Probably a good idea - I didn't check whether request_irq() just uses the pointer to the string or takes a copy of the string (I should've done before making the suggestion...) -- RMK's Patch system: http://www.armlinux.org.uk/developer/patches/ FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up according to speedtest.net.