Geert,
On 05/07/2019 03:52 PM, Geert Uytterhoeven wrote:
> External E-Mail
>
>
> Hi Jonas,
>
> On Tue, May 7, 2019 at 1:14 PM Jonas Bonn <[email protected]> wrote:
>> On 07/05/2019 12:50, Geert Uytterhoeven wrote:
>>> On Tue, May 7, 2019 at 12:42 PM <[email protected]> wrote:
>>>> On 05/07/2019 12:53 PM, Geert Uytterhoeven wrote:
>>>>> On Wed, Mar 20, 2019 at 8:16 AM Jonas Bonn <[email protected]> wrote:
>>>>>> Both the BP[0-2] bits and the TBPROT bit are supported on this chip.
>>>>>> Tested and verified on a Cypress s25fl512s.
>>>>>>
>>>>>> Signed-off-by: Jonas Bonn <[email protected]>
>>>>>
>>>>> This is now commit dcb4b22eeaf44f91 ("spi-nor: s25fl512s supports region
>>>>> locking") in mtd/next.
>>>>>
>>>>>> --- a/drivers/mtd/spi-nor/spi-nor.c
>>>>>> +++ b/drivers/mtd/spi-nor/spi-nor.c
>>>>>> @@ -1898,7 +1898,9 @@ static const struct flash_info spi_nor_ids[] = {
>>>>>> SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ |
>>>>>> USE_CLSR) },
>>>>>> { "s25fl256s0", INFO(0x010219, 0x4d00, 256 * 1024, 128,
>>>>>> USE_CLSR) },
>>>>>> { "s25fl256s1", INFO(0x010219, 0x4d01, 64 * 1024, 512,
>>>>>> SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | USE_CLSR) },
>>>>>> - { "s25fl512s", INFO6(0x010220, 0x4d0080, 256 * 1024, 256,
>>>>>> SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | USE_CLSR) },
>>>>>> + { "s25fl512s", INFO6(0x010220, 0x4d0080, 256 * 1024, 256,
>>>>>> + SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ |
>>>>>> + SPI_NOR_HAS_LOCK | SPI_NOR_HAS_TB | USE_CLSR) },
>>>>>
>>>>> Setting SPI_NOR_HAS_LOCK causes the QSPI FLASH on r8a7791/koelsch to fail
>>>>> probing.
>>>>>
>>>>> Before/after:
>>>>>
>>>>> -m25p80 spi0.0: s25fl512s (65536 Kbytes)
>>>>> -3 fixed-partitions partitions found on MTD device spi0.0
>>>>> -Creating 3 MTD partitions on "spi0.0":
>>>>> -0x000000000000-0x000000080000 : "loader"
>>>>> -0x000000080000-0x000000600000 : "user"
>>>>> -0x000000600000-0x000004000000 : "flash"
>>>>> +m25p80 spi0.0: Erase Error occurred
>>>>> +m25p80 spi0.0: Erase Error occurred
>>>>> +m25p80 spi0.0: timeout while writing configuration register
>>>>> +m25p80 spi0.0: quad mode not supported
>>>>> +m25p80: probe of spi0.0 failed with error -5
>>>>>
>>
>> In drivers/mtd/spi-nor/spi-nor.c you have:
>>
>> static int spi_nor_init(struct spi_nor *nor)
>> {
>> int err;
>>
>> /*
>> * Atmel, SST, Intel/Numonyx, and others serial NOR tend to power up
>> * with the software protection bits set
>> */
>> if (JEDEC_MFR(nor->info) == SNOR_MFR_ATMEL ||
>> JEDEC_MFR(nor->info) == SNOR_MFR_INTEL ||
>> JEDEC_MFR(nor->info) == SNOR_MFR_SST ||
>> nor->info->flags & SPI_NOR_HAS_LOCK) {
>> write_enable(nor);
>
> With more debug prints:
>
> m25p80 spi0.0: spi_nor_init:3925: write_enable() returned 0
>
>> write_sr(nor, 0);
>
> m25p80 spi0.0: spi_nor_init:3927: write_sr() returned 0
>
>> spi_nor_wait_till_ready(nor);
>
> m25p80 spi0.0: spi_nor_sr_ready:533: sr = 0x3
> m25p80 spi0.0: spi_nor_sr_ready:533: sr = 0x3
> m25p80 spi0.0: spi_nor_sr_ready:533: sr = 0x3
> ...
> m25p80 spi0.0: spi_nor_sr_ready:533: sr = 0xff
> m25p80 spi0.0: Erase Error occurred
> m25p80 spi0.0: spi_nor_init:3929: spi_nor_wait_till_ready() returned -5
>
>> }
>>
>> if (nor->quad_enable) {
>> err = nor->quad_enable(nor);
>
> m25p80 spi0.0: spi_nor_sr_ready:533: sr = 0xff
> m25p80 spi0.0: Erase Error occurred
> m25p80 spi0.0: timeout while writing configuration register
> m25p80 spi0.0: spi_nor_init:3937: spansion_quad_enable() returned -5
>
>> if (err) {
>> dev_err(nor->dev, "quad mode not supported\n");
>> return err;
>> }
>> }
>>
>> This is the only meaningful thing that I can see may have changed with
>> this flag. We now have an additional write_enable before quad_enable.
>> What happens if you swap those two blocks above so that quad_enable is
>> called first?
>
> With the two blocks swapped:
>
> m25p80 spi0.0: spi_nor_sr_ready:533: sr = 0x0
> m25p80 spi0.0: spi_nor_init:3919: spansion_quad_enable() returned 0
> m25p80 spi0.0: spi_nor_init:3937: write_enable() returned 0
> m25p80 spi0.0: spi_nor_init:3939: write_sr() returned 0
> m25p80 spi0.0: spi_nor_sr_ready:533: sr = 0x3
> m25p80 spi0.0: spi_nor_sr_ready:533: sr = 0x3
> m25p80 spi0.0: spi_nor_sr_ready:533: sr = 0x3
> ...
> m25p80 spi0.0: spi_nor_sr_ready:533: sr = 0xff
> m25p80 spi0.0: Erase Error occurred
> m25p80 spi0.0: spi_nor_init:3941: spi_nor_wait_till_ready() returned -5
> m25p80 spi0.0: s25fl512s (65536 Kbytes)
> 3 fixed-partitions partitions found on MTD device spi0.0
> Creating 3 MTD partitions on "spi0.0":
> 0x000000000000-0x000000080000 : "loader"
> 0x000000080000-0x000000600000 : "user"
> 0x000000600000-0x000004000000 : "flash"
>
> Note that spi_nor_wait_till_ready() still fails.
>
> While the device (which contains the boot loader) now appears to be
> detected fine, reading returns bogus repetitive data, for all partitions:
>
> # hd /dev/mtd0 | head
> 00000000 33 33 33 33 33 33 33 33 33 33 3b bb ff ff ff ff
> |3333333333;.....|
> 00000010 ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
> |................|
> *
> 00001000 33 33 33 33 33 33 33 33 33 33 3b bb ff ff ff ff
> |3333333333;.....|
> 00001010 ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
> |................|
> *
>
> If I remove the call to "write_sr(nor, 0)", everything works as
> before, regardless
> of swapping the blocks.
>
Can you try this one?
diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
index 73172d7f512b..b94a6eaaaca5 100644
--- a/drivers/mtd/spi-nor/spi-nor.c
+++ b/drivers/mtd/spi-nor/spi-nor.c
@@ -1551,6 +1551,7 @@ static int spansion_read_cr_quad_enable(struct spi_nor
*nor)
u8 sr_cr[2];
int ret;
+ dev_err(dev, "%s\n", __FUNCTION__);
/* Check current Quad Enable bit value. */
ret = read_cr(nor);
if (ret < 0) {
@@ -3911,6 +3912,12 @@ static int spi_nor_setup(struct spi_nor *nor,
static int spi_nor_init(struct spi_nor *nor)
{
int err;
+ u8 val;
+ u8 mask = SR_BP2 | SR_BP1 | SR_BP0;
+
+ /* Check current Quad Enable bit value. */
+ val = read_cr(nor);
+ dev_err(nor->dev, "%s val = %02x\n", val);
/*
* Atmel, SST, Intel/Numonyx, and others serial NOR tend to power up
@@ -3921,7 +3928,7 @@ static int spi_nor_init(struct spi_nor *nor)
JEDEC_MFR(nor->info) == SNOR_MFR_SST ||
nor->info->flags & SPI_NOR_HAS_LOCK) {
write_enable(nor);
- write_sr(nor, 0);
+ write_sr(nor, val & ~mask);
spi_nor_wait_till_ready(nor);
}
