All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/2] mtd: spi-nor: Add Global Block Unlock command
@ 2021-01-20 10:54 ` Tudor Ambarus
  0 siblings, 0 replies; 12+ messages in thread
From: Tudor Ambarus @ 2021-01-20 10:54 UTC (permalink / raw)
  To: michael, vigneshr
  Cc: miquel.raynal, richard, linux-mtd, linux-kernel,
	Kavyasree.Kotagiri, Tudor Ambarus

The Global Block Unlock command has different names depending
on the manufacturer, but always the same command value: 0x98.
Macronix's MX25U12835F names it Gang Block Unlock, Winbound's
W25Q128FV names it Global Block Unlock and Microchip's
SST26VF064B names it Global Block Protection Unlock.

Used in the Individual Block Protection mode, which is mutual
exclusive with the Block Protection mode (BP0-3).

Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com>
---
 drivers/mtd/spi-nor/core.c  | 35 +++++++++++++++++++++++++++++++++++
 drivers/mtd/spi-nor/core.h  |  1 +
 include/linux/mtd/spi-nor.h |  1 +
 3 files changed, 37 insertions(+)

diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
index 20df44b753da..cb275608cb7f 100644
--- a/drivers/mtd/spi-nor/core.c
+++ b/drivers/mtd/spi-nor/core.c
@@ -853,6 +853,41 @@ int spi_nor_wait_till_ready(struct spi_nor *nor)
 						    DEFAULT_READY_WAIT_JIFFIES);
 }
 
+/**
+ * spi_nor_global_block_unlock() - Unlock Global Block Protection.
+ * @nor:	pointer to 'struct spi_nor'.
+ *
+ * Return: 0 on success, -errno otherwise.
+ */
+int spi_nor_global_block_unlock(struct spi_nor *nor)
+{
+	int ret;
+
+	ret = spi_nor_write_enable(nor);
+	if (ret)
+		return ret;
+
+	if (nor->spimem) {
+		struct spi_mem_op op =
+			SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_GBULK, 1),
+				   SPI_MEM_OP_NO_ADDR,
+				   SPI_MEM_OP_NO_DUMMY,
+				   SPI_MEM_OP_NO_DATA);
+
+		ret = spi_mem_exec_op(nor->spimem, &op);
+	} else {
+		ret = spi_nor_controller_ops_write_reg(nor, SPINOR_OP_GBULK,
+						       NULL, 0);
+	}
+
+	if (ret) {
+		dev_dbg(nor->dev, "error %d on Global Block Unlock\n", ret);
+		return ret;
+	}
+
+	return spi_nor_wait_till_ready(nor);
+}
+
 /**
  * spi_nor_write_sr() - Write the Status Register.
  * @nor:	pointer to 'struct spi_nor'.
diff --git a/drivers/mtd/spi-nor/core.h b/drivers/mtd/spi-nor/core.h
index d631ee299de3..eb26796db026 100644
--- a/drivers/mtd/spi-nor/core.h
+++ b/drivers/mtd/spi-nor/core.h
@@ -434,6 +434,7 @@ int spi_nor_write_disable(struct spi_nor *nor);
 int spi_nor_set_4byte_addr_mode(struct spi_nor *nor, bool enable);
 int spi_nor_write_ear(struct spi_nor *nor, u8 ear);
 int spi_nor_wait_till_ready(struct spi_nor *nor);
+int spi_nor_global_block_unlock(struct spi_nor *nor);
 int spi_nor_lock_and_prep(struct spi_nor *nor);
 void spi_nor_unlock_and_unprep(struct spi_nor *nor);
 int spi_nor_sr1_bit6_quad_enable(struct spi_nor *nor);
diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h
index d13958de6d8a..a0d572855444 100644
--- a/include/linux/mtd/spi-nor.h
+++ b/include/linux/mtd/spi-nor.h
@@ -53,6 +53,7 @@
 #define SPINOR_OP_WREAR		0xc5	/* Write Extended Address Register */
 #define SPINOR_OP_SRSTEN	0x66	/* Software Reset Enable */
 #define SPINOR_OP_SRST		0x99	/* Software Reset */
+#define SPINOR_OP_GBULK		0x98    /* Global Block Unlock */
 
 /* 4-byte address opcodes - used on Spansion and some Macronix flashes. */
 #define SPINOR_OP_READ_4B	0x13	/* Read data bytes (low frequency) */
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 1/2] mtd: spi-nor: Add Global Block Unlock command
@ 2021-01-20 10:54 ` Tudor Ambarus
  0 siblings, 0 replies; 12+ messages in thread
From: Tudor Ambarus @ 2021-01-20 10:54 UTC (permalink / raw)
  To: michael, vigneshr
  Cc: Tudor Ambarus, richard, linux-kernel, linux-mtd,
	Kavyasree.Kotagiri, miquel.raynal

The Global Block Unlock command has different names depending
on the manufacturer, but always the same command value: 0x98.
Macronix's MX25U12835F names it Gang Block Unlock, Winbound's
W25Q128FV names it Global Block Unlock and Microchip's
SST26VF064B names it Global Block Protection Unlock.

Used in the Individual Block Protection mode, which is mutual
exclusive with the Block Protection mode (BP0-3).

Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com>
---
 drivers/mtd/spi-nor/core.c  | 35 +++++++++++++++++++++++++++++++++++
 drivers/mtd/spi-nor/core.h  |  1 +
 include/linux/mtd/spi-nor.h |  1 +
 3 files changed, 37 insertions(+)

diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
index 20df44b753da..cb275608cb7f 100644
--- a/drivers/mtd/spi-nor/core.c
+++ b/drivers/mtd/spi-nor/core.c
@@ -853,6 +853,41 @@ int spi_nor_wait_till_ready(struct spi_nor *nor)
 						    DEFAULT_READY_WAIT_JIFFIES);
 }
 
+/**
+ * spi_nor_global_block_unlock() - Unlock Global Block Protection.
+ * @nor:	pointer to 'struct spi_nor'.
+ *
+ * Return: 0 on success, -errno otherwise.
+ */
+int spi_nor_global_block_unlock(struct spi_nor *nor)
+{
+	int ret;
+
+	ret = spi_nor_write_enable(nor);
+	if (ret)
+		return ret;
+
+	if (nor->spimem) {
+		struct spi_mem_op op =
+			SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_GBULK, 1),
+				   SPI_MEM_OP_NO_ADDR,
+				   SPI_MEM_OP_NO_DUMMY,
+				   SPI_MEM_OP_NO_DATA);
+
+		ret = spi_mem_exec_op(nor->spimem, &op);
+	} else {
+		ret = spi_nor_controller_ops_write_reg(nor, SPINOR_OP_GBULK,
+						       NULL, 0);
+	}
+
+	if (ret) {
+		dev_dbg(nor->dev, "error %d on Global Block Unlock\n", ret);
+		return ret;
+	}
+
+	return spi_nor_wait_till_ready(nor);
+}
+
 /**
  * spi_nor_write_sr() - Write the Status Register.
  * @nor:	pointer to 'struct spi_nor'.
diff --git a/drivers/mtd/spi-nor/core.h b/drivers/mtd/spi-nor/core.h
index d631ee299de3..eb26796db026 100644
--- a/drivers/mtd/spi-nor/core.h
+++ b/drivers/mtd/spi-nor/core.h
@@ -434,6 +434,7 @@ int spi_nor_write_disable(struct spi_nor *nor);
 int spi_nor_set_4byte_addr_mode(struct spi_nor *nor, bool enable);
 int spi_nor_write_ear(struct spi_nor *nor, u8 ear);
 int spi_nor_wait_till_ready(struct spi_nor *nor);
+int spi_nor_global_block_unlock(struct spi_nor *nor);
 int spi_nor_lock_and_prep(struct spi_nor *nor);
 void spi_nor_unlock_and_unprep(struct spi_nor *nor);
 int spi_nor_sr1_bit6_quad_enable(struct spi_nor *nor);
diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h
index d13958de6d8a..a0d572855444 100644
--- a/include/linux/mtd/spi-nor.h
+++ b/include/linux/mtd/spi-nor.h
@@ -53,6 +53,7 @@
 #define SPINOR_OP_WREAR		0xc5	/* Write Extended Address Register */
 #define SPINOR_OP_SRSTEN	0x66	/* Software Reset Enable */
 #define SPINOR_OP_SRST		0x99	/* Software Reset */
+#define SPINOR_OP_GBULK		0x98    /* Global Block Unlock */
 
 /* 4-byte address opcodes - used on Spansion and some Macronix flashes. */
 #define SPINOR_OP_READ_4B	0x13	/* Read data bytes (low frequency) */
-- 
2.25.1


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 2/2] mtd: spi-nor: sst: Add support for Global Unlock on sst26vf
  2021-01-20 10:54 ` Tudor Ambarus
@ 2021-01-20 10:54   ` Tudor Ambarus
  -1 siblings, 0 replies; 12+ messages in thread
From: Tudor Ambarus @ 2021-01-20 10:54 UTC (permalink / raw)
  To: michael, vigneshr
  Cc: miquel.raynal, richard, linux-mtd, linux-kernel,
	Kavyasree.Kotagiri, Tudor Ambarus

Even if sst26vf shares the SPINOR_OP_GBULK opcode with
Macronix (ex. MX25U12835F) and Winbound (ex. W25Q128FV),
it has its own Individual Block Protection scheme, which
is also capable to read-lock individual parameter blocks.
Thus the sst26vf's Individual Block Protection scheme will
reside in the sst.c manufacturer driver.

Add support to unlock the entire flash memory. The device
is write-protected by default after a power-on reset cycle
(volatile software protection), in order to avoid inadvertent
writes during power-up. Could do an erase, write, read back,
and compare when MTD_SPI_NOR_SWP_DISABLE_ON_VOLATILE=y.

Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com>
---
 drivers/mtd/spi-nor/sst.c | 38 ++++++++++++++++++++++++++++++++++++--
 1 file changed, 36 insertions(+), 2 deletions(-)

diff --git a/drivers/mtd/spi-nor/sst.c b/drivers/mtd/spi-nor/sst.c
index 00e48da0744a..1cd2a360c41e 100644
--- a/drivers/mtd/spi-nor/sst.c
+++ b/drivers/mtd/spi-nor/sst.c
@@ -8,6 +8,39 @@
 
 #include "core.h"
 
+static int sst26vf_lock(struct spi_nor *nor, loff_t ofs, uint64_t len)
+{
+	return -EOPNOTSUPP;
+}
+
+static int sst26vf_unlock(struct spi_nor *nor, loff_t ofs, uint64_t len)
+{
+	if (!ofs && len == nor->params->size)
+		return spi_nor_global_block_unlock(nor);
+
+	return -EOPNOTSUPP;
+}
+
+static int sst26vf_is_locked(struct spi_nor *nor, loff_t ofs, uint64_t len)
+{
+	return -EOPNOTSUPP;
+}
+
+static const struct spi_nor_locking_ops sst26vf_locking_ops = {
+	.lock = sst26vf_lock,
+	.unlock = sst26vf_unlock,
+	.is_locked = sst26vf_is_locked,
+};
+
+static void sst26vf_default_init(struct spi_nor *nor)
+{
+	nor->params->locking_ops = &sst26vf_locking_ops;
+}
+
+static const struct spi_nor_fixups sst26vf_fixups = {
+	.default_init = sst26vf_default_init,
+};
+
 static const struct flash_info sst_parts[] = {
 	/* SST -- large erase sizes are "overlays", "sectors" are 4K */
 	{ "sst25vf040b", INFO(0xbf258d, 0, 64 * 1024,  8,
@@ -39,8 +72,9 @@ static const struct flash_info sst_parts[] = {
 	{ "sst26vf016b", INFO(0xbf2641, 0, 64 * 1024, 32,
 			      SECT_4K | SPI_NOR_DUAL_READ) },
 	{ "sst26vf064b", INFO(0xbf2643, 0, 64 * 1024, 128,
-			      SECT_4K | SPI_NOR_DUAL_READ |
-			      SPI_NOR_QUAD_READ) },
+			      SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ |
+			      SPI_NOR_HAS_LOCK | SPI_NOR_SWP_IS_VOLATILE)
+		.fixups = &sst26vf_fixups },
 };
 
 static int sst_write(struct mtd_info *mtd, loff_t to, size_t len,
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 2/2] mtd: spi-nor: sst: Add support for Global Unlock on sst26vf
@ 2021-01-20 10:54   ` Tudor Ambarus
  0 siblings, 0 replies; 12+ messages in thread
From: Tudor Ambarus @ 2021-01-20 10:54 UTC (permalink / raw)
  To: michael, vigneshr
  Cc: Tudor Ambarus, richard, linux-kernel, linux-mtd,
	Kavyasree.Kotagiri, miquel.raynal

Even if sst26vf shares the SPINOR_OP_GBULK opcode with
Macronix (ex. MX25U12835F) and Winbound (ex. W25Q128FV),
it has its own Individual Block Protection scheme, which
is also capable to read-lock individual parameter blocks.
Thus the sst26vf's Individual Block Protection scheme will
reside in the sst.c manufacturer driver.

Add support to unlock the entire flash memory. The device
is write-protected by default after a power-on reset cycle
(volatile software protection), in order to avoid inadvertent
writes during power-up. Could do an erase, write, read back,
and compare when MTD_SPI_NOR_SWP_DISABLE_ON_VOLATILE=y.

Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com>
---
 drivers/mtd/spi-nor/sst.c | 38 ++++++++++++++++++++++++++++++++++++--
 1 file changed, 36 insertions(+), 2 deletions(-)

diff --git a/drivers/mtd/spi-nor/sst.c b/drivers/mtd/spi-nor/sst.c
index 00e48da0744a..1cd2a360c41e 100644
--- a/drivers/mtd/spi-nor/sst.c
+++ b/drivers/mtd/spi-nor/sst.c
@@ -8,6 +8,39 @@
 
 #include "core.h"
 
+static int sst26vf_lock(struct spi_nor *nor, loff_t ofs, uint64_t len)
+{
+	return -EOPNOTSUPP;
+}
+
+static int sst26vf_unlock(struct spi_nor *nor, loff_t ofs, uint64_t len)
+{
+	if (!ofs && len == nor->params->size)
+		return spi_nor_global_block_unlock(nor);
+
+	return -EOPNOTSUPP;
+}
+
+static int sst26vf_is_locked(struct spi_nor *nor, loff_t ofs, uint64_t len)
+{
+	return -EOPNOTSUPP;
+}
+
+static const struct spi_nor_locking_ops sst26vf_locking_ops = {
+	.lock = sst26vf_lock,
+	.unlock = sst26vf_unlock,
+	.is_locked = sst26vf_is_locked,
+};
+
+static void sst26vf_default_init(struct spi_nor *nor)
+{
+	nor->params->locking_ops = &sst26vf_locking_ops;
+}
+
+static const struct spi_nor_fixups sst26vf_fixups = {
+	.default_init = sst26vf_default_init,
+};
+
 static const struct flash_info sst_parts[] = {
 	/* SST -- large erase sizes are "overlays", "sectors" are 4K */
 	{ "sst25vf040b", INFO(0xbf258d, 0, 64 * 1024,  8,
@@ -39,8 +72,9 @@ static const struct flash_info sst_parts[] = {
 	{ "sst26vf016b", INFO(0xbf2641, 0, 64 * 1024, 32,
 			      SECT_4K | SPI_NOR_DUAL_READ) },
 	{ "sst26vf064b", INFO(0xbf2643, 0, 64 * 1024, 128,
-			      SECT_4K | SPI_NOR_DUAL_READ |
-			      SPI_NOR_QUAD_READ) },
+			      SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ |
+			      SPI_NOR_HAS_LOCK | SPI_NOR_SWP_IS_VOLATILE)
+		.fixups = &sst26vf_fixups },
 };
 
 static int sst_write(struct mtd_info *mtd, loff_t to, size_t len,
-- 
2.25.1


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/2] mtd: spi-nor: Add Global Block Unlock command
  2021-01-20 10:54 ` Tudor Ambarus
@ 2021-01-20 12:29   ` Pratyush Yadav
  -1 siblings, 0 replies; 12+ messages in thread
From: Pratyush Yadav @ 2021-01-20 12:29 UTC (permalink / raw)
  To: Tudor Ambarus
  Cc: michael, vigneshr, richard, linux-kernel, linux-mtd,
	Kavyasree.Kotagiri, miquel.raynal

Hi Tudor,

On 20/01/21 12:54PM, Tudor Ambarus wrote:
> The Global Block Unlock command has different names depending
> on the manufacturer, but always the same command value: 0x98.
> Macronix's MX25U12835F names it Gang Block Unlock, Winbound's
> W25Q128FV names it Global Block Unlock and Microchip's
> SST26VF064B names it Global Block Protection Unlock.
> 
> Used in the Individual Block Protection mode, which is mutual

Nitpick: s/mutual/mutually/

> exclusive with the Block Protection mode (BP0-3).
> 
> Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com>
> ---
>  drivers/mtd/spi-nor/core.c  | 35 +++++++++++++++++++++++++++++++++++
>  drivers/mtd/spi-nor/core.h  |  1 +
>  include/linux/mtd/spi-nor.h |  1 +
>  3 files changed, 37 insertions(+)
> 
> diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
> index 20df44b753da..cb275608cb7f 100644
> --- a/drivers/mtd/spi-nor/core.c
> +++ b/drivers/mtd/spi-nor/core.c
> @@ -853,6 +853,41 @@ int spi_nor_wait_till_ready(struct spi_nor *nor)
>  						    DEFAULT_READY_WAIT_JIFFIES);
>  }
>  
> +/**
> + * spi_nor_global_block_unlock() - Unlock Global Block Protection.
> + * @nor:	pointer to 'struct spi_nor'.
> + *
> + * Return: 0 on success, -errno otherwise.
> + */
> +int spi_nor_global_block_unlock(struct spi_nor *nor)
> +{
> +	int ret;
> +
> +	ret = spi_nor_write_enable(nor);
> +	if (ret)
> +		return ret;
> +
> +	if (nor->spimem) {
> +		struct spi_mem_op op =
> +			SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_GBULK, 1),

Set the buswidth to 0...

> +				   SPI_MEM_OP_NO_ADDR,
> +				   SPI_MEM_OP_NO_DUMMY,
> +				   SPI_MEM_OP_NO_DATA);

... and run the op through spi_nor_spimem_setup_op().

With this fixed,

Reviewed-by: Pratyush Yadav <p.yadav@ti.com>

> +
> +		ret = spi_mem_exec_op(nor->spimem, &op);
> +	} else {
> +		ret = spi_nor_controller_ops_write_reg(nor, SPINOR_OP_GBULK,
> +						       NULL, 0);
> +	}
> +
> +	if (ret) {
> +		dev_dbg(nor->dev, "error %d on Global Block Unlock\n", ret);
> +		return ret;
> +	}
> +
> +	return spi_nor_wait_till_ready(nor);
> +}
> +
>  /**
>   * spi_nor_write_sr() - Write the Status Register.
>   * @nor:	pointer to 'struct spi_nor'.
> diff --git a/drivers/mtd/spi-nor/core.h b/drivers/mtd/spi-nor/core.h
> index d631ee299de3..eb26796db026 100644
> --- a/drivers/mtd/spi-nor/core.h
> +++ b/drivers/mtd/spi-nor/core.h
> @@ -434,6 +434,7 @@ int spi_nor_write_disable(struct spi_nor *nor);
>  int spi_nor_set_4byte_addr_mode(struct spi_nor *nor, bool enable);
>  int spi_nor_write_ear(struct spi_nor *nor, u8 ear);
>  int spi_nor_wait_till_ready(struct spi_nor *nor);
> +int spi_nor_global_block_unlock(struct spi_nor *nor);
>  int spi_nor_lock_and_prep(struct spi_nor *nor);
>  void spi_nor_unlock_and_unprep(struct spi_nor *nor);
>  int spi_nor_sr1_bit6_quad_enable(struct spi_nor *nor);
> diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h
> index d13958de6d8a..a0d572855444 100644
> --- a/include/linux/mtd/spi-nor.h
> +++ b/include/linux/mtd/spi-nor.h
> @@ -53,6 +53,7 @@
>  #define SPINOR_OP_WREAR		0xc5	/* Write Extended Address Register */
>  #define SPINOR_OP_SRSTEN	0x66	/* Software Reset Enable */
>  #define SPINOR_OP_SRST		0x99	/* Software Reset */
> +#define SPINOR_OP_GBULK		0x98    /* Global Block Unlock */
>  
>  /* 4-byte address opcodes - used on Spansion and some Macronix flashes. */
>  #define SPINOR_OP_READ_4B	0x13	/* Read data bytes (low frequency) */

-- 
Regards,
Pratyush Yadav
Texas Instruments India

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/2] mtd: spi-nor: Add Global Block Unlock command
@ 2021-01-20 12:29   ` Pratyush Yadav
  0 siblings, 0 replies; 12+ messages in thread
From: Pratyush Yadav @ 2021-01-20 12:29 UTC (permalink / raw)
  To: Tudor Ambarus
  Cc: vigneshr, richard, linux-kernel, michael, linux-mtd,
	Kavyasree.Kotagiri, miquel.raynal

Hi Tudor,

On 20/01/21 12:54PM, Tudor Ambarus wrote:
> The Global Block Unlock command has different names depending
> on the manufacturer, but always the same command value: 0x98.
> Macronix's MX25U12835F names it Gang Block Unlock, Winbound's
> W25Q128FV names it Global Block Unlock and Microchip's
> SST26VF064B names it Global Block Protection Unlock.
> 
> Used in the Individual Block Protection mode, which is mutual

Nitpick: s/mutual/mutually/

> exclusive with the Block Protection mode (BP0-3).
> 
> Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com>
> ---
>  drivers/mtd/spi-nor/core.c  | 35 +++++++++++++++++++++++++++++++++++
>  drivers/mtd/spi-nor/core.h  |  1 +
>  include/linux/mtd/spi-nor.h |  1 +
>  3 files changed, 37 insertions(+)
> 
> diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
> index 20df44b753da..cb275608cb7f 100644
> --- a/drivers/mtd/spi-nor/core.c
> +++ b/drivers/mtd/spi-nor/core.c
> @@ -853,6 +853,41 @@ int spi_nor_wait_till_ready(struct spi_nor *nor)
>  						    DEFAULT_READY_WAIT_JIFFIES);
>  }
>  
> +/**
> + * spi_nor_global_block_unlock() - Unlock Global Block Protection.
> + * @nor:	pointer to 'struct spi_nor'.
> + *
> + * Return: 0 on success, -errno otherwise.
> + */
> +int spi_nor_global_block_unlock(struct spi_nor *nor)
> +{
> +	int ret;
> +
> +	ret = spi_nor_write_enable(nor);
> +	if (ret)
> +		return ret;
> +
> +	if (nor->spimem) {
> +		struct spi_mem_op op =
> +			SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_GBULK, 1),

Set the buswidth to 0...

> +				   SPI_MEM_OP_NO_ADDR,
> +				   SPI_MEM_OP_NO_DUMMY,
> +				   SPI_MEM_OP_NO_DATA);

... and run the op through spi_nor_spimem_setup_op().

With this fixed,

Reviewed-by: Pratyush Yadav <p.yadav@ti.com>

> +
> +		ret = spi_mem_exec_op(nor->spimem, &op);
> +	} else {
> +		ret = spi_nor_controller_ops_write_reg(nor, SPINOR_OP_GBULK,
> +						       NULL, 0);
> +	}
> +
> +	if (ret) {
> +		dev_dbg(nor->dev, "error %d on Global Block Unlock\n", ret);
> +		return ret;
> +	}
> +
> +	return spi_nor_wait_till_ready(nor);
> +}
> +
>  /**
>   * spi_nor_write_sr() - Write the Status Register.
>   * @nor:	pointer to 'struct spi_nor'.
> diff --git a/drivers/mtd/spi-nor/core.h b/drivers/mtd/spi-nor/core.h
> index d631ee299de3..eb26796db026 100644
> --- a/drivers/mtd/spi-nor/core.h
> +++ b/drivers/mtd/spi-nor/core.h
> @@ -434,6 +434,7 @@ int spi_nor_write_disable(struct spi_nor *nor);
>  int spi_nor_set_4byte_addr_mode(struct spi_nor *nor, bool enable);
>  int spi_nor_write_ear(struct spi_nor *nor, u8 ear);
>  int spi_nor_wait_till_ready(struct spi_nor *nor);
> +int spi_nor_global_block_unlock(struct spi_nor *nor);
>  int spi_nor_lock_and_prep(struct spi_nor *nor);
>  void spi_nor_unlock_and_unprep(struct spi_nor *nor);
>  int spi_nor_sr1_bit6_quad_enable(struct spi_nor *nor);
> diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h
> index d13958de6d8a..a0d572855444 100644
> --- a/include/linux/mtd/spi-nor.h
> +++ b/include/linux/mtd/spi-nor.h
> @@ -53,6 +53,7 @@
>  #define SPINOR_OP_WREAR		0xc5	/* Write Extended Address Register */
>  #define SPINOR_OP_SRSTEN	0x66	/* Software Reset Enable */
>  #define SPINOR_OP_SRST		0x99	/* Software Reset */
> +#define SPINOR_OP_GBULK		0x98    /* Global Block Unlock */
>  
>  /* 4-byte address opcodes - used on Spansion and some Macronix flashes. */
>  #define SPINOR_OP_READ_4B	0x13	/* Read data bytes (low frequency) */

-- 
Regards,
Pratyush Yadav
Texas Instruments India

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/2] mtd: spi-nor: sst: Add support for Global Unlock on sst26vf
  2021-01-20 10:54   ` Tudor Ambarus
@ 2021-01-20 12:29     ` Pratyush Yadav
  -1 siblings, 0 replies; 12+ messages in thread
From: Pratyush Yadav @ 2021-01-20 12:29 UTC (permalink / raw)
  To: Tudor Ambarus
  Cc: michael, vigneshr, richard, linux-kernel, linux-mtd,
	Kavyasree.Kotagiri, miquel.raynal

Hi Tudor,

On 20/01/21 12:54PM, Tudor Ambarus wrote:
> Even if sst26vf shares the SPINOR_OP_GBULK opcode with
> Macronix (ex. MX25U12835F) and Winbound (ex. W25Q128FV),
> it has its own Individual Block Protection scheme, which
> is also capable to read-lock individual parameter blocks.
> Thus the sst26vf's Individual Block Protection scheme will
> reside in the sst.c manufacturer driver.
> 
> Add support to unlock the entire flash memory. The device
> is write-protected by default after a power-on reset cycle
> (volatile software protection), in order to avoid inadvertent
> writes during power-up. Could do an erase, write, read back,
> and compare when MTD_SPI_NOR_SWP_DISABLE_ON_VOLATILE=y.
> 
> Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com>
> ---
>  drivers/mtd/spi-nor/sst.c | 38 ++++++++++++++++++++++++++++++++++++--
>  1 file changed, 36 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/mtd/spi-nor/sst.c b/drivers/mtd/spi-nor/sst.c
> index 00e48da0744a..1cd2a360c41e 100644
> --- a/drivers/mtd/spi-nor/sst.c
> +++ b/drivers/mtd/spi-nor/sst.c
> @@ -8,6 +8,39 @@
>  
>  #include "core.h"
>  
> +static int sst26vf_lock(struct spi_nor *nor, loff_t ofs, uint64_t len)
> +{
> +	return -EOPNOTSUPP;
> +}
> +
> +static int sst26vf_unlock(struct spi_nor *nor, loff_t ofs, uint64_t len)
> +{
> +	if (!ofs && len == nor->params->size)

Nitpick: ofs is not a boolean value. Don't treat it as such. (ofs == 0 
&& len == nor->params->size) makes the intent much clearer.

> +		return spi_nor_global_block_unlock(nor);
> +
> +	return -EOPNOTSUPP;
> +}
> +
> +static int sst26vf_is_locked(struct spi_nor *nor, loff_t ofs, uint64_t len)
> +{
> +	return -EOPNOTSUPP;
> +}
> +
> +static const struct spi_nor_locking_ops sst26vf_locking_ops = {
> +	.lock = sst26vf_lock,
> +	.unlock = sst26vf_unlock,
> +	.is_locked = sst26vf_is_locked,
> +};
> +
> +static void sst26vf_default_init(struct spi_nor *nor)
> +{
> +	nor->params->locking_ops = &sst26vf_locking_ops;
> +}
> +
> +static const struct spi_nor_fixups sst26vf_fixups = {
> +	.default_init = sst26vf_default_init,
> +};
> +
>  static const struct flash_info sst_parts[] = {
>  	/* SST -- large erase sizes are "overlays", "sectors" are 4K */
>  	{ "sst25vf040b", INFO(0xbf258d, 0, 64 * 1024,  8,
> @@ -39,8 +72,9 @@ static const struct flash_info sst_parts[] = {
>  	{ "sst26vf016b", INFO(0xbf2641, 0, 64 * 1024, 32,
>  			      SECT_4K | SPI_NOR_DUAL_READ) },
>  	{ "sst26vf064b", INFO(0xbf2643, 0, 64 * 1024, 128,
> -			      SECT_4K | SPI_NOR_DUAL_READ |
> -			      SPI_NOR_QUAD_READ) },
> +			      SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ |
> +			      SPI_NOR_HAS_LOCK | SPI_NOR_SWP_IS_VOLATILE)
> +		.fixups = &sst26vf_fixups },
>  };
>  
>  static int sst_write(struct mtd_info *mtd, loff_t to, size_t len,
> -- 
> 2.25.1
> 
> 
> ______________________________________________________
> Linux MTD discussion mailing list
> http://lists.infradead.org/mailman/listinfo/linux-mtd/

-- 
Regards,
Pratyush Yadav
Texas Instruments India

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/2] mtd: spi-nor: sst: Add support for Global Unlock on sst26vf
@ 2021-01-20 12:29     ` Pratyush Yadav
  0 siblings, 0 replies; 12+ messages in thread
From: Pratyush Yadav @ 2021-01-20 12:29 UTC (permalink / raw)
  To: Tudor Ambarus
  Cc: vigneshr, richard, linux-kernel, michael, linux-mtd,
	Kavyasree.Kotagiri, miquel.raynal

Hi Tudor,

On 20/01/21 12:54PM, Tudor Ambarus wrote:
> Even if sst26vf shares the SPINOR_OP_GBULK opcode with
> Macronix (ex. MX25U12835F) and Winbound (ex. W25Q128FV),
> it has its own Individual Block Protection scheme, which
> is also capable to read-lock individual parameter blocks.
> Thus the sst26vf's Individual Block Protection scheme will
> reside in the sst.c manufacturer driver.
> 
> Add support to unlock the entire flash memory. The device
> is write-protected by default after a power-on reset cycle
> (volatile software protection), in order to avoid inadvertent
> writes during power-up. Could do an erase, write, read back,
> and compare when MTD_SPI_NOR_SWP_DISABLE_ON_VOLATILE=y.
> 
> Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com>
> ---
>  drivers/mtd/spi-nor/sst.c | 38 ++++++++++++++++++++++++++++++++++++--
>  1 file changed, 36 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/mtd/spi-nor/sst.c b/drivers/mtd/spi-nor/sst.c
> index 00e48da0744a..1cd2a360c41e 100644
> --- a/drivers/mtd/spi-nor/sst.c
> +++ b/drivers/mtd/spi-nor/sst.c
> @@ -8,6 +8,39 @@
>  
>  #include "core.h"
>  
> +static int sst26vf_lock(struct spi_nor *nor, loff_t ofs, uint64_t len)
> +{
> +	return -EOPNOTSUPP;
> +}
> +
> +static int sst26vf_unlock(struct spi_nor *nor, loff_t ofs, uint64_t len)
> +{
> +	if (!ofs && len == nor->params->size)

Nitpick: ofs is not a boolean value. Don't treat it as such. (ofs == 0 
&& len == nor->params->size) makes the intent much clearer.

> +		return spi_nor_global_block_unlock(nor);
> +
> +	return -EOPNOTSUPP;
> +}
> +
> +static int sst26vf_is_locked(struct spi_nor *nor, loff_t ofs, uint64_t len)
> +{
> +	return -EOPNOTSUPP;
> +}
> +
> +static const struct spi_nor_locking_ops sst26vf_locking_ops = {
> +	.lock = sst26vf_lock,
> +	.unlock = sst26vf_unlock,
> +	.is_locked = sst26vf_is_locked,
> +};
> +
> +static void sst26vf_default_init(struct spi_nor *nor)
> +{
> +	nor->params->locking_ops = &sst26vf_locking_ops;
> +}
> +
> +static const struct spi_nor_fixups sst26vf_fixups = {
> +	.default_init = sst26vf_default_init,
> +};
> +
>  static const struct flash_info sst_parts[] = {
>  	/* SST -- large erase sizes are "overlays", "sectors" are 4K */
>  	{ "sst25vf040b", INFO(0xbf258d, 0, 64 * 1024,  8,
> @@ -39,8 +72,9 @@ static const struct flash_info sst_parts[] = {
>  	{ "sst26vf016b", INFO(0xbf2641, 0, 64 * 1024, 32,
>  			      SECT_4K | SPI_NOR_DUAL_READ) },
>  	{ "sst26vf064b", INFO(0xbf2643, 0, 64 * 1024, 128,
> -			      SECT_4K | SPI_NOR_DUAL_READ |
> -			      SPI_NOR_QUAD_READ) },
> +			      SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ |
> +			      SPI_NOR_HAS_LOCK | SPI_NOR_SWP_IS_VOLATILE)
> +		.fixups = &sst26vf_fixups },
>  };
>  
>  static int sst_write(struct mtd_info *mtd, loff_t to, size_t len,
> -- 
> 2.25.1
> 
> 
> ______________________________________________________
> Linux MTD discussion mailing list
> http://lists.infradead.org/mailman/listinfo/linux-mtd/

-- 
Regards,
Pratyush Yadav
Texas Instruments India

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/2] mtd: spi-nor: Add Global Block Unlock command
  2021-01-20 12:29   ` Pratyush Yadav
@ 2021-01-20 13:01     ` Tudor.Ambarus
  -1 siblings, 0 replies; 12+ messages in thread
From: Tudor.Ambarus @ 2021-01-20 13:01 UTC (permalink / raw)
  To: p.yadav
  Cc: michael, vigneshr, richard, linux-kernel, linux-mtd,
	Kavyasree.Kotagiri, miquel.raynal

On 1/20/21 2:29 PM, Pratyush Yadav wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> Hi Tudor,

Hi, Pratyush,

Thanks for reviewing this.

> 
> On 20/01/21 12:54PM, Tudor Ambarus wrote:
>> The Global Block Unlock command has different names depending
>> on the manufacturer, but always the same command value: 0x98.
>> Macronix's MX25U12835F names it Gang Block Unlock, Winbound's
>> W25Q128FV names it Global Block Unlock and Microchip's
>> SST26VF064B names it Global Block Protection Unlock.
>>
>> Used in the Individual Block Protection mode, which is mutual
> 
> Nitpick: s/mutual/mutually/
> 
>> exclusive with the Block Protection mode (BP0-3).
>>
>> Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com>
>> ---
>>  drivers/mtd/spi-nor/core.c  | 35 +++++++++++++++++++++++++++++++++++
>>  drivers/mtd/spi-nor/core.h  |  1 +
>>  include/linux/mtd/spi-nor.h |  1 +
>>  3 files changed, 37 insertions(+)
>>
>> diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
>> index 20df44b753da..cb275608cb7f 100644
>> --- a/drivers/mtd/spi-nor/core.c
>> +++ b/drivers/mtd/spi-nor/core.c
>> @@ -853,6 +853,41 @@ int spi_nor_wait_till_ready(struct spi_nor *nor)
>>                                                   DEFAULT_READY_WAIT_JIFFIES);
>>  }
>>
>> +/**
>> + * spi_nor_global_block_unlock() - Unlock Global Block Protection.
>> + * @nor:     pointer to 'struct spi_nor'.
>> + *
>> + * Return: 0 on success, -errno otherwise.
>> + */
>> +int spi_nor_global_block_unlock(struct spi_nor *nor)
>> +{
>> +     int ret;
>> +
>> +     ret = spi_nor_write_enable(nor);
>> +     if (ret)
>> +             return ret;
>> +
>> +     if (nor->spimem) {
>> +             struct spi_mem_op op =
>> +                     SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_GBULK, 1),
> 
> Set the buswidth to 0...
> 
>> +                                SPI_MEM_OP_NO_ADDR,
>> +                                SPI_MEM_OP_NO_DUMMY,
>> +                                SPI_MEM_OP_NO_DATA);
> 
> ... and run the op through spi_nor_spimem_setup_op().

Right, I thought that SPINOR_OP_GBULK is available just in single SPI
mode, but I see it can be issued in QPI mode as well. Will change.

Cheers,
ta
> 
> With this fixed,
> 
> Reviewed-by: Pratyush Yadav <p.yadav@ti.com>
> 
>> +
>> +             ret = spi_mem_exec_op(nor->spimem, &op);
>> +     } else {
>> +             ret = spi_nor_controller_ops_write_reg(nor, SPINOR_OP_GBULK,
>> +                                                    NULL, 0);
>> +     }
>> +
>> +     if (ret) {
>> +             dev_dbg(nor->dev, "error %d on Global Block Unlock\n", ret);
>> +             return ret;
>> +     }
>> +
>> +     return spi_nor_wait_till_ready(nor);
>> +}
>> +
>>  /**
>>   * spi_nor_write_sr() - Write the Status Register.
>>   * @nor:     pointer to 'struct spi_nor'.
>> diff --git a/drivers/mtd/spi-nor/core.h b/drivers/mtd/spi-nor/core.h
>> index d631ee299de3..eb26796db026 100644
>> --- a/drivers/mtd/spi-nor/core.h
>> +++ b/drivers/mtd/spi-nor/core.h
>> @@ -434,6 +434,7 @@ int spi_nor_write_disable(struct spi_nor *nor);
>>  int spi_nor_set_4byte_addr_mode(struct spi_nor *nor, bool enable);
>>  int spi_nor_write_ear(struct spi_nor *nor, u8 ear);
>>  int spi_nor_wait_till_ready(struct spi_nor *nor);
>> +int spi_nor_global_block_unlock(struct spi_nor *nor);
>>  int spi_nor_lock_and_prep(struct spi_nor *nor);
>>  void spi_nor_unlock_and_unprep(struct spi_nor *nor);
>>  int spi_nor_sr1_bit6_quad_enable(struct spi_nor *nor);
>> diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h
>> index d13958de6d8a..a0d572855444 100644
>> --- a/include/linux/mtd/spi-nor.h
>> +++ b/include/linux/mtd/spi-nor.h
>> @@ -53,6 +53,7 @@
>>  #define SPINOR_OP_WREAR              0xc5    /* Write Extended Address Register */
>>  #define SPINOR_OP_SRSTEN     0x66    /* Software Reset Enable */
>>  #define SPINOR_OP_SRST               0x99    /* Software Reset */
>> +#define SPINOR_OP_GBULK              0x98    /* Global Block Unlock */
>>
>>  /* 4-byte address opcodes - used on Spansion and some Macronix flashes. */
>>  #define SPINOR_OP_READ_4B    0x13    /* Read data bytes (low frequency) */
> 
> --
> Regards,
> Pratyush Yadav
> Texas Instruments India
> 


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/2] mtd: spi-nor: Add Global Block Unlock command
@ 2021-01-20 13:01     ` Tudor.Ambarus
  0 siblings, 0 replies; 12+ messages in thread
From: Tudor.Ambarus @ 2021-01-20 13:01 UTC (permalink / raw)
  To: p.yadav
  Cc: vigneshr, richard, linux-kernel, michael, linux-mtd,
	Kavyasree.Kotagiri, miquel.raynal

On 1/20/21 2:29 PM, Pratyush Yadav wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> Hi Tudor,

Hi, Pratyush,

Thanks for reviewing this.

> 
> On 20/01/21 12:54PM, Tudor Ambarus wrote:
>> The Global Block Unlock command has different names depending
>> on the manufacturer, but always the same command value: 0x98.
>> Macronix's MX25U12835F names it Gang Block Unlock, Winbound's
>> W25Q128FV names it Global Block Unlock and Microchip's
>> SST26VF064B names it Global Block Protection Unlock.
>>
>> Used in the Individual Block Protection mode, which is mutual
> 
> Nitpick: s/mutual/mutually/
> 
>> exclusive with the Block Protection mode (BP0-3).
>>
>> Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com>
>> ---
>>  drivers/mtd/spi-nor/core.c  | 35 +++++++++++++++++++++++++++++++++++
>>  drivers/mtd/spi-nor/core.h  |  1 +
>>  include/linux/mtd/spi-nor.h |  1 +
>>  3 files changed, 37 insertions(+)
>>
>> diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
>> index 20df44b753da..cb275608cb7f 100644
>> --- a/drivers/mtd/spi-nor/core.c
>> +++ b/drivers/mtd/spi-nor/core.c
>> @@ -853,6 +853,41 @@ int spi_nor_wait_till_ready(struct spi_nor *nor)
>>                                                   DEFAULT_READY_WAIT_JIFFIES);
>>  }
>>
>> +/**
>> + * spi_nor_global_block_unlock() - Unlock Global Block Protection.
>> + * @nor:     pointer to 'struct spi_nor'.
>> + *
>> + * Return: 0 on success, -errno otherwise.
>> + */
>> +int spi_nor_global_block_unlock(struct spi_nor *nor)
>> +{
>> +     int ret;
>> +
>> +     ret = spi_nor_write_enable(nor);
>> +     if (ret)
>> +             return ret;
>> +
>> +     if (nor->spimem) {
>> +             struct spi_mem_op op =
>> +                     SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_GBULK, 1),
> 
> Set the buswidth to 0...
> 
>> +                                SPI_MEM_OP_NO_ADDR,
>> +                                SPI_MEM_OP_NO_DUMMY,
>> +                                SPI_MEM_OP_NO_DATA);
> 
> ... and run the op through spi_nor_spimem_setup_op().

Right, I thought that SPINOR_OP_GBULK is available just in single SPI
mode, but I see it can be issued in QPI mode as well. Will change.

Cheers,
ta
> 
> With this fixed,
> 
> Reviewed-by: Pratyush Yadav <p.yadav@ti.com>
> 
>> +
>> +             ret = spi_mem_exec_op(nor->spimem, &op);
>> +     } else {
>> +             ret = spi_nor_controller_ops_write_reg(nor, SPINOR_OP_GBULK,
>> +                                                    NULL, 0);
>> +     }
>> +
>> +     if (ret) {
>> +             dev_dbg(nor->dev, "error %d on Global Block Unlock\n", ret);
>> +             return ret;
>> +     }
>> +
>> +     return spi_nor_wait_till_ready(nor);
>> +}
>> +
>>  /**
>>   * spi_nor_write_sr() - Write the Status Register.
>>   * @nor:     pointer to 'struct spi_nor'.
>> diff --git a/drivers/mtd/spi-nor/core.h b/drivers/mtd/spi-nor/core.h
>> index d631ee299de3..eb26796db026 100644
>> --- a/drivers/mtd/spi-nor/core.h
>> +++ b/drivers/mtd/spi-nor/core.h
>> @@ -434,6 +434,7 @@ int spi_nor_write_disable(struct spi_nor *nor);
>>  int spi_nor_set_4byte_addr_mode(struct spi_nor *nor, bool enable);
>>  int spi_nor_write_ear(struct spi_nor *nor, u8 ear);
>>  int spi_nor_wait_till_ready(struct spi_nor *nor);
>> +int spi_nor_global_block_unlock(struct spi_nor *nor);
>>  int spi_nor_lock_and_prep(struct spi_nor *nor);
>>  void spi_nor_unlock_and_unprep(struct spi_nor *nor);
>>  int spi_nor_sr1_bit6_quad_enable(struct spi_nor *nor);
>> diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h
>> index d13958de6d8a..a0d572855444 100644
>> --- a/include/linux/mtd/spi-nor.h
>> +++ b/include/linux/mtd/spi-nor.h
>> @@ -53,6 +53,7 @@
>>  #define SPINOR_OP_WREAR              0xc5    /* Write Extended Address Register */
>>  #define SPINOR_OP_SRSTEN     0x66    /* Software Reset Enable */
>>  #define SPINOR_OP_SRST               0x99    /* Software Reset */
>> +#define SPINOR_OP_GBULK              0x98    /* Global Block Unlock */
>>
>>  /* 4-byte address opcodes - used on Spansion and some Macronix flashes. */
>>  #define SPINOR_OP_READ_4B    0x13    /* Read data bytes (low frequency) */
> 
> --
> Regards,
> Pratyush Yadav
> Texas Instruments India
> 

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/2] mtd: spi-nor: sst: Add support for Global Unlock on sst26vf
  2021-01-20 12:29     ` Pratyush Yadav
@ 2021-01-20 13:02       ` Tudor.Ambarus
  -1 siblings, 0 replies; 12+ messages in thread
From: Tudor.Ambarus @ 2021-01-20 13:02 UTC (permalink / raw)
  To: p.yadav
  Cc: michael, vigneshr, richard, linux-kernel, linux-mtd,
	Kavyasree.Kotagiri, miquel.raynal

On 1/20/21 2:29 PM, Pratyush Yadav wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> Hi Tudor,
> 
> On 20/01/21 12:54PM, Tudor Ambarus wrote:
>> Even if sst26vf shares the SPINOR_OP_GBULK opcode with
>> Macronix (ex. MX25U12835F) and Winbound (ex. W25Q128FV),
>> it has its own Individual Block Protection scheme, which
>> is also capable to read-lock individual parameter blocks.
>> Thus the sst26vf's Individual Block Protection scheme will
>> reside in the sst.c manufacturer driver.
>>
>> Add support to unlock the entire flash memory. The device
>> is write-protected by default after a power-on reset cycle
>> (volatile software protection), in order to avoid inadvertent
>> writes during power-up. Could do an erase, write, read back,
>> and compare when MTD_SPI_NOR_SWP_DISABLE_ON_VOLATILE=y.
>>
>> Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com>
>> ---
>>  drivers/mtd/spi-nor/sst.c | 38 ++++++++++++++++++++++++++++++++++++--
>>  1 file changed, 36 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/mtd/spi-nor/sst.c b/drivers/mtd/spi-nor/sst.c
>> index 00e48da0744a..1cd2a360c41e 100644
>> --- a/drivers/mtd/spi-nor/sst.c
>> +++ b/drivers/mtd/spi-nor/sst.c
>> @@ -8,6 +8,39 @@
>>
>>  #include "core.h"
>>
>> +static int sst26vf_lock(struct spi_nor *nor, loff_t ofs, uint64_t len)
>> +{
>> +     return -EOPNOTSUPP;
>> +}
>> +
>> +static int sst26vf_unlock(struct spi_nor *nor, loff_t ofs, uint64_t len)
>> +{
>> +     if (!ofs && len == nor->params->size)
> 
> Nitpick: ofs is not a boolean value. Don't treat it as such. (ofs == 0
> && len == nor->params->size) makes the intent much clearer.

I agree, will change. Cheers,
ta

> 
>> +             return spi_nor_global_block_unlock(nor);
>> +
>> +     return -EOPNOTSUPP;
>> +}
>> +
>> +static int sst26vf_is_locked(struct spi_nor *nor, loff_t ofs, uint64_t len)
>> +{
>> +     return -EOPNOTSUPP;
>> +}
>> +
>> +static const struct spi_nor_locking_ops sst26vf_locking_ops = {
>> +     .lock = sst26vf_lock,
>> +     .unlock = sst26vf_unlock,
>> +     .is_locked = sst26vf_is_locked,
>> +};
>> +
>> +static void sst26vf_default_init(struct spi_nor *nor)
>> +{
>> +     nor->params->locking_ops = &sst26vf_locking_ops;
>> +}
>> +
>> +static const struct spi_nor_fixups sst26vf_fixups = {
>> +     .default_init = sst26vf_default_init,
>> +};
>> +
>>  static const struct flash_info sst_parts[] = {
>>       /* SST -- large erase sizes are "overlays", "sectors" are 4K */
>>       { "sst25vf040b", INFO(0xbf258d, 0, 64 * 1024,  8,
>> @@ -39,8 +72,9 @@ static const struct flash_info sst_parts[] = {
>>       { "sst26vf016b", INFO(0xbf2641, 0, 64 * 1024, 32,
>>                             SECT_4K | SPI_NOR_DUAL_READ) },
>>       { "sst26vf064b", INFO(0xbf2643, 0, 64 * 1024, 128,
>> -                           SECT_4K | SPI_NOR_DUAL_READ |
>> -                           SPI_NOR_QUAD_READ) },
>> +                           SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ |
>> +                           SPI_NOR_HAS_LOCK | SPI_NOR_SWP_IS_VOLATILE)
>> +             .fixups = &sst26vf_fixups },
>>  };
>>
>>  static int sst_write(struct mtd_info *mtd, loff_t to, size_t len,
>> --
>> 2.25.1
>>
>>
>> ______________________________________________________
>> Linux MTD discussion mailing list
>> http://lists.infradead.org/mailman/listinfo/linux-mtd/
> 
> --
> Regards,
> Pratyush Yadav
> Texas Instruments India
> 


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/2] mtd: spi-nor: sst: Add support for Global Unlock on sst26vf
@ 2021-01-20 13:02       ` Tudor.Ambarus
  0 siblings, 0 replies; 12+ messages in thread
From: Tudor.Ambarus @ 2021-01-20 13:02 UTC (permalink / raw)
  To: p.yadav
  Cc: vigneshr, richard, linux-kernel, michael, linux-mtd,
	Kavyasree.Kotagiri, miquel.raynal

On 1/20/21 2:29 PM, Pratyush Yadav wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> Hi Tudor,
> 
> On 20/01/21 12:54PM, Tudor Ambarus wrote:
>> Even if sst26vf shares the SPINOR_OP_GBULK opcode with
>> Macronix (ex. MX25U12835F) and Winbound (ex. W25Q128FV),
>> it has its own Individual Block Protection scheme, which
>> is also capable to read-lock individual parameter blocks.
>> Thus the sst26vf's Individual Block Protection scheme will
>> reside in the sst.c manufacturer driver.
>>
>> Add support to unlock the entire flash memory. The device
>> is write-protected by default after a power-on reset cycle
>> (volatile software protection), in order to avoid inadvertent
>> writes during power-up. Could do an erase, write, read back,
>> and compare when MTD_SPI_NOR_SWP_DISABLE_ON_VOLATILE=y.
>>
>> Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com>
>> ---
>>  drivers/mtd/spi-nor/sst.c | 38 ++++++++++++++++++++++++++++++++++++--
>>  1 file changed, 36 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/mtd/spi-nor/sst.c b/drivers/mtd/spi-nor/sst.c
>> index 00e48da0744a..1cd2a360c41e 100644
>> --- a/drivers/mtd/spi-nor/sst.c
>> +++ b/drivers/mtd/spi-nor/sst.c
>> @@ -8,6 +8,39 @@
>>
>>  #include "core.h"
>>
>> +static int sst26vf_lock(struct spi_nor *nor, loff_t ofs, uint64_t len)
>> +{
>> +     return -EOPNOTSUPP;
>> +}
>> +
>> +static int sst26vf_unlock(struct spi_nor *nor, loff_t ofs, uint64_t len)
>> +{
>> +     if (!ofs && len == nor->params->size)
> 
> Nitpick: ofs is not a boolean value. Don't treat it as such. (ofs == 0
> && len == nor->params->size) makes the intent much clearer.

I agree, will change. Cheers,
ta

> 
>> +             return spi_nor_global_block_unlock(nor);
>> +
>> +     return -EOPNOTSUPP;
>> +}
>> +
>> +static int sst26vf_is_locked(struct spi_nor *nor, loff_t ofs, uint64_t len)
>> +{
>> +     return -EOPNOTSUPP;
>> +}
>> +
>> +static const struct spi_nor_locking_ops sst26vf_locking_ops = {
>> +     .lock = sst26vf_lock,
>> +     .unlock = sst26vf_unlock,
>> +     .is_locked = sst26vf_is_locked,
>> +};
>> +
>> +static void sst26vf_default_init(struct spi_nor *nor)
>> +{
>> +     nor->params->locking_ops = &sst26vf_locking_ops;
>> +}
>> +
>> +static const struct spi_nor_fixups sst26vf_fixups = {
>> +     .default_init = sst26vf_default_init,
>> +};
>> +
>>  static const struct flash_info sst_parts[] = {
>>       /* SST -- large erase sizes are "overlays", "sectors" are 4K */
>>       { "sst25vf040b", INFO(0xbf258d, 0, 64 * 1024,  8,
>> @@ -39,8 +72,9 @@ static const struct flash_info sst_parts[] = {
>>       { "sst26vf016b", INFO(0xbf2641, 0, 64 * 1024, 32,
>>                             SECT_4K | SPI_NOR_DUAL_READ) },
>>       { "sst26vf064b", INFO(0xbf2643, 0, 64 * 1024, 128,
>> -                           SECT_4K | SPI_NOR_DUAL_READ |
>> -                           SPI_NOR_QUAD_READ) },
>> +                           SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ |
>> +                           SPI_NOR_HAS_LOCK | SPI_NOR_SWP_IS_VOLATILE)
>> +             .fixups = &sst26vf_fixups },
>>  };
>>
>>  static int sst_write(struct mtd_info *mtd, loff_t to, size_t len,
>> --
>> 2.25.1
>>
>>
>> ______________________________________________________
>> Linux MTD discussion mailing list
>> http://lists.infradead.org/mailman/listinfo/linux-mtd/
> 
> --
> Regards,
> Pratyush Yadav
> Texas Instruments India
> 

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2021-01-20 14:17 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-01-20 10:54 [PATCH 1/2] mtd: spi-nor: Add Global Block Unlock command Tudor Ambarus
2021-01-20 10:54 ` Tudor Ambarus
2021-01-20 10:54 ` [PATCH 2/2] mtd: spi-nor: sst: Add support for Global Unlock on sst26vf Tudor Ambarus
2021-01-20 10:54   ` Tudor Ambarus
2021-01-20 12:29   ` Pratyush Yadav
2021-01-20 12:29     ` Pratyush Yadav
2021-01-20 13:02     ` Tudor.Ambarus
2021-01-20 13:02       ` Tudor.Ambarus
2021-01-20 12:29 ` [PATCH 1/2] mtd: spi-nor: Add Global Block Unlock command Pratyush Yadav
2021-01-20 12:29   ` Pratyush Yadav
2021-01-20 13:01   ` Tudor.Ambarus
2021-01-20 13:01     ` Tudor.Ambarus

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.