-
Notifications
You must be signed in to change notification settings - Fork 8.2k
drivers: mspi: Support MSPI driver for STM32. #96670
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
base: main
Are you sure you want to change the base?
drivers: mspi: Support MSPI driver for STM32. #96670
Conversation
834dc73 to
7e8241c
Compare
ed327ea to
a09a921
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please rename to dts/bindings/mspi/st,stm32-xspi-controller.yaml MSPI is the subsytem api and XSPI is the ST IP name.
Apply everywhere (driver files, ....) on this PR..
drivers/mspi/mspi_stm32.c
Outdated
|
|
||
| } | ||
|
|
||
| static int flash_stm32_xspi_dma_init(DMA_HandleTypeDef *hdma, struct stream *dma_stream) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
s/flash_stm32_xspi_dma_init/mspi_stm32_xspi_dma_init
| For example | ||
| dma-names = "tx_rx"; | ||
|
|
||
| ssht-enable: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For the three IPs, these bindings will have to propose the same configuration options as the union of dts/bindings/memory-controllers/st,stm32-xspi-psram.yaml and dts/bindings/flash_controller/st,stm32-xspi-nor.yaml
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note that this may potentially be done as a follow up PR
e0849ca to
806f0f8
Compare
drivers/mspi/mspi_stm32_qspi.c
Outdated
|
|
||
| LOG_MODULE_REGISTER(mspi_stm32_qspi, CONFIG_MSPI_LOG_LEVEL); | ||
|
|
||
| static inline int mspi_context_lock(struct mspi_context *ctx, const struct mspi_dev_id *req, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should this be mspi_context_lock(struct mspi_stm32_context *ctx ?
6bc0020 to
bb1245b
Compare
| device_type = "memory"; | ||
| reg = <0x90000000 DT_SIZE_M(256)>; | ||
| zephyr,memory-region = "QSPI"; | ||
| zephyr,memory-attr = <( DT_MEM_ARM(ATTR_MPU_RAM_NOCACHE) )>; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Preferrably without the outer parentheeses, e.g.:
zephyr,memory-attr = <DT_MEM_ARM(ATTR_MPU_FLASH)>;
| command-length ="INSTR_2_BYTE"; | ||
| rx-dummy = <20>; | ||
| jedec-id = [ c2 85 3a ]; | ||
| partitions { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Coudl you add an empty line above?
| #address-cells = <1>; | ||
| #size-cells = <0>; | ||
| op-mode = "MSPI_CONTROLLER"; | ||
| status = "okay"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Prefer have the status property last among the node properties. Not a blocking issue.
drivers/mspi/mspi_stm32_ospi.c
Outdated
|
|
||
| e_return: | ||
|
|
||
| k_mutex_unlock(&data->lock); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mutex was lokced only upon data->dev_id != dev_id. Condition to test also here.
Ditto for the unlock instruction above at line 1098.
drivers/mspi/mspi_stm32_ospi.c
Outdated
| .dev_cfg = {0}, \ | ||
| .xip_cfg = {0}, \ | ||
| .ctx.lock = Z_SEM_INITIALIZER(mspi_stm32_dev_data_##index.ctx.lock, 0, 1), \ | ||
| OSPI_DMA_CHANNEL(DT_DRV_INST(index), tx_rx)}; \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| OSPI_DMA_CHANNEL(DT_DRV_INST(index), tx_rx)}; \ | |
| OSPI_DMA_CHANNEL(DT_DRV_INST(index), tx_rx), \ | |
| }; \ |
96c07b3 to
cc1fc60
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To be applied for all instances in all files.
@ExaltZephyr Please apply review remarks (the ones you agree with) for all instances across all files. Otherwise, comment with your disagreement under the unaddressed ones.
2fa8377 to
adf9ad9
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry for flooding with coding style comments. I tried to be exhaustive, covering the 3 drivers.
drivers/mspi/mspi_stm32_ospi.c
Outdated
| uint32_t addr = dev_data->memmap_base_addr + packet->address; | ||
| uint32_t size = packet->num_bytes; | ||
|
|
||
| size = (size + 31U) & ~31U; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Keep size = (size + 31U) & ~31U; above? If you keep this, asserting size below is useless.
| switch (access_mode) { | ||
| case MSPI_ACCESS_SYNC: | ||
| hal_ret = HAL_OSPI_Receive(&dev_data->hmspi.ospi, packet->data_buf, | ||
| HAL_OSPI_TIMEOUT_DEFAULT_VALUE); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hal_ret value is not tested. Ditto in MSPI_TX path below.
drivers/mspi/mspi_stm32_xspi.c
Outdated
| * | ||
| * @retval 0 if successful. | ||
| * @retval -EINVAL invalid capabilities, failed to configure device. | ||
| * @retval -ENOTSUP capability not supported by MSPI peripheral. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There seems to be other possible return value. Mentioning A negative errno value upon failure would maybe be simpler.
This comment applies to other function inline description comments in this P-R.
adf9ad9 to
b2b9d14
Compare
@swift-tk Yes, that’s correct — currently, we only support one device connected to the controller. I don’t think there’s a plan to extend this for now. |
56ba9f4 to
af6db91
Compare
| dma-names: | ||
| description: | | ||
| DMA channel name. If DMA should be used, expected value is "tx_rx". |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you could add here:
enum:
- "tx_rx"
Ditto for the QSPI and XSPI bindings.
| dmas: | ||
| description: | | ||
| Optional DMA channel specifier, required for DMA transactions. | ||
| For example dmas for TX/RX on xspi |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here also, dmas description is not needed since already described in the relevant DMA binding description.
drivers/mspi/Kconfig.stm32
Outdated
| help | ||
| Enable QSPI driver for STM32 family of processors. | ||
|
|
||
| #DT_STM32_OCTOSPI_HAS_DMA := $(dt_compat_any_has_prop,$(DT_COMPAT_ST_STM32_OSPI),dmas) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you remove this line?
drivers/mspi/mspi_stm32.h
Outdated
| bool dst_addr_increment; | ||
| }; | ||
|
|
||
| union hmspi_handle { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| union hmspi_handle { | |
| union mspi_stm32_handle { |
drivers/mspi/mspi_stm32_xspi.c
Outdated
| } | ||
| #else | ||
| ret = mspi_stm32_xspi_access(controller, packet, | ||
| (ctx->xfer.async == true) ? MSPI_ACCESS_ASYNC |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
drivers/mspi/mspi_stm32_xspi.c
Outdated
| if (((xfer->packets->cmd == SPI_NOR_OCMD_RDSR) || | ||
| (xfer->packets->cmd == SPI_NOR_CMD_RDSR))) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| if (((xfer->packets->cmd == SPI_NOR_OCMD_RDSR) || | |
| (xfer->packets->cmd == SPI_NOR_CMD_RDSR))) { | |
| if ((xfer->packets->cmd == SPI_NOR_OCMD_RDSR) || | |
| (xfer->packets->cmd == SPI_NOR_CMD_RDSR)) { |
| #endif | ||
|
|
||
| #if defined(DLYB_XSPI1) || defined(DLYB_XSPI2) || defined(DLYB_OCTOSPI1) || defined(DLYB_OCTOSPI2) | ||
| HAL_XSPI_DLYB_CfgTypeDef mspi_delay_block_cfg = {0}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you add an empty line below?
drivers/mspi/mspi_stm32_xspi.c
Outdated
| (void)pm_device_runtime_put(spec->bus); | ||
|
|
||
| if (ret == 0) { | ||
| LOG_INF("MSPI config'd"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| LOG_INF("MSPI config'd"); | |
| LOG_INF("MSPI configured"); |
Does this really belong to the info log level? What about LOG_DBG()?
| .xip_cfg = {0}, \ | ||
| .ctx.lock = Z_SEM_INITIALIZER(mspi_stm32_dev_data_##index.ctx.lock, 0, 1), \ | ||
| XSPI_DMA_CHANNEL(DT_DRV_INST(index), tx, TX, MEMORY, PERIPHERAL) \ | ||
| XSPI_DMA_CHANNEL(DT_DRV_INST(index), rx, RX, PERIPHERAL, MEMORY)}; \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| XSPI_DMA_CHANNEL(DT_DRV_INST(index), rx, RX, PERIPHERAL, MEMORY)}; \ | |
| XSPI_DMA_CHANNEL(DT_DRV_INST(index), rx, RX, PERIPHERAL, MEMORY), \ | |
| }; \ |
drivers/mspi/mspi_stm32_ospi.c
Outdated
| * | ||
| * @retval 0 if successful. | ||
| * @retval -ESTALE device ID don't match, need to call mspi_dev_config first. | ||
| * @retval -Error transfer failed. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| * @retval -Error transfer failed. | |
| * @retval A negative errno value upon failure. |
8 other occurrences in this commit.
ecba19e to
04891a6
Compare
This commit adds the main DTS configurations required to enable MSPI/OSPI/QSPI support on STM32. Signed-off-by: Sara Touqan <[email protected]> Signed-off-by: Sarah Younis <[email protected]> Signed-off-by: Mohammad Odeh <[email protected]>
This commit introduces support for the mspi and ospi drivers on STM32, enabling functionality APIs for MSPI/OSPI/QSPI host controllers.. Signed-off-by: Sara Touqan <[email protected]> Signed-off-by: Sarah Younis <[email protected]> Signed-off-by: Mohammad Odeh <[email protected]>
this commit enables building and running the sample on stm32h573i_dk board,stm32h735g_disco board, arduino_giga_r1 board, and b_u585i_iot02a board by providing the required overlay and configuration updates. Signed-off-by: Sara Touqan <[email protected]> Signed-off-by: Sarah Younis <[email protected]> Signed-off-by: Mohammad Odeh <[email protected]>
04891a6 to
af6a86c
Compare
|



This PR introduces support for MSPI driver on STM32, enabling functionality APIs for MSPI host controllers.
mspi_stm32_xspi driver
ospi_stm32 driver
mspi_stm32_qspi driver
Notes: