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

Sai Pavan Boddu posted 1 patch 4 years, 7 months ago
Test asan passed
Test checkpatch passed
Test FreeBSD passed
Test docker-mingw@fedora passed
Test docker-clang@ubuntu passed
Test docker-quick@centos7 passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/1571307474-16222-1-git-send-email-sai.pavan.boddu@xilinx.com
Maintainers: Alistair Francis <alistair@alistair23.me>
hw/ssi/xilinx_spips.c | 7 +++++++
1 file changed, 7 insertions(+)
[QEMU][PATCH v2] ssi: xilinx_spips: Skip update of cs and fifo releated to spips in gqspi
Posted by Sai Pavan Boddu 4 years, 7 months ago
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
Posted by Francisco Iglesias 4 years, 7 months ago
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
Posted by Sai Pavan Boddu 4 years, 7 months ago
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
> >
> >

Re: [QEMU][PATCH v2] ssi: xilinx_spips: Skip update of cs and fifo releated to spips in gqspi
Posted by Philippe Mathieu-Daudé 4 years, 7 months ago
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);
>