[PATCH v2 1/2] spi: Handle spi-{tx, rx}-bus-width 0 as SPI_NO_TX/SPI_NO_RX

The spi-peripheral-props binding shipped in dts/upstream allows a bus
width of 0, meaning no RX or TX is possible on this device. The
switches in spi_slave_of_to_plat() only handle 1/2/4/8, so a width of
0 falls through to the default case and warns "spi-rx-bus-width 0 not
supported" on every boot, even though the devicetree is valid per the
binding. The fact that the wire is missing is then dropped from
plat->mode.

Map 0 to new SPI_NO_TX/SPI_NO_RX mode bits, as Linux has done since
v5.12 ("spi: Add SPI_NO_TX/RX support", mainline d962608ce218).
Bits 16 and 17 are the first free mode bits.

Mapping the bits is not enough on its own, as Quentin pointed out:
nothing would stop a caller from asking for a transfer in a direction
that has no wire, and every controller driver would need its own
guard. Validate centrally in dm_spi_xfer() instead, matching Linux's
__spi_validate(): a din on a SPI_NO_RX device or a dout on a SPI_NO_TX
device fails with -EINVAL before it reaches the driver. A new sandbox
test covers both rejections and the accepted case.

This comes up on devices with no MISO line at all, such as a
write-only SPI display described with spi-rx-bus-width = <0>.

Signed-off-by: Cole Munz <[email protected]>
---
Changes in v2: added the dm_spi_xfer() validation and the sandbox
test. Rerun on this branch:

  $ ./u-boot -T -c "ut dm dm_test_spi_xfer_no_rx_tx"   failures: 0
  (spi_xfer, spi_flash, spi_find, spi_claim_bus, spi_set_wordlen
  also all failures: 0)

With the dm_spi_xfer() hunk reverted the new test fails both -EINVAL
assertions, so it does test the right thing.

checkpatch flags the #if CONFIG_IS_ENABLED(DM_SPI_FLASH) guard and the
DM_TEST-after-brace placement in the test; both copy the idiom the
existing tests in test/dm/spi.c use, so I kept them consistent.

 drivers/spi/spi-uclass.c | 16 ++++++++++++++++
 include/spi.h            |  2 ++
 test/dm/spi.c            | 41 ++++++++++++++++++++++++++++++++++++++++
 3 files changed, 59 insertions(+)

diff --git a/drivers/spi/spi-uclass.c b/drivers/spi/spi-uclass.c
index 120565df1497..dd1843ffac19 100644
--- a/drivers/spi/spi-uclass.c
+++ b/drivers/spi/spi-uclass.c
@@ -105,6 +105,7 @@ int dm_spi_set_wordlen(struct udevice *dev, unsigned int wordlen)
 int dm_spi_xfer(struct udevice *dev, unsigned int bitlen,
 		const void *dout, void *din, unsigned long flags)
 {
+	struct dm_spi_slave_plat *slave_plat = dev_get_parent_plat(dev);
 	struct udevice *bus = dev->parent;
 	struct dm_spi_ops *ops = spi_get_ops(bus);
 
@@ -113,6 +114,15 @@ int dm_spi_xfer(struct udevice *dev, unsigned int bitlen,
 	if (!ops->xfer)
 		return -ENOSYS;
 
+	/*
+	 * A device with no wire in one direction cannot transfer in it,
+	 * so reject the request here rather than in every driver.
+	 */
+	if (din && (slave_plat->mode & SPI_NO_RX))
+		return -EINVAL;
+	if (dout && (slave_plat->mode & SPI_NO_TX))
+		return -EINVAL;
+
 	return ops->xfer(dev, bitlen, dout, din, flags);
 }
 
@@ -229,6 +239,9 @@ static int spi_child_post_bind(struct udevice *dev)
 	/* Device DUAL/QUAD mode */
 	value = dev_read_u32_default(dev, "spi-tx-bus-width", 1);
 	switch (value) {
+	case 0:
+		mode |= SPI_NO_TX;
+		break;
 	case 1:
 		break;
 	case 2:
@@ -247,6 +260,9 @@ static int spi_child_post_bind(struct udevice *dev)
 
 	value = dev_read_u32_default(dev, "spi-rx-bus-width", 1);
 	switch (value) {
+	case 0:
+		mode |= SPI_NO_RX;
+		break;
 	case 1:
 		break;
 	case 2:
diff --git a/include/spi.h b/include/spi.h
index 97096a775262..f477763bdc60 100644
--- a/include/spi.h
+++ b/include/spi.h
@@ -34,6 +34,8 @@ struct spinand_info;
 #define SPI_RX_QUAD	BIT(13)			/* receive with 4 wires */
 #define SPI_TX_OCTAL	BIT(14)			/* transmit with 8 wires */
 #define SPI_RX_OCTAL	BIT(15)			/* receive with 8 wires */
+#define SPI_NO_TX	BIT(16)			/* no transmit wire */
+#define SPI_NO_RX	BIT(17)			/* no receive wire */
 
 /* Header byte that marks the start of the message */
 #define SPI_PREAMBLE_END_BYTE	0xec
diff --git a/test/dm/spi.c b/test/dm/spi.c
index a89ba06274fc..cb8f88713a9e 100644
--- a/test/dm/spi.c
+++ b/test/dm/spi.c
@@ -216,3 +216,44 @@ static int dm_test_spi_xfer(struct unit_test_state *uts)
 	return 0;
 }
 DM_TEST(dm_test_spi_xfer, UTF_SCAN_PDATA | UTF_SCAN_FDT);
+
+/* Test that a transfer is rejected when the device has no wire for it */
+static int dm_test_spi_xfer_no_rx_tx(struct unit_test_state *uts)
+{
+	struct dm_spi_slave_plat *plat;
+	struct spi_slave *slave;
+	struct udevice *bus;
+	const int busnum = 0, cs = 0;
+	const char dout[5] = {0x9f};
+	unsigned char din[5];
+	uint saved_mode;
+
+	ut_assertok(spi_get_bus_and_cs(busnum, cs, &bus, &slave));
+	ut_assertok(spi_claim_bus(slave));
+	plat = dev_get_parent_plat(slave->dev);
+	saved_mode = plat->mode;
+
+	plat->mode |= SPI_NO_RX;
+	ut_asserteq(-EINVAL, spi_xfer(slave, 40, dout, din,
+				      SPI_XFER_BEGIN | SPI_XFER_END));
+
+	plat->mode = saved_mode | SPI_NO_TX;
+	ut_asserteq(-EINVAL, spi_xfer(slave, 40, dout, din,
+				      SPI_XFER_BEGIN | SPI_XFER_END));
+
+	plat->mode = saved_mode;
+	ut_assertok(spi_xfer(slave, 40, dout, din,
+			     SPI_XFER_BEGIN | SPI_XFER_END));
+	spi_release_bus(slave);
+
+	/*
+	 * Since we are about to destroy all devices, we must tell sandbox
+	 * to forget the emulation device
+	 */
+#if CONFIG_IS_ENABLED(DM_SPI_FLASH)
+	sandbox_sf_unbind_emul(state_get_current(), busnum, cs);
+#endif
+
+	return 0;
+}
+DM_TEST(dm_test_spi_xfer_no_rx_tx, UTF_SCAN_PDATA | UTF_SCAN_FDT);
-- 
2.55.0