mtd: spi-nor: keep lock bits if they are non-volatile
authorMichael Walle <michael@walle.cc>
Thu, 3 Dec 2020 16:29:59 +0000 (17:29 +0100)
committerVignesh Raghavendra <vigneshr@ti.com>
Mon, 7 Dec 2020 17:31:15 +0000 (23:01 +0530)
Traditionally, Linux unlocks the whole flash because there are legacy
devices which has the write protection bits set by default at startup.
If you actually want to use the flash protection bits, eg. because there
is a read-only part for a bootloader, this automatic unlocking is
harmful. If there is no hardware write protection in place (usually
called WP#), a startup of the kernel just discards this protection.

I've gone through the datasheets of all the flashes (except the Intel
ones where I could not find any datasheet nor reference) which supports
the unlocking feature and looked how the sector protection was
implemented. The currently supported flashes can be divided into the
following two categories:
 (1) block protection bits are non-volatile. Thus they keep their values
     at reset and power-cycle
 (2) flashes where these bits are volatile. After reset or power-cycle,
     the whole memory array is protected.
     (a) some devices needs a special "Global Unprotect" command, eg.
         the Atmel AT25DF041A.
     (b) some devices require to clear the BPn bits in the status
         register.

Due to the reasons above, we do not want to clear the bits for flashes
which belong to category (1). Fortunately for us, only Atmel flashes
fall into category (2a). Implement the "Global Protect" and "Global
Unprotect" commands for these. For (2b) we can use normal block
protection locking scheme.

This patch adds a new flag to indicate the case (2). Only if we have
such a flash we unlock the whole flash array. To be backwards compatible
it also introduces a kernel configuration option which restores the
complete legacy behavior ("Disable write protection on any flashes").
Hopefully, this will clean up "unlock the entire flash for legacy
devices" once and for all.

For reference here are the actually commits which introduced the legacy
behavior (and extended the behavior to other chip manufacturers):

commit f80e521c916cb ("mtd: m25p80: add support for the Intel/Numonyx {16,32,64}0S33B SPI flash chips")
commit ea60658a08f8f ("mtd: m25p80: disable SST software protection bits by default")
commit 7228982442365 ("[MTD] m25p80: fix bug - ATmel spi flash fails to be copied to")

Actually, this might also fix handling of the Atmel AT25DF flashes,
because the original commit 7228982442365 ("[MTD] m25p80: fix bug -
ATmel spi flash fails to be copied to") was writing a 0 to the status
register, which is a "Global Unprotect". This might not be the case in
the current code which only handles the block protection bits BP2, BP1
and BP0. Thus, it depends on the current contents of the status register
if this unlock actually corresponds to a "Global Unprotect" command. In
the worst case, the current code might leave the AT25DF flashes in a
write protected state.

The commit 191f5c2ed4b6f ("mtd: spi-nor: use 16-bit WRR command when QE
is set on spansion flashes") changed that behavior by just clearing BP2
to BP0 instead of writing a 0 to the status register.

Further, the commit 3e0930f109e76 ("mtd: spi-nor: Rework the disabling
of block write protection") expanded the unlock_all() feature to ANY
flash which supports locking.

Signed-off-by: Michael Walle <michael@walle.cc>
Signed-off-by: Vignesh Raghavendra <vigneshr@ti.com>
Reviewed-by: Tudor Ambarus <tudor.ambarus@microchip.com>
Link: https://lore.kernel.org/r/20201203162959.29589-8-michael@walle.cc
drivers/mtd/spi-nor/Kconfig
drivers/mtd/spi-nor/atmel.c
drivers/mtd/spi-nor/core.c
drivers/mtd/spi-nor/core.h
drivers/mtd/spi-nor/esmt.c
drivers/mtd/spi-nor/intel.c
drivers/mtd/spi-nor/sst.c

index ffc4b38..24cd25d 100644 (file)
@@ -24,6 +24,50 @@ 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).
 
+choice
+       prompt "Software write protection at boot"
+       default MTD_SPI_NOR_SWP_DISABLE_ON_VOLATILE
+
+config MTD_SPI_NOR_SWP_DISABLE
+       bool "Disable SWP on any flashes (legacy behavior)"
+       help
+         This option disables the software write protection on any SPI
+         flashes at boot-up.
+
+         Depending on the flash chip this either clears the block protection
+         bits or does a "Global Unprotect" command.
+
+         Don't use this if you intent to use the software write protection
+         of your SPI flash. This is only to keep backwards compatibility.
+
+config MTD_SPI_NOR_SWP_DISABLE_ON_VOLATILE
+       bool "Disable SWP on flashes w/ volatile protection bits"
+       help
+         Some SPI flashes have volatile block protection bits, ie. after a
+         power-up or a reset the flash is software write protected by
+         default.
+
+         This option disables the software write protection for these kind
+         of flashes while keeping it enabled for any other SPI flashes
+         which have non-volatile write protection bits.
+
+         If the software write protection will be disabled depending on
+         the flash either the block protection bits are cleared or a
+         "Global Unprotect" command is issued.
+
+         If you are unsure, select this option.
+
+config MTD_SPI_NOR_SWP_KEEP
+       bool "Keep software write protection as is"
+       help
+         If you select this option the software write protection of any
+         SPI flashes will not be changed. If your flash is software write
+         protected or will be automatically software write protected after
+         power-up you have to manually unlock it before you are able to
+         write to it.
+
+endchoice
+
 source "drivers/mtd/spi-nor/controllers/Kconfig"
 
 endif # MTD_SPI_NOR
index deacf87..1fea5ca 100644 (file)
@@ -8,6 +8,8 @@
 
 #include "core.h"
 
+#define ATMEL_SR_GLOBAL_PROTECT_MASK GENMASK(5, 2)
+
 /*
  * The Atmel AT25FS010/AT25FS040 parts have some weird configuration for the
  * block protection bits. We don't support them. But legacy behavior in linux
@@ -55,6 +57,104 @@ static const struct spi_nor_fixups atmel_at25fs_fixups = {
        .default_init = atmel_at25fs_default_init,
 };
 
+/**
+ * atmel_set_global_protection - Do a Global Protect or Unprotect command
+ * @nor:       pointer to 'struct spi_nor'
+ * @ofs:       offset in bytes
+ * @len:       len in bytes
+ * @is_protect:        if true do a Global Protect otherwise it is a Global Unprotect
+ *
+ * Return: 0 on success, -error otherwise.
+ */
+static int atmel_set_global_protection(struct spi_nor *nor, loff_t ofs,
+                                      uint64_t len, bool is_protect)
+{
+       int ret;
+       u8 sr;
+
+       /* We only support locking the whole flash array */
+       if (ofs || len != nor->params->size)
+               return -EINVAL;
+
+       ret = spi_nor_read_sr(nor, nor->bouncebuf);
+       if (ret)
+               return ret;
+
+       sr = nor->bouncebuf[0];
+
+       /* SRWD bit needs to be cleared, otherwise the protection doesn't change */
+       if (sr & SR_SRWD) {
+               sr &= ~SR_SRWD;
+               ret = spi_nor_write_sr_and_check(nor, sr);
+               if (ret) {
+                       dev_dbg(nor->dev, "unable to clear SRWD bit, WP# asserted?\n");
+                       return ret;
+               }
+       }
+
+       if (is_protect) {
+               sr |= ATMEL_SR_GLOBAL_PROTECT_MASK;
+               /*
+                * Set the SRWD bit again as soon as we are protecting
+                * anything. This will ensure that the WP# pin is working
+                * correctly. By doing this we also behave the same as
+                * spi_nor_sr_lock(), which sets SRWD if any block protection
+                * is active.
+                */
+               sr |= SR_SRWD;
+       } else {
+               sr &= ~ATMEL_SR_GLOBAL_PROTECT_MASK;
+       }
+
+       nor->bouncebuf[0] = sr;
+
+       /*
+        * We cannot use the spi_nor_write_sr_and_check() because this command
+        * isn't really setting any bits, instead it is an pseudo command for
+        * "Global Unprotect" or "Global Protect"
+        */
+       return spi_nor_write_sr(nor, nor->bouncebuf, 1);
+}
+
+static int atmel_global_protect(struct spi_nor *nor, loff_t ofs, uint64_t len)
+{
+       return atmel_set_global_protection(nor, ofs, len, true);
+}
+
+static int atmel_global_unprotect(struct spi_nor *nor, loff_t ofs, uint64_t len)
+{
+       return atmel_set_global_protection(nor, ofs, len, false);
+}
+
+static int atmel_is_global_protected(struct spi_nor *nor, loff_t ofs, uint64_t len)
+{
+       int ret;
+
+       if (ofs >= nor->params->size || (ofs + len) > nor->params->size)
+               return -EINVAL;
+
+       ret = spi_nor_read_sr(nor, nor->bouncebuf);
+       if (ret)
+               return ret;
+
+       return ((nor->bouncebuf[0] & ATMEL_SR_GLOBAL_PROTECT_MASK) == ATMEL_SR_GLOBAL_PROTECT_MASK);
+}
+
+static const struct spi_nor_locking_ops atmel_global_protection_ops = {
+       .lock = atmel_global_protect,
+       .unlock = atmel_global_unprotect,
+       .is_locked = atmel_is_global_protected,
+};
+
+static void atmel_global_protection_default_init(struct spi_nor *nor)
+{
+       nor->params->locking_ops = &atmel_global_protection_ops;
+}
+
+static const struct spi_nor_fixups atmel_global_protection_fixups = {
+       .default_init = atmel_global_protection_default_init,
+};
+
 static const struct flash_info atmel_parts[] = {
        /* Atmel -- some are (confusingly) marketed as "DataFlash" */
        { "at25fs010",  INFO(0x1f6601, 0, 32 * 1024,   4, SECT_4K | SPI_NOR_HAS_LOCK)
@@ -62,18 +162,32 @@ static const struct flash_info atmel_parts[] = {
        { "at25fs040",  INFO(0x1f6604, 0, 64 * 1024,   8, SECT_4K | SPI_NOR_HAS_LOCK)
                .fixups = &atmel_at25fs_fixups },
 
-       { "at25df041a", INFO(0x1f4401, 0, 64 * 1024,   8, SECT_4K | SPI_NOR_HAS_LOCK) },
-       { "at25df321",  INFO(0x1f4700, 0, 64 * 1024,  64, SECT_4K | SPI_NOR_HAS_LOCK) },
-       { "at25df321a", INFO(0x1f4701, 0, 64 * 1024,  64, SECT_4K | SPI_NOR_HAS_LOCK) },
-       { "at25df641",  INFO(0x1f4800, 0, 64 * 1024, 128, SECT_4K | SPI_NOR_HAS_LOCK) },
+       { "at25df041a", INFO(0x1f4401, 0, 64 * 1024,   8,
+                            SECT_4K | SPI_NOR_HAS_LOCK | SPI_NOR_SWP_IS_VOLATILE)
+                       .fixups = &atmel_global_protection_fixups },
+       { "at25df321",  INFO(0x1f4700, 0, 64 * 1024,  64,
+                            SECT_4K | SPI_NOR_HAS_LOCK | SPI_NOR_SWP_IS_VOLATILE)
+                       .fixups = &atmel_global_protection_fixups },
+       { "at25df321a", INFO(0x1f4701, 0, 64 * 1024,  64,
+                            SECT_4K | SPI_NOR_HAS_LOCK | SPI_NOR_SWP_IS_VOLATILE)
+                       .fixups = &atmel_global_protection_fixups },
+       { "at25df641",  INFO(0x1f4800, 0, 64 * 1024, 128,
+                            SECT_4K | SPI_NOR_HAS_LOCK | SPI_NOR_SWP_IS_VOLATILE)
+                       .fixups = &atmel_global_protection_fixups },
 
        { "at25sl321",  INFO(0x1f4216, 0, 64 * 1024, 64,
                             SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
 
        { "at26f004",   INFO(0x1f0400, 0, 64 * 1024,  8, SECT_4K) },
-       { "at26df081a", INFO(0x1f4501, 0, 64 * 1024, 16, SECT_4K | SPI_NOR_HAS_LOCK) },
-       { "at26df161a", INFO(0x1f4601, 0, 64 * 1024, 32, SECT_4K | SPI_NOR_HAS_LOCK) },
-       { "at26df321",  INFO(0x1f4700, 0, 64 * 1024, 64, SECT_4K | SPI_NOR_HAS_LOCK) },
+       { "at26df081a", INFO(0x1f4501, 0, 64 * 1024, 16,
+                            SECT_4K | SPI_NOR_HAS_LOCK | SPI_NOR_SWP_IS_VOLATILE)
+                       .fixups = &atmel_global_protection_fixups },
+       { "at26df161a", INFO(0x1f4601, 0, 64 * 1024, 32,
+                            SECT_4K | SPI_NOR_HAS_LOCK | SPI_NOR_SWP_IS_VOLATILE)
+                       .fixups = &atmel_global_protection_fixups },
+       { "at26df321",  INFO(0x1f4700, 0, 64 * 1024, 64,
+                            SECT_4K | SPI_NOR_HAS_LOCK | SPI_NOR_SWP_IS_VOLATILE)
+                       .fixups = &atmel_global_protection_fixups },
 
        { "at45db081d", INFO(0x1f2500, 0, 64 * 1024, 16, SECT_4K) },
 };
index 53603ef..20df44b 100644 (file)
@@ -377,7 +377,7 @@ int spi_nor_write_disable(struct spi_nor *nor)
  *
  * Return: 0 on success, -errno otherwise.
  */
-static int spi_nor_read_sr(struct spi_nor *nor, u8 *sr)
+int spi_nor_read_sr(struct spi_nor *nor, u8 *sr)
 {
        int ret;
 
@@ -861,7 +861,7 @@ int spi_nor_wait_till_ready(struct spi_nor *nor)
  *
  * Return: 0 on success, -errno otherwise.
  */
-static int spi_nor_write_sr(struct spi_nor *nor, const u8 *sr, size_t len)
+int spi_nor_write_sr(struct spi_nor *nor, const u8 *sr, size_t len)
 {
        int ret;
 
@@ -3140,6 +3140,8 @@ static void spi_nor_try_unlock_all(struct spi_nor *nor)
        if (!(nor->flags & SNOR_F_HAS_LOCK))
                return;
 
+       dev_dbg(nor->dev, "Unprotecting entire flash array\n");
+
        ret = spi_nor_unlock(&nor->mtd, 0, nor->params->size);
        if (ret)
                dev_dbg(nor->dev, "Failed to unlock the entire flash memory array\n");
@@ -3161,7 +3163,20 @@ static int spi_nor_init(struct spi_nor *nor)
                return err;
        }
 
-       spi_nor_try_unlock_all(nor);
+       /*
+        * Some SPI NOR flashes are write protected by default after a power-on
+        * reset cycle, in order to avoid inadvertent writes during power-up.
+        * Backward compatibility imposes to unlock the entire flash memory
+        * array at power-up by default. Depending on the kernel configuration
+        * (1) do nothing, (2) always unlock the entire flash array or (3)
+        * unlock the entire flash array only when the software write
+        * protection bits are volatile. The latter is indicated by
+        * SNOR_F_SWP_IS_VOLATILE.
+        */
+       if (IS_ENABLED(CONFIG_MTD_SPI_NOR_SWP_DISABLE) ||
+           (IS_ENABLED(CONFIG_MTD_SPI_NOR_SWP_DISABLE_ON_VOLATILE) &&
+            nor->flags & SNOR_F_SWP_IS_VOLATILE))
+               spi_nor_try_unlock_all(nor);
 
        if (nor->addr_width == 4 &&
            nor->read_proto != SNOR_PROTO_8_8_8_DTR &&
@@ -3460,6 +3475,8 @@ int spi_nor_scan(struct spi_nor *nor, const char *name,
                nor->flags |= SNOR_F_NO_OP_CHIP_ERASE;
        if (info->flags & USE_CLSR)
                nor->flags |= SNOR_F_USE_CLSR;
+       if (info->flags & SPI_NOR_SWP_IS_VOLATILE)
+               nor->flags |= SNOR_F_SWP_IS_VOLATILE;
 
        if (info->flags & SPI_NOR_4BIT_BP) {
                nor->flags |= SNOR_F_HAS_4BIT_BP;
index caf8b7d..d631ee2 100644 (file)
@@ -28,6 +28,7 @@ enum spi_nor_option_flags {
        SNOR_F_HAS_SR_BP3_BIT6  = BIT(13),
        SNOR_F_IO_MODE_EN_VOLATILE = BIT(14),
        SNOR_F_SOFT_RESET       = BIT(15),
+       SNOR_F_SWP_IS_VOLATILE  = BIT(16),
 };
 
 struct spi_nor_read_command {
@@ -332,6 +333,11 @@ struct flash_info {
                                                 * available I/O mode via a
                                                 * volatile bit.
                                                 */
+#define SPI_NOR_SWP_IS_VOLATILE        BIT(22) /*
+                                        * Flash has volatile software write
+                                        * protection bits. Usually these will
+                                        * power-up in a write-protected state.
+                                        */
 
        /* Part specific fixup hooks. */
        const struct spi_nor_fixups *fixups;
@@ -433,6 +439,8 @@ void spi_nor_unlock_and_unprep(struct spi_nor *nor);
 int spi_nor_sr1_bit6_quad_enable(struct spi_nor *nor);
 int spi_nor_sr2_bit1_quad_enable(struct spi_nor *nor);
 int spi_nor_sr2_bit7_quad_enable(struct spi_nor *nor);
+int spi_nor_read_sr(struct spi_nor *nor, u8 *sr);
+int spi_nor_write_sr(struct spi_nor *nor, const u8 *sr, size_t len);
 int spi_nor_write_sr_and_check(struct spi_nor *nor, u8 sr1);
 
 int spi_nor_xread_sr(struct spi_nor *nor, u8 *sr);
index c931700..cfc9218 100644 (file)
@@ -11,7 +11,7 @@
 static const struct flash_info esmt_parts[] = {
        /* ESMT */
        { "f25l32pa", INFO(0x8c2016, 0, 64 * 1024, 64,
-                          SECT_4K | SPI_NOR_HAS_LOCK) },
+                          SECT_4K | SPI_NOR_HAS_LOCK | SPI_NOR_SWP_IS_VOLATILE) },
        { "f25l32qa", INFO(0x8c4116, 0, 64 * 1024, 64,
                           SECT_4K | SPI_NOR_HAS_LOCK) },
        { "f25l64qa", INFO(0x8c4117, 0, 64 * 1024, 128,
index 6c31bef..8ece9cc 100644 (file)
 
 static const struct flash_info intel_parts[] = {
        /* Intel/Numonyx -- xxxs33b */
-       { "160s33b",  INFO(0x898911, 0, 64 * 1024,  32, SPI_NOR_HAS_LOCK) },
-       { "320s33b",  INFO(0x898912, 0, 64 * 1024,  64, SPI_NOR_HAS_LOCK) },
-       { "640s33b",  INFO(0x898913, 0, 64 * 1024, 128, SPI_NOR_HAS_LOCK) },
+       { "160s33b",  INFO(0x898911, 0, 64 * 1024,  32,
+                          SPI_NOR_HAS_LOCK | SPI_NOR_SWP_IS_VOLATILE) },
+       { "320s33b",  INFO(0x898912, 0, 64 * 1024,  64,
+                          SPI_NOR_HAS_LOCK | SPI_NOR_SWP_IS_VOLATILE) },
+       { "640s33b",  INFO(0x898913, 0, 64 * 1024, 128,
+                          SPI_NOR_HAS_LOCK | SPI_NOR_SWP_IS_VOLATILE) },
 };
 
 const struct spi_nor_manufacturer spi_nor_intel = {
index 0d9d319..00e48da 100644 (file)
 static const struct flash_info sst_parts[] = {
        /* SST -- large erase sizes are "overlays", "sectors" are 4K */
        { "sst25vf040b", INFO(0xbf258d, 0, 64 * 1024,  8,
-                             SECT_4K | SST_WRITE | SPI_NOR_HAS_LOCK) },
+                             SECT_4K | SST_WRITE | SPI_NOR_HAS_LOCK | SPI_NOR_SWP_IS_VOLATILE) },
        { "sst25vf080b", INFO(0xbf258e, 0, 64 * 1024, 16,
-                             SECT_4K | SST_WRITE | SPI_NOR_HAS_LOCK) },
+                             SECT_4K | SST_WRITE | SPI_NOR_HAS_LOCK | SPI_NOR_SWP_IS_VOLATILE) },
        { "sst25vf016b", INFO(0xbf2541, 0, 64 * 1024, 32,
-                             SECT_4K | SST_WRITE | SPI_NOR_HAS_LOCK) },
+                             SECT_4K | SST_WRITE | SPI_NOR_HAS_LOCK | SPI_NOR_SWP_IS_VOLATILE) },
        { "sst25vf032b", INFO(0xbf254a, 0, 64 * 1024, 64,
-                             SECT_4K | SST_WRITE | SPI_NOR_HAS_LOCK) },
+                             SECT_4K | SST_WRITE | SPI_NOR_HAS_LOCK | SPI_NOR_SWP_IS_VOLATILE) },
        { "sst25vf064c", INFO(0xbf254b, 0, 64 * 1024, 128,
-                             SECT_4K | SPI_NOR_4BIT_BP | SPI_NOR_HAS_LOCK) },
+                             SECT_4K | SPI_NOR_4BIT_BP | SPI_NOR_HAS_LOCK |
+                             SPI_NOR_SWP_IS_VOLATILE) },
        { "sst25wf512",  INFO(0xbf2501, 0, 64 * 1024,  1,
-                             SECT_4K | SST_WRITE | SPI_NOR_HAS_LOCK) },
+                             SECT_4K | SST_WRITE | SPI_NOR_HAS_LOCK | SPI_NOR_SWP_IS_VOLATILE) },
        { "sst25wf010",  INFO(0xbf2502, 0, 64 * 1024,  2,
-                             SECT_4K | SST_WRITE | SPI_NOR_HAS_LOCK) },
+                             SECT_4K | SST_WRITE | SPI_NOR_HAS_LOCK | SPI_NOR_SWP_IS_VOLATILE) },
        { "sst25wf020",  INFO(0xbf2503, 0, 64 * 1024,  4,
-                             SECT_4K | SST_WRITE | SPI_NOR_HAS_LOCK) },
+                             SECT_4K | SST_WRITE | SPI_NOR_HAS_LOCK | SPI_NOR_SWP_IS_VOLATILE) },
        { "sst25wf020a", INFO(0x621612, 0, 64 * 1024,  4, SECT_4K | SPI_NOR_HAS_LOCK) },
        { "sst25wf040b", INFO(0x621613, 0, 64 * 1024,  8, SECT_4K | SPI_NOR_HAS_LOCK) },
        { "sst25wf040",  INFO(0xbf2504, 0, 64 * 1024,  8,
-                             SECT_4K | SST_WRITE | SPI_NOR_HAS_LOCK) },
+                             SECT_4K | SST_WRITE | SPI_NOR_HAS_LOCK | SPI_NOR_SWP_IS_VOLATILE) },
        { "sst25wf080",  INFO(0xbf2505, 0, 64 * 1024, 16,
-                             SECT_4K | SST_WRITE | SPI_NOR_HAS_LOCK) },
+                             SECT_4K | SST_WRITE | SPI_NOR_HAS_LOCK | SPI_NOR_SWP_IS_VOLATILE) },
        { "sst26wf016b", INFO(0xbf2651, 0, 64 * 1024, 32,
                              SECT_4K | SPI_NOR_DUAL_READ |
                              SPI_NOR_QUAD_READ) },