[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH-2.12 v2 2/3] xilinx_spips: Set all of the reset
From: |
francisco iglesias |
Subject: |
Re: [Qemu-devel] [PATCH-2.12 v2 2/3] xilinx_spips: Set all of the reset values |
Date: |
Mon, 11 Dec 2017 22:17:51 +0100 |
On 11 December 2017 at 18:27, Alistair Francis <address@hidden>
wrote:
> On Wed, Dec 6, 2017 at 3:39 PM, francisco iglesias
> <address@hidden> wrote:
> > Hi Alistair,
> >
> > On 6 December 2017 at 23:22, Alistair Francis <
> address@hidden>
> > wrote:
> >>
> >> Following the ZynqMP register spec let's ensure that all reset values
> >> are set.
> >>
> >> Signed-off-by: Alistair Francis <address@hidden>
> >> ---
> >> V2:
> >> - Don't bother double setting registers
> >>
> >> hw/ssi/xilinx_spips.c | 35 ++++++++++++++++++++++++++++++-----
> >> include/hw/ssi/xilinx_spips.h | 2 +-
> >> 2 files changed, 31 insertions(+), 6 deletions(-)
> >>
> >> diff --git a/hw/ssi/xilinx_spips.c b/hw/ssi/xilinx_spips.c
> >> index 899db814ee..b8182cfd74 100644
> >> --- a/hw/ssi/xilinx_spips.c
> >> +++ b/hw/ssi/xilinx_spips.c
> >> @@ -66,6 +66,7 @@
> >>
> >> /* interrupt mechanism */
> >> #define R_INTR_STATUS (0x04 / 4)
> >> +#define R_INTR_STATUS_RESET (0x104)
> >> #define R_INTR_EN (0x08 / 4)
> >> #define R_INTR_DIS (0x0C / 4)
> >> #define R_INTR_MASK (0x10 / 4)
> >> @@ -102,6 +103,9 @@
> >> #define R_SLAVE_IDLE_COUNT (0x24 / 4)
> >> #define R_TX_THRES (0x28 / 4)
> >> #define R_RX_THRES (0x2C / 4)
> >> +#define R_GPIO (0x30 / 4)
> >> +#define R_LPBK_DLY_ADJ (0x38 / 4)
> >> +#define R_LPBK_DLY_ADJ_RESET (0x33)
> >> #define R_TXD1 (0x80 / 4)
> >> #define R_TXD2 (0x84 / 4)
> >> #define R_TXD3 (0x88 / 4)
> >> @@ -140,8 +144,12 @@
> >> #define R_GQSPI_IER (0x108 / 4)
> >> #define R_GQSPI_IDR (0x10c / 4)
> >> #define R_GQSPI_IMR (0x110 / 4)
> >> +#define R_GQSPI_IMR_RESET (0xfbe)
> >> #define R_GQSPI_TX_THRESH (0x128 / 4)
> >> #define R_GQSPI_RX_THRESH (0x12c / 4)
> >> +#define R_GQSPI_GPIO_THRESH (0x130 / 4)
> >
> >
> > According to doc (mentioned in patch 0/3) the address above, 0x130, is
> > "GQSPI GPIO for Write Protect". Should we rename the define to
> R_GQSPI_GPIO?
> > (Based on doc and that the other WP is named R_GPIO).
>
> Hmmm... I auto generated these names, so somewhere internally we call
> it GQSPI_GPIO_THRESH, but apparently not in the documentation.
>
> All the other auto generated code (headers for standalone
> applications) will have a similar auto generated name, so I'm tempted
> to keep it as this.
Hi Alistair,
I see your point here and since autogenerated is less error prone and the
rest of the patch is looking good:
Reviewed-by: Francisco Iglesias <address@hidden>
(If you decide to go on and rename the define you can keep my reviewed-by
tag).
Best regards,
Francisco Iglesias
> Otherwise the register is technically just called
> GQSPI_GPIO, according to the documentation. That doesn't seem to clash
> with anything else.
>
>
I think changing it to GQSPI_GPIO makes the most sense then. That way
> it matches the documentation and is still searchably close to the auto
> generated string.
>
>
Good catch!
>
> Alistair
>
> >
> > Best regards,
> > Francisco Iglesias
> >
> >>
> >> +#define R_GQSPI_LPBK_DLY_ADJ (0x138 / 4)
> >> +#define R_GQSPI_LPBK_DLY_ADJ_RESET (0x33)
> >> #define R_GQSPI_CNFG (0x100 / 4)
> >> FIELD(GQSPI_CNFG, MODE_EN, 30, 2)
> >> FIELD(GQSPI_CNFG, GEN_FIFO_START_MODE, 29, 1)
> >> @@ -177,8 +185,16 @@
> >> FIELD(GQSPI_GF_SNAPSHOT, EXPONENT, 9, 1)
> >> FIELD(GQSPI_GF_SNAPSHOT, DATA_XFER, 8, 1)
> >> FIELD(GQSPI_GF_SNAPSHOT, IMMEDIATE_DATA, 0, 8)
> >> -#define R_GQSPI_MOD_ID (0x168 / 4)
> >> -#define R_GQSPI_MOD_ID_VALUE 0x010A0000
> >> +#define R_GQSPI_MOD_ID (0x1fc / 4)
> >> +#define R_GQSPI_MOD_ID_RESET (0x10a0000)
> >> +
> >> +#define R_QSPIDMA_DST_CTRL (0x80c / 4)
> >> +#define R_QSPIDMA_DST_CTRL_RESET (0x803ffa00)
> >> +#define R_QSPIDMA_DST_I_MASK (0x820 / 4)
> >> +#define R_QSPIDMA_DST_I_MASK_RESET (0xfe)
> >> +#define R_QSPIDMA_DST_CTRL2 (0x824 / 4)
> >> +#define R_QSPIDMA_DST_CTRL2_RESET (0x081bfff8)
> >> +
> >> /* size of TXRX FIFOs */
> >> #define RXFF_A (128)
> >> #define TXFF_A (128)
> >> @@ -351,11 +367,20 @@ static void xlnx_zynqmp_qspips_reset(DeviceState
> *d)
> >> fifo8_reset(&s->rx_fifo_g);
> >> fifo8_reset(&s->rx_fifo_g);
> >> fifo32_reset(&s->fifo_g);
> >> + s->regs[R_INTR_STATUS] = R_INTR_STATUS_RESET;
> >> + s->regs[R_GPIO] = 1;
> >> + s->regs[R_LPBK_DLY_ADJ] = R_LPBK_DLY_ADJ_RESET;
> >> + s->regs[R_GQSPI_GFIFO_THRESH] = 0x10;
> >> + s->regs[R_MOD_ID] = 0x01090101;
> >> + s->regs[R_GQSPI_IMR] = R_GQSPI_IMR_RESET;
> >> s->regs[R_GQSPI_TX_THRESH] = 1;
> >> s->regs[R_GQSPI_RX_THRESH] = 1;
> >> - s->regs[R_GQSPI_GFIFO_THRESH] = 1;
> >> - s->regs[R_GQSPI_IMR] = GQSPI_IXR_MASK;
> >> - s->regs[R_MOD_ID] = 0x01090101;
> >> + s->regs[R_GQSPI_GPIO_THRESH] = 1;
> >> + s->regs[R_GQSPI_LPBK_DLY_ADJ] = R_GQSPI_LPBK_DLY_ADJ_RESET;
> >> + s->regs[R_GQSPI_MOD_ID] = R_GQSPI_MOD_ID_RESET;
> >> + s->regs[R_QSPIDMA_DST_CTRL] = R_QSPIDMA_DST_CTRL_RESET;
> >> + s->regs[R_QSPIDMA_DST_I_MASK] = R_QSPIDMA_DST_I_MASK_RESET;
> >> + s->regs[R_QSPIDMA_DST_CTRL2] = R_QSPIDMA_DST_CTRL2_RESET;
> >> s->man_start_com_g = false;
> >> s->gqspi_irqline = 0;
> >> xlnx_zynqmp_qspips_update_ixr(s);
> >> diff --git a/include/hw/ssi/xilinx_spips.h
> b/include/hw/ssi/xilinx_spips.h
> >> index 75fc94ce5d..d398a4e81c 100644
> >> --- a/include/hw/ssi/xilinx_spips.h
> >> +++ b/include/hw/ssi/xilinx_spips.h
> >> @@ -32,7 +32,7 @@
> >> typedef struct XilinxSPIPS XilinxSPIPS;
> >>
> >> #define XLNX_SPIPS_R_MAX (0x100 / 4)
> >> -#define XLNX_ZYNQMP_SPIPS_R_MAX (0x200 / 4)
> >> +#define XLNX_ZYNQMP_SPIPS_R_MAX (0x830 / 4)
> >>
> >> /* Bite off 4k chunks at a time */
> >> #define LQSPI_CACHE_SIZE 1024
> >> --
> >> 2.14.1
> >>
> >
>
[Qemu-devel] [PATCH-2.12 v2 3/3] xilinx_spips: Use memset instead of a for loop to zero registers, Alistair Francis, 2017/12/06