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

["chaochao2021666@163.com" <chaochao2021666@163.com>]

On 2021/11/15 13:57, Tudor.Ambarus@microchip.com wrote:

> Hi,
>
> + Michael
>
> On 11/15/21 4:37 AM,chaochao2021666@163.com  wrote:
>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>>
>> From: chao zeng<chao.zeng@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 before operating the flash
>> and give a prompt to show it has been locked if the write-protection
>> has enbale
>>
>> Signed-off-by: chao zeng<chao.zeng@siemens.com>
>>
>> ---
>>
>> Changes for V2:
>>       - Return 0 not ENOPROTOOPT to refelect the flash feature
>>       - Output prompt information
>> ---
>>   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 f461082e03..995801817d 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)) {
>> +               printf("SF: Flash is locked\n");
> I would use a debug message, it's a flash specific thing. Also, I would update
> a bit the message, something like
> "SF: Flash has protected areas in the requested length. Writes will be ignored on those."
>
>> +               return 0;
> Michael has suggested to drop this line. I agree with him, check the conversation
> on the previous email thread.
>
> Cheers,
> ta
>
>> +       }
>> +
>>          return mtd->_write(mtd, offset, len, &retlen, buf);
>>   }
>>
>> @@ -127,6 +132,11 @@ static int spi_flash_std_erase(struct udevice *dev, u32 offset, size_t len)
>>          instr.addr = offset;
>>          instr.len = len;
>>
>> +       if (flash->flash_is_locked && flash->flash_is_locked(flash, offset, len)) {
>> +               printf("SF: Flash is locked\n");
>> +               return 0;
>> +       }
>> +
>>          return mtd->_erase(mtd, &instr);
>>   }
>>
>> --
>> 2.33.1
>>
>>

the background is we like to use sf command to operate the flash under 
uboot shell,

"sf erase" command still would show the prompt  "erase ok" even though  
write-enable has enabled.


So at the beginning  I'd like to return an error ,so the sf operation 
would show "erase failed" when operating the write-enabled devices.


I'm agree with onlyoutput information to prompt the user the operation 
unsuccessful.

But It should explicitly giveclear hints,so I suggest at here using 
printf not debug.


BRs

Chao