On Mon, Jul 6, 2026 at 4:11 PM Philippe Mathieu-Daudé
<[email protected]> wrote:
>
> On 30/6/26 15:57, Bin Meng wrote:
> > The ZynqMP generic FIFO encodes dummy phases as a number of
> > dummy cycles. QEMU's SSI bus transfers whole bytes, so the
> > controller model must convert the programmed cycle count to the
> > number of SSI byte transfers needed for the selected SPI, dual SPI
> > or quad SPI mode.
> >
> > The legacy Xilinx QSPI snoop paths had the opposite problem after
> > the m25p80 dummy handling was fixed. They still treated each dummy
> > byte queued through the FIFO as a request to generate several SSI
> > transfers based on the current link width. The flash model now
> > consumes dummy phases as byte counts, so the manual FIFO path should
> > forward one SSI transfer per dummy byte.
> >
> > Update the Xilinx QSPI dummy accounting consistently for the generic
> > FIFO, manual FIFO and LQSPI direct-read paths. Also make the command
> > table report the dummy byte counts consumed by m25p80 for dual and
> > quad output reads, and account for the mode byte before LQSPI data
> > reads begin.
> >
> > This matches the ZynqMP TRM (ug1085, v2.2 [1]) description of the
> > generic FIFO dummy cycle entry and keeps the controller side aligned
> > with the flash model's dummy byte ownership.
> >
> > The description of the genenic command fifo register says:
> >
> > When [receive, transmit, data_xfer] = [0,0,1], the [immediate_data]
> > field represents the number of dummy cycle sent on the SPI interface.
> >
> > [1]
> > https://www.xilinx.com/support/documentation/user_guides/ug1085-zynq-ultrascale-trm.pdf
> > table 24‐22, an example of Generic FIFO Contents for Quad I/O Read
> > Command (EBh)
> >
> > Fixes: ef06ca3946e2 ("xilinx_spips: Add support for RX discard and RX
> > drain")
> > Fixes: c95997a39de6 ("xilinx_spips: Add support for the ZynqMP Generic
> > QSPI")
> > Signed-off-by: Bin Meng <[email protected]>
> > ---
> >
> > include/hw/ssi/xilinx_spips.h | 2 +-
> > hw/ssi/xilinx_spips.c | 120 ++++++++++++++++++++++++----------
> > 2 files changed, 88 insertions(+), 34 deletions(-)
>
>
> > diff --git a/hw/ssi/xilinx_spips.c b/hw/ssi/xilinx_spips.c
> > index f6e717bc01..fa98dc2dd8 100644
> > --- a/hw/ssi/xilinx_spips.c
> > +++ b/hw/ssi/xilinx_spips.c
> > @@ -192,6 +192,10 @@
> > FIELD(GQSPI_GF_SNAPSHOT, EXPONENT, 9, 1)
> > FIELD(GQSPI_GF_SNAPSHOT, DATA_XFER, 8, 1)
> > FIELD(GQSPI_GF_SNAPSHOT, IMMEDIATE_DATA, 0, 8)
> > +#define GQSPI_GF_MODE_SPI 1
> > +#define GQSPI_GF_MODE_DSPI 2
> > +#define GQSPI_GF_MODE_QSPI 3
>
>
> > @@ -457,6 +461,7 @@ static void
> > xlnx_zynqmp_qspips_flush_fifo_g(XlnxZynqMPQSPIPS *s)
> > int i;
> >
> > if (!s->regs[R_GQSPI_DATA_STS]) {
> > + uint32_t prev_gf_snapshot = s->regs[R_GQSPI_GF_SNAPSHOT];
> > uint8_t imm;
> >
> > s->regs[R_GQSPI_GF_SNAPSHOT] = fifo32_pop(&s->fifo_g);
> > @@ -484,7 +489,54 @@ static void
> > xlnx_zynqmp_qspips_flush_fifo_g(XlnxZynqMPQSPIPS *s)
> > }
> > s->regs[R_GQSPI_DATA_STS] = 1ul << imm;
> > } else {
> > - s->regs[R_GQSPI_DATA_STS] = imm;
> > + /*
> > + * When [receive, transmit, data_xfer] = [0,0,1], it
> > represents
> > + * the number of dummy cycle sent on the SPI interface. We
> > need
> > + * to convert the number of dummy cycles to bytes
> > according to
> > + * the SPI mode being used.
> > + *
> > + * Ref: ug1085 v2.2 (December 2020) table 24‐22, an
> > example of
> > + * Generic FIFO Contents for Quad I/O Read Command
> > (EBh)
> > + */
> > + if (!ARRAY_FIELD_EX32(s->regs, GQSPI_GF_SNAPSHOT,
> > TRANSMIT) &&
> > + !ARRAY_FIELD_EX32(s->regs, GQSPI_GF_SNAPSHOT,
> > RECIEVE)) {
> > + uint8_t spi_mode = ARRAY_FIELD_EX32(s->regs,
> > GQSPI_GF_SNAPSHOT, SPI_MODE);
> > + /*
> > + * Some ZynqMP GQSPI drivers, such as Linux, use the
> > data
> > + * bus width in the dummy GENFIFO entry only to
> > configure
> > + * the controller mode. The immediate value is already
> > + * the number of dummy cycles for the dummy phase,
> > which
> > + * follows the address bus width. Reuse the previous TX
> > + * phase mode to convert cycles to SSI bytes.
> > + *
> > + * This does not make the model Linux-only. U-Boot
> > emits
> > + * the dummy entry with op->dummy.buswidth, so the
> > entry
> > + * mode already matches the dummy phase. Its opcode and
> > + * address phases are immediate entries, not DATA_XFER
> > TX
> > + * entries, so the override below is not taken for
> > U-Boot.
> > + */
> > + if (FIELD_EX32(prev_gf_snapshot, GQSPI_GF_SNAPSHOT,
> > + DATA_XFER) &&
> > + FIELD_EX32(prev_gf_snapshot, GQSPI_GF_SNAPSHOT,
> > + TRANSMIT) &&
> > + !FIELD_EX32(prev_gf_snapshot, GQSPI_GF_SNAPSHOT,
> > + RECIEVE)) {
> > + spi_mode = FIELD_EX32(prev_gf_snapshot,
> > + GQSPI_GF_SNAPSHOT, SPI_MODE);
> > + }
> > +
> > + if (spi_mode == GQSPI_GF_MODE_QSPI) {
> > + s->regs[R_GQSPI_DATA_STS] = ROUND_UP(imm * 4, 8) /
> > 8;
> > + } else if (spi_mode == GQSPI_GF_MODE_DSPI) {
> > + s->regs[R_GQSPI_DATA_STS] = ROUND_UP(imm * 2, 8) /
> > 8;
> > + } else if (spi_mode == GQSPI_GF_MODE_SPI) {
> > + s->regs[R_GQSPI_DATA_STS] = ROUND_UP(imm * 1, 8) /
> > 8;
> > + } else {
> > + qemu_log_mask(LOG_GUEST_ERROR, "Unknown SPI MODE:
> > 0x%x ", spi_mode);
>
> Is it safe to let the previous s->regs[R_GQSPI_DATA_STS] value?
Yes, the previous value remains zero and won't pass the sanity check
below (see /* Zero length transfer check */)
>
> Alternatively simpler as:
>
> if (spi_mode) {
> s->regs[R_GQSPI_DATA_STS] = 2 * (spi_mode - 1) * imm;
No, imm should be divided by 8 as we are counting bytes here, not cycles.
>
> > + }
> > + } else {
> > + s->regs[R_GQSPI_DATA_STS] = imm;
> > + }
> > }
> > }
> > /* Zero length transfer check */
> > @@ -550,7 +602,7 @@ static void
> > xlnx_zynqmp_qspips_flush_fifo_g(XlnxZynqMPQSPIPS *s)
> > }
> > }
>
Regards,
Bin
