[QEMU][PATCH v2] ssi: xilinx_spips: Skip update of cs and fifo releated to spips in gqspi

[QEMU][PATCH v2] ssi: xilinx_spips: Skip update of cs and fifo releated to spips in gqspi

[Sai Pavan Boddu]

GQSPI handles chip selects and fifos in a different way compared to
spips. So skip update of cs and fifos related to spips in gqspi mode.

Signed-off-by: Sai Pavan Boddu <sai.pavan.boddu@xilinx.com>
---
Changes for V2:
    Just skip update of spips cs and fifos
    Update commit message accordingly

 hw/ssi/xilinx_spips.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/hw/ssi/xilinx_spips.c b/hw/ssi/xilinx_spips.c
index a309c71..27154b0 100644
--- a/hw/ssi/xilinx_spips.c
+++ b/hw/ssi/xilinx_spips.c
@@ -1022,6 +1022,13 @@ static void xilinx_spips_write(void *opaque, hwaddr addr,
     }
     s->regs[addr] = (s->regs[addr] & ~mask) | (value & mask);
 no_reg_update:
+    /* In GQSPI mode skip update of CS and fifo's related to spips */
+    if (object_dynamic_cast(OBJECT(s), TYPE_XLNX_ZYNQMP_QSPIPS)) {
+        XlnxZynqMPQSPIPS *ss = XLNX_ZYNQMP_QSPIPS(s);
+        if (ARRAY_FIELD_EX32(ss->regs, GQSPI_SELECT, GENERIC_QSPI_EN)) {
+            return;
+        }
+    }
     xilinx_spips_update_cs_lines(s);
     xilinx_spips_check_flush(s);
     xilinx_spips_update_cs_lines(s);
-- 
2.7.4

Re: [QEMU][PATCH v2] ssi: xilinx_spips: Skip update of cs and fifo releated to spips in gqspi

[Philippe Mathieu-Daudé]

Hi,

On 10/17/19 12:17 PM, Sai Pavan Boddu wrote:
> GQSPI handles chip selects and fifos in a different way compared to
> spips. So skip update of cs and fifos related to spips in gqspi mode.
> 
> Signed-off-by: Sai Pavan Boddu <sai.pavan.boddu@xilinx.com>
> ---
> Changes for V2:
>      Just skip update of spips cs and fifos
>      Update commit message accordingly
> 
>   hw/ssi/xilinx_spips.c | 7 +++++++
>   1 file changed, 7 insertions(+)
> 
> diff --git a/hw/ssi/xilinx_spips.c b/hw/ssi/xilinx_spips.c
> index a309c71..27154b0 100644
> --- a/hw/ssi/xilinx_spips.c
> +++ b/hw/ssi/xilinx_spips.c
> @@ -1022,6 +1022,13 @@ static void xilinx_spips_write(void *opaque, hwaddr addr,
>       }
>       s->regs[addr] = (s->regs[addr] & ~mask) | (value & mask);
>   no_reg_update:
> +    /* In GQSPI mode skip update of CS and fifo's related to spips */
> +    if (object_dynamic_cast(OBJECT(s), TYPE_XLNX_ZYNQMP_QSPIPS)) {

object_dynamic_cast() is expensive, please add a 'bool is_qpspi' in 
XilinxQSPIPS and set it in xlnx_zynqmp_qspips_init().

> +        XlnxZynqMPQSPIPS *ss = XLNX_ZYNQMP_QSPIPS(s);
> +        if (ARRAY_FIELD_EX32(ss->regs, GQSPI_SELECT, GENERIC_QSPI_EN)) {
> +            return;
> +        }
> +    }
>       xilinx_spips_update_cs_lines(s);
>       xilinx_spips_check_flush(s);
>       xilinx_spips_update_cs_lines(s);
> 

Re: [QEMU][PATCH v2] ssi: xilinx_spips: Skip update of cs and fifo releated to spips in gqspi

[Francisco Iglesias]

Hi Sai,

On [2019 Oct 17] Thu 15:47:54, Sai Pavan Boddu wrote:
> GQSPI handles chip selects and fifos in a different way compared to
> spips. So skip update of cs and fifos related to spips in gqspi mode.
> 
> Signed-off-by: Sai Pavan Boddu <sai.pavan.boddu@xilinx.com>
> ---
> Changes for V2:
>     Just skip update of spips cs and fifos
>     Update commit message accordingly
> 
>  hw/ssi/xilinx_spips.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/hw/ssi/xilinx_spips.c b/hw/ssi/xilinx_spips.c
> index a309c71..27154b0 100644
> --- a/hw/ssi/xilinx_spips.c
> +++ b/hw/ssi/xilinx_spips.c
> @@ -1022,6 +1022,13 @@ static void xilinx_spips_write(void *opaque, hwaddr addr,
>      }
>      s->regs[addr] = (s->regs[addr] & ~mask) | (value & mask);
>  no_reg_update:
> +    /* In GQSPI mode skip update of CS and fifo's related to spips */
> +    if (object_dynamic_cast(OBJECT(s), TYPE_XLNX_ZYNQMP_QSPIPS)) {
> +        XlnxZynqMPQSPIPS *ss = XLNX_ZYNQMP_QSPIPS(s);
> +        if (ARRAY_FIELD_EX32(ss->regs, GQSPI_SELECT, GENERIC_QSPI_EN)) {
> +            return;
> +        }
> +    }

Above corrects the issue for the zynqmp but not for the other two models
(below functions shouldn't be called when writing the mentioned config
regs for them either), would it be ok for you to expand to the switch
cases you had in v1 (into the switch in this function and return after
updating the reg values)? (the correction will then spawn all three
models)

Best regards,
Francisco Iglesias

>      xilinx_spips_update_cs_lines(s);
>      xilinx_spips_check_flush(s);
>      xilinx_spips_update_cs_lines(s);
> -- 
> 2.7.4
> 
> 

RE: [QEMU][PATCH v2] ssi: xilinx_spips: Skip update of cs and fifo releated to spips in gqspi

[Sai Pavan Boddu]

Hi Francisco,

Thanks I will send a V3 following your suggestion.

Regards,
Sai Pavan

> -----Original Message-----
> From: Francisco Iglesias <frasse.iglesias@gmail.com>
> Sent: Thursday, October 17, 2019 7:05 PM
> To: Sai Pavan Boddu <saipava@xilinx.com>
> Cc: Alistair Francis <alistair@alistair23.me>; Edgar Iglesias
> <edgari@xilinx.com>; Peter Maydell <peter.maydell@linaro.org>; qemu-
> devel@nongnu.org
> Subject: Re: [QEMU][PATCH v2] ssi: xilinx_spips: Skip update of cs and fifo
> releated to spips in gqspi
> 
> Hi Sai,
> 
> On [2019 Oct 17] Thu 15:47:54, Sai Pavan Boddu wrote:
> > GQSPI handles chip selects and fifos in a different way compared to
> > spips. So skip update of cs and fifos related to spips in gqspi mode.
> >
> > Signed-off-by: Sai Pavan Boddu <sai.pavan.boddu@xilinx.com>
> > ---
> > Changes for V2:
> >     Just skip update of spips cs and fifos
> >     Update commit message accordingly
> >
> >  hw/ssi/xilinx_spips.c | 7 +++++++
> >  1 file changed, 7 insertions(+)
> >
> > diff --git a/hw/ssi/xilinx_spips.c b/hw/ssi/xilinx_spips.c index
> > a309c71..27154b0 100644
> > --- a/hw/ssi/xilinx_spips.c
> > +++ b/hw/ssi/xilinx_spips.c
> > @@ -1022,6 +1022,13 @@ static void xilinx_spips_write(void *opaque,
> hwaddr addr,
> >      }
> >      s->regs[addr] = (s->regs[addr] & ~mask) | (value & mask);
> >  no_reg_update:
> > +    /* In GQSPI mode skip update of CS and fifo's related to spips */
> > +    if (object_dynamic_cast(OBJECT(s), TYPE_XLNX_ZYNQMP_QSPIPS)) {
> > +        XlnxZynqMPQSPIPS *ss = XLNX_ZYNQMP_QSPIPS(s);
> > +        if (ARRAY_FIELD_EX32(ss->regs, GQSPI_SELECT, GENERIC_QSPI_EN))
> {
> > +            return;
> > +        }
> > +    }
> 
> Above corrects the issue for the zynqmp but not for the other two models
> (below functions shouldn't be called when writing the mentioned config regs
> for them either), would it be ok for you to expand to the switch cases you
> had in v1 (into the switch in this function and return after updating the reg
> values)? (the correction will then spawn all three
> models)
> 
> Best regards,
> Francisco Iglesias
> 
> >      xilinx_spips_update_cs_lines(s);
> >      xilinx_spips_check_flush(s);
> >      xilinx_spips_update_cs_lines(s);
> > --
> > 2.7.4
> >
> >