Re: [PATCH v4 2/4] mtd: spi-nor: implement OTP support for Winbond and similar flashes

[Michael Walle]

Am 2021-03-15 09:20, schrieb Tudor.Ambarus@xxxxxxxxxxxxx:
> On 3/6/21 2:05 AM, Michael Walle wrote:
> > EXTERNAL EMAIL: Do not click links or open attachments unless you know 
> > the content is safe
> >
> > Use the new OTP ops to implement OTP access on Winbond flashes. Most
> > Winbond flashes provides up to four different OTP regions ("Security
> > Registers").
> >
> > Winbond devices use a special opcode to read and write to the OTP
> > regions, just like the RDSFDP opcode. In fact, it seems that the
> > (undocumented) first OTP area of the newer flashes is the actual SFDP
> > table.
> >
> > On a side note, Winbond devices also allow erasing the OTP regions as
> > long as the area isn't locked down.
> >
> > Signed-off-by: Michael Walle <michael@xxxxxxxx>
> > ---
> >  drivers/mtd/spi-nor/core.c  |   2 +-
> >  drivers/mtd/spi-nor/core.h  |   6 ++
> >  drivers/mtd/spi-nor/otp.c   | 164 
> > ++++++++++++++++++++++++++++++++++++
> >  include/linux/mtd/spi-nor.h |   9 ++
> >  4 files changed, 180 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
> > index 0c5c757fa95b..ef7df26896f1 100644
> > --- a/drivers/mtd/spi-nor/core.c
> > +++ b/drivers/mtd/spi-nor/core.c
> > @@ -1034,7 +1034,7 @@ static int 
> > spi_nor_write_16bit_sr_and_check(struct spi_nor *nor, u8 sr1)
> >   *
> >   * Return: 0 on success, -errno otherwise.
> >   */
> > -static int spi_nor_write_16bit_cr_and_check(struct spi_nor *nor, u8 
> > cr)
> > +int spi_nor_write_16bit_cr_and_check(struct spi_nor *nor, u8 cr)
> >  {
> >         int ret;
> >         u8 *sr_cr = nor->bouncebuf;
> > diff --git a/drivers/mtd/spi-nor/core.h b/drivers/mtd/spi-nor/core.h
> > index ec8da1243846..dfbf6ba42b57 100644
> > --- a/drivers/mtd/spi-nor/core.h
> > +++ b/drivers/mtd/spi-nor/core.h
> > @@ -496,6 +496,7 @@ int spi_nor_read_sr(struct spi_nor *nor, u8 *sr);
> >  int spi_nor_read_cr(struct spi_nor *nor, u8 *cr);
> >  int spi_nor_write_sr(struct spi_nor *nor, const u8 *sr, size_t len);
> >  int spi_nor_write_sr_and_check(struct spi_nor *nor, u8 sr1);
> > +int spi_nor_write_16bit_cr_and_check(struct spi_nor *nor, u8 cr);
> >
> >  int spi_nor_xread_sr(struct spi_nor *nor, u8 *sr);
> >  ssize_t spi_nor_read_data(struct spi_nor *nor, loff_t from, size_t 
> > len,
> > @@ -503,6 +504,11 @@ ssize_t spi_nor_read_data(struct spi_nor *nor, 
> > loff_t from, size_t len,
> >  ssize_t spi_nor_write_data(struct spi_nor *nor, loff_t to, size_t 
> > len,
> >                            const u8 *buf);
> >
> > +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, u8 *buf);
> > +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);
> > +
> >  int spi_nor_hwcaps_read2cmd(u32 hwcaps);
> >  u8 spi_nor_convert_3to4_read(u8 opcode);
> >  void spi_nor_set_read_settings(struct spi_nor_read_command *read,
> > diff --git a/drivers/mtd/spi-nor/otp.c b/drivers/mtd/spi-nor/otp.c
> > index 4e301fd5156b..4e8da9108c77 100644
> > --- a/drivers/mtd/spi-nor/otp.c
> > +++ b/drivers/mtd/spi-nor/otp.c
> > @@ -15,6 +15,170 @@
> >  #define spi_nor_otp_region_len(nor) ((nor)->params->otp.org->len)
> >  #define spi_nor_otp_n_regions(nor) 
> > ((nor)->params->otp.org->n_regions)
> >
> > +/**
> > + * spi_nor_otp_read_secr() - read OTP data
> > + * @nor:       pointer to 'struct spi_nor'
> > + * @from:       offset to read from
> > + * @len:        number of bytes to read
> > + * @buf:        pointer to dst buffer
>
> is buf DMA-able?

That's actually the same description as spi_nor_read_data().
Looks like the spimem will provide a DMA-able buffer on
the fly if necessary. I'm not sure about the the
spi_nor_controller_ops.

> > + *
> > + * Read OTP data from one region by using the SPINOR_OP_RSECR 
> > commands. This
> > + * method is used on GigaDevice and Winbond flashes.
> > + *
> > + * Return: number of bytes read successfully, -errno otherwise
> > + */
> > +int spi_nor_otp_read_secr(struct spi_nor *nor, loff_t addr, size_t 
> > len, u8 *buf)
> > +{
> > +       u8 addr_width, read_opcode, read_dummy;
> > +       struct spi_mem_dirmap_desc *rdesc;
> > +       enum spi_nor_protocol read_proto;
> > +       int ret;
> > +
> > +       read_opcode = nor->read_opcode;
> > +       addr_width = nor->addr_width;
> > +       read_dummy = nor->read_dummy;
> > +       read_proto = nor->read_proto;
> > +       rdesc = nor->dirmap.rdesc;
> > +
> > +       nor->read_opcode = SPINOR_OP_RSECR;
> > +       nor->addr_width = 3;
> > +       nor->read_dummy = 8;
> > +       nor->read_proto = SNOR_PROTO_1_1_1;
>
> any winbond/gigadevice flashes with octal dtr support? Do they
> provide SEC Register opcodes for octal dtr?

AFAIK there are no winbond flashes with 8 bit I/O. There are
4bit/DTR modes, but not for the security registers.

I don't know what you had in mind. But I don't think it is
worth to read the 3x 256byte data faster than single bit I/O.

-michael