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: 27

Inputs:

Sources:

sdk-nrf: PR head: 0720d3ca57c60bb813d4f8e14826b7e827c071f2

more details

sdk-nrf:

PR head: 0720d3ca57c60bb813d4f8e14826b7e827c071f2
merge base: 1d5fa93d98afc9d70656ac6b9311f01180d94a46
target head (main): 762492f3d6d86efb602182ce86ce21a3be588260
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
- sdk-nrf test count: 42
✅ 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

@@ -96,6 +96,7 @@ typedef struct {

 typedef struct {
 	nrfe_mspi_opcode_t opcode; /* Same as application's request. */
+	uint32_t reserved : 24; /* Reserved in order to align data to 32 bits. */


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

@@ -731,7 +745,7 @@ static int nrfe_mspi_init(const struct device *dev)
 		.callback = flpr_fault_handler,
 		.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

+		/* This is to avoid writing outside of data buffer in case when buffer_length%4 !=
+		 * 0.
+		 */
+		switch (NRFX_CEIL_DIV(last_word_bits - penultimate_word_shift, BITS_IN_BYTE)) {


Needs default.

Changed switch to for

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

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

+		if ((penultimate_word_shift != 0) && (i > 1)) {
+			penultimate_word = data[i - 2];
+
+			penultimate_word = (penultimate_word >> (penultimate_word_shift)) |


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

+	uint32_t last_word = 0;
+	uint32_t penultimate_word = 0;
+	uint16_t prev_out = 0;
+	uint16_t cnt0 = 0;
+	uint16_t cnt0_prev = 0;


Can be only declared here and defined later?

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

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

+
+		/* Wait for reception to end, transfer has to be stopped "manualy".
+		 * Setting SHIFTCTRLB cannot be used because with RTPERIPHCTRL:STOPCOUNTERS == 1,
+		 * counter stops 1 clock to early.


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

+#define STD_PAD_BIAS_CNT0_THRESHOLD 1 /* 32MHz */
+#define RX_CNT0_MIN_VALUE 2 /* 21.333333MHz */


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

-			memcpy((void *)packet->data_buf, (void *)ipc_receive_buffer, ipc_received);
+	if (packet->dir == MSPI_RX) {
+#ifdef CONFIG_MSPI_NRFE_IPC_NO_COPY
+		/* Not aligned buffer. */


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

+#define STD_PAD_BIAS_CNT0_THRESHOLD 1 /* 32MHz */
+#define RX_CNT0_MIN_VALUE 2 /* 21.333333MHz */


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 <michal.frankiewicz@nordicsemi.no>

Modified return buffer alignment. Signed-off-by: Michal Frankiewicz <michal.frankiewicz@nordicsemi.no>

Added no copy functionality to RX. Signed-off-by: Michal Frankiewicz <michal.frankiewicz@nordicsemi.no>

Increased partition size for flipper code. Signed-off-by: Michal Frankiewicz <michal.frankiewicz@nordicsemi.no>

sonarqubecloud · 2025-03-21T16:29:07Z

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

Open

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 added 6 commits March 21, 2025 17:19

applications: sdp: mspi: RX Flow change

0f5ed6d

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 <michal.frankiewicz@nordicsemi.no>

drivers: mspi: RX Flow change

e4b4c26

Modified return buffer alignment. Signed-off-by: Michal Frankiewicz <michal.frankiewicz@nordicsemi.no>

applications: sdp: mspi: RX no copy

37b5e9f

Added no copy functionality to RX. Signed-off-by: Michal Frankiewicz <michal.frankiewicz@nordicsemi.no>

drivers: mspi: RX no copy - drivers

9938580

Added no copy functionality to RX. Signed-off-by: Michal Frankiewicz <michal.frankiewicz@nordicsemi.no>

applications: sdp: mspi: Code size

6e4dd4e

Increased partition size for flipper code. Signed-off-by: Michal Frankiewicz <michal.frankiewicz@nordicsemi.no>

snippets: sdp: mspi: Code size

0720d3c

Increased partition size for flipper code. Signed-off-by: Michal Frankiewicz <michal.frankiewicz@nordicsemi.no>

mif1-nordic force-pushed the rx_fLow branch from de9df28 to 0720d3c Compare March 21, 2025 16:22

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Rx f low #20731

Rx f low #20731

mif1-nordic commented Mar 4, 2025

NordicBuilder commented Mar 4, 2025 •

edited

Loading

github-actions bot commented Mar 4, 2025 •

edited

Loading

mif1-nordic left a comment

jaz1-nordic Mar 13, 2025

mif1-nordic Mar 18, 2025

mif1-nordic Mar 21, 2025

jaz1-nordic Mar 18, 2025

mif1-nordic Mar 18, 2025

jaz1-nordic Mar 18, 2025

mif1-nordic Mar 18, 2025

jaz1-nordic Mar 19, 2025 •

edited

Loading

mif1-nordic Mar 20, 2025

masz-nordic Mar 19, 2025

mif1-nordic Mar 20, 2025

masz-nordic Mar 19, 2025

mif1-nordic Mar 20, 2025

masz-nordic Mar 19, 2025

mif1-nordic Mar 20, 2025

masz-nordic Mar 20, 2025

mif1-nordic Mar 20, 2025

masz-nordic Mar 20, 2025

mif1-nordic Mar 20, 2025

magp-nordic Mar 20, 2025

masz-nordic Mar 20, 2025

mif1-nordic Mar 21, 2025

magp-nordic Mar 20, 2025

magp-nordic left a comment •

edited

Loading

sonarqubecloud bot commented Mar 21, 2025

		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.

		#define STD_PAD_BIAS_CNT0_THRESHOLD 1 /* 32MHz */
		#define RX_CNT0_MIN_VALUE 2 /* 21.333333MHz */

Rx f low #20731

Are you sure you want to change the base?

Rx f low #20731

Conversation

mif1-nordic commented Mar 4, 2025

NordicBuilder commented Mar 4, 2025 • edited Loading

CI Information

Inputs:

Sources:

Github labels

Outputs:

Toolchain

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

github-actions bot commented Mar 4, 2025 • edited Loading

mif1-nordic left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jaz1-nordic Mar 19, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

magp-nordic left a comment • edited Loading

Choose a reason for hiding this comment

sonarqubecloud bot commented Mar 21, 2025

Quality Gate passed

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