[PATCH] sf: Querying write-protect status before operating the

[Tudor.Ambarus at microchip.com]

On 11/8/21 11:50 AM, Michael Walle wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
>> On Fri, Nov 5, 2021 at 10:47 PM <Tudor.Ambarus at microchip.com> wrote:
>>>
>>> Hi,
>>>
>>> On 6/22/21 8:21 AM, chao zeng wrote:
>>>> From: Chao Zeng <chao.zeng at siemens.com>
>>>>
>>>> When operating the write-protection flash,spi_flash_std_write() and
>>>> spi_flash_std_erase() would return wrong result.The flash is protected,
>>>> but write or erase the flash would show "OK".
>>>>
>>>> Check the flash write protection state if the write-protection has enbale
>>>> before operating the flash.
>>>>
>>>> Signed-off-by: Chao Zeng <chao.zeng at siemens.com>
>>>> ---
>>>>
>>>>  drivers/mtd/spi/sf_probe.c | 10 ++++++++++
>>>>  1 file changed, 10 insertions(+)
>>>>
>>>> diff --git a/drivers/mtd/spi/sf_probe.c b/drivers/mtd/spi/sf_probe.c
>>>> index 3befbe91ca..f06e6b88bd 100644
>>>> --- a/drivers/mtd/spi/sf_probe.c
>>>> +++ b/drivers/mtd/spi/sf_probe.c
>>>> @@ -109,6 +109,11 @@ static int spi_flash_std_write(struct udevice *dev, u32 offset, size_t len,
>>>>       struct mtd_info *mtd = &flash->mtd;
>>>>       size_t retlen;
>>>>
>>>> +     if (flash->flash_is_locked && flash->flash_is_locked(flash, offset, len)) {
>>>> +             debug("SF: Flash is locked\n");
>>>> +             return -ENOPROTOOPT;
>>>
>>> Keep a debug message, but return 0 please. Writes or erases on protected areas
>>> are ignored by the flash, we should reflect that in the code.
> 
> Mh, will this then make the whole write fail? We do rely on the fact,
> that we can update the whole flash image, but the first sectors will
> be 'skipped' because the first are write-protected. I guess this patch
> will then break this.
> 
> Shouldn't this then be on a per sector basis?
> 

Writes are already very slow, interrogating the flash for each sector will
make the writes even slower. We may extend the API to have a function that
return the map of locked sectors, so that we can read only once and decide
on a per sector basis.
But you are right, it would be better to have the granularity per sector.
Other option would be to just check if flash is locked at this level, just
print a debug message then fallthrough to the write/erase method and let
the flash ignore the opcode on protected areas.

Cheers,
ta