Re: [PATCH] spi: xilinx_spi: fix premature transfer termination at low SCK

[Michal Simek]

On 8/13/26 14:06, Suraj Kakade wrote:

start_transfer() asserts SPICR_MASTER_INHIBIT as soon as SPISR_TX_EMPTY
is seen. SPISR_TX_EMPTY only indicates that the TX FIFO has drained;
the final byte may still be in the shift pipeline and actively
transmitting on the SPI bus. At low SCK frequencies the gap between
FIFO-empty and the last bit finishing its shift-out becomes wide
enough that MASTER_INHIBIT gets asserted before the transfer actually
completes, truncating the last byte and leaving the clock generation
logic in an error state.

Switch to polling IPISR_DTR_EMPTY instead, which is only set once the
last byte has genuinely finished shifting out to the slave, and only
assert MASTER_INHIBIT afterwards. Clear any stale DTR_EMPTY before
starting a transfer and after detecting completion, since IPISR is
toggle-on-write. This fix works correctly at both high and low SCK
frequencies.

Fixes: 0c0de58f7b30 ("spi: xilinx_spi: Modify transfer logic xilinx_spi_xfer() 
function")
Signed-off-by: Padmarao Begari <padmarao.beg...@amd.com>
Signed-off-by: Suraj Kakade <suraj.hanumantkak...@amd.com>
---
  drivers/spi/xilinx_spi.c | 21 ++++++++++++++++++++-
  1 file changed, 20 insertions(+), 1 deletion(-)

diff --git a/drivers/spi/xilinx_spi.c b/drivers/spi/xilinx_spi.c
index b2af17ebae9..011bbc93076 100644
--- a/drivers/spi/xilinx_spi.c
+++ b/drivers/spi/xilinx_spi.c
@@ -53,6 +53,8 @@
  #define SPISR_RX_FULL         BIT(1)
  #define SPISR_RX_EMPTY                BIT(0)

+#define IPISR_DTR_EMPTY BIT(2)

+
  /* SPI Data Transmit Register (spidtr), [1] p12, [2] p12 */
  #define SPIDTR_8BIT_MASK      GENMASK(7, 0)
  #define SPIDTR_16BIT_MASK     GENMASK(15, 0)
@@ -256,6 +258,13 @@ static int start_transfer(struct udevice *dev, const void 
*dout, void *din, u32
                reg = readl(&regs->spicr) | SPICR_MASTER_INHIBIT;
                writel(reg, &regs->spicr);
                count = xilinx_spi_fill_txfifo(bus, txp, txbytes);
+               /*
+                * Clear any stale DTR_EMPTY before starting. IPISR is
+                * toggle-on-write, so only write (toggle) the bit when it is
+                * actually set, otherwise the write would spuriously set it.
+                */
+               if (readl(&regs->ipisr) & IPISR_DTR_EMPTY)
+                       writel(IPISR_DTR_EMPTY, &regs->ipisr);
                /* Enable master transaction */
                reg = readl(&regs->spicr) & ~SPICR_MASTER_INHIBIT;
                writel(reg, &regs->spicr);
@@ -263,12 +272,22 @@ static int start_transfer(struct udevice *dev, const void 
*dout, void *din, u32
                if (txp)
                        txp += count;

- ret = wait_for_bit_le32(&regs->spisr, SPISR_TX_EMPTY, true,

+               /*
+                * Wait for the transfer to fully complete before inhibiting
+                * the master. The final byte may still be in the shift
+                * pipeline. Poll IPISR_DTR_EMPTY, which is set only once the
+                * last byte has been shifted out to the slave. Asserting
+                * MASTER_INHIBIT before this stops SCK and truncates the last
+                * byte, which is observable at low SCK frequencies.
+                */
+               ret = wait_for_bit_le32(&regs->ipisr, IPISR_DTR_EMPTY, true,
                                        XILINX_SPISR_TIMEOUT, false);
                if (ret < 0) {
                        printf("XILSPI error: Xfer timeout\n");
                        return ret;
                }
+               /* IPISR is toggle-on-write; write 1 to clear DTR_EMPTY */
+               writel(IPISR_DTR_EMPTY, &regs->ipisr);

reg = readl(&regs->spicr) | SPICR_MASTER_INHIBIT;

                writel(reg, &regs->spicr);

Applied.
M