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

Hi Cole,

On 8/20/26 5:26 PM, Cole Munz wrote:
> The controller has a transfer-mode field that can run transmit-only or
> receive-only instead of both, which leaves the unused FIFO out of the
> transfer entirely. The driver never used it for that: claim_bus always
> programmed TMOD_TR, and the only other mode came from an opportunistic
> switch to TMOD_RO for read-only transfers.
> 
> A device described with spi-{tx,rx}-bus-width = <0> has no wire in that
> direction at all, so now that the width reaches plat->mode as
> SPI_NO_TX/SPI_NO_RX, pick the transfer mode from it. A write-only
> display stops clocking receive bytes nobody reads.
> 
> The transmit-only case needs one more change. The 8-bit loop paces
> itself on the receive FIFO and sets toread unconditionally, so with no
> receive path it would wait on a FIFO that stays empty forever. Leave
> toread at zero there and let the existing wait_till_not_busy() at the
> end of the chunk handle completion, which is the same thing that
> already covers a transmit component today.
> 
> The restore at the end of a read-only transfer went back to a hardcoded
> TMOD_TR, which would undo the device's own mode. Restore what the mode
> asks for instead.
> 
> Signed-off-by: Cole Munz <[email protected]>
> Tested-by: Alexey Charkov <[email protected]>
> ---
> Changes in v2: reworded the transfer mode comment in claim_bus, picked
> up the Tested-by. No functional change since v1; compile check rerun:
> 
>    $ make jaguar-rk3588_defconfig
>    $ make CROSS_COMPILE=aarch64-linux-gnu- drivers/spi/rk_spi.o
>    CC      drivers/spi/rk_spi.o     (exit 0)
> 
>   drivers/spi/rk_spi.c | 27 +++++++++++++++++++++++----
>   1 file changed, 23 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/spi/rk_spi.c b/drivers/spi/rk_spi.c
> index 2c3d70ba7159..81785b5e949f 100644
> --- a/drivers/spi/rk_spi.c
> +++ b/drivers/spi/rk_spi.c
> @@ -283,6 +283,20 @@ static int rockchip_spi_probe(struct udevice *bus)
>   	return 0;
>   }
>   
> +/*
> + * A device that declares spi-{tx,rx}-bus-width = <0> has no wire in that
> + * direction, so the controller can drop the matching FIFO entirely instead
> + * of clocking bytes nobody reads.
> + */
> +static u32 rkspi_base_tmod(struct rockchip_spi_priv *priv)
> +{
> +	if (priv->mode & SPI_NO_RX)
> +		return TMOD_TO;
> +	if (priv->mode & SPI_NO_TX)
> +		return TMOD_RO;
> +	return TMOD_TR;
> +}
> +
>   static int rockchip_spi_claim_bus(struct udevice *dev)
>   {
>   	struct udevice *bus = dev->parent;
> @@ -329,8 +343,8 @@ static int rockchip_spi_claim_bus(struct udevice *dev)
>   	/* Frame Format */
>   	ctrlr0 |= FRF_SPI << FRF_SHIFT;
>   
> -	/* Tx and Rx mode */
> -	ctrlr0 |= TMOD_TR << TMOD_SHIFT;
> +	/* Configure RX/TX mode */
> +	ctrlr0 |= rkspi_base_tmod(priv) << TMOD_SHIFT;
>   
>   	writel(ctrlr0, &regs->ctrlr0);
>   
> @@ -472,7 +486,12 @@ static int rockchip_spi_xfer(struct udevice *dev, unsigned int bitlen,
>   		writel(todo - 1, &regs->ctrlr1);
>   		rkspi_enable_chip(regs, true);
>   
> -		toread = todo;
> +		/*
> +		 * In transmit-only mode the RX FIFO never fills, so waiting
> +		 * on it would hang. Completion is handled by the
> +		 * wait_till_not_busy() below instead.
> +		 */

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?

Cheers,
Quentin