linux-kernel - Re: [PATCH] mtd: spi-nor: use 16-bit WRR command when QE is set on spansion flashes

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 11 Jun 2019 10:35:59 +0200
From:   Geert Uytterhoeven <geert@...ux-m68k.org>
To:     Tudor Ambarus <Tudor.Ambarus@...rochip.com>
Cc:     Marek Vasut <marek.vasut+renesas@...il.com>,
        Marek Vasut <marek.vasut@...il.com>,
        "R, Vignesh" <vigneshr@...com>, Jonas Bonn <jonas@...rbonn.se>,
        David Woodhouse <dwmw2@...radead.org>,
        Brian Norris <computersforpeace@...il.com>,
        Miquel Raynal <miquel.raynal@...tlin.com>,
        Richard Weinberger <richard@....at>,
        MTD Maling List <linux-mtd@...ts.infradead.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Linux-Renesas <linux-renesas-soc@...r.kernel.org>
Subject: Re: [PATCH] mtd: spi-nor: use 16-bit WRR command when QE is set on
 spansion flashes

Hi Tudor,

On Mon, Jun 10, 2019 at 8:24 AM <Tudor.Ambarus@...rochip.com> wrote:
> From: Tudor Ambarus <tudor.ambarus@...rochip.com>
>
> SPI memory devices from different manufacturers have widely
> different configurations for Status, Control and Configuration
> registers. JEDEC 216C defines a new map for these common register
> bits and their functions, and describes how the individual bits may
> be accessed for a specific device. For the JEDEC 216B compliant
> flashes, we can partially deduce Status and Configuration registers
> functions by inspecting the 16th DWORD of BFPT. Older flashes that
> don't declare the SFDP tables (SPANSION FL512SAIFG1 311QQ063 A ©11
> SPANSION) let the software decide how to interact with these registers.
>
> The commit dcb4b22eeaf4 ("spi-nor: s25fl512s supports region locking")
> uncovered a probe error for s25fl512s, when the QUAD bit CR[1] was set
> in the bootloader. When this bit is set, only the Write Register
> WRR command format with 16 data bits may be used, WRR with 8 bits
> is not recognized and hence the error when trying to clear the block
> protection bits.
>
> Fix the above by using 16-bits WRR command when Quad bit is set.
>
> Backward compatibility should be fine. The newly introduced
> spi_nor_spansion_clear_sr_bp() is tightly coupled with the
> spansion_quad_enable() function. Both assume that the Write Register
> with 16 bits, together with the Read Configuration Register (35h)
> instructions are supported.
>
> Reported-by: Geert Uytterhoeven <geert@...ux-m68k.org>
> Signed-off-by: Tudor Ambarus <tudor.ambarus@...rochip.com>
> ---
> Geert, Jonas,
>
> This patch is compile-tested only. I don't have the flash, I need your
> help for testing this.

Thanks, this revives access to the s25fl512s on Koelsch.

Fixes: dcb4b22eeaf44f91 ("spi-nor: s25fl512s supports region locking")
Tested-by: Geert Uytterhoeven <geert+renesas@...der.be>Hi Tudor,

Two questions below...

> --- a/drivers/mtd/spi-nor/spi-nor.c
> +++ b/drivers/mtd/spi-nor/spi-nor.c

> +static int spi_nor_spansion_clear_sr_bp(struct spi_nor *nor)
> +{

[...]

> +        * When the configuration register QUAD bit CR[1] is 1, only
> +        * the WRR command format with 16 data bits may be used.

s/WRR/WRSR/?

> +        */
> +       if (ret & CR_QUAD_EN_SPAN) {
> +               sr_cr[1] = ret;
> +
> +               ret = read_sr(nor);
> +               if (ret < 0) {
> +                       dev_err(nor->dev,
> +                               "error while reading status register\n");
> +                       return ret;
> +               }
> +               sr_cr[0] = ret & ~mask;
> +
> +               ret = write_sr_cr(nor, sr_cr);
> +               if (ret)
> +                       dev_err(nor->dev, "16-bit write register failed\n");
> +               return ret;
> +       }
> +
> +       /* If quad bit is not set, use 8-bit WRR command. */

Likewise.

> +       return spi_nor_clear_sr_bp(nor);
> +}

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@...ux-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