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

[Michael Walle]

Am 2020-11-28 09:25, schrieb Tudor.Ambarus@xxxxxxxxxxxxx:
> 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@xxxxxxxx>
> > ---
> > 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