[PATCH] mtd: spi-nor: macronix: Add block protection support to mx25u6435f

[Ikjoon Jang]

HI Michael, thanks for the review.

On Tue, Apr 13, 2021 at 8:26 PM Michael Walle <michael at walle.cc> wrote:
>
> Hi Ikjoon,
>
> Am 2021-04-13 14:02, schrieb Ikjoon Jang:
> > This patch adds block protection support to Macronix mx25u6432f and
> > mx25u6435f. Two different chips share the same JEDEC ID while only
> > mx25u6423f support section protections. And two chips have slightly
> > different definitions of BP bits than generic (ST Micro)
> > implementation.
>
> What is different compared to the current implementation? Could you give
> an example?

Two chips have different definitions on BP and  T/B bits.
- 35f has 4 BPs without T/B, BP3 behaves like T/B bit.
- 32f has 4 BPs plus T/B on CR.

# MX25U6435F

 BPs | BP3=0 | BP3=1
---------------------
 000 | None  | 1/2
 001 | 1/128 | 3/4
 010 | 1/64  | 7/8
 011 | 1/32  | 15/16
 100 | 1/16  | 31/32
 101 | 1/8   | 63/64
 110 | 1/4   | 127/128
 111 | 1/2   | All

# MX25U6432F

  BPs | T/B=0 | T/B=1
---------------------
 0000 | None  | None
 0001 | 1/128 | 1/128
 0010 | 1/64  | 1/64
 0011 | 1/32  | 1/32
 0100 | 1/16  | 1/16
 0101 | 1/8   | 1/8
 0110 | 1/4   | 1/4
 0111 | 1/2   | 1/2
 1xxx | All   | All

It seems that 35f has a unique definition on bottom protections than others.
Assuming there's no way to distinguish between mx25u6435f and 32f,
This patch simply takes the common parts only - BP[2:0]
without using T/B or BP3 at all.

But the current swp implementation implies that "BP with all ones"
is to be 'all' protection while in this approach it's 1/2,
A hidden BP3 should be involved for 'all' protection.

>
> > So this patch defines a new spi_nor_locking_ops only for macronix
> > until this could be merged into a generic swp implementation.
>
> TBH, I don't really like the code duplication and I'd presume that it
> won't ever be merged with the generic code.

Agree, I hope I could make a more generalized version into swp.c.

Honestly I expected that I just needed to add one line of SPI_NOR_HAS_LOCK
to flash_info to support mx256432f (this was the main purpose of my patch)
before I read the datasheets. :)

>
> You also assume that both the WPSEL and T/B bit are 0, which might not
> be true. Please note that both are write-once, thus should only be read.

yep, that also should be considered,
I'm thinking just not to support WPSEL=1 case for now.

>
> See also:
> https://lore.kernel.org/linux-mtd/346332bf6ab0dd92b9ffd9e126b6b97c@walle.cc/
>

Thanks, let me try it.

> -michael