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

SPI broken on stm32f412 on master #21679

Closed
StefJar opened this issue Jan 3, 2020 · 3 comments · Fixed by #21722
Closed

SPI broken on stm32f412 on master #21679

StefJar opened this issue Jan 3, 2020 · 3 comments · Fixed by #21722
Labels
bug The issue is a bug, or the PR is fixing a bug

Comments

@StefJar
Copy link

StefJar commented Jan 3, 2020

I pulled the latest master and the SPI stopped working.

My config:

CONFIG_SPI=y
CONFIG_SPI_1=y
CONFIG_SPI_1_OP_MODES=1
CONFIG_SPI_3=y
CONFIG_SPI_3_OP_MODES=1
CONFIG_SPI_STM32_INTERRUPT=y

but it does not matter if we are using interrupts or not.

the code:

static sst26vf016b_error_t sst26vf016b_wakeup(void) {
	LOG_DBG("wakeup");
	u8_t buffer_tx[] = {sst26vf016b_op_RDPD};
	u8_t buffer_rx[1];
	struct spi_buf tx_buf [] = {
		{
		.buf = buffer_tx,
		.len = sizeof(buffer_tx)/sizeof(u8_t)
		}
	};
	const struct spi_buf_set tx = {
		.buffers = tx_buf,
		.count = sizeof(tx_buf)/sizeof(struct spi_buf)
	};
	const struct spi_buf rx_buf []= {
		{
		.buf = NULL,
		.len = 4,
		},
		{
		.buf = buffer_rx,
		.len = sizeof(buffer_rx),
		}
	};
	const struct spi_buf_set rx = {
		.buffers = rx_buf,
		.count = sizeof(rx_buf)/sizeof(struct spi_buf)
	};

	if (spi_transceive(sst26vf016b.spiDev, &sst26vf016b.spi_conf, &tx, &rx)) {
		return sst26vf016b_error_SPI;
	}

	if (SST26VF016B_JEDEC_DeviceID != buffer_rx[0]) {
		LOG_ERR("wrong device id! current DeviceID=0x%X vs DeviceID=0x%X",
				buffer_rx[0],
				SST26VF016B_JEDEC_DeviceID
			);
		return sst26vf016b_error_wakeup_wrongid;
	}
	k_sleep(K_MSEC(10)); // 10 msec till the device has left deep sleep
	return sst26vf016b_error_none;
}

My log print:

[00:00:01.421,000] <dbg> spi_ll_stm32.spi_stm32_configure: Installed config 0x20018ba4: freq 12000000Hz (div = 2), mode 0/0/0, slave 0
[00:00:01.434,000] <dbg> spi_ll_stm32.spi_context_buffers_setup: tx_bufs 0x200179a8 - rx_bufs 0x20017990 - 1
[00:00:01.444,000] <dbg> spi_ll_stm32.spi_context_buffers_setup: current_tx 0x200179b0 (1), current_rx 0x20017998 (2), tx buf/len 0x200179bc/1, rx buf/len 0x00000000/4
[00:00:01.459,000] <dbg> spi_ll_stm32.spi_context_update_tx: tx buf/len 0x00000000/0

on the logic analyzer:
Screenshot from 2020-01-03 16-38-10
Screenshot from 2020-01-03 16-38-34

Some comment:
1.) I checked the SPI coms via an older firmware that only uses STM32HAL and it works there. So we can say that the (flash) IC is not broken.
2.) there is no timeout, so the firmware breaks at https://github.com/zephyrproject-rtos/zephyr/blob/master/drivers/spi/spi_ll_stm32.c#L495 while waiting
3.) looks for me that the tx is send, but it somehow messes up while rx. There were some changes and maybe the rx_buf setup is no invaild

@StefJar StefJar added the bug The issue is a bug, or the PR is fixing a bug label Jan 3, 2020
@StefJar
Copy link
Author

StefJar commented Jan 3, 2020

I forgot the old version for comparison:

*** Booting Zephyr OS build zephyr-v2.1.0-383-g17d066b9dc0e ***

the latest version:

*** Booting Zephyr OS build zephyr-v2.1.0-634-gbd9962d8d98c ***

And a print out for the old SPI com:

Screenshot from 2020-01-03 17-17-13

@Marco-Peter
Copy link
Contributor

I experienced the same issue with the effect that the SPI_NOR driver did not initialize anymore (it receives the wrong device id).
By trial and error I could locate the offending commit being 2ce8fa1 but I still need to understand how.

erwango added a commit to erwango/zephyr that referenced this issue Jan 6, 2020
Behavior of sending 16bits in 8bits mode when possible is not working
with all slaves, as it appears.
should be
It should be optional and not be active by default.

Fixes zephyrproject-rtos#21546
Fixes zephyrproject-rtos#21679

Signed-off-by: Erwan Gouriou <erwan.gouriou@linaro.org>
@StefJar
Copy link
Author

StefJar commented Jan 6, 2020

@erwango I put your changes in but it is still broken
I attached a logic analser shot

old working version:
Screenshot from 2020-01-06 10-51-06

latest fix:
Screenshot from 2020-01-06 10-42-48

for me it looks that the rx never stops (spi_context_wait_for_completion does not return). That is why the CS does not go high

I have the same behavior on SPI1 and SPI3 and different ICs. On SPI1 I have a SST26VF016B flash and on SPI3 I have some sensors (ADS1292 & LSM6DS3).

erwango added a commit to erwango/zephyr that referenced this issue Jan 6, 2020
Refactoring of function spi_stm32_shift_m8/16 in PR zephyrproject-rtos#20948 which
introduced optional behavior "Allow 16bit sends in 8bit mode if
possible in master-only mode" has not been done with respecting
previous function behavior.

Fix newly created functions to respect flow of previous code:
-Do not impact spi context before tx empty condition is verified
-Update tx context and get tx_frame under spi_context_tx_on condition
(not spi_context_tx_buf_on condition)
-Always update rx context after data reception, not only under
spi_context_rx_buf_on condition

Last fix is known to fix regression on some cases.
Two first changes may be conservative and revisited individually
later on, aim is to minimize the changes in a first iteration.

Fixes zephyrproject-rtos#21679

Signed-off-by: Erwan Gouriou <erwan.gouriou@linaro.org>
erwango added a commit to erwango/zephyr that referenced this issue Jan 6, 2020
…ossible"

This reverts commit 2ce8fa1.

Fixes zephyrproject-rtos#21546
Fixes zephyrproject-rtos#21679

Signed-off-by: Erwan Gouriou <erwan.gouriou@linaro.org>
jhedberg pushed a commit that referenced this issue Jan 7, 2020
…ossible"

This reverts commit 2ce8fa1.

Fixes #21546
Fixes #21679

Signed-off-by: Erwan Gouriou <erwan.gouriou@linaro.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug The issue is a bug, or the PR is fixing a bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants