Re: [PATCH] mtd: spinand: Support Paragon PN26G01A and PN26G02A

[Schrempf Frieder <frieder.schrempf@xxxxxxxxxx>]

On 05.06.19 23:35, Jeff Kletsky wrote:
> From: Jeff Kletsky <git-commits@xxxxxxxxxxxx>
> 
> These Paragon chips are very similar to other 1Gb/2Gb chips
> in terms of their layout and command timings.
> 
> One notable difference is that "Minimum number of valid blocks"
> (Nvb) is 1003 per Gb, rather than the common 1004. As a result,
> the bad-block reservation is 21 per Gb, rather than 20 per Gb.

Nitpick: Actually the maximum number of bad blocks is not "21 * capacity 
in Gb", but "(20 * capacity in Gb) + 1". So the 1Gb type has a maximum 
of 21 and the 2Gb type of 41 bad blocks.

> 
> Datasheets available at
> http://www.xtxtech.com/upfile/2016082517274590.pdf
> http://www.xtxtech.com/upfile/2016082517282329.pdf
> 
> Signed-off-by: Jeff Kletsky <git-commits@xxxxxxxxxxxx>
> ---
>   drivers/mtd/nand/spi/Makefile  |   2 +-
>   drivers/mtd/nand/spi/core.c    |   1 +
>   drivers/mtd/nand/spi/paragon.c | 142 +++++++++++++++++++++++++++++++++
>   include/linux/mtd/spinand.h    |   1 +
>   4 files changed, 145 insertions(+), 1 deletion(-)
>   create mode 100644 drivers/mtd/nand/spi/paragon.c
> 
> diff --git a/drivers/mtd/nand/spi/Makefile b/drivers/mtd/nand/spi/Makefile
> index 753125082640..9662b9c1d5a9 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 := core.o gigadevice.o macronix.o micron.o toshiba.o winbond.o
> +spinand-objs := core.o gigadevice.o macronix.o micron.o paragon.o toshiba.o winbond.o
>   obj-$(CONFIG_MTD_SPI_NAND) += spinand.o
> diff --git a/drivers/mtd/nand/spi/core.c b/drivers/mtd/nand/spi/core.c
> index 556bfdb34455..f0f3528aab8f 100644
> --- a/drivers/mtd/nand/spi/core.c
> +++ b/drivers/mtd/nand/spi/core.c
> @@ -757,6 +757,7 @@ static const struct spinand_manufacturer *spinand_manufacturers[] = {
>   	&gigadevice_spinand_manufacturer,
>   	&macronix_spinand_manufacturer,
>   	&micron_spinand_manufacturer,
> +	&paragon_spinand_manufacturer,
>   	&toshiba_spinand_manufacturer,
>   	&winbond_spinand_manufacturer,
>   };
> diff --git a/drivers/mtd/nand/spi/paragon.c b/drivers/mtd/nand/spi/paragon.c
> new file mode 100644
> index 000000000000..dd863dbc593a
> --- /dev/null
> +++ b/drivers/mtd/nand/spi/paragon.c
> @@ -0,0 +1,142 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2019 Jeff Kletsky
> + *
> + * Author: Jeff Kletsky <git-commits@xxxxxxxxxxxx>
> + */
> +
> +#include <linux/device.h>
> +#include <linux/kernel.h>
> +#include <linux/mtd/spinand.h>
> +
> +
> +#define SPINAND_MFR_PARAGON	0xa1
> +
> +
> +#define PN26G0XA_STATUS_ECC_BITMASK		(3 << 4)
> +
> +#define PN26G0XA_STATUS_ECC_NONE_DETECTED	(0 << 4)
> +#define PN26G0XA_STATUS_ECC_1_7_CORRECTED	(1 << 4)
> +#define PN26G0XA_STATUS_ECC_ERRORED		(2 << 4)
> +#define PN26G0XA_STATUS_ECC_8_CORRECTED		(3 << 4)
> +
> +
> +static SPINAND_OP_VARIANTS(read_cache_variants,
> +		SPINAND_PAGE_READ_FROM_CACHE_QUADIO_OP(0, 2, NULL, 0),
> +		SPINAND_PAGE_READ_FROM_CACHE_X4_OP(0, 1, NULL, 0),
> +		SPINAND_PAGE_READ_FROM_CACHE_DUALIO_OP(0, 1, NULL, 0),
> +		SPINAND_PAGE_READ_FROM_CACHE_X2_OP(0, 1, NULL, 0),
> +		SPINAND_PAGE_READ_FROM_CACHE_OP(true, 0, 1, NULL, 0),
> +		SPINAND_PAGE_READ_FROM_CACHE_OP(false, 0, 1, NULL, 0));
> +
> +static SPINAND_OP_VARIANTS(write_cache_variants,
> +		SPINAND_PROG_LOAD_X4(true, 0, NULL, 0),
> +		SPINAND_PROG_LOAD(true, 0, NULL, 0));
> +
> +static SPINAND_OP_VARIANTS(update_cache_variants,
> +		SPINAND_PROG_LOAD_X4(false, 0, NULL, 0),
> +		SPINAND_PROG_LOAD(false, 0, NULL, 0));
> +
> +
> +static int pn26g0xa_ooblayout_ecc(struct mtd_info *mtd, int section,
> +				   struct mtd_oob_region *region)
> +{
> +	if (section)
> +		return -ERANGE;
> +
> +	region->offset = 64;
> +	region->length = 64;
> +
> +	return 0;
> +}
> +
> +static int pn26g0xa_ooblayout_free(struct mtd_info *mtd, int section,
> +				   struct mtd_oob_region *region)
> +{
> +	if (section)
> +		return -ERANGE;
> +
> +	region->offset = 1;	/* Reserved byte for BBM */
> +	region->length = 63;
> +
> +	return 0;
> +}

The OOB layout above seems to suggest, that each OOB area has one half 
of user data and one half of ECC data. But the table in the datasheet 
shows a separation into four sections, each consisting of 2 bytes user 
data and 13 bytes ECC data, following one 64 byte block of unprotected 
user data.

Also the table shows 4 bytes reserved for the bad block marker instead 
of only 1.

> +
> +static int pn26g0xa_ecc_get_status(struct spinand_device *spinand,
> +				   u8 status)
> +{
> +	switch (status & PN26G0XA_STATUS_ECC_BITMASK) {
> +	case PN26G0XA_STATUS_ECC_NONE_DETECTED:
> +		return 0;
> +
> +	case PN26G0XA_STATUS_ECC_1_7_CORRECTED:
> +		return 7;	/* Return upper limit by convention */
> +
> +	case PN26G0XA_STATUS_ECC_8_CORRECTED:
> +		return 8;
> +
> +	case PN26G0XA_STATUS_ECC_ERRORED:
> +		return -EBADMSG;
> +
> +	default:
> +		break;
> +	}
> +
> +	return -EINVAL;
> +}
> +
> +static const struct mtd_ooblayout_ops pn26g0xa_ooblayout = {
> +	.ecc = pn26g0xa_ooblayout_ecc,
> +	.free = pn26g0xa_ooblayout_free,
> +};
> +
> +
> +static const struct spinand_info paragon_spinand_table[] = {
> +	SPINAND_INFO("PN26G01A", 0xe1,
> +		     NAND_MEMORG(1, 2048, 128, 64, 1024, 21, 1, 1, 1),
> +		     NAND_ECCREQ(8, 512),
> +		     SPINAND_INFO_OP_VARIANTS(&read_cache_variants,
> +					      &write_cache_variants,
> +					      &update_cache_variants),
> +		     0,
> +		     SPINAND_ECCINFO(&pn26g0xa_ooblayout,
> +				     pn26g0xa_ecc_get_status)),
> +	SPINAND_INFO("PN26G02A", 0xe2,
> +		     NAND_MEMORG(1, 2048, 128, 64, 1024, 21, 1, 1, 1),

I think this is the 2G type and it should have twice as much blocks as 
the 1G type, so 2048 instead of 1024. And the maximum number of bad 
blocks should be 41 instead of 21.

> +		     NAND_ECCREQ(8, 512),
> +		     SPINAND_INFO_OP_VARIANTS(&read_cache_variants,
> +					      &write_cache_variants,
> +					      &update_cache_variants),
> +		     0,
> +		     SPINAND_ECCINFO(&pn26g0xa_ooblayout,
> +				     pn26g0xa_ecc_get_status)),
> +};
> +
> +static int paragon_spinand_detect(struct spinand_device *spinand)
> +{
> +	u8 *id = spinand->id.data;
> +	int ret;
> +
> +	/* Read ID returns [0][MID][DID] */
> +
> +	if (id[1] != SPINAND_MFR_PARAGON)
> +		return 0;
> +
> +	ret = spinand_match_and_init(spinand, paragon_spinand_table,
> +				     ARRAY_SIZE(paragon_spinand_table),
> +				     id[2]);
> +	if (ret)
> +		return ret;
> +
> +	return 1;
> +}
> +
> +static const struct spinand_manufacturer_ops paragon_spinand_manuf_ops = {
> +	.detect = paragon_spinand_detect,
> +};
> +
> +const struct spinand_manufacturer paragon_spinand_manufacturer = {
> +	.id = SPINAND_MFR_PARAGON,
> +	.name = "Paragon",
> +	.ops = &paragon_spinand_manuf_ops,
> +};
> diff --git a/include/linux/mtd/spinand.h b/include/linux/mtd/spinand.h
> index fbc0423bb4ae..4ea558bd3c46 100644
> --- a/include/linux/mtd/spinand.h
> +++ b/include/linux/mtd/spinand.h
> @@ -227,6 +227,7 @@ struct spinand_manufacturer {
>   extern const struct spinand_manufacturer gigadevice_spinand_manufacturer;
>   extern const struct spinand_manufacturer macronix_spinand_manufacturer;
>   extern const struct spinand_manufacturer micron_spinand_manufacturer;
> +extern const struct spinand_manufacturer paragon_spinand_manufacturer;
>   extern const struct spinand_manufacturer toshiba_spinand_manufacturer;
>   extern const struct spinand_manufacturer winbond_spinand_manufacturer;
>   
> 
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/