From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:39029) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZrqMz-0004Kt-4J for qemu-devel@nongnu.org; Thu, 29 Oct 2015 12:48:10 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZrqMx-0005XR-Ej for qemu-devel@nongnu.org; Thu, 29 Oct 2015 12:48:09 -0400 Received: from mail-oi0-x233.google.com ([2607:f8b0:4003:c06::233]:36260) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZrqMx-0005X9-4C for qemu-devel@nongnu.org; Thu, 29 Oct 2015 12:48:07 -0400 Received: by oiao187 with SMTP id o187so41457678oia.3 for ; Thu, 29 Oct 2015 09:48:06 -0700 (PDT) MIME-Version: 1.0 Sender: alistair23@gmail.com In-Reply-To: References: From: Alistair Francis Date: Thu, 29 Oct 2015 09:47:37 -0700 Message-ID: Content-Type: text/plain; charset=UTF-8 Subject: Re: [Qemu-devel] [PATCH v3 3/5] xilinx_spips: Seperate the state struct into a header List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Peter Crosthwaite Cc: Edgar Iglesias , Peter Maydell , "Edgar E. Iglesias" , "qemu-devel@nongnu.org Developers" , Alistair Francis On Wed, Oct 28, 2015 at 6:47 PM, Peter Crosthwaite wrote: > > > On Wed, Oct 28, 2015 at 10:32 AM, Alistair Francis > wrote: >> >> Seperate out the XilinxSPIPS struct into a seperate header >> file. >> >> Signed-off-by: Alistair Francis >> --- >> V2: >> - Only split out required #defines >> - Prefix XLNX_SPIPS_ >> >> hw/ssi/xilinx_spips.c | 54 ++++--------------------------- >> include/hw/ssi/xilinx_spips.h | 74 >> +++++++++++++++++++++++++++++++++++++++++++ >> 2 files changed, 81 insertions(+), 47 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..1b7b3fb 100644 >> --- a/hw/ssi/xilinx_spips.c >> +++ b/hw/ssi/xilinx_spips.c >> @@ -29,6 +29,7 @@ >> #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 >> @@ -101,10 +102,6 @@ >> #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 +132,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 +147,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 && >> @@ -257,7 +217,7 @@ static void xilinx_spips_reset(DeviceState *d) >> XilinxSPIPS *s = XILINX_SPIPS(d); >> >> int i; >> - for (i = 0; i < R_MAX; i++) { >> + for (i = 0; i < XLNX_SPIPS_R_MAX; i++) { >> s->regs[i] = 0; >> } >> >> @@ -269,7 +229,7 @@ static void xilinx_spips_reset(DeviceState *d) >> s->regs[R_TX_THRES] = 1; >> s->regs[R_RX_THRES] = 1; >> /* FIXME: move magic number definition somewhere sensible */ >> - s->regs[R_MOD_ID] = 0x01090106; >> + s->regs[XLNX_SPIPS_R_MOD_ID] = 0x01090106; >> s->regs[R_LQSPI_CFG] = R_LQSPI_CFG_RESET; >> s->snoop_state = SNOOP_CHECKING; >> xilinx_spips_update_ixr(s); >> @@ -427,7 +387,7 @@ static uint64_t xilinx_spips_read(void *opaque, hwaddr >> addr, >> case R_SLAVE_IDLE_COUNT: >> mask = 0xFF; >> break; >> - case R_MOD_ID: >> + case XLNX_SPIPS_R_MOD_ID: >> mask = 0x01FFFFFF; >> break; >> case R_INTR_EN: >> @@ -500,7 +460,7 @@ static void xilinx_spips_write(void *opaque, hwaddr >> addr, >> break; >> case R_RX_DATA: >> case R_INTR_MASK: >> - case R_MOD_ID: >> + case XLNX_SPIPS_R_MOD_ID: >> mask = 0; >> break; >> case R_TX_DATA: >> @@ -664,7 +624,7 @@ static void xilinx_spips_realize(DeviceState *dev, >> Error **errp) >> } >> >> memory_region_init_io(&s->iomem, OBJECT(s), xsc->reg_ops, s, >> - "spi", R_MAX*4); >> + "spi", XLNX_SPIPS_R_MAX*4); >> sysbus_init_mmio(sbd, &s->iomem); >> >> s->irqline = -1; >> @@ -708,7 +668,7 @@ static const VMStateDescription vmstate_xilinx_spips = >> { >> .fields = (VMStateField[]) { >> VMSTATE_FIFO8(tx_fifo, XilinxSPIPS), >> VMSTATE_FIFO8(rx_fifo, XilinxSPIPS), >> - VMSTATE_UINT32_ARRAY(regs, XilinxSPIPS, R_MAX), >> + VMSTATE_UINT32_ARRAY(regs, XilinxSPIPS, XLNX_SPIPS_R_MAX), >> VMSTATE_UINT8(snoop_state, XilinxSPIPS), >> VMSTATE_END_OF_LIST() >> } >> diff --git a/include/hw/ssi/xilinx_spips.h b/include/hw/ssi/xilinx_spips.h >> new file mode 100644 >> index 0000000..f01d276 >> --- /dev/null >> +++ b/include/hw/ssi/xilinx_spips.h >> @@ -0,0 +1,74 @@ >> +/* >> + * 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; >> + >> +#define XLNX_SPIPS_R_MOD_ID (0xFC / 4) >> + >> +#define XLNX_SPIPS_R_MAX (XLNX_SPIPS_R_MOD_ID + 1) > > > This incremental definition of R_MAX doesn't really play well with splitting > R_MAX off as you need to take both R_MAX and just the one programmers model > def to the header. It makes R_MOD_ID inconsistent with the others. Instead I > think just leave R_MOD_ID behind (without rename) in the C file, and > > #define XLNX_SPIPS_R_MAX (0x100/4) Makes sense. Will fix in the next version. Thanks, Alistair > > Regards, > Peter > >> + >> +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[XLNX_SPIPS_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 */ >> -- >> 2.5.0 >> >