1. Patchew
  2. QEMU
  3. View series

[PATCH] ssi: xilinx_spips: Filter the non spi registers transactions

Sai Pavan Boddu posted 1 patch 4 years, 6 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/1571035899-9692-1-git-send-email-sai.pavan.boddu@xilinx.com
Maintainers: Alistair Francis <alistair@alistair23.me>
hw/ssi/xilinx_spips.c | 26 ++++++++++++++++++++++++--
1 file changed, 24 insertions(+), 2 deletions(-)
[PATCH] ssi: xilinx_spips: Filter the non spi registers transactions
Posted by Sai Pavan Boddu 4 years, 6 months ago 
ZynqMP/Versal specific qspi registers should be handled inside
zynqmp_qspi_read/write calls. When few of these transactions are handled by
spi hooks we see state change in spi bus unexpectedly.

Signed-off-by: Sai Pavan Boddu <sai.pavan.boddu@xilinx.com>
---
 hw/ssi/xilinx_spips.c | 26 ++++++++++++++++++++++++--
 1 file changed, 24 insertions(+), 2 deletions(-)

diff --git a/hw/ssi/xilinx_spips.c b/hw/ssi/xilinx_spips.c
index a309c71..4f9f8e0 100644
--- a/hw/ssi/xilinx_spips.c
+++ b/hw/ssi/xilinx_spips.c
@@ -109,6 +109,7 @@
 #define R_GPIO              (0x30 / 4)
 #define R_LPBK_DLY_ADJ      (0x38 / 4)
 #define R_LPBK_DLY_ADJ_RESET (0x33)
+#define R_IOU_TAPDLY_BYPASS (0x3C / 4)
 #define R_TXD1              (0x80 / 4)
 #define R_TXD2              (0x84 / 4)
 #define R_TXD3              (0x88 / 4)
@@ -139,6 +140,8 @@
 #define R_LQSPI_STS         (0xA4 / 4)
 #define LQSPI_STS_WR_RECVD      (1 << 1)
 
+#define R_DUMMY_CYCLE_EN    (0xC8 / 4)
+#define R_ECO               (0xF8 / 4)
 #define R_MOD_ID            (0xFC / 4)
 
 #define R_GQSPI_SELECT          (0x144 / 4)
@@ -938,7 +941,16 @@ static uint64_t xlnx_zynqmp_qspips_read(void *opaque,
     int shortfall;
 
     if (reg <= R_MOD_ID) {
-        return xilinx_spips_read(opaque, addr, size);
+        switch (addr) {
+        case R_GPIO:
+        case R_LPBK_DLY_ADJ:
+        case R_IOU_TAPDLY_BYPASS:
+        case R_DUMMY_CYCLE_EN:
+        case R_ECO:
+            return s->regs[addr / 4];
+        default:
+            return xilinx_spips_read(opaque, addr, size);
+        }
     } else {
         switch (reg) {
         case R_GQSPI_RXD:
@@ -1063,7 +1075,17 @@ static void xlnx_zynqmp_qspips_write(void *opaque, hwaddr addr,
     uint32_t reg = addr / 4;
 
     if (reg <= R_MOD_ID) {
-        xilinx_qspips_write(opaque, addr, value, size);
+        switch (reg) {
+        case R_GPIO:
+        case R_LPBK_DLY_ADJ:
+        case R_IOU_TAPDLY_BYPASS:
+        case R_DUMMY_CYCLE_EN:
+        case R_ECO:
+            s->regs[addr] = value;
+            break;
+        default:
+            xilinx_qspips_write(opaque, addr, value, size);
+        }
     } else {
         switch (reg) {
         case R_GQSPI_CNFG:
-- 
2.7.4
Re: [PATCH] ssi: xilinx_spips: Filter the non spi registers transactions
Posted by Alistair Francis 4 years, 6 months ago 
On Sun, Oct 13, 2019 at 11:51 PM Sai Pavan Boddu
<sai.pavan.boddu@xilinx.com> wrote:
>
> ZynqMP/Versal specific qspi registers should be handled inside
> zynqmp_qspi_read/write calls. When few of these transactions are handled by
> spi hooks we see state change in spi bus unexpectedly.
>
> Signed-off-by: Sai Pavan Boddu <sai.pavan.boddu@xilinx.com>
> ---
>  hw/ssi/xilinx_spips.c | 26 ++++++++++++++++++++++++--
>  1 file changed, 24 insertions(+), 2 deletions(-)
>
> diff --git a/hw/ssi/xilinx_spips.c b/hw/ssi/xilinx_spips.c
> index a309c71..4f9f8e0 100644
> --- a/hw/ssi/xilinx_spips.c
> +++ b/hw/ssi/xilinx_spips.c
> @@ -109,6 +109,7 @@
>  #define R_GPIO              (0x30 / 4)
>  #define R_LPBK_DLY_ADJ      (0x38 / 4)
>  #define R_LPBK_DLY_ADJ_RESET (0x33)
> +#define R_IOU_TAPDLY_BYPASS (0x3C / 4)
>  #define R_TXD1              (0x80 / 4)
>  #define R_TXD2              (0x84 / 4)
>  #define R_TXD3              (0x88 / 4)
> @@ -139,6 +140,8 @@
>  #define R_LQSPI_STS         (0xA4 / 4)
>  #define LQSPI_STS_WR_RECVD      (1 << 1)
>
> +#define R_DUMMY_CYCLE_EN    (0xC8 / 4)
> +#define R_ECO               (0xF8 / 4)
>  #define R_MOD_ID            (0xFC / 4)
>
>  #define R_GQSPI_SELECT          (0x144 / 4)
> @@ -938,7 +941,16 @@ static uint64_t xlnx_zynqmp_qspips_read(void *opaque,
>      int shortfall;
>
>      if (reg <= R_MOD_ID) {
> -        return xilinx_spips_read(opaque, addr, size);
> +        switch (addr) {
> +        case R_GPIO:
> +        case R_LPBK_DLY_ADJ:
> +        case R_IOU_TAPDLY_BYPASS:
> +        case R_DUMMY_CYCLE_EN:
> +        case R_ECO:
> +            return s->regs[addr / 4];
> +        default:
> +            return xilinx_spips_read(opaque, addr, size);

This doesn't seem right. This should have no functional change for the
read function and has the consequence of not printing the memory
accesses. If you try to debug this code now you won't see all of these
operations in the log.

> +        }
>      } else {
>          switch (reg) {
>          case R_GQSPI_RXD:
> @@ -1063,7 +1075,17 @@ static void xlnx_zynqmp_qspips_write(void *opaque, hwaddr addr,
>      uint32_t reg = addr / 4;
>
>      if (reg <= R_MOD_ID) {
> -        xilinx_qspips_write(opaque, addr, value, size);
> +        switch (reg) {
> +        case R_GPIO:
> +        case R_LPBK_DLY_ADJ:
> +        case R_IOU_TAPDLY_BYPASS:
> +        case R_DUMMY_CYCLE_EN:
> +        case R_ECO:
> +            s->regs[addr] = value;
> +            break;
> +        default:
> +            xilinx_qspips_write(opaque, addr, value, size);
> +        }

For the write code it looks like this skips the "no_reg_update" goto.
Maybe that is the issue that you are seeing?

Alistair

>      } else {
>          switch (reg) {
>          case R_GQSPI_CNFG:
> --
> 2.7.4
>
>
RE: [PATCH] ssi: xilinx_spips: Filter the non spi registers transactions
Posted by Sai Pavan Boddu 4 years, 6 months ago 
Hi Alistair,

> -----Original Message-----
> From: Alistair Francis <alistair23@gmail.com>
> Sent: Thursday, October 17, 2019 4:39 AM
> To: Sai Pavan Boddu <saipava@xilinx.com>
> Cc: Alistair Francis <alistair@alistair23.me>; Peter Maydell
> <peter.maydell@linaro.org>; qemu-devel@nongnu.org Developers <qemu-
> devel@nongnu.org>
> Subject: Re: [PATCH] ssi: xilinx_spips: Filter the non spi registers transactions
> 
> On Sun, Oct 13, 2019 at 11:51 PM Sai Pavan Boddu
> <sai.pavan.boddu@xilinx.com> wrote:
> >
> > ZynqMP/Versal specific qspi registers should be handled inside
> > zynqmp_qspi_read/write calls. When few of these transactions are
> > handled by spi hooks we see state change in spi bus unexpectedly.
> >
> > Signed-off-by: Sai Pavan Boddu <sai.pavan.boddu@xilinx.com>
> > ---
> >  hw/ssi/xilinx_spips.c | 26 ++++++++++++++++++++++++--
> >  1 file changed, 24 insertions(+), 2 deletions(-)
> >
> > diff --git a/hw/ssi/xilinx_spips.c b/hw/ssi/xilinx_spips.c index
> > a309c71..4f9f8e0 100644
> > --- a/hw/ssi/xilinx_spips.c
> > +++ b/hw/ssi/xilinx_spips.c
> > @@ -109,6 +109,7 @@
> >  #define R_GPIO              (0x30 / 4)
> >  #define R_LPBK_DLY_ADJ      (0x38 / 4)
> >  #define R_LPBK_DLY_ADJ_RESET (0x33)
> > +#define R_IOU_TAPDLY_BYPASS (0x3C / 4)
> >  #define R_TXD1              (0x80 / 4)
> >  #define R_TXD2              (0x84 / 4)
> >  #define R_TXD3              (0x88 / 4)
> > @@ -139,6 +140,8 @@
> >  #define R_LQSPI_STS         (0xA4 / 4)
> >  #define LQSPI_STS_WR_RECVD      (1 << 1)
> >
> > +#define R_DUMMY_CYCLE_EN    (0xC8 / 4)
> > +#define R_ECO               (0xF8 / 4)
> >  #define R_MOD_ID            (0xFC / 4)
> >
> >  #define R_GQSPI_SELECT          (0x144 / 4)
> > @@ -938,7 +941,16 @@ static uint64_t xlnx_zynqmp_qspips_read(void
> *opaque,
> >      int shortfall;
> >
> >      if (reg <= R_MOD_ID) {
> > -        return xilinx_spips_read(opaque, addr, size);
> > +        switch (addr) {
> > +        case R_GPIO:
> > +        case R_LPBK_DLY_ADJ:
> > +        case R_IOU_TAPDLY_BYPASS:
> > +        case R_DUMMY_CYCLE_EN:
> > +        case R_ECO:
> > +            return s->regs[addr / 4];
> > +        default:
> > +            return xilinx_spips_read(opaque, addr, size);
> 
> This doesn't seem right. This should have no functional change for the read
> function and has the consequence of not printing the memory accesses. If
> you try to debug this code now you won't see all of these operations in the
> log.
[Sai Pavan Boddu] Yeah reads do not have any issue. But I see your point of debug prints.
> 
> > +        }
> >      } else {
> >          switch (reg) {
> >          case R_GQSPI_RXD:
> > @@ -1063,7 +1075,17 @@ static void xlnx_zynqmp_qspips_write(void
> *opaque, hwaddr addr,
> >      uint32_t reg = addr / 4;
> >
> >      if (reg <= R_MOD_ID) {
> > -        xilinx_qspips_write(opaque, addr, value, size);
> > +        switch (reg) {
> > +        case R_GPIO:
> > +        case R_LPBK_DLY_ADJ:
> > +        case R_IOU_TAPDLY_BYPASS:
> > +        case R_DUMMY_CYCLE_EN:
> > +        case R_ECO:
> > +            s->regs[addr] = value;
> > +            break;
> > +        default:
> > +            xilinx_qspips_write(opaque, addr, value, size);
> > +        }
> 
> For the write code it looks like this skips the "no_reg_update" goto.
> Maybe that is the issue that you are seeing?
[Sai Pavan Boddu] yes, no_reg_update triggers update of cs lines.
We can also put a check there to skip no_reg_update when it’s a zynqmp qspi.
I will try that and send a V2.

Thanks
Sai Pavan
> 
> Alistair
> 
> >      } else {
> >          switch (reg) {
> >          case R_GQSPI_CNFG:
> > --
> > 2.7.4
> >
> >

Patchew 2.1-devel-0870f84

© 2016 - 2024 Red Hat, Inc.