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

Tudor.Ambarus
2021-03-09T07:50:44Z

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.