Re: [PATCH v5 3/5] mtd: spi-nor: otp: return -EROFS if region is read-only

[Michael Walle]

Am 2021-06-07 07:46, schrieb Vignesh Raghavendra:
> On 6/4/21 6:45 PM, Michael Walle wrote:
> > Am 2021-06-04 15:07, schrieb Tudor.Ambarus@xxxxxxxxxxxxx:
> > > On 6/4/21 1:02 PM, Michael Walle wrote:
> > > > EXTERNAL EMAIL: Do not click links or open attachments unless you
> > > > know the content is safe
> > > >
> > > > SPI NOR flashes will just ignore program commands if the OTP region 
> > > > is
> > > > locked. Thus, a user might not notice that the intended write didn't 
> > > > end
> > > > up in the flash. Return -EROFS to the user in this case. From what I 
> > > > can
> > > > tell, chips/cfi_cmdset_0001.c also return this error code.
> > > >
> > > > One could optimize spi_nor_mtd_otp_range_is_locked() to read the 
> > > > status
> > > > register only once and not for every OTP region, but for that we 
> > > > would
> > > > need some more invasive changes. Given that this is
> > > > one-time-programmable memory and the normal access mode is reading, 
> > > > we
> > > > just live with the small overhead.
> > > >
> > > > Fixes: 069089acf88b ("mtd: spi-nor: add OTP support")
> > > > Signed-off-by: Michael Walle <michael@xxxxxxxx>
> > > > Reviewed-by: Pratyush Yadav <p.yadav@xxxxxx>
> > > > ---
> > > >  drivers/mtd/spi-nor/otp.c | 36 ++++++++++++++++++++++++++++++++++++
> > > >  1 file changed, 36 insertions(+)
> > > >
> > > > diff --git a/drivers/mtd/spi-nor/otp.c b/drivers/mtd/spi-nor/otp.c
> > > > index 3898ed67ba1c..063f8fb68649 100644
> > > > --- a/drivers/mtd/spi-nor/otp.c
> > > > +++ b/drivers/mtd/spi-nor/otp.c
> > > > @@ -249,6 +249,32 @@ static int spi_nor_mtd_otp_info(struct mtd_info
> > > > *mtd, size_t len,
> > > >         return ret;
> > > >  }
> > > >
> > > > +static int spi_nor_mtd_otp_range_is_locked(struct spi_nor *nor,
> > > > loff_t ofs,
> > > > +                                          size_t len)
> > > > +{
> > > > +       const struct spi_nor_otp_ops *ops = nor->params->otp.ops;
> > > > +       unsigned int region;
> > > > +       int locked;
> > > > +
> > > > +       if (!len)
> > > > +               return 0;
> > > > +
> > >
> > > You won't need this if you put patch 4/5 before this one. With this:
> >
> > This patch will get backported to the stable kernels. Patch 4 on the
> > other hand does not.
>
> I don't see why 4/5 cannot be marked for backport too as it makes 3/5
> much cleaner?

What kind of problem does 4/5 fix? I can't see how that patch would
apply to any rule in Documentation/process/stable-kernel-rules.rst.

But sure, adding the same Fixes: tag, I can swap those two.

-michael