diff mbox series

[DEBUG] mtd: spi-nor: dump DWORDs of the Basic Flash Parameter Table

Message ID 20170907185456.4631-1-cyrille.pitchen@wedev4u.fr
State Not Applicable
Headers show
Series [DEBUG] mtd: spi-nor: dump DWORDs of the Basic Flash Parameter Table | expand

Commit Message

Cyrille Pitchen Sept. 7, 2017, 6:54 p.m. UTC
debug purpose only, should not be merged!

Signed-off-by: Cyrille Pitchen <cyrille.pitchen@wedev4u.fr>
---

Hi Geert,

Can you apply this patch on your tree then report me what was printed, please?
I have an idea of the root cause of your issue then a potential work-around
but I first need to validate my assumption to confirm that the work-around
would actually work.

For instance, here is what I get with a Macronix MX25L25673G (same JEDEC ID as
MX25L25635E):

[    0.700000] atmel_qspi f0020000.spi: DWORD1 = 0xfffb20e5
[    0.710000] atmel_qspi f0020000.spi: DWORD2 = 0x0fffffff
[    0.710000] atmel_qspi f0020000.spi: DWORD3 = 0x6b08eb44
[    0.720000] atmel_qspi f0020000.spi: DWORD4 = 0xbb043b08
[    0.720000] atmel_qspi f0020000.spi: DWORD5 = 0xfffffffe
[    0.720000] atmel_qspi f0020000.spi: DWORD6 = 0xff00ffff
[    0.730000] atmel_qspi f0020000.spi: DWORD7 = 0xeb44ffff
[    0.730000] atmel_qspi f0020000.spi: DWORD8 = 0x520f200c
[    0.740000] atmel_qspi f0020000.spi: DWORD9 = 0xff00d810
[    0.740000] atmel_qspi f0020000.spi: DWORD10 = 0x00dd59d6
[    0.740000] atmel_qspi f0020000.spi: DWORD11 = 0xdb039f82
[    0.750000] atmel_qspi f0020000.spi: DWORD12 = 0x38670344
[    0.750000] atmel_qspi f0020000.spi: DWORD13 = 0xb030b030
[    0.760000] atmel_qspi f0020000.spi: DWORD14 = 0x5cd5bdf7
[    0.760000] atmel_qspi f0020000.spi: DWORD15 = 0xff299e4a
[    0.760000] atmel_qspi f0020000.spi: DWORD16 = 0x85f950f0
[    0.770000] atmel_qspi f0020000.spi: BFPT version 1.6 (length = 16)
[    0.770000] atmel_qspi f0020000.spi: mx25l25635e (32768 Kbytes)

Best regards,

Cyrille

 drivers/mtd/spi-nor/spi-nor.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

Comments

Cyrille Pitchen Sept. 7, 2017, 7:28 p.m. UTC | #1
Hi again,

Le 07/09/2017 à 20:54, Cyrille Pitchen a écrit :
> debug purpose only, should not be merged!
> 
> Signed-off-by: Cyrille Pitchen <cyrille.pitchen@wedev4u.fr>
> ---
> 
> Hi Geert,
> 
> Can you apply this patch on your tree then report me what was printed, please?
> I have an idea of the root cause of your issue then a potential work-around
> but I first need to validate my assumption to confirm that the work-around
> would actually work.
>

If you could also dump the value of the 'addr' argument of
spi_nor_read_sfdp_dma_unsafe() just before the for () loop below in the
very same function. Actually, I suspect the SFDP tables of your SPI NOR
memory sample to have been programmed with invalid values, neither
compliant with the JEDEC JESD216 specification nor with the Cypress
datasheet for this memory part.

> For instance, here is what I get with a Macronix MX25L25673G (same JEDEC ID as
> MX25L25635E):
> 
> [    0.700000] atmel_qspi f0020000.spi: DWORD1 = 0xfffb20e5
> [    0.710000] atmel_qspi f0020000.spi: DWORD2 = 0x0fffffff
> [    0.710000] atmel_qspi f0020000.spi: DWORD3 = 0x6b08eb44
> [    0.720000] atmel_qspi f0020000.spi: DWORD4 = 0xbb043b08
> [    0.720000] atmel_qspi f0020000.spi: DWORD5 = 0xfffffffe
> [    0.720000] atmel_qspi f0020000.spi: DWORD6 = 0xff00ffff
> [    0.730000] atmel_qspi f0020000.spi: DWORD7 = 0xeb44ffff
> [    0.730000] atmel_qspi f0020000.spi: DWORD8 = 0x520f200c
> [    0.740000] atmel_qspi f0020000.spi: DWORD9 = 0xff00d810
> [    0.740000] atmel_qspi f0020000.spi: DWORD10 = 0x00dd59d6
> [    0.740000] atmel_qspi f0020000.spi: DWORD11 = 0xdb039f82
> [    0.750000] atmel_qspi f0020000.spi: DWORD12 = 0x38670344
> [    0.750000] atmel_qspi f0020000.spi: DWORD13 = 0xb030b030
> [    0.760000] atmel_qspi f0020000.spi: DWORD14 = 0x5cd5bdf7
> [    0.760000] atmel_qspi f0020000.spi: DWORD15 = 0xff299e4a
> [    0.760000] atmel_qspi f0020000.spi: DWORD16 = 0x85f950f0
> [    0.770000] atmel_qspi f0020000.spi: BFPT version 1.6 (length = 16)
> [    0.770000] atmel_qspi f0020000.spi: mx25l25635e (32768 Kbytes)
> 
> Best regards,
> 
> Cyrille
> 
>  drivers/mtd/spi-nor/spi-nor.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
> index 05254dd6a4a0..5066d99b9f50 100644
> --- a/drivers/mtd/spi-nor/spi-nor.c
> +++ b/drivers/mtd/spi-nor/spi-nor.c
> @@ -2136,8 +2136,14 @@ static int spi_nor_parse_bfpt(struct spi_nor *nor,
>  		return err;
>  
>  	/* Fix endianness of the BFPT DWORDs. */
> -	for (i = 0; i < BFPT_DWORD_MAX; i++)
> +	for (i = 0; i < BFPT_DWORD_MAX; i++) {
>  		bfpt.dwords[i] = le32_to_cpu(bfpt.dwords[i]);
> +		dev_info(nor->dev, "DWORD%d = 0x%08x\n", i + 1, bfpt.dwords[i]);
> +	}
> +	dev_info(nor->dev, "BFPT version %d.%d (length = %u)\n",
> +		 bfpt_header->major,
> +		 bfpt_header->minor,
> +		 bfpt_header->length);
>  
>  	/* Number of address bytes. */
>  	switch (bfpt.dwords[BFPT_DWORD(1)] & BFPT_DWORD1_ADDRESS_BYTES_MASK) {
>
Geert Uytterhoeven Sept. 11, 2017, 8:58 a.m. UTC | #2
Hi Cyrille,

On Thu, Sep 7, 2017 at 9:28 PM, Cyrille Pitchen
<cyrille.pitchen@wedev4u.fr> wrote:
>> Can you apply this patch on your tree then report me what was printed, please?
>> I have an idea of the root cause of your issue then a potential work-around
>> but I first need to validate my assumption to confirm that the work-around
>> would actually work.

+m25p80 spi0.0: DWORD1 = 0xffffffff
+m25p80 spi0.0: DWORD2 = 0xffffffff
+m25p80 spi0.0: DWORD3 = 0xffffffff
+m25p80 spi0.0: DWORD4 = 0xffffffff
+m25p80 spi0.0: DWORD5 = 0xffffffff
+m25p80 spi0.0: DWORD6 = 0xffffffff
+m25p80 spi0.0: DWORD7 = 0xffffffff
+m25p80 spi0.0: DWORD8 = 0xffffffff
+m25p80 spi0.0: DWORD9 = 0xffffffff
+m25p80 spi0.0: DWORD10 = 0x00000000
+m25p80 spi0.0: DWORD11 = 0x00000000
+m25p80 spi0.0: DWORD12 = 0x00000000
+m25p80 spi0.0: DWORD13 = 0x00000000
+m25p80 spi0.0: DWORD14 = 0x00000000
+m25p80 spi0.0: DWORD15 = 0x00000000
+m25p80 spi0.0: DWORD16 = 0x00000000
+m25p80 spi0.0: BFPT version 1.0 (length = 9)

> If you could also dump the value of the 'addr' argument of
> spi_nor_read_sfdp_dma_unsafe() just before the for () loop below in the
> very same function. Actually, I suspect the SFDP tables of your SPI NOR

+m25p80 spi0.0: addr = 0x448

> memory sample to have been programmed with invalid values, neither
> compliant with the JEDEC JESD216 specification nor with the Cypress
> datasheet for this memory part.

Sounds plausible.
I get the same values when disabling DMA, so it's not due to bad DMA handling.
All Renesas boards I have local or remote access to have spansion,s25fl512s.

Thanks!

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
Boris Brezillon Sept. 12, 2017, 1:12 p.m. UTC | #3
Hi Geert,

On Mon, 11 Sep 2017 10:58:36 +0200
Geert Uytterhoeven <geert@linux-m68k.org> wrote:

> Hi Cyrille,
> 
> On Thu, Sep 7, 2017 at 9:28 PM, Cyrille Pitchen
> <cyrille.pitchen@wedev4u.fr> wrote:
> >> Can you apply this patch on your tree then report me what was printed, please?
> >> I have an idea of the root cause of your issue then a potential work-around
> >> but I first need to validate my assumption to confirm that the work-around
> >> would actually work.  
> 
> +m25p80 spi0.0: DWORD1 = 0xffffffff
> +m25p80 spi0.0: DWORD2 = 0xffffffff
> +m25p80 spi0.0: DWORD3 = 0xffffffff
> +m25p80 spi0.0: DWORD4 = 0xffffffff
> +m25p80 spi0.0: DWORD5 = 0xffffffff
> +m25p80 spi0.0: DWORD6 = 0xffffffff
> +m25p80 spi0.0: DWORD7 = 0xffffffff
> +m25p80 spi0.0: DWORD8 = 0xffffffff
> +m25p80 spi0.0: DWORD9 = 0xffffffff
> +m25p80 spi0.0: DWORD10 = 0x00000000
> +m25p80 spi0.0: DWORD11 = 0x00000000
> +m25p80 spi0.0: DWORD12 = 0x00000000
> +m25p80 spi0.0: DWORD13 = 0x00000000
> +m25p80 spi0.0: DWORD14 = 0x00000000
> +m25p80 spi0.0: DWORD15 = 0x00000000
> +m25p80 spi0.0: DWORD16 = 0x00000000
> +m25p80 spi0.0: BFPT version 1.0 (length = 9)
> 
> > If you could also dump the value of the 'addr' argument of
> > spi_nor_read_sfdp_dma_unsafe() just before the for () loop below in the
> > very same function. Actually, I suspect the SFDP tables of your SPI NOR  
> 
> +m25p80 spi0.0: addr = 0x448
> 
> > memory sample to have been programmed with invalid values, neither
> > compliant with the JEDEC JESD216 specification nor with the Cypress
> > datasheet for this memory part.  
> 
> Sounds plausible.
> I get the same values when disabling DMA, so it's not due to bad DMA handling.
> All Renesas boards I have local or remote access to have spansion,s25fl512s.

Can you try with the following patch?

Thanks,

Boris

--->8---
From 000ff63fdb149d87d755483f5edc0aba010da6b4 Mon Sep 17 00:00:00 2001
From: Boris Brezillon <boris.brezillon@free-electrons.com>
Date: Tue, 12 Sep 2017 15:10:35 +0200
Subject: [PATCH] mtd: spi-nor: Check consistency of the memory size extracted
 from the SFDP

One field of the flash parameter table contains information about the
flash device size.
Most of the time the data extracted from this field is valid, but
sometimes the BFPT section of the SFDP table is corrupted or invalid and
this field is set to 0xffffffff, thus resulting in an integer overflow
when setting params->size.

Since NOR devices are anayway always smaller than 2^64 bytes, we can
easily stop the BFPT parsing if the size reported in this table is
invalid.

Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com>
---
 drivers/mtd/spi-nor/spi-nor.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
index cf1d4a15e10a..665ccae1d090 100644
--- a/drivers/mtd/spi-nor/spi-nor.c
+++ b/drivers/mtd/spi-nor/spi-nor.c
@@ -2127,6 +2127,15 @@ static int spi_nor_parse_bfpt(struct spi_nor *nor,
 	params->size = bfpt.dwords[BFPT_DWORD(2)];
 	if (params->size & BIT(31)) {
 		params->size &= ~BIT(31);
+
+		/*
+		 * Prevent overflows on params->size. Anyway, a NOR of 1^64
+		 * bytes is unlikely to exist so this error probably means
+		 * the BFPT we are reading is corrupted/wrong.
+		 */
+		if (params->size > 63)
+			return -EINVAL;
+
 		params->size = 1ULL << params->size;
 	} else {
 		params->size++;
Boris Brezillon Sept. 13, 2017, 1:33 p.m. UTC | #4
Adding back original recipients (you seem to have trimmed the Cc-list).

Hi Geert,

On Tue, 12 Sep 2017 15:29:23 +0200
Geert Uytterhoeven <geert@linux-m68k.org> wrote:

> Hi Boris,
> 
> On Tue, Sep 12, 2017 at 3:12 PM, Boris Brezillon
> <boris.brezillon@free-electrons.com> wrote:
> > On Mon, 11 Sep 2017 10:58:36 +0200
> > Geert Uytterhoeven <geert@linux-m68k.org> wrote:  
> >> On Thu, Sep 7, 2017 at 9:28 PM, Cyrille Pitchen
> >> <cyrille.pitchen@wedev4u.fr> wrote:  
> >> >> Can you apply this patch on your tree then report me what was printed, please?
> >> >> I have an idea of the root cause of your issue then a potential work-around
> >> >> but I first need to validate my assumption to confirm that the work-around
> >> >> would actually work.  
> >>
> >> +m25p80 spi0.0: DWORD1 = 0xffffffff
> >> +m25p80 spi0.0: DWORD2 = 0xffffffff
> >> +m25p80 spi0.0: DWORD3 = 0xffffffff
> >> +m25p80 spi0.0: DWORD4 = 0xffffffff
> >> +m25p80 spi0.0: DWORD5 = 0xffffffff
> >> +m25p80 spi0.0: DWORD6 = 0xffffffff
> >> +m25p80 spi0.0: DWORD7 = 0xffffffff
> >> +m25p80 spi0.0: DWORD8 = 0xffffffff
> >> +m25p80 spi0.0: DWORD9 = 0xffffffff
> >> +m25p80 spi0.0: DWORD10 = 0x00000000
> >> +m25p80 spi0.0: DWORD11 = 0x00000000
> >> +m25p80 spi0.0: DWORD12 = 0x00000000
> >> +m25p80 spi0.0: DWORD13 = 0x00000000
> >> +m25p80 spi0.0: DWORD14 = 0x00000000
> >> +m25p80 spi0.0: DWORD15 = 0x00000000
> >> +m25p80 spi0.0: DWORD16 = 0x00000000
> >> +m25p80 spi0.0: BFPT version 1.0 (length = 9)
> >>  
> >> > If you could also dump the value of the 'addr' argument of
> >> > spi_nor_read_sfdp_dma_unsafe() just before the for () loop below in the
> >> > very same function. Actually, I suspect the SFDP tables of your SPI NOR  
> >>
> >> +m25p80 spi0.0: addr = 0x448
> >>  
> >> > memory sample to have been programmed with invalid values, neither
> >> > compliant with the JEDEC JESD216 specification nor with the Cypress
> >> > datasheet for this memory part.  
> >>
> >> Sounds plausible.
> >> I get the same values when disabling DMA, so it's not due to bad DMA handling.
> >> All Renesas boards I have local or remote access to have spansion,s25fl512s.  
> >
> > Can you try with the following patch?  
> 
> Thanks, that fixes it:
> 
> -m25p80 spi0.0: s25fl512s (0 Kbytes)
> +m25p80 spi0.0: s25fl512s (65536 Kbytes)
>  3 ofpart partitions found on MTD device spi0.0
>  Creating 3 MTD partitions on "spi0.0":
>  0x000000000000-0x000000080000 : "loader"
> -mtd: partition "loader" is out of reach -- disabled
>  0x000000080000-0x000000600000 : "user"
> -mtd: partition "user" is out of reach -- disabled
>  0x000000600000-0x000004000000 : "flash"
> -mtd: partition "flash" is out of reach -- disabled
> 
> BTW, perhaps the driver should print a warning, so the user knows his
> FLASH isn't SFDP compliant?

Yep, but I'll let Cyrille fix this aspect in a follow-up patch since
I'm not sure how specific the error message should be (a generic
"failed to read/decode SFDP" error or something more specific for each
error case).

> 
> > --->8---  
> > From 000ff63fdb149d87d755483f5edc0aba010da6b4 Mon Sep 17 00:00:00 2001
> > From: Boris Brezillon <boris.brezillon@free-electrons.com>
> > Date: Tue, 12 Sep 2017 15:10:35 +0200
> > Subject: [PATCH] mtd: spi-nor: Check consistency of the memory size extracted
> >  from the SFDP
> >
> > One field of the flash parameter table contains information about the
> > flash device size.
> > Most of the time the data extracted from this field is valid, but
> > sometimes the BFPT section of the SFDP table is corrupted or invalid and
> > this field is set to 0xffffffff, thus resulting in an integer overflow
> > when setting params->size.
> >
> > Since NOR devices are anayway always smaller than 2^64 bytes, we can
> > easily stop the BFPT parsing if the size reported in this table is
> > invalid.
> >
> > Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com>  
> 
> Tested-by: Geert Uytterhoeven <geert+renesas@glider.be>

I'm just waiting for a review from Cyrille or Marek and if they are okay
I'll queue both patches to l2-mtd and send a PR to Linus (hopefully
before -rc1 is out).

Thanks for your help with this bug.

Regards,

Boris
Cyrille Pitchen Sept. 14, 2017, 4:44 p.m. UTC | #5
Le 12/09/2017 à 15:12, Boris Brezillon a écrit :
> Hi Geert,
> 
> On Mon, 11 Sep 2017 10:58:36 +0200
> Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> 
>> Hi Cyrille,
>>
>> On Thu, Sep 7, 2017 at 9:28 PM, Cyrille Pitchen
>> <cyrille.pitchen@wedev4u.fr> wrote:
>>>> Can you apply this patch on your tree then report me what was printed, please?
>>>> I have an idea of the root cause of your issue then a potential work-around
>>>> but I first need to validate my assumption to confirm that the work-around
>>>> would actually work.  
>>
>> +m25p80 spi0.0: DWORD1 = 0xffffffff
>> +m25p80 spi0.0: DWORD2 = 0xffffffff
>> +m25p80 spi0.0: DWORD3 = 0xffffffff
>> +m25p80 spi0.0: DWORD4 = 0xffffffff
>> +m25p80 spi0.0: DWORD5 = 0xffffffff
>> +m25p80 spi0.0: DWORD6 = 0xffffffff
>> +m25p80 spi0.0: DWORD7 = 0xffffffff
>> +m25p80 spi0.0: DWORD8 = 0xffffffff
>> +m25p80 spi0.0: DWORD9 = 0xffffffff
>> +m25p80 spi0.0: DWORD10 = 0x00000000
>> +m25p80 spi0.0: DWORD11 = 0x00000000
>> +m25p80 spi0.0: DWORD12 = 0x00000000
>> +m25p80 spi0.0: DWORD13 = 0x00000000
>> +m25p80 spi0.0: DWORD14 = 0x00000000
>> +m25p80 spi0.0: DWORD15 = 0x00000000
>> +m25p80 spi0.0: DWORD16 = 0x00000000
>> +m25p80 spi0.0: BFPT version 1.0 (length = 9)
>>
>>> If you could also dump the value of the 'addr' argument of
>>> spi_nor_read_sfdp_dma_unsafe() just before the for () loop below in the
>>> very same function. Actually, I suspect the SFDP tables of your SPI NOR  
>>
>> +m25p80 spi0.0: addr = 0x448
>>
>>> memory sample to have been programmed with invalid values, neither
>>> compliant with the JEDEC JESD216 specification nor with the Cypress
>>> datasheet for this memory part.  
>>
>> Sounds plausible.
>> I get the same values when disabling DMA, so it's not due to bad DMA handling.
>> All Renesas boards I have local or remote access to have spansion,s25fl512s.
> 
> Can you try with the following patch?
> 
> Thanks,
> 
> Boris
> 
> --->8---
> From 000ff63fdb149d87d755483f5edc0aba010da6b4 Mon Sep 17 00:00:00 2001
> From: Boris Brezillon <boris.brezillon@free-electrons.com>
> Date: Tue, 12 Sep 2017 15:10:35 +0200
> Subject: [PATCH] mtd: spi-nor: Check consistency of the memory size extracted
>  from the SFDP
> 
> One field of the flash parameter table contains information about the
> flash device size.
> Most of the time the data extracted from this field is valid, but
> sometimes the BFPT section of the SFDP table is corrupted or invalid and
> this field is set to 0xffffffff, thus resulting in an integer overflow
> when setting params->size.
> 
> Since NOR devices are anayway always smaller than 2^64 bytes, we can
> easily stop the BFPT parsing if the size reported in this table is
> invalid.
> 
> Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com>
Acked-by: Cyrille Pitchen <cyrille.pitchen@wedev4u.com>

with few comments below:
> ---
>  drivers/mtd/spi-nor/spi-nor.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
> index cf1d4a15e10a..665ccae1d090 100644
> --- a/drivers/mtd/spi-nor/spi-nor.c
> +++ b/drivers/mtd/spi-nor/spi-nor.c
> @@ -2127,6 +2127,15 @@ static int spi_nor_parse_bfpt(struct spi_nor *nor,
>  	params->size = bfpt.dwords[BFPT_DWORD(2)];
>  	if (params->size & BIT(31)) {
>  		params->size &= ~BIT(31);
> +
> +		/*
> +		 * Prevent overflows on params->size. Anyway, a NOR of 1^64
typo: should be 2^64
> +		 * bytes is unlikely to exist so this error probably means

Here the size is still expressed in bits, not yet in byte, the
conversion is done right after this chunk.
> +		 * the BFPT we are reading is corrupted/wrong.
> +		 */
> +		if (params->size > 63)
> +			return -EINVAL;
> +
>  		params->size = 1ULL << params->size;
>  	} else {
>  		params->size++;
>
Boris Brezillon Sept. 19, 2017, 8:11 p.m. UTC | #6
On Thu, 14 Sep 2017 18:44:31 +0200
Cyrille Pitchen <cyrille.pitchen@wedev4u.fr> wrote:

> Le 12/09/2017 à 15:12, Boris Brezillon a écrit :
> > Hi Geert,
> > 
> > On Mon, 11 Sep 2017 10:58:36 +0200
> > Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> >   
> >> Hi Cyrille,
> >>
> >> On Thu, Sep 7, 2017 at 9:28 PM, Cyrille Pitchen
> >> <cyrille.pitchen@wedev4u.fr> wrote:  
> >>>> Can you apply this patch on your tree then report me what was printed, please?
> >>>> I have an idea of the root cause of your issue then a potential work-around
> >>>> but I first need to validate my assumption to confirm that the work-around
> >>>> would actually work.    
> >>
> >> +m25p80 spi0.0: DWORD1 = 0xffffffff
> >> +m25p80 spi0.0: DWORD2 = 0xffffffff
> >> +m25p80 spi0.0: DWORD3 = 0xffffffff
> >> +m25p80 spi0.0: DWORD4 = 0xffffffff
> >> +m25p80 spi0.0: DWORD5 = 0xffffffff
> >> +m25p80 spi0.0: DWORD6 = 0xffffffff
> >> +m25p80 spi0.0: DWORD7 = 0xffffffff
> >> +m25p80 spi0.0: DWORD8 = 0xffffffff
> >> +m25p80 spi0.0: DWORD9 = 0xffffffff
> >> +m25p80 spi0.0: DWORD10 = 0x00000000
> >> +m25p80 spi0.0: DWORD11 = 0x00000000
> >> +m25p80 spi0.0: DWORD12 = 0x00000000
> >> +m25p80 spi0.0: DWORD13 = 0x00000000
> >> +m25p80 spi0.0: DWORD14 = 0x00000000
> >> +m25p80 spi0.0: DWORD15 = 0x00000000
> >> +m25p80 spi0.0: DWORD16 = 0x00000000
> >> +m25p80 spi0.0: BFPT version 1.0 (length = 9)
> >>  
> >>> If you could also dump the value of the 'addr' argument of
> >>> spi_nor_read_sfdp_dma_unsafe() just before the for () loop below in the
> >>> very same function. Actually, I suspect the SFDP tables of your SPI NOR    
> >>
> >> +m25p80 spi0.0: addr = 0x448
> >>  
> >>> memory sample to have been programmed with invalid values, neither
> >>> compliant with the JEDEC JESD216 specification nor with the Cypress
> >>> datasheet for this memory part.    
> >>
> >> Sounds plausible.
> >> I get the same values when disabling DMA, so it's not due to bad DMA handling.
> >> All Renesas boards I have local or remote access to have spansion,s25fl512s.  
> > 
> > Can you try with the following patch?
> > 
> > Thanks,
> > 
> > Boris
> >   
> > --->8---  
> > From 000ff63fdb149d87d755483f5edc0aba010da6b4 Mon Sep 17 00:00:00 2001
> > From: Boris Brezillon <boris.brezillon@free-electrons.com>
> > Date: Tue, 12 Sep 2017 15:10:35 +0200
> > Subject: [PATCH] mtd: spi-nor: Check consistency of the memory size extracted
> >  from the SFDP
> > 
> > One field of the flash parameter table contains information about the
> > flash device size.
> > Most of the time the data extracted from this field is valid, but
> > sometimes the BFPT section of the SFDP table is corrupted or invalid and
> > this field is set to 0xffffffff, thus resulting in an integer overflow
> > when setting params->size.
> > 
> > Since NOR devices are anayway always smaller than 2^64 bytes, we can
> > easily stop the BFPT parsing if the size reported in this table is
> > invalid.
> > 
> > Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com>  
> Acked-by: Cyrille Pitchen <cyrille.pitchen@wedev4u.com>

Applied after fixing the things you pointed below.

> 
> with few comments below:
> > ---
> >  drivers/mtd/spi-nor/spi-nor.c | 9 +++++++++
> >  1 file changed, 9 insertions(+)
> > 
> > diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
> > index cf1d4a15e10a..665ccae1d090 100644
> > --- a/drivers/mtd/spi-nor/spi-nor.c
> > +++ b/drivers/mtd/spi-nor/spi-nor.c
> > @@ -2127,6 +2127,15 @@ static int spi_nor_parse_bfpt(struct spi_nor *nor,
> >  	params->size = bfpt.dwords[BFPT_DWORD(2)];
> >  	if (params->size & BIT(31)) {
> >  		params->size &= ~BIT(31);
> > +
> > +		/*
> > +		 * Prevent overflows on params->size. Anyway, a NOR of 1^64  
> typo: should be 2^64
> > +		 * bytes is unlikely to exist so this error probably means  
> 
> Here the size is still expressed in bits, not yet in byte, the
> conversion is done right after this chunk.
> > +		 * the BFPT we are reading is corrupted/wrong.
> > +		 */
> > +		if (params->size > 63)
> > +			return -EINVAL;
> > +
> >  		params->size = 1ULL << params->size;
> >  	} else {
> >  		params->size++;
> >   
> 
> 
> ______________________________________________________
> Linux MTD discussion mailing list
> http://lists.infradead.org/mailman/listinfo/linux-mtd/
diff mbox series

Patch

diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
index 05254dd6a4a0..5066d99b9f50 100644
--- a/drivers/mtd/spi-nor/spi-nor.c
+++ b/drivers/mtd/spi-nor/spi-nor.c
@@ -2136,8 +2136,14 @@  static int spi_nor_parse_bfpt(struct spi_nor *nor,
 		return err;
 
 	/* Fix endianness of the BFPT DWORDs. */
-	for (i = 0; i < BFPT_DWORD_MAX; i++)
+	for (i = 0; i < BFPT_DWORD_MAX; i++) {
 		bfpt.dwords[i] = le32_to_cpu(bfpt.dwords[i]);
+		dev_info(nor->dev, "DWORD%d = 0x%08x\n", i + 1, bfpt.dwords[i]);
+	}
+	dev_info(nor->dev, "BFPT version %d.%d (length = %u)\n",
+		 bfpt_header->major,
+		 bfpt_header->minor,
+		 bfpt_header->length);
 
 	/* Number of address bytes. */
 	switch (bfpt.dwords[BFPT_DWORD(1)] & BFPT_DWORD1_ADDRESS_BYTES_MASK) {