[PATCH 0/2] enable lock interface for macronix chips

[PATCH 0/2] enable lock interface for macronix chips

[Ivan Mikhaylov]

TB(Top/Bottome protection bit) is not present in the status register for
macronix chips, do not touch TB bit in status register in this case.
Enable lock interface for most of macronix chips which are suitable for it.

Tested only mx25l51245g (BP0-3).

Ivan Mikhaylov (2):
  mtd: spi-nor: do not touch TB bit without SPI_NOR_HAS_TB
  mtd: spi-nor: enable lock interface for macronix chips

 drivers/mtd/spi-nor/core.c     | 22 +++++++---
 drivers/mtd/spi-nor/macronix.c | 75 ++++++++++++++++++++++------------
 2 files changed, 66 insertions(+), 31 deletions(-)

-- 
2.21.1

[PATCH 1/2] mtd: spi-nor: do not touch TB bit without SPI_NOR_HAS_TB

[Ivan Mikhaylov]

Some chips like macronix don't have TB(Top/Bottom protection)
bit in the status register. Do not write tb_mask inside status
register, unless SPI_NOR_HAS_TB is present for the chip.

Signed-off-by: Ivan Mikhaylov <i.mikhaylov@yadro.com>
---
 drivers/mtd/spi-nor/core.c | 22 ++++++++++++++++------
 1 file changed, 16 insertions(+), 6 deletions(-)

diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
index 0369d98b2d12..f9853dd566dc 100644
--- a/drivers/mtd/spi-nor/core.c
+++ b/drivers/mtd/spi-nor/core.c
@@ -1735,13 +1735,18 @@ static int spi_nor_sr_lock(struct spi_nor *nor, loff_t ofs, uint64_t len)
 			return -EINVAL;
 	}
 
-	status_new = (status_old & ~mask & ~tb_mask) | val;
+	if (nor->flags & SNOR_F_HAS_SR_TB)
+		status_new = (status_old & ~mask & ~tb_mask) | val;
+	else
+		status_new = (status_old & ~mask) | val;
 
 	/* Disallow further writes if WP pin is asserted */
 	status_new |= SR_SRWD;
 
-	if (!use_top)
-		status_new |= tb_mask;
+	if (!use_top) {
+		if (nor->flags & SNOR_F_HAS_SR_TB)
+			status_new |= tb_mask;
+	}
 
 	/* Don't bother if they're the same */
 	if (status_new == status_old)
@@ -1817,14 +1822,19 @@ static int spi_nor_sr_unlock(struct spi_nor *nor, loff_t ofs, uint64_t len)
 			return -EINVAL;
 	}
 
-	status_new = (status_old & ~mask & ~tb_mask) | val;
+	if (nor->flags & SNOR_F_HAS_SR_TB)
+		status_new = (status_old & ~mask & ~tb_mask) | val;
+	else
+		status_new = (status_old & ~mask) | val;
 
 	/* Don't protect status register if we're fully unlocked */
 	if (lock_len == 0)
 		status_new &= ~SR_SRWD;
 
-	if (!use_top)
-		status_new |= tb_mask;
+	if (!use_top) {
+		if (nor->flags & SNOR_F_HAS_SR_TB)
+			status_new |= tb_mask;
+	}
 
 	/* Don't bother if they're the same */
 	if (status_new == status_old)
-- 
2.21.1

[PATCH 2/2] mtd: spi-nor: enable lock interface for macronix chips

[Ivan Mikhaylov]

Add locks for whole macronix chip series with BP0-2 and BP0-3 bits.

Tested with mx25l51245g(BP0-3).

Signed-off-by: Ivan Mikhaylov <i.mikhaylov@yadro.com>
---
 drivers/mtd/spi-nor/macronix.c | 75 ++++++++++++++++++++++------------
 1 file changed, 50 insertions(+), 25 deletions(-)

diff --git a/drivers/mtd/spi-nor/macronix.c b/drivers/mtd/spi-nor/macronix.c
index 96735d83c77c..80de43eb05d6 100644
--- a/drivers/mtd/spi-nor/macronix.c
+++ b/drivers/mtd/spi-nor/macronix.c
@@ -37,53 +37,78 @@ static const struct flash_info macronix_parts[] = {
 	/* Macronix */
 	{ "mx25l512e",   INFO(0xc22010, 0, 64 * 1024,   1, SECT_4K) },
 	{ "mx25l2005a",  INFO(0xc22012, 0, 64 * 1024,   4, SECT_4K) },
-	{ "mx25l4005a",  INFO(0xc22013, 0, 64 * 1024,   8, SECT_4K) },
-	{ "mx25l8005",   INFO(0xc22014, 0, 64 * 1024,  16, 0) },
-	{ "mx25l1606e",  INFO(0xc22015, 0, 64 * 1024,  32, SECT_4K) },
-	{ "mx25l3205d",  INFO(0xc22016, 0, 64 * 1024,  64, SECT_4K) },
-	{ "mx25l3255e",  INFO(0xc29e16, 0, 64 * 1024,  64, SECT_4K) },
-	{ "mx25l6405d",  INFO(0xc22017, 0, 64 * 1024, 128, SECT_4K) },
-	{ "mx25u2033e",  INFO(0xc22532, 0, 64 * 1024,   4, SECT_4K) },
+	{ "mx25l4005a",  INFO(0xc22013, 0, 64 * 1024,   8,
+			      SECT_4K | SPI_NOR_HAS_LOCK) },
+	{ "mx25l8005",   INFO(0xc22014, 0, 64 * 1024,  16,
+			      SPI_NOR_HAS_LOCK) },
+	{ "mx25l1606e",  INFO(0xc22015, 0, 64 * 1024,  32,
+			      SECT_4K | SPI_NOR_HAS_LOCK | SPI_NOR_4BIT_BP) },
+	{ "mx25l3205d",  INFO(0xc22016, 0, 64 * 1024,  64,
+			      SECT_4K | SPI_NOR_HAS_LOCK | SPI_NOR_4BIT_BP) },
+	{ "mx25l3255e",  INFO(0xc29e16, 0, 64 * 1024,  64,
+			      SECT_4K | SPI_NOR_HAS_LOCK | SPI_NOR_4BIT_BP) },
+	{ "mx25l6405d",  INFO(0xc22017, 0, 64 * 1024, 128,
+			      SECT_4K | SPI_NOR_HAS_LOCK | SPI_NOR_4BIT_BP) },
+	{ "mx25u2033e",  INFO(0xc22532, 0, 64 * 1024,   4,
+			      SECT_4K | SPI_NOR_HAS_LOCK) },
 	{ "mx25u3235f",	 INFO(0xc22536, 0, 64 * 1024,  64,
 			      SECT_4K | SPI_NOR_DUAL_READ |
-			      SPI_NOR_QUAD_READ) },
-	{ "mx25u4035",   INFO(0xc22533, 0, 64 * 1024,   8, SECT_4K) },
-	{ "mx25u8035",   INFO(0xc22534, 0, 64 * 1024,  16, SECT_4K) },
-	{ "mx25u6435f",  INFO(0xc22537, 0, 64 * 1024, 128, SECT_4K) },
-	{ "mx25l12805d", INFO(0xc22018, 0, 64 * 1024, 256, 0) },
-	{ "mx25l12855e", INFO(0xc22618, 0, 64 * 1024, 256, 0) },
+			      SPI_NOR_QUAD_READ | SPI_NOR_HAS_LOCK |
+			      SPI_NOR_4BIT_BP) },
+	{ "mx25u4035",   INFO(0xc22533, 0, 64 * 1024,   8,
+			      SECT_4K | SPI_NOR_HAS_LOCK | SPI_NOR_4BIT_BP) },
+	{ "mx25u8035",   INFO(0xc22534, 0, 64 * 1024,  16,
+			      SECT_4K | SPI_NOR_HAS_LOCK | SPI_NOR_4BIT_BP) },
+	{ "mx25u6435f",  INFO(0xc22537, 0, 64 * 1024, 128,
+			      SECT_4K | SPI_NOR_HAS_LOCK | SPI_NOR_4BIT_BP) },
+	{ "mx25l12805d", INFO(0xc22018, 0, 64 * 1024, 256,
+			      SPI_NOR_HAS_LOCK | SPI_NOR_4BIT_BP) },
+	{ "mx25l12855e", INFO(0xc22618, 0, 64 * 1024, 256,
+			      SPI_NOR_HAS_LOCK | SPI_NOR_4BIT_BP) },
 	{ "mx25r3235f",  INFO(0xc22816, 0, 64 * 1024,  64,
 			      SECT_4K | SPI_NOR_DUAL_READ |
-			      SPI_NOR_QUAD_READ) },
+			      SPI_NOR_QUAD_READ | SPI_NOR_HAS_LOCK |
+			      SPI_NOR_4BIT_BP) },
 	{ "mx25u12835f", INFO(0xc22538, 0, 64 * 1024, 256,
 			      SECT_4K | SPI_NOR_DUAL_READ |
-			      SPI_NOR_QUAD_READ) },
+			      SPI_NOR_QUAD_READ | SPI_NOR_HAS_LOCK |
+			      SPI_NOR_4BIT_BP) },
 	{ "mx25l25635e", INFO(0xc22019, 0, 64 * 1024, 512,
-			      SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ)
+			      SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ |
+			      SPI_NOR_HAS_LOCK | SPI_NOR_4BIT_BP)
 		.fixups = &mx25l25635_fixups },
 	{ "mx25u25635f", INFO(0xc22539, 0, 64 * 1024, 512,
-			      SECT_4K | SPI_NOR_4B_OPCODES) },
+			      SECT_4K | SPI_NOR_4B_OPCODES | SPI_NOR_HAS_LOCK |
+			      SPI_NOR_4BIT_BP) },
 	{ "mx25u51245g", INFO(0xc2253a, 0, 64 * 1024, 1024,
 			      SECT_4K | SPI_NOR_DUAL_READ |
-			      SPI_NOR_QUAD_READ | SPI_NOR_4B_OPCODES) },
+			      SPI_NOR_QUAD_READ | SPI_NOR_4B_OPCODES |
+			      SPI_NOR_HAS_LOCK | SPI_NOR_4BIT_BP) },
 	{ "mx25v8035f",  INFO(0xc22314, 0, 64 * 1024,  16,
 			      SECT_4K | SPI_NOR_DUAL_READ |
-			      SPI_NOR_QUAD_READ) },
-	{ "mx25l25655e", INFO(0xc22619, 0, 64 * 1024, 512, 0) },
+			      SPI_NOR_QUAD_READ | SPI_NOR_HAS_LOCK |
+			      SPI_NOR_4BIT_BP) },
+	{ "mx25l25655e", INFO(0xc22619, 0, 64 * 1024, 512,
+			      SPI_NOR_HAS_LOCK | SPI_NOR_4BIT_BP) },
 	{ "mx25l51245g", INFO(0xc2201a, 0, 64 * 1024, 1024,
 			      SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ |
-			      SPI_NOR_4B_OPCODES) },
+			      SPI_NOR_4B_OPCODES | SPI_NOR_HAS_LOCK |
+			      SPI_NOR_4BIT_BP) },
 	{ "mx66l51235l", INFO(0xc2201a, 0, 64 * 1024, 1024,
 			      SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ |
-			      SPI_NOR_4B_OPCODES) },
+			      SPI_NOR_4B_OPCODES | SPI_NOR_HAS_LOCK |
+			      SPI_NOR_4BIT_BP) },
 	{ "mx66u51235f", INFO(0xc2253a, 0, 64 * 1024, 1024,
 			      SECT_4K | SPI_NOR_DUAL_READ |
-			      SPI_NOR_QUAD_READ | SPI_NOR_4B_OPCODES) },
+			      SPI_NOR_QUAD_READ | SPI_NOR_4B_OPCODES |
+			      SPI_NOR_HAS_LOCK | SPI_NOR_4BIT_BP) },
 	{ "mx66l1g45g",  INFO(0xc2201b, 0, 64 * 1024, 2048,
 			      SECT_4K | SPI_NOR_DUAL_READ |
-			      SPI_NOR_QUAD_READ) },
+			      SPI_NOR_QUAD_READ | SPI_NOR_HAS_LOCK |
+			      SPI_NOR_4BIT_BP) },
 	{ "mx66l1g55g",  INFO(0xc2261b, 0, 64 * 1024, 2048,
-			      SPI_NOR_QUAD_READ) },
+			      SPI_NOR_QUAD_READ | SPI_NOR_HAS_LOCK |
+			      SPI_NOR_4BIT_BP) },
 };
 
 static void macronix_default_init(struct spi_nor *nor)
-- 
2.21.1

Re: [PATCH 0/2] enable lock interface for macronix chips

[Ivan Mikhaylov]

On Wed, 2020-08-12 at 18:18 +0300, Ivan Mikhaylov wrote:
> TB(Top/Bottome protection bit) is not present in the status register for
> macronix chips, do not touch TB bit in status register in this case.
> Enable lock interface for most of macronix chips which are suitable for it.
> 
> Tested only mx25l51245g (BP0-3).
> 
> Ivan Mikhaylov (2):
>   mtd: spi-nor: do not touch TB bit without SPI_NOR_HAS_TB
>   mtd: spi-nor: enable lock interface for macronix chips
> 
>  drivers/mtd/spi-nor/core.c     | 22 +++++++---
>  drivers/mtd/spi-nor/macronix.c | 75 ++++++++++++++++++++++------------
>  2 files changed, 66 insertions(+), 31 deletions(-)
> 

ping

Re: [PATCH 2/2] mtd: spi-nor: enable lock interface for macronix chips

[Rasmus Villemoes]

On 12/08/2020 17.18, Ivan Mikhaylov wrote:
> Add locks for whole macronix chip series with BP0-2 and BP0-3 bits.
> 
> Tested with mx25l51245g(BP0-3).

Hmm. I've tried adding support for locking on Macronix to U-Boot
(https://patchwork.ozlabs.org/project/uboot/patch/20200326114257.1782-3-rasmus.villemoes@prevas.dk/),
but that was quite a bit more involved than this. Note in particular the
first part of my commit message:

  Macronix chips implements locking in (power-of-two multiple of) 64K
  blocks, not as a fraction of the chip's size.

At least, that was true for the chip I was interested in and the few
others whose data sheets I grabbed to double-check. So I'm a bit
skeptical that this can work out-of-the-box without introducing a new
struct spi_nor_locking_ops.

Rasmus

Re: [PATCH 2/2] mtd: spi-nor: enable lock interface for macronix chips

[Ivan Mikhaylov]

On Mon, 2020-08-31 at 12:55 +0200, Rasmus Villemoes wrote:
> On 12/08/2020 17.18, Ivan Mikhaylov wrote:
> > Add locks for whole macronix chip series with BP0-2 and BP0-3 bits.
> > 
> > Tested with mx25l51245g(BP0-3).
> 
> Hmm. I've tried adding support for locking on Macronix to U-Boot
> (
> https://patchwork.ozlabs.org/project/uboot/patch/20200326114257.1782-3-rasmus.villemoes@prevas.dk/
> ),
> but that was quite a bit more involved than this. Note in particular the
> first part of my commit message:
> 
>   Macronix chips implements locking in (power-of-two multiple of) 64K
>   blocks, not as a fraction of the chip's size.
> 
> At least, that was true for the chip I was interested in and the few
> others whose data sheets I grabbed to double-check. So I'm a bit
> skeptical that this can work out-of-the-box without introducing a new
> struct spi_nor_locking_ops.
> 
> Rasmus

Rasmus, but there is already locking of power-of-two as I see from the code,
I'll double check on hw. Also compared documentation n25q512ax3(micron, which
HAS_LOCK) to mx25l25635e(macronix) and they have same block protection table
bits for example. I'd be glad to hear from maintainers on this spot.

Thanks.