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

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

[David Bauer]

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;
 	nor->params->quad_enable = spi_nor_sr1_bit6_quad_enable;
 	nor->params->set_4byte_addr_mode = spi_nor_set_4byte_addr_mode;
 }
-- 
2.30.1


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

[PATCH 2/2] mtd: spi-nor: use 4 bit BP for large Macronix flash

[David Bauer]

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

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


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

Re: [PATCH 2/2] mtd: spi-nor: use 4 bit BP for large Macronix flash

[Michael Walle]

[This time with the correct message id for the reply. Sorry]

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>

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

Re: [PATCH 2/2] mtd: spi-nor: use 4 bit BP for large Macronix flash

[David Bauer]

Hello Michael,

On 3/8/21 1:37 PM, Michael Walle wrote:
> [This time with the correct message id for the reply. Sorry]
> 
> 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.

I've had a second look and i didn't spot SNOR_F_HAS_LOCK can be set per-flash. I was under the
impression to add SNOR_F_HAS_LOCK for the entire vendor in order to use SPI_NOR_HAS_LOCK per-flash.

So 1/2 is not necessary at all.

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

I've checked the datasheet of all flash chips I'm adding the block protection support
for, shall i still limit myself to devices I can test even though the datasheet indicate
support?

Best
David

> 
> -michael
> 
>>
>> Signed-off-by: David Bauer <mail@david-bauer.net>

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

Re: [PATCH 2/2] mtd: spi-nor: use 4 bit BP for large Macronix flash

[Tudor.Ambarus]

On 3/8/21 8:01 PM, David Bauer wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> Hello Michael,
> 
> On 3/8/21 1:37 PM, Michael Walle wrote:
>> [This time with the correct message id for the reply. Sorry]
>>
>> 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.
> 
> I've had a second look and i didn't spot SNOR_F_HAS_LOCK can be set per-flash. I was under the
> impression to add SNOR_F_HAS_LOCK for the entire vendor in order to use SPI_NOR_HAS_LOCK per-flash.
> 
> So 1/2 is not necessary at all.
> 
>>
>> 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.
> 
> I've checked the datasheet of all flash chips I'm adding the block protection support
> for, shall i still limit myself to devices I can test even though the datasheet indicate
> support?
> 

Yes, please add support just for the ones that you can test.
There are manufacturers that modify flash capabilities from a revision
to another, while keeping the same flash ID. All the flags that are added
must be tested, so that we know for sure that at least a revision of the
flash supports them, and that we should preserve backward compatibility in
software.
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

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

[Michael Walle]

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

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

Re: [PATCH 2/2] mtd: spi-nor: use 4 bit BP for large Macronix flash

[Michael Walle]

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)

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

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

[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.


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

Regards
Vignesh

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

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

[Michael Walle]

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

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