On 05/07/2019 04:15 PM, Tudor Ambarus wrote:
> 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);
dev_err(nor->dev, "%s val = %02x\n", __FUNCTION__, 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);
> }
>
>
