Re: [PATCH v6 4/5] mtd: spi-nor: atmel: Fix unlock_all() for AT25FS010/040

[Michael Walle <michael@walle.cc>]

Am 2020-11-28 09:25, schrieb Tudor.Ambarus@microchip.com:
> On 11/26/20 10:26 PM, Michael Walle wrote:
>> EXTERNAL EMAIL: Do not click links or open attachments unless you know 
>> the content is safe
>> 
>> These flashes have some weird BP bits mapping which aren't supported 
>> in
>> the current locking code. Just add a simple unlock op to unprotect the
>> entire flash array which is needed for legacy behavior.
>> 
>> Signed-off-by: Michael Walle <michael@walle.cc>
>> ---
>> changes since v5
>>  - new patch
>> 
>>  drivers/mtd/spi-nor/atmel.c | 53 
>> +++++++++++++++++++++++++++++++++++--
>>  drivers/mtd/spi-nor/core.c  |  2 +-
>>  drivers/mtd/spi-nor/core.h  |  1 +
>>  3 files changed, 53 insertions(+), 3 deletions(-)
>> 
>> diff --git a/drivers/mtd/spi-nor/atmel.c b/drivers/mtd/spi-nor/atmel.c
>> index 49d392c6c8bc..fe6a4653823d 100644
>> --- a/drivers/mtd/spi-nor/atmel.c
>> +++ b/drivers/mtd/spi-nor/atmel.c
>> @@ -8,10 +8,59 @@
>> 
>>  #include "core.h"
>> 
>> +/*
>> + * The Atmel AT25FS010/AT25FS040 parts have some weird configuration 
>> for the
>> + * block protection bits. We don't support them. But legacy behaviour 
>> in linux
>> + * is to unlock the whole flash array on startup. Therefore, we have 
>> to support
>> + * exactly this operation.
>> + */
>> +static int atmel_at25fs_lock(struct spi_nor *nor, loff_t ofs, 
>> uint64_t len)
>> +{
>> +       return -EOPNOTSUPP;
>> +}
>> +
>> +static int atmel_at25fs_unlock(struct spi_nor *nor, loff_t ofs, 
>> uint64_t len)
>> +{
>> +       /* We only support unlocking the whole flash array */
>> +       if (ofs || len != nor->params->size)
>> +               return -EINVAL;
>> +
>> +       /*
>> +        * Write 0x00 to the status register to try to disable the 
>> write
>> +        * protection. This will fail if SRWD (the datasheet calls it 
>> WPEN) is
>> +        * set. But there is nothing we can do.
>> +        */
> 
> can't we do the same as you did in 5/5?

Sure, but - assuming it is only used for the legacy unlock all operation 
- the
outcome will be the same. It will either keep being locked or all will 
be
unlocked.

That being said, I can also change it to the same as the 
global_unprotect().
I don't have any option on that other than this is simpler.

-michael