Re: [PATCH v2 2/2] spi: rockchip: skip the unused FIFO direction on a one-wire device

Hi Cole,

On 8/20/26 7:00 PM, Cole Munz wrote:
> Hi Quentin,
> 
> On 8/20/26 5:57 PM, Quentin Schulz wrote:
>> I got confused by the wording here. Can I suggest:
>>
>> /* When RX wire is not routed, the RX FIFO can never fill, so waiting on
>> it would hang. */
>>
>> I don't understand the context for the second sentence though, we are
>> always waiting until not busy, if there's something to transmit, it
>> doesn't have anything to do with the RX path does it? What am I missing
>> here?
> 
> The RX drain was doing double duty in TMOD_TR. Byte N only shows up in
> the RX FIFO after byte N has gone out on the wire. Draining toread

Are you sure? The controller needs to tell the peripheral which data 
it's interested in. Therefore, the peripheral cannot send data to the 
controller until it knows what it wants? c.f. 
https://learn.sparkfun.com/tutorials/serial-peripheral-interface-spi/all 
in the Receiving data section. Concurrent TX and RX may happen but it's 
not a given (and in any case, you'd have a first PICO transfer to 
specify what to read next on the POCI line).

At a logic level, the current while loop may very well write all of 
towrite without a single toread being read (due to the RX FIFO being 
empty in the controller) and then go through all toread, or it could 
write 8b and read 8b at a time (or a mixed number of 8b write/read, 
though I don't think this is necessarily possible).

> bytes is therefore also what made the loop wait for the transfer
> itself. With toread forced to zero the loop exits as soon as the last
> byte lands in the TX FIFO, which says nothing about the wire. From
> there the wait_till_not_busy() below is the only wait left. That is
> what the sentence tried to point at.
> 
> You're right that it explained none of that. v3 takes your first
> sentence and adds the pacing part:
> 
> 	When the RX wire is not routed, the RX FIFO never fills,
> 	so waiting on it would hang. Draining it was also what
> 	paced this loop against the wire, so the transmit-only

The controller has 64x 16b-wide RX and TX FIFOs, so my reading of the 
TRM means that you can have up to 64 16b data pending in RX and/or TX 
FIFOs. So I'm not sure "pacing" is the correct term here. If we're doing 
full-duplex (which depends on the device and how we write the driver 
since we need to write once in the TX FIFO before doing full-duplex and 
both reading and writing at the same time), then I think it's pretty 
much guaranteed we're sending and reading bits on PICO/POCI at the same 
clock edge. If we aren't doing full-duplex, then we may have to wait to 
read from the RX FIFO after we've actually sent stuff on the wire from 
the TX FIFO. In conclusion, the wait_till_not_busy() is actually only 
useful if we're doing a tx-only transfer as writing to the TX FIFO 
doesn't mean it's sent over the wire. If we agree on this, then I think 
it's more appropriate to reword the comment just before 
rkspi_wait_till_not_busy() to specify we wait till the TX FIFO has been 
sent over the wire before starting a new transfer as disabling the 
controller will clear all FIFOs.

Cheers,
Quentin