Re: [PATCH v3 1/5] hw/ssi: Add Synopsys DWC SSI standard PIO controller
Kangjie Huang <[email protected]>
| Newsgroups | org.nongnu.qemu-riscv,org.nongnu.qemu-devel |
|---|---|
| Message-ID | <CAAW0U4afZ9Y6_dcypihVTSTWLpe0yVqfndWCOD15jU5xKZQk1g@mail.gmail.com> |
Bin Meng <[email protected]> 于2026年8月16日周日 23:28写道: > > + case A_TXFLR: > > + value = fifo32_num_used(&s->tx_fifo); > > + if (s->phase == DWC_SSI_PHASE_STANDARD_TX_ONLY) { > > + dwc_ssi_run_transfer(s); > > + } > > The modeling of transferring data from TX FIFO needs to be reconsidered. > > IIUC, currently the TX-only transfer advances in 64-frame batches > (DWC_SSI_PIO_TX_BATCH) and, once the phase is > DWC_SSI_PHASE_STANDARD_TX_ONLY, dwc_ssi_push_tx() deliberately skips > dwc_ssi_run_transfer(). The remaining FIFO entries only reach the wire > when the guest reads TXFLR or SR (see below). > > What if an interrupt-driven guest that waits for TXE without reading > those registers stalls once TXFTHR > 0: the FIFO stays above the > threshold, TXE never asserts, and nothing drains it. > > I suggest draining on DR write, like sifive_spi model does. No other > hw/ssi model advances its TX FIFO on status-register reads. sifive_spi > flushes the whole TX FIFO synchronously on the TXDATA write > (sifive_spi_flush_txfifo()), and with a FIFO depth of at most 256 > entries that is cheap. I'd drop DWC_SSI_PIO_TX_BATCH and have every DR > write run the transfer until the FIFO drains. That also removes the > asymmetry with the TR path, which already drains fully. > > > + break; > > + case A_SR: > > + value = dwc_ssi_status(s); > > + if (s->phase == DWC_SSI_PHASE_STANDARD_TX_ONLY) { > > + dwc_ssi_run_transfer(s); > > + } > > + break; Hi Bin, Thank you for reviewing the V3 patch. I agree that these issues need to be fixed. For the TX-only FIFO progress issue, the current 64-frame batching depends on reads from TXFLR/SR. In IRQ mode, the transfer may stop because TXE is never raised. While preparing, I found another issue in the K230 vendor Linux native-CS SPI-mem path. The driver first fills the TX FIFO before enabling SER. It then reads TXFLR before writing the remaining data: K230 SDK source: https://github.com/kendryte/k230_sdk/blob/v2.0/src/little/linux/drivers/spi/spi-dw-core-k230.c#L709-L742 len = min(dws->fifo_len, dws->tx_len); while (len--) dw_write_io_reg(dws, DW_SPI_DR, *buf++); len = dws->tx_len - ((void *)buf - dws->tx); k230_dw_spi_set_cs(spi, false); while (len) { entries = readl_relaxed(dws->regs + DW_SPI_TXFLR); if (!entries) { dev_err(&dws->master->dev, "CS de-assertion on Tx\n"); return -EIO; } room = min(dws->fifo_len - entries, len); ... write remaining frames to DR ... } For the current K230 profile, fifo_len is 256. When i test a 260-byte TX-only transfer has 4 bytes left after the first fill. If we remove the 64-frame limit and drain the FIFO immediately when SER starts the transfer, TXFLR may become zero before the driver writes these 4 bytes. The driver then returns "CS de-assertion on Tx". If only DR writes can advance the model, TXFLR may stay full and the driver will not issue a DR write to create room. The SDK also says that native CS is controlled automatically: when the TX FIFO has no data left, the controller de-asserts native CS. If the driver refills the FIFO too late, the transaction may end early. The K230-specific DWC SSI driver also comments out the generic SPI IRQ setup, so its standard transfer path uses polling. The generic DWC SSI driver still uses IRQ mode when an IRQ is available. K230 DWC SSI driver: https://github.com/kendryte/k230_sdk/blob/v2.0/src/little/linux/drivers/spi/spi-dw-core-k230.c#L589-L597 Generic DWC SSI driver: https://github.com/torvalds/linux/blob/8d3ae59288f1e7d58d76558a6ee96d533bc5019f/drivers/spi/spi-dw-core.c#L416-L465 We need to fix the FIFO stall in IRQ mode, but the native-CS path must not see TXFLR=0 too early. I am not sure how to handle both cases. Could you please advise whether we need another way to advance the FIFO, such as a QEMU timer or another mechanism? Regards, Kangjie