On 22 February 2018 at 23:38, Alistair Francis <alistai...@gmail.com> wrote:
> On Thu, Feb 22, 2018 at 2:28 PM, Francisco Iglesias
> <frasse.igles...@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.igles...@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.fran...@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
> >
> >
>
