Re: [PATCH v2 1/2] mtd: spi-nor: add Global Block Unlock support

[<Tudor.Ambarus@microchip.com>]

On 08/25/2019 03:49 PM, Boris Brezillon wrote:
> External E-Mail
> 
> 
> On Sat, 24 Aug 2019 12:27:12 +0000
> <Tudor.Ambarus@microchip.com> wrote:
> 
>> From: Tudor Ambarus <tudor.ambarus@microchip.com>
>>
>> To avoid inadvertent writes during power-up, some flashes are
>> write-protected by default after a power-on reset cycle.
>> A Global Block-Protection Unlock command offers a single
>> command cycle that unlocks the entire memory array. This is
>> identical with what other nor flashes are doing by clearing
>> the block protection bits from the status register: disable
>> the write protection after a power-on reset cycle.
>>
>> We can't determine this purely by manufacturer type and it's not
>> autodetectable by anything like SFDP, so make a new flag for it:
>> UNLOCK_GLOBAL_BLOCK.
>>
>> Note that the Global Block Unlock command has different names
>> depending on the manufacturer, but always the same command value:
>> 0x98. Macronix's MX25U12835F names it Gang Block Unlock,
>> Winbound's W25Q128FV names it Global Block Unlock and
>> Microchip's SST26VF064B names it Global Block Protection Unlock.
>>
>> Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com>
>> ---
>> v2: the check for UNLOCK_GLOBAL_BLOCK should be done the first
>> thing in spi_nor_disable_block_protection(). We use it for a faster
>> throughput, a single command cycle that unlocks the entire memory
>> array. Fix it.
>>
>>  drivers/mtd/spi-nor/spi-nor.c | 46 ++++++++++++++++++++++++++++++++++++++++++-
>>  include/linux/mtd/spi-nor.h   |  1 +
>>  2 files changed, 46 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
>> index 1896d36a7d11..c0ba6fe62461 100644
>> --- a/drivers/mtd/spi-nor/spi-nor.c
>> +++ b/drivers/mtd/spi-nor/spi-nor.c
>> @@ -196,7 +196,7 @@ struct flash_info {
>>  	u16		page_size;
>>  	u16		addr_width;
>>  
>> -	u16		flags;
>> +	u32		flags;
>>  #define SECT_4K			BIT(0)	/* SPINOR_OP_BE_4K works uniformly */
>>  #define SPI_NOR_NO_ERASE	BIT(1)	/* No erase command needed */
>>  #define SST_WRITE		BIT(2)	/* use SST byte programming */
>> @@ -233,6 +233,7 @@ struct flash_info {
>>  #define SPI_NOR_SKIP_SFDP	BIT(13)	/* Skip parsing of SFDP tables */
>>  #define USE_CLSR		BIT(14)	/* use CLSR command */
>>  #define SPI_NOR_OCTAL_READ	BIT(15)	/* Flash supports Octal Read */
>> +#define UNLOCK_GLOBAL_BLOCK	BIT(16)	/* Unlock global block protection */
> 
> Let's add the corresponding SNOR_F_ flag so that manufacturer/sfdp init
> can set the flag directly.

Not really needed, because we can't determine this by parsing SFDP, there is no
field in SFDP to indicate this support. You can't set this at manufacturer
level, because it is not a per-manufacturer thing. Some flashes can support it,
some not even if all from the same manufacturer. So this global unlock command
it's at the per-chip level, and for the moment we should keep this flag just in
the flash_info's flags.