| Message ID | 20210520155854.16547-4-michael@walle.cc |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | mtd: spi-nor: otp: 4 byte mode fix and erase support | expand |
On 20/05/21 05:58PM, Michael Walle wrote: > Winbond flashes with OTP support provide a command to erase the OTP > data. This might come in handy during development. > > This was tested with a Winbond W25Q32JW on a LS1028A SoC with the > NXP FSPI controller. > > Signed-off-by: Michael Walle <michael@walle.cc> > --- [...] > diff --git a/drivers/mtd/spi-nor/otp.c b/drivers/mtd/spi-nor/otp.c > index ec0c1b33f7cc..2dc315b6bffc 100644 > --- a/drivers/mtd/spi-nor/otp.c > +++ b/drivers/mtd/spi-nor/otp.c > @@ -111,6 +111,34 @@ int spi_nor_otp_write_secr(struct spi_nor *nor, loff_t addr, size_t len, > return ret ?: written; > } > > +/** > + * spi_nor_otp_erase_secr() - erase a security register > + * @nor: pointer to 'struct spi_nor' > + * @addr: offset of the security register to be erased > + * > + * Erase a security register by using the SPINOR_OP_ESECR command. This method > + * is used on GigaDevice and Winbond flashes to erase OTP data. > + * > + * Return: 0 on success, -errno otherwise > + */ > +int spi_nor_otp_erase_secr(struct spi_nor *nor, loff_t addr) > +{ > + u8 erase_opcode = nor->erase_opcode; > + int ret; > + > + ret = spi_nor_write_enable(nor); > + if (ret) > + return ret; > + > + nor->erase_opcode = SPINOR_OP_ESECR; > + ret = spi_nor_erase_sector(nor, addr); > + nor->erase_opcode = erase_opcode; > + if (ret) > + return ret; > + > + return spi_nor_wait_till_ready(nor); The datasheet for W25Q32JW says in Section 8.2.29: The Security Register Lock Bits (LB3-1) in the Status Register-2 can be used to OTP protect the security registers. Once a lock bit is set to 1, the corresponding security register will be permanently locked, Erase Security Register instruction to that register will be ignored So if the region is locked, the flash will happily accept the erase and simply do nothing. So will the program. So when the OTP region is locked and someone does an erase-program cycle, they will think their data went through even though it was simply thrown away by the flash. I think you should check that bit before doing these operations to make sure it is actually allowed. If it isn't, return an error code (-EPERM maybe). > +} > + > static int spi_nor_otp_lock_bit_cr(unsigned int region) > { > static const int lock_bits[] = { SR2_LB1, SR2_LB2, SR2_LB3 }; [...]
Am 2021-05-20 19:51, schrieb Pratyush Yadav: > On 20/05/21 05:58PM, Michael Walle wrote: >> Winbond flashes with OTP support provide a command to erase the OTP >> data. This might come in handy during development. >> >> This was tested with a Winbond W25Q32JW on a LS1028A SoC with the >> NXP FSPI controller. >> >> Signed-off-by: Michael Walle <michael@walle.cc> >> --- > [...] >> diff --git a/drivers/mtd/spi-nor/otp.c b/drivers/mtd/spi-nor/otp.c >> index ec0c1b33f7cc..2dc315b6bffc 100644 >> --- a/drivers/mtd/spi-nor/otp.c >> +++ b/drivers/mtd/spi-nor/otp.c >> @@ -111,6 +111,34 @@ int spi_nor_otp_write_secr(struct spi_nor *nor, >> loff_t addr, size_t len, >> return ret ?: written; >> } >> >> +/** >> + * spi_nor_otp_erase_secr() - erase a security register >> + * @nor: pointer to 'struct spi_nor' >> + * @addr: offset of the security register to be erased >> + * >> + * Erase a security register by using the SPINOR_OP_ESECR command. >> This method >> + * is used on GigaDevice and Winbond flashes to erase OTP data. >> + * >> + * Return: 0 on success, -errno otherwise >> + */ >> +int spi_nor_otp_erase_secr(struct spi_nor *nor, loff_t addr) >> +{ >> + u8 erase_opcode = nor->erase_opcode; >> + int ret; >> + >> + ret = spi_nor_write_enable(nor); >> + if (ret) >> + return ret; >> + >> + nor->erase_opcode = SPINOR_OP_ESECR; >> + ret = spi_nor_erase_sector(nor, addr); >> + nor->erase_opcode = erase_opcode; >> + if (ret) >> + return ret; >> + >> + return spi_nor_wait_till_ready(nor); > > The datasheet for W25Q32JW says in Section 8.2.29: > > The Security Register Lock Bits (LB3-1) in the Status Register-2 can > be used to OTP protect the security registers. Once a lock bit is set > to 1, the corresponding security register will be permanently locked, > Erase Security Register instruction to that register will be ignored > > So if the region is locked, the flash will happily accept the erase and > simply do nothing. So will the program. So when the OTP region is > locked > and someone does an erase-program cycle, they will think their data > went > through even though it was simply thrown away by the flash. Btw, this is also how it is handled for the "normal" data write protection. If block protection bits are set, we happily accept erase and write commands although the flash will discard the commands. > I think you should check that bit before doing these operations to make > sure it is actually allowed. If it isn't, return an error code (-EPERM > maybe). It's not that easy. You'd have to check the entire range, not just a part of it. Otherwise you'll end up with half of the data committed to the flash and the other half rejected with EPERM. So you'd have to do in spi_nor_mtd_otp_erase() and spi_nor_mtd_otp_write(). I'm not opposed to it, but I tried to behave in the same way as the other MTD chips which supports the OTP ops and I thought they would ignore it, too. But I just had another look: - chips/cfi_cmdset_0001.c: seems to return -EROFS (partial writes are possible?). I couldn't find any datasheets where command 0xc0 is used to write OTP data. Thus I can only guess that status bit 1 is program failed or something like that [1]. All the datasheets I found will tell you its bit 0. - chips/cfi_cmdset_0002.c: seems to ignore errors - nand/onenand/onenand_base.c: I really don't know.. If no one opposes I'll add that check with EROFS. -michael [1] https://elixir.bootlin.com/linux/v5.13-rc2/source/drivers/mtd/chips/cfi_cmdset_0001.c#L1606
diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c index bd2c7717eb10..9551effb6a44 100644 --- a/drivers/mtd/spi-nor/core.c +++ b/drivers/mtd/spi-nor/core.c @@ -1318,7 +1318,7 @@ static u32 spi_nor_convert_addr(struct spi_nor *nor, loff_t addr) /* * Initiate the erasure of a single sector */ -static int spi_nor_erase_sector(struct spi_nor *nor, u32 addr) +int spi_nor_erase_sector(struct spi_nor *nor, u32 addr) { int i; diff --git a/drivers/mtd/spi-nor/core.h b/drivers/mtd/spi-nor/core.h index 28a2e0be97a3..9398a8738857 100644 --- a/drivers/mtd/spi-nor/core.h +++ b/drivers/mtd/spi-nor/core.h @@ -207,6 +207,7 @@ struct spi_nor_otp_organization { * @read: read from the SPI NOR OTP area. * @write: write to the SPI NOR OTP area. * @lock: lock an OTP region. + * @erase: erase an OTP region. * @is_locked: check if an OTP region of the SPI NOR is locked. */ struct spi_nor_otp_ops { @@ -214,6 +215,7 @@ struct spi_nor_otp_ops { int (*write)(struct spi_nor *nor, loff_t addr, size_t len, const u8 *buf); int (*lock)(struct spi_nor *nor, unsigned int region); + int (*erase)(struct spi_nor *nor, loff_t addr); int (*is_locked)(struct spi_nor *nor, unsigned int region); }; @@ -503,10 +505,12 @@ ssize_t spi_nor_read_data(struct spi_nor *nor, loff_t from, size_t len, u8 *buf); ssize_t spi_nor_write_data(struct spi_nor *nor, loff_t to, size_t len, const u8 *buf); +int spi_nor_erase_sector(struct spi_nor *nor, u32 addr); int spi_nor_otp_read_secr(struct spi_nor *nor, loff_t addr, size_t len, u8 *buf); int spi_nor_otp_write_secr(struct spi_nor *nor, loff_t addr, size_t len, const u8 *buf); +int spi_nor_otp_erase_secr(struct spi_nor *nor, loff_t addr); int spi_nor_otp_lock_sr2(struct spi_nor *nor, unsigned int region); int spi_nor_otp_is_locked_sr2(struct spi_nor *nor, unsigned int region); diff --git a/drivers/mtd/spi-nor/otp.c b/drivers/mtd/spi-nor/otp.c index ec0c1b33f7cc..2dc315b6bffc 100644 --- a/drivers/mtd/spi-nor/otp.c +++ b/drivers/mtd/spi-nor/otp.c @@ -111,6 +111,34 @@ int spi_nor_otp_write_secr(struct spi_nor *nor, loff_t addr, size_t len, return ret ?: written; } +/** + * spi_nor_otp_erase_secr() - erase a security register + * @nor: pointer to 'struct spi_nor' + * @addr: offset of the security register to be erased + * + * Erase a security register by using the SPINOR_OP_ESECR command. This method + * is used on GigaDevice and Winbond flashes to erase OTP data. + * + * Return: 0 on success, -errno otherwise + */ +int spi_nor_otp_erase_secr(struct spi_nor *nor, loff_t addr) +{ + u8 erase_opcode = nor->erase_opcode; + int ret; + + ret = spi_nor_write_enable(nor); + if (ret) + return ret; + + nor->erase_opcode = SPINOR_OP_ESECR; + ret = spi_nor_erase_sector(nor, addr); + nor->erase_opcode = erase_opcode; + if (ret) + return ret; + + return spi_nor_wait_till_ready(nor); +} + static int spi_nor_otp_lock_bit_cr(unsigned int region) { static const int lock_bits[] = { SR2_LB1, SR2_LB2, SR2_LB3 }; @@ -316,12 +344,14 @@ static int spi_nor_mtd_otp_write(struct mtd_info *mtd, loff_t to, size_t len, return spi_nor_mtd_otp_read_write(mtd, to, len, retlen, buf, true); } -static int spi_nor_mtd_otp_lock(struct mtd_info *mtd, loff_t from, size_t len) +static int spi_nor_mtd_otp_lock_or_erase(struct mtd_info *mtd, loff_t from, + size_t len, bool is_erase) { struct spi_nor *nor = mtd_to_spi_nor(mtd); const struct spi_nor_otp_ops *ops = nor->params->otp.ops; const size_t rlen = spi_nor_otp_region_len(nor); unsigned int region; + loff_t rstart; int ret; if (from < 0 || (from + len) > spi_nor_otp_size(nor)) @@ -337,7 +367,13 @@ static int spi_nor_mtd_otp_lock(struct mtd_info *mtd, loff_t from, size_t len) while (len) { region = spi_nor_otp_offset_to_region(nor, from); - ret = ops->lock(nor, region); + + if (is_erase) { + rstart = spi_nor_otp_region_start(nor, region); + ret = ops->erase(nor, rstart); + } else { + ret = ops->lock(nor, region); + } if (ret) goto out; @@ -351,6 +387,23 @@ static int spi_nor_mtd_otp_lock(struct mtd_info *mtd, loff_t from, size_t len) return ret; } +static int spi_nor_mtd_otp_lock(struct mtd_info *mtd, loff_t from, size_t len) +{ + return spi_nor_mtd_otp_lock_or_erase(mtd, from, len, false); +} + +static int spi_nor_mtd_otp_erase(struct mtd_info *mtd, loff_t from, size_t len) +{ + struct spi_nor *nor = mtd_to_spi_nor(mtd); + const struct spi_nor_otp_ops *ops = nor->params->otp.ops; + + /* OTP erase is optional */ + if (!ops->erase) + return -EOPNOTSUPP; + + return spi_nor_mtd_otp_lock_or_erase(mtd, from, len, true); +} + void spi_nor_otp_init(struct spi_nor *nor) { struct mtd_info *mtd = &nor->mtd; @@ -374,4 +427,5 @@ void spi_nor_otp_init(struct spi_nor *nor) mtd->_read_user_prot_reg = spi_nor_mtd_otp_read; mtd->_write_user_prot_reg = spi_nor_mtd_otp_write; mtd->_lock_user_prot_reg = spi_nor_mtd_otp_lock; + mtd->_erase_user_prot_reg = spi_nor_mtd_otp_erase; } diff --git a/drivers/mtd/spi-nor/winbond.c b/drivers/mtd/spi-nor/winbond.c index 9a81c67a60c6..96573f61caf5 100644 --- a/drivers/mtd/spi-nor/winbond.c +++ b/drivers/mtd/spi-nor/winbond.c @@ -139,6 +139,7 @@ static int winbond_set_4byte_addr_mode(struct spi_nor *nor, bool enable) static const struct spi_nor_otp_ops winbond_otp_ops = { .read = spi_nor_otp_read_secr, .write = spi_nor_otp_write_secr, + .erase = spi_nor_otp_erase_secr, .lock = spi_nor_otp_lock_sr2, .is_locked = spi_nor_otp_is_locked_sr2, };
Winbond flashes with OTP support provide a command to erase the OTP data. This might come in handy during development. This was tested with a Winbond W25Q32JW on a LS1028A SoC with the NXP FSPI controller. Signed-off-by: Michael Walle <michael@walle.cc> --- drivers/mtd/spi-nor/core.c | 2 +- drivers/mtd/spi-nor/core.h | 4 +++ drivers/mtd/spi-nor/otp.c | 58 +++++++++++++++++++++++++++++++++-- drivers/mtd/spi-nor/winbond.c | 1 + 4 files changed, 62 insertions(+), 3 deletions(-)