[PATCH v2 5/5] tty/serial: Add earlycon support for Spreadtrum serial driver

One Thousand Gnomes
2014-10-21T12:26:30Z

On Mon, 20 Oct 2014 18:23:38 +0800
Orson Zhai <orsonzhai@gmail.com> wrote:

> On Sun, Oct 19, 2014 at 5:06 AM, One Thousand Gnomes
> <gnomes@lxorguk.ukuu.org.uk> wrote:
> > On Fri, 17 Oct 2014 17:54:25 +0800
> > Chunyan Zhang <chunyan.zhang@spreadtrum.com> 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