Messages in this thread | | | Date | Thu, 14 Apr 2022 17:06:44 +0200 | From | Boris Brezillon <> | Subject | Re: [PATCH 1/2] mtd: spinand: add support for detection with param page |
| |
On Thu, 14 Apr 2022 22:34:25 +0800 Chuanhong Guo <gch981213@gmail.com> wrote:
> SPI-NAND detection using chip ID isn't always reliable. > Here are two known cases: > 1. ESMT uses JEDEC ID from other vendors. This may collapse with future > chips.
Really?! So the manufacturer id matching is not even possible?
> 2. Winbond W25N01KV uses the exact same JEDEC ID as W25N01GV while > having completely different chip parameters. > > Recent SPI-NANDs have a parameter page which is stored in the same > format as raw NAND ONFI data. There are strings for chip manufacturer > and chip model. Chip ECC requirement and memory organization are > available too. > This patch adds support for detecting SPI-NANDs with the parameter page > after ID matching failed. In this detection, memory organization and > ECC requirements are taken from the parameter page, and other SPI-NAND > specific parameters are obtained by matching chip model string with > a separated table.
Oh come on! Looks like they never learn from their mistakes...
> > It's common for vendors to release a series of SPI-NANDs with the same > SPI-NAND parameters in different voltages and/or capacities. The chip > table defined in this patch supports multiple model strings in one > entry, and multiple chip models can be covered using only one entry.
That's really disappointing. And I guess the ONFI-page read commands are not even standard, and each vendor will come with its own sequence to read it. What's bothering me the most is the fact that ONFI parameter pages are not even designed for SPI NANDs. They have a bunch of parameters that don't really apply to SPI NANDs, and we're still lacking SPI-specific info, like the read/write/erase command variants, the maximum SPI bus width (AKA SPI modes)...
To sum-up, if we have to support reading the ONFI parameter page, I'd rather keep it vendor-private (but that's assuming the vendor ID detection works, and you seem to imply ESMT cheats on that too), and use it only as a way to distinguish between 2 chip variants.
> > Signed-off-by: Chuanhong Guo <gch981213@gmail.com> > --- > > I'm not brave enough to touch raw nand code without testing, so > sanitize_string, onfi_crc16 and nand_bit_wise_majority are > duplicated from raw/nand_onfi.c into spi/onfi.c > > drivers/mtd/nand/spi/Makefile | 2 +- > drivers/mtd/nand/spi/core.c | 23 +-- > drivers/mtd/nand/spi/onfi.c | 296 ++++++++++++++++++++++++++++++++++
Please, no. Try to move the common code to drivers/mtd/nand/onfi.c instead of duplicating it. AFAICT, the only thing that's spinand specific is spinand_onfi_read() and the last part of spinand_onfi_detect(). I'm sure we can find a way to expose generic onfi_parsing() helpers.
| |