Skip to content

drivers: mspi: Add driver for Designware SSI based controllers #80042

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

Merged
merged 7 commits into from
Mar 7, 2025

Conversation

anangl
Copy link
Member

@anangl anangl commented Oct 18, 2024

Add a generic driver for MSPI controllers based on the DesignWare SSI core. With small vendor-specific adaptations covering integration details, it should be possible to use the driver for various devices.

anangl added a commit to anangl/sdk-zephyr that referenced this pull request Oct 18, 2024
…trollers

Add a generic driver for controllers based on the Designware SSI core.
With small vendor-specific adaptations covering integration details,
it should be possible to use it for many devices.

Signed-off-by: Andrzej Głąbek <[email protected]>
(cherry picked from commit aea4e024685afd5a4a42252869d6e74d0b9fcecb)

Upstream PR: zephyrproject-rtos/zephyr#80042
anangl added a commit to anangl/sdk-zephyr that referenced this pull request Oct 18, 2024
…devices

Add a flash driver with intent to handle variuos flash devices
connected over MSPI bus as long as they support JEDEC SFDP.

Signed-off-by: Andrzej Głąbek <[email protected]>
(cherry picked from commit 21c32872cf50da2811f783ef4d53ba8c776c053c)

Upstream PR: zephyrproject-rtos/zephyr#80042
anangl added a commit to anangl/sdk-zephyr that referenced this pull request Oct 18, 2024
This is a follow-up to commit 45d827a.

Although routing for those pins is configured via UICR, pinctrl still
needs to be involved so that it is possible to set desired drive mode
for them etc.
Add also the missing RWDS pin.

Signed-off-by: Andrzej Głąbek <[email protected]>
(cherry picked from commit 2e243281b6a7fb27e44f142ee702ab0ff76cc79f)

Upstream PR: zephyrproject-rtos/zephyr#80042
anangl added a commit to anangl/sdk-zephyr that referenced this pull request Oct 18, 2024
…XMIF node

This is a follow-up to commit cdf45cb234077522b5cef2da084869af43d42dc1.

Adjust the DTS node for the EXMIF peripheral so that it is possible to
handle the peripheral with the generic MSPI driver for DW SSI based
controllers and use all its data lines in communication.
Also adjust the related board files accordingly.

Signed-off-by: Andrzej Głąbek <[email protected]>
(cherry picked from commit ad6bd27e4b1dddd5bdb045ae7204393a2e68d51c)

Upstream PR: zephyrproject-rtos/zephyr#80042
anangl added a commit to anangl/sdk-zephyr that referenced this pull request Oct 18, 2024
…mspi-nor" devices

Extend several flash samples and tests so that they can also be used
with "jedec,mspi-nor" devices.
Add configurations needed for the nrf54h20dk/nrf54h20/cpuapp target.

Signed-off-by: Andrzej Głąbek <[email protected]>
(cherry picked from commit 280e331a584d80925711077fed8eefa09843e5bf)

Upstream PR: zephyrproject-rtos/zephyr#80042
anangl added a commit to anangl/sdk-zephyr that referenced this pull request Oct 18, 2024
This is a follow-up to commit cdf45cb234077522b5cef2da084869af43d42dc1.

Adjust the DTS node for the nRF EXMIF peripheral so that it is possible
to handle the peripheral with the generic MSPI driver for DW SSI based
controllers and use all its data lines in communication.
Also adjust the related board files accordingly.

Signed-off-by: Andrzej Głąbek <[email protected]>
(cherry picked from commit 7e089051bb197229f591f5fda539abc4ee5f0e68)

Upstream PR: zephyrproject-rtos/zephyr#80042
anangl added a commit to anangl/sdk-zephyr that referenced this pull request Oct 18, 2024
…mspi-nor" devices

Extend several flash samples and tests so that they can also be used
with "jedec,mspi-nor" devices.
Add configurations needed for the nrf54h20dk/nrf54h20/cpuapp target.

Signed-off-by: Andrzej Głąbek <[email protected]>
(cherry picked from commit 8d9b9a41109b52490ff3768b79e80a1ac517ac0d)

Upstream PR: zephyrproject-rtos/zephyr#80042
anangl added a commit to anangl/sdk-zephyr that referenced this pull request Oct 21, 2024
…trollers

Add a generic driver for controllers based on the Designware SSI core.
With small vendor-specific adaptations covering integration details,
it should be possible to use it for many devices.

Signed-off-by: Andrzej Głąbek <[email protected]>
(cherry picked from commit aea4e024685afd5a4a42252869d6e74d0b9fcecb)

Upstream PR: zephyrproject-rtos/zephyr#80042
anangl added a commit to anangl/sdk-zephyr that referenced this pull request Oct 21, 2024
…devices

Add a flash driver with intent to handle variuos flash devices
connected over MSPI bus as long as they support JEDEC SFDP.

Signed-off-by: Andrzej Głąbek <[email protected]>
(cherry picked from commit 21c32872cf50da2811f783ef4d53ba8c776c053c)

Upstream PR: zephyrproject-rtos/zephyr#80042
anangl added a commit to anangl/sdk-zephyr that referenced this pull request Oct 21, 2024
This is a follow-up to commit 45d827a.

Although routing for those pins is configured via UICR, pinctrl still
needs to be involved so that it is possible to set desired drive mode
for them etc.
Add also the missing RWDS pin.

Signed-off-by: Andrzej Głąbek <[email protected]>
(cherry picked from commit 2e243281b6a7fb27e44f142ee702ab0ff76cc79f)

Upstream PR: zephyrproject-rtos/zephyr#80042
anangl added a commit to anangl/sdk-zephyr that referenced this pull request Oct 21, 2024
This is a follow-up to commit cdf45cb234077522b5cef2da084869af43d42dc1.

Adjust the DTS node for the nRF EXMIF peripheral so that it is possible
to handle the peripheral with the generic MSPI driver for DW SSI based
controllers and use all its data lines in communication.
Also adjust the related board files accordingly.

Signed-off-by: Andrzej Głąbek <[email protected]>
(cherry picked from commit 7e089051bb197229f591f5fda539abc4ee5f0e68)

Upstream PR: zephyrproject-rtos/zephyr#80042
anangl added a commit to anangl/sdk-zephyr that referenced this pull request Oct 21, 2024
…mspi-nor" devices

Extend several flash samples and tests so that they can also be used
with "jedec,mspi-nor" devices.
Add configurations needed for the nrf54h20dk/nrf54h20/cpuapp target.

Signed-off-by: Andrzej Głąbek <[email protected]>
(cherry picked from commit 8d9b9a41109b52490ff3768b79e80a1ac517ac0d)

Upstream PR: zephyrproject-rtos/zephyr#80042
anangl added a commit to anangl/sdk-zephyr that referenced this pull request Oct 25, 2024
Upstream PR: 'zephyrproject-rtos/zephyr#80042'

Signed-off-by: Andrzej Głąbek <[email protected]>
@anangl anangl force-pushed the mspi_dw branch 4 times, most recently from 4d1ad2d to 37c40d7 Compare November 8, 2024 13:32
@anangl
Copy link
Member Author

anangl commented Nov 8, 2024

Rebased.

@anangl anangl force-pushed the mspi_dw branch 2 times, most recently from 229e2f3 to b88d298 Compare November 8, 2024 13:38
@anangl anangl force-pushed the mspi_dw branch 2 times, most recently from a00f1f7 to 17dfe10 Compare December 3, 2024 22:35
@anangl anangl marked this pull request as ready for review December 3, 2024 22:36
@anangl anangl requested a review from swift-tk February 20, 2025 11:56
Copy link
Collaborator

@swift-tk swift-tk left a comment

Choose a reason for hiding this comment

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

I think we should return the pm_device_*(dev_config->bus) calls to the controller as it seems to be leaking. For other stuff in the flash device driver, it can be further refined in following PRs.

}
}

(void)k_sem_take(&dev_data->ctx_lock, K_FOREVER);
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't suppose you can open/close a certain device in different context, as they are blocked by acquire below

k_sem_take(&dev_data->acquired, K_FOREVER);

In contrast, you can open/close different device in different context, but these device may share the same controller. That's why it should be a mutex(Ln675) which is capable of inheriting priority. This sem here is then redundent as mspi API calls from flash device driver layer are encapsulated between mspi_dev_config and mspi_get_channel_status and then again encapsulated between acquire and release.
So what is the reason for not following this with other drivers?

Comment on lines 56 to 71
static int vendor_specific_xip_disable(const struct device *dev,
const struct mspi_dev_id *dev_id,
const struct mspi_xip_cfg *cfg)
Copy link
Collaborator

Choose a reason for hiding this comment

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

@anangl resolved or not?


dev_data->xfer.cmd_length = 2;
dev_data->xfer.addr_length = 4;
dev_data->xfer.rx_dummy = 20;
Copy link
Collaborator

Choose a reason for hiding this comment

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

okay, there is only two other places that needs change/clarify

anangl added 7 commits March 5, 2025 09:31
Add a generic driver for MSPI controllers based on the DesignWare
SSI core. With small vendor-specific adaptations covering integration
details, it should be possible to use the driver for various devices.

Signed-off-by: Andrzej Głąbek <[email protected]>
Add a flash driver intended to handle various flash devices
connected over MSPI bus as long as they support JEDEC SFDP.
This is an initial commit providing only basic operations
in Octal I/O mode with some hard-coded values for Macronix
MX25Ux series chips.

Signed-off-by: Andrzej Głąbek <[email protected]>
This is a follow-up to commit 45d827a.

Although routing for those pins is configured via UICR, pinctrl still
needs to be involved so that it is possible to set desired drive mode
for them etc.
Add also the missing RWDS pin.

Signed-off-by: Andrzej Głąbek <[email protected]>
This is a follow-up to commit cdf45cb.

Adjust the DTS node for the nRF EXMIF peripheral so that it is possible
to handle the peripheral with the generic MSPI driver for DW SSI based
controllers and use all its data lines in communication.
Also adjust the related board files accordingly.

Signed-off-by: Andrzej Głąbek <[email protected]>
Extend several flash samples and tests so that they can also be used
with "jedec,mspi-nor" devices.
Add configurations needed for the nrf54h20dk/nrf54h20/cpuapp target.

Signed-off-by: Andrzej Głąbek <[email protected]>
Access to this region must be requested through UICR by a local
domain that want to use the Execute In Place (XIP) feature of
the EXMIF peripheral.

Signed-off-by: Andrzej Głąbek <[email protected]>
Add nRF54H20 DK specific entries to allow using the sample on this
board.

Signed-off-by: Andrzej Głąbek <[email protected]>
@anangl
Copy link
Member Author

anangl commented Mar 5, 2025

I think we should return the pm_device_*(dev_config->bus) calls to the controller as it seems to be leaking. For other stuff in the flash device driver, it can be further refined in following PRs.

The controller driver (mspi_dw.c) makes the pm_device_runtime_*() calls for itself (get right before a transfer and put after it is finished), just in case its user would not do it. But having these calls also in the flash driver has the advantage that the controller will not be unnecessarily suspended between transfers if an operation consists of several (a write operation requires executing WREN and then the actual write, an erase operation may involve erasing of multiple sectors for example). You can see that the spi_nor driver uses the same approach:

(void)pm_device_runtime_get(cfg->spi.bus);

I refactored a bit the logic in acquire() because in its previous shape the acquired semaphore would incorrectly remain taken when the function was unsuccessful due to failed pm_device_runtime_get() or mspi_dev_config(). But I kept the pm_device_runtime_get(dev_config->bus) call there for the reason I explained above.

@anangl anangl requested a review from swift-tk March 5, 2025 09:39
@swift-tk
Copy link
Collaborator

swift-tk commented Mar 5, 2025

The controller driver (mspi_dw.c) makes the pm_device_runtime_*() calls for itself

This is exactly why I think the control of the controller state is leaking into flash driver, because there will be different implementation of the controller and logics as to where to put the pm_device_runtime_*() calls. This in result will potentially create asymmetry calls of put and get and then result in incorrect state of the controller.

@swift-tk
Copy link
Collaborator

swift-tk commented Mar 5, 2025

The updated code does not conflict with the upcoming PM_RUNTIME support in Ambiq controllers but it would prematurly wake up the controller, so I would not block the PR. However, I still insist that the PM managment should be left within each device itself and not leaking into other devices whatever the hierarchy relationship between them.

@anangl
Copy link
Member Author

anangl commented Mar 5, 2025

The controller driver (mspi_dw.c) makes the pm_device_runtime_*() calls for itself

This is exactly why I think the control of the controller state is leaking into flash driver, because there will be different implementation of the controller and logics as to where to put the pm_device_runtime_*() calls. This in result will potentially create asymmetry calls of put and get and then result in incorrect state of the controller.

The runtime PM resumes and suspends devices based on their usage count - the device stays resumed as long as there was more get() than put() calls executed, so it is not a problem at all that the calls are done on multiple layers. Just every layer needs to take care of doing as many put() calls as it did the get() ones and I'm doing it in these drivers here. So I don't quite understand what you actually see as potentially problematic.

@swift-tk
Copy link
Collaborator

swift-tk commented Mar 5, 2025

The runtime PM resumes and suspends devices based on their usage count - the device stays resumed as long as there was more get() than put() calls executed, so it is not a problem at all that the calls are done on multiple layers. Just every layer needs to take care of doing as many put() calls as it did the get() ones and I'm doing it in these drivers here. So I don't quite understand what you actually see as potentially problematic.

I'm only putting one get() in mspi_dev_config() and one put() in mspi_get_channel_status in no error conditions as turning our controller on and off in different places is needless and may be more time and power consuming. In fact, it is already slower by doing so because of the frequent acquire and release in flash drivers. It is just going to be even slower with all these needless counting in flash drivers. So basically I'm just allowing the usage count to be 0 or 1 to save more CPU cycles.

@gmarull gmarull added this to the v4.2.0 milestone Mar 5, 2025
@carlescufi carlescufi merged commit ac3355a into zephyrproject-rtos:main Mar 7, 2025
29 checks passed
@anangl anangl deleted the mspi_dw branch March 8, 2025 12:18
danieldegrasse added a commit to danieldegrasse/tt-zephyr-platforms that referenced this pull request Apr 16, 2025
Import MSPI driver introduced in
zephyrproject-rtos/zephyr#80042.

The driver has been lightly modified to add the ability to set the
RX_DELAY register.

Once we resynchronize with upstream Zephyr, this change can be turned
into a patch to the driver.

Signed-off-by: Daniel DeGrasse <[email protected]>
danieldegrasse added a commit to danieldegrasse/tt-zephyr-platforms that referenced this pull request May 2, 2025
Import MSPI driver introduced in
zephyrproject-rtos/zephyr#80042.

The driver has been lightly modified to add the ability to set the
RX_DELAY register.

Once we resynchronize with upstream Zephyr, this change can be turned
into a patch to the driver.

Signed-off-by: Daniel DeGrasse <[email protected]>
danieldegrasse added a commit to danieldegrasse/tt-zephyr-platforms that referenced this pull request May 14, 2025
Import MSPI driver introduced in
zephyrproject-rtos/zephyr#80042.

The driver has been lightly modified to add the ability to set the
RX_DELAY register.

Once we resynchronize with upstream Zephyr, this change can be turned
into a patch to the driver.

Signed-off-by: Daniel DeGrasse <[email protected]>
danieldegrasse added a commit to danieldegrasse/tt-zephyr-platforms that referenced this pull request May 15, 2025
Import MSPI driver introduced in
zephyrproject-rtos/zephyr#80042.

The driver has been lightly modified to add the ability to set the
RX_DELAY register.

Once we resynchronize with upstream Zephyr, this change can be turned
into a patch to the driver.

Signed-off-by: Daniel DeGrasse <[email protected]>
danieldegrasse added a commit to danieldegrasse/tt-zephyr-platforms that referenced this pull request May 16, 2025
Import MSPI driver introduced in
zephyrproject-rtos/zephyr#80042.

The driver has been lightly modified to add the ability to set the
RX_DELAY register.

Once we resynchronize with upstream Zephyr, this change can be turned
into a patch to the driver.

Signed-off-by: Daniel DeGrasse <[email protected]>
danieldegrasse added a commit to tenstorrent/tt-zephyr-platforms that referenced this pull request May 16, 2025
Import MSPI driver introduced in
zephyrproject-rtos/zephyr#80042.

The driver has been lightly modified to add the ability to set the
RX_DELAY register.

Once we resynchronize with upstream Zephyr, this change can be turned
into a patch to the driver.

Signed-off-by: Daniel DeGrasse <[email protected]>
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.

7 participants