[2/8] pl011: Move registers' definition in a separate file

---
 xen/drivers/char/pl011.c         |   48 +----------------------
 xen/include/asm-arm/pl011-uart.h |   80 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 81 insertions(+), 47 deletions(-)
 create mode 100644 xen/include/asm-arm/pl011-uart.h

Should we prefix them with PL011 or something?
 On Thu, 2013-07-25 at 17:59 +0100, Julien Grall wrote:
> ---
>  xen/drivers/char/pl011.c         |   48 +----------------------
>  xen/include/asm-arm/pl011-uart.h |   80 ++++++++++++++++++++++++++++++++++++++
>  2 files changed, 81 insertions(+), 47 deletions(-)
>  create mode 100644 xen/include/asm-arm/pl011-uart.h
> 
> diff --git a/xen/drivers/char/pl011.c b/xen/drivers/char/pl011.c
> index 9456f20..fd87e68 100644
> --- a/xen/drivers/char/pl011.c
> +++ b/xen/drivers/char/pl011.c
> @@ -28,6 +28,7 @@
>  #include <asm/device.h>
>  #include <xen/mm.h>
>  #include <xen/vmap.h>
> +#include <asm/pl011-uart.h>
>  
>  static struct pl011 {
>      unsigned int baud, clock_hz, data_bits, parity, stop_bits;
> @@ -41,53 +42,6 @@ static struct pl011 {
>      /* bool_t probing, intr_works; */
>  } pl011_com = {0};
>  
> -/* PL011 register addresses */
> -#define DR     (0x00)
> -#define RSR    (0x04)
> -#define FR     (0x18)
> -#define ILPR   (0x20)
> -#define IBRD   (0x24)
> -#define FBRD   (0x28)
> -#define LCR_H  (0x2c)
> -#define CR     (0x30)
> -#define IFLS   (0x34)
> -#define IMSC   (0x38)
> -#define RIS    (0x3c)
> -#define MIS    (0x40)
> -#define ICR    (0x44)
> -#define DMACR  (0x48)
> -
> -/* CR bits */
> -#define RXE    (1<<9) /* Receive enable */
> -#define TXE    (1<<8) /* Transmit enable */
> -#define UARTEN (1<<0) /* UART enable */
> -
> -/* FR bits */
> -#define TXFE   (1<<7) /* TX FIFO empty */
> -#define RXFE   (1<<4) /* RX FIFO empty */
> -
> -/* LCR_H bits */
> -#define SPS    (1<<7) /* Stick parity select */
> -#define FEN    (1<<4) /* FIFO enable */
> -#define STP2   (1<<3) /* Two stop bits select */
> -#define EPS    (1<<2) /* Even parity select */
> -#define PEN    (1<<1) /* Parity enable */
> -#define BRK    (1<<0) /* Send break */
> -
> -/* Interrupt bits (IMSC, MIS, ICR) */
> -#define OEI   (1<<10) /* Overrun Error interrupt mask */
> -#define BEI   (1<<9)  /* Break Error interrupt mask */
> -#define PEI   (1<<8)  /* Parity Error interrupt mask */
> -#define FEI   (1<<7)  /* Framing Error interrupt mask */
> -#define RTI   (1<<6)  /* Receive Timeout interrupt mask */
> -#define TXI   (1<<5)  /* Transmit interrupt mask */
> -#define RXI   (1<<4)  /* Receive interrupt mask */
> -#define DSRMI (1<<3)  /* nUARTDSR Modem interrupt mask */
> -#define DCDMI (1<<2)  /* nUARTDCD Modem interrupt mask */
> -#define CTSMI (1<<1)  /* nUARTCTS Modem interrupt mask */
> -#define RIMI  (1<<0)  /* nUARTRI Modem interrupt mask */
> -#define ALLI  OEI|BEI|PEI|FEI|RTI|TXI|RXI|DSRMI|DCDMI|CTSMI|RIMI
> -
>  /* These parity settings can be ORed directly into the LCR. */
>  #define PARITY_NONE  (0)
>  #define PARITY_ODD   (PEN)
> diff --git a/xen/include/asm-arm/pl011-uart.h b/xen/include/asm-arm/pl011-uart.h
> new file mode 100644
> index 0000000..8c4edd4
> --- /dev/null
> +++ b/xen/include/asm-arm/pl011-uart.h
> @@ -0,0 +1,80 @@
> +/*
> + * xen/include/asm-arm/pl011-uart.h
> + *
> + * Common constant definition between early printk and the UART driver
> + * for the pl011 UART
> + *
> + * Tim Deegan <tim@xen.org>
> + * Copyright (c) 2011 Citrix Systems.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + */
> +
> +#ifndef __ASM_ARM_PL011_H
> +#define __ASM_ARM_PL011_H
> +
> +/* PL011 register addresses */
> +#define DR     (0x00)
> +#define RSR    (0x04)
> +#define FR     (0x18)
> +#define ILPR   (0x20)
> +#define IBRD   (0x24)
> +#define FBRD   (0x28)
> +#define LCR_H  (0x2c)
> +#define CR     (0x30)
> +#define IFLS   (0x34)
> +#define IMSC   (0x38)
> +#define RIS    (0x3c)
> +#define MIS    (0x40)
> +#define ICR    (0x44)
> +#define DMACR  (0x48)
> +
> +/* CR bits */
> +#define RXE    (1<<9) /* Receive enable */
> +#define TXE    (1<<8) /* Transmit enable */
> +#define UARTEN (1<<0) /* UART enable */
> +
> +/* FR bits */
> +#define TXFE   (1<<7) /* TX FIFO empty */
> +#define RXFE   (1<<4) /* RX FIFO empty */
> +
> +/* LCR_H bits */
> +#define SPS    (1<<7) /* Stick parity select */
> +#define FEN    (1<<4) /* FIFO enable */
> +#define STP2   (1<<3) /* Two stop bits select */
> +#define EPS    (1<<2) /* Even parity select */
> +#define PEN    (1<<1) /* Parity enable */
> +#define BRK    (1<<0) /* Send break */
> +
> +/* Interrupt bits (IMSC, MIS, ICR) */
> +#define OEI   (1<<10) /* Overrun Error interrupt mask */
> +#define BEI   (1<<9)  /* Break Error interrupt mask */
> +#define PEI   (1<<8)  /* Parity Error interrupt mask */
> +#define FEI   (1<<7)  /* Framing Error interrupt mask */
> +#define RTI   (1<<6)  /* Receive Timeout interrupt mask */
> +#define TXI   (1<<5)  /* Transmit interrupt mask */
> +#define RXI   (1<<4)  /* Receive interrupt mask */
> +#define DSRMI (1<<3)  /* nUARTDSR Modem interrupt mask */
> +#define DCDMI (1<<2)  /* nUARTDCD Modem interrupt mask */
> +#define CTSMI (1<<1)  /* nUARTCTS Modem interrupt mask */
> +#define RIMI  (1<<0)  /* nUARTRI Modem interrupt mask */
> +#define ALLI  OEI|BEI|PEI|FEI|RTI|TXI|RXI|DSRMI|DCDMI|CTSMI|RIMI
> +
> +#endif /* __ASM_ARM_PL011_H */
> +
> +/*
> + * Local variables:
> + * mode: C
> + * c-file-style: "BSD"
> + * c-basic-offset: 4
> + * indent-tabs-mode: nil
> + * End:
> + */

On 07/29/2013 05:24 PM, Ian Campbell wrote:
> Should we prefix them with PL011 or something?

This header is intended to be used only in the C drivers and early printk.

But I can prefix by UART... which is the real name for the registers. I
didn't choose this solution to avoid lots of changes in the PL011 driver.

On Mon, 2013-07-29 at 17:35 +0100, Julien Grall wrote:
> On 07/29/2013 05:24 PM, Ian Campbell wrote:
> > Should we prefix them with PL011 or something?
> 
> This header is intended to be used only in the C drivers and early printk.
> 
> But I can prefix by UART... which is the real name for the registers.

"real name" in what sense? The offsets are specific to the PL011, aren't
they? Or are you saying they are common with the 16550 offsets?

It's bad enough that the 16550 header uses UART prefix without adding a
second potentially conflicting user of that namespace

> I didn't choose this solution to avoid lots of changes in the PL011 driver.

That's a reasonable point. Lets leave them alone until it becomes a real
problem.

Ian.

On 30 July 2013 11:00, Ian Campbell <Ian.Campbell@citrix.com> wrote:
> On Mon, 2013-07-29 at 17:35 +0100, Julien Grall wrote:
>> On 07/29/2013 05:24 PM, Ian Campbell wrote:
>> > Should we prefix them with PL011 or something?
>>
>> This header is intended to be used only in the C drivers and early printk.
>>
>> But I can prefix by UART... which is the real name for the registers.
>
> "real name" in what sense? The offsets are specific to the PL011, aren't
> they? Or are you saying they are common with the 16550 offsets?

Theses offsets are specific to the PL011. For "real name" I mean the
name used in the documentation. For instance DR is named UARTDR...

> It's bad enough that the 16550 header uses UART prefix without adding a
> second potentially conflicting user of that namespace
>
>> I didn't choose this solution to avoid lots of changes in the PL011 driver.
>
> That's a reasonable point. Lets leave them alone until it becomes a real
> problem.
>
> Ian.
>
>