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

Cole Munz <[email protected]>
Newsgroups org.u-boot-project.lists.u-boot
Message-ID <69c35eba7ee1a353107459b8f8032e92506a3fb1.1787045568.git.Munzzyy1@proton.me>
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]>
---
Alexey, this is the FIFO wiring you asked about. It applies on top of
"spi: Handle spi-{tx,rx}-bus-width 0 as SPI_NO_TX/SPI_NO_RX", since it
needs those bits to exist.

You are right that the first patch is a no-op on its own. With this one
the Flipper One display bus stops running the receive FIFO at all: the
MISO pin is the end-of-frame GPIO, so every byte the controller clocked
in was discarded by the loop anyway.

The part that is not just a mode select is the 8-bit loop. It sets
toread = todo whether or not the caller passed a din, and drains the
receive FIFO to pace itself. In TMOD_TO that FIFO never fills, so it
would spin forever. Leaving toread at zero and letting the existing
rkspi_wait_till_not_busy() close out the chunk keeps the timing the same
for the transmit case, which already relied on that call.

Verification, and its limits. I have no Rockchip board, so this is
compile-tested and reasoned from the driver, not run:

 $ make jaguar-rk3588_defconfig
 $ make CROSS_COMPILE=aarch64-linux-gnu- drivers/spi/rk_spi.o
 CC      drivers/spi/rk_spi.o     (exit 0)

checkpatch --strict is 0/0/0. A full board build stops in binman for want
of BL31 and tee.bin, identically with and without this patch, so that one
is my missing blobs rather than the change.

What I cannot check here is the hardware behaviour: that TMOD_TO really
does leave the receive FIFO idle on a real part, and that a write-only
display still clocks out correctly. If you have a board in front of you,
that is the bit worth a look.

 drivers/spi/rk_spi.c | 25 ++++++++++++++++++++++---
 1 file changed, 22 insertions(+), 3 deletions(-)

diff --git a/drivers/spi/rk_spi.c b/drivers/spi/rk_spi.c
index 2c3d70ba7159..6c2ed3a90bf9 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;
@@ -330,7 +344,7 @@ static int rockchip_spi_claim_bus(struct udevice *dev)
 	ctrlr0 |= FRF_SPI << FRF_SHIFT;
 
 	/* Tx and Rx mode */
-	ctrlr0 |= TMOD_TR << TMOD_SHIFT;
+	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.
+		 */
+		toread = (priv->mode & SPI_NO_RX) ? 0 : todo;
 		/* Only write if we have something to write */
 		towrite = out ? todo : 0;
 		while (toread || towrite) {
@@ -513,7 +532,7 @@ static int rockchip_spi_xfer(struct udevice *dev, unsigned int bitlen,
 	if (!out)
 		clrsetbits_le32(&regs->ctrlr0,
 				TMOD_MASK << TMOD_SHIFT,
-				TMOD_TR << TMOD_SHIFT);
+				rkspi_base_tmod(priv) << TMOD_SHIFT);
 
 	return ret;
 }
-- 
2.55.0
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.