Messages in this thread | | | From | Geert Uytterhoeven <> | Date | Tue, 11 Jun 2019 10:35:59 +0200 | Subject | Re: [PATCH] mtd: spi-nor: use 16-bit WRR command when QE is set on spansion flashes |
| |
Hi Tudor,
On Mon, Jun 10, 2019 at 8:24 AM <Tudor.Ambarus@microchip.com> wrote: > From: Tudor Ambarus <tudor.ambarus@microchip.com> > > SPI memory devices from different manufacturers have widely > different configurations for Status, Control and Configuration > registers. JEDEC 216C defines a new map for these common register > bits and their functions, and describes how the individual bits may > be accessed for a specific device. For the JEDEC 216B compliant > flashes, we can partially deduce Status and Configuration registers > functions by inspecting the 16th DWORD of BFPT. Older flashes that > don't declare the SFDP tables (SPANSION FL512SAIFG1 311QQ063 A ©11 > SPANSION) let the software decide how to interact with these registers. > > The commit dcb4b22eeaf4 ("spi-nor: s25fl512s supports region locking") > uncovered a probe error for s25fl512s, when the QUAD bit CR[1] was set > in the bootloader. When this bit is set, only the Write Register > WRR command format with 16 data bits may be used, WRR with 8 bits > is not recognized and hence the error when trying to clear the block > protection bits. > > Fix the above by using 16-bits WRR command when Quad bit is set. > > Backward compatibility should be fine. The newly introduced > spi_nor_spansion_clear_sr_bp() is tightly coupled with the > spansion_quad_enable() function. Both assume that the Write Register > with 16 bits, together with the Read Configuration Register (35h) > instructions are supported. > > Reported-by: Geert Uytterhoeven <geert@linux-m68k.org> > Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com> > --- > Geert, Jonas, > > This patch is compile-tested only. I don't have the flash, I need your > help for testing this.
Thanks, this revives access to the s25fl512s on Koelsch.
Fixes: dcb4b22eeaf44f91 ("spi-nor: s25fl512s supports region locking") Tested-by: Geert Uytterhoeven <geert+renesas@glider.be>Hi Tudor,
Two questions below...
> --- a/drivers/mtd/spi-nor/spi-nor.c > +++ b/drivers/mtd/spi-nor/spi-nor.c
> +static int spi_nor_spansion_clear_sr_bp(struct spi_nor *nor) > +{
[...]
> + * When the configuration register QUAD bit CR[1] is 1, only > + * the WRR command format with 16 data bits may be used.
s/WRR/WRSR/?
> + */ > + if (ret & CR_QUAD_EN_SPAN) { > + sr_cr[1] = ret; > + > + ret = read_sr(nor); > + if (ret < 0) { > + dev_err(nor->dev, > + "error while reading status register\n"); > + return ret; > + } > + sr_cr[0] = ret & ~mask; > + > + ret = write_sr_cr(nor, sr_cr); > + if (ret) > + dev_err(nor->dev, "16-bit write register failed\n"); > + return ret; > + } > + > + /* If quad bit is not set, use 8-bit WRR command. */
Likewise.
> + return spi_nor_clear_sr_bp(nor); > +}
Gr{oetje,eeting}s,
Geert
-- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
| |