Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
IronPort-SDR: 10ujuXmvau6TEwbh6ZLuN2OUZI9XUKtJAKrEpZs4ebYLJhjdCX7i5CMK8G2oC6OzM1UlKIxPhr
 dpMuAMgBEdCduhItJAZqJ84cJEjfT7m4DGI0VC8GkX0c/ha+pqOX7M0UHFrJDEUerquqtg4i6A
 jRkSQU+ohe+qn2ErWdGLxiSMXt8E16+rcY9g0j7376QipILOFkI98wJ6nofq/t1et26sNBiAko
 sacTquKfj2SqCxDCJNdldGWWiaHQJeE8Fz6kqZLnloi4hSe5nVerQXXACI8z58TrUJDNpi+7x1
 eh4=
From:   <Tudor.Ambarus@microchip.com>
To:     <mantas@8devices.com>
CC:     <gch981213@gmail.com>, <robimarko@gmail.com>,
        <linux-mtd@lists.infradead.org>, <miquel.raynal@bootlin.com>,
        <richard@nod.at>, <vigneshr@ti.com>, <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] mtd: spi-nor: fix 4-byte opcode support for w25q256
Thread-Topic: [PATCH] mtd: spi-nor: fix 4-byte opcode support for w25q256
Thread-Index: AQHWFwHieCOFTav6rUeke0dD3DMEOg==
Date:   Tue, 21 Apr 2020 06:58:55 +0000
Message-ID: <14454062.LuVNJbZNcS@192.168.0.120>
References: <1586958510-24012-1-git-send-email-mantas@8devices.com>
 <43054851.jYS1km7NsV@192.168.0.120>
 <e4826369-f239-4b67-1da8-f7d9c8357abe@8devices.com>
In-Reply-To: <e4826369-f239-4b67-1da8-f7d9c8357abe@8devices.com>
Accept-Language: en-US
Content-Language: en-US
received-spf: None (protection.outlook.com: microchip.com does not designate
 permitted sender hosts)
Content-Type: text/plain; charset="us-ascii"
Content-ID: <8CC4F17F81862A49AAFE2791F1894642@namprd11.prod.outlook.com>
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-Network-Message-Id: c7b6a09a-c8f3-4137-b303-08d7e5c17594
X-MS-Exchange-CrossTenant-originalarrivaltime: 21 Apr 2020 06:58:55.8224
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 3f4057f3-b418-4d4e-ba84-d55b4e897d88
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: joi7H8eXjHhdhE8xYVxkPjRo5T1FbaPSAT2+CeiT+0f7FZYxsDhTlrvnnXBffGy13G+6hfsSDug90SMMsY8CJB/0uymemwjl6H5yL4voRLw=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BY5PR11MB4119
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On Tuesday, April 21, 2020 9:08:30 AM EEST Mantas wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know th=
e
> content is safe
> On 2020-04-20 13:53, Tudor.Ambarus@microchip.com wrote:
> > On Wednesday, April 15, 2020 4:48:30 PM EEST Mantas Pucka wrote:
> >> EXTERNAL EMAIL: Do not click links or open attachments unless you know
> >> the
> >> content is safe
> >>=20
> >> There are 2 different chips (w25q256fv and w25q256jv) that share
> >> the same JEDEC ID. Only w25q256jv fully supports 4-byte opcodes.
> >> Use SFDP header version to differentiate between them.
> >>=20
> >> Signed-off-by: Mantas Pucka <mantas@8devices.com>
> >> ---
> >>=20
> >>   drivers/mtd/spi-nor/sfdp.c    |  4 ----
> >>   drivers/mtd/spi-nor/sfdp.h    |  6 ++++++
> >>   drivers/mtd/spi-nor/winbond.c | 30 ++++++++++++++++++++++++++++--
> >>   3 files changed, 34 insertions(+), 6 deletions(-)
> >>=20
> >> diff --git a/drivers/mtd/spi-nor/sfdp.c b/drivers/mtd/spi-nor/sfdp.c
> >> index f6038d3..27838f6 100644
> >> --- a/drivers/mtd/spi-nor/sfdp.c
> >> +++ b/drivers/mtd/spi-nor/sfdp.c
> >> @@ -21,10 +21,6 @@
> >>=20
> >>   #define SFDP_4BAIT_ID          0xff84  /* 4-byte Address Instruction
> >>   Table
> >>=20
> >> */
> >>=20
> >>   #define SFDP_SIGNATURE         0x50444653U
> >>=20
> >> -#define SFDP_JESD216_MAJOR     1
> >> -#define SFDP_JESD216_MINOR     0
> >> -#define SFDP_JESD216A_MINOR    5
> >> -#define SFDP_JESD216B_MINOR    6
> >>=20
> >>   struct sfdp_header {
> >>  =20
> >>          u32             signature; /* Ox50444653U <=3D> "SFDP" */
> >>=20
> >> diff --git a/drivers/mtd/spi-nor/sfdp.h b/drivers/mtd/spi-nor/sfdp.h
> >> index e0a8ded..b84abd0 100644
> >> --- a/drivers/mtd/spi-nor/sfdp.h
> >> +++ b/drivers/mtd/spi-nor/sfdp.h
> >> @@ -7,6 +7,12 @@
> >>=20
> >>   #ifndef __LINUX_MTD_SFDP_H
> >>   #define __LINUX_MTD_SFDP_H
> >>=20
> >> +/* SFDP revisions */
> >> +#define SFDP_JESD216_MAJOR     1
> >> +#define SFDP_JESD216_MINOR     0
> >> +#define SFDP_JESD216A_MINOR    5
> >> +#define SFDP_JESD216B_MINOR    6
> >> +
> >>=20
> >>   /* Basic Flash Parameter Table */
> >>  =20
> >>   /*
> >>=20
> >> diff --git a/drivers/mtd/spi-nor/winbond.c
> >> b/drivers/mtd/spi-nor/winbond.c
> >> index 17deaba..50b2478 100644
> >> --- a/drivers/mtd/spi-nor/winbond.c
> >> +++ b/drivers/mtd/spi-nor/winbond.c
> >> @@ -8,6 +8,32 @@
> >>=20
> >>   #include "core.h"
> >>=20
> >> +static int
> >> +w25q256_post_bfpt_fixups(struct spi_nor *nor,
> >> +                        const struct sfdp_parameter_header *bfpt_head=
er,
> >> +                        const struct sfdp_bfpt *bfpt,
> >> +                        struct spi_nor_flash_parameter *params)
> >> +{
> >> +       /*
> >> +        * W25Q256JV supports 4B opcodes but W25Q256FV does not.
> >> +        * Unfortunately, Winbond has re-used the same JEDEC ID for bo=
th
> >> +        * variants which prevents us from defining a new entry in the
> >> parts +        * table.
> >> +        * To differentiate between W25Q256JV and W25Q256FV check SFDP
> >> header +        * version: only JV has JESD216A compliant structure
> >> (version 5) +        */
> >> +
> >> +       if (bfpt_header->major =3D=3D SFDP_JESD216_MAJOR &&
> >> +           bfpt_header->minor =3D=3D SFDP_JESD216A_MINOR)
> >=20
> > Not sure if this is generic enough. Are you sure that the JV version wi=
ll
> > never have an update for the sfdp tables?
>=20
> No, I'm not sure. I also don't know about other changes that may come
> with a version update: will it have 4B opcode table? will it be

new 4bait table will just OR the SNOR_F_4B_OPCODES flags, no problem with=20
that.

> different version again (say KV) with it's own quirks? Fix only what
> needs fixing was the idea. But I guess chances of new chip with no 4B
> opcodes and new SFDP table are pretty slim, so I'm OK with having >=3D in=
 v2.

stripping 4B opcodes from a revision to another would be a first, but you'r=
e=20
right, we can fix others when needed, so no need for a v2.
>=20
> >> +               nor->flags |=3D SNOR_F_4B_OPCODES;
> >> +
> >> +       return 0;
> >> +}
> >> +
> >> +static struct spi_nor_fixups w25q256_fixups =3D {
> >> +       .post_bfpt =3D w25q256_post_bfpt_fixups,
> >> +};
> >> +
> >=20
> > If the post_bfpt hook is called, you already have a valid bfpt table. I=
f
> > the differentiator between the JV and FV versions is that only the JV
> > defines the SFDP tables, then your w25q256_post_bfpt_fixups() can look
> > as:
> >=20
> > static int w25q256_post_bfpt_fixups()
> > {
> >=20
> >       nor->flags |=3D SNOR_F_4B_OPCODES;
> >       return 0;
> >=20
> > }
>=20
> FV chip that I have, do actually have SFDP tables (with
> minor_version=3D=3D0). I've saw Chuanhong reporting that some FV chips do=
n't
> have SFDP, but certainly this is not the case for all of them.
>=20
oh, the horror :). I think I have a w25q256 somewhere, allow me some time t=
o=20
do some tests.

Cheers,
ta