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

tests: drivers: mspi: make MSPI API test more generic #81870

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

jaz1-nordic
Copy link
Contributor

@jaz1-nordic jaz1-nordic commented Nov 25, 2024

Make MSPI API test more generic

jaz1-nordic added a commit to jaz1-nordic/ncs-sdk-zephyr that referenced this pull request Nov 25, 2024
Configure SDP MSPI pins to switch their control to VPR core

Upstream PR: zephyrproject-rtos/zephyr#81870

Signed-off-by: Jakub Zymelka <[email protected]>
jaz1-nordic added a commit to jaz1-nordic/ncs-sdk-zephyr that referenced this pull request Nov 25, 2024
Add ifdefs to make MSPI API test more generic.

Upstream PR: zephyrproject-rtos/zephyr#81870

Signed-off-by: Jakub Zymelka <[email protected]>
jaz1-nordic added a commit to jaz1-nordic/ncs-sdk-zephyr that referenced this pull request Nov 25, 2024
Add nRF54l15 overlay file to be able to run SDP MSPI API test.

Upstream PR: zephyrproject-rtos/zephyr#81870

Signed-off-by: Jakub Zymelka <[email protected]>
@jaz1-nordic jaz1-nordic changed the title Changes needed for SDP MSPI tests: drivers: mspi: make MSPI API test more generic Nov 25, 2024
masz-nordic
masz-nordic previously approved these changes Nov 25, 2024
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 wonder do we really need to associate CONFIG_MSPI_ASYNC with mspi_register_callback?

Add ifdefs to make MSPI API test more generic.

Signed-off-by: Jakub Zymelka <[email protected]>
@jaz1-nordic
Copy link
Contributor Author

I wonder do we really need to associate CONFIG_MSPI_ASYNC with mspi_register_callback?

From what I see in current drivers, asynchronous operations are closely related to the use of callbacks, e.g. here:

if (xfer->async) {
cb = data->cbs[MSPI_BUS_XFER_COMPLETE];
cb_ctx = data->cb_ctxs[MSPI_BUS_XFER_COMPLETE];

Unfortunately, current drivers, despite using asynchronous xfers, do not have CONFIG_MSPI_ASYNC set to y in their KConfig files.
Consequently, making API callback testing dependent on CONFIG_MSPI_ASYNC may reduce the scope of testing these drivers.
Due to the above, I changed the test implementation.

@swift-tk
Copy link
Collaborator

swift-tk commented Nov 27, 2024

I wonder do we really need to associate CONFIG_MSPI_ASYNC with mspi_register_callback?

From what I see in current drivers, asynchronous operations are closely related to the use of callbacks, e.g. here:

if (xfer->async) {
cb = data->cbs[MSPI_BUS_XFER_COMPLETE];
cb_ctx = data->cb_ctxs[MSPI_BUS_XFER_COMPLETE];

Unfortunately, current drivers, despite using asynchronous xfers, do not have CONFIG_MSPI_ASYNC set to y in their KConfig files.
Consequently, making API callback testing dependent on CONFIG_MSPI_ASYNC may reduce the scope of testing these drivers.
Due to the above, I changed the test implementation.

That is when explicitly making a transfer with mspi_transcieve. In fact, there could be other situation where a callback need to be registered. i.e. fault or XIP related events. But I do acknowledge that the test should be constrained if a controller does not support callback of any kind.
I think we have two options here.

  1. Add a CONFIG_MSPI_CALLBACK to the Kconfig
  2. Make the test skip only -ENOTSUP return value and still pass for others

Your recent edit matches the second option, it is not perfect but can work.
I wonder what other people think.
@jaz1-nordic Nordic controller support no callbacks? No hardware interrupts of any kind?

@swift-tk
Copy link
Collaborator

@danieldegrasse @erwango Does NXP and ST controller support any callbacks? And of what type?

@danieldegrasse
Copy link
Collaborator

@danieldegrasse @erwango Does NXP and ST controller support any callbacks? And of what type?

In theory the NXP FLEXSPI controller supports interrupt driven operation (or DMA). We could issue callbacks for several different device-specific errors, as well as a callback for transfer completion.

However in practice, we likely won't enable that in many cases- the architecture of most chips using the FLEXSPI is such that they execute from the same QSPI flash that the FLEXSPI is managing, so when a page program command is sent only code executing in RAM can run until the page program completes (because the QSPI chip will be unresponsive to reads). This means we will likely only implement blocking reads in the initial version of the MSPI driver for the FLEXSPI.

@swift-tk
Copy link
Collaborator

swift-tk commented Dec 4, 2024

@danieldegrasse @erwango Does NXP and ST controller support any callbacks? And of what type?

In theory the NXP FLEXSPI controller supports interrupt driven operation (or DMA). We could issue callbacks for several different device-specific errors, as well as a callback for transfer completion.

However in practice, we likely won't enable that in many cases- the architecture of most chips using the FLEXSPI is such that they execute from the same QSPI flash that the FLEXSPI is managing, so when a page program command is sent only code executing in RAM can run until the page program completes (because the QSPI chip will be unresponsive to reads). This means we will likely only implement blocking reads in the initial version of the MSPI driver for the FLEXSPI.

Thanks!! In the case of page program that typically transfers <=256 bytes, it may be ok to be blocking. I'd like to point out that the controller driver shouldn't be skewed towards a specific use i.e. flash. And there may be value to also use dma in synchronous operations which could free up CPU while waiting for transfer complete.

@danieldegrasse
Copy link
Collaborator

Thanks!! In the case of page program that typically transfers <=256 bytes, it may be ok to be blocking. I'd like to point out that the controller driver shouldn't be skewed towards a specific use i.e. flash. And there may be value to also use dma in synchronous operations which could free up CPU while waiting for transfer complete.

Sure, longer term the driver would likely (and should) support DMA/Interrupts. Just noting that the first iteration will probably be based around supporting flash

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.

6 participants