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

Cole Munz <[email protected]>
Newsgroups org.u-boot-project.lists.u-boot
Message-ID <ae6a85d6f985b63c8f5a3fe8deeab89d79757b14.1787245113.git.Munzzyy1@proton.me>
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.

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]>
Acked-by: Quentin Schulz <[email protected]>
---
Changes in v3: dropped the happy path assertion from the test per
Quentin, collected the Acked-by. Rerun after the trim:

  $ ./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)

 drivers/spi/spi-uclass.c | 16 ++++++++++++++++
 include/spi.h            |  2 ++
 test/dm/spi.c            | 39 +++++++++++++++++++++++++++++++++++++++
 3 files changed, 57 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..a0da12ab6bc0 100644
--- a/test/dm/spi.c
+++ b/test/dm/spi.c
@@ -216,3 +216,42 @@ 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;
+	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
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.