[PATCH] hw/ssi/stm32f2xx_spi: avoid transferring data on DR reads

Manuele Conti posted 1 patch 1 day, 12 hours ago
MAINTAINERS                      |  1 +
hw/ssi/stm32f2xx_spi.c           |  1 -
tests/qtest/meson.build          |  2 ++
tests/qtest/stm32f2xx-spi-test.c | 50 ++++++++++++++++++++++++++++++++
4 files changed, 53 insertions(+), 1 deletion(-)
create mode 100644 tests/qtest/stm32f2xx-spi-test.c
[PATCH] hw/ssi/stm32f2xx_spi: avoid transferring data on DR reads
Posted by Manuele Conti 1 day, 12 hours ago
Writing SPI_DR already performs the synchronous SSI transfer and stores
its response in spi_dr. Reading SPI_DR performs another transfer, using
the previous response as transmit data, and returns the new response.
Consequently a normal write/read pair clocks two frames on the bus and
can skip bytes in a peripheral's response.

Return the response saved by the write and clear RXNE without starting
another transfer. Repeated reads then leave the peripheral state alone.

Add a qtest using the upstream netduino2 machine and an M25P80 flash.
Check the JEDEC ID, RXNE clearing, and repeated DR reads. The test requires
no board changes and reproduces the unwanted extra transfers before the
fix.

Fixes: 5ae74402d1f ("STM32F2xx: Add the SPI device")
Signed-off-by: Manuele Conti <conti.manuele@gmail.com>
---
 MAINTAINERS                      |  1 +
 hw/ssi/stm32f2xx_spi.c           |  1 -
 tests/qtest/meson.build          |  2 ++
 tests/qtest/stm32f2xx-spi-test.c | 50 ++++++++++++++++++++++++++++++++
 4 files changed, 53 insertions(+), 1 deletion(-)
 create mode 100644 tests/qtest/stm32f2xx-spi-test.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 902db77..822c60a 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1201,6 +1201,7 @@ F: hw/char/stm32f2xx_usart.c
 F: hw/timer/stm32f2xx_timer.c
 F: hw/adc/*
 F: hw/ssi/stm32f2xx_spi.c
+F: tests/qtest/stm32f2xx-spi-test.c
 F: include/hw/*/stm32*.h
 
 STM32F405
diff --git a/hw/ssi/stm32f2xx_spi.c b/hw/ssi/stm32f2xx_spi.c
index 871d573..da1dbb7 100644
--- a/hw/ssi/stm32f2xx_spi.c
+++ b/hw/ssi/stm32f2xx_spi.c
@@ -82,7 +82,6 @@ static uint64_t stm32f2xx_spi_read(void *opaque, hwaddr addr,
     case STM_SPI_SR:
         return s->spi_sr;
     case STM_SPI_DR:
-        stm32f2xx_spi_transfer(s);
         s->spi_sr &= ~STM_SPI_SR_RXNE;
         return s->spi_dr;
     case STM_SPI_CRCPR:
diff --git a/tests/qtest/meson.build b/tests/qtest/meson.build
index 78b3010..5984e9c 100644
--- a/tests/qtest/meson.build
+++ b/tests/qtest/meson.build
@@ -238,6 +238,8 @@ qtests_stm32l4x5 = \
    'stm32l4x5_usart-test']
 
 qtests_arm = \
+  (config_all_devices.has_key('CONFIG_NETDUINO2') and
+   config_all_devices.has_key('CONFIG_SSI_M25P80') ? ['stm32f2xx-spi-test'] : []) + \
   (config_all_devices.has_key('CONFIG_MPS2') ? ['sse-timer-test'] : []) + \
   (config_all_devices.has_key('CONFIG_CMSDK_APB_DUALTIMER') ? ['cmsdk-apb-dualtimer-test'] : []) + \
   (config_all_devices.has_key('CONFIG_CMSDK_APB_TIMER') ? ['cmsdk-apb-timer-test'] : []) + \
diff --git a/tests/qtest/stm32f2xx-spi-test.c b/tests/qtest/stm32f2xx-spi-test.c
new file mode 100644
index 0000000..619191c
--- /dev/null
+++ b/tests/qtest/stm32f2xx-spi-test.c
@@ -0,0 +1,50 @@
+/*
+ * STM32F2XX SPI controller tests
+ *
+ * Copyright (c) 2026 Manuele Conti <conti.manuele@gmail.com>
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#include "qemu/osdep.h"
+#include "libqtest.h"
+
+/* The first SSI bus found by -device belongs to SPI3. */
+#define SPI3_BASE 0x40003c00
+#define SPI_CR1   (SPI3_BASE + 0x00)
+#define SPI_SR    (SPI3_BASE + 0x08)
+#define SPI_DR    (SPI3_BASE + 0x0c)
+#define SPI_RXNE  (1U << 0)
+
+static void test_jedec_id(void)
+{
+    static const uint8_t jedec_id[] = { 0x20, 0x20, 0x14 };
+    QTestState *qts = qtest_init("-machine netduino2 "
+                               "-device m25p80,bus=ssi");
+
+    /* Enable SPI in master mode, with software slave management. */
+    qtest_writel(qts, SPI_CR1, (1U << 9) | (1U << 8) |
+                             (1U << 6) | (1U << 2));
+
+    qtest_writeb(qts, SPI_DR, 0x9f);
+    qtest_readb(qts, SPI_DR);
+
+    for (unsigned i = 0; i < ARRAY_SIZE(jedec_id); i++) {
+        qtest_writeb(qts, SPI_DR, 0);
+        g_assert_cmphex(qtest_readl(qts, SPI_SR) & SPI_RXNE, ==, SPI_RXNE);
+        g_assert_cmphex(qtest_readb(qts, SPI_DR), ==, jedec_id[i]);
+        g_assert_cmphex(qtest_readl(qts, SPI_SR) & SPI_RXNE, ==, 0);
+
+        /* Reading DR again must not clock another byte out of the flash. */
+        g_assert_cmphex(qtest_readb(qts, SPI_DR), ==, jedec_id[i]);
+        g_assert_cmphex(qtest_readl(qts, SPI_SR) & SPI_RXNE, ==, 0);
+    }
+
+    qtest_quit(qts);
+}
+
+int main(int argc, char **argv)
+{
+    g_test_init(&argc, &argv, NULL);
+    qtest_add_func("/stm32f2xx/spi/jedec-id", test_jedec_id);
+    return g_test_run();
+}
-- 
2.54.0 (Apple Git-157)