Re: [PATCH 2/2] mtd: spi-nor: refine Spansion S25FL512S ID

From: Geert Uytterhoeven <geert@linux-m68k.org>
To: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
Cc: Marek Vasut <marek.vasut@gmail.com>,
	Tudor Ambarus <tudor.ambarus@microchip.com>,
	David Woodhouse <dwmw2@infradead.org>,
	Brian Norris <computersforpeace@gmail.com>,
	Boris Brezillon <bbrezillon@kernel.org>,
	Richard Weinberger <richard@nod.at>,
	MTD Maling List <linux-mtd@lists.infradead.org>,
	Linux-Renesas <linux-renesas-soc@vger.kernel.org>
Subject: Re: [PATCH 2/2] mtd: spi-nor: refine Spansion S25FL512S ID
Date: Tue, 5 Mar 2019 15:05:09 +0100	[thread overview]
Message-ID: <CAMuHMdVjvbcK21r9ajp9xKEGx=VHbXkrSB0ob2EjR8+dmNVdPw@mail.gmail.com> (raw)
In-Reply-To: <09a2508e-f63e-4804-bf11-76832faae1f5@cogentembedded.com>

Hi Sergei,

On Wed, Jan 16, 2019 at 6:53 PM Sergei Shtylyov
<sergei.shtylyov@cogentembedded.com> 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@cogentembedded.com>

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.

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.

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@linux-m68k.org

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