Re: [PATCH v5 1/3] mtd: spi-nor: add support for is25wp256

From: Sagar Kadam
Date: Mon Jun 24 2019 - 08:40:34 EST


Hello Vignesh,

On Mon, Jun 24, 2019 at 3:04 PM Vignesh Raghavendra <vigneshr@xxxxxx> wrote:
>
> Hi,
>
> On 21/06/19 3:58 PM, Sagar Kadam wrote:
> > Hello Vignesh,
> >
> > On Fri, Jun 21, 2019 at 11:33 AM Vignesh Raghavendra <vigneshr@xxxxxx> wrote:
> >>
> >> Hi,
> >>
> >> On 17/06/19 8:48 PM, Sagar Kadam wrote:
> >>> Hello Vignesh,
> >>>
> >>> Thanks for your review comments.
> >>>
> >>> On Sun, Jun 16, 2019 at 6:14 PM Vignesh Raghavendra <vigneshr@xxxxxx> wrote:
> >>>>
> >>>> Hi,
> >>>>
> >>>> On 12-Jun-19 4:17 PM, Sagar Shrikant Kadam wrote:
> >>>> [...]
> >>>>
> >>>>> @@ -4129,7 +4137,7 @@ int spi_nor_scan(struct spi_nor *nor, const char *name,
> >>>>> if (ret)
> >>>>> return ret;
> >>>>>
> >>>>> - if (nor->addr_width) {
> >>>>> + if (nor->addr_width && JEDEC_MFR(info) != SNOR_MFR_ISSI) {
> >>>>> /* already configured from SFDP */
> >>>>
> >>>> Hmm, why would you want to ignore addr_width that's read from SFDP table?
> >>>
> >>> The SFDP table for ISSI device considered here, has addr_width set to
> >>> 3 byte, and the flash considered
> >>> here is 32MB. With 3 byte address width we won't be able to access
> >>> flash memories higher address range.
> >>
> >> Is it specific to a particular ISSI part as indicated here[1]? If so,
> >> please submit solution agreed there i.e. use spi_nor_fixups callback
> >>
> >> [1]https://patchwork.ozlabs.org/patch/1056049/
> >>
> >
> > Thanks for sharing the link.
> > From what I understand here, it seems that "Address Bytes" of SFDP
> > table for the device under
> > consideration (is25lp256) supports 3 byte only Addressing mode
> > (DWORD1[18:17] = 0b00.
> > where as that of ISSI device (is25LP/WP 256Mb/512/Mb/1Gb) support 3 or
> > 4 byte Addressing mode DWORD1[18:17] = 0b01.
> >
>
> Okay, so that SFDP table entry is correct. SPI NOR framework should
> using 4 byte addressing if WORD1[18:17] = 0b01. Could you see if below
> diff helps:
>
Thank-you for the suggestion.
I applied it, and observed, that data in SFDP table mentioned in
document received
from ISSI support doesn't match with what is actually present on the
device (I have raised a query with issi support for the same)
The WP device also has the same SFDP entry as the LP device (the one
which you shared).
So, will submit V7 with the solution agreed in the link you shared above.
https://patchwork.ozlabs.org/patch/1056049/
Apologies for the confusion, so please excuse the v6 which I submitted earlier.

Thanks & BR,
Sagar Kadam

> --->8---
> diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
> index c0a8837c0575..ebf32aebe5e9 100644
> --- a/drivers/mtd/spi-nor/spi-nor.c
> +++ b/drivers/mtd/spi-nor/spi-nor.c
> @@ -2808,6 +2808,7 @@ static int spi_nor_parse_bfpt(struct spi_nor *nor,
> break;
>
> case BFPT_DWORD1_ADDRESS_BYTES_4_ONLY:
> + case BFPT_DWORD1_ADDRESS_BYTES_3_OR_4:
> nor->addr_width = 4;
> break;


>
>
>
>
> --
> Regards
> Vignesh