SPI: ESP32: chip-select misbehaviour with multiple gpio-cs

[MrMarteng]

Describe the bug
There seems to be a race-condition that causes gpio-cs to be not released properly before the next transfer begins. The cs-line stays active and communication to the device interferes. I identified this on esp32c3.

To Reproduce
I first noticed it when using 2 cs-gpios for different devices drivers connected to the same SPI bus.
The application is based on esp32c3_devkitm:

&spi2 {
	#address-cells = <1>;
	#size-cells = <0>;
	status = "okay";
	dma-enabled;
	pinctrl-0 = <&spim2_default>;
	pinctrl-names = "default";
	cs-gpios = <&gpio0 10 GPIO_ACTIVE_LOW>, 	// ST7789V CS
			<&gpio0 1 GPIO_ACTIVE_LOW>;	// trf7977 CS

	st7789v: st7789v@0 {
		compatible = "sitronix,st7789v";
		reg = <0>;
		status = "okay";
		/* ... */
	};

        trf79xx_nfc: ti_trf7970@1 {
		compatible = "ti,trf79xx";
		reg = <1>;
		status = "okay";
                /* ... */
	};

After identifying this issue I wrote a small sample program to manually evaluate the behavior.
In this sample program 2 threads are running and continuously sending data on the SPI bus.
Thread1 sends the 1024 bytes with the value 0xFF every 250 ms. It uses gpio0-1 for chip-select:

config.cs.gpio.port = DEVICE_DT_GET(DT_NODELABEL(gpio0));
config.cs.gpio.pin = 1;
config.cs.gpio.dt_flags = GPIO_ACTIVE_LOW;
config.frequency = 100000;
config.operation = SPI_OP_MODE_MASTER | SPI_TRANSFER_MSB | SPI_WORD_SET(8) | SPI_MODE_CPHA | SPI_MODE_CPOL;
gpio_pin_configure(config.cs.gpio.port, config.cs.gpio.pin, GPIO_OUTPUT_INACTIVE | GPIO_ACTIVE_LOW);

memset(data, 0xFFu, sizeof(data));
spi_buf.buf = data;
spi_buf.len = sizeof(data);
spi_bufset.buffers = &spi_buf;
spi_bufset.count = 1;

while (true)
{
    spi_write(pSpi, &config, &spi_bufset);

    k_sleep(K_MSEC(250));
}

Thread1 sends the 512 bytes with the value 0x00 every 1 s. It uses gpio0-10 for chip-select:

memset(data, 0x00u, sizeof(data));
config.cs.gpio.port = DEVICE_DT_GET(DT_NODELABEL(gpio0));
config.cs.gpio.pin = 10;
config.cs.gpio.dt_flags = GPIO_ACTIVE_LOW;
config.frequency = 1000;
config.operation =
    SPI_OP_MODE_MASTER | SPI_TRANSFER_MSB | SPI_WORD_SET(8) | SPI_MODE_CPHA | /*SPI_LOCK_ON | SPI_HOLD_ON_CS | */SPI_MODE_CPOL;
gpio_pin_configure(config.cs.gpio.port, config.cs.gpio.pin, GPIO_OUTPUT_INACTIVE | GPIO_ACTIVE_LOW);

while (true)
{
    spi_buf.buf = data;
    spi_buf.len = sizeof(data);
    spi_bufset.buffers = &spi_buf;
    spi_bufset.count = 1;

    spi_write(pSpi, &config, &spi_bufset);

    k_sleep(K_MSEC(1000));
}

I took a scope with my logic analyzer:

The line D4/IRQ pin is low within the transceive() function in spi_esp32_spim.c.
gpio0-1 is signal "CS"
gpio0-10 is signal "CS2"

Expected behavior
The signal "CS" should only be active (low) when transfer from thread 1 is active. The signal "CS2" should only be active (low) when transfer from thread 2 is active. But as shown in the trace above, they are interfering.

Impact
The impact depends on the devices are affected. It could vary between data corruption and complete denial of service.
In my example the trf7977 spi frontend hangs completely after a while as it couldn't handle random data from the other driver. The chip is no longer responding to any transfer.

Environment (please complete the following information):

• OS: macOS (Sonoma 14.1.1)
• Zephyr SDK 0.16.3
• Zephyr running on latest manifest-rev (90b9809)

[MrMarteng]

I further digged into this issue and it looks like that something within the SPI HAL layer is overwriting the pin configuration. Zephyr SPI layer is working as intended. The CS gpio is triggered accordingly. Further investigation within esp HAL is required.

[ndrs-pst]

@MrMarteng What happen if you enable SPI_LOCK_ON in config.operation of both thread ?
Since spi_context_lock() and spi_context_release() appear to be used for handling race conditions in transceive() function in spi_esp32_spim.c

[MrMarteng]

@MrMarteng What happen if you enable SPI_LOCK_ON in config.operation of both thread ? Since spi_context_lock() and spi_context_release() appear to be used for handling race conditions in transceive() function in spi_esp32_spim.c

Thanks for your reply. I actually was using the SPI_LOCK_ON feature when noticing this issue the first time. That also was the reason for adding the D4 toggle signal within the transceiver() function. The context lock is working properly. If this would be an issue I would expect some glitches in the D4 debug signal (and honestly I also would expect that such an issue would have also been notice by others).
The spi common layer and the gpio driver is working properly. And as mentioned in my last comment I suppose the issue is related in ESP HAL layer, that maybe triggers asynchronous actions when receiving an interrupt.

[ndrs-pst]

@MrMarteng Let's dig down to the following in esp32c3_devkitm-pinctrl.dtsi, may be you need to remove SPIM2_CSEL_GPIO10 from pinmux ?, since cs was control via gpio.

	spim2_default: spim2_default {
		group1 {
			pinmux = <SPIM2_MISO_GPIO2>,
				 <SPIM2_SCLK_GPIO6>,
				 <SPIM2_CSEL_GPIO10>;
		};
		group2 {
			pinmux = <SPIM2_MOSI_GPIO7>;
			output-low;
		};
	};

[MrMarteng]

You are absolutly right. Sorry for not mentioning this. I did adjust the native CS-functionality in the pinctrl file as mentioned.
If that is helpful I could provide a complete workspace. BTW: I also disabled I2C0 to avoid reconfiguration of GPIO1. But in my opinion the issue can be easily reproduced, by two concurrent SPI queries on the same bus. They are working properly when executed independently:

Thread1 (250ms):

Thread2 (1000ms):

[ndrs-pst]

@MrMarteng Absolutely, a complete workspace would be incredibly helpful for a thorough examination. Since I only have the ESP32-S3-DevKitM at hand, I can attempt to replicate the conditions and see if the same problem arises.

[MrMarteng]

@ndrs-pst @sylvioalves I created a small app-workspace to reproduce the behaviour: https://github.com/MrMarteng/zephyr_esp32-spics_bug

The sample is manually configuring the GPIO and passing the Chip-Select to the SPI transfer, but the behaviour is identical to defining CS via devicetree.

I am meanwhile quite sure, that this is a bug with ESP32 HAL layer - unless I am doing some terribly wrong. It would be nice to get some feedback on this. I tried to dive deeper into the HAL layer, but it's very encapsulated.

[rftafas]

@MrMarteng

I read you are using 512 bytes transactions. Are you using DMA?

[MrMarteng]

In my initial project, where I identified this behaviour, DMA was enabled. In the example project it looks like DMA was not enabled.