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

[Sagar Kadam]

Hi Vignesh,

On Mon, Jun 24, 2019 at 6:37 PM Vignesh Raghavendra <vigneshr@xxxxxx> wrote:
>
>
>
> On 24/06/19 6:10 PM, Sagar Kadam wrote:
> > 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.
> >
>
> There is an updated version of the patch:
> https://patchwork.ozlabs.org/patch/1071453/
>
> You may want to align with Liu Xiang to avoid duplication of effort
>

Ok. It seems Liu Xiang, is working on supporting is25LP256 device,
while the one considered here is WP
Although both are very similar devices, they differ a bit maybe with
Operating voltage, ID values, die revision etc..
I will sync-up with him about his v4 plan, and update you accordingly.
Thanks for the link.

BR,
Sagar Kadam


> > 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
>
> --
> Regards
> Vignesh