[Qemu-devel] [PATCH v2 0/2] xilinx_spips: Update CS assertion when striping

[Qemu-devel] [PATCH v2 0/2] xilinx_spips: Update CS assertion when striping

[Francisco Iglesias]

Hi,

The first patch in this series attempts to correct the slave selection when
using the striping functionality in the QSPI. The second patch in the series
updates the QIOR/QIOR4 commands to use 8 dummy cycles in the QSPI for matching
Micron (Numonyx) flashes (the default target flash type of the QSPI).

Best regards,
Francisco Iglesias

Changelog:
v1 -> v2
  * Attempted to improve readability in the patch 'xilinx_spips: Enable only
    two slaves when reading/writing with stripe' when selecting chip selects.


Francisco Iglesias (2):
  xilinx_spips: Enable only two slaves when reading/writing with stripe
  xilinx_spips: Use 8 dummy cycles with the QIOR/QIOR4 commands

 hw/ssi/xilinx_spips.c | 43 ++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 38 insertions(+), 5 deletions(-)

-- 
2.11.0

[Qemu-devel] [PATCH v2 1/2] xilinx_spips: Enable only two slaves when reading/writing with stripe

[Francisco Iglesias]

Assert only the lower cs on bus 0 and upper cs on bus 1 when both buses and
chip selects are enabled (e.g reading/writing with stripe).

Signed-off-by: Francisco Iglesias <frasse.iglesias@gmail.com>
---
 hw/ssi/xilinx_spips.c | 41 +++++++++++++++++++++++++++++++++++++----
 1 file changed, 37 insertions(+), 4 deletions(-)

diff --git a/hw/ssi/xilinx_spips.c b/hw/ssi/xilinx_spips.c
index 8af36ca3d4..0cb484ecf4 100644
--- a/hw/ssi/xilinx_spips.c
+++ b/hw/ssi/xilinx_spips.c
@@ -223,7 +223,7 @@ static void xilinx_spips_update_cs(XilinxSPIPS *s, int field)
 {
     int i;
 
-    for (i = 0; i < s->num_cs; i++) {
+    for (i = 0; i < s->num_cs * s->num_busses; i++) {
         bool old_state = s->cs_lines_state[i];
         bool new_state = field & (1 << i);
 
@@ -234,7 +234,7 @@ static void xilinx_spips_update_cs(XilinxSPIPS *s, int field)
         }
         qemu_set_irq(s->cs_lines[i], !new_state);
     }
-    if (!(field & ((1 << s->num_cs) - 1))) {
+    if (!(field & ((1 << (s->num_cs * s->num_busses)) - 1))) {
         s->snoop_state = SNOOP_CHECKING;
         s->cmd_dummies = 0;
         s->link_state = 1;
@@ -248,7 +248,40 @@ static void xlnx_zynqmp_qspips_update_cs_lines(XlnxZynqMPQSPIPS *s)
 {
     if (s->regs[R_GQSPI_GF_SNAPSHOT]) {
         int field = ARRAY_FIELD_EX32(s->regs, GQSPI_GF_SNAPSHOT, CHIP_SELECT);
-        xilinx_spips_update_cs(XILINX_SPIPS(s), field);
+        bool upper_cs_sel = field & (1 << 1);
+        bool lower_cs_sel = field & 1;
+        bool bus0_enabled;
+        bool bus1_enabled;
+        uint8_t buses;
+        int cs = 0;
+
+        buses = ARRAY_FIELD_EX32(s->regs, GQSPI_GF_SNAPSHOT, DATA_BUS_SELECT);
+        bus0_enabled = buses & 1;
+        bus1_enabled = buses & (1 << 1);
+
+        if (bus0_enabled && bus1_enabled) {
+            if (lower_cs_sel) {
+                cs |= 1;
+            }
+            if (upper_cs_sel) {
+                cs |= 1 << 3;
+            }
+        } else if (bus0_enabled) {
+            if (lower_cs_sel) {
+                cs |= 1;
+            }
+            if (upper_cs_sel) {
+                cs |= 1 << 1;
+            }
+        } else if (bus1_enabled) {
+            if (lower_cs_sel) {
+                cs |= 1 << 2;
+            }
+            if (upper_cs_sel) {
+                cs |= 1 << 3;
+            }
+        }
+        xilinx_spips_update_cs(XILINX_SPIPS(s), cs);
     }
 }
 
@@ -260,7 +293,7 @@ static void xilinx_spips_update_cs_lines(XilinxSPIPS *s)
     if (num_effective_busses(s) == 2) {
         /* Single bit chip-select for qspi */
         field &= 0x1;
-        field |= field << 1;
+        field |= field << 3;
     /* Dual stack U-Page */
     } else if (s->regs[R_LQSPI_CFG] & LQSPI_CFG_TWO_MEM &&
                s->regs[R_LQSPI_STS] & LQSPI_CFG_U_PAGE) {
-- 
2.11.0

[Qemu-devel] [PATCH v2 2/2] xilinx_spips: Use 8 dummy cycles with the QIOR/QIOR4 commands

[Francisco Iglesias]

Use 8 dummy cycles (4 dummy bytes) with the QIOR/QIOR4 commands in legacy mode
for matching what is expected by Micron (Numonyx) flashes (the default target
flash type of the QSPI).

Signed-off-by: Francisco Iglesias <frasse.iglesias@gmail.com>
Tested-by: Alistair Francis <alistair.francis@xilinx.com>
Reviewed-by: Alistair Francis <alistair.francis@xilinx.com>
---
 hw/ssi/xilinx_spips.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/ssi/xilinx_spips.c b/hw/ssi/xilinx_spips.c
index 0cb484ecf4..426f971311 100644
--- a/hw/ssi/xilinx_spips.c
+++ b/hw/ssi/xilinx_spips.c
@@ -577,7 +577,7 @@ static int xilinx_spips_num_dummies(XilinxQSPIPS *qs, uint8_t command)
         return 2;
     case QIOR:
     case QIOR_4:
-        return 5;
+        return 4;
     default:
         return -1;
     }
-- 
2.11.0

Re: [Qemu-devel] [PATCH v2 1/2] xilinx_spips: Enable only two slaves when reading/writing with stripe

[Alistair Francis]

On Fri, Feb 23, 2018 at 3:22 PM, Francisco Iglesias
<frasse.iglesias@gmail.com> wrote:
> Assert only the lower cs on bus 0 and upper cs on bus 1 when both buses and
> chip selects are enabled (e.g reading/writing with stripe).
>
> Signed-off-by: Francisco Iglesias <frasse.iglesias@gmail.com>

Reviewed-by: Alistair Francis <alistair.francis@xilinx.com>
Tested-by: Alistair Francis <alistair.francis@xilinx.com>

Alistair

> ---
>  hw/ssi/xilinx_spips.c | 41 +++++++++++++++++++++++++++++++++++++----
>  1 file changed, 37 insertions(+), 4 deletions(-)
>
> diff --git a/hw/ssi/xilinx_spips.c b/hw/ssi/xilinx_spips.c
> index 8af36ca3d4..0cb484ecf4 100644
> --- a/hw/ssi/xilinx_spips.c
> +++ b/hw/ssi/xilinx_spips.c
> @@ -223,7 +223,7 @@ static void xilinx_spips_update_cs(XilinxSPIPS *s, int field)
>  {
>      int i;
>
> -    for (i = 0; i < s->num_cs; i++) {
> +    for (i = 0; i < s->num_cs * s->num_busses; i++) {
>          bool old_state = s->cs_lines_state[i];
>          bool new_state = field & (1 << i);
>
> @@ -234,7 +234,7 @@ static void xilinx_spips_update_cs(XilinxSPIPS *s, int field)
>          }
>          qemu_set_irq(s->cs_lines[i], !new_state);
>      }
> -    if (!(field & ((1 << s->num_cs) - 1))) {
> +    if (!(field & ((1 << (s->num_cs * s->num_busses)) - 1))) {
>          s->snoop_state = SNOOP_CHECKING;
>          s->cmd_dummies = 0;
>          s->link_state = 1;
> @@ -248,7 +248,40 @@ static void xlnx_zynqmp_qspips_update_cs_lines(XlnxZynqMPQSPIPS *s)
>  {
>      if (s->regs[R_GQSPI_GF_SNAPSHOT]) {
>          int field = ARRAY_FIELD_EX32(s->regs, GQSPI_GF_SNAPSHOT, CHIP_SELECT);
> -        xilinx_spips_update_cs(XILINX_SPIPS(s), field);
> +        bool upper_cs_sel = field & (1 << 1);
> +        bool lower_cs_sel = field & 1;
> +        bool bus0_enabled;
> +        bool bus1_enabled;
> +        uint8_t buses;
> +        int cs = 0;
> +
> +        buses = ARRAY_FIELD_EX32(s->regs, GQSPI_GF_SNAPSHOT, DATA_BUS_SELECT);
> +        bus0_enabled = buses & 1;
> +        bus1_enabled = buses & (1 << 1);
> +
> +        if (bus0_enabled && bus1_enabled) {
> +            if (lower_cs_sel) {
> +                cs |= 1;
> +            }
> +            if (upper_cs_sel) {
> +                cs |= 1 << 3;
> +            }
> +        } else if (bus0_enabled) {
> +            if (lower_cs_sel) {
> +                cs |= 1;
> +            }
> +            if (upper_cs_sel) {
> +                cs |= 1 << 1;
> +            }
> +        } else if (bus1_enabled) {
> +            if (lower_cs_sel) {
> +                cs |= 1 << 2;
> +            }
> +            if (upper_cs_sel) {
> +                cs |= 1 << 3;
> +            }
> +        }
> +        xilinx_spips_update_cs(XILINX_SPIPS(s), cs);
>      }
>  }
>
> @@ -260,7 +293,7 @@ static void xilinx_spips_update_cs_lines(XilinxSPIPS *s)
>      if (num_effective_busses(s) == 2) {
>          /* Single bit chip-select for qspi */
>          field &= 0x1;
> -        field |= field << 1;
> +        field |= field << 3;
>      /* Dual stack U-Page */
>      } else if (s->regs[R_LQSPI_CFG] & LQSPI_CFG_TWO_MEM &&
>                 s->regs[R_LQSPI_STS] & LQSPI_CFG_U_PAGE) {
> --
> 2.11.0
>
>

Re: [Qemu-devel] [PATCH v2 1/2] xilinx_spips: Enable only two slaves when reading/writing with stripe

[francisco iglesias]

On Tuesday, 27 February 2018, Alistair Francis <alistair23@gmail.com> wrote:

> On Fri, Feb 23, 2018 at 3:22 PM, Francisco Iglesias
> <frasse.iglesias@gmail.com> wrote:
> > Assert only the lower cs on bus 0 and upper cs on bus 1 when both buses
> and
> > chip selects are enabled (e.g reading/writing with stripe).
> >
> > Signed-off-by: Francisco Iglesias <frasse.iglesias@gmail.com>
>
> Reviewed-by: Alistair Francis <alistair.francis@xilinx.com>
> Tested-by: Alistair Francis <alistair.francis@xilinx.com>
>
> Alistair
>
> > ---
> >  hw/ssi/xilinx_spips.c | 41 +++++++++++++++++++++++++++++++++++++----
> >  1 file changed, 37 insertions(+), 4 deletions(-)
> >
> > diff --git a/hw/ssi/xilinx_spips.c b/hw/ssi/xilinx_spips.c
> > index 8af36ca3d4..0cb484ecf4 100644
> > --- a/hw/ssi/xilinx_spips.c
> > +++ b/hw/ssi/xilinx_spips.c
> > @@ -223,7 +223,7 @@ static void xilinx_spips_update_cs(XilinxSPIPS *s,
> int field)
> >  {
> >      int i;
> >
> > -    for (i = 0; i < s->num_cs; i++) {
> > +    for (i = 0; i < s->num_cs * s->num_busses; i++) {
> >          bool old_state = s->cs_lines_state[i];
> >          bool new_state = field & (1 << i);
> >
> > @@ -234,7 +234,7 @@ static void xilinx_spips_update_cs(XilinxSPIPS *s,
> int field)
> >          }
> >          qemu_set_irq(s->cs_lines[i], !new_state);
> >      }
> > -    if (!(field & ((1 << s->num_cs) - 1))) {
> > +    if (!(field & ((1 << (s->num_cs * s->num_busses)) - 1))) {
> >          s->snoop_state = SNOOP_CHECKING;
> >          s->cmd_dummies = 0;
> >          s->link_state = 1;
> > @@ -248,7 +248,40 @@ static void xlnx_zynqmp_qspips_update_cs_lines(XlnxZynqMPQSPIPS
> *s)
> >  {
> >      if (s->regs[R_GQSPI_GF_SNAPSHOT]) {
> >          int field = ARRAY_FIELD_EX32(s->regs, GQSPI_GF_SNAPSHOT,
> CHIP_SELECT);
> > -        xilinx_spips_update_cs(XILINX_SPIPS(s), field);
> > +        bool upper_cs_sel = field & (1 << 1);
> > +        bool lower_cs_sel = field & 1;
> > +        bool bus0_enabled;
> > +        bool bus1_enabled;
> > +        uint8_t buses;
> > +        int cs = 0;
> > +
> > +        buses = ARRAY_FIELD_EX32(s->regs, GQSPI_GF_SNAPSHOT,
> DATA_BUS_SELECT);
> > +        bus0_enabled = buses & 1;
> > +        bus1_enabled = buses & (1 << 1);
> > +
> > +        if (bus0_enabled && bus1_enabled) {
> > +            if (lower_cs_sel) {
> > +                cs |= 1;
> > +            }
> > +            if (upper_cs_sel) {
> > +                cs |= 1 << 3;
> > +            }
> > +        } else if (bus0_enabled) {
> > +            if (lower_cs_sel) {
> > +                cs |= 1;
> > +            }
> > +            if (upper_cs_sel) {
> > +                cs |= 1 << 1;
> > +            }
> > +        } else if (bus1_enabled) {
> > +            if (lower_cs_sel) {
> > +                cs |= 1 << 2;
> > +            }
> > +            if (upper_cs_sel) {
> > +                cs |= 1 << 3;
> > +            }
> > +        }
> > +        xilinx_spips_update_cs(XILINX_SPIPS(s), cs);
> >      }
> >  }
> >
> > @@ -260,7 +293,7 @@ static void xilinx_spips_update_cs_lines(XilinxSPIPS
> *s)
> >      if (num_effective_busses(s) == 2) {
> >          /* Single bit chip-select for qspi */
> >          field &= 0x1;
> > -        field |= field << 1;
> > +        field |= field << 3;
> >      /* Dual stack U-Page */
> >      } else if (s->regs[R_LQSPI_CFG] & LQSPI_CFG_TWO_MEM &&
> >                 s->regs[R_LQSPI_STS] & LQSPI_CFG_U_PAGE) {
> > --
> > 2.11.0
> >
> >
>

Hi Alistair,

Thank you very much once again for reviewing and testing!

Best regards,
Francisco Iglesias

Re: [Qemu-devel] [PATCH v2 0/2] xilinx_spips: Update CS assertion when striping

[Peter Maydell]

On 23 February 2018 at 23:22, Francisco Iglesias
<frasse.iglesias@gmail.com> wrote:
> Hi,
>
> The first patch in this series attempts to correct the slave selection when
> using the striping functionality in the QSPI. The second patch in the series
> updates the QIOR/QIOR4 commands to use 8 dummy cycles in the QSPI for matching
> Micron (Numonyx) flashes (the default target flash type of the QSPI).
>
> Best regards,
> Francisco Iglesias
>
> Changelog:
> v1 -> v2
>   * Attempted to improve readability in the patch 'xilinx_spips: Enable only
>     two slaves when reading/writing with stripe' when selecting chip selects.
>
>
> Francisco Iglesias (2):
>   xilinx_spips: Enable only two slaves when reading/writing with stripe
>   xilinx_spips: Use 8 dummy cycles with the QIOR/QIOR4 commands
>
>  hw/ssi/xilinx_spips.c | 43 ++++++++++++++++++++++++++++++++++++++-----
>  1 file changed, 38 insertions(+), 5 deletions(-)



Applied to target-arm.next, thanks.

-- PMM

Re: [Qemu-devel] [PATCH v2 0/2] xilinx_spips: Update CS assertion when striping

[francisco iglesias]

On Tuesday, 27 February 2018, Peter Maydell <peter.maydell@linaro.org>
wrote:

> On 23 February 2018 at 23:22, Francisco Iglesias
> <frasse.iglesias@gmail.com> wrote:
> > Hi,
> >
> > The first patch in this series attempts to correct the slave selection
> when
> > using the striping functionality in the QSPI. The second patch in the
> series
> > updates the QIOR/QIOR4 commands to use 8 dummy cycles in the QSPI for
> matching
> > Micron (Numonyx) flashes (the default target flash type of the QSPI).
> >
> > Best regards,
> > Francisco Iglesias
> >
> > Changelog:
> > v1 -> v2
> >   * Attempted to improve readability in the patch 'xilinx_spips: Enable
> only
> >     two slaves when reading/writing with stripe' when selecting chip
> selects.
> >
> >
> > Francisco Iglesias (2):
> >   xilinx_spips: Enable only two slaves when reading/writing with stripe
> >   xilinx_spips: Use 8 dummy cycles with the QIOR/QIOR4 commands
> >
> >  hw/ssi/xilinx_spips.c | 43 ++++++++++++++++++++++++++++++++++++++-----
> >  1 file changed, 38 insertions(+), 5 deletions(-)
>
>
>
> Applied to target-arm.next, thanks.
>
> -- PMM
>

Hi Peter,

Thank you very much!

Best regards,
Francisco Iglesias