-
Notifications
You must be signed in to change notification settings - Fork 7.9k
drivers: memc: add driver for stm32 ospi psram #92381
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?
Conversation
752b380
to
3e5e735
Compare
Thank you Mario! Could you tag this for backport into v4.1? |
@dkouba-atym I dont think I can or am allowed to. I dont really see any relevance of this driver for v4.1. |
8b0fd92
to
6efb10f
Compare
Only fixes are backported. |
17525a8
to
823d932
Compare
I was able to integrate the driver with an STM32U5A9J overlay and successfully write and read an APS6408L-3OBM-BA using the supplied test program. |
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 a short sentence in the commit message like the below?
Add an overlay DTSI in tests/drivers/memc/ram for that board to pass this test
over its PSRAM only.
Better to extend the test. |
Maybe simply get the RAM size from the DT to extended tested RAM locations. |
712c1ed
to
7bc7a79
Compare
Is there a reason why we are explicitly naming the linker regions in the For example, this is a code snippet I use:
On the RT1064, there is no associated However, on the U585, the name printed out in the memory + flash summary is Took a bunch of digging to figure out the difference here, and I would suggest using the way Zephyr automatically assigns names over a custom |
7fe13b5
to
26b2866
Compare
@mariopaja see #95299 |
6fecc43
to
e37608e
Compare
It seems like removing |
Add a driver for STM32 OSPI PSRAM in memory mapped mode. Enable OSPI PSRAM on b_u585i_iot02a. Signed-off-by: Mario Paja <[email protected]>
e37608e
to
4371901
Compare
|
Unfortunately, I’ve found that the provided driver isn’t compatible with the STM32H735 SoC. I’m trying to use the RAM on the stm32h735g_disco. Do you think it would be possible to support it as well? |
Hi @Piziwate, BTW is uses 128-Mbit HyperRAM™ based on STM32H735G-DK |
Yes, I know—and that’s exactly my problem. This MCU is interesting for our project, and using HyperRAM makes for a simple design. Unfortunately, I’m not quite sure how to integrate it with Zephyr. I thought Octo PSRAM was similar. I tried using the driver from this PR, but I’m getting errors like: ‘error: unknown type name ‘HAL_OSPI_DLYB_CfgTypeDef’.’ |
I think |
Okay, that makes more sense! In that case, shouldn’t you add a dependency at the Kconfig level to prevent it from being (unsuccessfully) used on other STM32s? |
Perhaps you are right |
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.
Thanks for pushing this !
#include <zephyr/device.h> | ||
#include <zephyr/drivers/clock_control.h> | ||
#include <zephyr/drivers/clock_control/stm32_clock_control.h> | ||
#include <zephyr/drivers/gpio.h> |
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.
Doesn't look required.
OSPIM_CfgTypeDef sOspiManagerCfg; | ||
HAL_OSPI_DLYB_CfgTypeDef HAL_OSPI_DLYB_Cfg_Struct; |
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 use zephyr style for new variables
OSPIM_CfgTypeDef sOspiManagerCfg; | |
HAL_OSPI_DLYB_CfgTypeDef HAL_OSPI_DLYB_Cfg_Struct; | |
OSPIM_CfgTypeDef ospim_cfg; | |
HAL_OSPI_DLYB_CfgTypeDef dlyb_cfg; |
|
||
static int ap_memory_write_reg(OSPI_HandleTypeDef *hospi, uint32_t address, uint8_t *value) | ||
{ | ||
OSPI_RegularCmdTypeDef sCommand = {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.
OSPI_RegularCmdTypeDef sCommand = {0}; | |
OSPI_RegularCmdTypeDef cmd = {0}; |
uint32_t latency_cycles) | ||
{ | ||
|
||
OSPI_RegularCmdTypeDef sCommand = {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.
ditto
struct memc_stm32_ospi_psram_data *dev_data = dev->data; | ||
OSPI_HandleTypeDef *hospi = &dev_data->hospi; | ||
OSPI_RegularCmdTypeDef sCommand = {0}; | ||
OSPI_MemoryMappedTypeDef sMemMappedCfg = {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.
OSPI_MemoryMappedTypeDef sMemMappedCfg = {0}; | |
OSPI_MemoryMappedTypeDef mem_mapped_cfg = {0}; |
select USE_STM32_HAL_DMA_EX | ||
select USE_STM32_HAL_DMA |
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.
Are these required by OSPI HAL ?
Otherwise not required
@Piziwate As a naive fix, that's unlikely to work correctly, try guarding |
@JarmouniA @erwango should I make it U5xx specific, or should I do the following:
|
IMO , we should focus on U5 for now and expansion to a new series should be kept for a future PR. Otherwise we'll lose ourselves in workarounds |
Then the driver config should have |
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 there should be 1 commit for the driver part (incl. the DT bindings addition), 1 for board b_u585i_iot02a and a last one to enable test on that board, which may request to add memc
tag in the supported
list in the board YAML file. Maybe these 2 last commits could be squashed together.
STM32 OSPI PSRAM memory controller. | ||
Provides a communication interface allowing the microcontroller | ||
to communicate with an external PSRAM memory via the OSPI peripheral. | ||
The PSRAM can be configured as memory mapped and allow read/write |
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.
The PSRAM can be configured as memory mapped and allow read/write | |
The PSRAM can be configured as memory mapped and allows read/write |
#define STM32_OSPI_NODE DT_INST_PARENT(0) | ||
|
||
#ifdef CONFIG_SHARED_MULTI_HEAP | ||
struct shared_multi_heap_region smh_psram = { |
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.
static const
?
#define WRITE_CMD 0x8080 | ||
/* Read Operations */ | ||
#define READ_CMD 0x0000 | ||
/* Default dummy clocks cycles */ |
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 above?
- 4 | ||
- 5 | ||
|
||
refresh-rate: |
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.
The property name is a bit ambiguous. How about adaptive-refresh-rate
or auto-adjust-refresh-rate
?
type: int | ||
default: 0 | ||
description: | | ||
Refresh coverage: |
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 readability (of pasr
property name acronym), I would suggest Partial Array Self-Refresh coverage:
device will continually wrap within. | ||
If enabled, the device will burst through the initial wrapped burst length | ||
once, then continue to burst incrementally up to maximum column address | ||
(1K) before wrapping around within the entire column address space |
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.
(1K) before wrapping around within the entire column address space | |
(1K) before wrapping around within the entire column address space. |
Add STM32 OSPI PSRAM driver support in memory mapped mode
Current driver configuration based on
aps6408l
Test application 1
https://github.com/mariopaja/hello_psram
Custom U575 Board:
b_u585i_iot02a:
Test application 2
tests/drivers/memc/ram