Skip to contentSkip to conversation

Update the reset values of the Xilinx ZynqMP QSPI

PATCH Superseded Alistair Francis 2 participants 8 messages Dec 6, 2017 – Dec 12, 2017 Arm/Zynq architecture

v2 of 3 Superseded Latest is v3 →

lore
Share & tools
lore

Alistair Francis

author
#0 lore raw

v2 cover Update the reset values of the Xilinx ZynqMP QSPI

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: [email protected]

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(-)
Signature · 1 line
2.14.1
author
#1 lore raw

PATCH v2 1/3 xilinx_spips: Update the QSPI Mod ID reset value

Update the reset value to match the latest ZynqMP register spec.

Signed-off-by Alistair Francis <[email protected]>
Reviewed-by KONRAD Frederic <[email protected]>
Reviewed-by Francisco Iglesias <[email protected]>

---

 hw/ssi/xilinx_spips.c | 1 +
 1 file changed, 1 insertion(+)
hw/ssi/xilinx_spips.c +1 -0
index ad1b2ba79f..899db814ee 100644
@@ -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);
Signature · 1 line
2.14.1
author
#2 lore raw

PATCH v2 2/3 xilinx_spips: Set all of the reset values

In reply to Alistair Francis

Following the ZynqMP register spec let's ensure that all reset values are set.

Signed-off-by Alistair Francis <[email protected]>

---
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(-)
hw/ssi/xilinx_spips.c +30 -5
index 899db814ee..b8182cfd74 100644
@@ -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);
include/hw/ssi/xilinx_spips.h +1 -1
index 75fc94ce5d..d398a4e81c 100644
@@ -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
Signature · 1 line
2.14.1
#3 lore raw

Re: [Qemu-devel] [PATCH-2.12 v2 2/3] xilinx_spips: Set all of the reset values

Hi Alistair,

Quoted 35 lines · Alistair Francis

Following the ZynqMP register spec let's ensure that all reset values are set.

Signed-off-by Alistair Francis <[email protected]>

---
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(-)
hw/ssi/xilinx_spips.c +4 -0
index 899db814ee..b8182cfd74 100644
@@ -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)

hw/ssi/xilinx_spips.c line 151

Quoted 8 lines
hw/ssi/xilinx_spips.c +2 -0
@@ -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

Quoted 65 lines
hw/ssi/xilinx_spips.c +2 -0
+#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)
hw/ssi/xilinx_spips.c +22 -5
@@ -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);
include/hw/ssi/xilinx_spips.h +1 -2
index 75fc94ce5d..d398a4e81c 100644
@@ -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

author
#4 lore raw

Re: [Qemu-devel] [PATCH-2.12 v2 2/3] xilinx_spips: Set all of the reset values

Quoted 44 lines · francisco iglesias

Hi Alistair,

Quoted 40 lines · Alistair Francis

Following the ZynqMP register spec let's ensure that all reset values are set.

Signed-off-by Alistair Francis <[email protected]>

---
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(-)
hw/ssi/xilinx_spips.c +5 -0
index 899db814ee..b8182cfd74 100644
@@ -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)
Quoted 8 lines

hw/ssi/xilinx_spips.c line 150

Quoted 3 lines
hw/ssi/xilinx_spips.c +1 -0
 #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

Quoted 70 lines

Best regards,
Francisco Iglesias

Quoted 65 lines
hw/ssi/xilinx_spips.c +24 -5
+#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);
include/hw/ssi/xilinx_spips.h +1 -2
index 75fc94ce5d..d398a4e81c 100644
@@ -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

Reviewed-by
#5 lore raw

Re: [Qemu-devel] [PATCH-2.12 v2 2/3] xilinx_spips: Set all of the reset values

Quoted 55 lines · Alistair Francis
Quoted 3 lines · francisco iglesias

Hi Alistair,

On 6 December 2017 at 23:22, Alistair Francis <

[email protected]>

Quoted 40 lines
Quoted 39 lines · wrote:

Following the ZynqMP register spec let's ensure that all reset values are set.

Signed-off-by Alistair Francis <[email protected]>

---
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(-)
hw/ssi/xilinx_spips.c +4 -0
index 899db814ee..b8182cfd74 100644
@@ -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)
Quoted 8 lines

hw/ssi/xilinx_spips.c line 150

Quoted 4 lines
hw/ssi/xilinx_spips.c +2 -0
+#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?

Quoted 8 lines
Quoted 1 line

(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 <[email protected]>

(If you decide to go on and rename the define you can keep my reviewed-by tag).

Best regards,
Francisco Iglesias

Quoted 5 lines

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

Quoted 4 lines

it matches the documentation and is still searchably close to the auto generated string.

Good catch!

Quoted 76 lines

Alistair

Quoted 22 lines

Best regards,
Francisco Iglesias

Quoted 18 lines
hw/ssi/xilinx_spips.c +8 -2
+#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)
Quoted 8 lines

hw/ssi/xilinx_spips.c static void xlnx_zynqmp_qspips_reset(DeviceState

Quoted 8 lines
hw/ssi/xilinx_spips.c +4 -0
+#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)

Quoted 16 lines
Quoted 16 lines
hw/ssi/xilinx_spips.c +8 -3
     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;
Quoted 8 lines

Review

Quoted 8 lines
hw/ssi/xilinx_spips.c +4 -0
+    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);
(quoted diff) +0 -0

b/include/hw/ssi/xilinx_spips.h

Quoted 16 lines
Quoted 15 lines

index 75fc94ce5d..d398a4e81c 100644

include/hw/ssi/xilinx_spips.h +1 -2
@@ -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

author
#6 lore raw

Re: [Qemu-devel] [PATCH-2.12 v2 2/3] xilinx_spips: Set all of the reset values

Quoted 70 lines · francisco iglesias
Quoted 55 lines · Alistair Francis
Quoted 3 lines · francisco iglesias

Hi Alistair,

On 6 December 2017 at 23:22, Alistair Francis <

[email protected]>

Quoted 40 lines
Quoted 39 lines · wrote:

Following the ZynqMP register spec let's ensure that all reset values are set.

Signed-off-by Alistair Francis <[email protected]>

---
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(-)
hw/ssi/xilinx_spips.c +4 -0
index 899db814ee..b8182cfd74 100644
@@ -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)
Quoted 8 lines

hw/ssi/xilinx_spips.c line 150

Quoted 4 lines
hw/ssi/xilinx_spips.c +2 -0
+#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?

Quoted 8 lines
Quoted 1 line

(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,

Quoted 7 lines

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 <[email protected]>

(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

Quoted 93 lines

Best regards,
Francisco Iglesias

Quoted 5 lines

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

Quoted 4 lines

it matches the documentation and is still searchably close to the auto generated string.

Good catch!

Quoted 76 lines

Alistair

Quoted 22 lines

Best regards,
Francisco Iglesias

Quoted 18 lines
hw/ssi/xilinx_spips.c +8 -2
+#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)
Quoted 8 lines

hw/ssi/xilinx_spips.c static void xlnx_zynqmp_qspips_reset(DeviceState

Quoted 8 lines
hw/ssi/xilinx_spips.c +4 -0
+#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)

Quoted 16 lines
Quoted 16 lines
hw/ssi/xilinx_spips.c +8 -3
     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;
Quoted 8 lines

Review

Quoted 8 lines
hw/ssi/xilinx_spips.c +4 -0
+    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);
(quoted diff) +0 -0

b/include/hw/ssi/xilinx_spips.h

Quoted 16 lines
Quoted 15 lines

index 75fc94ce5d..d398a4e81c 100644

include/hw/ssi/xilinx_spips.h +1 -2
@@ -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

author
#7 lore raw

PATCH v2 3/3 xilinx_spips: Use memset instead of a for loop to zero registers

In reply to Alistair Francis

Use memset() instead of a for loop to zero all of the registers.

Signed-off-by Alistair Francis <[email protected]>
Reviewed-by KONRAD Frederic <[email protected]>
Reviewed-by Francisco Iglesias <[email protected]>

---

 hw/ssi/xilinx_spips.c | 11 +++--------
 1 file changed, 3 insertions(+), 8 deletions(-)
hw/ssi/xilinx_spips.c +3 -8
index b8182cfd74..59d42bfce7 100644
@@ -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);
Signature · 1 line
2.14.1