linux-kernel - Re: [PATCH RFC] mtd: spi-nor: unset quad

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: Windows password security audit tool. GUI, reports in PDF.

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

Date:   Fri, 4 Mar 2022 07:14:34 +0000
From:   <Tudor.Ambarus@...rochip.com>
To:     <michael@...le.cc>, <p.yadav@...com>, <miquel.raynal@...tlin.com>,
        <richard@....at>, <vigneshr@...com>
CC:     <linux-mtd@...ts.infradead.org>, <linux-kernel@...r.kernel.org>,
        <heiko.thiery@...il.com>
Subject: Re: [PATCH RFC] mtd: spi-nor: unset quad_enable if SFDP doesn't
 specify it

On 3/1/22 14:49, Michael Walle wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> For flashes which use the first JESD216 revision, we don't know
> which enable method for quad mode we can use. The default one in
> spi_nor_init_default_params() is wrong for at least the Macronix
> MX25L12835F, thus clear the enable method. Flashes with such an
> SFDP revision will have to use a flash (and SFDP revision)
> specific fixup.

I expect this to break support for flashes that use the quad_enable
method selected in spi_nor_init_default_params() and define the first
JESD216 revision, but we should fix all sooner or later. I vote to queue
this in. Maybe you can update the commit message and explain why would some
flashes fail to enable quad mode, similar to what I did.

Cheers,
ta
> 
> Signed-off-by: Michael Walle <michael@...le.cc>
> ---
> Please note, completely, untested. Heiko, could you test this without
> having the second series from Tudor applied? Then you should have at
> least a working flash without quad mode.
> 
>  drivers/mtd/spi-nor/sfdp.c | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/mtd/spi-nor/sfdp.c b/drivers/mtd/spi-nor/sfdp.c
> index a5211543d30d..c23e85274ff2 100644
> --- a/drivers/mtd/spi-nor/sfdp.c
> +++ b/drivers/mtd/spi-nor/sfdp.c
> @@ -549,6 +549,14 @@ static int spi_nor_parse_bfpt(struct spi_nor *nor,
>         map->uniform_erase_type = map->uniform_region.offset &
>                                   SNOR_ERASE_TYPE_MASK;
> 
> +       /*
> +        * The first JESD216 revision doesn't specify a method to enable
> +        * quad mode. spi_nor_init_default_params() will set a legacy
> +        * default method to enable quad mode. We have to disable it
> +        * again.

and here to update the comment and explain that you expect these flashes to
set the quad enable method in a post_bfpt hook.

> +        */
> +       params->quad_enable = NULL;
> +
>         /* Stop here if not JESD216 rev A or later. */
>         if (bfpt_header->length == BFPT_DWORD_MAX_JESD216)
>                 return spi_nor_post_bfpt_fixups(nor, bfpt_header, &bfpt);
> @@ -562,7 +570,6 @@ static int spi_nor_parse_bfpt(struct spi_nor *nor,
>         /* Quad Enable Requirements. */
>         switch (bfpt.dwords[BFPT_DWORD(15)] & BFPT_DWORD15_QER_MASK) {
>         case BFPT_DWORD15_QER_NONE:
> -               params->quad_enable = NULL;
>                 break;
> 
>         case BFPT_DWORD15_QER_SR2_BIT1_BUGGY:
> --
> 2.30.2
>