lkml.org 
[lkml]   [2021]   [Apr]   [14]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
Date
SubjectRe: [PATCH] mtd: spi-nor: macronix: Add block protection support to mx25u6435f
HI Michael, thanks for the review.

On Tue, Apr 13, 2021 at 8:26 PM Michael Walle <michael@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

\
 
 \ /
  Last update: 2021-04-14 08:55    [W:0.074 / U:1.020 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site