Re: [PATCH] spi: spi-pci1xxxx: quiesce DMA engines on transfer timeout
From: Weibin Liu
Date: Thu Oct 15 2026 - 22:01:16 EST
On Wed, Oct 14, 2026 at 03:37:15PM +0000, Mark Brown wrote:
> Why is the sensible thing to reset both channels immediately rather than
> try to let the other one complete (or time out) if it's active?
The two engines are not two independent channels that could each
finish their own work: they are the two legs of the same transfer.
The RD engine moves the TX direction (host to device), the WR engine
moves the RX direction (device to host), both are tied together by
dma_completion_count, and complete() only fires on the WR engine's
done for the last rx_sgl. When one engine is wedged, the other can
never reach the end of the transfer, so "letting it complete" has no
reachable exit in this state machine; it would only run into its own
100 ms timeout.
Waiting would also not be safe: once transfer_one() returns, the SPI
core is free to unmap or reuse the buffers, and an engine that is
still enabled keeps DMAing into that memory. The timeout path has to
stop the engines itself, and stopping both immediately just ends that
window. The stop sequence is the one the driver already uses for its
abort paths (disable, poll for the readback to clear, drain this
instance's pending status, synchronize_irq); only the trigger changes,
from an abort interrupt to a timeout with no interrupt at all.
The cost to be upfront about is that the engine enable bits are shared
between the two SPI instances, so a transfer in flight on the peer
instance is interrupted and sees one recoverable -ETIMEDOUT (its own
wait has the same 100 ms bound). The abort paths write the same
direction-wide enable bits with the same blast radius, so this patch
does not introduce a new category of interference. The interrupt
enables are likewise shared per direction, which is why the cleanup
drains only this instance's status bits instead of masking the lines.
I'll follow up with a v2 that makes the re-enable conditional: the
engines are turned back on only once both were confirmed to have
stopped, and are otherwise left disabled - matching the abort paths,
where a failed stop returns -ECANCELED and the engines stay disabled.
Unconditionally re-enabling an engine that failed to stop would cancel
the pending stop request.
I don't have this part on a board and QEMU has no model of it. If
someone at Microchip could exercise the timeout recovery path on
hardware (in particular whether the 1 ms stop poll budget is enough
there), that would be much appreciated.