diff mbox series

[1/2] mtd: spi-nor: add block protection flags to macronix

Message ID 20210303094833.139221-1-mail@david-bauer.net
State Changes Requested
Delegated to: Ambarus Tudor
Headers show
Series [1/2] mtd: spi-nor: add block protection flags to macronix | expand

Commit Message

David Bauer March 3, 2021, 9:48 a.m. UTC
Macronix flash chips support block protection by using BP bits in the
read status register. Add the corresponding flag to indicate block
protection support.

Otherwise, locked blocks are not unlocked when requested.

Signed-off-by: David Bauer <mail@david-bauer.net>
---
 drivers/mtd/spi-nor/macronix.c | 1 +
 1 file changed, 1 insertion(+)

Comments

Michael Walle March 8, 2021, 9:16 a.m. UTC | #1
> Macronix flash chips support block protection by using BP bits in the
> read status register. Add the corresponding flag to indicate block
> protection support.
> 
> Otherwise, locked blocks are not unlocked when requested.
> 
> Signed-off-by: David Bauer <mail@david-bauer.net>
> ---
>  drivers/mtd/spi-nor/macronix.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/mtd/spi-nor/macronix.c b/drivers/mtd/spi-nor/macronix.c
> index 9203abaac229..2d39dd32a64e 100644
> --- a/drivers/mtd/spi-nor/macronix.c
> +++ b/drivers/mtd/spi-nor/macronix.c
> @@ -94,6 +94,7 @@ static const struct flash_info macronix_parts[] = {
>  
>  static void macronix_default_init(struct spi_nor *nor)
>  {
> +	nor->flags |= SNOR_F_HAS_LOCK;

Please do not add global locking support. Add it per flash device.
See also:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=e6204d4620276398ed7317d64c369813a1f96615

-michael

>  	nor->params->quad_enable = spi_nor_sr1_bit6_quad_enable;
>  	nor->params->set_4byte_addr_mode = spi_nor_set_4byte_addr_mode;
>  }
Michael Walle March 8, 2021, 10:32 a.m. UTC | #2
Hi David,

Thanks for your patch.

> Macronix SPI-NOR chips with 128 or more 64k blocks have 4 block
> protection bits in their status register. Add the corresponding
> flag in order to clear these bits when unloking the flash.
> 
> Otherwise, the flash might not be writable depending on the state the
> bootloader left the flash in.
> 
> Fixes commit 62593cf40b23 ("mtd: spi-nor: refactor block protection functions")

Macronix didn't support locking before your patch 1/2, right?
Therefore, this patch will just adding features.

Please limit this patch to devices which you are able to test and
mention in the commit log on what SPI controller it was tested on.

-michael

> 
> Signed-off-by: David Bauer <mail@david-bauer.net>
---
 drivers/mtd/spi-nor/macronix.c | 31 ++++++++++++++++++-------------
 1 file changed, 18 insertions(+), 13 deletions(-)

diff --git a/drivers/mtd/spi-nor/macronix.c b/drivers/mtd/spi-nor/macronix.c
index 2d39dd32a64e..4f9fa8b8d57f 100644
--- a/drivers/mtd/spi-nor/macronix.c
+++ b/drivers/mtd/spi-nor/macronix.c
@@ -50,8 +50,8 @@ static const struct flash_info macronix_parts[] = {
 	{ "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, SECT_4K) },
-	{ "mx25l12855e", INFO(0xc22618, 0, 64 * 1024, 256, 0) },
+	{ "mx25l12805d", INFO(0xc22018, 0, 64 * 1024, 256, SECT_4K | SPI_NOR_4BIT_BP) },
+	{ "mx25l12855e", INFO(0xc22618, 0, 64 * 1024, 256, SPI_NOR_4BIT_BP) },
 	{ "mx25r1635f",  INFO(0xc22815, 0, 64 * 1024,  32,
 			      SECT_4K | SPI_NOR_DUAL_READ |
 			      SPI_NOR_QUAD_READ) },
@@ -60,36 +60,41 @@ static const struct flash_info macronix_parts[] = {
 			      SPI_NOR_QUAD_READ) },
 	{ "mx25u12835f", INFO(0xc22538, 0, 64 * 1024, 256,
 			      SECT_4K | SPI_NOR_DUAL_READ |
-			      SPI_NOR_QUAD_READ) },
+			      SPI_NOR_QUAD_READ | 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_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_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_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) },
+	{ "mx25l25655e", INFO(0xc22619, 0, 64 * 1024, 512, 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_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_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_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_4BIT_BP) },
 	{ "mx66l1g55g",  INFO(0xc2261b, 0, 64 * 1024, 2048,
-			      SPI_NOR_QUAD_READ) },
+			      SPI_NOR_QUAD_READ | SPI_NOR_4BIT_BP) },
 	{ "mx66u2g45g",	 INFO(0xc2253c, 0, 64 * 1024, 4096,
 			      SECT_4K | SPI_NOR_DUAL_READ |
-			      SPI_NOR_QUAD_READ | SPI_NOR_4B_OPCODES) },
+			      SPI_NOR_QUAD_READ | SPI_NOR_4B_OPCODES |
+			      SPI_NOR_4BIT_BP) },
 };
 
 static void macronix_default_init(struct spi_nor *nor)
Vignesh Raghavendra March 8, 2021, 2:20 p.m. UTC | #3
On 3/3/21 3:18 PM, David Bauer wrote:
> Macronix flash chips support block protection by using BP bits in the
> read status register. Add the corresponding flag to indicate block
> protection support.
> 
> Otherwise, locked blocks are not unlocked when requested.
> 
> Signed-off-by: David Bauer <mail@david-bauer.net>
> ---
>  drivers/mtd/spi-nor/macronix.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/mtd/spi-nor/macronix.c b/drivers/mtd/spi-nor/macronix.c
> index 9203abaac229..2d39dd32a64e 100644
> --- a/drivers/mtd/spi-nor/macronix.c
> +++ b/drivers/mtd/spi-nor/macronix.c
> @@ -94,6 +94,7 @@ static const struct flash_info macronix_parts[] = {
>  
>  static void macronix_default_init(struct spi_nor *nor)
>  {
> +	nor->flags |= SNOR_F_HAS_LOCK;

We need to take into account the state of TB bit (Config Reg bit3). This
is an OTP bit and driver should not ideally change it but should
consider the state of the bit.

Without looking at TB bit, current locking implementation would be
incomplete.


>  	nor->params->quad_enable = spi_nor_sr1_bit6_quad_enable;
>  	nor->params->set_4byte_addr_mode = spi_nor_set_4byte_addr_mode;
>  }
> 

Regards
Vignesh
Michael Walle March 8, 2021, 3:20 p.m. UTC | #4
Am 2021-03-08 15:20, schrieb Vignesh Raghavendra:
> On 3/3/21 3:18 PM, David Bauer wrote:
>> Macronix flash chips support block protection by using BP bits in the
>> read status register. Add the corresponding flag to indicate block
>> protection support.
>> 
>> Otherwise, locked blocks are not unlocked when requested.
>> 
>> Signed-off-by: David Bauer <mail@david-bauer.net>
>> ---
>>  drivers/mtd/spi-nor/macronix.c | 1 +
>>  1 file changed, 1 insertion(+)
>> 
>> diff --git a/drivers/mtd/spi-nor/macronix.c 
>> b/drivers/mtd/spi-nor/macronix.c
>> index 9203abaac229..2d39dd32a64e 100644
>> --- a/drivers/mtd/spi-nor/macronix.c
>> +++ b/drivers/mtd/spi-nor/macronix.c
>> @@ -94,6 +94,7 @@ static const struct flash_info macronix_parts[] = {
>> 
>>  static void macronix_default_init(struct spi_nor *nor)
>>  {
>> +	nor->flags |= SNOR_F_HAS_LOCK;
> 
> We need to take into account the state of TB bit (Config Reg bit3). 
> This
> is an OTP bit and driver should not ideally change it but should
> consider the state of the bit.
> 
> Without looking at TB bit, current locking implementation would be
> incomplete.

If someone wants to dig into that, some time ago I tried it myself:
https://github.com/mwalle/linux/commit/1a56154bbe1696268d1245eea9e170a2feb7ca46
https://github.com/mwalle/linux/commit/7f3d4f7cbac87a6c5b900be7164dabb8991b0f93

I've never managed to post them to the list and they aren't tested
and they have yet another new flag.. And there was some catch with
it, I don't remember anymore right now.

-michael
diff mbox series

Patch

diff --git a/drivers/mtd/spi-nor/macronix.c b/drivers/mtd/spi-nor/macronix.c
index 9203abaac229..2d39dd32a64e 100644
--- a/drivers/mtd/spi-nor/macronix.c
+++ b/drivers/mtd/spi-nor/macronix.c
@@ -94,6 +94,7 @@  static const struct flash_info macronix_parts[] = {
 
 static void macronix_default_init(struct spi_nor *nor)
 {
+	nor->flags |= SNOR_F_HAS_LOCK;
 	nor->params->quad_enable = spi_nor_sr1_bit6_quad_enable;
 	nor->params->set_4byte_addr_mode = spi_nor_set_4byte_addr_mode;
 }