From 27fd8a26177fa05264cb2a8f9db9a0eaf62d863e Mon Sep 17 00:00:00 2001 From: "J.P. Hutchins" Date: Sat, 27 May 2023 14:32:01 -0700 Subject: [PATCH 1/5] drivers: flash: implement wp-gpios and hold-gpios Adds the wp-gpios & hold-gpios from jedec,spi-nor compatible to spi_nor.c. Signed-off-by: J.P. Hutchins --- drivers/flash/spi_nor.c | 73 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 73 insertions(+) diff --git a/drivers/flash/spi_nor.c b/drivers/flash/spi_nor.c index 994aedd3d210fe..745a4a6b56539a 100644 --- a/drivers/flash/spi_nor.c +++ b/drivers/flash/spi_nor.c @@ -1,6 +1,7 @@ /* * Copyright (c) 2018 Savoir-Faire Linux. * Copyright (c) 2020 Peter Bigot Consulting, LLC + * Copyright (c) 2023 Intercreate, Inc. * * This driver is heavily inspired from the spi_flash_w25qxxdv.c SPI NOR driver. * @@ -62,6 +63,12 @@ LOG_MODULE_REGISTER(spi_nor, CONFIG_FLASH_LOG_LEVEL); #define T_DPDD_MS 0 #endif /* DPD_WAKEUP_SEQUENCE */ +#define _INST_HAS_WP_OR(inst) DT_INST_NODE_HAS_PROP(inst, wp_gpios) || +#define ANY_INST_HAS_WP_GPIOS DT_INST_FOREACH_STATUS_OKAY(_INST_HAS_WP_OR) 0 + +#define _INST_HAS_HOLD_OR(inst) DT_INST_NODE_HAS_PROP(inst, hold_gpios) || +#define ANY_INST_HAS_HOLD_GPIOS DT_INST_FOREACH_STATUS_OKAY(_INST_HAS_HOLD_OR) 0 + /* Build-time data associated with the device. */ struct spi_nor_config { /* Devicetree SPI configuration */ @@ -102,6 +109,15 @@ struct spi_nor_config { * This information cannot be derived from SFDP. */ uint8_t has_lock; + +#if ANY_INST_HAS_WP_GPIOS + /* The write-protect GPIO (wp-gpios) */ + const struct gpio_dt_spec *wp; +#endif +#if ANY_INST_HAS_HOLD_GPIOS + /* The hold GPIO (hold-gpios) */ + const struct gpio_dt_spec *hold; +#endif }; /** @@ -822,8 +838,17 @@ static int spi_nor_erase(const struct device *dev, off_t addr, size_t size) static int spi_nor_write_protection_set(const struct device *dev, bool write_protect) { +#if ANY_INST_HAS_WP_GPIOS + const struct spi_nor_config *cfg = dev->config; +#endif int ret; +#if ANY_INST_HAS_WP_GPIOS + if (cfg->wp) { + gpio_pin_set_dt(cfg->wp, write_protect); + } +#endif + ret = spi_nor_cmd_write(dev, (write_protect) ? SPI_NOR_CMD_WRDI : SPI_NOR_CMD_WREN); @@ -1300,12 +1325,37 @@ static int spi_nor_pm_control(const struct device *dev, enum pm_device_action ac */ static int spi_nor_init(const struct device *dev) { +#if (ANY_INST_HAS_WP_GPIOS || ANY_INST_HAS_HOLD_GPIOS) + const struct spi_nor_config *cfg = dev->config; +#endif + if (IS_ENABLED(CONFIG_MULTITHREADING)) { struct spi_nor_data *const driver_data = dev->data; k_sem_init(&driver_data->sem, 1, K_SEM_MAX_LIMIT); } +#if ANY_INST_HAS_WP_GPIOS + if (cfg->wp) { + if (!device_is_ready(cfg->wp->port)) { + return -ENODEV; + } + if (gpio_pin_configure_dt(cfg->wp, GPIO_OUTPUT_ACTIVE)) { + return -ENODEV; + } + } +#endif /* ANY_INST_HAS_WP_GPIOS */ +#if ANY_INST_HAS_HOLD_GPIOS + if (cfg->hold) { + if (!device_is_ready(cfg->hold->port)) { + return -ENODEV; + } + if (gpio_pin_configure_dt(cfg->hold, GPIO_OUTPUT_INACTIVE)) { + return -ENODEV; + } + } +#endif /* ANY_INST_HAS_HOLD_GPIOS */ + return spi_nor_configure(dev); } @@ -1402,6 +1452,21 @@ BUILD_ASSERT(DT_INST_PROP(0, has_lock) == (DT_INST_PROP(0, has_lock) & 0xFF), "Need support for lock clear beyond SR1"); #endif +#define INST_HAS_WP_GPIO(idx) DT_INST_NODE_HAS_PROP(idx, wp_gpios) + +#define INST_WP_GPIO_SPEC(idx) \ + IF_ENABLED(INST_HAS_WP_GPIO(idx), (static const struct gpio_dt_spec wp_##idx = \ + GPIO_DT_SPEC_INST_GET(idx, wp_gpios);)) + +#define INST_HAS_HOLD_GPIO(idx) DT_INST_NODE_HAS_PROP(idx, hold_gpios) + +#define INST_HOLD_GPIO_SPEC(idx) \ + IF_ENABLED(INST_HAS_HOLD_GPIO(idx), (static const struct gpio_dt_spec hold_##idx = \ + GPIO_DT_SPEC_INST_GET(idx, hold_gpios);)) + +INST_WP_GPIO_SPEC(0) +INST_HOLD_GPIO_SPEC(0) + static const struct spi_nor_config spi_nor_config_0 = { .spi = SPI_DT_SPEC_INST_GET(0, SPI_WORD_SET(8), CONFIG_SPI_NOR_CS_WAIT_DELAY), @@ -1431,6 +1496,14 @@ static const struct spi_nor_config spi_nor_config_0 = { #endif /* CONFIG_SPI_NOR_SFDP_DEVICETREE */ #endif /* CONFIG_SPI_NOR_SFDP_RUNTIME */ + +#if DT_INST_NODE_HAS_PROP(0, wp_gpios) + .wp = &wp_0, +#endif + +#if DT_INST_NODE_HAS_PROP(0, hold_gpios) + .hold = &hold_0, +#endif }; static struct spi_nor_data spi_nor_data_0; From 4a6c102df37fbb287bf5312291aeaeba6b04a955 Mon Sep 17 00:00:00 2001 From: "J.P. Hutchins" Date: Wed, 31 May 2023 14:04:20 -0700 Subject: [PATCH 2/5] test: drivers: flash: build spi_nor.c wp & hold Test spi_nor.c driver builds with or without wp-gpios and hold-gpios. Signed-off-by: J.P. Hutchins --- .../common/boards/nrf52840dk_spi_nor.overlay | 61 ++++++++++++++++++ .../boards/nrf52840dk_spi_nor_wp_hold.overlay | 63 +++++++++++++++++++ tests/drivers/flash/common/testcase.yaml | 10 +++ 3 files changed, 134 insertions(+) create mode 100644 tests/drivers/flash/common/boards/nrf52840dk_spi_nor.overlay create mode 100644 tests/drivers/flash/common/boards/nrf52840dk_spi_nor_wp_hold.overlay diff --git a/tests/drivers/flash/common/boards/nrf52840dk_spi_nor.overlay b/tests/drivers/flash/common/boards/nrf52840dk_spi_nor.overlay new file mode 100644 index 00000000000000..1042b429ee3784 --- /dev/null +++ b/tests/drivers/flash/common/boards/nrf52840dk_spi_nor.overlay @@ -0,0 +1,61 @@ +/* + * Copyright (c) 2023 Intercreate, Inc. + * + * SPDX-License-Identifier: Apache-2.0 + * + * Build test for jedec,spi-nor compatible (drivers/flash/spi_nor.c) + */ + +/ { + aliases { + spi-flash0 = &mx25v1635fzui; + }; +}; + +/delete-node/ &mx25r64; + +&pinctrl { + spi0_default: spi0_default { + group1 { + psels = , + , + ; + }; + }; + + spi0_sleep: spi0_sleep { + group1 { + psels = , + , + ; + low-power-enable; + }; + }; +}; + +&spi0 { + compatible = "nordic,nrf-spim"; + status = "okay"; + cs-gpios = <&gpio0 13 GPIO_ACTIVE_LOW>; // mx25v16 + pinctrl-0 = <&spi0_default>; + pinctrl-1 = <&spi0_sleep>; + pinctrl-names = "default", "sleep"; + + mx25v1635fzui: mx25v1635fzui@0 { + compatible = "jedec,spi-nor"; + status = "okay"; + reg = <0>; + spi-max-frequency = <8000000>; // chip supports 80Mhz, SPI0 supports 8MHz + size = <0x1000000>; // bits + has-dpd; + t-enter-dpd = <10000>; + t-exit-dpd = <45000>; + jedec-id = [ C2 23 15 ]; + sfdp-bfp = [ + e5 20 f1 ff ff ff ff 00 44 eb 08 6b 08 3b 04 bb + ee ff ff ff ff ff 00 ff ff ff 00 ff 0c 20 0f 52 + 10 d8 00 ff 23 72 f1 00 82 ec 04 c2 44 83 48 44 + 30 b0 30 b0 f7 c4 d5 5c 00 be 29 ff f0 d0 ff ff + ]; + }; +}; diff --git a/tests/drivers/flash/common/boards/nrf52840dk_spi_nor_wp_hold.overlay b/tests/drivers/flash/common/boards/nrf52840dk_spi_nor_wp_hold.overlay new file mode 100644 index 00000000000000..f17febff6c305b --- /dev/null +++ b/tests/drivers/flash/common/boards/nrf52840dk_spi_nor_wp_hold.overlay @@ -0,0 +1,63 @@ +/* + * Copyright (c) 2023 Intercreate, Inc. + * + * SPDX-License-Identifier: Apache-2.0 + * + * Build test for jedec,spi-nor compatible (drivers/flash/spi_nor.c) wp-gpios and hold-gpios + */ + +/ { + aliases { + spi-flash0 = &mx25v1635fzui; + }; +}; + +/delete-node/ &mx25r64; + +&pinctrl { + spi0_default: spi0_default { + group1 { + psels = , + , + ; + }; + }; + + spi0_sleep: spi0_sleep { + group1 { + psels = , + , + ; + low-power-enable; + }; + }; +}; + +&spi0 { + compatible = "nordic,nrf-spim"; + status = "okay"; + cs-gpios = <&gpio0 13 GPIO_ACTIVE_LOW>; // mx25v16 + pinctrl-0 = <&spi0_default>; + pinctrl-1 = <&spi0_sleep>; + pinctrl-names = "default", "sleep"; + + mx25v1635fzui: mx25v1635fzui@0 { + compatible = "jedec,spi-nor"; + status = "okay"; + reg = <0>; + spi-max-frequency = <8000000>; // chip supports 80Mhz, SPI0 supports 8MHz + size = <0x1000000>; // bits + hold-gpios = <&gpio0 14 GPIO_ACTIVE_LOW>; + wp-gpios = <&gpio0 15 GPIO_ACTIVE_LOW>; + has-dpd; + t-enter-dpd = <10000>; + t-exit-dpd = <45000>; + jedec-id = [ C2 23 15 ]; + sfdp-bfp = [ + e5 20 f1 ff ff ff ff 00 44 eb 08 6b 08 3b 04 bb + ee ff ff ff ff ff 00 ff ff ff 00 ff 0c 20 0f 52 + 10 d8 00 ff 23 72 f1 00 82 ec 04 c2 44 83 48 44 + 30 b0 30 b0 f7 c4 d5 5c 00 be 29 ff f0 d0 ff ff + ]; + }; +}; diff --git a/tests/drivers/flash/common/testcase.yaml b/tests/drivers/flash/common/testcase.yaml index c37515e91409d1..3fb40e64fe0785 100644 --- a/tests/drivers/flash/common/testcase.yaml +++ b/tests/drivers/flash/common/testcase.yaml @@ -80,3 +80,13 @@ tests: platform_allow: mr_canhubk3 extra_configs: - CONFIG_FLASH_NXP_S32_QSPI_NOR_SFDP_RUNTIME=y + drivers.flash.common.spi_nor: + platform_allow: nrf52840dk_nrf52840 + extra_args: + - OVERLAY_CONFIG=boards/nrf52840dk_flash_spi.conf + - DTC_OVERLAY_FILE=boards/nrf52840dk_spi_nor.overlay + drivers.flash.common.spi_nor_wp_hold: + platform_allow: nrf52840dk_nrf52840 + extra_args: + - OVERLAY_CONFIG=boards/nrf52840dk_flash_spi.conf + - DTC_OVERLAY_FILE=boards/nrf52840dk_spi_nor_wp_hold.overlay From 43c667402435bd64cf3476c0186cbf45c0c45e77 Mon Sep 17 00:00:00 2001 From: "J.P. Hutchins" Date: Wed, 21 Jun 2023 18:29:02 -0700 Subject: [PATCH 3/5] drivers: flash: add LOG_ERR for pin init -ENODEV returned in 4 cases, logs can distinguish Signed-off-by: J.P. Hutchins --- drivers/flash/spi_nor.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/drivers/flash/spi_nor.c b/drivers/flash/spi_nor.c index 745a4a6b56539a..4817b113b48475 100644 --- a/drivers/flash/spi_nor.c +++ b/drivers/flash/spi_nor.c @@ -1338,9 +1338,11 @@ static int spi_nor_init(const struct device *dev) #if ANY_INST_HAS_WP_GPIOS if (cfg->wp) { if (!device_is_ready(cfg->wp->port)) { + LOG_ERR("Write-protect pin not ready"); return -ENODEV; } if (gpio_pin_configure_dt(cfg->wp, GPIO_OUTPUT_ACTIVE)) { + LOG_ERR("Write-protect pin failed to set active") return -ENODEV; } } @@ -1348,9 +1350,11 @@ static int spi_nor_init(const struct device *dev) #if ANY_INST_HAS_HOLD_GPIOS if (cfg->hold) { if (!device_is_ready(cfg->hold->port)) { + LOG_ERR("Hold pin not ready"); return -ENODEV; } if (gpio_pin_configure_dt(cfg->hold, GPIO_OUTPUT_INACTIVE)) { + LOG_ERR("Hold pin failed to set inactive") return -ENODEV; } } From dcf2571148d77033a3374408bb363ffe01af8dda Mon Sep 17 00:00:00 2001 From: "J.P. Hutchins" Date: Wed, 21 Jun 2023 18:51:15 -0700 Subject: [PATCH 4/5] drivers: flash: cleanup #ifs; fix missing ; Fixes made running tests/drivers/flash Signed-off-by: J.P. Hutchins --- drivers/flash/spi_nor.c | 29 ++++++++++++----------------- 1 file changed, 12 insertions(+), 17 deletions(-) diff --git a/drivers/flash/spi_nor.c b/drivers/flash/spi_nor.c index 4817b113b48475..9d480c1326393d 100644 --- a/drivers/flash/spi_nor.c +++ b/drivers/flash/spi_nor.c @@ -69,6 +69,8 @@ LOG_MODULE_REGISTER(spi_nor, CONFIG_FLASH_LOG_LEVEL); #define _INST_HAS_HOLD_OR(inst) DT_INST_NODE_HAS_PROP(inst, hold_gpios) || #define ANY_INST_HAS_HOLD_GPIOS DT_INST_FOREACH_STATUS_OKAY(_INST_HAS_HOLD_OR) 0 +#define DEV_CFG(_dev_) ((const struct spi_nor_config * const) (_dev_)->config) + /* Build-time data associated with the device. */ struct spi_nor_config { /* Devicetree SPI configuration */ @@ -838,14 +840,11 @@ static int spi_nor_erase(const struct device *dev, off_t addr, size_t size) static int spi_nor_write_protection_set(const struct device *dev, bool write_protect) { -#if ANY_INST_HAS_WP_GPIOS - const struct spi_nor_config *cfg = dev->config; -#endif int ret; #if ANY_INST_HAS_WP_GPIOS - if (cfg->wp) { - gpio_pin_set_dt(cfg->wp, write_protect); + if (DEV_CFG(dev)->wp) { + gpio_pin_set_dt(DEV_CFG(dev)->wp, write_protect); } #endif @@ -1325,10 +1324,6 @@ static int spi_nor_pm_control(const struct device *dev, enum pm_device_action ac */ static int spi_nor_init(const struct device *dev) { -#if (ANY_INST_HAS_WP_GPIOS || ANY_INST_HAS_HOLD_GPIOS) - const struct spi_nor_config *cfg = dev->config; -#endif - if (IS_ENABLED(CONFIG_MULTITHREADING)) { struct spi_nor_data *const driver_data = dev->data; @@ -1336,25 +1331,25 @@ static int spi_nor_init(const struct device *dev) } #if ANY_INST_HAS_WP_GPIOS - if (cfg->wp) { - if (!device_is_ready(cfg->wp->port)) { + if (DEV_CFG(dev)->wp) { + if (!device_is_ready(DEV_CFG(dev)->wp->port)) { LOG_ERR("Write-protect pin not ready"); return -ENODEV; } - if (gpio_pin_configure_dt(cfg->wp, GPIO_OUTPUT_ACTIVE)) { - LOG_ERR("Write-protect pin failed to set active") + if (gpio_pin_configure_dt(DEV_CFG(dev)->wp, GPIO_OUTPUT_ACTIVE)) { + LOG_ERR("Write-protect pin failed to set active"); return -ENODEV; } } #endif /* ANY_INST_HAS_WP_GPIOS */ #if ANY_INST_HAS_HOLD_GPIOS - if (cfg->hold) { - if (!device_is_ready(cfg->hold->port)) { + if (DEV_CFG(dev)->hold) { + if (!device_is_ready(DEV_CFG(dev)->hold->port)) { LOG_ERR("Hold pin not ready"); return -ENODEV; } - if (gpio_pin_configure_dt(cfg->hold, GPIO_OUTPUT_INACTIVE)) { - LOG_ERR("Hold pin failed to set inactive") + if (gpio_pin_configure_dt(DEV_CFG(dev)->hold, GPIO_OUTPUT_INACTIVE)) { + LOG_ERR("Hold pin failed to set inactive"); return -ENODEV; } } From f910020e50f2c9da8799837d0e2a244b79798402 Mon Sep 17 00:00:00 2001 From: "J.P. Hutchins" Date: Mon, 7 Aug 2023 11:40:48 -0700 Subject: [PATCH 5/5] drivers: flash: fix hw write protect before sw This change sets write-protect pin disabled BEFORE SW write-protect disable and write-protect pin enabled AFTER SW write-protect enable. Signed-off-by: J.P. Hutchins --- drivers/flash/spi_nor.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/drivers/flash/spi_nor.c b/drivers/flash/spi_nor.c index 9d480c1326393d..e1df675e69a3c3 100644 --- a/drivers/flash/spi_nor.c +++ b/drivers/flash/spi_nor.c @@ -843,8 +843,8 @@ static int spi_nor_write_protection_set(const struct device *dev, int ret; #if ANY_INST_HAS_WP_GPIOS - if (DEV_CFG(dev)->wp) { - gpio_pin_set_dt(DEV_CFG(dev)->wp, write_protect); + if (DEV_CFG(dev)->wp && write_protect == false) { + gpio_pin_set_dt(DEV_CFG(dev)->wp, 0); } #endif @@ -857,6 +857,12 @@ static int spi_nor_write_protection_set(const struct device *dev, ret = spi_nor_cmd_write(dev, SPI_NOR_CMD_ULBPR); } +#if ANY_INST_HAS_WP_GPIOS + if (DEV_CFG(dev)->wp && write_protect == true) { + gpio_pin_set_dt(DEV_CFG(dev)->wp, 1); + } +#endif + return ret; }