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
