Rx f low #20731

mif1-nordic · 2025-03-04T15:37:45Z

depends on #20602

NordicBuilder · 2025-03-04T15:40:29Z

CI Information

^{To view the history of this post, clich the 'edited' button above}
Build number: 30

Inputs:

Sources:

sdk-nrf: PR head: eb9018bfd298c4fda423fab402454d8981c29ed9

more details

sdk-nrf:

PR head: eb9018bfd298c4fda423fab402454d8981c29ed9
merge base: 1d5fa93d98afc9d70656ac6b9311f01180d94a46
target head (main): 648c06e61f46d5f2f56ff94db222762c663fd519
Diff

Github labels

Enabled	Name	Description
	ci-disabled	Disable the ci execution
	ci-all-test	Run all of ci, no test spec filtering will be done
	ci-force-downstream	Force execution of downstream even if twister fails
	ci-run-twister	Force run twister
	ci-run-zephyr-twister	Force run zephyr twister

List of changed files detected by CI (8)

applications
│  ├── sdp
│  │  ├── mspi
│  │  │  ├── boards
│  │  │  │  │ nrf54l15dk_nrf54l15_cpuflpr.overlay
│  │  │  ├── src
│  │  │  │  ├── hrt
│  │  │  │  │  ├── hrt-nrf54l15.s
│  │  │  │  │  ├── hrt.c
│  │  │  │  │  │ hrt.h
│  │  │  │  │ main.c
drivers
│  ├── mspi
│  │  │ mspi_nrfe.c
include
│  ├── drivers
│  │  ├── mspi
│  │  │  │ nrfe_mspi.h
snippets
│  ├── sdp
│  │  ├── mspi
│  │  │  ├── soc
│  │  │  │  │ nrf54l15_cpuapp.overlay

Outputs:

Toolchain

Version: 4ffa2202d5
Build docker image: docker-dtr.nordicsemi.no/sw-production/ncs-build:4ffa2202d5_e579f9fbe6

Test Spec & Results: ✅ Success; ❌ Failure; 🟠 Queued; 🟡 Progress; ◻️ Skipped; ⚠️ Quarantine

◻️ Toolchain - Skipped: existing toolchain is used
✅ Build twister - Skipped: Skipping Build & Test as it succeeded in a previous run: 29
✅ Integration tests
- ✅ test-low-level

Disabled integration tests

- desktop52_verification
- doc-internal
- test-fw-nrfconnect-apps
- test-fw-nrfconnect-ble_samples
- test-fw-nrfconnect-boot
- test-fw-nrfconnect-chip
- test-fw-nrfconnect-fem
- test-fw-nrfconnect-nfc
- test-fw-nrfconnect-nrf-iot_cloud
- test-fw-nrfconnect-nrf-iot_libmodem-nrf
- test-fw-nrfconnect-nrf-iot_mosh
- test-fw-nrfconnect-nrf-iot_positioning
- test-fw-nrfconnect-nrf-iot_samples
- test-fw-nrfconnect-nrf-iot_serial_lte_modem
- test-fw-nrfconnect-nrf-iot_thingy91
- test-fw-nrfconnect-nrf-iot_zephyr_lwm2m
- test-fw-nrfconnect-nrf_crypto
- test-fw-nrfconnect-proprietary_esb
- test-fw-nrfconnect-ps
- test-fw-nrfconnect-rpc
- test-fw-nrfconnect-rs
- test-fw-nrfconnect-tfm
- test-fw-nrfconnect-thread
- test-fw-nrfconnect-zigbee
- test-sdk-audio
- test-sdk-dfu
- test-sdk-find-my
- test-sdk-mcuboot
- test-sdk-pmic-samples
- test-sdk-wifi
- test-secdom-samples-public

Note: This message is automatically posted and updated by the CI

github-actions · 2025-03-04T15:57:04Z

You can find the documentation preview for this PR here.

drivers/mspi/mspi_nrfe.c

mif1-nordic

reviewed during meeting

applications/sdp/mspi/src/hrt/hrt.c

applications/sdp/mspi/src/main.c

applications/sdp/mspi/src/hrt/hrt.c

applications/sdp/mspi/src/hrt/hrt.h

drivers/mspi/mspi_nrfe.c

applications/sdp/mspi/src/hrt/hrt.c

jaz1-nordic · 2025-03-13T14:40:43Z

include/drivers/mspi/nrfe_mspi.h

I'm not sure if this will work correctly without the __packed attribute on every compiler.

changed opcode to be uint32_t

cnahged nrfe_mspi_opcode_t size to 32 bits

jaz1-nordic · 2025-03-18T09:07:12Z

drivers/mspi/mspi_nrfe.c

 		.user_data = NULL,
 		.flags = 0,
-		.ticks = counter_us_to_ticks(flpr_fault_timer, CONFIG_MSPI_NRFE_FAULT_TIMEOUT)
+		.ticks = counter_us_to_ticks(flpr_fault_timer, CONFIG_MSPI_NRFE_FAULT_TIMEOUT),


Don't use auto formatting on the whole file. Only use it on the parts of the file that you have changed.
-> The rule is to not add such unnecessary changes to the PR. In the long run it disrupts/makes it harder to merge the whole file with the code of others.

jaz1-nordic · 2025-03-18T09:10:16Z

include/drivers/mspi/nrfe_mspi.h

 	nrfe_mspi_opcode_t opcode; /* Same as application's request. */
+	unsigned reserved : 24; /* Reserved to ensure proper alignment of data field. */


Suggested change

nrfe_mspi_opcode_t opcode; /* Same as application's request. */

unsigned reserved : 24; /* Reserved to ensure proper alignment of data field. */

nrfe_mspi_opcode_t opcode : 32; /* Same as application's request. */

What about this?

this does not compile

I confirm, it does not compile due to the added short-enums compiler flag. Another solution is to use a union. This is a transparent solution, and it causes the field size to be correct and the data is shifted by 32 bits.

typedef struct {
union {
nrfe_mspi_opcode_t opcode; /* Same as application's request. */
unsigned : 32;
};
uint8_t data;
} nrfe_mspi_flpr_response_msg_t;

But I think the least intrusive thing to do would be to simply add the value NRFE_MSPI_OPCODES_MAX = 0xFFFFFFFF at the end of nrfe_mspi_opcode_t, which would force the enum size to 32bits.

fixed using this solution: NRFE_MSPI_OPCODES_MAX = 0xFFFFFFFF

masz-nordic · 2025-03-19T15:54:30Z

applications/sdp/mspi/src/hrt/hrt.c

Needs default.

Changed switch to for

masz-nordic · 2025-03-19T15:54:44Z

applications/sdp/mspi/src/hrt/hrt.c

Suggested change

penultimate_word = (penultimate_word >> (penultimate_word_shift)) |

penultimate_word = (penultimate_word >> penultimate_word_shift) |

masz-nordic · 2025-03-19T16:06:06Z

applications/sdp/mspi/src/hrt/hrt.c

Can be only declared here and defined later?

masz-nordic · 2025-03-20T11:33:44Z

applications/sdp/mspi/src/hrt/hrt.c

Suggested change

* counter stops 1 clock to early.

* counter stops 1 clock too early.

masz-nordic · 2025-03-20T11:35:01Z

applications/sdp/mspi/src/main.c

Please add comments describing those defines.

I think we can limit the max possible freq to 21 MHz on the APP side, but can be left as is for now.

masz-nordic · 2025-03-20T11:41:52Z

drivers/mspi/mspi_nrfe.c

Please describe in comment how this is different from alignment done above.

applications/sdp/mspi/src/main.c

applications/sdp/mspi/src/hrt/hrt.c

applications/sdp/mspi/src/main.c

magp-nordic · 2025-03-20T15:42:26Z

applications/sdp/mspi/src/main.c

I think we can limit the max possible freq to 21 MHz on the APP side, but can be left as is for now.

applications/sdp/mspi/src/hrt/hrt.c

drivers/mspi/mspi_nrfe.c

magp-nordic

Overall LGTM, just the recent changes need to be pushed.

Modified hrt rx flow to use write function for command, address and dummy cycles. Modified Rx function to work in 32 bit mode instead of 8 bit. Signed-off-by: Michal Frankiewicz <[email protected]>

Modified return buffer alignment. Signed-off-by: Michal Frankiewicz <[email protected]>

Added no copy functionality to RX. Signed-off-by: Michal Frankiewicz <[email protected]>

Increased partition size for flipper code. Signed-off-by: Michal Frankiewicz <[email protected]>

sonarqubecloud · 2025-03-24T08:03:08Z

Quality Gate passed

Issues
5 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

mif1-nordic requested review from a team as code owners March 4, 2025 15:37

github-actions bot added the changelog-entry-required Update changelog before merge. Remove label if entry is not needed or already added. label Mar 4, 2025

masz-nordic added this to the 3.0.0 milestone Mar 6, 2025

mif1-nordic force-pushed the rx_fLow branch 6 times, most recently from 3b6d9cf to 3a72ed1 Compare March 11, 2025 08:57

mif1-nordic mentioned this pull request Mar 11, 2025

Rx dummy cycles #20309

Closed

mif1-nordic force-pushed the rx_fLow branch 6 times, most recently from 7d6bc30 to 27c9038 Compare March 12, 2025 15:05

mif1-nordic requested a review from a team as a code owner March 12, 2025 15:05

nordic-segl reviewed Mar 12, 2025

View reviewed changes

drivers/mspi/mspi_nrfe.c Outdated Show resolved Hide resolved

mif1-nordic force-pushed the rx_fLow branch 2 times, most recently from 22b33e4 to 3970be6 Compare March 13, 2025 12:01

masz-nordic mentioned this pull request Mar 13, 2025

sdp: switch to Quad mode by default #20807

Merged

mif1-nordic force-pushed the rx_fLow branch 2 times, most recently from 3d7957c to 2499e70 Compare March 13, 2025 13:46

mif1-nordic commented Mar 14, 2025

View reviewed changes

mif1-nordic force-pushed the rx_fLow branch 2 times, most recently from 5278151 to 6b7ac9f Compare March 17, 2025 16:39

mif1-nordic force-pushed the rx_fLow branch 2 times, most recently from 2b7e509 to f160adf Compare March 18, 2025 08:22

nordicjm reviewed Mar 18, 2025

View reviewed changes

applications/sdp/mspi/src/hrt/hrt.c Outdated Show resolved Hide resolved

applications/sdp/mspi/src/hrt/hrt.c Outdated Show resolved Hide resolved

applications/sdp/mspi/src/hrt/hrt.c Outdated Show resolved Hide resolved

applications/sdp/mspi/src/hrt/hrt.c Outdated Show resolved Hide resolved

jaz1-nordic reviewed Mar 18, 2025

View reviewed changes

mif1-nordic force-pushed the rx_fLow branch 2 times, most recently from dac73d9 to de9df28 Compare March 19, 2025 09:15

masz-nordic reviewed Mar 20, 2025

View reviewed changes

magp-nordic reviewed Mar 20, 2025

View reviewed changes

magp-nordic approved these changes Mar 21, 2025

View reviewed changes

mif1-nordic force-pushed the rx_fLow branch 2 times, most recently from 0720d3c to bc42eff Compare March 24, 2025 07:54

mif1-nordic added 6 commits March 24, 2025 08:56

applications: sdp: mspi: RX Flow change

4fd2edc

Modified hrt rx flow to use write function for command, address and dummy cycles. Modified Rx function to work in 32 bit mode instead of 8 bit. Signed-off-by: Michal Frankiewicz <[email protected]>

drivers: mspi: RX Flow change

b69e516

Modified return buffer alignment. Signed-off-by: Michal Frankiewicz <[email protected]>

applications: sdp: mspi: RX no copy

2f0d4ce

Added no copy functionality to RX. Signed-off-by: Michal Frankiewicz <[email protected]>

drivers: mspi: RX no copy - drivers

6177bbe

Added no copy functionality to RX. Signed-off-by: Michal Frankiewicz <[email protected]>

applications: sdp: mspi: Code size

c6eddf1

Increased partition size for flipper code. Signed-off-by: Michal Frankiewicz <[email protected]>

snippets: sdp: mspi: Code size

eb9018b

Increased partition size for flipper code. Signed-off-by: Michal Frankiewicz <[email protected]>

mif1-nordic force-pushed the rx_fLow branch from bc42eff to eb9018b Compare March 24, 2025 07:57

masz-nordic approved these changes Mar 24, 2025

View reviewed changes

masz-nordic requested a review from nordicjm March 24, 2025 11:17

nordicjm approved these changes Mar 24, 2025

View reviewed changes

masz-nordic merged commit 9255061 into nrfconnect:main Mar 24, 2025
14 checks passed

		nrfe_mspi_opcode_t opcode; /* Same as application's request. */
		unsigned reserved : 24; /* Reserved to ensure proper alignment of data field. */

	nrfe_mspi_opcode_t opcode; /* Same as application's request. */
	unsigned reserved : 24; /* Reserved to ensure proper alignment of data field. */
	nrfe_mspi_opcode_t opcode : 32; /* Same as application's request. */

	penultimate_word = (penultimate_word >> (penultimate_word_shift)) \|
	penultimate_word = (penultimate_word >> penultimate_word_shift) \|

	* counter stops 1 clock to early.
	* counter stops 1 clock too early.

Rx f low #20731

Rx f low #20731

Uh oh!

Conversation

mif1-nordic commented Mar 4, 2025

Uh oh!

NordicBuilder commented Mar 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

CI Information

Inputs:

Sources:

Github labels

Outputs:

Toolchain

Test Spec & Results: ✅ Success; ❌ Failure; 🟠 Queued; 🟡 Progress; ◻️ Skipped; ⚠️ Quarantine

Uh oh!

github-actions bot commented Mar 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

mif1-nordic left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jaz1-nordic Mar 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

NordicBuilder commented Mar 4, 2025 •

edited

Loading

github-actions bot commented Mar 4, 2025 •

edited

Loading

jaz1-nordic Mar 19, 2025 •

edited

Loading

magp-nordic left a comment •

edited

Loading