public inbox for linux-mtd@lists.infradead.org
 help / color / mirror / Atom feed
* [PATCH] mtd: spinand: Add support for XTX XT26xxxDxxxxx
@ 2023-10-07  9:09 Bruce Suen
  2023-10-09 12:58 ` Miquel Raynal
  0 siblings, 1 reply; 4+ messages in thread
From: Bruce Suen @ 2023-10-07  9:09 UTC (permalink / raw)
  To: linux-mtd; +Cc: miquel.raynal, richard, vigneshr, Bruce Suen

Add Support XTX Technology XT26G01DXXXXX, XT26G11DXXXXX, XT26Q01DXXXXX,
XT26G02DXXXXX, XT26G12DXXXXX, XT26Q02DXXXXX, XT26G04DXXXXX, and
XT26Q04DXXXXX SPI NAND.

These are 3V/1.8V 1G/2G/4Gbit serial SLC NAND flash device with on-die
ECC(8bit strength per 512bytes).

Datasheet Links:
- http://www.xtxtech.com/download/?AId=458
- http://www.xtxtech.com/download/?AId=495

Signed-off-by: Bruce Suen <bruce_suen@163.com>
---
 drivers/mtd/nand/spi/xtx.c | 135 +++++++++++++++++++++++++++++++++++++
 1 file changed, 135 insertions(+)

diff --git a/drivers/mtd/nand/spi/xtx.c b/drivers/mtd/nand/spi/xtx.c
index 3911520f7..ddc69ef76 100644
--- a/drivers/mtd/nand/spi/xtx.c
+++ b/drivers/mtd/nand/spi/xtx.c
@@ -15,6 +15,13 @@
 #define XT26G0XA_STATUS_ECC_8_CORRECTED	(3 << 4)
 #define XT26G0XA_STATUS_ECC_UNCOR_ERROR	(2 << 4)
 
+#define XT26XXXD_STATUS_ECC_MASK	    GENMASK(7, 4)
+#define XT26XXXD_STATUS_ECC_NO_DETECTED     (0 << 4)
+#define XT26XXXD_STATUS_ECC_1_7_CORRECTED   (1 << 4)
+#define XT26XXXD_STATUS_ECC_8_CORRECTED     (3 << 4)
+#define XT26XXXD_STATUS_ECC_UNCOR_ERROR     (2 << 4)
+
 static SPINAND_OP_VARIANTS(read_cache_variants,
 		SPINAND_PAGE_READ_FROM_CACHE_QUADIO_OP(0, 1, NULL, 0),
 		SPINAND_PAGE_READ_FROM_CACHE_X4_OP(0, 1, NULL, 0),
@@ -84,6 +91,54 @@ static int xt26g0xa_ecc_get_status(struct spinand_device *spinand,
 	return status >> 2;
 }
 
+
+static int xt26xxxd_ooblayout_ecc(struct mtd_info *mtd, int section,
+				   struct mtd_oob_region *region)
+{
+	if (section)
+		return -ERANGE;
+
+	region->offset = mtd->oobsize / 2;
+	region->length = mtd->oobsize / 2;
+
+	return 0;
+}
+
+static int xt26xxxd_ooblayout_free(struct mtd_info *mtd, int section,
+				   struct mtd_oob_region *region)
+{
+	if (section)
+		return -ERANGE;
+
+	region->offset = 1;
+	region->length = mtd->oobsize / 2 - 1;
+
+	return 0;
+}
+
+static const struct mtd_ooblayout_ops xt26xxxd_ooblayout = {
+	.ecc = xt26xxxd_ooblayout_ecc,
+	.free = xt26xxxd_ooblayout_free,
+};
+
+static int xt26xxxd_ecc_get_status(struct spinand_device *spinand,
+					 u8 status)
+{
+	switch (status & STATUS_ECC_MASK) {
+	case XT26XXXD_STATUS_ECC_NO_DETECTED:
+		return 0;
+	case XT26XXXD_STATUS_ECC_UNCOR_ERROR:
+		return -EBADMSG;
+	case XT26XXXD_STATUS_ECC_1_7_CORRECTED:
+		return 4+((status & XT26XXXD_STATUS_ECC_MASK) >> 6);
+	case XT26XXXD_STATUS_ECC_8_CORRECTED:
+		return 8;
+	default:
+		break;
+	}
+
+	return -EINVAL;
+}
 static const struct spinand_info xtx_spinand_table[] = {
 	SPINAND_INFO("XT26G01A",
 		     SPINAND_ID(SPINAND_READID_METHOD_OPCODE_ADDR, 0xE1),
@@ -115,6 +170,86 @@ static const struct spinand_info xtx_spinand_table[] = {
 		     SPINAND_HAS_QE_BIT,
 		     SPINAND_ECCINFO(&xt26g0xa_ooblayout,
 				     xt26g0xa_ecc_get_status)),
+	SPINAND_INFO("XT26G01D",
+		     SPINAND_ID(SPINAND_READID_METHOD_OPCODE_ADDR, 0x31),
+		     NAND_MEMORG(1, 2048, 128, 64, 1024, 20, 1, 1, 1),
+		     NAND_ECCREQ(8, 512),
+		     SPINAND_INFO_OP_VARIANTS(&read_cache_variants,
+					      &write_cache_variants,
+					      &update_cache_variants),
+		     0,
+		     SPINAND_ECCINFO(&xt26xxxd_ooblayout,
+				     xt26xxxd_ecc_get_status)),
+	SPINAND_INFO("XT26G11D",
+		     SPINAND_ID(SPINAND_READID_METHOD_OPCODE_ADDR, 0x34),
+		     NAND_MEMORG(1, 2048, 128, 64, 1024, 20, 1, 1, 1),
+		     NAND_ECCREQ(8, 512),
+		     SPINAND_INFO_OP_VARIANTS(&read_cache_variants,
+					      &write_cache_variants,
+					      &update_cache_variants),
+		     SPINAND_HAS_QE_BIT,
+		     SPINAND_ECCINFO(&xt26xxxd_ooblayout,
+				     xt26xxxd_ecc_get_status)),
+	SPINAND_INFO("XT26Q01D",
+		     SPINAND_ID(SPINAND_READID_METHOD_OPCODE_ADDR, 0x51),
+		     NAND_MEMORG(1, 2048, 128, 64, 1024, 20, 1, 1, 1),
+		     NAND_ECCREQ(8, 512),
+		     SPINAND_INFO_OP_VARIANTS(&read_cache_variants,
+					      &write_cache_variants,
+					      &update_cache_variants),
+		     0,
+		     SPINAND_ECCINFO(&xt26xxxd_ooblayout,
+				     xt26xxxd_ecc_get_status)),
+	SPINAND_INFO("XT26G02D",
+		     SPINAND_ID(SPINAND_READID_METHOD_OPCODE_ADDR, 0x32),
+		     NAND_MEMORG(1, 2048, 128, 64, 2048, 40, 1, 1, 1),
+		     NAND_ECCREQ(8, 512),
+		     SPINAND_INFO_OP_VARIANTS(&read_cache_variants,
+					      &write_cache_variants,
+					      &update_cache_variants),
+		     0,
+		     SPINAND_ECCINFO(&xt26xxxd_ooblayout,
+				     xt26xxxd_ecc_get_status)),
+	SPINAND_INFO("XT26G12D",
+		     SPINAND_ID(SPINAND_READID_METHOD_OPCODE_ADDR, 0x35),
+		     NAND_MEMORG(1, 2048, 128, 64, 2048, 40, 1, 1, 1),
+		     NAND_ECCREQ(8, 512),
+		     SPINAND_INFO_OP_VARIANTS(&read_cache_variants,
+					      &write_cache_variants,
+					      &update_cache_variants),
+		     SPINAND_HAS_QE_BIT,
+		     SPINAND_ECCINFO(&xt26xxxd_ooblayout,
+				     xt26xxxd_ecc_get_status)),
+	SPINAND_INFO("XT26Q02D",
+		     SPINAND_ID(SPINAND_READID_METHOD_OPCODE_ADDR, 0x52),
+		     NAND_MEMORG(1, 2048, 128, 64, 2048, 40, 1, 1, 1),
+		     NAND_ECCREQ(8, 512),
+		     SPINAND_INFO_OP_VARIANTS(&read_cache_variants,
+					      &write_cache_variants,
+					      &update_cache_variants),
+		     0,
+		     SPINAND_ECCINFO(&xt26xxxd_ooblayout,
+				     xt26xxxd_ecc_get_status)),
+	SPINAND_INFO("XT26G04D",
+		     SPINAND_ID(SPINAND_READID_METHOD_OPCODE_ADDR, 0x33),
+		     NAND_MEMORG(1, 4096, 256, 64, 2048, 40, 1, 1, 1),
+		     NAND_ECCREQ(8, 512),
+		     SPINAND_INFO_OP_VARIANTS(&read_cache_variants,
+					      &write_cache_variants,
+					      &update_cache_variants),
+		     0,
+		     SPINAND_ECCINFO(&xt26xxxd_ooblayout,
+				     xt26xxxd_ecc_get_status)),
+	SPINAND_INFO("XT26Q04D",
+		     SPINAND_ID(SPINAND_READID_METHOD_OPCODE_ADDR, 0x53),
+		     NAND_MEMORG(1, 4096, 256, 64, 2048, 40, 1, 1, 1),
+		     NAND_ECCREQ(8, 512),
+		     SPINAND_INFO_OP_VARIANTS(&read_cache_variants,
+					      &write_cache_variants,
+					      &update_cache_variants),
+		     0,
+		     SPINAND_ECCINFO(&xt26xxxd_ooblayout,
+				     xt26xxxd_ecc_get_status)),
 };
 
 static const struct spinand_manufacturer_ops xtx_spinand_manuf_ops = {
-- 
2.34.1


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

^ permalink raw reply related	[flat|nested] 4+ messages in thread
* Re: [PATCH] mtd: spinand: Add support for XTX XT26xxxDxxxxx
  2023-10-07  9:09 [PATCH] mtd: spinand: Add support for XTX XT26xxxDxxxxx Bruce Suen
@ 2023-10-09 12:58 ` Miquel Raynal
  2023-10-11  3:09   ` Bruce Suen
  0 siblings, 1 reply; 4+ messages in thread
From: Miquel Raynal @ 2023-10-09 12:58 UTC (permalink / raw)
  To: Bruce Suen; +Cc: linux-mtd, richard, vigneshr

Hi Bruce,

bruce_suen@163.com wrote on Sat,  7 Oct 2023 05:09:54 -0400:

> Add Support XTX Technology XT26G01DXXXXX, XT26G11DXXXXX, XT26Q01DXXXXX,
> XT26G02DXXXXX, XT26G12DXXXXX, XT26Q02DXXXXX, XT26G04DXXXXX, and
> XT26Q04DXXXXX SPI NAND.
> 
> These are 3V/1.8V 1G/2G/4Gbit serial SLC NAND flash device with on-die
> ECC(8bit strength per 512bytes).
> 
> Datasheet Links:
> - http://www.xtxtech.com/download/?AId=458
> - http://www.xtxtech.com/download/?AId=495
> 
> Signed-off-by: Bruce Suen <bruce_suen@163.com>
> ---
>  drivers/mtd/nand/spi/xtx.c | 135 +++++++++++++++++++++++++++++++++++++
>  1 file changed, 135 insertions(+)
> 
> diff --git a/drivers/mtd/nand/spi/xtx.c b/drivers/mtd/nand/spi/xtx.c
> index 3911520f7..ddc69ef76 100644
> --- a/drivers/mtd/nand/spi/xtx.c
> +++ b/drivers/mtd/nand/spi/xtx.c
> @@ -15,6 +15,13 @@
>  #define XT26G0XA_STATUS_ECC_8_CORRECTED	(3 << 4)
>  #define XT26G0XA_STATUS_ECC_UNCOR_ERROR	(2 << 4)
>  
> +#define XT26XXXD_STATUS_ECC_MASK	    GENMASK(7, 4)
> +#define XT26XXXD_STATUS_ECC_NO_DETECTED     (0 << 4)
> +#define XT26XXXD_STATUS_ECC_1_7_CORRECTED   (1 << 4)
> +#define XT26XXXD_STATUS_ECC_8_CORRECTED     (3 << 4)
> +#define XT26XXXD_STATUS_ECC_UNCOR_ERROR     (2 << 4)
> +
>  static SPINAND_OP_VARIANTS(read_cache_variants,
>  		SPINAND_PAGE_READ_FROM_CACHE_QUADIO_OP(0, 1, NULL, 0),
>  		SPINAND_PAGE_READ_FROM_CACHE_X4_OP(0, 1, NULL, 0),
> @@ -84,6 +91,54 @@ static int xt26g0xa_ecc_get_status(struct spinand_device *spinand,
>  	return status >> 2;
>  }
>  
> +
> +static int xt26xxxd_ooblayout_ecc(struct mtd_info *mtd, int section,
> +				   struct mtd_oob_region *region)
> +{
> +	if (section)
> +		return -ERANGE;
> +
> +	region->offset = mtd->oobsize / 2;
> +	region->length = mtd->oobsize / 2;

This is strange, are you sure this is always the case? Isn't the offset
related to the number of bytes used by the ECC engine strength rather
than a position in oobsize?
 
> +
> +	return 0;
> +}
> +
> +static int xt26xxxd_ooblayout_free(struct mtd_info *mtd, int section,
> +				   struct mtd_oob_region *region)
> +{
> +	if (section)
> +		return -ERANGE;
> +
> +	region->offset = 1;

Please reserve 2 bytes for the BBM.

> +	region->length = mtd->oobsize / 2 - 1;

"- 2"

> +
> +	return 0;
> +}
> +
> +static const struct mtd_ooblayout_ops xt26xxxd_ooblayout = {
> +	.ecc = xt26xxxd_ooblayout_ecc,
> +	.free = xt26xxxd_ooblayout_free,
> +};
> +
> +static int xt26xxxd_ecc_get_status(struct spinand_device *spinand,
> +					 u8 status)
> +{
> +	switch (status & STATUS_ECC_MASK) {
> +	case XT26XXXD_STATUS_ECC_NO_DETECTED:
> +		return 0;
> +	case XT26XXXD_STATUS_ECC_UNCOR_ERROR:
> +		return -EBADMSG;
> +	case XT26XXXD_STATUS_ECC_1_7_CORRECTED:
> +		return 4+((status & XT26XXXD_STATUS_ECC_MASK) >> 6);

Can you justify the calculation?

Also please run checkpatch.pl --strict.

> +	case XT26XXXD_STATUS_ECC_8_CORRECTED:
> +		return 8;
> +	default:
> +		break;
> +	}
> +
> +	return -EINVAL;
> +}
>  static const struct spinand_info xtx_spinand_table[] = {
>  	SPINAND_INFO("XT26G01A",
>  		     SPINAND_ID(SPINAND_READID_METHOD_OPCODE_ADDR, 0xE1),
> @@ -115,6 +170,86 @@ static const struct spinand_info xtx_spinand_table[] = {
>  		     SPINAND_HAS_QE_BIT,
>  		     SPINAND_ECCINFO(&xt26g0xa_ooblayout,
>  				     xt26g0xa_ecc_get_status)),
> +	SPINAND_INFO("XT26G01D",
> +		     SPINAND_ID(SPINAND_READID_METHOD_OPCODE_ADDR, 0x31),
> +		     NAND_MEMORG(1, 2048, 128, 64, 1024, 20, 1, 1, 1),
> +		     NAND_ECCREQ(8, 512),
> +		     SPINAND_INFO_OP_VARIANTS(&read_cache_variants,
> +					      &write_cache_variants,
> +					      &update_cache_variants),
> +		     0,
> +		     SPINAND_ECCINFO(&xt26xxxd_ooblayout,
> +				     xt26xxxd_ecc_get_status)),
> +	SPINAND_INFO("XT26G11D",
> +		     SPINAND_ID(SPINAND_READID_METHOD_OPCODE_ADDR, 0x34),
> +		     NAND_MEMORG(1, 2048, 128, 64, 1024, 20, 1, 1, 1),
> +		     NAND_ECCREQ(8, 512),
> +		     SPINAND_INFO_OP_VARIANTS(&read_cache_variants,
> +					      &write_cache_variants,
> +					      &update_cache_variants),
> +		     SPINAND_HAS_QE_BIT,
> +		     SPINAND_ECCINFO(&xt26xxxd_ooblayout,
> +				     xt26xxxd_ecc_get_status)),
> +	SPINAND_INFO("XT26Q01D",
> +		     SPINAND_ID(SPINAND_READID_METHOD_OPCODE_ADDR, 0x51),
> +		     NAND_MEMORG(1, 2048, 128, 64, 1024, 20, 1, 1, 1),
> +		     NAND_ECCREQ(8, 512),
> +		     SPINAND_INFO_OP_VARIANTS(&read_cache_variants,
> +					      &write_cache_variants,
> +					      &update_cache_variants),
> +		     0,
> +		     SPINAND_ECCINFO(&xt26xxxd_ooblayout,
> +				     xt26xxxd_ecc_get_status)),
> +	SPINAND_INFO("XT26G02D",
> +		     SPINAND_ID(SPINAND_READID_METHOD_OPCODE_ADDR, 0x32),
> +		     NAND_MEMORG(1, 2048, 128, 64, 2048, 40, 1, 1, 1),
> +		     NAND_ECCREQ(8, 512),
> +		     SPINAND_INFO_OP_VARIANTS(&read_cache_variants,
> +					      &write_cache_variants,
> +					      &update_cache_variants),
> +		     0,
> +		     SPINAND_ECCINFO(&xt26xxxd_ooblayout,
> +				     xt26xxxd_ecc_get_status)),
> +	SPINAND_INFO("XT26G12D",
> +		     SPINAND_ID(SPINAND_READID_METHOD_OPCODE_ADDR, 0x35),
> +		     NAND_MEMORG(1, 2048, 128, 64, 2048, 40, 1, 1, 1),
> +		     NAND_ECCREQ(8, 512),
> +		     SPINAND_INFO_OP_VARIANTS(&read_cache_variants,
> +					      &write_cache_variants,
> +					      &update_cache_variants),
> +		     SPINAND_HAS_QE_BIT,
> +		     SPINAND_ECCINFO(&xt26xxxd_ooblayout,
> +				     xt26xxxd_ecc_get_status)),
> +	SPINAND_INFO("XT26Q02D",
> +		     SPINAND_ID(SPINAND_READID_METHOD_OPCODE_ADDR, 0x52),
> +		     NAND_MEMORG(1, 2048, 128, 64, 2048, 40, 1, 1, 1),
> +		     NAND_ECCREQ(8, 512),
> +		     SPINAND_INFO_OP_VARIANTS(&read_cache_variants,
> +					      &write_cache_variants,
> +					      &update_cache_variants),
> +		     0,
> +		     SPINAND_ECCINFO(&xt26xxxd_ooblayout,
> +				     xt26xxxd_ecc_get_status)),
> +	SPINAND_INFO("XT26G04D",
> +		     SPINAND_ID(SPINAND_READID_METHOD_OPCODE_ADDR, 0x33),
> +		     NAND_MEMORG(1, 4096, 256, 64, 2048, 40, 1, 1, 1),
> +		     NAND_ECCREQ(8, 512),
> +		     SPINAND_INFO_OP_VARIANTS(&read_cache_variants,
> +					      &write_cache_variants,
> +					      &update_cache_variants),
> +		     0,
> +		     SPINAND_ECCINFO(&xt26xxxd_ooblayout,
> +				     xt26xxxd_ecc_get_status)),
> +	SPINAND_INFO("XT26Q04D",
> +		     SPINAND_ID(SPINAND_READID_METHOD_OPCODE_ADDR, 0x53),
> +		     NAND_MEMORG(1, 4096, 256, 64, 2048, 40, 1, 1, 1),
> +		     NAND_ECCREQ(8, 512),
> +		     SPINAND_INFO_OP_VARIANTS(&read_cache_variants,
> +					      &write_cache_variants,
> +					      &update_cache_variants),
> +		     0,
> +		     SPINAND_ECCINFO(&xt26xxxd_ooblayout,
> +				     xt26xxxd_ecc_get_status)),
>  };
>  
>  static const struct spinand_manufacturer_ops xtx_spinand_manuf_ops = {


Thanks,
Miquèl

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

^ permalink raw reply	[flat|nested] 4+ messages in thread
* Re: [PATCH] mtd: spinand: Add support for XTX XT26xxxDxxxxx
  2023-10-09 12:58 ` Miquel Raynal
@ 2023-10-11  3:09   ` Bruce Suen
  2023-10-11  7:09     ` Miquel Raynal
  0 siblings, 1 reply; 4+ messages in thread
From: Bruce Suen @ 2023-10-11  3:09 UTC (permalink / raw)
  To: Miquel Raynal; +Cc: linux-mtd, richard, vigneshr

Hi Miquel,

Thanks for your review.

On 10/9/23 08:58, Miquel Raynal wrote:
> Hi Bruce,
>
> bruce_suen@163.com wrote on Sat,  7 Oct 2023 05:09:54 -0400:
>
>> Add Support XTX Technology XT26G01DXXXXX, XT26G11DXXXXX, XT26Q01DXXXXX,
>> XT26G02DXXXXX, XT26G12DXXXXX, XT26Q02DXXXXX, XT26G04DXXXXX, and
>> XT26Q04DXXXXX SPI NAND.
>>
>> These are 3V/1.8V 1G/2G/4Gbit serial SLC NAND flash device with on-die
>> ECC(8bit strength per 512bytes).
>>
>> Datasheet Links:
>> - http://www.xtxtech.com/download/?AId=458
>> - http://www.xtxtech.com/download/?AId=495
>>
>> Signed-off-by: Bruce Suen <bruce_suen@163.com>
>> ---
>>   drivers/mtd/nand/spi/xtx.c | 135 +++++++++++++++++++++++++++++++++++++
>>   1 file changed, 135 insertions(+)
>>
>> diff --git a/drivers/mtd/nand/spi/xtx.c b/drivers/mtd/nand/spi/xtx.c
>> index 3911520f7..ddc69ef76 100644
>> --- a/drivers/mtd/nand/spi/xtx.c
>> +++ b/drivers/mtd/nand/spi/xtx.c
>> @@ -15,6 +15,13 @@
>>   #define XT26G0XA_STATUS_ECC_8_CORRECTED	(3 << 4)
>>   #define XT26G0XA_STATUS_ECC_UNCOR_ERROR	(2 << 4)
>>   
>> +#define XT26XXXD_STATUS_ECC_MASK	    GENMASK(7, 4)
>> +#define XT26XXXD_STATUS_ECC_NO_DETECTED     (0 << 4)
>> +#define XT26XXXD_STATUS_ECC_1_7_CORRECTED   (1 << 4)
>> +#define XT26XXXD_STATUS_ECC_8_CORRECTED     (3 << 4)
>> +#define XT26XXXD_STATUS_ECC_UNCOR_ERROR     (2 << 4)
>> +
>>   static SPINAND_OP_VARIANTS(read_cache_variants,
>>   		SPINAND_PAGE_READ_FROM_CACHE_QUADIO_OP(0, 1, NULL, 0),
>>   		SPINAND_PAGE_READ_FROM_CACHE_X4_OP(0, 1, NULL, 0),
>> @@ -84,6 +91,54 @@ static int xt26g0xa_ecc_get_status(struct spinand_device *spinand,
>>   	return status >> 2;
>>   }
>>   
>> +
>> +static int xt26xxxd_ooblayout_ecc(struct mtd_info *mtd, int section,
>> +				   struct mtd_oob_region *region)
>> +{
>> +	if (section)
>> +		return -ERANGE;
>> +
>> +	region->offset = mtd->oobsize / 2;
>> +	region->length = mtd->oobsize / 2;
> This is strange, are you sure this is always the case? Isn't the offset
> related to the number of bytes used by the ECC engine strength rather
> than a position in oobsize?

I'm sure for this,for XT26XXXD, the half of oob cannot be used by user 
although ecc engine does not use up half of the oob.

>   
>> +
>> +	return 0;
>> +}
>> +
>> +static int xt26xxxd_ooblayout_free(struct mtd_info *mtd, int section,
>> +				   struct mtd_oob_region *region)
>> +{
>> +	if (section)
>> +		return -ERANGE;
>> +
>> +	region->offset = 1;
> Please reserve 2 bytes for the BBM.
I'm confused about this.I see some drivers reserve 1 byte for BBM, some 
drivers reserve 2 bytes for BBM.
I can reserve 2 bytes for the BBM in next version of patch.
>
>> +	region->length = mtd->oobsize / 2 - 1;
> "- 2"
>
>> +
>> +	return 0;
>> +}
>> +
>> +static const struct mtd_ooblayout_ops xt26xxxd_ooblayout = {
>> +	.ecc = xt26xxxd_ooblayout_ecc,
>> +	.free = xt26xxxd_ooblayout_free,
>> +};
>> +
>> +static int xt26xxxd_ecc_get_status(struct spinand_device *spinand,
>> +					 u8 status)
>> +{
>> +	switch (status & STATUS_ECC_MASK) {
>> +	case XT26XXXD_STATUS_ECC_NO_DETECTED:
>> +		return 0;
>> +	case XT26XXXD_STATUS_ECC_UNCOR_ERROR:
>> +		return -EBADMSG;
>> +	case XT26XXXD_STATUS_ECC_1_7_CORRECTED:
>> +		return 4+((status & XT26XXXD_STATUS_ECC_MASK) >> 6);
> Can you justify the calculation?
AS described on page 42 of datasheet:

"ECCS3/ECCS2/ECCS1/ECCS0 provides ECC Status as follows:
XX00b= No bit errors were detected during the previous read algorithm.
0001b= Bit errors(≤4) were detected and corrected.
0101b= Bit errors(=5) were detected and corrected.
1001b= Bit errors(=6) were detected and corrected.
1101b= Bit errors(=7) were detected and corrected.
XX10b= Bit errors greater than ECC capability (8 bits) and not corrected
XX11b= Bit errors reach ECC capability (8 bits) and corrected"

ECCS1,ECCS0 = 00: No bit errors
ECCS1,ECCS0 = 10: bit errors > 8
ECCS1,ECCS0 = 11: bit errors = 8
ECCS1,ECCS0 = 01: 1<=bit errors<=7

so I use 4 cases to represent it.

for ECCS1,ECCS0 = 11, bitfilps can be subdivided by ECCS3 ECCS2:

ECCS3 ECCS2 = 00: bitfip<=4=4+0, I return 4 for this;
ECCS3 ECCS2 = 01: bitfip=5=4+1, I return 5 for this;
ECCS3 ECCS2 = 10: bitfip=6=4+2, I return 6 for this;
ECCS3 ECCS2 = 11: bitfip=7=4+3, I return 7 for this;

so,I use "4+((status & XT26XXXD_STATUS_ECC_MASK) >> 6)" to represent it.

>
> Also please run checkpatch.pl --strict.
I run checkpatch.pl without --strict,there are no checks.
I run checkpatch.pl --strict,there are 5 checks.
I'm sorry for that,I will fix it in next version of patch.
>> +	case XT26XXXD_STATUS_ECC_8_CORRECTED:
>> +		return 8;
>> +	default:
>> +		break;
>> +	}
>> +
>> +	return -EINVAL;
>> +}
>>   static const struct spinand_info xtx_spinand_table[] = {
>>   	SPINAND_INFO("XT26G01A",
>>   		     SPINAND_ID(SPINAND_READID_METHOD_OPCODE_ADDR, 0xE1),
>> @@ -115,6 +170,86 @@ static const struct spinand_info xtx_spinand_table[] = {
>>   		     SPINAND_HAS_QE_BIT,
>>   		     SPINAND_ECCINFO(&xt26g0xa_ooblayout,
>>   				     xt26g0xa_ecc_get_status)),
>> +	SPINAND_INFO("XT26G01D",
>> +		     SPINAND_ID(SPINAND_READID_METHOD_OPCODE_ADDR, 0x31),
>> +		     NAND_MEMORG(1, 2048, 128, 64, 1024, 20, 1, 1, 1),
>> +		     NAND_ECCREQ(8, 512),
>> +		     SPINAND_INFO_OP_VARIANTS(&read_cache_variants,
>> +					      &write_cache_variants,
>> +					      &update_cache_variants),
>> +		     0,
>> +		     SPINAND_ECCINFO(&xt26xxxd_ooblayout,
>> +				     xt26xxxd_ecc_get_status)),
>> +	SPINAND_INFO("XT26G11D",
>> +		     SPINAND_ID(SPINAND_READID_METHOD_OPCODE_ADDR, 0x34),
>> +		     NAND_MEMORG(1, 2048, 128, 64, 1024, 20, 1, 1, 1),
>> +		     NAND_ECCREQ(8, 512),
>> +		     SPINAND_INFO_OP_VARIANTS(&read_cache_variants,
>> +					      &write_cache_variants,
>> +					      &update_cache_variants),
>> +		     SPINAND_HAS_QE_BIT,
>> +		     SPINAND_ECCINFO(&xt26xxxd_ooblayout,
>> +				     xt26xxxd_ecc_get_status)),
>> +	SPINAND_INFO("XT26Q01D",
>> +		     SPINAND_ID(SPINAND_READID_METHOD_OPCODE_ADDR, 0x51),
>> +		     NAND_MEMORG(1, 2048, 128, 64, 1024, 20, 1, 1, 1),
>> +		     NAND_ECCREQ(8, 512),
>> +		     SPINAND_INFO_OP_VARIANTS(&read_cache_variants,
>> +					      &write_cache_variants,
>> +					      &update_cache_variants),
>> +		     0,
>> +		     SPINAND_ECCINFO(&xt26xxxd_ooblayout,
>> +				     xt26xxxd_ecc_get_status)),
>> +	SPINAND_INFO("XT26G02D",
>> +		     SPINAND_ID(SPINAND_READID_METHOD_OPCODE_ADDR, 0x32),
>> +		     NAND_MEMORG(1, 2048, 128, 64, 2048, 40, 1, 1, 1),
>> +		     NAND_ECCREQ(8, 512),
>> +		     SPINAND_INFO_OP_VARIANTS(&read_cache_variants,
>> +					      &write_cache_variants,
>> +					      &update_cache_variants),
>> +		     0,
>> +		     SPINAND_ECCINFO(&xt26xxxd_ooblayout,
>> +				     xt26xxxd_ecc_get_status)),
>> +	SPINAND_INFO("XT26G12D",
>> +		     SPINAND_ID(SPINAND_READID_METHOD_OPCODE_ADDR, 0x35),
>> +		     NAND_MEMORG(1, 2048, 128, 64, 2048, 40, 1, 1, 1),
>> +		     NAND_ECCREQ(8, 512),
>> +		     SPINAND_INFO_OP_VARIANTS(&read_cache_variants,
>> +					      &write_cache_variants,
>> +					      &update_cache_variants),
>> +		     SPINAND_HAS_QE_BIT,
>> +		     SPINAND_ECCINFO(&xt26xxxd_ooblayout,
>> +				     xt26xxxd_ecc_get_status)),
>> +	SPINAND_INFO("XT26Q02D",
>> +		     SPINAND_ID(SPINAND_READID_METHOD_OPCODE_ADDR, 0x52),
>> +		     NAND_MEMORG(1, 2048, 128, 64, 2048, 40, 1, 1, 1),
>> +		     NAND_ECCREQ(8, 512),
>> +		     SPINAND_INFO_OP_VARIANTS(&read_cache_variants,
>> +					      &write_cache_variants,
>> +					      &update_cache_variants),
>> +		     0,
>> +		     SPINAND_ECCINFO(&xt26xxxd_ooblayout,
>> +				     xt26xxxd_ecc_get_status)),
>> +	SPINAND_INFO("XT26G04D",
>> +		     SPINAND_ID(SPINAND_READID_METHOD_OPCODE_ADDR, 0x33),
>> +		     NAND_MEMORG(1, 4096, 256, 64, 2048, 40, 1, 1, 1),
>> +		     NAND_ECCREQ(8, 512),
>> +		     SPINAND_INFO_OP_VARIANTS(&read_cache_variants,
>> +					      &write_cache_variants,
>> +					      &update_cache_variants),
>> +		     0,
>> +		     SPINAND_ECCINFO(&xt26xxxd_ooblayout,
>> +				     xt26xxxd_ecc_get_status)),
>> +	SPINAND_INFO("XT26Q04D",
>> +		     SPINAND_ID(SPINAND_READID_METHOD_OPCODE_ADDR, 0x53),
>> +		     NAND_MEMORG(1, 4096, 256, 64, 2048, 40, 1, 1, 1),
>> +		     NAND_ECCREQ(8, 512),
>> +		     SPINAND_INFO_OP_VARIANTS(&read_cache_variants,
>> +					      &write_cache_variants,
>> +					      &update_cache_variants),
>> +		     0,
>> +		     SPINAND_ECCINFO(&xt26xxxd_ooblayout,
>> +				     xt26xxxd_ecc_get_status)),
>>   };
>>   
>>   static const struct spinand_manufacturer_ops xtx_spinand_manuf_ops = {
>
> Thanks,
> Miquèl


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

^ permalink raw reply	[flat|nested] 4+ messages in thread
* Re: [PATCH] mtd: spinand: Add support for XTX XT26xxxDxxxxx
  2023-10-11  3:09   ` Bruce Suen
@ 2023-10-11  7:09     ` Miquel Raynal
  0 siblings, 0 replies; 4+ messages in thread
From: Miquel Raynal @ 2023-10-11  7:09 UTC (permalink / raw)
  To: Bruce Suen; +Cc: linux-mtd, richard, vigneshr

Hi Bruce,

> "ECCS3/ECCS2/ECCS1/ECCS0 provides ECC Status as follows:
> XX00b= No bit errors were detected during the previous read algorithm.
> 0001b= Bit errors(≤4) were detected and corrected.
> 0101b= Bit errors(=5) were detected and corrected.
> 1001b= Bit errors(=6) were detected and corrected.
> 1101b= Bit errors(=7) were detected and corrected.
> XX10b= Bit errors greater than ECC capability (8 bits) and not corrected
> XX11b= Bit errors reach ECC capability (8 bits) and corrected"

Nice

> 
> ECCS1,ECCS0 = 00: No bit errors
> ECCS1,ECCS0 = 10: bit errors > 8
> ECCS1,ECCS0 = 11: bit errors = 8
> ECCS1,ECCS0 = 01: 1<=bit errors<=7
> 
> so I use 4 cases to represent it.
> 
> for ECCS1,ECCS0 = 11, bitfilps can be subdivided by ECCS3 ECCS2:
> 
> ECCS3 ECCS2 = 00: bitfip<=4=4+0, I return 4 for this;
> ECCS3 ECCS2 = 01: bitfip=5=4+1, I return 5 for this;
> ECCS3 ECCS2 = 10: bitfip=6=4+2, I return 6 for this;
> ECCS3 ECCS2 = 11: bitfip=7=4+3, I return 7 for this;

Please use FIELD_GET() and create macros to clarify what you are
looking at. Please include all values.

> so,I use "4+((status & XT26XXXD_STATUS_ECC_MASK) >> 6)" to represent it.
> 
> >
> > Also please run checkpatch.pl --strict.  
> I run checkpatch.pl without --strict,there are no checks.
> I run checkpatch.pl --strict,there are 5 checks.
> I'm sorry for that,I will fix it in next version of patch.
> >> +	case XT26XXXD_STATUS_ECC_8_CORRECTED:
> >> +		return 8;
> >> +	default:
> >> +		break;
> >> +	}
> >> +
> >> +	return -EINVAL;
> >> +}

Thanks,
Miquèl

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

^ permalink raw reply	[flat|nested] 4+ messages in thread
end of thread, other threads:[~2023-10-11  7:10 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-10-07  9:09 [PATCH] mtd: spinand: Add support for XTX XT26xxxDxxxxx Bruce Suen
2023-10-09 12:58 ` Miquel Raynal
2023-10-11  3:09   ` Bruce Suen
2023-10-11  7:09     ` Miquel Raynal

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox