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

[Orson Zhai]

On Tue, Oct 21, 2014 at 8:26 PM, One Thousand Gnomes
<gnomes@xxxxxxxxxxxxxxxxxxx> wrote:
> On Mon, 20 Oct 2014 18:23:38 +0800
> Orson Zhai <orsonzhai@xxxxxxxxx> wrote:
>
>> On Sun, Oct 19, 2014 at 5:06 AM, One Thousand Gnomes
>> <gnomes@xxxxxxxxxxxxxxxxxxx> wrote:
>> > On Fri, 17 Oct 2014 17:54:25 +0800
>> > Chunyan Zhang <chunyan.zhang@xxxxxxxxxxxxxx> 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
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/