[U-Boot] [PATCH v4 3/3] spi: Add SPI NOR protection mechanism

Jagan Teki jteki at openedev.com
Thu Oct 1 15:48:23 CEST 2015


On 30 September 2015 at 23:33, Fabio Estevam <festevam at gmail.com> wrote:
> From: Fabio Estevam <fabio.estevam at freescale.com>
>
> Many SPI flashes have protection bits (BP2, BP1 and BP0) in the
> status register that can protect selected regions of the SPI NOR.
>
> Take these bits into account when performing erase operations,
> making sure that the protected areas are skipped.
>
> Introduce the CONFIG_SPI_FLASH_STM_PROTECT option that can be
> selectedby systems that want to protect regions of SPI NOR flash
> using the same programming model as in the ST Micro SPI NOR flashes,
> like for example the M25P32.
>
> Tested on a mx6qsabresd:
>
> => sf probe
> SF: Detected M25P32 with page size 256 Bytes, erase size 64 KiB, total 4 MiB
> => sf protect on  0x3f0000 0x10000
> => sf erase 0x3f0000 0x10000
> offset 0x3f0000 is protected and cannot be erased
> SF: 65536 bytes @ 0x3f0000 Erased: ERROR
> => sf protect off  0x3f0000 0x10000
> => sf erase 0x3f0000 0x10000
> SF: 65536 bytes @ 0x3f0000 Erased: OK
>
> Signed-off-by: Fabio Estevam <fabio.estevam at freescale.com>
> ---
> Changes since v3:
> - Split this patch in two parts (Jagan)
>
>  common/cmd_sf.c               | 35 +++++++++++++++++++++++++++++++++++
>  drivers/mtd/spi/Kconfig       | 15 +++++++++++++++
>  drivers/mtd/spi/sf_internal.h | 10 ++++------
>  drivers/mtd/spi/sf_ops.c      | 33 +++++++++++++++++++++++++++++++++
>  drivers/mtd/spi/sf_probe.c    | 27 +++++++++++++++++++++++++++
>  include/spi_flash.h           | 18 ++++++++++++++++++
>  6 files changed, 132 insertions(+), 6 deletions(-)
>
> diff --git a/common/cmd_sf.c b/common/cmd_sf.c
> index ac7f5df..bb39cb7 100644
> --- a/common/cmd_sf.c
> +++ b/common/cmd_sf.c
> @@ -348,6 +348,37 @@ static int do_spi_flash_erase(int argc, char * const argv[])
>         return ret == 0 ? 0 : 1;
>  }
>
> +static int do_spi_protect(int argc, char * const argv[])
> +{
> +       int ret = 0;
> +       loff_t start, len;
> +       bool prot = false;
> +
> +       if (argc != 4)
> +               return -1;
> +
> +       if (!str2off(argv[2], &start)) {
> +               puts("start sector is not a valid number\n");
> +               return 1;
> +       }
> +
> +       if (!str2off(argv[3], &len)) {
> +               puts("len is not a valid number\n");
> +               return 1;
> +       }
> +
> +       if (strcmp(argv[1], "on") == 0)
> +               prot = true;
> +       else if (strcmp(argv[1], "off") == 0)
> +               prot = false;
> +       else
> +               return -1;  /* Unknown parameter */
> +
> +       ret = spi_flash_protect(flash, start, len, prot);
> +
> +       return ret == 0 ? 0 : 1;
> +}
> +
>  #ifdef CONFIG_CMD_SF_TEST
>  enum {
>         STAGE_ERASE,
> @@ -540,6 +571,8 @@ static int do_spi_flash(cmd_tbl_t *cmdtp, int flag, int argc,
>                 ret = do_spi_flash_read_write(argc, argv);
>         else if (strcmp(cmd, "erase") == 0)
>                 ret = do_spi_flash_erase(argc, argv);
> +       else if (strcmp(cmd, "protect") == 0)
> +               ret = do_spi_protect(argc, argv);
>  #ifdef CONFIG_CMD_SF_TEST
>         else if (!strcmp(cmd, "test"))
>                 ret = do_spi_flash_test(argc, argv);
> @@ -579,5 +612,7 @@ U_BOOT_CMD(
>         "sf update addr offset|partition len    - erase and write `len' bytes from memory\n"
>         "                                         at `addr' to flash at `offset'\n"
>         "                                         or to start of mtd `partition'\n"
> +       "sf protect on/off sector len           - protect/unprotect 'len' bytes starting\n"
> +       "                                         at address 'sector'\n"

To be in-line with mtd lock utils, how about this notation
sf protect lock
sf protect unlock
sf protect islocked

>         SF_TEST_HELP
>  );
> diff --git a/drivers/mtd/spi/Kconfig b/drivers/mtd/spi/Kconfig
> index 3f7433c..2ee1089 100644
> --- a/drivers/mtd/spi/Kconfig
> +++ b/drivers/mtd/spi/Kconfig
> @@ -101,6 +101,21 @@ config SPI_FLASH_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 SPI_FLASH_STM_PROTECT

Remove this macro, just use existing CONFIG_SPI_FLASH_STMICRO as all
these code is specific to these parts.

> +       bool "Use STM flash protection mechanism"
> +       depends on SPI_FLASH
> +       help
> +         Enable the built-in protection mechanism provided by the
> +         BP2, BP1 and BP0 bits from the status register present
> +         on ST-Micro flashes such as M25P32. Please refer to the
> +         M25P32 datasheet to understand how to program these bits
> +         in order to protect a selected region of the SPI NOR flash.
> +
> +         This same bit protection programming model applies to SPI
> +         NOR flashes from other manufacturers such as:
> +         - Micron M25P32
> +         - SST SST25V32B
> +
>  config SPI_FLASH_DATAFLASH
>         bool "AT45xxx DataFlash support"
>         depends on SPI_FLASH && DM_SPI_FLASH
> diff --git a/drivers/mtd/spi/sf_internal.h b/drivers/mtd/spi/sf_internal.h
> index 9c95d56..8fb1b18 100644
> --- a/drivers/mtd/spi/sf_internal.h
> +++ b/drivers/mtd/spi/sf_internal.h
> @@ -162,12 +162,6 @@ int spi_flash_cmd_write(struct spi_slave *spi, const u8 *cmd, size_t cmd_len,
>  /* Flash erase(sectors) operation, support all possible erase commands */
>  int spi_flash_cmd_erase_ops(struct spi_flash *flash, u32 offset, size_t len);
>
> -/* Read the status register */
> -int spi_flash_cmd_read_status(struct spi_flash *flash, u8 *rs);
> -
> -/* Program the status register */
> -int spi_flash_cmd_write_status(struct spi_flash *flash, u8 ws);

Please don't remove this stuff - this is mtd/spi

> -
>  /* Read the config register */
>  int spi_flash_cmd_read_config(struct spi_flash *flash, u8 *rc);
>
> @@ -222,6 +216,10 @@ int spi_flash_read_common(struct spi_flash *flash, const u8 *cmd,
>  int spi_flash_cmd_read_ops(struct spi_flash *flash, u32 offset,
>                 size_t len, void *data);
>
> +int spi_flash_cmd_lock_ops(struct spi_flash *flash, u32 offset, size_t len);
> +int spi_flash_cmd_unlock_ops(struct spi_flash *flash, u32 offset, size_t len);
> +int spi_flash_cmd_is_locked_ops(struct spi_flash *flash, u32 offset, size_t len);
> +
>  #ifdef CONFIG_SPI_FLASH_MTD
>  int spi_flash_mtd_register(struct spi_flash *flash);
>  void spi_flash_mtd_unregister(void);
> diff --git a/drivers/mtd/spi/sf_ops.c b/drivers/mtd/spi/sf_ops.c
> index e12f8ee..281b06d 100644
> --- a/drivers/mtd/spi/sf_ops.c
> +++ b/drivers/mtd/spi/sf_ops.c
> @@ -276,6 +276,11 @@ int spi_flash_cmd_erase_ops(struct spi_flash *flash, u32 offset, size_t len)
>                 return -1;
>         }
>
> +       if (flash->is_locked(flash, offset, len) > 0) {
> +               printf("offset 0x%x is protected and cannot be erased\n", offset);
> +               return -EINVAL;
> +       }

Can we handle - if the part of the sectors were protected and
remaining were not, so it will erase only unprotected sectors and show
"these many sectors were locked and unable to erase"?

> +
>         cmd[0] = flash->erase_cmd;
>         while (len) {
>                 erase_addr = offset;
> @@ -318,6 +323,11 @@ int spi_flash_cmd_write_ops(struct spi_flash *flash, u32 offset,
>
>         page_size = flash->page_size;
>
> +       if (flash->is_locked(flash, offset, len) > 0) {
> +               printf("offset 0x%x is protected and cannot be written\n", offset);
> +               return -EINVAL;
> +       }
> +
>         cmd[0] = flash->write_cmd;
>         for (actual = 0; actual < len; actual += chunk_len) {
>                 write_addr = offset;
> @@ -356,6 +366,21 @@ int spi_flash_cmd_write_ops(struct spi_flash *flash, u32 offset,
>         return ret;
>  }
>
> +int spi_flash_cmd_lock_ops(struct spi_flash *flash, u32 offset, size_t len)
> +{
> +       return stm_lock(flash, offset, len);
> +}
> +
> +int spi_flash_cmd_unlock_ops(struct spi_flash *flash, u32 offset, size_t len)
> +{
> +       return stm_unlock(flash, offset, len);
> +}
> +
> +int spi_flash_cmd_is_locked_ops(struct spi_flash *flash, u32 offset, size_t len)
> +{
> +       return stm_is_locked(flash, offset, len);
> +}
> +
>  int spi_flash_read_common(struct spi_flash *flash, const u8 *cmd,
>                 size_t cmd_len, void *data, size_t data_len)
>  {
> @@ -761,3 +786,11 @@ int stm_unlock(struct spi_flash *nor, u32 ofs, u32 len)
>         return 0;
>  }
>  #endif  /* CONFIG_SPI_FLASH_STM_PROTECT */
> +
> +int spi_flash_protect(struct spi_flash *nor, loff_t ofs, u32 len, bool prot)
> +{
> +       if (prot)
> +               return nor->lock(nor, ofs, len);
> +       else
> +               return nor->unlock(nor, ofs, len);

%s/nor/flash/g

Define this spi_flash_protect in include/spi_flash not in ops.

> +}
> diff --git a/drivers/mtd/spi/sf_probe.c b/drivers/mtd/spi/sf_probe.c
> index 954376d..6da9d6c 100644
> --- a/drivers/mtd/spi/sf_probe.c
> +++ b/drivers/mtd/spi/sf_probe.c
> @@ -149,6 +149,9 @@ static int spi_flash_validate_params(struct spi_slave *spi, u8 *idcode,
>  #endif
>         flash->erase = spi_flash_cmd_erase_ops;
>         flash->read = spi_flash_cmd_read_ops;
> +       flash->lock = spi_flash_cmd_lock_ops;
> +       flash->unlock = spi_flash_cmd_unlock_ops;
> +       flash->is_locked = spi_flash_cmd_is_locked_ops;
>  #endif
>
>         /* Compute the flash size */
> @@ -469,6 +472,27 @@ int spi_flash_std_erase(struct udevice *dev, u32 offset, size_t len)
>         return spi_flash_cmd_erase_ops(flash, offset, len);
>  }
>
> +int spi_flash_std_lock(struct udevice *dev, u32 offset, size_t len)
> +{
> +       struct spi_flash *flash = dev_get_uclass_priv(dev);
> +
> +       return spi_flash_cmd_lock_ops(flash, offset, len);
> +}
> +
> +int spi_flash_std_unlock(struct udevice *dev, u32 offset, size_t len)
> +{
> +       struct spi_flash *flash = dev_get_uclass_priv(dev);
> +
> +       return spi_flash_cmd_unlock_ops(flash, offset, len);
> +}
> +
> +int spi_flash_std_is_locked(struct udevice *dev, u32 offset, size_t len)
> +{
> +       struct spi_flash *flash = dev_get_uclass_priv(dev);
> +
> +       return spi_flash_cmd_is_locked_ops(flash, offset, len);
> +}
> +
>  int spi_flash_std_probe(struct udevice *dev)
>  {
>         struct spi_slave *slave = dev_get_parentdata(dev);
> @@ -485,6 +509,9 @@ static const struct dm_spi_flash_ops spi_flash_std_ops = {
>         .read = spi_flash_std_read,
>         .write = spi_flash_std_write,
>         .erase = spi_flash_std_erase,
> +       .lock = spi_flash_std_lock,
> +       .unlock = spi_flash_std_unlock,
> +       .is_locked = spi_flash_std_is_locked,
>  };
>
>  static const struct udevice_id spi_flash_std_ids[] = {
> diff --git a/include/spi_flash.h b/include/spi_flash.h
> index 3b2d555..28ec357 100644
> --- a/include/spi_flash.h
> +++ b/include/spi_flash.h
> @@ -105,6 +105,9 @@ struct spi_flash {
>         int (*write)(struct spi_flash *flash, u32 offset, size_t len,
>                         const void *buf);
>         int (*erase)(struct spi_flash *flash, u32 offset, size_t len);
> +       int (*lock)(struct spi_flash *flash, u32 offset, size_t len);
> +       int (*unlock)(struct spi_flash *flash, u32 offset, size_t len);
> +       int (*is_locked)(struct spi_flash *flash, u32 offset, size_t len);
>  #endif
>  };
>
> @@ -113,8 +116,23 @@ struct dm_spi_flash_ops {
>         int (*write)(struct udevice *dev, u32 offset, size_t len,
>                      const void *buf);
>         int (*erase)(struct udevice *dev, u32 offset, size_t len);
> +       int (*lock)(struct udevice *dev, u32 offset, size_t len);
> +       int (*unlock)(struct udevice *dev, u32 offset, size_t len);
> +       int (*is_locked)(struct udevice *dev, u32 offset, size_t len);
>  };
>
> +
> +/* Read the status register */
> +int spi_flash_cmd_read_status(struct spi_flash *flash, u8 *rs);
> +
> +/* Program the status register */
> +int spi_flash_cmd_write_status(struct spi_flash *flash, u8 ws);
> +
> +int stm_is_locked(struct spi_flash *nor, loff_t ofs, u32 len);
> +int stm_lock(struct spi_flash *nor, u32 ofs, u32 len);
> +int stm_unlock(struct spi_flash *nor, u32 ofs, u32 len);
> +int spi_flash_protect(struct spi_flash *nor, loff_t ofs, u32 len, bool prot);

Move these into mtd/spi

> +
>  /* Access the serial operations for a device */
>  #define sf_get_ops(dev) ((struct dm_spi_flash_ops *)(dev)->driver->ops)
>

-- 
Jagan | openedev.


More information about the U-Boot mailing list