Re: [PATCH v4 2/3] spi-nor: s25fl512s supports region locking

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 

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 <jonas@xxxxxxxxxxx> wrote:
>> On 07/05/2019 12:50, Geert Uytterhoeven wrote:
>>> On Tue, May 7, 2019 at 12:42 PM <Tudor.Ambarus@xxxxxxxxxxxxx> wrote:
>>>> On 05/07/2019 12:53 PM, Geert Uytterhoeven wrote:
>>>>> On Wed, Mar 20, 2019 at 8:16 AM Jonas Bonn <jonas@xxxxxxxxxxx> 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 <jonas@xxxxxxxxxxx>
>>>>>
>>>>> 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);
        }
 

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
[Index of Archives]     [LARTC]     [Bugtraq]     [Yosemite Forum]     [Photo]

  Powered by Linux