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

[Michael Walle]

Am 2021-05-31 10:52, schrieb Tudor.Ambarus@xxxxxxxxxxxxx:
> On 5/21/21 10:40 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.
>
> :)
>
> Shouldn't we change
> struct spi_nor_otp_ops {
> 	...
>         int (*lock)(struct spi_nor *nor, unsigned int region);
>         int (*is_locked)(struct spi_nor *nor, unsigned int region);
> };
>
> to:
> struct spi_nor_otp_ops {
> 	...
>         int (*lock)(struct spi_nor *nor, loff_t addr, size_t len);
>
>         int (*is_locked)(struct spi_nor *nor, loff_t addr, size_t len);
> };
>
> instead?

I had that, but then
 (1) it doesn't fit the hardware (the one's I know of) and the function
     itself would need to convert to the given range
 (2) each lock()/is_locked() would need to implement the "if at least
     one region is locked everything is locked", which might lead to
     different implementations.
 (3) in what address space is addr and len? I'd presume the one of the
     device (so is orthogonal to read()/write()). So if you get lets
     say addr=0x1000 len=512, you'd need to convert that into region
     0 and 1. Thus you'd have this mapping cluttered over all functions.
     And additionally, you'd first need to convert the mtd offsets
     addr=0 len=512 to addr=0x1000 and len=512.

-michael