github-actions[bot]
commented
1 month ago Hello @swift-tk, and thank you very much for your first pull request to the Zephyr project! Our Continuous Integration pipeline will execute a series of checks on your Pull Request commit messages and code, and you are expected to address any failures by updating the PR. Please take a look at our commit message guidelines to find out how to format your commit messages, and at our contribution workflow to understand how to update your Pull Request. If you haven't already, please make sure to review the project's Contributor Expectations and update (by amending and force-pushing the commits) your pull request if necessary. If you are stuck or need help please join us on Discord and ask your question there. Additionally, you can escalate the review when applicable. 😊
swift-tk
RichardSWheatley
zephyrbot
AlessandroLuo
aaronyegx
FRASTM
carlescufi
erwango
dleach02
danieldegrasse
Description
This PR primarily forces on creating a generic MSPI(multi-bit SPI) controller API that typically is used when interfacing with high speed pSRAM/flash memory, displays, and sensors devices.
I have finished below items from https://github.com/zephyrproject-rtos/zephyr/issues/70723#issuecomment-2045544160. Also see this issue for more detailed RFC.
Q & A
Q: Why include sensors?
A: The MSPI would still be compatible with generic serial SPI interface and the API is expandable to support RTIO.
Q: Use of Kconfig MSPI_COMPLETION_TIMEOUT_TOLERANCE
A: This is a maximum time allowed for a transfer regardless of error or large data size. The unit is not yet determined, maybe in milli-seconds.
Q: Why won't we include a parameter for the maximum time to wait for the next transfer.
A: MSPI only cares about the transfer, it is all about the transfer. It does not care what happens in between transfers. Unlike SDIO, the protocol dictates a controlled flow and that there is a response for every command. This is not true for SPI. The MSPI controller can't be responsible for knowing the internal state of specific devices. This is the responsibility of the specific device driver.
Q: Purpose of
struct mspi_dev_cfgA: This contains the device specific hardware controller settings. However, they are device properties and should be derived from device datasheets. While often time the value of its member may frequently change, the settings derived from DTS that filled within the struct should represent the most frequently used settings during run-time. When value of its member changes, the mspi_dev_config API should be used for device operation mode switching.
For future enhancement, enhanced XIP or DMA that requires sending only cmd and address one time may be supported but will not be in the context of this PR.
Q: Support for memory mapped device
A: The XIP address should be obtained from devicetree memory regions. For multi-slave scenario, each device's mapped address may be determined by the start address along with device index and its size. This can be done within the controller or pass by
struct mspi_xip_configGoing to defer code linking from this PR as it is more of kernel enhancement and also for the fact that we may need more time to think about it.
Q: Reasons when choosing the member of
struct mspi_xferA:
struct mspi_dev_cfg. For the fact that, devices may require different ways to manage operation modes and controller hardware/hal implementation differences. The purpose of any high speed MSPI implementation should be data streaming as fast as possible so it doesn't seem necessary to include mode switching in this level of API while software queue like RTIO might be more appropriate when 90% of the time during run-time should require no mode switching.Q: MSPI slave usage
A: When an MSPI controller act as a slave, it is essentially an MSPI device. A slave device driver could be created for this purpose to interface with the existing controller driver and the APIs would still apply. But there is no plan to show an implementation of a mspi slave in this PR and the details may subject to change.
If you think there might be a conflict with the current proposal, please point it out. At this point, the use of MSPI slave may be very specific and uncommon.
Action items:
[x] To add an additional timeout parameter in
struct mspi_xferto configure a time out value per transfer, this value should be smaller than CONFIG_MSPI_COMPLETION_TIMEOUT_TOLERANCE. Left the timeout as constant in device drivers for the time being.[x] To expand
mspi_data_rateas devices may switch between 1s-4s-4s, 1s-4s-4d and 1s-4d-4d, and all other possible combinations. It now has four types: S_S_S, S_S_D, S_D_D, D_D_D. In combination withmspi_io_mode, more transfer types can be supported.[ ] Would like to consolidate on API return values
mspi_ambiq_config.-ENOTSUPfor when member ofmspi_cfgvalue is not supported by the controller.-EHOSTDOWNfor HAL return fail.-EPERMfor fail to initialize because the controller has already been initialized.mspi_dev_config-EBUSYfor when the controller is busy due to in transfer or gaining access mutex time out.-ENODEVfor failing to validatemspi_dev_id.-ENOTSUPfor when member ofmspi_dev_cfgormspi_dev_cfg_masknot supported by the controller.-EHOSTDOWNfor HAL return fail.mspi_xip_config-ESTALEfor when the request is denied as it doesn't own access mutex.-EHOSTDOWNfor HAL return fail.-EBUSYfor when the controller is still busy processing transfers and does not allow configuration.mspi_scramble_config-ESTALEfor when the request is denied as it doesn't own access mutex.-EHOSTDOWNfor HAL return fail.-EBUSYfor when the controller is still busy processing transfers and does not allow configuration.mspi_timing_config-ESTALEfor when the request is denied as it doesn't own access mutex.-EHOSTDOWNfor HAL return fail.-EBUSYfor when the controller is still busy processing transfers and does not allow configuration.-ENOTSUPfor when the parameters not supported by the controller.mspi_get_channel_status-EBUSYfor when the controller is still busy processing transfermspi_transceive-ESTALEfor when the request is denied as it doesn't own access mutex.-EIOfor when IO error occurs-EFAULTfor incorrectmspi_xferparameters such as empty buffer-EBUSYfor timeout when gaining the the transfer context lock.-ENOMEMfor when the hardware command queue is full.[ ] Consider how to properly link code/data from MSPI XIP region, especially in multi-slave scenario. This https://github.com/zephyrproject-rtos/zephyr/pull/68888 can work, however, there might be some unknown side effects when moving to PRE-KERNEL.