SPI transceive function only transmitting first tx_buffer on Sifive's MCU

[0x414D47]

Describe the bug
The spi_transceive function only transmits/receives the first buffer in spi_buf_set tx.

I used the following code for creating and transmitting two SPI buffers using the SPI drivers for SiFive's MCUs (on a HiFive1 Rev. B board). This code should transmits two tx_buffers, each with two bytes of data, i.e. 4 bytes in total.

#include <zephyr.h>
#include <device.h>
#include <devicetree.h>
#include <drivers/gpio.h>
#include <sys/printk.h>
#include <drivers/spi.h>

/* 1000 msec = 1 sec */
#define SLEEP_TIME_MS   1000

/* The devicetree node identifier for the "led0" alias. */
#define LED0_NODE DT_ALIAS(led0)

#if DT_NODE_HAS_STATUS(LED0_NODE, okay)
#define LED0	DT_GPIO_LABEL(LED0_NODE, gpios)
#define PIN	DT_GPIO_PIN(LED0_NODE, gpios)
#define FLAGS	DT_GPIO_FLAGS(LED0_NODE, gpios)
#endif

#define MY_SPI DT_PATH(soc, spi_10024000)

const struct device *dev_spi;
const struct device *dev_led;
int counter = 0;

static const struct spi_config spi_cfg = {
	.operation = SPI_WORD_SET(8) | SPI_TRANSFER_MSB,
	.frequency = 100000,
	.slave = 0,
};

static void led_init(void)
{
	int ret;

	dev_led = device_get_binding(LED0);
	if (dev_led == NULL) {
		printk("Could not get LED device\n");
		while(1);
	}

	ret = gpio_pin_configure(dev_led, PIN, GPIO_OUTPUT_ACTIVE | FLAGS);
	if (ret < 0) {
		while(1);
	}
}

static void spi_init(void)
{
	dev_spi = device_get_binding(DT_LABEL(MY_SPI));
	if (dev_spi == NULL) {
		printk("Could not get SPI device\n");
		while(1);
	}
}

void spi_test_send(void)
{
	int err;
	static uint8_t tx_buffer[2];
	static uint8_t tx_buffer_2[2];

	static uint8_t rx_buffer[10] = {0};
	static uint8_t rx_buffer_2[20] = {0};

	const struct spi_buf tx_buf[2] = {
		{
			.buf = tx_buffer,
			.len = sizeof(tx_buffer),
		},
		{
			.buf = tx_buffer_2,
			.len = sizeof(tx_buffer_2)
		}
	};
	const struct spi_buf_set tx = {
		.buffers = &tx_buf,
		.count = 2
	};

	struct spi_buf rx_buf[] = {
		{
			.buf = rx_buffer,
			.len = sizeof(rx_buffer),
		},
		{
			.buf = rx_buffer_2,
			.len = sizeof(rx_buffer_2)
		}
	};
	const struct spi_buf_set rx = {
		.buffers = &rx_buf,
		.count = 2
	};

	tx_buffer[0] = counter;
	tx_buffer[1] = counter+1;
	tx_buffer_2[0] = counter+2;
	tx_buffer_2[1] = counter+3;

	printk("RX: %X %X %X %X \n",rx_buffer[0], rx_buffer[1], rx_buffer[2], rx_buffer[3]);
	printk("RX2: %X %X %X %X \n",rx_buffer_2[0], rx_buffer_2[1], rx_buffer_2[2], rx_buffer_2[3]);

	printk("Transfer...\n");

	err = spi_transceive(dev_spi, &spi_cfg, &tx, &rx);

	if (err) {
		printk("SPI error: %d\n", err);
	} else {
		printk("RX: %X %X %X %X \n",rx_buffer[0], rx_buffer[1], rx_buffer[2], rx_buffer[3]);
		printk("RX2: %X %X %X %X \n\n",rx_buffer_2[0], rx_buffer_2[1], rx_buffer_2[2], rx_buffer_2[3]);
	}	
}

void main(void)
{
	led_init();	
	spi_init();
	printk("Zephyr SPI Driver Test\n");

	while (1) {
		gpio_pin_set(dev_led, PIN, (int)true); // LED pin used as Chip Select
		spi_test_send();
		gpio_pin_set(dev_led, PIN, (int)false);

		k_msleep(SLEEP_TIME_MS);

		printk("%ds\n", counter++);
	}
}

The used project configuration is as follows:

CONFIG_GPIO=y
CONFIG_SPI=y
CONFIG_SPI_SIFIVE=y
CONFIG_MAIN_STACK_SIZE=4096

To Reproduce
Steps to reproduce the behavior:

1. Connect MOSI and MISO pins (Pins with labels 11 and 12 on the board).
2. Execute west build -p -b hifive1_revb && west flash
3. Open terminal (115200 8-N-1).
4. See the received values RX and RX2 (for the first and second rx_buffer, respectively) before and after the transmission.

Expected behavior
The receive buffers (RX and/or RX2) should contain the 4 bytes sent.

Impact
Some libraries and drivers transmit several buffers over SPI (e.g. LoRa driver). This issue makes it impossible to use such libraries and drivers.

Logs and console output
The first two transfers:

[10:20:03:319] 0s
[10:20:03:326] RX: 0 1 0 0 
[10:20:03:326] RX2: 0 0 0 0 
[10:20:03:326] Transfer...
[10:20:03:335] RX: 1 2 0 0 
[10:20:03:338] RX2: 0 0 0 0 
[10:20:03:345] 
[10:20:04:331] 1s
[10:20:04:382] RX: 1 2 0 0 
[10:20:04:397] RX2: 0 0 0 0 
[10:20:04:397] Transfer...
[10:20:04:397] RX: 2 3 0 0 
[10:20:04:397] RX2: 0 0 0 0 

Note that the second rx_buffer is completely empty and that only the first tx_buffer was transmitted (two bytes of data).

Here is an image of what is being transmitted:

Only the first two bytes (corresponding to the first tx_buffer) are being transmitted. There is a total of 10 bytes being read by the function because of the size of the first rx_buffer being 10 bytes.

Environment (please complete the following information):

• OS: Ubuntu 20.04
• Toolchain: zephyr-sdk-0.12.2
• Zephyr version (displayed while building with west): 2.4.99

Possible root of the issue
Looking at the function spi_sifive_transceive (in the file drivers/spi_sifive.c), I realised that the function spi_context_buffers_setup (in the file drivers/spi_context.h) is being called for preparing the transmission by "loading" the first tx_buffer into the structure 'ctx', but the function spi_context_update_tx is not called at all. This last function updates the tx_buffer to be sent.

Additional context
I became aware of this issue while trying to use the LoRa drivers, which use two buffers for sending commands and parameters to the transceiver module. I submitted it here: #34246.

[0x414D47]

Update:
In order to confirm the root of the issue, I modified the function spi_sifive_transceive (in a very simplistic way) for updating the context of transmission and reception. For that, I replaced the line spi_sifive_xfer(dev, hw_cs_control); with this piece of code:

/* Perform transfer */
for (uint8_t tx_buf_id = 0; tx_buf_id < tx_bufs->count; tx_buf_id++){
  spi_sifive_xfer(dev, hw_cs_control);
  spi_context_update_tx(&SPI_DATA(dev)->ctx, 1, tx_bufs->buffers[tx_buf_id].len);
  spi_context_update_rx(&SPI_DATA(dev)->ctx, 1, tx_bufs->buffers[tx_buf_id].len);
}

With such a modification, the amount of buffers is taken into consideration and the first rx_buffer is going to receive the data of all tx_buffers.

The console output then shows a correct behavior:
[12:57:07:851] 0s
[12:57:07:862] RX: 0 1 2 3
[12:57:07:862] RX2: 0 0 0 0
[12:57:07:862] Transfer...
[12:57:07:873] RX: 1 2 3 4
[12:57:07:882] RX2: 0 0 0 0
[12:57:07:882]
[12:57:08:864] 1s
[12:57:08:970] RX: 1 2 3 4
[12:57:08:970] RX2: 0 0 0 0
[12:57:08:970] Transfer...
[12:57:08:980] RX: 2 3 4 5
[12:57:08:980] RX2: 0 0 0 0

[katsuster]

Sorry for late. I have finished to reproduce this problem on my board...
And I'll try your suggestion and create patch & PR. Thanks!

[katsuster]

@armandomiguel
I created patch that send/recv all data in spi_sifive_xfer() because if first xfer() call is finished, CS is accidentally negated.
I guessed it's not your intention. If I got some mistakes, please tell me.