driver: flash_mspi_nor: Allow specific read/write IO modes and frequencies #97504

utsavm9 · 2025-10-14T00:11:36Z

Consider these MSPI flash parts: Gigadevice GD25LE64E, Macronix MX25U6432F, Winbond W25Q64JW. If only D0 and D1 lines are connected to these flash parts, then Dual IO 1-1-2 DREAD 0x3B and Single IO PP 0x02 are the best read and write commands available to use. Notice that neither Dual IO PP_1_1_2 0xA2 command is supported in any of these flash parts, and neither these flash parts support Dual Peripheral Interface (like QPI, OPI), so all general commands need to be in Single IO.

The driver currently provides no way to use Dual IO Read and Single IO for the rest of the commands currently, and would erroneously use Single IO PP command in Dual IO mode. This PR fixes and adds support for that.

Target Board

nRF54H20 MSPI and flash driver to support Gigadevice GD25LE64E, Macronix MX25U6432F and Winbond W25Q64JW.

Modifications

JEDEC standard specifies different commands for read/writes in different MSPI IO modes. Driver should let device tree specify which IO mode and frequency to use for read and writes, defaulting to the overall IO mode and frequency when not specified.
With different IO modes using different number of lines, one mspi-max-frequency is not the highest possible frequency to use for all modes. Allow read/write frequency to be granularly specified.

Testing

The following modes for flash driver worked in local testing:

Single IO for everything

mspi-io-mode = "MSPI_IO_MODE_SINGLE";
mspi-max-frequency = <DT_FREQ_M(66)>;

Single IO general and write, Dual IO reads

mspi-io-mode = "MSPI_IO_MODE_SINGLE";
mspi-max-frequency = <DT_FREQ_M(66)>;
read-command = <0x3B>; // DREAD Dual IO 1-1-2
read-io-mode = "MSPI_IO_MODE_DUAL_1_1_2";
read-frequency = <DT_FREQ_M(40)>;

Quad IO for everything, Single IO for writes

mspi-io-mode = "MSPI_IO_MODE_QUAD_1_1_4";
mspi-max-frequency = <DT_FREQ_M(40)>;
read-command = <0x6B>; // QREAD Quad IO 1-1-4
read-io-mode = "MSPI_IO_MODE_QUAD_1_1_4";
write-command = <0x02>; // PP Single IO
write-io-mode = "MSPI_IO_MODE_SINGLE";
write-frequency = <DT_FREQ_M(66)>;

utsavm9 · 2025-10-14T01:40:43Z

Request review from @anangl too

swift-tk

You are adding write/read frequencies but not write/read io_modes to struct mspi_dev_cfg, why?
Anyway, I don't think this is the correct approach.
Please also include the specific devices in the description you are targeting so we can check the datasheets.

I do understand there might be a need for some flash devices that have different maximum frequency for write and read operation and even different io mode. But changing the mspi scope seems wrong, so maybe just add bindings to the flash scope(jedec,mspi-nor).

utsavm9 · 2025-10-14T03:37:34Z

You are adding write/read frequencies but not write/read io_modes to struct mspi_dev_cfg, why?

@swift-tk The read-command and write-command themselves convey the IO mode. The problem with the driver right now is that it would follow Dual IO from mspi-io-mode even when it detects from SFDP-BPT that only Single IO writes are supported and use the Single IO command in Dual IO mode.

I can add mspi-read-io-mode too if that is the preference, but is inferring that from the command not ideal?
I noticed read-command themselves were present in MSPI device binding instead of MSPI Flash NOR bindings, so I followed that, but if that is not the preference, I can move it to MSPI Flash bindings.

Any other changes that you would recommend to add this support?

swift-tk · 2025-10-15T04:00:11Z

You are adding write/read frequencies but not write/read io_modes to struct mspi_dev_cfg, why?

@swift-tk The read-command and write-command themselves convey the IO mode. The problem with the driver right now is that it would follow Dual IO from mspi-io-mode even when it detects from SFDP-BPT that only Single IO writes are supported and use the Single IO command in Dual IO mode.

You can separate io-mode for write and read but only on the flash device level, do not change the controller API unless this separation is generally appliable among controller IPs. I would also suggest the same for frequencies.

To reiterate, the MSPI controllers should not be liable on their peripheral device states which should be managed by device drivers themselves. When device state changes such as frequency or io mode, the device driver should call mspi_dev_config API to reconfigure the controller.

I can add mspi-read-io-mode too if that is the preference, but is inferring that from the command not ideal? I noticed read-command themselves were present in MSPI device binding instead of MSPI Flash NOR bindings, so I followed that, but if that is not the preference, I can move it to MSPI Flash bindings.

The cmd_to_io_mode translation does not apply to all flash devices does it? I believe some of that requires quirk depending on the flash device you are using.

Remove unnecessary function argument, makes later commits also simpler. Signed-off-by: Utsav Munendra <[email protected]>

Instead of just tracking in a bool whether the MSPI device is in Standard MSPI vs. QPI/OPI config, track the entire MSPI config which was last applied. This makes it easier later to track more than two configs to apply based on the next command to transceive. Signed-off-by: Utsav Munendra <[email protected]>

utsavm9 · 2025-10-15T23:20:19Z

Re-did the PR after previous comments, might have oversimplied the IO mode selection logic while missing some flash mode transition, so requesting careful review. Works for all configs I can apply with my hardware setup. Commits are self-contained to help with review.

Track MSPI XIP config too in order to be able to detect and switch out of this config when needed. Signed-off-by: Utsav Munendra <[email protected]>

Also in preparation for allowing control command frequency to be different from the read/write frequency and initialization frequency. Signed-off-by: Utsav Munendra <[email protected]>

Remove the Boolean tracking of MSPI IO mode as we can now rely on tracking the entire dev config applied to the MSPI device, multiple of which will exist in later commits. Signed-off-by: Utsav Munendra <[email protected]>

The driver currently provides no way to use Dual IO Read and Single IO for the rest of the commands currently, and would erroneously use Single IO PP command in Dual IO mode. This PR fixes and adds support for that. Signed-off-by: Utsav Munendra <[email protected]>

sonarqubecloud · 2025-10-15T23:32:39Z

Quality Gate passed

Issues
0 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

swift-tk · 2025-10-16T07:47:20Z

drivers/flash/flash_mspi_nor.c

-
-	dev_data->in_target_io_mode = false;
+	dev_data->last_applied_cfg = &dev_data->mspi_control_cfg;
+	dev_data->mspi_control_cfg.freq = target_freq;


I don't quite understand the logic here. renaming mspi_nor_init_cfg to mspi_control_cfg, putting it in data, and then changing the freq back and forth..
The original intention is to maintain a lower freq for commands so we don't have to worry about timing issues and to maintain maximum capability for those flash chips that may have lower freq when accessing registers than accessing memory.

Even in the original intention, we switch out of 50MHz by the time switch_to_target_io_mode() was run for control commands too. Renamed mspi_nor_init_cfg to mspi_control_cfg because I want to have a MSPI dev config for all control (not read & write) commands like read status, erase etc. and not for use just during init. Perhaps mspi_base_cfg is a better name?

Moved to data from config because the frequency is changed during runtime and cannot be const anymore. Frequency needs to be <= 50MHz only during initial commands like Read ID because I believe JEDEC compliance mandates supporting upto 50MHz. After init, we should use device tree specified frequency.

The switching to target IO mode means the device is ready to conduct memory access, thus target frequency should be used. The mspi_nor_init_cfg is not only used during init phase but also in cmd phase if register access cmd aren't available in target IO mode.
Maintaining freq <= 50MHz in "cmd mode" should have very limited effect towards bandwidth as the transfers are short. Most of the time is instead spent waiting for and polling the flash status, which dominates the overall latency by orders of magnitude.
Please keep this section the same as before.

If you are suggesting to leave the mspi_nor_init_cfg untouched, that is going to change the behavior of the driver, by the way. Also, in this PR, the target frequency is saved in the config early on, but only applied after switch_to_target_io_mode() for control commands.

Assume Quad IO and not QPI, then all status reg read after initialization happens in mspi_max_frequency currently on mainline and this PR. Using the init config with <= 50MHz freq instead will barely affect bandwidth indeed, but I would lean towards not changing the behavior if there is no reason to.

I will leave mspi_nor_init_cfg untouched if you believe that change in behavior is OK though.

To be clear, mspi_nor_init_cfg and all related change should be reverted. So no need to get the target freq early on.
No, I think below code already guaranteed that reg read would go back to using mspi_nor_init_cfg

if (dev_data->in_target_io_mode) { if (dev_data->packet.num_bytes != 0 || (dev_data->xfer.addr_length != 0 && !dev_config->single_io_addr)) { /* Only the IO mode is to be changed, so the * initial configuration structure can be used * for this operation. */ cfg = &dev_config->mspi_nor_init_cfg; } }

~~I'll revert it.~~ Latest comments seem to approve of this now. Though, the code you highlighted on mainline would only pick Single IO out of mspi_nor_init_cfg to be applied with mspi_dev_config but this PR picks both IO mode and frequency out of the chosen configs, so that will result in init freq being applied which was not the case before.

swift-tk · 2025-10-16T08:44:00Z

drivers/flash/flash_mspi_nor.c

+			cfg = dev_data->write_cfg;
+		} else {
+			/* For all other commands, use control command config */
+			cfg = &dev_data->mspi_control_cfg;


I've lost track, does this change the driver behavior? I think it does...
@anangl

It should not!

All this is wrapped within a !dev_config->multi_io_cmd so drivers set with QPI or OPI as the IO mode skip this config selection logic anyways like mainline. They stay in QPI/OPI forever after initialization.

Mainline drivers currently do not override read and write IO mode, so both read and write configs point to mspi_nor_cfg and that is applied just like on mainline for read and writes.

For all control commands, just select Single IO mode always because codepath is not taken for QPI/OPI anyways. I believe the earlier logic accomplished the same, but instead of always selecting Single IO mode, it was trying to detect when Octal/Quad/Dual IO 1-1-x or 1-x-x specifically would not work.

A change in behavior could be that mspi_dev_config is called more times than before, but the communication with flash should stay the same for a device-tree config as far as I have tried.

In my opinion it's okay as it is now.

Sorry, now as in how it is on mainline or this PR?

In this PR.

swift-tk · 2025-10-16T09:37:35Z

drivers/flash/flash_mspi_nor.c

+	if (cfg && cfg != dev_data->last_applied_cfg) {
 		rc = mspi_dev_config(dev_config->bus, &dev_config->mspi_id,
-				     MSPI_DEVICE_CONFIG_IO_MODE, cfg);
+			MSPI_DEVICE_CONFIG_IO_MODE | MSPI_DEVICE_CONFIG_FREQUENCY, cfg);


okay, I clearly miss it from before. Configuring the freqeuncy here is reasonable and aligned with the original intent.

Yeah, read, write and control commands after this PR could all have different frequencies (needed because of different IO modes) so that's why worked backwards from here for this commit 9d54c63

anangl · 2025-10-16T10:13:24Z

drivers/flash/flash_mspi_nor.c

 			return rc;
 		}
-		dev_data->last_applied_cfg = NULL;
+		dev_data->last_applied_cfg = mspi_cfg;


I don't think this is needed. Neither IO mode nor frequency is reconfigured for XIP (see XIP_DEV_CFG_MASK), so all changes related to XIP can be dropped IMO.

anangl · 2025-10-17T15:01:19Z

drivers/flash/flash_mspi_nor.c

+	/* Do initial checks at max 50MHz required to be supported by JEDEC */
+	dev_data->mspi_control_cfg.freq = MIN(target_freq, MHZ(50));


I'd suggest to not move mspi_control_cfg to dev_data just to be able to do the above modification, but use here a local variable instead:

Suggested change

/* Do initial checks at max 50MHz required to be supported by JEDEC */

dev_data->mspi_control_cfg.freq = MIN(target_freq, MHZ(50));

/* Do initial checks at max 50MHz required to be supported by JEDEC */

init_mspi_control_cfg = dev_config->mspi_control_cfg;

init_mspi_control_cfg.freq = MIN(init_mspi_control_cfg.freq, MHZ(50));

This way we'll avoid forcing the flash_mspi_nor_data structure to be initialized with non-zero content what unnecessarily consumes some flash.

zephyrbot added area: Devicetree area: Boards/SoCs area: Flash area: Devicetree Bindings area: MSPI labels Oct 14, 2025

zephyrbot requested review from de-nordic, decsny, mbolivar, rghaddab, rruuaanng and swift-tk October 14, 2025 00:13

zephyrbot assigned swift-tk Oct 14, 2025

utsavm9 force-pushed the dual_io branch from 38f4dc0 to 288c95b Compare October 14, 2025 01:39

utsavm9 force-pushed the dual_io branch from 288c95b to 62c6d4e Compare October 14, 2025 01:45

swift-tk requested changes Oct 14, 2025

View reviewed changes

utsavm9 added 2 commits October 15, 2025 15:59

drivers: flash_mspi_nor: Simplify perform_xfer function

7dd9611

Remove unnecessary function argument, makes later commits also simpler. Signed-off-by: Utsav Munendra <[email protected]>

utsavm9 force-pushed the dual_io branch from 62c6d4e to f66954b Compare October 15, 2025 23:17

utsavm9 requested a review from swift-tk October 15, 2025 23:18

utsavm9 added 4 commits October 15, 2025 16:23

drivers: flash_mspi_nor: Track XIP config as part of driver data

570d7ce

Track MSPI XIP config too in order to be able to detect and switch out of this config when needed. Signed-off-by: Utsav Munendra <[email protected]>

drivers: flash_mspi_nor: Flash control commands to use their own configs

9d54c63

Also in preparation for allowing control command frequency to be different from the read/write frequency and initialization frequency. Signed-off-by: Utsav Munendra <[email protected]>

drivers: flash_mspi_nor: Remove bool tracking of target IO mode

7560183

Remove the Boolean tracking of MSPI IO mode as we can now rely on tracking the entire dev config applied to the MSPI device, multiple of which will exist in later commits. Signed-off-by: Utsav Munendra <[email protected]>

utsavm9 force-pushed the dual_io branch from f66954b to 049bf4b Compare October 15, 2025 23:27

swift-tk reviewed Oct 16, 2025

View reviewed changes

anangl reviewed Oct 17, 2025

View reviewed changes

		/* Do initial checks at max 50MHz required to be supported by JEDEC */
		dev_data->mspi_control_cfg.freq = MIN(target_freq, MHZ(50));

driver: flash_mspi_nor: Allow specific read/write IO modes and frequencies #97504

Are you sure you want to change the base?

driver: flash_mspi_nor: Allow specific read/write IO modes and frequencies #97504

Conversation

utsavm9 commented Oct 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Target Board

Modifications

Testing

Uh oh!

utsavm9 commented Oct 14, 2025

Uh oh!

swift-tk left a comment • 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!

utsavm9 commented Oct 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

swift-tk commented Oct 15, 2025

Uh oh!

utsavm9 commented Oct 15, 2025

Uh oh!

sonarqubecloud bot commented Oct 15, 2025

Quality Gate passed

Uh oh!

Choose a reason for hiding this comment

Uh oh!

utsavm9 Oct 16, 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!

swift-tk Oct 16, 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!

utsavm9 Oct 16, 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!

utsavm9 Oct 16, 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!

utsavm9 Oct 16, 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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

utsavm9 commented Oct 14, 2025 •

edited

Loading

swift-tk left a comment •

edited

Loading

utsavm9 commented Oct 14, 2025 •

edited

Loading

utsavm9 Oct 16, 2025 •

edited

Loading

swift-tk Oct 16, 2025 •

edited

Loading

utsavm9 Oct 16, 2025 •

edited

Loading

utsavm9 Oct 16, 2025 •

edited

Loading

utsavm9 Oct 16, 2025 •

edited

Loading