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;
From:   Sagar Kadam <sagar.kadam@sifive.com>
To:     Pratyush Yadav <me@yadavpratyush.com>
CC:     "linux-riscv@lists.infradead.org" <linux-riscv@lists.infradead.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "linux-mtd@lists.infradead.org" <linux-mtd@lists.infradead.org>,
        "tudor.ambarus@microchip.com" <tudor.ambarus@microchip.com>,
        "miquel.raynal@bootlin.com" <miquel.raynal@bootlin.com>,
        "richard@nod.at" <richard@nod.at>,
        "vigneshr@ti.com" <vigneshr@ti.com>,
        "anup.patel@wdc.com" <anup.patel@wdc.com>,
        "aou@eecs.berkeley.edu" <aou@eecs.berkeley.edu>,
        "palmer@dabbelt.com" <palmer@dabbelt.com>,
        Paul Walmsley <paul.walmsley@sifive.com>
Subject: RE: [PATCH v1 2/2] spi: nor: update page program settings for
 is25wp256 using post bfpt fixup
Thread-Topic: [PATCH v1 2/2] spi: nor: update page program settings for
 is25wp256 using post bfpt fixup
Thread-Index: AQHWKeYAxlvBjs/+4kCjqacoVp02OaioumMAgAAX5fA=
Date:   Fri, 15 May 2020 08:41:29 +0000
Message-ID: <BYAPR13MB2614A8044A8AA51A3C0E020A99BD0@BYAPR13MB2614.namprd13.prod.outlook.com>
References: <1589457051-5410-1-git-send-email-sagar.kadam@sifive.com>
 <1589457051-5410-3-git-send-email-sagar.kadam@sifive.com>
 <20200515070436.hk5ho7ygluqznpcq@yadavpratyush.com>
In-Reply-To: <20200515070436.hk5ho7ygluqznpcq@yadavpratyush.com>
Accept-Language: en-US
Content-Language: en-US
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-Network-Message-Id: 8fdd7f0a-8395-4fd2-d43f-08d7f8abc379
X-MS-Exchange-CrossTenant-originalarrivaltime: 15 May 2020 08:41:29.6673
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 22f88e9d-ae0d-4ed9-b984-cdc9be1529f1
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: vgXSQfxrF5eor72FFZSYnPwV24LvVmHw5Cr1n4qpZtDJhm/OfxxikI51/FuZfsGYZe8lsT4fWlUEqPcAfHYB5w==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BYAPR13MB2725
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

Hi Pratyush,

> -----Original Message-----
> From: Pratyush Yadav <me@yadavpratyush.com>
> Sent: Friday, May 15, 2020 12:35 PM
> To: Sagar Kadam <sagar.kadam@sifive.com>
> Cc: linux-riscv@lists.infradead.org; linux-kernel@vger.kernel.org; linux-
> mtd@lists.infradead.org; tudor.ambarus@microchip.com;
> miquel.raynal@bootlin.com; richard@nod.at; vigneshr@ti.com;
> anup.patel@wdc.com; aou@eecs.berkeley.edu; palmer@dabbelt.com; Paul
> Walmsley <paul.walmsley@sifive.com>
> Subject: Re: [PATCH v1 2/2] spi: nor: update page program settings for
> is25wp256 using post bfpt fixup
>=20
> [External Email] Do not click links or attachments unless you recognize t=
he
> sender and know the content is safe
>=20
> Hi Sagar,
>=20
> On 14/05/20 04:50AM, Sagar Shrikant Kadam wrote:
> > During SFDP parsing it is seen that the IS25WP256d device is missing 4B=
AIT
> > (4-Byte address instruction table), due to which it's page program
> > capacity doesn't get correctly populated and the device gets configured
> > with 4-byte Address Serial Input Page Program i.e. SNOR_PROTO_1_1_1
> > even though it can work with SNOR_PROTO_1_1_4.
> >
> > Here using the post bfpt fixup hooks we update the page program
> > settings to 4-byte QUAD Input Page program operations.
> >
> > The patch is tested on HiFive Unleashed A00 board and it benefits
> > few seconds of average write time for entire flash write.
> >
> > QUAD Input Page Program operations:
> > > time mtd_debug write /dev/mtd0 0 33554432 rd32M
> > Copied 33554432 bytes from rd32M to address 0x00000000 in flash
> > real    0m 32.85s
> > user    0m 0.00s
> > sys     0m 31.79s
> >
> > Serial Input Page Program operations:
> > > time mtd_debug write /dev/mtd0 0 33554432 rd32M
> > Copied 33554432 bytes from rd32M to address 0x00000000 in flash
> > real    0m 35.87s
> > user    0m 0.00s
> > sys     0m 35.42s
> >
> > Signed-off-by: Sagar Shrikant Kadam <sagar.kadam@sifive.com>
> > ---
> >  drivers/mtd/spi-nor/issi.c | 16 ++++++++++++++++
> >  1 file changed, 16 insertions(+)
> >
> > diff --git a/drivers/mtd/spi-nor/issi.c b/drivers/mtd/spi-nor/issi.c
> > index ffcb60e..9eb6e82 100644
> > --- a/drivers/mtd/spi-nor/issi.c
> > +++ b/drivers/mtd/spi-nor/issi.c
> > @@ -23,6 +23,22 @@ is25lp256_post_bfpt_fixups(struct spi_nor *nor,
> >               BFPT_DWORD1_ADDRESS_BYTES_3_ONLY)
> >               nor->addr_width =3D 4;
> >
> > +     /*
> > +      * On IS25WP256d device 4-Byte address instruction table doesn't
> > +      * get populated and so the device get's configured with 4-byte
> > +      * Address Serial Input Page Program i.e. SNOR_PROTO_1_1_1 even
> > +      * though it supports SNOR_PROTO_1_1_4, so priorotize QUAD write
> > +      * over SINGLE write if device id table holds SPI_NOR_QUAD_READ.
> > +      */
> > +     if (strcmp(nor->info->name, "is25wp256") =3D=3D 0) {
>=20
> Instead of doing this, wouldn't it make more sense to have a separate
> fixup hook for is25wp256? Does this device also need the above address
> width fixup? If it does, maybe that can be split into a separate
> function, and used by both the fixups?
>=20
Thanks for suggestion. Yes this device requires the above address width fix=
up.
I suspect that this QUAD mode fix might also be required for "is25lp256" de=
vice.
But since I don't have it on my board, I couldn't validate it. If someone c=
ould give it a try
on "is25lp256" device and confirm this, then I guess we can remove this che=
ck from=20
here "if (strcmp(nor->info->name, "is25wp256") =3D=3D 0)" and rename the is=
25lp256_post_bfpt_fixups
to is25lpwp256_post_bfpt_fixups to use command fixup for both flash devices=
,
else I am also ok to split it into separate function as suggested that can =
be used by both fixup's

Thanks & BR,
Sagar Kadam

> > +             if (nor->info->flags & SPI_NOR_QUAD_READ) {
> > +                     params->hwcaps.mask |=3D SNOR_HWCAPS_PP_1_1_4;
> > +                     spi_nor_set_pp_settings
> > +                             (&params->page_programs[SNOR_CMD_PP_1_1_4=
],
> > +                              SPINOR_OP_PP_1_1_4,
> > +                              SNOR_PROTO_1_1_4);
> > +             }
> > +     }
> >       return 0;
> >  }
>=20
> --
> Regards,
> Pratyush Yadav