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?

Alternatively simpler as:

 if (spi_mode) {
     s->regs[R_GQSPI_DATA_STS] = 2 * (spi_mode - 1) * imm;

+                    }
+                } 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)
      }
  }

Reply via email to