* [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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.