drivers/flash/nrf_qspi_nor: add support for enter 4-byte addressing mode #41743

nvlsianpu · 2022-01-12T12:57:18Z

Added basic support for enter 4-byte addressing command.
Patch supports command 0xB7 (Enter 4-Byte Address Mode),
with or without preceding WREN.

Similar as for SPI-NOR the enter-4byte-addr property of
memory node is used or describing how to Enter 4-Byte Addressing
mode.
Worth to notice that along with that property the address-size-32
property is expected as it switch the driver to use 4-byte addressing
in operations.

Signed-off-by: Andrzej Puzdrowski andrzej.puzdrowski@nordicsemi.no

~~not tested on hardware yet~~
was tested using MX25L51245G

de-nordic · 2022-01-12T16:06:49Z

drivers/flash/nrf_qspi_nor.c

@@ -564,7 +576,20 @@ static int qspi_nrfx_configure(const struct device *dev)
 		}
 	}

-	return 0;
+	if (INST_0_4BA != 0) {


Maybe we should add LOG_DBG that would be stating that we got into 4-byte address mode.

de-nordic · 2022-01-12T16:19:00Z

Shouldn't we also address usage of sfdp-bfp in DTS? The enter_4byte_addr, as the binding states, is less important than sfdp bpfs, which stores the same info.

nvlsianpu · 2022-01-13T16:45:27Z

Shouldn't we also address usage of sfdp-bfp in DTS? The enter_4byte_addr, as the binding states, is less important than sfdp bpfs, which stores the same info.

This driver doesn't support DTS bundled sfd-bfp. I think runtime processing of these build time records is not something I should apply in this PR. I was looking into spi-nor driver: It looks like support of that possibly results in implementation of features unused in given application.

Anyway, adding (runtime) read of DW16 of SFDP BFP make sense to me here.

nvlsianpu · 2022-01-14T12:03:25Z

@de-nordic After I have been looking into spi_nor driver I conclude that the read and the runtime processing of SFDP might be implemented as there. Best approach would be extraction of common code from spi_nor.c (at last commonality of functionality of spi_nor_process_sfdp() function) and re-usage in both drivers. Such a task would be over what I want to introduce here.
#41819

de-nordic · 2022-01-14T13:03:55Z

@de-nordic After I have been looking into spi_nor driver I conclude that the read and the runtime processing of SFDP might be implemented as there. Best approach would be extraction of common code from spi_nor.c (at last commonality of functionality of spi_nor_process_sfdp() function) and re-usage in both drivers. Such a task would be over what I want to introduce here. #41819

Great. So I leave adding the LOG_DBG line up to you. I have looked at the code and it looks OK.
In the future we should probably add some defines to cover the bit meanings in the BFP bytes, but that will be a lot of work.

de-nordic

Looks OK. Thanks.

anangl · 2022-01-18T14:02:56Z

drivers/flash/nrf_qspi_nor.c

+		if (INST_0_4BA & 0x02) {
+			qspi_nor_write_protection_set(dev, false);
+		}
+
+		struct qspi_cmd cmd = {
+			.op_code = SPI_NOR_CMD_4BA,
+			.rx_buf = NULL,
+			.tx_buf = NULL,
+		};
+		ret = qspi_send_cmd(dev, &cmd, false);


How about just:

struct qspi_cmd cmd = { .op_code = SPI_NOR_CMD_4BA, }; ret = qspi_send_cmd(dev, &cmd, (INST_0_4BA & 0x02));

?

and right, the rest of struct members will be zeroed automatically.

anangl · 2022-01-18T14:07:22Z

drivers/flash/nrf_qspi_nor.c

+		if (ret < 0) {
+			LOG_DBG("E4BA cmd issued.");
+		} else {
+			LOG_ERR("E4BA cmd issue failed: %d.", ret);
+		}


ret < 0 means that an error occurred, so this should be the other way around.

anangl · 2022-01-18T14:08:10Z

drivers/flash/nrf_qspi_nor.c

+#if DT_INST_NODE_HAS_PROP(0, enter_4byte_addr)
+#define INST_0_4BA DT_INST_PROP(0, enter_4byte_addr)


Suggested change

#if DT_INST_NODE_HAS_PROP(0, enter_4byte_addr)

#define INST_0_4BA DT_INST_PROP(0, enter_4byte_addr)

#define INST_0_4BA DT_INST_PROP_OR(0, enter_4byte_addr, 0)

richesonk · 2022-03-04T16:33:02Z

What is the status of this pull request? I'm very interested in this change as the solution to #40453

nvlsianpu · 2022-03-10T15:23:43Z

@richesonk I'm going to test this finally.

anangl · 2022-03-15T12:07:04Z

drivers/flash/nrf_qspi_nor.c

+#if (INST_0_4BA != 0)
+BUILD_ASSERT(((INST_0_4BA & 0x03) != 0),
+	     "Driver only supports command (0xB7) for entering 4 byte addressing mode");
+BUILD_ASSERT((DT_INST_PROP(0, address_size_32) == NRF_QSPI_ADDRMODE_32BIT),


Suggested change

BUILD_ASSERT((DT_INST_PROP(0, address_size_32) == NRF_QSPI_ADDRMODE_32BIT),

BUILD_ASSERT(DT_INST_PROP(0, address_size_32),

This is a boolean property, so it should not be compared with the value of NRF_QSPI_ADDRMODE_32BIT (which by chance evaluates to 1, hence it works in the current form).

dastarling · 2022-03-21T19:12:05Z

I also need this change for an active development project soon. I hope it makes the next NCS release. Thanks

nvlsianpu · 2022-03-22T10:44:32Z

@de-nordic @anangl Can you revisit?

anangl · 2022-03-22T10:58:02Z

tests/drivers/flash/boards/nrf52840dk_mx25l51245g.overlay

@@ -0,0 +1,37 @@
+


You should add a test configuration in testcase.yaml that will use this overlay so that CI has a chance to check it.

anangl · 2022-03-22T11:06:05Z

tests/drivers/flash/boards/nrf52840dk_mx25l51245g.overlay

+&qspi {
+	sck-pin = <30>;
+	io-pins = <29>, <28>, <04>, <03>;
+	csn-pins = <31>;


This board (nRF52840 DK) uses PINCTRL now, so you need to rebase and then modify pinctrl-* properties instead.

Suggested change

&qspi {

sck-pin = <30>;

io-pins = <29>, <28>, <04>, <03>;

csn-pins = <31>;

&pinctrl {

qspi_default: qspi_default {

group1 {

psels = <NRF_PSEL(QSPI_SCK, 0, 30)>,

<NRF_PSEL(QSPI_IO0, 0, 29)>,

<NRF_PSEL(QSPI_IO1, 0, 28)>,

<NRF_PSEL(QSPI_IO2, 0, 04)>,

<NRF_PSEL(QSPI_IO3, 0, 03)>,

<NRF_PSEL(QSPI_CSN, 0, 31)>;

};

};

qspi_sleep: qspi_sleep {

group1 {

psels = <NRF_PSEL(QSPI_SCK, 0, 30)>,

<NRF_PSEL(QSPI_IO0, 0, 29)>,

<NRF_PSEL(QSPI_IO1, 0, 28)>,

<NRF_PSEL(QSPI_IO2, 0, 04)>,

<NRF_PSEL(QSPI_IO3, 0, 03)>,

<NRF_PSEL(QSPI_CSN, 0, 31)>;

low-power-enable;

};

};

};

&qspi {

pinctrl-0 = <&qspi_default_alt>;

pinctrl-1 = <&qspi_sleep_alt>;

pinctrl-names = "default", "sleep";

anangl · 2022-03-22T14:15:04Z

tests/drivers/flash/testcase.yaml

+    extra_args: OVERLAY_CONFIG=boards/nrf52840_flash_qspi.conf
+      DTC_OVERLAY_FILE=./boards/nrf52840dk_mx25l51245g.overlay


Is this "./" actually needed?

Suggested change

extra_args: OVERLAY_CONFIG=boards/nrf52840_flash_qspi.conf

DTC_OVERLAY_FILE=./boards/nrf52840dk_mx25l51245g.overlay

extra_args: OVERLAY_CONFIG=boards/nrf52840_flash_qspi.conf

DTC_OVERLAY_FILE=boards/nrf52840dk_mx25l51245g.overlay

'./' is not needed.

Added basic support for enter 4-byte addressing command. Patch supports command 0xB7 (Enter 4-Byte Address Mode), with or without preceding WREN. Similar as for SPI-NOR the `enter-4byte-addr` property of memory node is used or describing how to Enter 4-Byte Addressing mode. Worth to notice that along with that property the `address-size-32` property is expected as it switch the driver to use 4-byte addressing in operations. Signed-off-by: Andrzej Puzdrowski <andrzej.puzdrowski@nordicsemi.no>

Added overlay with DTS of MX25L51245G qspi flash memory. DTS configures qspi clock to 2 MHz which is supported by nRF52840. Signed-off-by: Andrzej Puzdrowski <andrzej.puzdrowski@nordicsemi.no>

nvlsianpu added area: Flash platform: nRF Nordic nRFx DNM This PR should not be merged (Do Not Merge) labels Jan 12, 2022

carlescufi requested review from anangl and de-nordic January 12, 2022 13:57

de-nordic reviewed Jan 12, 2022

View reviewed changes

zephyrbot assigned nvlsianpu Jan 13, 2022

de-nordic previously approved these changes Jan 14, 2022

View reviewed changes

nvlsianpu force-pushed the nrf_qspi_nor/4byteMode branch from 63cf20f to 114f457 Compare January 14, 2022 14:25

anangl reviewed Jan 18, 2022

View reviewed changes

nvlsianpu force-pushed the nrf_qspi_nor/4byteMode branch from 114f457 to 0f01f03 Compare January 20, 2022 17:41

dleach02 dismissed de-nordic’s stale review via 0f01f03 February 11, 2022 16:03

de-nordic self-requested a review March 4, 2022 16:52

de-nordic previously approved these changes Mar 4, 2022

View reviewed changes

nvlsianpu removed the DNM This PR should not be merged (Do Not Merge) label Mar 14, 2022

carlescufi requested a review from anangl March 15, 2022 10:51

anangl reviewed Mar 15, 2022

View reviewed changes

nvlsianpu dismissed de-nordic’s stale review via bc0f3a1 March 22, 2022 09:52

nvlsianpu force-pushed the nrf_qspi_nor/4byteMode branch from 0f01f03 to bc0f3a1 Compare March 22, 2022 09:52

nvlsianpu requested a review from nashif as a code owner March 22, 2022 09:52

github-actions bot added the area: Tests Issues related to a particular existing or missing test label Mar 22, 2022

carlescufi requested a review from anangl March 22, 2022 10:51

carlescufi requested a review from de-nordic March 22, 2022 10:51

anangl requested changes Mar 22, 2022

View reviewed changes

nvlsianpu force-pushed the nrf_qspi_nor/4byteMode branch 2 times, most recently from ea55ed2 to d21447e Compare March 22, 2022 14:07

anangl previously approved these changes Mar 22, 2022

View reviewed changes

nvlsianpu dismissed anangl’s stale review via df7d819 March 22, 2022 14:40

nvlsianpu force-pushed the nrf_qspi_nor/4byteMode branch from d21447e to df7d819 Compare March 22, 2022 14:40

anangl previously approved these changes Mar 22, 2022

View reviewed changes

de-nordic previously approved these changes Mar 22, 2022

View reviewed changes

nvlsianpu dismissed stale reviews from de-nordic and anangl via d214543 March 23, 2022 11:37

nvlsianpu force-pushed the nrf_qspi_nor/4byteMode branch from df7d819 to d214543 Compare March 23, 2022 11:37

anangl previously approved these changes Mar 23, 2022

View reviewed changes

de-nordic previously approved these changes Mar 23, 2022

View reviewed changes

nvlsianpu dismissed stale reviews from de-nordic and anangl via 9c46b98 March 23, 2022 13:14

nvlsianpu force-pushed the nrf_qspi_nor/4byteMode branch 2 times, most recently from 9c46b98 to 1b83bf9 Compare March 23, 2022 14:04

nvlsianpu added 2 commits March 23, 2022 15:29

tests/driver/flash: overlay for MX25L51245G

f440424

Added overlay with DTS of MX25L51245G qspi flash memory. DTS configures qspi clock to 2 MHz which is supported by nRF52840. Signed-off-by: Andrzej Puzdrowski <andrzej.puzdrowski@nordicsemi.no>

nvlsianpu force-pushed the nrf_qspi_nor/4byteMode branch from 1b83bf9 to f440424 Compare March 23, 2022 14:29

carlescufi requested review from anangl and de-nordic March 23, 2022 15:00

carlescufi approved these changes Mar 23, 2022

View reviewed changes

anangl approved these changes Mar 23, 2022

View reviewed changes

carlescufi merged commit b5b05bc into zephyrproject-rtos:main Mar 23, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

drivers/flash/nrf_qspi_nor: add support for enter 4-byte addressing mode #41743

drivers/flash/nrf_qspi_nor: add support for enter 4-byte addressing mode #41743

nvlsianpu commented Jan 12, 2022 •

edited

de-nordic Jan 12, 2022

nvlsianpu Jan 14, 2022

de-nordic commented Jan 12, 2022

nvlsianpu commented Jan 13, 2022 •

edited

nvlsianpu commented Jan 14, 2022 •

edited

de-nordic commented Jan 14, 2022

de-nordic left a comment

anangl Jan 18, 2022

nvlsianpu Jan 20, 2022 •

edited

anangl Jan 18, 2022

anangl Jan 18, 2022

richesonk commented Mar 4, 2022

nvlsianpu commented Mar 10, 2022

anangl Mar 15, 2022

nvlsianpu Mar 22, 2022

dastarling commented Mar 21, 2022

nvlsianpu commented Mar 22, 2022

anangl Mar 22, 2022

nvlsianpu Mar 22, 2022

anangl Mar 22, 2022 •

edited

nvlsianpu Mar 22, 2022

anangl Mar 22, 2022

nvlsianpu Mar 22, 2022

		#if DT_INST_NODE_HAS_PROP(0, enter_4byte_addr)
		#define INST_0_4BA DT_INST_PROP(0, enter_4byte_addr)

	#if DT_INST_NODE_HAS_PROP(0, enter_4byte_addr)
	#define INST_0_4BA DT_INST_PROP(0, enter_4byte_addr)
	#define INST_0_4BA DT_INST_PROP_OR(0, enter_4byte_addr, 0)

	BUILD_ASSERT((DT_INST_PROP(0, address_size_32) == NRF_QSPI_ADDRMODE_32BIT),
	BUILD_ASSERT(DT_INST_PROP(0, address_size_32),

-&qspi {
-	sck-pin = <30>;
-	io-pins = <29>, <28>, <04>, <03>;
-	csn-pins = <31>;
+&pinctrl {
+	qspi_default: qspi_default {
+		group1 {
+			psels = <NRF_PSEL(QSPI_SCK, 0, 30)>,
+				<NRF_PSEL(QSPI_IO0, 0, 29)>,
+				<NRF_PSEL(QSPI_IO1, 0, 28)>,
+				<NRF_PSEL(QSPI_IO2, 0, 04)>,
+				<NRF_PSEL(QSPI_IO3, 0, 03)>,
+				<NRF_PSEL(QSPI_CSN, 0, 31)>;
+		};
+	};
+	qspi_sleep: qspi_sleep {
+		group1 {
+			psels = <NRF_PSEL(QSPI_SCK, 0, 30)>,
+				<NRF_PSEL(QSPI_IO0, 0, 29)>,
+				<NRF_PSEL(QSPI_IO1, 0, 28)>,
+				<NRF_PSEL(QSPI_IO2, 0, 04)>,
+				<NRF_PSEL(QSPI_IO3, 0, 03)>,
+				<NRF_PSEL(QSPI_CSN, 0, 31)>;
+			low-power-enable;
+		};
+	};
+};
+&qspi {
+	pinctrl-0 = <&qspi_default_alt>;
+	pinctrl-1 = <&qspi_sleep_alt>;
+	pinctrl-names = "default", "sleep";

		extra_args: OVERLAY_CONFIG=boards/nrf52840_flash_qspi.conf
		DTC_OVERLAY_FILE=./boards/nrf52840dk_mx25l51245g.overlay

drivers/flash/nrf_qspi_nor: add support for enter 4-byte addressing mode #41743

drivers/flash/nrf_qspi_nor: add support for enter 4-byte addressing mode #41743

Conversation

nvlsianpu commented Jan 12, 2022 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

de-nordic commented Jan 12, 2022

nvlsianpu commented Jan 13, 2022 • edited

nvlsianpu commented Jan 14, 2022 • edited

de-nordic commented Jan 14, 2022

de-nordic left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nvlsianpu Jan 20, 2022 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

richesonk commented Mar 4, 2022

nvlsianpu commented Mar 10, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dastarling commented Mar 21, 2022

nvlsianpu commented Mar 22, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

anangl Mar 22, 2022 • edited

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

nvlsianpu commented Jan 12, 2022 •

edited

nvlsianpu commented Jan 13, 2022 •

edited

nvlsianpu commented Jan 14, 2022 •

edited

nvlsianpu Jan 20, 2022 •

edited

anangl Mar 22, 2022 •

edited