Re: [PATCH] mtd: spi-nor: use 16-bit WRR command when QE is set on spansion flashes
Date: Wed Jun 19 2019 - 11:52:30 EST
On 06/11/2019 11:35 AM, Geert Uytterhoeven wrote:
> Hi Tudor,
> On Mon, Jun 10, 2019 at 8:24 AM <Tudor.Ambarus@xxxxxxxxxxxxx> wrote:
>> From: Tudor Ambarus <tudor.ambarus@xxxxxxxxxxxxx>
>> 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 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@xxxxxxxxxxxxxx>
>> Signed-off-by: Tudor Ambarus <tudor.ambarus@xxxxxxxxxxxxx>
>> 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")
I didn't add the Fixes tag because this commit helped us discover a case that
has not been taken into consideration before. It didn't introduce a bug, but
rather revealed one. However, it's not the time to walk over this thin line, so
I'll add it, thanks!
> Tested-by: Geert Uytterhoeven <geert+renesas@xxxxxxxxx>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 is 1, only
>> + * the WRR command format with 16 data bits may be used.
S25FL512S named it "Write Registers" command and chose the "WRR" acronym.
JESD216D names it "Write Register" command and doesn't suggest an acronym. I'll
s/"WRR"/"Write Register command", to use the JESD216D naming and avoid confusion.
I also forgot to describe int (*clear_sr_bp), v2 will follow. Will keep the R-b
and T-b tags since I'll just update comments.