Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932413AbaJUMeL (ORCPT ); Tue, 21 Oct 2014 08:34:11 -0400 Received: from 251.110.2.81.in-addr.arpa ([81.2.110.251]:36144 "EHLO lxorguk.ukuu.org.uk" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932304AbaJUMeJ (ORCPT ); Tue, 21 Oct 2014 08:34:09 -0400 Date: Tue, 21 Oct 2014 13:26:30 +0100 From: One Thousand Gnomes To: Orson Zhai Cc: Chunyan Zhang , Catalin Marinas , "gregkh@linuxfoundation.org" , "ijc+devicetree@hellion.org.uk" , "jslaby@suse.cz" , Kumar Gala , Mark Brown , Mark Rutland , "m-karicheri2@ti.com" , Pawel Moll , Ramkumar Ramachandra , "rrichter@cavium.com" , "robh+dt@kernel.org" , Will Deacon , "Geng Ren ( =?UTF-8?B?5Lu76LWT?=)" , "Zhizhou Zhang ( =?UTF-8?B?5byg5rK75rSy?=)" , "devicetree@vger.kernel.org" , "linux-arm-kernel@lists.infradead.org" , "linux-kernel@vger.kernel.org" , sprdlinux Subject: Re: [PATCH v2 5/5] tty/serial: Add earlycon support for Spreadtrum serial driver Message-ID: <20141021132630.79ce15ba@alan.etchedpixels.co.uk> In-Reply-To: References: <1413539665-11484-1-git-send-email-chunyan.zhang@spreadtrum.com> <1413539665-11484-6-git-send-email-chunyan.zhang@spreadtrum.com> <20141018220615.33643542@alan.etchedpixels.co.uk> Organization: Intel Corporation X-Mailer: Claws Mail 3.9.3 (GTK+ 2.24.23; x86_64-pc-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Mon, 20 Oct 2014 18:23:38 +0800 Orson Zhai wrote: > On Sun, Oct 19, 2014 at 5:06 AM, One Thousand Gnomes > wrote: > > On Fri, 17 Oct 2014 17:54:25 +0800 > > Chunyan Zhang wrote: > > > >> Add serial driver for spreadtrum sharkl platform with earlycon > >> support at first. > > > >> +#define UART_TXD 0x0000 > >> +#define UART_RXD 0x0004 > >> +#define UART_STS0 0x0008 > >> +#define UART_STS1 0x000C > >> +#define UART_IEN 0x0010 > >> +#define UART_ICLR 0x0014 > >> +#define UART_CTL0 0x0018 > >> +#define UART_CTL1 0x001C > >> +#define UART_CTL2 0x0020 > >> +#define UART_CLKD0 0x0024 > >> +#define UART_CLKD1 0x0028 > >> +#define UART_STS2 0x002C > >> + > >> +/*line status */ > >> +#define UART_LSR_TX_OVER (0x1<<15) > > > > Given we use UART_ for all the 8250 defines it might be better to use > > something else - SHARK_LSR_TX_OVER etc to avoid future confusion > > > > Does it matter if those macro are only used in the specific c file? > >From my point of view, private register macro could be treated as > static definitions like static variable or functions in C file. > I also noted that many people use a prefix for the other > manufacturers' macro definition in source file . > I have no intention to break the habit of kernel but just for discussion. Your problem is this. In the kernel headers are definitions like include/uapi/linux/serial_reg.h:#define UART_TX 0 /* Out: Transmit buffer */ you are adding identical (but conflicting in places) defines. If some random change in the include files causes serial_reg.h to be included by a header you include then it will break. In addition people habitually use tools like LXR to figure out where defines and functions are. Having these conflicting definitions will cause confusion. Better they have prefixes. Alan -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/