Re: [PATCH v2] mtd: spinand: add support for ESMT F50x1G41LB

From: Miquel Raynal <miquel.raynal@bootlin.com>
To: Chuanhong Guo <gch981213@gmail.com>
Cc: linux-mtd@lists.infradead.org,
	Richard Weinberger <richard@nod.at>,
	Vignesh Raghavendra <vigneshr@ti.com>,
	Patrice Chotard <patrice.chotard@foss.st.com>,
	Boris Brezillon <boris.brezillon@collabora.com>,
	Christophe Kerello <christophe.kerello@foss.st.com>,
	Mark Brown <broonie@kernel.org>, Daniel Palmer <daniel@0x0f.com>,
	open list <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v2] mtd: spinand: add support for ESMT F50x1G41LB
Date: Wed, 13 Apr 2022 17:28:19 +0200	[thread overview]
Message-ID: <20220413172819.438f79e4@xps13> (raw)
In-Reply-To: <CAJsYDV+3J0ipbR+N-xE=DH-WXsjierdHe_pJtKf1Xbt7fdaiWw@mail.gmail.com>

Hello,

gch981213@gmail.com wrote on Wed, 13 Apr 2022 22:50:43 +0800:

> Hi!
> 
> On Wed, Apr 13, 2022 at 8:58 PM Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> > [...]  
> > > This patch is made purely based on datasheet info without testing
> > > on any actual chips.  
> >
> > Do you plan to get one of these any time soon?  
> 
> No. I already have way more spi-nand chips than I possibly
> need due to my last GigaDevice submission :)

:)

> 
> I need to replace the spi-nand driver for mediatek in OpenWrt
> with my recent submission, and this chip is used in one of the
> currently supported router. So I made this patch from
> datasheet for it.
> 
> BTW Rockchip added identical chip support in their u-boot.[0]
> I assume they've tested it.
> 
> > I am not really confident merging a 100% non-tested driver :)  
> 
> I can understand that.
> I'll roll this patch out in OpenWrt anyway. I can wait for a test
> there and resubmit piled downstream patches for chip supports
> after next OpenWrt stable release.

Yes, please, any feedback from the OpenWrt community would be good. If
you can get a Tested-by I'll take the patch (resend it with the tag so
that I don't miss it).

> > [...]  
> > > +
> > > +/* ESMT uses GigaDevice 0xc8 JECDEC ID on some SPI NANDs */
> > > +#define SPINAND_MFR_ESMT_C8                  0xc8  
> >
> > What happens if the gigadevice driver probes first?  
> 
> Their device ID aren't conflicting yet, so nothing will happen
> at the moment.
> 
> There is a solution for future conflict: Recent SPI-NAND chips
> contain a parameter page which has the exact chip vendor
> and model. We can do one more detection with the parameter
> page content.
> Winbond W25N01KV is a 2k+96 SPI-NAND with 4-bit ECC.
> It uses the exact same chip id as the current W25N01GV
> (2k+64 1-bit ECC). We need to support detection using
> parameter page for this crazy decision by Winbond anyway.

:')

> I'll try to code something for this with my free time.
> My current idea is: We first do a detection based on chip id.
> If that failed, try to read the parameter page. If we got a
> valid one, match the chip vendor and model string.
> 
> Any thoughts?

Yeah that looks reasonable.

> (BTW this ESMT chip is POWERCHIP PSU1GS20DX
> according to the parameter page in their datasheet.
> But I can't find a datasheet for this model number.)
> 
> >  
> > > +
> > > +#define F50L2G41XA_ECC_STATUS_MASK           GENMASK(6, 4)
> > > +#define F50L2G41XA_STATUS_ECC_1_3_BITFLIPS   (1 << 4)
> > > +#define F50L2G41XA_STATUS_ECC_4_6_BITFLIPS   (3 << 4)
> > > +#define F50L2G41XA_STATUS_ECC_7_8_BITFLIPS   (5 << 4)  
> 
> Oops. These are left-over defines when I discovered that ESMT 2G/4G
> SPI-NANDs are repackaged micron parts and dropped their support
> from this file.
> A v3 is needed anyway. I'll wait for a test before submitting it.
> 
> [0]: https://github.com/rockchip-linux/u-boot/commit/52b0060178285488737854a48ddecd381f8b236e

Thanks,
Miquèl

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/