Re: [Qemu-devel] [PATCH v1 2/3] xilinx_spips: Seperate the state struct into a header

[Peter Crosthwaite]

On Tue, Sep 29, 2015 at 4:03 PM, Alistair Francis
<address@hidden> wrote:
> Seperate out the XilinxSPIPS struct into a seperate header
> file.
>
> Signed-off-by: Alistair Francis <address@hidden>
> ---
>
>  hw/ssi/xilinx_spips.c         | 104 +-------------------------------
>  include/hw/ssi/xilinx_spips.h | 134 
> ++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 135 insertions(+), 103 deletions(-)
>  create mode 100644 include/hw/ssi/xilinx_spips.h
>
> diff --git a/hw/ssi/xilinx_spips.c b/hw/ssi/xilinx_spips.c
> index e9471ff..417c581 100644
> --- a/hw/ssi/xilinx_spips.c
> +++ b/hw/ssi/xilinx_spips.c
> @@ -26,9 +26,8 @@
>  #include "sysemu/sysemu.h"
>  #include "hw/ptimer.h"
>  #include "qemu/log.h"
> -#include "qemu/fifo8.h"
> -#include "hw/ssi/ssi.h"
>  #include "qemu/bitops.h"
> +#include "hw/ssi/xilinx_spips.h"
>
>  #ifndef XILINX_SPIPS_ERR_DEBUG
>  #define XILINX_SPIPS_ERR_DEBUG 0
> @@ -41,70 +40,6 @@
>      } \
>  } while (0);
>
> -/* config register */
> -#define R_CONFIG            (0x00 / 4)
> -#define IFMODE              (1U << 31)
> -#define ENDIAN              (1 << 26)

Some of these macro names are too generic for the global namespace.
Until we have a proper solution on how to handle namespace collisions
for programmers model macros, I suggest taking the bare minimum to the
header (which is going to whatever is used for the struct def itself).
Preface those defs with XLNX_SPIPS_ accordingly (if there are any -
probably just R_MAX).

Regards,
Peter

> -#define MODEFAIL_GEN_EN     (1 << 17)
> -#define MAN_START_COM       (1 << 16)
> -#define MAN_START_EN        (1 << 15)
> -#define MANUAL_CS           (1 << 14)
> -#define CS                  (0xF << 10)
> -#define CS_SHIFT            (10)
> -#define PERI_SEL            (1 << 9)
> -#define REF_CLK             (1 << 8)
> -#define FIFO_WIDTH          (3 << 6)
> -#define BAUD_RATE_DIV       (7 << 3)
> -#define CLK_PH              (1 << 2)
> -#define CLK_POL             (1 << 1)
> -#define MODE_SEL            (1 << 0)
> -#define R_CONFIG_RSVD       (0x7bf40000)
> -
> -/* interrupt mechanism */
> -#define R_INTR_STATUS       (0x04 / 4)
> -#define R_INTR_EN           (0x08 / 4)
> -#define R_INTR_DIS          (0x0C / 4)
> -#define R_INTR_MASK         (0x10 / 4)
> -#define IXR_TX_FIFO_UNDERFLOW   (1 << 6)
> -#define IXR_RX_FIFO_FULL        (1 << 5)
> -#define IXR_RX_FIFO_NOT_EMPTY   (1 << 4)
> -#define IXR_TX_FIFO_FULL        (1 << 3)
> -#define IXR_TX_FIFO_NOT_FULL    (1 << 2)
> -#define IXR_TX_FIFO_MODE_FAIL   (1 << 1)
> -#define IXR_RX_FIFO_OVERFLOW    (1 << 0)
> -#define IXR_ALL                 ((IXR_TX_FIFO_UNDERFLOW<<1)-1)
> -
> -#define R_EN                (0x14 / 4)
> -#define R_DELAY             (0x18 / 4)
> -#define R_TX_DATA           (0x1C / 4)
> -#define R_RX_DATA           (0x20 / 4)
> -#define R_SLAVE_IDLE_COUNT  (0x24 / 4)
> -#define R_TX_THRES          (0x28 / 4)
> -#define R_RX_THRES          (0x2C / 4)
> -#define R_TXD1              (0x80 / 4)
> -#define R_TXD2              (0x84 / 4)
> -#define R_TXD3              (0x88 / 4)
> -
> -#define R_LQSPI_CFG         (0xa0 / 4)
> -#define R_LQSPI_CFG_RESET       0x03A002EB
> -#define LQSPI_CFG_LQ_MODE       (1U << 31)
> -#define LQSPI_CFG_TWO_MEM       (1 << 30)
> -#define LQSPI_CFG_SEP_BUS       (1 << 30)
> -#define LQSPI_CFG_U_PAGE        (1 << 28)
> -#define LQSPI_CFG_MODE_EN       (1 << 25)
> -#define LQSPI_CFG_MODE_WIDTH    8
> -#define LQSPI_CFG_MODE_SHIFT    16
> -#define LQSPI_CFG_DUMMY_WIDTH   3
> -#define LQSPI_CFG_DUMMY_SHIFT   8
> -#define LQSPI_CFG_INST_CODE     0xFF
> -
> -#define R_LQSPI_STS         (0xA4 / 4)
> -#define LQSPI_STS_WR_RECVD      (1 << 1)
> -
> -#define R_MOD_ID            (0xFC / 4)
> -
> -#define R_MAX (R_MOD_ID+1)
> -
>  /* size of TXRX FIFOs */
>  #define RXFF_A          32
>  #define TXFF_A          32
> @@ -135,30 +70,6 @@ typedef enum {
>  } FlashCMD;
>
>  typedef struct {
> -    SysBusDevice parent_obj;
> -
> -    MemoryRegion iomem;
> -    MemoryRegion mmlqspi;
> -
> -    qemu_irq irq;
> -    int irqline;
> -
> -    uint8_t num_cs;
> -    uint8_t num_busses;
> -
> -    uint8_t snoop_state;
> -    qemu_irq *cs_lines;
> -    SSIBus **spi;
> -
> -    Fifo8 rx_fifo;
> -    Fifo8 tx_fifo;
> -
> -    uint8_t num_txrx_bytes;
> -
> -    uint32_t regs[R_MAX];
> -} XilinxSPIPS;
> -
> -typedef struct {
>      XilinxSPIPS parent_obj;
>
>      uint8_t lqspi_buf[LQSPI_CACHE_SIZE];
> @@ -174,19 +85,6 @@ typedef struct XilinxSPIPSClass {
>      uint32_t tx_fifo_size;
>  } XilinxSPIPSClass;
>
> -#define TYPE_XILINX_SPIPS "xlnx.ps7-spi"
> -#define TYPE_XILINX_QSPIPS "xlnx.ps7-qspi"
> -
> -#define XILINX_SPIPS(obj) \
> -     OBJECT_CHECK(XilinxSPIPS, (obj), TYPE_XILINX_SPIPS)
> -#define XILINX_SPIPS_CLASS(klass) \
> -     OBJECT_CLASS_CHECK(XilinxSPIPSClass, (klass), TYPE_XILINX_SPIPS)
> -#define XILINX_SPIPS_GET_CLASS(obj) \
> -     OBJECT_GET_CLASS(XilinxSPIPSClass, (obj), TYPE_XILINX_SPIPS)
> -
> -#define XILINX_QSPIPS(obj) \
> -     OBJECT_CHECK(XilinxQSPIPS, (obj), TYPE_XILINX_QSPIPS)
> -
>  static inline int num_effective_busses(XilinxSPIPS *s)
>  {
>      return (s->regs[R_LQSPI_CFG] & LQSPI_CFG_SEP_BUS &&
> diff --git a/include/hw/ssi/xilinx_spips.h b/include/hw/ssi/xilinx_spips.h
> new file mode 100644
> index 0000000..31671ec
> --- /dev/null
> +++ b/include/hw/ssi/xilinx_spips.h
> @@ -0,0 +1,134 @@
> +/*
> + * Header file for the Xilinx Zynq SPI controller
> + *
> + * Copyright (C) 2015 Xilinx Inc
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a 
> copy
> + * of this software and associated documentation files (the "Software"), to 
> deal
> + * in the Software without restriction, including without limitation the 
> rights
> + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
> + * copies of the Software, and to permit persons to whom the Software is
> + * furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice shall be included in
> + * all copies or substantial portions of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING 
> FROM,
> + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
> + * THE SOFTWARE.
> + */
> +
> +#ifndef XLNX_SPIPS_H
> +#define XLNX_SPIPS_H
> +
> +#include "hw/ssi/ssi.h"
> +#include "qemu/fifo8.h"
> +
> +typedef struct XilinxSPIPS XilinxSPIPS;
> +
> +/* config register */
> +#define R_CONFIG                (0x00 / 4)
> +#define IFMODE                  (1U << 31)
> +#define ENDIAN                  (1 << 26)
> +#define MODEFAIL_GEN_EN         (1 << 17)
> +#define MAN_START_COM           (1 << 16)
> +#define MAN_START_EN            (1 << 15)
> +#define MANUAL_CS               (1 << 14)
> +#define CS                      (0xF << 10)
> +#define CS_SHIFT                (10)
> +#define PERI_SEL                (1 << 9)
> +#define REF_CLK                 (1 << 8)
> +#define FIFO_WIDTH              (3 << 6)
> +#define BAUD_RATE_DIV           (7 << 3)
> +#define CLK_PH                  (1 << 2)
> +#define CLK_POL                 (1 << 1)
> +#define MODE_SEL                (1 << 0)
> +#define R_CONFIG_RSVD           (0x7bf40000)
> +
> +/* interrupt mechanism */
> +#define R_INTR_STATUS           (0x04 / 4)
> +#define R_INTR_EN               (0x08 / 4)
> +#define R_INTR_DIS              (0x0C / 4)
> +#define R_INTR_MASK             (0x10 / 4)
> +#define IXR_TX_FIFO_UNDERFLOW   (1 << 6)
> +#define IXR_RX_FIFO_FULL        (1 << 5)
> +#define IXR_RX_FIFO_NOT_EMPTY   (1 << 4)
> +#define IXR_TX_FIFO_FULL        (1 << 3)
> +#define IXR_TX_FIFO_NOT_FULL    (1 << 2)
> +#define IXR_TX_FIFO_MODE_FAIL   (1 << 1)
> +#define IXR_RX_FIFO_OVERFLOW    (1 << 0)
> +#define IXR_ALL                 ((IXR_TX_FIFO_UNDERFLOW << 1) - 1)
> +
> +#define R_EN                    (0x14 / 4)
> +#define R_DELAY                 (0x18 / 4)
> +#define R_TX_DATA               (0x1C / 4)
> +#define R_RX_DATA               (0x20 / 4)
> +#define R_SLAVE_IDLE_COUNT      (0x24 / 4)
> +#define R_TX_THRES              (0x28 / 4)
> +#define R_RX_THRES              (0x2C / 4)
> +#define R_TXD1                  (0x80 / 4)
> +#define R_TXD2                  (0x84 / 4)
> +#define R_TXD3                  (0x88 / 4)
> +
> +#define R_LQSPI_CFG             (0xa0 / 4)
> +#define R_LQSPI_CFG_RESET       0x03A002EB
> +#define LQSPI_CFG_LQ_MODE       (1U << 31)
> +#define LQSPI_CFG_TWO_MEM       (1 << 30)
> +#define LQSPI_CFG_SEP_BUS       (1 << 30)
> +#define LQSPI_CFG_U_PAGE        (1 << 28)
> +#define LQSPI_CFG_MODE_EN       (1 << 25)
> +#define LQSPI_CFG_MODE_WIDTH    8
> +#define LQSPI_CFG_MODE_SHIFT    16
> +#define LQSPI_CFG_DUMMY_WIDTH   3
> +#define LQSPI_CFG_DUMMY_SHIFT   8
> +#define LQSPI_CFG_INST_CODE     0xFF
> +
> +#define R_LQSPI_STS             (0xA4 / 4)
> +#define LQSPI_STS_WR_RECVD      (1 << 1)
> +
> +#define R_MOD_ID                (0xFC / 4)
> +
> +#define R_MAX                   (R_MOD_ID + 1)
> +
> +struct XilinxSPIPS {
> +    SysBusDevice parent_obj;
> +
> +    MemoryRegion iomem;
> +    MemoryRegion mmlqspi;
> +
> +    qemu_irq irq;
> +    int irqline;
> +
> +    uint8_t num_cs;
> +    uint8_t num_busses;
> +
> +    uint8_t snoop_state;
> +    qemu_irq *cs_lines;
> +    SSIBus **spi;
> +
> +    Fifo8 rx_fifo;
> +    Fifo8 tx_fifo;
> +
> +    uint8_t num_txrx_bytes;
> +
> +    uint32_t regs[R_MAX];
> +};
> +
> +#define TYPE_XILINX_SPIPS "xlnx.ps7-spi"
> +#define TYPE_XILINX_QSPIPS "xlnx.ps7-qspi"
> +
> +#define XILINX_SPIPS(obj) \
> +     OBJECT_CHECK(XilinxSPIPS, (obj), TYPE_XILINX_SPIPS)
> +#define XILINX_SPIPS_CLASS(klass) \
> +     OBJECT_CLASS_CHECK(XilinxSPIPSClass, (klass), TYPE_XILINX_SPIPS)
> +#define XILINX_SPIPS_GET_CLASS(obj) \
> +     OBJECT_GET_CLASS(XilinxSPIPSClass, (obj), TYPE_XILINX_SPIPS)
> +
> +#define XILINX_QSPIPS(obj) \
> +     OBJECT_CHECK(XilinxQSPIPS, (obj), TYPE_XILINX_QSPIPS)
> +
> +#endif /* XLNX_SPIPS_H */
> --
> 1.9.1
>