* [Qemu-devel] [PATCH-2.12 v2 0/3] Update the reset values of the Xilinx ZynqMP QSPI
@ 2017-12-06 22:22 Alistair Francis
2017-12-06 22:22 ` [Qemu-devel] [PATCH-2.12 v2 1/3] xilinx_spips: Update the QSPI Mod ID reset value Alistair Francis
` (2 more replies)
0 siblings, 3 replies; 8+ messages in thread
From: Alistair Francis @ 2017-12-06 22:22 UTC (permalink / raw)
To: qemu-devel, edgar.iglesias, edgar.iglesias
Cc: alistair.francis, alistair23, frasse.iglesias, frederic.konrad
Update the reset values of the Xilinx ZynqMP QSPI device to match the
resister spec here:
https://www.xilinx.com/html_docs/registers/ug1087/ug1087-zynq-ultrascale-registers.html
V2:
- Don't double set registers
Based-on: 20171126231634.9531-14-frasse.iglesias@gmail.com
Alistair Francis (3):
xilinx_spips: Update the QSPI Mod ID reset value
xilinx_spips: Set all of the reset values
xilinx_spips: Use memset instead of a for loop to zero registers
hw/ssi/xilinx_spips.c | 45 +++++++++++++++++++++++++++++++------------
include/hw/ssi/xilinx_spips.h | 2 +-
2 files changed, 34 insertions(+), 13 deletions(-)
--
2.14.1
^ permalink raw reply [flat|nested] 8+ messages in thread* [Qemu-devel] [PATCH-2.12 v2 1/3] xilinx_spips: Update the QSPI Mod ID reset value 2017-12-06 22:22 [Qemu-devel] [PATCH-2.12 v2 0/3] Update the reset values of the Xilinx ZynqMP QSPI Alistair Francis @ 2017-12-06 22:22 ` Alistair Francis 2017-12-06 22:22 ` [Qemu-devel] [PATCH-2.12 v2 2/3] xilinx_spips: Set all of the reset values Alistair Francis 2017-12-06 22:22 ` [Qemu-devel] [PATCH-2.12 v2 3/3] xilinx_spips: Use memset instead of a for loop to zero registers Alistair Francis 2 siblings, 0 replies; 8+ messages in thread From: Alistair Francis @ 2017-12-06 22:22 UTC (permalink / raw) To: qemu-devel, edgar.iglesias, edgar.iglesias Cc: alistair.francis, alistair23, frasse.iglesias, frederic.konrad Update the reset value to match the latest ZynqMP register spec. Signed-off-by: Alistair Francis <alistair.francis@xilinx.com> Reviewed-by: KONRAD Frederic <frederic.konrad@adacore.com> Reviewed-by: Francisco Iglesias <frasse.iglesias@gmail.com> --- hw/ssi/xilinx_spips.c | 1 + 1 file changed, 1 insertion(+) diff --git a/hw/ssi/xilinx_spips.c b/hw/ssi/xilinx_spips.c index ad1b2ba79f..899db814ee 100644 --- a/hw/ssi/xilinx_spips.c +++ b/hw/ssi/xilinx_spips.c @@ -355,6 +355,7 @@ static void xlnx_zynqmp_qspips_reset(DeviceState *d) 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->man_start_com_g = false; s->gqspi_irqline = 0; xlnx_zynqmp_qspips_update_ixr(s); -- 2.14.1 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* [Qemu-devel] [PATCH-2.12 v2 2/3] xilinx_spips: Set all of the reset values 2017-12-06 22:22 [Qemu-devel] [PATCH-2.12 v2 0/3] Update the reset values of the Xilinx ZynqMP QSPI Alistair Francis 2017-12-06 22:22 ` [Qemu-devel] [PATCH-2.12 v2 1/3] xilinx_spips: Update the QSPI Mod ID reset value Alistair Francis @ 2017-12-06 22:22 ` Alistair Francis 2017-12-06 23:39 ` francisco iglesias 2017-12-06 22:22 ` [Qemu-devel] [PATCH-2.12 v2 3/3] xilinx_spips: Use memset instead of a for loop to zero registers Alistair Francis 2 siblings, 1 reply; 8+ messages in thread From: Alistair Francis @ 2017-12-06 22:22 UTC (permalink / raw) To: qemu-devel, edgar.iglesias, edgar.iglesias Cc: alistair.francis, alistair23, frasse.iglesias, frederic.konrad Following the ZynqMP register spec let's ensure that all reset values are set. Signed-off-by: Alistair Francis <alistair.francis@xilinx.com> --- 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) +#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 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH-2.12 v2 2/3] xilinx_spips: Set all of the reset values 2017-12-06 22:22 ` [Qemu-devel] [PATCH-2.12 v2 2/3] xilinx_spips: Set all of the reset values Alistair Francis @ 2017-12-06 23:39 ` francisco iglesias 2017-12-11 17:27 ` Alistair Francis 0 siblings, 1 reply; 8+ messages in thread From: francisco iglesias @ 2017-12-06 23:39 UTC (permalink / raw) To: Alistair Francis Cc: qemu-devel@nongnu.org Developers, Edgar Iglesias, Edde, Alistair Francis, frederic.konrad Hi Alistair, On 6 December 2017 at 23:22, Alistair Francis <alistair.francis@xilinx.com> wrote: > Following the ZynqMP register spec let's ensure that all reset values > are set. > > Signed-off-by: Alistair Francis <alistair.francis@xilinx.com> > --- > 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). 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 > > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH-2.12 v2 2/3] xilinx_spips: Set all of the reset values 2017-12-06 23:39 ` francisco iglesias @ 2017-12-11 17:27 ` Alistair Francis 2017-12-11 21:17 ` francisco iglesias 0 siblings, 1 reply; 8+ messages in thread From: Alistair Francis @ 2017-12-11 17:27 UTC (permalink / raw) To: francisco iglesias Cc: Alistair Francis, qemu-devel@nongnu.org Developers, Edgar Iglesias, Edde, KONRAD Frederic On Wed, Dec 6, 2017 at 3:39 PM, francisco iglesias <frasse.iglesias@gmail.com> wrote: > Hi Alistair, > > On 6 December 2017 at 23:22, Alistair Francis <alistair.francis@xilinx.com> > wrote: >> >> Following the ZynqMP register spec let's ensure that all reset values >> are set. >> >> Signed-off-by: Alistair Francis <alistair.francis@xilinx.com> >> --- >> 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. 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 >> > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH-2.12 v2 2/3] xilinx_spips: Set all of the reset values 2017-12-11 17:27 ` Alistair Francis @ 2017-12-11 21:17 ` francisco iglesias 2017-12-12 18:57 ` Alistair Francis 0 siblings, 1 reply; 8+ messages in thread From: francisco iglesias @ 2017-12-11 21:17 UTC (permalink / raw) To: Alistair Francis Cc: qemu-devel@nongnu.org Developers, Edgar Iglesias, Edde, KONRAD Frederic On 11 December 2017 at 18:27, Alistair Francis <alistair.francis@xilinx.com> wrote: > On Wed, Dec 6, 2017 at 3:39 PM, francisco iglesias > <frasse.iglesias@gmail.com> wrote: > > Hi Alistair, > > > > On 6 December 2017 at 23:22, Alistair Francis < > alistair.francis@xilinx.com> > > wrote: > >> > >> Following the ZynqMP register spec let's ensure that all reset values > >> are set. > >> > >> Signed-off-by: Alistair Francis <alistair.francis@xilinx.com> > >> --- > >> 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 <frasse.iglesias@gmail.com> (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 > >> > > > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH-2.12 v2 2/3] xilinx_spips: Set all of the reset values 2017-12-11 21:17 ` francisco iglesias @ 2017-12-12 18:57 ` Alistair Francis 0 siblings, 0 replies; 8+ messages in thread From: Alistair Francis @ 2017-12-12 18:57 UTC (permalink / raw) To: francisco iglesias Cc: Alistair Francis, Edgar Iglesias, Edde, KONRAD Frederic, qemu-devel@nongnu.org Developers On Mon, Dec 11, 2017 at 1:17 PM, francisco iglesias <frasse.iglesias@gmail.com> wrote: > On 11 December 2017 at 18:27, Alistair Francis <alistair.francis@xilinx.com> > wrote: > >> On Wed, Dec 6, 2017 at 3:39 PM, francisco iglesias >> <frasse.iglesias@gmail.com> wrote: >> > Hi Alistair, >> > >> > On 6 December 2017 at 23:22, Alistair Francis < >> alistair.francis@xilinx.com> >> > wrote: >> >> >> >> Following the ZynqMP register spec let's ensure that all reset values >> >> are set. >> >> >> >> Signed-off-by: Alistair Francis <alistair.francis@xilinx.com> >> >> --- >> >> 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 <frasse.iglesias@gmail.com> > > (If you decide to go on and rename the define you can keep my reviewed-by > tag). I did end up swapping it, as it makes sense to match the documentation. Sending a V3 now. Thanks for your review. Alistair > > 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 >> >> >> > >> ^ permalink raw reply [flat|nested] 8+ messages in thread
* [Qemu-devel] [PATCH-2.12 v2 3/3] xilinx_spips: Use memset instead of a for loop to zero registers 2017-12-06 22:22 [Qemu-devel] [PATCH-2.12 v2 0/3] Update the reset values of the Xilinx ZynqMP QSPI Alistair Francis 2017-12-06 22:22 ` [Qemu-devel] [PATCH-2.12 v2 1/3] xilinx_spips: Update the QSPI Mod ID reset value Alistair Francis 2017-12-06 22:22 ` [Qemu-devel] [PATCH-2.12 v2 2/3] xilinx_spips: Set all of the reset values Alistair Francis @ 2017-12-06 22:22 ` Alistair Francis 2 siblings, 0 replies; 8+ messages in thread From: Alistair Francis @ 2017-12-06 22:22 UTC (permalink / raw) To: qemu-devel, edgar.iglesias, edgar.iglesias Cc: alistair.francis, alistair23, frasse.iglesias, frederic.konrad Use memset() instead of a for loop to zero all of the registers. Signed-off-by: Alistair Francis <alistair.francis@xilinx.com> Reviewed-by: KONRAD Frederic <frederic.konrad@adacore.com> Reviewed-by: Francisco Iglesias <frasse.iglesias@gmail.com> --- hw/ssi/xilinx_spips.c | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/hw/ssi/xilinx_spips.c b/hw/ssi/xilinx_spips.c index b8182cfd74..59d42bfce7 100644 --- a/hw/ssi/xilinx_spips.c +++ b/hw/ssi/xilinx_spips.c @@ -329,10 +329,7 @@ static void xilinx_spips_reset(DeviceState *d) { XilinxSPIPS *s = XILINX_SPIPS(d); - int i; - for (i = 0; i < XLNX_SPIPS_R_MAX; i++) { - s->regs[i] = 0; - } + memset(s->regs, 0, sizeof(s->regs)); fifo8_reset(&s->rx_fifo); fifo8_reset(&s->rx_fifo); @@ -357,13 +354,11 @@ static void xilinx_spips_reset(DeviceState *d) static void xlnx_zynqmp_qspips_reset(DeviceState *d) { XlnxZynqMPQSPIPS *s = XLNX_ZYNQMP_QSPIPS(d); - int i; xilinx_spips_reset(d); - for (i = 0; i < XLNX_ZYNQMP_SPIPS_R_MAX; i++) { - s->regs[i] = 0; - } + memset(s->regs, 0, sizeof(s->regs)); + fifo8_reset(&s->rx_fifo_g); fifo8_reset(&s->rx_fifo_g); fifo32_reset(&s->fifo_g); -- 2.14.1 ^ permalink raw reply related [flat|nested] 8+ messages in thread
end of thread, other threads:[~2017-12-12 18:58 UTC | newest] Thread overview: 8+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2017-12-06 22:22 [Qemu-devel] [PATCH-2.12 v2 0/3] Update the reset values of the Xilinx ZynqMP QSPI Alistair Francis 2017-12-06 22:22 ` [Qemu-devel] [PATCH-2.12 v2 1/3] xilinx_spips: Update the QSPI Mod ID reset value Alistair Francis 2017-12-06 22:22 ` [Qemu-devel] [PATCH-2.12 v2 2/3] xilinx_spips: Set all of the reset values Alistair Francis 2017-12-06 23:39 ` francisco iglesias 2017-12-11 17:27 ` Alistair Francis 2017-12-11 21:17 ` francisco iglesias 2017-12-12 18:57 ` Alistair Francis 2017-12-06 22:22 ` [Qemu-devel] [PATCH-2.12 v2 3/3] xilinx_spips: Use memset instead of a for loop to zero registers Alistair Francis
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).