diff mbox series

[v2,5/5] mtd: spi-nor: add Kconfig option to disable write protection at power-up

Message ID 20190717113047.32596-1-tudor.ambarus@microchip.com
State Changes Requested
Delegated to: Vignesh R
Headers show
Series None | expand

Commit Message

Tudor Ambarus July 17, 2019, 11:30 a.m. UTC 
From: Tudor Ambarus <tudor.ambarus@microchip.com>

Some spi-nor flashes come write protected by default after a
power-on sequence to avoid destructing commands (erase, write)
during power-up.

Backward compatibility imposes to disable the write protection
at power-up by default. Add a Kconfig option to let the user
benefit of the power-up write protection.

Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com>
---
v2: mark spi_nor_spansion_clear_sr_bp() and
spi_nor_unlock_global_block_protection() definitions as __maybe_unused,
to avoid -Wunused-function warnings when
MTD_SPI_NOR_DISABLE_POWER_UP_WRITE_PROTECTION is not selected.

 drivers/mtd/spi-nor/Kconfig   | 8 ++++++++
 drivers/mtd/spi-nor/spi-nor.c | 7 +++++--
 2 files changed, 13 insertions(+), 2 deletions(-)

Comments

Raghavendra, Vignesh Aug. 7, 2019, 4:24 p.m. UTC | #1 
Hi Tudor

On 17-Jul-19 5:00 PM, Tudor.Ambarus@microchip.com wrote:
> From: Tudor Ambarus <tudor.ambarus@microchip.com>
[...]
> 
>  drivers/mtd/spi-nor/Kconfig   | 8 ++++++++
>  drivers/mtd/spi-nor/spi-nor.c | 7 +++++--
>  2 files changed, 13 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/mtd/spi-nor/Kconfig b/drivers/mtd/spi-nor/Kconfig
> index 6de83277ce8b..b550e10657f1 100644
> --- a/drivers/mtd/spi-nor/Kconfig
> +++ b/drivers/mtd/spi-nor/Kconfig
> @@ -22,6 +22,14 @@ config MTD_SPI_NOR_USE_4K_SECTORS
>  	  Please note that some tools/drivers/filesystems may not work with
>  	  4096 B erase size (e.g. UBIFS requires 15 KiB as a minimum).
>  
> +config MTD_SPI_NOR_DISABLE_POWER_UP_WRITE_PROTECTION
> +	bool "Disable write protection during power-up"
> +	default y
> +	help
> +	   Some spi-nor flashes are write protected by default after a power-on
> +	   reset cycle, in order to avoid inadvertend writes during power-up.
> +	   Disable the write protection during power-up.
> +


Hmm, how about setting MTD_POWERUP_LOCK flag in mtd->flags whenever
flash powers up with WP enabled? User can disable WP by declaring
partitions rw or keep partitions locked with ro. MTD core takes care of
calling mtd->unlock() depending up on the rw/ro flag as part of
add_mtd_device()

We could probably enhance spi_nor_unlock() to use
spi_nor_unlock_global_block_protection() when asked to unlock entire flash.

Kconfig option does not scale well for multi-platform build. There would
not be a way to have WP enabled on one platform but disabled on other.


Regards
Vignesh

>  config SPI_ASPEED_SMC
>  	tristate "Aspeed flash controllers in SPI mode"
>  	depends on ARCH_ASPEED || COMPILE_TEST
> diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
> index ffb53740031c..9b948295ef27 100644
> --- a/drivers/mtd/spi-nor/spi-nor.c
> +++ b/drivers/mtd/spi-nor/spi-nor.c
> @@ -1684,7 +1684,7 @@ static int spi_nor_clear_sr_bp(struct spi_nor *nor)
>   *
>   * Return: 0 on success, -errno otherwise.
>   */
> -static int spi_nor_spansion_clear_sr_bp(struct spi_nor *nor)
> +static int __maybe_unused spi_nor_spansion_clear_sr_bp(struct spi_nor *nor)
>  {
>  	int ret;
>  	u8 mask = SR_BP2 | SR_BP1 | SR_BP0;
> @@ -1726,7 +1726,8 @@ static int spi_nor_spansion_clear_sr_bp(struct spi_nor *nor)
>  	return spi_nor_clear_sr_bp(nor);
>  }
>  
> -static int spi_nor_unlock_global_block_protection(struct spi_nor *nor)
> +static int __maybe_unused
> +spi_nor_unlock_global_block_protection(struct spi_nor *nor)
>  {
>  	int ret;
>  
> @@ -4049,6 +4050,7 @@ static int spi_nor_init(struct spi_nor *nor)
>  {
>  	int err;
>  
> +#ifdef CONFIG_MTD_SPI_NOR_DISABLE_POWER_UP_WRITE_PROTECTION
>  	/*
>  	 * Atmel, SST, Intel/Numonyx, and others serial NOR tend to power up
>  	 * with the software protection bits set.
> @@ -4082,6 +4084,7 @@ static int spi_nor_init(struct spi_nor *nor)
>  			return err;
>  		}
>  	}
> +#endif
>  
>  	if (nor->quad_enable) {
>  		err = nor->quad_enable(nor);
>
diff mbox series

Patch

diff --git a/drivers/mtd/spi-nor/Kconfig b/drivers/mtd/spi-nor/Kconfig
index 6de83277ce8b..b550e10657f1 100644
--- a/drivers/mtd/spi-nor/Kconfig
+++ b/drivers/mtd/spi-nor/Kconfig
@@ -22,6 +22,14 @@  config MTD_SPI_NOR_USE_4K_SECTORS
 	  Please note that some tools/drivers/filesystems may not work with
 	  4096 B erase size (e.g. UBIFS requires 15 KiB as a minimum).
 
+config MTD_SPI_NOR_DISABLE_POWER_UP_WRITE_PROTECTION
+	bool "Disable write protection during power-up"
+	default y
+	help
+	   Some spi-nor flashes are write protected by default after a power-on
+	   reset cycle, in order to avoid inadvertend writes during power-up.
+	   Disable the write protection during power-up.
+
 config SPI_ASPEED_SMC
 	tristate "Aspeed flash controllers in SPI mode"
 	depends on ARCH_ASPEED || COMPILE_TEST
diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
index ffb53740031c..9b948295ef27 100644
--- a/drivers/mtd/spi-nor/spi-nor.c
+++ b/drivers/mtd/spi-nor/spi-nor.c
@@ -1684,7 +1684,7 @@  static int spi_nor_clear_sr_bp(struct spi_nor *nor)
  *
  * Return: 0 on success, -errno otherwise.
  */
-static int spi_nor_spansion_clear_sr_bp(struct spi_nor *nor)
+static int __maybe_unused spi_nor_spansion_clear_sr_bp(struct spi_nor *nor)
 {
 	int ret;
 	u8 mask = SR_BP2 | SR_BP1 | SR_BP0;
@@ -1726,7 +1726,8 @@  static int spi_nor_spansion_clear_sr_bp(struct spi_nor *nor)
 	return spi_nor_clear_sr_bp(nor);
 }
 
-static int spi_nor_unlock_global_block_protection(struct spi_nor *nor)
+static int __maybe_unused
+spi_nor_unlock_global_block_protection(struct spi_nor *nor)
 {
 	int ret;
 
@@ -4049,6 +4050,7 @@  static int spi_nor_init(struct spi_nor *nor)
 {
 	int err;
 
+#ifdef CONFIG_MTD_SPI_NOR_DISABLE_POWER_UP_WRITE_PROTECTION
 	/*
 	 * Atmel, SST, Intel/Numonyx, and others serial NOR tend to power up
 	 * with the software protection bits set.
@@ -4082,6 +4084,7 @@  static int spi_nor_init(struct spi_nor *nor)
 			return err;
 		}
 	}
+#endif
 
 	if (nor->quad_enable) {
 		err = nor->quad_enable(nor);