* [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
* [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
* 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
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).