Hi, Geert,
On 03/05/2019 04:05 PM, Geert Uytterhoeven wrote:
> Hi Sergei,
>
> On Wed, Jan 16, 2019 at 6:53 PM Sergei Shtylyov
> <sergei.shtylyov@xxxxxxxxxxxxxxxxxx> wrote:
>> Spansion S25FL512S ID is erroneously using 5-byte JEDEC ID, while the chip
>> family ID is stored in the 6th byte. Due to using only 5-byte ID, it's also
>> covering S25FS512S and now that we have added 6-byte ID for that chip, we
>> can convert S25FL512S to using a proper 6-byte ID as well...
>>
>> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@xxxxxxxxxxxxxxxxxx>
>
> This is now commit a2126b0a010905e5 ("mtd: spi-nor: refine Spansion
> S25FL512S ID"), and turns out to cause a regression on r8a7791/koelsch.
> Dmesg diff 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: unrecognized JEDEC id bytes: 01, 02, 20
>
> As the (old) U-Boot on my Koelsch keeps many module clocks enabled, I
> typically merge in my topic/renesas-debug branch, which makes sure all
> non-critical module clocks are disabled early during boot, to catch
> drivers not properly implementing Runtime PM.
>
> However, it turns out this has some impact on JEDEC ID detection:
> - When module clocks are left untouched, spi_nor_read_id() reads
> 0x01:0x02:0x20:0x4d:0x00:0x80.
> - When my debug code has disabled module clocks during early boot,
> The last byte is 0x00.
>
> Before the above commit, only the first 5 bytes were compared, and the
> last byte was ignored, thus not causing problems.
> When comparing all 6 bytes, detection fails if the last byte is 0x00.
>
We'll have to understand why just the 6th byte is different.
> I believe mainline U-Boot for R-Car Gen2 boards doesn't keep the QSPI
> module clock enabled, so this commit may breaks those boards.
Who provides the flash's input clock and at what frequency?
Cheers,
ta
>
> To be investigated more (e.g. with a logic analyzer)...
>
>> --- linux-mtd.orig/drivers/mtd/spi-nor/spi-nor.c
>> +++ linux-mtd/drivers/mtd/spi-nor/spi-nor.c
>> @@ -1887,8 +1887,8 @@ static const struct flash_info spi_nor_i
>> { "s25sl064p", INFO(0x010216, 0x4d00, 64 * 1024, 128, SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
>> { "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) },
>> { "s25fs512s", INFO6(0x010220, 0x4d0081, 256 * 1024, 256, SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | USE_CLSR) },
>> - { "s25fl512s", INFO(0x010220, 0x4d00, 256 * 1024, 256, SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | USE_CLSR) },
>> { "s70fl01gs", INFO(0x010221, 0x4d00, 256 * 1024, 256, 0) },
>> { "s25sl12800", INFO(0x012018, 0x0300, 256 * 1024, 64, 0) },
>> { "s25sl12801", INFO(0x012018, 0x0301, 64 * 1024, 256, 0) },
>
> Gr{oetje,eeting}s,
>
> Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx
>
> 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
>
