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

Orson Zhai
2014-10-21T13:34:41Z

On Tue, Oct 21, 2014 at 8:26 PM, One Thousand Gnomes
<gnomes@lxorguk.ukuu.org.uk> wrote:
> 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.

This make much more sense, thanks for you explanation.

> Having these conflicting definitions will
> cause confusion. Better they have prefixes.
>
> Alan
>