[U-Boot] [PATCH v5 17/18] spi: Add SPI NOR protection mechanism
Jagan Teki
jteki at openedev.com
Tue Nov 3 15:51:32 CET 2015
Hi Fabio,
On 3 November 2015 at 05:08, 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.
>
> Tested on a mx6qsabresd:
>
> => sf probe
> SF: Detected M25P32 with page size 256 Bytes, erase size 64 KiB, total 4 MiB
> => sf protect lock 0x3f0000 0x10000
> => sf erase 0x3f0000 0x10000
> offset 0x3f0000 is protected and cannot be erased
> SF: 65536 bytes @ 0x3f0000 Erased: ERROR
> => sf protect unlock 0x3f0000 0x10000
> => sf erase 0x3f0000 0x10000
> SF: 65536 bytes @ 0x3f0000 Erased: OK
>
> Signed-off-by: Fabio Estevam <fabio.estevam at freescale.com>
> ---
> Changes since v4:
> - Only apply lock operations if the SPI NOR flash ID matches STMicro (Jagan)
>
> common/cmd_sf.c | 35 +++++++++++++++++++++++++
> drivers/mtd/spi/sf-uclass.c | 8 ++++++
> drivers/mtd/spi/sf_internal.h | 8 ++++++
> drivers/mtd/spi/sf_ops.c | 25 ++++++++++++++++++
> drivers/mtd/spi/sf_probe.c | 61 +++++++++++++++++++++++++++++++++++++++++++
> include/spi_flash.h | 41 +++++++++++++++++++++++++++++
> 6 files changed, 178 insertions(+)
>
> diff --git a/common/cmd_sf.c b/common/cmd_sf.c
> index ac7f5df..42862d9 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], "lock") == 0)
> + prot = true;
> + else if (strcmp(argv[1], "unlock") == 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 lock/unlock sector len - protect/unprotect 'len' bytes starting\n"
> + " at address 'sector'\n"
> SF_TEST_HELP
> );
> diff --git a/drivers/mtd/spi/sf-uclass.c b/drivers/mtd/spi/sf-uclass.c
> index 350e21a..7663885 100644
> --- a/drivers/mtd/spi/sf-uclass.c
> +++ b/drivers/mtd/spi/sf-uclass.c
> @@ -27,6 +27,14 @@ int spi_flash_erase_dm(struct udevice *dev, u32 offset, size_t len)
> return sf_get_ops(dev)->erase(dev, offset, len);
> }
>
> +int spi_flash_protect_dm(struct udevice *dev, u32 offset, size_t len, bool prot)
> +{
> + if (prot)
> + return sf_get_ops(dev)->lock(dev, offset, len);
> + else
> + return sf_get_ops(dev)->unlock(dev, offset, len);
> +}
> +
> /*
> * TODO(sjg at chromium.org): This is an old-style function. We should remove
> * it when all SPI flash drivers use dm
> diff --git a/drivers/mtd/spi/sf_internal.h b/drivers/mtd/spi/sf_internal.h
> index adfcd89..db46aa8 100644
> --- a/drivers/mtd/spi/sf_internal.h
> +++ b/drivers/mtd/spi/sf_internal.h
> @@ -177,6 +177,10 @@ 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);
> +
> /* Read the config register */
> int spi_flash_cmd_read_config(struct spi_flash *flash, u8 *rc);
>
> @@ -231,6 +235,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 4ada13f..99fc7f9 100644
> --- a/drivers/mtd/spi/sf_ops.c
> +++ b/drivers/mtd/spi/sf_ops.c
> @@ -268,6 +268,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;
> + }
> +
> cmd[0] = flash->erase_cmd;
> while (len) {
> erase_addr = offset;
> @@ -310,6 +315,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;
> + }
This flash lock check related to non-dm lock ops so it's look not good
to me as we assigned lock ops for both dm and non-dm cases.
> +
> cmd[0] = flash->write_cmd;
> for (actual = 0; actual < len; actual += chunk_len) {
> write_addr = offset;
> @@ -348,6 +358,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)
> {
> diff --git a/drivers/mtd/spi/sf_probe.c b/drivers/mtd/spi/sf_probe.c
> index c000c53..3626433 100644
> --- a/drivers/mtd/spi/sf_probe.c
> +++ b/drivers/mtd/spi/sf_probe.c
> @@ -130,6 +130,22 @@ bank_end:
> }
> #endif
>
> +int static is_stm(struct spi_slave *spi, struct spi_flash *flash)
> +{
> + u8 idcode[5];
> + int ret;
> +
> + /* Read the ID codes */
> + ret = spi_flash_cmd(spi, CMD_READ_ID, idcode, sizeof(idcode));
> + if (ret < 0)
> + return ret;
This is duplicate we have already got the idcode.
> +
> + if (idcode[0] == SPI_FLASH_CFI_MFR_STMICRO)
> + return 1;
> + else
> + return 0;
> +}
> +
> static int spi_flash_validate_params(struct spi_slave *spi, u8 *idcode,
> struct spi_flash *flash)
> {
> @@ -180,6 +196,13 @@ 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;
> +#if defined(CONFIG_SPI_FLASH_STMICRO)
> + if (is_stm(spi, flash) > 0) {
> + flash->lock = spi_flash_cmd_lock_ops;
> + flash->unlock = spi_flash_cmd_unlock_ops;
> + flash->is_locked = spi_flash_cmd_is_locked_ops;
> + }
> +#endif
> #endif
>
> /* Compute the flash size */
> @@ -482,6 +505,39 @@ 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);
> + struct spi_slave *spi = dev_get_parent_priv(dev);
> +
> + if (is_stm(spi, flash) <= 0)
> + return -EOPNOTSUPP;
> +
> + 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);
> + struct spi_slave *spi = dev_get_parent_priv(dev);
> +
> + if (is_stm(spi, flash) <= 0)
> + return -EOPNOTSUPP;
> +
> + 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);
> + struct spi_slave *spi = dev_get_parent_priv(dev);
> +
> + if (is_stm(spi, flash) <= 0)
> + return -EOPNOTSUPP;
> +
> + return spi_flash_cmd_is_locked_ops(flash, offset, len);
> +}
> +
> int spi_flash_std_probe(struct udevice *dev)
> {
> struct spi_slave *slave = dev_get_parent_priv(dev);
> @@ -498,6 +554,11 @@ 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,
> +#if defined(CONFIG_SPI_FLASH_STMICRO)
> + .lock = spi_flash_std_lock,
> + .unlock = spi_flash_std_unlock,
> + .is_locked = spi_flash_std_is_locked,
> +#endif
> };
>
> static const struct udevice_id spi_flash_std_ids[] = {
> diff --git a/include/spi_flash.h b/include/spi_flash.h
> index 4312d3d..23f30a5 100644
> --- a/include/spi_flash.h
> +++ b/include/spi_flash.h
> @@ -104,6 +104,9 @@ struct spi_flash {
> const void *buf);
> int (*erase)(struct spi_flash *flash, u32 offset, size_t len);
> #endif
> + 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);
> };
>
> struct dm_spi_flash_ops {
> @@ -111,6 +114,9 @@ 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);
> };
>
> /* Access the serial operations for a device */
> @@ -152,6 +158,20 @@ int spi_flash_write_dm(struct udevice *dev, u32 offset, size_t len,
> */
> int spi_flash_erase_dm(struct udevice *dev, u32 offset, size_t len);
>
> +/**
> + * spi_flash_protect_dm() - Protect/unprotect blocks of the SPI flash
> + *
> + * Note that @len must be a muiltiple of the flash sector size.
> + *
> + * @dev: SPI flash device
> + * @offset: Offset into device in bytes to start erasing
> + * @len: Number of bytes to erase
> + * @prot: True: lock the block or False: unlock the block
> + * @return 0 if OK, -ve on error
> + */
> +int spi_flash_protect_dm(struct udevice *dev, u32 offset, size_t len,
> + bool prot);
> +
> int spi_flash_probe_bus_cs(unsigned int busnum, unsigned int cs,
> unsigned int max_hz, unsigned int spi_mode,
> struct udevice **devp);
> @@ -183,6 +203,15 @@ static inline int spi_flash_erase(struct spi_flash *flash, u32 offset,
> return spi_flash_erase_dm(flash->dev, offset, len);
> }
>
> +static inline int spi_flash_protect(struct spi_flash *flash, loff_t ofs,
> + u32 len, bool prot)
> +{
> + if (!flash->lock)
> + return -EOPNOTSUPP;
> +
> + return spi_flash_protect_dm(flash->dev, ofs, len, prot);
> +}
> +
> struct sandbox_state;
>
> int sandbox_sf_bind_emul(struct sandbox_state *state, int busnum, int cs,
> @@ -225,6 +254,18 @@ static inline int spi_flash_erase(struct spi_flash *flash, u32 offset,
> {
> return flash->erase(flash, offset, len);
> }
> +
> +static inline int spi_flash_protect(struct spi_flash *flash, u32 ofs, u32 len,
> + bool prot)
> +{
> + if (!flash->lock)
> + return -EOPNOTSUPP;
> +
> + if (prot)
> + return flash->lock(flash, ofs, len);
> + else
> + return flash->unlock(flash, ofs, len);
> +}
> #endif
>
> void spi_boot(void) __noreturn;
> --
> 1.9.1
>
I will come back for this patch changes what I thought, probably lock
ops shouldn't be separate for dm and non-dm as we have different ops
based on the flash itself.
--
Jagan | openedev.
More information about the U-Boot
mailing list