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

From: <Tudor.Ambarus@microchip.com>
To: <mail@david-bauer.net>, <michael@walle.cc>
Cc: <miquel.raynal@bootlin.com>, <richard@nod.at>, <vigneshr@ti.com>,
	<linux-mtd@lists.infradead.org>
Subject: Re: [PATCH 2/2] mtd: spi-nor: use 4 bit BP for large Macronix flash
Date: Tue, 9 Mar 2021 07:49:14 +0000	[thread overview]
Message-ID: <a0e30c1d-32d4-ab61-b5ad-730af1b21c35@microchip.com> (raw)
In-Reply-To: <40e3dd20-b4ed-53f0-7dbc-e2a16857dfc1@david-bauer.net>

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/