Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

STM32 SPI Driver - Transmit (MOSI) Only - Infinite Loop on Tranceive #33169

Closed
heinwessels opened this issue Mar 9, 2021 · 10 comments
Closed
Assignees
Labels
area: SPI SPI bus bug The issue is a bug, or the PR is fixing a bug platform: STM32 ST Micro STM32 priority: low Low impact/importance bug
Milestone

Comments

@heinwessels
Copy link
Contributor

heinwessels commented Mar 9, 2021

I am trying to run the WS2812 LED Strip driver on an STM32L431 using the SPI mode. The SPI is used as Transmit Only, and only the MOSI pin is connected and defined in the DTS. When the LED driver is writing to the LED's the SPI driver gets stuck in an infinite loop waiting for the RXNE flag to be raised here.

To Reproduce
Steps to reproduce the behavior:

  1. Compile software for an STM32L431 using the mentioned WS2812 SPI example.
  2. Only supply a MOSI pin in the DTS.
  3. Add this line LL_SPI_SetRxFIFOThreshold(spi, LL_SPI_RX_FIFO_TH_QUARTER);
  4. Flash software.
  5. No LEDS flashing.

Or,

  1. Compile software for an STM32L431 using the mentioned WS2812 SPI example.
    1. Only supply a MOSI pin in the DTS.
  2. Add this line LL_SPI_SetRxFIFOThreshold(spi, LL_SPI_RX_FIFO_TH_QUARTER);
  3. Flash software.
  4. west debug
  5. Observe software getting stuck waiting for the RXNE flag here.

Expected behavior
I expect that if I use the SPI in transmit-only mode that it will not care about what happened on the MISO line (or lack thereof).
Possibly, instead of waiting for the RXNE flag when no rx buffer is supplied it could wait for !TXE, like this:

if (data->ctx.rx_buf){
    // If a RX buffer was supplied it means the user is expecting to recieve something
    while (!ll_func_rx_is_not_empty(spi)) {
        /* NOP */
    }
}
else{
    // There is no RX buffer supplied (it's NULL) which means the user isn't expecting anything back
    while (!ll_func_tx_is_empty(spi)) {
        // Poll the TXE to see when the transmit buffer is empty to signal that the transmit is complete
        /* NOP */
    }
}

I tested this example and it works in this use case. It's not very invasive and shouldn't interfere in normal use cases either.

Another way around this would be to allow the STM32 SPI driver to have a simple SPI_write function that would only check for the !TXE.

It also works if I add the clock pin to the DTS, but this pin will be used for the debugger which means this is not a viable solution since it breaks the debugging connection.

Impact
Currently, an ugly hack is required to circumvent this problem which cannot remain in the final product.

Environment (please complete the following information):

  • Zephyr commit develeping on: 89a9096
  • STM32L431

Additional context
This is how the SPI is setup in the device tree:

led_spi: &spi3 {
    compatible = "st,stm32-spi";
    status = "okay";
    pinctrl-0 = <&spi3_mosi_pb5>;
    led_strip: ws2812@0 {
        compatible = "worldsemi,ws2812-spi";
        label = "WS2812";
        reg = < 0x0 >;
        chain-length = < 0x40 >;
        spi-max-frequency = < SPI_FREQ >;
        spi-one-frame = < ONE_FRAME >;
        spi-zero-frame = < ZERO_FRAME >;
    };
};
@heinwessels heinwessels added the bug The issue is a bug, or the PR is fixing a bug label Mar 9, 2021
@erwango erwango self-assigned this Mar 9, 2021
@erwango erwango added area: SPI SPI bus platform: STM32 ST Micro STM32 labels Mar 9, 2021
@nashif nashif added the priority: low Low impact/importance bug label Mar 9, 2021
@erwango erwango assigned FRASTM and unassigned erwango Mar 31, 2021
@FRASTM
Copy link
Collaborator

FRASTM commented Apr 21, 2021

@heinwessels in item 3 of the bug description : where do you intend to "Add this line LL_SPI_SetRxFIFOThreshold(spi, LL_SPI_RX_FIFO_TH_QUARTER);"

@heinwessels
Copy link
Contributor Author

@FRASTM for debugging purposes I think simply added it to static void spi_stm32_shift_m(SPI_TypeDef *spi, struct spi_stm32_data *data).

@FRASTM
Copy link
Collaborator

FRASTM commented Apr 21, 2021

This use case is like a SPI simplex mode:
"The SPI can communicate in simplex mode by setting the SPI in transmit-only or in receiveonly using the RXONLY bit in the SPIx_CR2 register. In this configuration, only one line is
used for the transfer between the shift registers of the master and slave. The remaining
MISO and MOSI pins pair is not used for communication and can be used as standard
GPIOs.
• Transmit-only mode (RXONLY=0): The configuration settings are the same as for fullduplex. The application has to ignore the information captured on the unused input pin.
This pin can be used as a standard GPIO.
"

Actually, the drivers/spi/spi_ll_stm32.c is setting the full duplex by default. A possible solution could be to include the simplex mode there.

@affrinpinhero-2356
Copy link
Contributor

I have tested SPI in transmit only. And I have not observed any issue on as mentioned. For the reference an commenting the test codes and my changes and target below.
Note: my sample code transmit a string continuously.

Target Used: NUCLEO_F767ZI

Other file Modification I made as per the recommendation:
GPIO:

 			gpios = <&gpiob 14 GPIO_ACTIVE_HIGH>;
 			label = "User LD3";
 		};
+		gpio0: gpio_1 {
+			label = "User GPIO";
+			gpios = <&gpioa 6 GPIO_ACTIVE_HIGH>;
+		};
+		gpio1: gpio_2 {
+			label = "User GPIO";
+			gpios = <&gpiod 14 GPIO_ACTIVE_HIGH>;
+		};
 	};
 
 	gpio_keys {
@@ -58,6 +66,8 @@
 		led0 = &green_led;
 		led1 = &blue_led;
 		led2 = &red_led;
+		gpiopin  = &gpio0;
+		gpiopin1 = &gpio1;
 		sw0 = &user_button;
 	};
 };

SPI:

};
 
 &spi1 {
-	pinctrl-0 = <&spi1_nss_pa4 &spi1_sck_pa5
-		     &spi1_miso_pa6 &spi1_mosi_pa7>;
+
+	pinctrl-0 = <&spi1_nss_pa4 &spi1_sck_pa5 
+				&spi1_mosi_pa7>;

Driver Code( spi_ll_stm32.c ):

/* The update is ignored if TX is off. */
 		spi_context_update_tx(&data->ctx, 2, 1);
 	}
-
-	while (!ll_func_rx_is_not_empty(spi)) {
-		/* NOP */
+	if(data->ctx.rx_buf) {
+		while (!ll_func_rx_is_not_empty(spi)) {
+			/* NOP */
+		}
+	} else {
+		while (!ll_func_tx_is_empty(spi)) {
+			/* NOP */
+		}
 	}
 
 	if (SPI_WORD_SIZE_GET(data->ctx.config->operation) == 8) {

Test app: (Code snip)

static int stm32_spi_send(struct device *spi,
			  const struct spi_config *spi_cfg,
			  const uint8_t *data, size_t len)
{
	const struct spi_buf_set tx = {
		.buffers = &(const struct spi_buf){
			.buf = (uint8_t *)data,
			.len = len,
		},
		.count = 1,
	};

	return spi_write(spi, spi_cfg, &tx);
}

static int stm32_spi_send_str(struct device *spi,
			      const struct spi_config *spi_cfg,
			      const unsigned char *str)
{
	return stm32_spi_send(spi, spi_cfg, str, strlen(str));
}

static void stm32_spi_setup_nss_pin(void)
{
	static const struct pin_config pin_config = {
		.pin_num = STM32_PIN_PA4,
		.mode = STM32_PINMUX_ALT_FUNC_5 | STM32_PUSHPULL_PULLUP,
	};

	stm32_setup_pins(&pin_config, 1);
}

void main(void)
{
	const struct device *dev;
	bool led_is_on = true;
	int ret;

	/*SPI Config*/
	struct spi_config spi_cfg = {};
	struct device *spi;
	int err;

	dev = device_get_binding(LED0);
	if (dev == NULL) {
		return;
	}

	ret = gpio_pin_configure(dev, PIN, GPIO_OUTPUT_ACTIVE | FLAGS);
	if (ret < 0) {
		return;
	}

	printk("Starting stm32-spi on %s\n", CONFIG_BOARD);

	stm32_spi_setup_nss_pin();

	spi = device_get_binding("SPI_1");
	if (!spi) {
		printk("Could not find SPI driver\n");
		return;
	}

	spi_cfg.operation = SPI_WORD_SET(8) | SPI_TRANSFER_MSB |
			    SPI_OP_MODE_MASTER;

	spi_cfg.frequency = 1500000U;//1000000U;


	while (1) {
		gpio_pin_set(dev, PIN, (int)led_is_on);
		led_is_on = !led_is_on;

		err = stm32_spi_send_str(spi, &spi_cfg, "HELLO!");
		if (err)
			break;

	}

	if (err)
	printk("SPI send error %d\n", err);
}

@FRASTM
Copy link
Collaborator

FRASTM commented Apr 27, 2021

It confirms that without any valid Rx buffer, there is no need to loop on the RXNE : Tx-only mode.

@galak
Copy link
Collaborator

galak commented May 11, 2021

Fixed by #34731

@galak galak closed this as completed May 11, 2021
@ABOSTM ABOSTM reopened this May 26, 2021
@ABOSTM
Copy link
Collaborator

ABOSTM commented May 26, 2021

#34731 causes important regressions, so it is being reverted: #35661
So I reopen this issue.

@ABOSTM
Copy link
Collaborator

ABOSTM commented May 26, 2021

@heinwessels,
I tested on my side and I found no issue (Leds are blinking correctly).
So from my point of view there is no issue.

Here is my setup:
#############
Board: nucleo_l476rg
Led strip: WB2812B
SHA1: 374629a + revert 0129884
sample: samples/drivers/led_ws2812

I add nucleo_l476 overlay and conf file to the sample (see attached zip)
samples_led_ws2812.zip

Note : I did not add line LL_SPI_SetRxFIFOThreshold(spi, LL_SPI_RX_FIFO_TH_QUARTER);

It works even if I restrict dts spi to keep only sck and mosi.

&spi1 {
	pinctrl-0 = < &spi1_sck_pb3
		      &spi1_mosi_pa7>;
	status = "okay";
};

Maybe your problem comes from the fact you remove SCK from DTS, is it what you did ?
SPI cannot work without SCK that is part of SPI protocol (and hardware).
It is some kind of trick to use SPI hardware to drive WS2812 which only require a single line.

@heinwessels
Copy link
Contributor Author

heinwessels commented May 26, 2021

@ABOSTM thanks for the testing on your side.

Yes, I did remove the SCK from the DTS, as stated in my original description. The clock line was connected to the SWO, and caused issues while debugging/programming. And as you said, it isn't used by the WS2812 driver. I understand that Zephyr might not be built to accomodate this, but I thought it was reasonable to assume it should.

What I did notice is that that it works fine with the DMA. I had the infinite loop issue when I wasn't using the DMA. It's then when I had to add the LL_SPI_SetRxFIFOThreshold(spi, LL_SPI_RX_FIFO_TH_QUARTER);.

Later I moved to using the DMA for SPI and this solved my problems. I could remove the LL_SPI_SetRxFIFOThreshold(spi, LL_SPI_RX_FIFO_TH_QUARTER); and the DTS is fine without the SCK defined. This is how my setup is currently running, and hasn't shown any SPI issues over the past few months. So, for me at least, this is no longer an issue.

@ABOSTM
Copy link
Collaborator

ABOSTM commented May 26, 2021

Thanks for your feedback.

I understand that Zephyr might not be built to accomodate this

It is not a problem with Zephyr but rather a problem of hardware : SPI is not supposed to work with only MOSI signal.
So Hardware implementation doesn't support it as it is out of specification.

@ABOSTM ABOSTM closed this as completed May 26, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: SPI SPI bus bug The issue is a bug, or the PR is fixing a bug platform: STM32 ST Micro STM32 priority: low Low impact/importance bug
Projects
None yet
Development

No branches or pull requests

7 participants