Skip to content

stm32h7 spi driver uses SRAM2 buffers in NOCACHE region #61021

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.

Sign up for GitHub

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

Closed
wants to merge 2 commits into from
Closed

stm32h7 spi driver uses SRAM2 buffers in NOCACHE region #61021

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6c03000
drivers: spi: stm32 spi driver with null elements in buffer set
FRASTM Aug 1, 2023
b480a4c
tests: drivers: spi loopback on stm32h7 SRAM2 memory
FRASTM Jul 31, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 8 additions & 1 deletion drivers/spi/spi_ll_stm32.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -779,6 +779,12 @@ static bool spi_buf_set_in_nocache(const struct spi_buf_set *bufs)
}
return true;
}

/* Check if spi_buf_set structure has null elements */
static bool is_null_buffer(const struct spi_buf_set *tx_bufs)
{
return ((tx_bufs->count == 1) && (tx_bufs->buffers[0].buf == NULL));
}
#endif /* CONFIG_SOC_SERIES_STM32H7X */

static int transceive_dma(const struct device *dev,
Expand All @@ -803,7 +809,8 @@ static int transceive_dma(const struct device *dev,
}

#ifdef CONFIG_SOC_SERIES_STM32H7X
if ((tx_bufs != NULL && !spi_buf_set_in_nocache(tx_bufs)) ||
if ((tx_bufs != NULL && !is_null_buffer(tx_bufs) &&
!spi_buf_set_in_nocache(tx_bufs)) ||
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have re-read the SPI header code docs. This is what it exactly says about null buffers:

/**
 * @brief SPI buffer structure
 *
 * @param buf is a valid pointer on a data buffer, or NULL otherwise.
 * @param len is the length of the buffer or, if buf is NULL, will be the
 *    length which as to be sent as dummy bytes (as TX buffer) or
 *    the length of bytes that should be skipped (as RX buffer).
 */
struct spi_buf {
	void *buf;
	size_t len;
};

Any buffer in the buf_set could be null and then, dummy bytes should be sent or ignored. Therefore, I would change this function by:

static bool is_dummy_buffer(const struct spi_buf* buf)
{
	return buf->buf == NULL;
}
...
static bool spi_buf_set_in_nocache(const struct spi_buf_set *bufs)
{
	for (size_t i = 0; i < bufs->count; i++) {
		const struct spi_buf *buf = &bufs->buffers[i];

		if (!is_dummy_buffer(buf) &&
			!buf_in_nocache((uintptr_t)buf->buf, buf->len)) {
			return false;
		}
	}
	return true;
}

and leave this as it was:

#ifdef CONFIG_SOC_SERIES_STM32H7X
	if ((tx_bufs != NULL && !spi_buf_set_in_nocache(tx_bufs)) ||
		(rx_bufs != NULL && !spi_buf_set_in_nocache(rx_bufs))) {
		return -EFAULT;
	}
#endif /* CONFIG_SOC_SERIES_STM32H7X */

Copy link
Contributor

@dgastonochoa dgastonochoa Aug 24, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi again, don't worry about this change because I have included it in #61265

(rx_bufs != NULL && !spi_buf_set_in_nocache(rx_bufs))) {
return -EFAULT;
}
Expand Down
4 changes: 4 additions & 0 deletions tests/drivers/spi/spi_loopback/boards/nucleo_h723zg.overlay
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,10 @@
* SPDX-License-Identifier: Apache-2.0
*/

&sram2 {
zephyr,memory-attr = "RAM_NOCACHE";
};

/* Set div-q to get test clk freq into acceptable SPI freq range */
&pll {
/delete-property/ div-q;
Expand Down
4 changes: 4 additions & 0 deletions tests/drivers/spi/spi_loopback/boards/nucleo_h743zi.overlay
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,10 @@
* SPDX-License-Identifier: Apache-2.0
*/

&sram2 {
zephyr,memory-attr = "RAM_NOCACHE";
};

&spi1 {
dmas = <&dmamux1 0 38 (STM32_DMA_PERIPH_TX | STM32_DMA_PRIORITY_HIGH)
&dmamux1 1 37 (STM32_DMA_PERIPH_RX | STM32_DMA_PRIORITY_HIGH)>;
Expand Down
1 change: 1 addition & 0 deletions tests/drivers/spi/spi_loopback/boards/nucleo_h753zi.conf
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
CONFIG_NOCACHE_MEMORY=y
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is no longer necessary I believe.

37 changes: 37 additions & 0 deletions tests/drivers/spi/spi_loopback/boards/nucleo_h753zi.overlay
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
/*
* Copyright (c) 2021 STMicroelectronics
*
* SPDX-License-Identifier: Apache-2.0
*/

&sram2 {
zephyr,memory-attr = "RAM_NOCACHE";
};

&spi1 {
dmas = <&dmamux1 0 38 (STM32_DMA_PERIPH_TX | STM32_DMA_PRIORITY_HIGH)
&dmamux1 1 37 (STM32_DMA_PERIPH_RX | STM32_DMA_PRIORITY_HIGH)>;
dma-names = "tx", "rx";
slow@0 {
compatible = "test-spi-loopback-slow";
reg = <0>;
spi-max-frequency = <500000>;
};
fast@0 {
compatible = "test-spi-loopback-fast";
reg = <0>;
spi-max-frequency = <16000000>;
};
};

&dma1 {
status = "okay";
};

&dma2 {
status = "okay";
};

&dmamux1 {
status = "okay";
};
18 changes: 13 additions & 5 deletions tests/drivers/spi/spi_loopback/src/spi.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,13 +38,20 @@ static struct spi_dt_spec spi_slow = SPI_DT_SPEC_GET(SPI_SLOW_DEV, SPI_OP, 0);
#define BUF_SIZE 17
#define BUF2_SIZE 36

#if CONFIG_NOCACHE_MEMORY
#ifdef CONFIG_NOCACHE_MEMORY

#ifdef CONFIG_SOC_SERIES_STM32H7X
#define CACHE_SECTION __attribute__((__section__("SRAM2")))
#else
#define CACHE_SECTION __attribute__((__section__(".nocache")))
#endif /* CONFIG_SOC_SERIES_STM32H7X */

static const char tx_data[BUF_SIZE] = "0123456789abcdef\0";
static __aligned(32) char buffer_tx[BUF_SIZE] __used __attribute__((__section__(".nocache")));
static __aligned(32) char buffer_rx[BUF_SIZE] __used __attribute__((__section__(".nocache")));
static __aligned(32) char buffer_tx[BUF_SIZE] __used CACHE_SECTION;
static __aligned(32) char buffer_rx[BUF_SIZE] __used CACHE_SECTION;
static const char tx2_data[BUF2_SIZE] = "Thequickbrownfoxjumpsoverthelazydog\0";
static __aligned(32) char buffer2_tx[BUF2_SIZE] __used __attribute__((__section__(".nocache")));
static __aligned(32) char buffer2_rx[BUF2_SIZE] __used __attribute__((__section__(".nocache")));
static __aligned(32) char buffer2_tx[BUF2_SIZE] __used CACHE_SECTION;
static __aligned(32) char buffer2_rx[BUF2_SIZE] __used CACHE_SECTION;
#else
/* this src memory shall be in RAM to support using as a DMA source pointer.*/
static uint8_t buffer_tx[] = "0123456789abcdef\0";
Expand Down Expand Up @@ -598,6 +605,7 @@ static void *spi_loopback_setup(void)
memset(buffer2_tx, 0, sizeof(buffer2_tx));
memcpy(buffer2_tx, tx2_data, sizeof(tx2_data));
#endif

return NULL;
}

Expand Down