Re: [RFC PATCH] mtd: add OTP (one-time-programmable) erase ioctl

Am 2021-03-02 17:33, schrieb Vignesh Raghavendra:

On 3/2/21 9:49 PM, Michael Walle wrote:

quoted

Am 2021-03-02 16:30, schrieb Vignesh Raghavendra:

quoted

Hi,

On 3/2/21 4:39 PM, Michael Walle wrote:

quoted

This may sound like a contradiction but some SPI-NOR flashes really
support erasing their OTP region until it is finally locked. Having 
the
possibility to erase an OTP region might come in handy during
development.

The ioctl argument follows the OTPLOCK style.

Signed-off-by: Michael Walle <redacted>
---
OTP support for SPI-NOR flashes may be merged soon:
https://lore.kernel.org/linux-mtd/20210216162807.13509-1-michael@walle.cc/ (local)


Tudor suggested to add support for the OTP erase operation most 
SPI-NOR
flashes have:
https://lore.kernel.org/linux-mtd/d4f74b1b-fa1b-97ec-858c-d807fe1f9e57@microchip.com/ (local)


Therefore, this is an RFC to get some feedback on the MTD side, once
this
is finished, I can post a patch for mtd-utils. Then we'll have a
foundation
to add the support to SPI-NOR.

 drivers/mtd/mtdchar.c      |  7 ++++++-
 drivers/mtd/mtdcore.c      | 12 ++++++++++++
 include/linux/mtd/mtd.h    |  3 +++
 include/uapi/mtd/mtd-abi.h |  2 ++
 4 files changed, 23 insertions(+), 1 deletion(-)

diff --git a/drivers/mtd/mtdchar.c b/drivers/mtd/mtdchar.c
index 323035d4f2d0..da423dd031ae 100644
--- a/drivers/mtd/mtdchar.c
+++ b/drivers/mtd/mtdchar.c

@@ -661,6 +661,7 @@ static int mtdchar_ioctl(struct file *file, 

u_int
cmd, u_long arg)
     case OTPGETREGIONCOUNT:
     case OTPGETREGIONINFO:
     case OTPLOCK:
+    case OTPERASE:

This is not a Safe IOCTL. We are destroying OTP data. Need to check 
for
write permission before allowing the ioctl right?

Ah yes, of course. But this makes me wonder why OTPLOCK
is considered a safe command. As well as MEMLOCK and
MEMUNLOCK. And MEMSETBADBLOCK. Shouldn't these also
require write permissions?

Well, one argument would be that LOCK/UNLOCK in itself won't modify 
data
and thus does not need write permission.. Although can brick a flash
from ever being writable again and change content of flash registers.

Whether not you can brick a device (I agree with you), it is writing
to the protection bits. But what is more imporant is that OTPLOCK
is actually write-once.

I am fine with moving these to require write permissions as well
(probably OTPLOCK as well).

Ok, I'm unsure about MEMSETBADBLOCK, though.

-michael