Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Wed, 13 Apr 2022 14:58:43 +0200
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>,
        linux-kernel@vger.kernel.org (open list)
Subject: Re: [PATCH v2] mtd: spinand: add support for ESMT F50x1G41LB
Message-ID: <20220413145843.46a3d9b5@xps13>
In-Reply-To: <20220413083824.247136-1-gch981213@gmail.com>
References: <20220413083824.247136-1-gch981213@gmail.com>
Organization: Bootlin
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

Hi,

gch981213@gmail.com wrote on Wed, 13 Apr 2022 16:38:19 +0800:

> This patch adds support for ESMT F50L1G41LB and F50D1G41LB.
> It seems that ESMT likes to use random JEDEC ID from other vendors.
> Their 1G chips uses 0xc8 from GigaDevice and 2G/4G chips uses 0x2c from
> Micron. For this reason, the ESMT entry is named esmt_c8 with explicit
> JEDEC ID in variable name.
>=20
> Datasheets:
> https://www.esmt.com.tw/upload/pdf/ESMT/datasheets/F50L1G41LB(2M).pdf
> https://www.esmt.com.tw/upload/pdf/ESMT/datasheets/F50D1G41LB(2M).pdf
>=20
> Signed-off-by: Chuanhong Guo <gch981213@gmail.com>
> ---
> 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?

I am not really confident merging a 100% non-tested driver :)

> Change since v1: drop 0x7f padding from SPINAND_ID.
>=20
>  drivers/mtd/nand/spi/Makefile |  2 +-
>  drivers/mtd/nand/spi/core.c   |  1 +
>  drivers/mtd/nand/spi/esmt.c   | 94 +++++++++++++++++++++++++++++++++++
>  include/linux/mtd/spinand.h   |  1 +
>  4 files changed, 97 insertions(+), 1 deletion(-)
>  create mode 100644 drivers/mtd/nand/spi/esmt.c
>=20
> diff --git a/drivers/mtd/nand/spi/Makefile b/drivers/mtd/nand/spi/Makefile
> index 9662b9c1d5a9..7e3ab8a9aec7 100644
> --- a/drivers/mtd/nand/spi/Makefile
> +++ b/drivers/mtd/nand/spi/Makefile
> @@ -1,3 +1,3 @@
>  # SPDX-License-Identifier: GPL-2.0
> -spinand-objs :=3D core.o gigadevice.o macronix.o micron.o paragon.o tosh=
iba.o winbond.o
> +spinand-objs :=3D core.o esmt.o gigadevice.o macronix.o micron.o paragon=
.o toshiba.o winbond.o
>  obj-$(CONFIG_MTD_SPI_NAND) +=3D spinand.o
> diff --git a/drivers/mtd/nand/spi/core.c b/drivers/mtd/nand/spi/core.c
> index ff8336870bc0..6c5d79ec3501 100644
> --- a/drivers/mtd/nand/spi/core.c
> +++ b/drivers/mtd/nand/spi/core.c
> @@ -927,6 +927,7 @@ static const struct nand_ops spinand_ops =3D {
>  };
> =20
>  static const struct spinand_manufacturer *spinand_manufacturers[] =3D {
> +	&esmt_c8_spinand_manufacturer,
>  	&gigadevice_spinand_manufacturer,
>  	&macronix_spinand_manufacturer,
>  	&micron_spinand_manufacturer,
> diff --git a/drivers/mtd/nand/spi/esmt.c b/drivers/mtd/nand/spi/esmt.c
> new file mode 100644
> index 000000000000..f86716332893
> --- /dev/null
> +++ b/drivers/mtd/nand/spi/esmt.c
> @@ -0,0 +1,94 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Author:
> + *	Chuanhong Guo <gch981213@gmail.com>
> + */
> +
> +#include <linux/device.h>
> +#include <linux/kernel.h>
> +#include <linux/mtd/spinand.h>
> +
> +/* ESMT uses GigaDevice 0xc8 JECDEC ID on some SPI NANDs */
> +#define SPINAND_MFR_ESMT_C8			0xc8

What happens if the gigadevice driver probes first?

> +
> +#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)
> +


Thanks,
Miqu=C3=A8l