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

drivers: flash: Add flash driver for MRAM #69800

Merged
merged 2 commits into from Mar 8, 2024

Conversation

57300
Copy link
Contributor

@57300 57300 commented Mar 5, 2024

Basic driver utilizing the flash API for NVM operations on the nRF54H20.

anangl
anangl previously approved these changes Mar 5, 2024
carlescufi
carlescufi previously approved these changes Mar 5, 2024
57300 added a commit to 57300/sdk-zephyr that referenced this pull request Mar 5, 2024
Upstream PR: zephyrproject-rtos/zephyr#69800

Basic driver utilizing the flash API for NVM operations on the nRF54H20.

Signed-off-by: Grzegorz Swiderski <grzegorz.swiderski@nordicsemi.no>
(cherry picked from commit 0797e3c)
drivers/flash/soc_flash_nrf_mram.c Outdated Show resolved Hide resolved
drivers/flash/soc_flash_nrf_mram.c Outdated Show resolved Hide resolved
drivers/flash/soc_flash_nrf_mram.c Outdated Show resolved Hide resolved
drivers/flash/soc_flash_nrf_mram.c Show resolved Hide resolved
drivers/flash/soc_flash_nrf_mram.c Outdated Show resolved Hide resolved
drivers/flash/soc_flash_nrf_mram.c Outdated Show resolved Hide resolved
de-nordic
de-nordic previously approved these changes Mar 6, 2024
Copy link
Collaborator

@de-nordic de-nordic left a comment

Choose a reason for hiding this comment

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

Looks OK, although I think that we are missing "erase-block-size" optional binding in DTS for the MRAM, and write-block-size being read from the DTS.
The second issue seems to be also on soc-nrf-flash driver, so will not insist on implementing.
The first one seems controversial, but if author will not pursue we can just do another PR with the change to binding and driver.

57300 added a commit to 57300/sdk-zephyr that referenced this pull request Mar 6, 2024
Upstream PR: zephyrproject-rtos/zephyr#69800

Basic driver utilizing the flash API for NVM operations on the nRF54H20.

Signed-off-by: Grzegorz Swiderski <grzegorz.swiderski@nordicsemi.no>
(cherry picked from commit d3b56679837ce4045d9e2618a25d7e85168eeeb9)
anangl
anangl previously approved these changes Mar 7, 2024
@str4t0m str4t0m added the platform: nRF Nordic nRFx label Mar 7, 2024
This is supported in hardware, but it requires slightly more code to
perform the operations safely.

config SOC_FLASH_NRF_MRAM_LAYOUT_PAGE_SIZE
Copy link
Collaborator

@Laczen Laczen Mar 7, 2024

Choose a reason for hiding this comment

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

LGTM, but

This should be moved to the dts or fixed in the driver. This will avoid any confusion:

  • there are drivers where this isn't specified in the dts (because the blocks vary in size) in that case it is fixed by the driver.
  • there are drivers where this is specified and then it is used for the page layout.

Adding it as a kconfig might result in applications on the same device that get stuck because they where compiled with different kconfig options.

In case where ONE_BYTE_WRITE_ACCESS is not selected it should also be a multiple of the write-block-size.

Copy link
Collaborator

@Laczen Laczen Mar 7, 2024

Choose a reason for hiding this comment

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

To be fair, also the ONE_BYTE_WRITE_ACCESS should also come from the dts (by setting write-block-size equal to 1). But I didn't check how this is done on other drivers.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Alright, I'll move both Kconfigs to DT. I realize now that both write-block-size and erase-block-size seem to be treated as configurable anyway, which is strange, but I can play along. I'd like to come back to this at a future date.

Basic driver utilizing the flash API for NVM operations on the nRF54H20.

Signed-off-by: Grzegorz Swiderski <grzegorz.swiderski@nordicsemi.no>
The MRAM has no concept of erase blocks or pages, so this is treated as
driver configuration.

Signed-off-by: Grzegorz Swiderski <grzegorz.swiderski@nordicsemi.no>
@57300 57300 dismissed stale reviews from anangl and de-nordic via 8f8a078 March 7, 2024 14:59
@zephyrbot zephyrbot requested a review from galak March 7, 2024 15:00
@57300 57300 requested review from Laczen and de-nordic March 7, 2024 15:00
57300 added a commit to 57300/sdk-zephyr that referenced this pull request Mar 7, 2024
Upstream PR: zephyrproject-rtos/zephyr#69800

Basic driver utilizing the flash API for NVM operations on the nRF54H20.

Signed-off-by: Grzegorz Swiderski <grzegorz.swiderski@nordicsemi.no>
(cherry picked from commit 25b932167764cbec99f1e932703eb7da67a434eb)
57300 added a commit to 57300/sdk-zephyr that referenced this pull request Mar 7, 2024
…am1x

Upstream PR: zephyrproject-rtos/zephyr#69800

The MRAM has no concept of erase blocks or pages, so this is treated as
driver configuration.

Signed-off-by: Grzegorz Swiderski <grzegorz.swiderski@nordicsemi.no>
(cherry picked from commit 8f8a078ffee5fbc066666f6ae8c0c295ab6bdb36)
Copy link
Collaborator

@Laczen Laczen left a comment

Choose a reason for hiding this comment

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

LGTM

@henrikbrixandersen henrikbrixandersen merged commit beab89d into zephyrproject-rtos:main Mar 8, 2024
23 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

9 participants