Skip to contentSkip to conversation

bitops: Add support for order_base_2()

PATCH Superseded Fabio Estevam 2 participants 7 messages Sep 30, 2015 – Oct 5, 2015

Subsystems (3)
Arm (32-bit) Memory technology devices (MTD) Bitops API

v4 of 5 Superseded Latest is v5 →

lore
Share & tools
lore

Fabio Estevam

author
#0 lore raw

PATCH v4 1/3 bitops: Add support for order_base_2()

From: Fabio Estevam <[email protected]>

Add support for the order_base_2() macro (and its dependencies) from the Linux kernel.

This is useful for the SPI NOR unlock function.

Signed-off-by Fabio Estevam <[email protected]>

---
Changes since v3:
- None

 arch/arm/include/asm/bitops.h | 56 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 56 insertions(+)
arch/arm/include/asm/bitops.h +56 -0
index 9b78043..b9b6d21 100644
@@ -108,6 +108,62 @@ static inline int __ilog2(unsigned int x)
 	return generic_fls(x) - 1;
 }
 
+static inline int fls64(__u64 x)
+{
+	if (x == 0)
+		return 0;
+	return generic_fls(x) + 1;
+}
+
+static inline unsigned fls_long(unsigned long l)
+{
+	if (sizeof(l) == 4)
+		return generic_fls(l);
+	return fls64(l);
+}
+
+/*
+ * round up to nearest power of two
+ */
+static inline __attribute__((const))
+unsigned long __roundup_pow_of_two(unsigned long n)
+{
+	return 1UL << fls_long(n - 1);
+}
+
+/**
+ * roundup_pow_of_two - round the given value up to nearest power of two
+ * @n - parameter
+ *
+ * round the given value up to the nearest power of two
+ * - the result is undefined when n == 0
+ * - this can be used to initialise global variables from constant data
+ */
+#define roundup_pow_of_two(n)			\
+(						\
+	__builtin_constant_p(n) ? (		\
+		(n == 1) ? 1 :			\
+		(1UL << (__ilog2((n) - 1) + 1))	\
+				   ) :		\
+	__roundup_pow_of_two(n)			\
+)
+
+/**
+ * order_base_2 - calculate the (rounded up) base 2 order of the argument
+ * @n: parameter
+ *
+ * The first few values calculated by this routine:
+ *  ob2(0) = 0
+ *  ob2(1) = 0
+ *  ob2(2) = 1
+ *  ob2(3) = 2
+ *  ob2(4) = 2
+ *  ob2(5) = 3
+ *  ... and so on.
+ */
+
+#define order_base_2(n) __ilog2(roundup_pow_of_two(n))
+
 /*
  * ffz = Find First Zero in word. Undefined if no zero exists,
  * so code should check against ~0UL first..
Signature · 1 line
1.9.1
author
#1 lore raw

PATCH v4 2/3 spi: sf_ops: Add SPI protection mechanism from the kernel

From: Fabio Estevam <[email protected]>

Add the SPI NOR protection mechanism from the kernel.

This code is based on the work from Brian Norris <[email protected]>
https://patchwork.ozlabs.org/patch/513041/

Signed-off-by Fabio Estevam <[email protected]>

---
Changes since v1:
- Newly introduced as Jagan asked to split the original patch.

 drivers/mtd/spi/sf_ops.c | 188 +++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 188 insertions(+)
drivers/mtd/spi/sf_ops.c +188 -0
index 900ec1f..e12f8ee 100644
@@ -573,3 +573,191 @@ int sst_write_bp(struct spi_flash *flash, u32 offset, size_t len,
 	return ret;
 }
 #endif
+
+#ifdef CONFIG_SPI_FLASH_STM_PROTECT
+#define SR_BP0			BIT(2)	/* Block protect 0 */
+#define SR_BP1			BIT(3)	/* Block protect 1 */
+#define SR_BP2			BIT(4)	/* Block protect 2 */
+
+static void stm_get_locked_range(struct spi_flash *nor, u8 sr, loff_t *ofs,
+				 u32 *len)
+{
+	u8 mask = SR_BP2 | SR_BP1 | SR_BP0;
+	int shift = ffs(mask) - 1;
+	int pow;
+
+	if (!(sr & mask)) {
+		/* No protection */
+		*ofs = 0;
+		*len = 0;
+	} else {
+		pow = ((sr & mask) ^ mask) >> shift;
+		*len = nor->size >> pow;
+		*ofs = nor->size - *len;
+	}
+}
+
+/*
+ * Return 1 if the entire region is locked, 0 otherwise
+ */
+static int stm_is_locked_sr(struct spi_flash *nor, loff_t ofs, u32 len,
+			    u8 sr)
+{
+	loff_t lock_offs;
+	u32 lock_len;
+
+	stm_get_locked_range(nor, sr, &lock_offs, &lock_len);
+
+	return (ofs + len <= lock_offs + lock_len) && (ofs >= lock_offs);
+}
+
+/*
+ * Check if a region of the flash is (completely) locked. See stm_lock() for
+ * more info.
+ *
+ * Returns 1 if entire region is locked, 0 if any portion is unlocked, and
+ * negative on errors.
+ */
+int stm_is_locked(struct spi_flash *nor, loff_t ofs, u32 len)
+{
+	int status;
+	u8 sr;
+
+	status = spi_flash_cmd_read_status(nor, &sr);
+	if (status < 0)
+		return status;
+
+	return stm_is_locked_sr(nor, ofs, len, sr);
+}
+
+/*
+ * Lock a region of the flash. Compatible with ST Micro and similar flash.
+ * Supports only the block protection bits BP{0,1,2} in the status register
+ * (SR). Does not support these features found in newer SR bitfields:
+ *   - TB: top/bottom protect - only handle TB=0 (top protect)
+ *   - SEC: sector/block protect - only handle SEC=0 (block protect)
+ *   - CMP: complement protect - only support CMP=0 (range is not complemented)
+ *
+ * Sample table portion for 8MB flash (Winbond w25q64fw):
+ *
+ *   SEC  |  TB   |  BP2  |  BP1  |  BP0  |  Prot Length  | Protected Portion
+ *  --------------------------------------------------------------------------
+ *    X   |   X   |   0   |   0   |   0   |  NONE         | NONE
+ *    0   |   0   |   0   |   0   |   1   |  128 KB       | Upper 1/64
+ *    0   |   0   |   0   |   1   |   0   |  256 KB       | Upper 1/32
+ *    0   |   0   |   0   |   1   |   1   |  512 KB       | Upper 1/16
+ *    0   |   0   |   1   |   0   |   0   |  1 MB         | Upper 1/8
+ *    0   |   0   |   1   |   0   |   1   |  2 MB         | Upper 1/4
+ *    0   |   0   |   1   |   1   |   0   |  4 MB         | Upper 1/2
+ *    X   |   X   |   1   |   1   |   1   |  8 MB         | ALL
+ *
+ * Returns negative on errors, 0 on success.
+ */
+int stm_lock(struct spi_flash *nor, u32 ofs, u32 len)
+{
+	u8 status_old, status_new;
+	u8 mask = SR_BP2 | SR_BP1 | SR_BP0;
+	u8 shift = ffs(mask) - 1, pow, val;
+
+	spi_flash_cmd_read_status(nor, &status_old);
+
+	/* SPI NOR always locks to the end */
+	if (ofs + len != nor->size) {
+		/* Does combined region extend to end? */
+		if (!stm_is_locked_sr(nor, ofs + len, nor->size - ofs - len,
+				      status_old))
+			return -EINVAL;
+		len = nor->size - ofs;
+	}
+
+	/*
+	 * Need smallest pow such that:
+	 *
+	 *   1 / (2^pow) <= (len / size)
+	 *
+	 * so (assuming power-of-2 size) we do:
+	 *
+	 *   pow = ceil(log2(size / len)) = log2(size) - floor(log2(len))
+	 */
+	pow = __ilog2(nor->size) - __ilog2(len);
+	val = mask - (pow << shift);
+	if (val & ~mask)
+		return -EINVAL;
+
+	/* Don't "lock" with no region! */
+	if (!(val & mask))
+		return -EINVAL;
+
+	status_new = (status_old & ~mask) | val;
+
+	/* Only modify protection if it will not unlock other areas */
+	if ((status_new & mask) <= (status_old & mask))
+		return -EINVAL;
+
+	spi_flash_cmd_write_status(nor, status_new);
+
+	return 0;
+}
+
+/*
+ * Unlock a region of the flash. See stm_lock() for more info
+ *
+ * Returns negative on errors, 0 on success.
+ */
+int stm_unlock(struct spi_flash *nor, u32 ofs, u32 len)
+{
+	uint8_t status_old, status_new;
+	u8 mask = SR_BP2 | SR_BP1 | SR_BP0;
+	u8 shift = ffs(mask) - 1, pow, val;
+
+	spi_flash_cmd_read_status(nor, &status_old);
+
+	/* Cannot unlock; would unlock larger region than requested */
+	if (stm_is_locked_sr(nor, status_old, ofs - nor->erase_size,
+			     nor->erase_size))
+		return -EINVAL;
+	/*
+	 * Need largest pow such that:
+	 *
+	 *   1 / (2^pow) >= (len / size)
+	 *
+	 * so (assuming power-of-2 size) we do:
+	 *
+	 *   pow = floor(log2(size / len)) = log2(size) - ceil(log2(len))
+	 */
+	pow = __ilog2(nor->size) - order_base_2(nor->size - (ofs + len));
+	if (ofs + len == nor->size) {
+		val = 0; /* fully unlocked */
+	} else {
+		val = mask - (pow << shift);
+		/* Some power-of-two sizes are not supported */
+		if (val & ~mask)
+			return -EINVAL;
+	}
+
+	status_new = (status_old & ~mask) | val;
+
+	/* Only modify protection if it will not lock other areas */
+	if ((status_new & mask) >= (status_old & mask))
+		return -EINVAL;
+
+	spi_flash_cmd_write_status(nor, status_new);
+
+	return 0;
+}
+#else
+int stm_is_locked(struct spi_flash *nor, loff_t ofs, u32 len)
+{
+	return 0;
+}
+
+int stm_lock(struct spi_flash *nor, u32 ofs, u32 len)
+{
+	return 0;
+}
+
+int stm_unlock(struct spi_flash *nor, u32 ofs, u32 len)
+{
+	return 0;
+}
+#endif  /* CONFIG_SPI_FLASH_STM_PROTECT */
Signature · 1 line
1.9.1
author
#2 lore raw

PATCH v4 3/3 spi: Add SPI NOR protection mechanism

In reply to Fabio Estevam

From: Fabio Estevam <[email protected]>

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 <[email protected]>

---
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(-)
common/cmd_sf.c +35 -0
index ac7f5df..bb39cb7 100644
@@ -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"
 	SF_TEST_HELP
 );
drivers/mtd/spi/Kconfig +15 -0
index 3f7433c..2ee1089 100644
@@ -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
+	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
drivers/mtd/spi/sf_internal.h +4 -6
index 9c95d56..8fb1b18 100644
@@ -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);
-
 /* 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);
drivers/mtd/spi/sf_ops.c +33 -0
index e12f8ee..281b06d 100644
@@ -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;
+	}
+
 	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);
+}
drivers/mtd/spi/sf_probe.c +27 -0
index 954376d..6da9d6c 100644
@@ -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[] = {
include/spi_flash.h +18 -0
index 3b2d555..28ec357 100644
@@ -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);
+
 /* Access the serial operations for a device */
 #define sf_get_ops(dev) ((struct dm_spi_flash_ops *)(dev)->driver->ops)
Signature · 1 line
1.9.1
#3 lore raw

PATCH v4 3/3 spi: Add SPI NOR protection mechanism

Quoted 90 lines · Fabio Estevam

From: Fabio Estevam <[email protected]>

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 <[email protected]>

---
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(-)
common/cmd_sf.c +33 -0
index ac7f5df..bb39cb7 100644
@@ -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);

common/cmd_sf.c U_BOOT_CMD( · line 616

Quoted 5 lines
common/cmd_sf.c +2 -0
@@ -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

drivers/mtd/spi/Kconfig config SPI_FLASH_USE_4K_SECTORS · line 104

Quoted 11 lines
common/cmd_sf.c +0 -0
        SF_TEST_HELP
 );
drivers/mtd/spi/Kconfig +1 -0
index 3f7433c..2ee1089 100644
@@ -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.

Quoted 22 lines
drivers/mtd/spi/Kconfig +14 -0
+       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
drivers/mtd/spi/sf_internal.h +0 -0
index 9c95d56..8fb1b18 100644

drivers/mtd/spi/sf_internal.h int spi_flash_cmd_write(struct spi_slave *spi, const u8 *cmd, size_t cmd_len, · line 169

Quoted 8 lines
drivers/mtd/spi/sf_internal.h +0 -5
@@ -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

Quoted 20 lines
drivers/mtd/spi/sf_internal.h +4 -1
-
 /* 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);
drivers/mtd/spi/sf_ops.c +0 -0
index e12f8ee..281b06d 100644

drivers/mtd/spi/sf_ops.c int spi_flash_cmd_erase_ops(struct spi_flash *flash, u32 offset, size_t len) · line 282

Quoted 7 lines
drivers/mtd/spi/sf_ops.c +4 -0
@@ -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"?

Quoted 39 lines
drivers/mtd/spi/sf_ops.c +21 -0
+
        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)
 {

drivers/mtd/spi/sf_ops.c int stm_unlock(struct spi_flash *nor, u32 ofs, u32 len) · line 795

Quoted 10 lines
drivers/mtd/spi/sf_ops.c +7 -0
@@ -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.

Quoted 77 lines
drivers/mtd/spi/sf_ops.c +1 -0
+}
drivers/mtd/spi/sf_probe.c +27 -0
index 954376d..6da9d6c 100644
@@ -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[] = {
include/spi_flash.h +7 -0
index 3b2d555..28ec357 100644
@@ -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);
 };
 
+

include/spi_flash.h struct dm_spi_flash_ops { · line 134

Quoted 10 lines
include/spi_flash.h +10 -0
+/* 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

Quoted 4 lines
include/spi_flash.h +1 -0
+
 /* Access the serial operations for a device */
 #define sf_get_ops(dev) ((struct dm_spi_flash_ops *)(dev)->driver->ops)
Signature · 1 line
Jagan | openedev.
author
#4 lore raw

PATCH v4 3/3 spi: Add SPI NOR protection mechanism

Quoted 4 lines · Jagan Teki

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

Ok.

Quoted 8 lines
Quoted 8 lines
drivers/mtd/spi/Kconfig +0 -0
index 3f7433c..2ee1089 100644
@@ -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).
Quoted 4 lines

drivers/mtd/spi/Kconfig config SPI_FLASH_USE_4K_SECTORS · line 104

Quoted 1 line
drivers/mtd/spi/Kconfig +1 -0
+config SPI_FLASH_STM_PROTECT

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

Ok.

Quoted 8 lines
Quoted 8 lines
drivers/mtd/spi/sf_internal.h +0 -0
index 9c95d56..8fb1b18 100644
@@ -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);
Quoted 7 lines

drivers/mtd/spi/sf_internal.h int spi_flash_cmd_write(struct spi_slave *spi, const u8 *cmd, size_t cmd_len, · line 169

Quoted 5 lines
drivers/mtd/spi/sf_internal.h +0 -5
-/* 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

Ok.

Quoted 6 lines
Quoted 6 lines
drivers/mtd/spi/sf_ops.c +0 -0
@@ -276,6 +276,11 @@ int spi_flash_cmd_erase_ops(struct spi_flash *flash, u32 offset, size_t len)
                return -1;
        }
Quoted 8 lines

drivers/mtd/spi/sf_ops.c int spi_flash_cmd_erase_ops(struct spi_flash *flash, u32 offset, size_t len) · line 282

Quoted 4 lines
drivers/mtd/spi/sf_ops.c +4 -0
+       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"?

We can add this feature in subsequent patches.

Quoted 8 lines

drivers/mtd/spi/sf_ops.c int stm_unlock(struct spi_flash *nor, u32 ofs, u32 len) · line 795

Quoted 6 lines
drivers/mtd/spi/sf_ops.c +6 -0
+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

Ok.

Quoted 1 line

Define this spi_flash_protect in include/spi_flash not in ops.

Cannot do this as include/spi_flash.h is included by several files, and then we would get errors about funcion being redefined.

Quoted 4 lines
Quoted 4 lines
drivers/mtd/spi/sf_ops.c +4 -0
+/* Read the status register */
+int spi_flash_cmd_read_status(struct spi_flash *flash, u8 *rs);
+
+/* Program the status register */
Quoted 8 lines

drivers/mtd/spi/sf_ops.c

Quoted 6 lines
drivers/mtd/spi/sf_ops.c +6 -0
+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

Ok.

#5 lore raw

PATCH v4 3/3 spi: Add SPI NOR protection mechanism

Quoted 40 lines · Fabio Estevam
Quoted 4 lines · Jagan Teki

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

Ok.

Quoted 8 lines
Quoted 8 lines
drivers/mtd/spi/Kconfig +0 -0
index 3f7433c..2ee1089 100644
@@ -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).
Quoted 4 lines

drivers/mtd/spi/Kconfig config SPI_FLASH_USE_4K_SECTORS · line 104

Quoted 1 line
drivers/mtd/spi/Kconfig +1 -0
+config SPI_FLASH_STM_PROTECT

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

Ok.

Quoted 8 lines
Quoted 8 lines
drivers/mtd/spi/sf_internal.h +0 -0
index 9c95d56..8fb1b18 100644
@@ -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);
Quoted 7 lines

drivers/mtd/spi/sf_internal.h int spi_flash_cmd_write(struct spi_slave *spi, const u8 *cmd, size_t cmd_len, · line 169

Quoted 5 lines
drivers/mtd/spi/sf_internal.h +0 -5
-/* 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

Quoted 16 lines

Ok.

Quoted 14 lines

drivers/mtd/spi/sf_ops.c int spi_flash_cmd_erase_ops(struct spi_flash *flash, u32 offset, size_t len) · line 282

Quoted 10 lines
drivers/mtd/spi/sf_ops.c +4 -0
@@ -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"?

Couldn't understand why would we get an errors for define this in spi_flash.h Like spi_flash_erase/_read/_write spi_flash_protect is a command ops which would intern call mtd/spi operations as function pointers.

Quoted 37 lines

We can add this feature in subsequent patches.

Quoted 8 lines

drivers/mtd/spi/sf_ops.c int stm_unlock(struct spi_flash *nor, u32 ofs, u32 len) · line 795

Quoted 6 lines
drivers/mtd/spi/sf_ops.c +6 -0
+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

Ok.

Quoted 1 line

Define this spi_flash_protect in include/spi_flash not in ops.

Cannot do this as include/spi_flash.h is included by several files, and then we would get errors about funcion being redefined.

Quoted 4 lines
Quoted 4 lines
drivers/mtd/spi/sf_ops.c +4 -0
+/* Read the status register */
+int spi_flash_cmd_read_status(struct spi_flash *flash, u8 *rs);
+
+/* Program the status register */
Quoted 8 lines

drivers/mtd/spi/sf_ops.c

Quoted 6 lines
drivers/mtd/spi/sf_ops.c +6 -0
+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

Ok.
_______________________________________________
U-Boot mailing list
U-Boot at lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Signature · 1 line
Jagan | openedev.
author
#6 lore raw

PATCH v4 3/3 spi: Add SPI NOR protection mechanism

Quoted 4 lines · Jagan Teki

Couldn't understand why would we get an errors for define this in spi_flash.h Like spi_flash_erase/_read/_write spi_flash_protect is a command ops which would intern call mtd/spi operations as function pointers.

Ok, got your point. Fixed it in v6.