linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [RESEND PATCH 0/2] enable lock interface for macronix chips
@ 2020-09-21 11:24 Ivan Mikhaylov
  2020-09-21 11:24 ` [RESEND PATCH 1/2] mtd: spi-nor: do not touch TB bit without SPI_NOR_HAS_TB Ivan Mikhaylov
  2020-09-21 11:24 ` [RESEND PATCH 2/2] mtd: spi-nor: enable lock interface for macronix chips Ivan Mikhaylov
  0 siblings, 2 replies; 9+ messages in thread
From: Ivan Mikhaylov @ 2020-09-21 11:24 UTC (permalink / raw)
  To: Tudor Ambarus, Miquel Raynal, Richard Weinberger, Vignesh Raghavendra
  Cc: Ivan Mikhaylov, linux-mtd, linux-kernel

TB(Top/Bottom 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


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

* [RESEND PATCH 1/2] mtd: spi-nor: do not touch TB bit without SPI_NOR_HAS_TB
  2020-09-21 11:24 [RESEND PATCH 0/2] enable lock interface for macronix chips Ivan Mikhaylov
@ 2020-09-21 11:24 ` Ivan Mikhaylov
  2020-09-30  9:36   ` Vignesh Raghavendra
  2020-09-21 11:24 ` [RESEND PATCH 2/2] mtd: spi-nor: enable lock interface for macronix chips Ivan Mikhaylov
  1 sibling, 1 reply; 9+ messages in thread
From: Ivan Mikhaylov @ 2020-09-21 11:24 UTC (permalink / raw)
  To: Tudor Ambarus, Miquel Raynal, Richard Weinberger, Vignesh Raghavendra
  Cc: Ivan Mikhaylov, linux-mtd, linux-kernel

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


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

* [RESEND PATCH 2/2] mtd: spi-nor: enable lock interface for macronix chips
  2020-09-21 11:24 [RESEND PATCH 0/2] enable lock interface for macronix chips Ivan Mikhaylov
  2020-09-21 11:24 ` [RESEND PATCH 1/2] mtd: spi-nor: do not touch TB bit without SPI_NOR_HAS_TB Ivan Mikhaylov
@ 2020-09-21 11:24 ` Ivan Mikhaylov
  2020-09-30  9:40   ` Vignesh Raghavendra
  1 sibling, 1 reply; 9+ messages in thread
From: Ivan Mikhaylov @ 2020-09-21 11:24 UTC (permalink / raw)
  To: Tudor Ambarus, Miquel Raynal, Richard Weinberger, Vignesh Raghavendra
  Cc: Ivan Mikhaylov, linux-mtd, linux-kernel

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


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

* Re: [RESEND PATCH 1/2] mtd: spi-nor: do not touch TB bit without SPI_NOR_HAS_TB
  2020-09-21 11:24 ` [RESEND PATCH 1/2] mtd: spi-nor: do not touch TB bit without SPI_NOR_HAS_TB Ivan Mikhaylov
@ 2020-09-30  9:36   ` Vignesh Raghavendra
  2020-09-30 13:07     ` Ivan Mikhaylov
  0 siblings, 1 reply; 9+ messages in thread
From: Vignesh Raghavendra @ 2020-09-30  9:36 UTC (permalink / raw)
  To: Ivan Mikhaylov, Tudor Ambarus, Miquel Raynal, Richard Weinberger
  Cc: linux-mtd, linux-kernel



On 9/21/20 4:54 PM, Ivan Mikhaylov wrote:
> 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.
> 

Not entirely accurate.. Macronix chips have TB bit in config register
and is OTP and hence should not be touched ideally...

You still need to "read" that bit to determine actual scheme (Top vs
Bottom). This is needs to be done before 2/2 enables SPI_NOR_HAS_LOCK
flag for macronix flashes.

> 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;
>  

I guess macronix does not support SR_SRWD right? This needs special
treatment as well.

So either, macronix.c should implements its own locking ops or convert
this function in to more generic library so that its suitable to be
called from macronix.c file while hiding vendor specific stuff in that
driver,

Regards
Vignesh


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

* Re: [RESEND PATCH 2/2] mtd: spi-nor: enable lock interface for macronix chips
  2020-09-21 11:24 ` [RESEND PATCH 2/2] mtd: spi-nor: enable lock interface for macronix chips Ivan Mikhaylov
@ 2020-09-30  9:40   ` Vignesh Raghavendra
  2020-09-30 13:09     ` Ivan Mikhaylov
  0 siblings, 1 reply; 9+ messages in thread
From: Vignesh Raghavendra @ 2020-09-30  9:40 UTC (permalink / raw)
  To: Ivan Mikhaylov, Tudor Ambarus, Miquel Raynal, Richard Weinberger
  Cc: linux-mtd, linux-kernel

Hi,

On 9/21/20 4:54 PM, Ivan Mikhaylov wrote:
> Add locks for whole macronix chip series with BP0-2 and BP0-3 bits.
> 
> Tested with mx25l51245g(BP0-3).

Since you have only tested on flash that have 4bit BP, please don't
modify flashes that have 3bit BP. Lets be conservative and enable only
things that have been tested else we may end up with broken feature from
day 1.

Regards
Vignesh

> 
> 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)
> 

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

* Re: [RESEND PATCH 1/2] mtd: spi-nor: do not touch TB bit without SPI_NOR_HAS_TB
  2020-09-30  9:36   ` Vignesh Raghavendra
@ 2020-09-30 13:07     ` Ivan Mikhaylov
  2020-09-30 14:00       ` Vignesh Raghavendra
  0 siblings, 1 reply; 9+ messages in thread
From: Ivan Mikhaylov @ 2020-09-30 13:07 UTC (permalink / raw)
  To: Vignesh Raghavendra, Tudor Ambarus, Miquel Raynal, Richard Weinberger
  Cc: linux-mtd, linux-kernel

On Wed, 2020-09-30 at 15:06 +0530, Vignesh Raghavendra wrote:
> 
> On 9/21/20 4:54 PM, Ivan Mikhaylov wrote:
> > 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.
> > 
> 
> Not entirely accurate.. Macronix chips have TB bit in config register
> and is OTP and hence should not be touched ideally...
> 
> You still need to "read" that bit to determine actual scheme (Top vs
> Bottom). This is needs to be done before 2/2 enables SPI_NOR_HAS_LOCK
> flag for macronix flashes.

Vignesh, that's the point about this commit to generalize this part about TB bit
plus there is already exist SPI_NOR_HAS_TB flag which representing state of TB
existence. I didn't add any support for macronix's TB bit, that's true but
that's enough to make macronix chips able to use lock mechanism with default
'use_top' or any other chips which doesn't have TB bit.

> I guess macronix does not support SR_SRWD right? This needs special
> treatment as well.

It does support SR_SRWD as well. No need any special treatment here.

Thanks.


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

* Re: [RESEND PATCH 2/2] mtd: spi-nor: enable lock interface for macronix chips
  2020-09-30  9:40   ` Vignesh Raghavendra
@ 2020-09-30 13:09     ` Ivan Mikhaylov
  0 siblings, 0 replies; 9+ messages in thread
From: Ivan Mikhaylov @ 2020-09-30 13:09 UTC (permalink / raw)
  To: Vignesh Raghavendra, Tudor Ambarus, Miquel Raynal, Richard Weinberger
  Cc: linux-mtd, linux-kernel

On Wed, 2020-09-30 at 15:10 +0530, Vignesh Raghavendra wrote:
> Hi,
> 
> On 9/21/20 4:54 PM, Ivan Mikhaylov wrote:
> > Add locks for whole macronix chip series with BP0-2 and BP0-3 bits.
> > 
> > Tested with mx25l51245g(BP0-3).
> 
> Since you have only tested on flash that have 4bit BP, please don't
> modify flashes that have 3bit BP. Lets be conservative and enable only
> things that have been tested else we may end up with broken feature from
> day 1.
> 

Sure, will do then this way.

Thanks.


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

* Re: [RESEND PATCH 1/2] mtd: spi-nor: do not touch TB bit without SPI_NOR_HAS_TB
  2020-09-30 13:07     ` Ivan Mikhaylov
@ 2020-09-30 14:00       ` Vignesh Raghavendra
  2020-09-30 16:22         ` Ivan Mikhaylov
  0 siblings, 1 reply; 9+ messages in thread
From: Vignesh Raghavendra @ 2020-09-30 14:00 UTC (permalink / raw)
  To: Ivan Mikhaylov, Tudor Ambarus, Miquel Raynal, Richard Weinberger
  Cc: linux-mtd, linux-kernel



On 9/30/20 6:37 PM, Ivan Mikhaylov wrote:
> On Wed, 2020-09-30 at 15:06 +0530, Vignesh Raghavendra wrote:
>>
>> On 9/21/20 4:54 PM, Ivan Mikhaylov wrote:
>>> 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.
>>>
>>
>> Not entirely accurate.. Macronix chips have TB bit in config register
>> and is OTP and hence should not be touched ideally...
>>
>> You still need to "read" that bit to determine actual scheme (Top vs
>> Bottom). This is needs to be done before 2/2 enables SPI_NOR_HAS_LOCK
>> flag for macronix flashes.
> 
> Vignesh, that's the point about this commit to generalize this part about TB bit
> plus there is already exist SPI_NOR_HAS_TB flag which representing state of TB
> existence. I didn't add any support for macronix's TB bit, that's true but
> that's enough to make macronix chips able to use lock mechanism with default
> 'use_top' or any other chips which doesn't have TB bit.

Right, but 2/2 "enables" locking mechanism for Macronix flashes. Therefore its 
necessary to take TB bit into account so that implementation is correct. 
What if OTP bit is set as "use_bottom"? Although this is non default, 
we need to take care of this case for correctness.

> 
>> I guess macronix does not support SR_SRWD right? This needs special
>> treatment as well.
> 
> It does support SR_SRWD as well. No need any special treatment here.
> 

I did not find it in one Macronix datasheet at least:
https://www.macronix.com/Lists/Datasheet/Attachments/7902/MX25L25673G,%203V,%20256Mb,%20v1.6.pdf

Are you sure all Macronix flashes support SRWD?

> Thanks.
> 

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

* Re: [RESEND PATCH 1/2] mtd: spi-nor: do not touch TB bit without SPI_NOR_HAS_TB
  2020-09-30 14:00       ` Vignesh Raghavendra
@ 2020-09-30 16:22         ` Ivan Mikhaylov
  0 siblings, 0 replies; 9+ messages in thread
From: Ivan Mikhaylov @ 2020-09-30 16:22 UTC (permalink / raw)
  To: Vignesh Raghavendra, Tudor Ambarus, Miquel Raynal, Richard Weinberger
  Cc: linux-mtd, linux-kernel

On Wed, 2020-09-30 at 19:30 +0530, Vignesh Raghavendra wrote:
> 
> On 9/30/20 6:37 PM, Ivan Mikhaylov wrote:
> > On Wed, 2020-09-30 at 15:06 +0530, Vignesh Raghavendra wrote:
> > > On 9/21/20 4:54 PM, Ivan Mikhaylov wrote:
> > > > 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.
> > > > 
> > > 
> > > Not entirely accurate.. Macronix chips have TB bit in config register
> > > and is OTP and hence should not be touched ideally...
> > > 
> > > You still need to "read" that bit to determine actual scheme (Top vs
> > > Bottom). This is needs to be done before 2/2 enables SPI_NOR_HAS_LOCK
> > > flag for macronix flashes.
> > 
> > Vignesh, that's the point about this commit to generalize this part about TB
> > bit
> > plus there is already exist SPI_NOR_HAS_TB flag which representing state of
> > TB
> > existence. I didn't add any support for macronix's TB bit, that's true but
> > that's enough to make macronix chips able to use lock mechanism with default
> > 'use_top' or any other chips which doesn't have TB bit.
> 
> Right, but 2/2 "enables" locking mechanism for Macronix flashes. Therefore
> its 
> necessary to take TB bit into account so that implementation is correct. 
> What if OTP bit is set as "use_bottom"? Although this is non default, 
> we need to take care of this case for correctness.

Maybe wording of my commit message is incorrect, let's try to think about this
commit without macronix words in it. What do you think? Just additional patch
for control TB writes.

mtd: spi-nor: do not touch TB bit without SPI_NOR_HAS_TB
    
Do not write tb_mask inside status register, unless SPI_NOR_HAS_TB is   		
present for the chip.

If we talking from OTP point for Macronix then in this case better way to make
lock/unlock inside macronix.c which brings a lot of copypaste. I'll try to
rework it.

> > > I guess macronix does not support SR_SRWD right? This needs special
> > > treatment as well.
> > 
> > It does support SR_SRWD as well. No need any special treatment here.
> > 
> 
> I did not find it in one Macronix datasheet at least:
> https://www.macronix.com/Lists/Datasheet/Attachments/7902/MX25L25673G,%203V,%20256Mb,%20v1.6.pdf
> 
> Are you sure all Macronix flashes support SRWD?
> 

No, I'm not sure, I did it more than month ago and I've checked BP0-X bits +
SRWD bits in the documentation at this time for whole set of chips in
macronix.c. This one (mx25l25673g) not even listed in macronix.c. Also SRWD was
present there until 1.3 rev for this chip from documentation.

I've noticed one thing also:

	{ "mx25l51245g", INFO(0xc2201a, 0, 64 * 1024, 1024,
			      SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ |
			      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_HAS_LOCK |
			      SPI_NOR_4BIT_BP) },

mx25l51245g and mx66l51235l have same id and different flags(SECT_4K).
As example if you have mx66l51235l, driver will take mx25l51245g because it
comes first in the chip list. I don't think that's right but I didn't find
information how to distinguish them.

Thanks.


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

end of thread, other threads:[~2020-09-30 16:18 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-21 11:24 [RESEND PATCH 0/2] enable lock interface for macronix chips Ivan Mikhaylov
2020-09-21 11:24 ` [RESEND PATCH 1/2] mtd: spi-nor: do not touch TB bit without SPI_NOR_HAS_TB Ivan Mikhaylov
2020-09-30  9:36   ` Vignesh Raghavendra
2020-09-30 13:07     ` Ivan Mikhaylov
2020-09-30 14:00       ` Vignesh Raghavendra
2020-09-30 16:22         ` Ivan Mikhaylov
2020-09-21 11:24 ` [RESEND PATCH 2/2] mtd: spi-nor: enable lock interface for macronix chips Ivan Mikhaylov
2020-09-30  9:40   ` Vignesh Raghavendra
2020-09-30 13:09     ` Ivan Mikhaylov

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).