* [Qemu-devel] [PATCH 0/2] xilinx_spips: Update CS assertion when striping
@ 2018-02-22 22:28 Francisco Iglesias
2018-02-22 22:28 ` [Qemu-devel] [PATCH 1/2] xilinx_spips: Enable only two slaves when reading/writing with stripe Francisco Iglesias
2018-02-22 22:28 ` [Qemu-devel] [PATCH 2/2] xilinx_spips: Use 8 dummy cycles with the QIOR/QIOR4 commands Francisco Iglesias
0 siblings, 2 replies; 6+ messages in thread
From: Francisco Iglesias @ 2018-02-22 22:28 UTC (permalink / raw)
To: qemu-devel; +Cc: edgari, alistai, francisco.iglesias, peter.maydell
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
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 | 44 +++++++++++++++++++++++++++++++++++++++-----
1 file changed, 39 insertions(+), 5 deletions(-)
--
2.11.0
^ permalink raw reply [flat|nested] 6+ messages in thread
* [Qemu-devel] [PATCH 1/2] xilinx_spips: Enable only two slaves when reading/writing with stripe
2018-02-22 22:28 [Qemu-devel] [PATCH 0/2] xilinx_spips: Update CS assertion when striping Francisco Iglesias
@ 2018-02-22 22:28 ` Francisco Iglesias
2018-02-22 22:38 ` Alistair Francis
2018-02-22 22:28 ` [Qemu-devel] [PATCH 2/2] xilinx_spips: Use 8 dummy cycles with the QIOR/QIOR4 commands Francisco Iglesias
1 sibling, 1 reply; 6+ messages in thread
From: Francisco Iglesias @ 2018-02-22 22:28 UTC (permalink / raw)
To: qemu-devel; +Cc: edgari, alistai, francisco.iglesias, peter.maydell
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 | 42 ++++++++++++++++++++++++++++++++++++++----
1 file changed, 38 insertions(+), 4 deletions(-)
diff --git a/hw/ssi/xilinx_spips.c b/hw/ssi/xilinx_spips.c
index 8af36ca3d4..e566d179fe 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,41 @@ 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 both_buses_enabled;
+ uint8_t buses;
+ int cs = 0;
+
+ buses = ARRAY_FIELD_EX32(s->regs, GQSPI_GF_SNAPSHOT, DATA_BUS_SELECT);
+ both_buses_enabled = (buses & 0x3) == 0x3;
+
+ if (both_buses_enabled) {
+ /* Bus 0 lower cs */
+ if (field & 1) {
+ cs |= 1;
+ }
+ /* Bus 1 upper cs */
+ if (field & (1 << 1)) {
+ cs |= 1 << 3;
+ }
+ } else {
+ /* Bus 0 lower cs */
+ if (buses & 1 && field & 1) {
+ cs |= 1;
+ }
+ /* Bus 0 upper cs */
+ if (buses & 1 && field & (1 << 1)) {
+ cs |= 1 << 1;
+ }
+ /* Bus 1 lower cs */
+ if (buses & (1 << 1) && field & 1) {
+ cs |= 1 << 2;
+ }
+ /* Bus 1 upper cs */
+ if (buses & (1 << 1) && field & (1 << 1)) {
+ cs |= 1 << 3;
+ }
+ }
+ xilinx_spips_update_cs(XILINX_SPIPS(s), cs);
}
}
@@ -260,7 +294,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
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [Qemu-devel] [PATCH 2/2] xilinx_spips: Use 8 dummy cycles with the QIOR/QIOR4 commands
2018-02-22 22:28 [Qemu-devel] [PATCH 0/2] xilinx_spips: Update CS assertion when striping Francisco Iglesias
2018-02-22 22:28 ` [Qemu-devel] [PATCH 1/2] xilinx_spips: Enable only two slaves when reading/writing with stripe Francisco Iglesias
@ 2018-02-22 22:28 ` Francisco Iglesias
2018-02-22 22:36 ` Alistair Francis
1 sibling, 1 reply; 6+ messages in thread
From: Francisco Iglesias @ 2018-02-22 22:28 UTC (permalink / raw)
To: qemu-devel; +Cc: edgari, alistai, francisco.iglesias, peter.maydell
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>
---
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 e566d179fe..eb7fc0ea71 100644
--- a/hw/ssi/xilinx_spips.c
+++ b/hw/ssi/xilinx_spips.c
@@ -578,7 +578,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
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH 2/2] xilinx_spips: Use 8 dummy cycles with the QIOR/QIOR4 commands
2018-02-22 22:28 ` [Qemu-devel] [PATCH 2/2] xilinx_spips: Use 8 dummy cycles with the QIOR/QIOR4 commands Francisco Iglesias
@ 2018-02-22 22:36 ` Alistair Francis
0 siblings, 0 replies; 6+ messages in thread
From: Alistair Francis @ 2018-02-22 22:36 UTC (permalink / raw)
To: Francisco Iglesias
Cc: qemu-devel@nongnu.org Developers, Peter Maydell, Edgar Iglesias,
Alistair Francis, francisco.iglesias
On Thu, Feb 22, 2018 at 2:28 PM, Francisco Iglesias
<frasse.iglesias@gmail.com> wrote:
> 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 on Xilinx's ZynqMP
Tested-by: Alistair Francis <alistair.francis@xilinx.com>
Reviewed-by: Alistair Francis <alistair.francis@xilinx.com>
Alistair
> ---
> 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 e566d179fe..eb7fc0ea71 100644
> --- a/hw/ssi/xilinx_spips.c
> +++ b/hw/ssi/xilinx_spips.c
> @@ -578,7 +578,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
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH 1/2] xilinx_spips: Enable only two slaves when reading/writing with stripe
2018-02-22 22:28 ` [Qemu-devel] [PATCH 1/2] xilinx_spips: Enable only two slaves when reading/writing with stripe Francisco Iglesias
@ 2018-02-22 22:38 ` Alistair Francis
2018-02-23 6:24 ` francisco iglesias
0 siblings, 1 reply; 6+ messages in thread
From: Alistair Francis @ 2018-02-22 22:38 UTC (permalink / raw)
To: Francisco Iglesias
Cc: qemu-devel@nongnu.org Developers, Peter Maydell, Edgar Iglesias,
Alistair Francis, francisco.iglesias
On Thu, Feb 22, 2018 at 2:28 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>
> ---
> hw/ssi/xilinx_spips.c | 42 ++++++++++++++++++++++++++++++++++++++----
> 1 file changed, 38 insertions(+), 4 deletions(-)
>
> diff --git a/hw/ssi/xilinx_spips.c b/hw/ssi/xilinx_spips.c
> index 8af36ca3d4..e566d179fe 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,41 @@ 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 both_buses_enabled;
> + uint8_t buses;
> + int cs = 0;
> +
> + buses = ARRAY_FIELD_EX32(s->regs, GQSPI_GF_SNAPSHOT, DATA_BUS_SELECT);
> + both_buses_enabled = (buses & 0x3) == 0x3;
> +
> + if (both_buses_enabled) {
> + /* Bus 0 lower cs */
> + if (field & 1) {
> + cs |= 1;
> + }
> + /* Bus 1 upper cs */
> + if (field & (1 << 1)) {
> + cs |= 1 << 3;
> + }
> + } else {
> + /* Bus 0 lower cs */
> + if (buses & 1 && field & 1) {
> + cs |= 1;
> + }
> + /* Bus 0 upper cs */
> + if (buses & 1 && field & (1 << 1)) {
> + cs |= 1 << 1;
> + }
> + /* Bus 1 lower cs */
> + if (buses & (1 << 1) && field & 1) {
> + cs |= 1 << 2;
> + }
> + /* Bus 1 upper cs */
> + if (buses & (1 << 1) && field & (1 << 1)) {
> + cs |= 1 << 3;
> + }
It might make more sense to have the buses & 1 in it's own if
statement and have nested if statements here. Just to be easier to
follow.
Tested-by: Alistair Francis <alistair.francis@xilinx.com>
Alistair
> + }
> + xilinx_spips_update_cs(XILINX_SPIPS(s), cs);
> }
> }
>
> @@ -260,7 +294,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
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH 1/2] xilinx_spips: Enable only two slaves when reading/writing with stripe
2018-02-22 22:38 ` Alistair Francis
@ 2018-02-23 6:24 ` francisco iglesias
0 siblings, 0 replies; 6+ messages in thread
From: francisco iglesias @ 2018-02-23 6:24 UTC (permalink / raw)
To: Alistair Francis
Cc: qemu-devel@nongnu.org Developers, Peter Maydell, Edgar Iglesias,
Alistair Francis, Francisco Iglesias
On 22 February 2018 at 23:38, Alistair Francis <alistair23@gmail.com> wrote:
> On Thu, Feb 22, 2018 at 2:28 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>
> > ---
> > hw/ssi/xilinx_spips.c | 42 ++++++++++++++++++++++++++++++++++++++----
> > 1 file changed, 38 insertions(+), 4 deletions(-)
> >
> > diff --git a/hw/ssi/xilinx_spips.c b/hw/ssi/xilinx_spips.c
> > index 8af36ca3d4..e566d179fe 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,41 @@ 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 both_buses_enabled;
> > + uint8_t buses;
> > + int cs = 0;
> > +
> > + buses = ARRAY_FIELD_EX32(s->regs, GQSPI_GF_SNAPSHOT,
> DATA_BUS_SELECT);
> > + both_buses_enabled = (buses & 0x3) == 0x3;
> > +
> > + if (both_buses_enabled) {
> > + /* Bus 0 lower cs */
> > + if (field & 1) {
> > + cs |= 1;
> > + }
> > + /* Bus 1 upper cs */
> > + if (field & (1 << 1)) {
> > + cs |= 1 << 3;
> > + }
> > + } else {
> > + /* Bus 0 lower cs */
> > + if (buses & 1 && field & 1) {
> > + cs |= 1;
> > + }
> > + /* Bus 0 upper cs */
> > + if (buses & 1 && field & (1 << 1)) {
> > + cs |= 1 << 1;
> > + }
> > + /* Bus 1 lower cs */
> > + if (buses & (1 << 1) && field & 1) {
> > + cs |= 1 << 2;
> > + }
> > + /* Bus 1 upper cs */
> > + if (buses & (1 << 1) && field & (1 << 1)) {
> > + cs |= 1 << 3;
> > + }
>
> It might make more sense to have the buses & 1 in it's own if
> statement and have nested if statements here. Just to be easier to
> follow.
>
> Tested-by: Alistair Francis <alistair.francis@xilinx.com>
>
> Alistair
>
> Hi Alistair,
Thank you very much for reviewing and testing! Good point above, I'll do a
new version of the patch doing this.
Best regards,
Francisco Iglesias
> > + }
> > + xilinx_spips_update_cs(XILINX_SPIPS(s), cs);
> > }
> > }
> >
> > @@ -260,7 +294,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
> >
> >
>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2018-02-23 6:30 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-02-22 22:28 [Qemu-devel] [PATCH 0/2] xilinx_spips: Update CS assertion when striping Francisco Iglesias
2018-02-22 22:28 ` [Qemu-devel] [PATCH 1/2] xilinx_spips: Enable only two slaves when reading/writing with stripe Francisco Iglesias
2018-02-22 22:38 ` Alistair Francis
2018-02-23 6:24 ` francisco iglesias
2018-02-22 22:28 ` [Qemu-devel] [PATCH 2/2] xilinx_spips: Use 8 dummy cycles with the QIOR/QIOR4 commands Francisco Iglesias
2018-02-22 22:36 ` 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.