Re: [PATCH v2 1/2] spi-nor: add support for ISSI's block unlocking scheme

[Tudor.Ambarus]

Hi, Wesley, Palmer,

On 08/07/2018 10:40 PM, Palmer Dabbelt wrote:

First of all, thanks for the patience!

> From: "Wesley W. Terpstra" <wesley@xxxxxxxxxx>
> 
> ISSI uses a non-standard scheme to control block protection, with bit 5
> of the status registerr controlling an additional block protection bit.

Indeed, issi's block protection (BP3, BP2, BP1, BP0) bits are used in a
different way than how are used in stm_lock/unlock, even with the 4bit block
protection on its way (see [1]).

> This patch disables all the block protection bits whenever an ISSI chip
> is seen.

IS25WP256's datasheet says that "The default value of the BP0, BP1, BP2, BP3,
QE, and SRWD bits were set to â0â
 at factory.", which means that all memory blocks come unprotected (unlocked) by
default.

I see your patch as an extra safety measure for people that somehow
unfortunately set these block protection bits, so that they not end up with
locked blocks. Instead of adding this extra safety check/set, I would suggest to
actually add support for the issi's block protection scheme. In a perfect world,
this would fit in a per-manufacturer hook after the per-manufacturer split up
code will be integrated (see [2]).

> 
> We might also want to trigger an error when writing SR_TB to these
> chips, as it aliases with this extra protection bit in the status
> register.  It looks like that's always conditional on SNOR_F_HAS_SR_TB,
> so at least what's there is safe.

This problem will vanish when you'll have the issi's protection scheme implemented.

Cheers,
ta

[1] https://patchwork.ozlabs.org/patch/1011820/
[2] https://patchwork.ozlabs.org/project/linux-mtd/list/?series=80353