Messages in this thread | | | Subject | Re: [PATCH v5 3/5] mtd: spi-nor: otp: return -EROFS if region is read-only | From | Vignesh Raghavendra <> | Date | Mon, 7 Jun 2021 12:17:48 +0530 |
| |
On 6/7/21 11:38 AM, Michael Walle wrote: > 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@microchip.com: >>>> 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@walle.cc> >>>>> Reviewed-by: Pratyush Yadav <p.yadav@ti.com> >>>>> --- >>>>> 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. >
Looking further, I don't see the need for 4/5 to be a separate patch. Patch 4/5 is simplifying spi_nor_mtd_otp_range_is_locked() by ensuring 'len' passed is never 0 which can be done in 3/5 when introducing spi_nor_mtd_otp_range_is_locked().
So why not squashed it into 3/5.
> But sure, adding the same Fixes: tag, I can swap those two. > > -michael
| |