Hi Alistair,
> -----Original Message-----
> From: Alistair Francis <alistai...@gmail.com>
> Sent: Thursday, October 17, 2019 4:39 AM
> To: Sai Pavan Boddu <saip...@xilinx.com>
> Cc: Alistair Francis <alist...@alistair23.me>; Peter Maydell
> <peter.mayd...@linaro.org>; qemu-devel@nongnu.org Developers <qemu-
> de...@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.bo...@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.bo...@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
> >
> >
