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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20220413172819.438f79e4@xps13>
Date:   Wed, 13 Apr 2022 17:28:19 +0200
From:   Miquel Raynal <miquel.raynal@...tlin.com>
To:     Chuanhong Guo <gch981213@...il.com>
Cc:     linux-mtd@...ts.infradead.org, Richard Weinberger <richard@....at>,
        Vignesh Raghavendra <vigneshr@...com>,
        Patrice Chotard <patrice.chotard@...s.st.com>,
        Boris Brezillon <boris.brezillon@...labora.com>,
        Christophe Kerello <christophe.kerello@...s.st.com>,
        Mark Brown <broonie@...nel.org>,
        Daniel Palmer <daniel@...f.com>,
        open list <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v2] mtd: spinand: add support for ESMT F50x1G41LB

Hello,

gch981213@...il.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@...tlin.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