SDP MSPI driver #18893
SDP MSPI driver #18893
Conversation
github-actions
bot
added
manifest
changelog-entry-required
|
The following west manifest projects have changed revision in this Pull Request:
⛔ DNM label due to: 1 project with PR revision Note: This message is automatically posted and updated by the Manifest GitHub Action. |
CI InformationTo view the history of this post, clich the 'edited' button above Inputs:Sources:sdk-nrf: PR head: 167eea08bb5e67b603abaaadfa84ac7c3a69cff7 more detailssdk-nrf:
zephyr:
Github labels
List of changed files detected by CI (28)Outputs:ToolchainVersion: b77d8c1312 Test Spec & Results: ✅ Success; ❌ Failure; 🟠 Queued; 🟡 Progress; ◻️ Skipped;
|
|
You can find the documentation preview for this PR at this link. It will be updated about 10 minutes after the documentation build succeeds. Note: This comment is automatically posted by the Documentation Publishing GitHub Action. |
jaz1-nordic
force-pushed
the
nrfx-6633_mSPI_SDP
branch
2 times, most recently
from
e660c0e to
04e12ea
Compare
github-actions
bot
removed
the
changelog-entry-required
jaz1-nordic
force-pushed
the
nrfx-6633_mSPI_SDP
branch
from
04e12ea to
55aedeb
Compare
|
You can find the documentation preview for this PR at this link. It will be updated about 10 minutes after the documentation build succeeds. Note: This comment is automatically posted by the Documentation Publish GitHub Action. |
jaz1-nordic
force-pushed
the
nrfx-6633_mSPI_SDP
branch
3 times, most recently
from
91bb39f to
c2e11f9
Compare
Memory footprint analysis revealed the following potential issuessample.matter.template.debug[nrf7002dk/nrf5340/cpuapp]: High ROM usage: 911938[B] - link (cc: @kkasperczyk-no @ArekBalysNordic @markaj-nordic) Note: This message is automatically posted and updated by the CI (latest/sdk-nrf/PR-18893/28) |
jaz1-nordic
force-pushed
the
nrfx-6633_mSPI_SDP
branch
from
c2e11f9 to
eedb55d
Compare
jaz1-nordic
force-pushed
the
nrfx-6633_mSPI_SDP
branch
5 times, most recently
from
2df40f2 to
81dc879
Compare
jaz1-nordic
force-pushed
the
nrfx-6633_mSPI_SDP
branch
3 times, most recently
from
3b113cd to
2afc4b7
Compare
jaz1-nordic
force-pushed
the
nrfx-6633_mSPI_SDP
branch
2 times, most recently
from
7b4e1bd to
98f3508
Compare
jaz1-nordic
force-pushed
the
nrfx-6633_mSPI_SDP
branch
3 times, most recently
from
47238ee to
5b6c540
Compare
scripts/twister/alt/zephyr/tests/drivers/mspi/api/testcase.yaml
Outdated
Show resolved
Hide resolved
jaz1-nordic
force-pushed
the
nrfx-6633_mSPI_SDP
branch
3 times, most recently
from
fd6a31a to
bb4ab6a
Compare
jaz1-nordic
force-pushed
the
nrfx-6633_mSPI_SDP
branch
3 times, most recently
from
bfa93e8 to
c4b8b0c
Compare
| case NRFE_MSPI_TXRX: { | ||
| if (len) { | ||
| ipc_received = len - 1; | ||
| ipc_receive_buffer = (uint8_t *)&response->data; |
There was a problem hiding this comment.
It seems a bit too much to save an address of one uint8_t, instead of copying it, especially because the pointer will most likely be of 32-bit size. So instead of creating one uint8_t, we create a uint32_t.
Unless it is supposed to be a preparation for receiving more data from FLPR. In that case, it can be left as it is.
| return -EINVAL; | ||
| } | ||
|
|
||
| return xfer_packet(packet, xfer->timeout); |
There was a problem hiding this comment.
If this is the only place xfer_packet function is used, then maybe xfer_packet should be deleted and its body pasted here?
| #define D2_PIN 3 | ||
| #define D3_PIN 4 | ||
| #define CS_PIN 5 | ||
| #include <drivers/mspi/nrfe_mspi.h> |
There was a problem hiding this comment.
I think there could be a separate header file for pin numbers/pin translation. I am not a fan of including the whole nrfe_mspi.h here, HRT does not need to know FLPR response struct, for example.
However, I guess the pin numbers/pin translation can be changed in the future when pin checking is added, so it can stay as it is for now.
| typedef struct __packed { | ||
| uint8_t opcode; | ||
| struct mspi_cfg cfg; | ||
| } nrfe_mspi_cfg_t; | ||
|
|
||
| typedef struct __packed { | ||
| uint8_t opcode; | ||
| struct mspi_dev_cfg cfg; | ||
| } nrfe_mspi_dev_cfg_t; | ||
|
|
||
| typedef struct __packed { | ||
| uint8_t opcode; | ||
| struct mspi_xfer xfer; | ||
| } nrfe_mspi_xfer_t; | ||
|
|
||
| typedef struct __packed { | ||
| uint8_t opcode; | ||
| struct mspi_xfer_packet packet; | ||
| } nrfe_mspi_xfer_packet_t; |
There was a problem hiding this comment.
Nit: If these structs have to be defined anyway, in order to have the same enum sizes on APP and FLPR, then why not define them in nrfe_mspi.h and use them on APP side too while sending? Alternatively, a structure with union of Zephyr structures can be defined:
| typedef struct __packed { | |
| uint8_t opcode; | |
| struct mspi_cfg cfg; | |
| } nrfe_mspi_cfg_t; | |
| typedef struct __packed { | |
| uint8_t opcode; | |
| struct mspi_dev_cfg cfg; | |
| } nrfe_mspi_dev_cfg_t; | |
| typedef struct __packed { | |
| uint8_t opcode; | |
| struct mspi_xfer xfer; | |
| } nrfe_mspi_xfer_t; | |
| typedef struct __packed { | |
| uint8_t opcode; | |
| struct mspi_xfer_packet packet; | |
| } nrfe_mspi_xfer_packet_t; | |
| typedef union __packed { | |
| struct mspi_cfg cfg; | |
| struct mspi_dev_cfg dev_cfg; | |
| struct mspi_xfer xfer; | |
| struct mspi_xfer_packet packet; | |
| } nrfe_mspi_driver_data; | |
| typedef struct __packed { | |
| uint8_t opcode; | |
| union nrfe_mspi_driver_data data; | |
| } nrfe_mspi_ipc_data_t; |
| * nrfe_mspi_pinctrl_soc_pin_t *pins_cfg = (nrfe_mspi_pinctrl_soc_pin_t *)data; | ||
| * response.opcode = pins_cfg->opcode; | ||
| * | ||
| * for (uint8_t i = 0; i < NRFE_MSPI_PINS_MAX; i++) { | ||
| * uint32_t psel = NRF_GET_PIN(pins_cfg->pin[i]); | ||
| * uint32_t fun = NRF_GET_FUN(pins_cfg->pin[i]); | ||
| * NRF_GPIO_Type *reg = nrf_gpio_pin_port_decode(&psel); | ||
| * } |
There was a problem hiding this comment.
Nit: there should be no commented-out code.
| data_buffer[XFER_COMMAND_IDX] = 0xe5b326c1; | ||
| data_buffer[XFER_ADDRESS_IDX] = 0xaabbccdd; | ||
|
|
There was a problem hiding this comment.
Nit: since sample data is removed, this can also be removed.
Add changes related with SDP MSPI driver API test. Signed-off-by: Jakub Zymelka <[email protected]>
Add SDP MSPI driver, dts and include files. Signed-off-by: Jakub Zymelka <[email protected]>
Add new snippet files for SDP MSPI. Signed-off-by: Jakub Zymelka <[email protected]>
Add cmake files to be able to include the SDP MSPI application in solutions where SDP MSPI is required. Signed-off-by: Jakub Zymelka <[email protected]>
Add IPC solution based on icmsg to application for FLPR core to communicate with SDP MSPI driver. Signed-off-by: Jakub Zymelka <[email protected]>
Add yaml configuration to enable SDP MSPI API test. Signed-off-by: Jakub Zymelka <[email protected]>
masz-nordic
force-pushed
the
nrfx-6633_mSPI_SDP
branch
from
c4b8b0c to
167eea0
Compare
SDP MSPI driver