[PATCH v2 0/2] mtd: spinand: micron: correct parameters

[PATCH v2 0/2] mtd: spinand: micron: correct parameters

[Martin Kurbanov]

This patchset includes following fixes:
  1. Correct bitmask for ecc status.
  2. Fix oob layout: the first 4 bytes are reserved for bad block data.

Changes v2 since v1 at [1]:
  - Split into two individual patches.
  - Remove the fix for using only non-protected ECC bytes from OOB area.

Links:
  [1] https://lore.kernel.org/all/20230815161024.810729-1-mmkurbanov@sberdevices.ru/

Martin Kurbanov (2):
  mtd: spinand: micron: correct bitmask for ecc status
  mtd: spinand: micron: fixing the offset for OOB

 drivers/mtd/nand/spi/micron.c | 22 ++++++++++++++++++----
 1 file changed, 18 insertions(+), 4 deletions(-)

--
2.40.0


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

[PATCH v2 1/2] mtd: spinand: micron: correct bitmask for ecc status

[Martin Kurbanov]

Valid bitmask is 0x70 in the status register.

Signed-off-by: Martin Kurbanov <mmkurbanov@sberdevices.ru>
---
 drivers/mtd/nand/spi/micron.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mtd/nand/spi/micron.c b/drivers/mtd/nand/spi/micron.c
index 50b7295bc922..12601bc4227a 100644
--- a/drivers/mtd/nand/spi/micron.c
+++ b/drivers/mtd/nand/spi/micron.c
@@ -12,7 +12,7 @@
 
 #define SPINAND_MFR_MICRON		0x2c
 
-#define MICRON_STATUS_ECC_MASK		GENMASK(7, 4)
+#define MICRON_STATUS_ECC_MASK		GENMASK(6, 4)
 #define MICRON_STATUS_ECC_NO_BITFLIPS	(0 << 4)
 #define MICRON_STATUS_ECC_1TO3_BITFLIPS	(1 << 4)
 #define MICRON_STATUS_ECC_4TO6_BITFLIPS	(3 << 4)
-- 
2.40.0


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

[PATCH v2 2/2] mtd: spinand: micron: fixing the offset for OOB

[Martin Kurbanov]

The first 4 bytes are reserved for bad block data.

Signed-off-by: Martin Kurbanov <mmkurbanov@sberdevices.ru>
---
 drivers/mtd/nand/spi/micron.c | 20 +++++++++++++++++---
 1 file changed, 17 insertions(+), 3 deletions(-)

diff --git a/drivers/mtd/nand/spi/micron.c b/drivers/mtd/nand/spi/micron.c
index 12601bc4227a..64b41c7c9cce 100644
--- a/drivers/mtd/nand/spi/micron.c
+++ b/drivers/mtd/nand/spi/micron.c
@@ -57,6 +57,20 @@ static SPINAND_OP_VARIANTS(x1_write_cache_variants,
 static SPINAND_OP_VARIANTS(x1_update_cache_variants,
 			   SPINAND_PROG_LOAD(false, 0, NULL, 0));
 
+/*
+ * OOB spare area map (128 and 256 bytes)
+ *
+ *           +-----+-----------------+-------------------+---------------------+
+ *           | BBM |     Non ECC     |   ECC protected   |      ECC Area       |
+ *           |     | protected Area  |       Area        |                     |
+ * ----------+-----+-----------------+-------------------+---------------------+
+ *  oobsize  | 0:3 | 4:31 (28 bytes) | 32:63 (32 bytes)  | 64:127 (64 bytes)   |
+ * 128 bytes |     |                 |                   |                     |
+ * ----------+-----+-----------------+-------------------+---------------------+
+ *  oobsize  | 0:3 | 4:63 (60 bytes) | 64:127 (64 bytes) | 127:255 (128 bytes) |
+ * 256 bytes |     |                 |                   |                     |
+ * ----------+-----+-----------------+-------------------+---------------------+
+ */
 static int micron_8_ooblayout_ecc(struct mtd_info *mtd, int section,
 				  struct mtd_oob_region *region)
 {
@@ -75,9 +89,9 @@ static int micron_8_ooblayout_free(struct mtd_info *mtd, int section,
 	if (section)
 		return -ERANGE;
 
-	/* Reserve 2 bytes for the BBM. */
-	region->offset = 2;
-	region->length = (mtd->oobsize / 2) - 2;
+	/* Reserve 4 bytes for the BBM. */
+	region->offset = 4;
+	region->length = (mtd->oobsize / 2) - 4;
 
 	return 0;
 }
-- 
2.40.0


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

Re: [PATCH v2 1/2] mtd: spinand: micron: correct bitmask for ecc status

[Frieder Schrempf]

On 22.08.23 14:25, Martin Kurbanov wrote:
> Valid bitmask is 0x70 in the status register.
> 
> Signed-off-by: Martin Kurbanov <mmkurbanov@sberdevices.ru>

This should have a Fixes tag like:

Fixes: a508e8875e13 ("mtd: spinand: Add initial support for Micron
MT29F2G01ABAGD")

Reviewed-by: Frieder Schrempf <frieder.schrempf@kontron.de>

> ---
>  drivers/mtd/nand/spi/micron.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/mtd/nand/spi/micron.c b/drivers/mtd/nand/spi/micron.c
> index 50b7295bc922..12601bc4227a 100644
> --- a/drivers/mtd/nand/spi/micron.c
> +++ b/drivers/mtd/nand/spi/micron.c
> @@ -12,7 +12,7 @@
>  
>  #define SPINAND_MFR_MICRON		0x2c
>  
> -#define MICRON_STATUS_ECC_MASK		GENMASK(7, 4)
> +#define MICRON_STATUS_ECC_MASK		GENMASK(6, 4)
>  #define MICRON_STATUS_ECC_NO_BITFLIPS	(0 << 4)
>  #define MICRON_STATUS_ECC_1TO3_BITFLIPS	(1 << 4)
>  #define MICRON_STATUS_ECC_4TO6_BITFLIPS	(3 << 4)

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

Re: [PATCH v2 2/2] mtd: spinand: micron: fixing the offset for OOB

[Frieder Schrempf]

On 22.08.23 14:25, Martin Kurbanov wrote:
> The first 4 bytes are reserved for bad block data.
> 
> Signed-off-by: Martin Kurbanov <mmkurbanov@sberdevices.ru>

This should probably also have a Fixes tag like:

Fixes: a508e8875e13 ("mtd: spinand: Add initial support for Micron
MT29F2G01ABAGD")

Reviewed-by: Frieder Schrempf <frieder.schrempf@kontron.de>

> ---
>  drivers/mtd/nand/spi/micron.c | 20 +++++++++++++++++---
>  1 file changed, 17 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/mtd/nand/spi/micron.c b/drivers/mtd/nand/spi/micron.c
> index 12601bc4227a..64b41c7c9cce 100644
> --- a/drivers/mtd/nand/spi/micron.c
> +++ b/drivers/mtd/nand/spi/micron.c
> @@ -57,6 +57,20 @@ static SPINAND_OP_VARIANTS(x1_write_cache_variants,
>  static SPINAND_OP_VARIANTS(x1_update_cache_variants,
>  			   SPINAND_PROG_LOAD(false, 0, NULL, 0));
>  
> +/*
> + * OOB spare area map (128 and 256 bytes)
> + *
> + *           +-----+-----------------+-------------------+---------------------+
> + *           | BBM |     Non ECC     |   ECC protected   |      ECC Area       |
> + *           |     | protected Area  |       Area        |                     |
> + * ----------+-----+-----------------+-------------------+---------------------+
> + *  oobsize  | 0:3 | 4:31 (28 bytes) | 32:63 (32 bytes)  | 64:127 (64 bytes)   |
> + * 128 bytes |     |                 |                   |                     |
> + * ----------+-----+-----------------+-------------------+---------------------+
> + *  oobsize  | 0:3 | 4:63 (60 bytes) | 64:127 (64 bytes) | 127:255 (128 bytes) |
> + * 256 bytes |     |                 |                   |                     |
> + * ----------+-----+-----------------+-------------------+---------------------+
> + */
>  static int micron_8_ooblayout_ecc(struct mtd_info *mtd, int section,
>  				  struct mtd_oob_region *region)
>  {
> @@ -75,9 +89,9 @@ static int micron_8_ooblayout_free(struct mtd_info *mtd, int section,
>  	if (section)
>  		return -ERANGE;
>  
> -	/* Reserve 2 bytes for the BBM. */
> -	region->offset = 2;
> -	region->length = (mtd->oobsize / 2) - 2;
> +	/* Reserve 4 bytes for the BBM. */
> +	region->offset = 4;
> +	region->length = (mtd->oobsize / 2) - 4;
>  
>  	return 0;
>  }

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

Re: [PATCH v2 2/2] mtd: spinand: micron: fixing the offset for OOB

[Miquel Raynal]

Hi Martin,

mmkurbanov@sberdevices.ru wrote on Tue, 22 Aug 2023 15:25:34 +0300:

> The first 4 bytes are reserved for bad block data.

Are you sure about that? I've never seen 4-bytes BBM.

> Signed-off-by: Martin Kurbanov <mmkurbanov@sberdevices.ru>
> ---
>  drivers/mtd/nand/spi/micron.c | 20 +++++++++++++++++---
>  1 file changed, 17 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/mtd/nand/spi/micron.c b/drivers/mtd/nand/spi/micron.c
> index 12601bc4227a..64b41c7c9cce 100644
> --- a/drivers/mtd/nand/spi/micron.c
> +++ b/drivers/mtd/nand/spi/micron.c
> @@ -57,6 +57,20 @@ static SPINAND_OP_VARIANTS(x1_write_cache_variants,
>  static SPINAND_OP_VARIANTS(x1_update_cache_variants,
>  			   SPINAND_PROG_LOAD(false, 0, NULL, 0));
>  
> +/*
> + * OOB spare area map (128 and 256 bytes)
> + *
> + *           +-----+-----------------+-------------------+---------------------+
> + *           | BBM |     Non ECC     |   ECC protected   |      ECC Area       |
> + *           |     | protected Area  |       Area        |                     |
> + * ----------+-----+-----------------+-------------------+---------------------+
> + *  oobsize  | 0:3 | 4:31 (28 bytes) | 32:63 (32 bytes)  | 64:127 (64 bytes)   |
> + * 128 bytes |     |                 |                   |                     |
> + * ----------+-----+-----------------+-------------------+---------------------+
> + *  oobsize  | 0:3 | 4:63 (60 bytes) | 64:127 (64 bytes) | 127:255 (128 bytes) |
> + * 256 bytes |     |                 |                   |                     |
> + * ----------+-----+-----------------+-------------------+---------------------+
> + */
>  static int micron_8_ooblayout_ecc(struct mtd_info *mtd, int section,
>  				  struct mtd_oob_region *region)
>  {
> @@ -75,9 +89,9 @@ static int micron_8_ooblayout_free(struct mtd_info *mtd, int section,
>  	if (section)
>  		return -ERANGE;
>  
> -	/* Reserve 2 bytes for the BBM. */
> -	region->offset = 2;
> -	region->length = (mtd->oobsize / 2) - 2;
> +	/* Reserve 4 bytes for the BBM. */
> +	region->offset = 4;
> +	region->length = (mtd->oobsize / 2) - 4;
>  
>  	return 0;
>  }


Thanks,
Miquèl

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

Re: [PATCH v2 2/2] mtd: spinand: micron: fixing the offset for OOB

[Martin Kurbanov]

Hi Miquel,

On 22.08.2023 16:35, Miquel Raynal wrote:
> Hi Martin,
> 
> mmkurbanov@sberdevices.ru wrote on Tue, 22 Aug 2023 15:25:34 +0300:
> 
>> The first 4 bytes are reserved for bad block data.
> 
> Are you sure about that? I've never seen 4-bytes BBM.

Yes, I'm sure. I have checked in all the relevant datasheets:
----------------------------------------------------------------------------------
|        Name        | Max Byte | Min Byte |  Area   |        Description        |
|                    | Address  | Address  |         |                           |
|--------------------+----------+----------+---------+---------------------------|
| MT29F2G01ABAGD [1] |          |          |         |                           |
|--------------------|          |          |         |                           |
| MT29F2G01ABBGD [2] |          |          |         |                           |
|--------------------|   803h   |   800h   |         |                           |
| MT29F1G01ABAFD [3] |          |          |         |                           |
|--------------------|          |          |         |                           |
| MT29F4G01ADAGD [4] |          |          |         |                           |
|--------------------+----------+----------| Spare 0 | Reserved (bad block data) |
| MT29F4G01ABAFD [5] |          |          |         |                           |
|--------------------|          |          |         |                           |
| MT29F4G01ABBFD [6] |          |          |         |                           |
|--------------------|  1003h   |  1000h   |         |                           |
| MT29F8G01ADAFD [7] |          |          |         |                           |
|--------------------|          |          |         |                           |
| MT29F8G01ADBFD [8] |          |          |         |                           |
----------------------------------------------------------------------------------

Note: to view the datasheet on the Micron (https://www.micron.com/) website, you need
to register.

Links:
  [1] https://datasheet.lcsc.com/lcsc/1912111437_Micron-Tech-MT29F2G01ABAGDWB-IT-G_C410863.pdf - page 45
  [2] https://www.micron.com/-/media/client/global/documents/products/data-sheet/nand-flash/70-series/m79a_2gb_1_8v_nand_spi.pdf - page 46
  [3] https://datasheet.lcsc.com/lcsc/2209201030_Micron-Tech-MT29F1G01ABAFDWB-IT-F_C2905686.pdf - page 46
  [4] https://www.micron.com/-/media/client/global/documents/products/data-sheet/nand-flash/70-series/m79a_ddp_4gb_3v_nand_spi.pdf - page 46
  [5] https://www.micron.com/-/media/client/global/documents/products/data-sheet/nand-flash/70-series/m70a_4gb_3v_nand_spi.pdf - page 47
  [6] https://www.micron.com/-/media/client/global/documents/products/data-sheet/nand-flash/70-series/m70a_4gb_1_8v_nand_spi.pdf - page 49
  [7] https://www.micron.com/-/media/client/global/documents/products/data-sheet/nand-flash/70-series/m70a_ddp_8gb_3v_nand_spi.pdf - page 48
  [8] https://www.micron.com/-/media/client/global/documents/products/data-sheet/nand-flash/70-series/m70a_ddp_8gb_1_8v_nand_spi.pdf - page 48

-- 
Best Regards,
Martin Kurbanov

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

Re: [PATCH v2 2/2] mtd: spinand: micron: fixing the offset for OOB

[Miquel Raynal]

Hi Martin,

mmkurbanov@sberdevices.ru wrote on Tue, 22 Aug 2023 19:57:11 +0300:

> Hi Miquel,
> 
> On 22.08.2023 16:35, Miquel Raynal wrote:
> > Hi Martin,
> > 
> > mmkurbanov@sberdevices.ru wrote on Tue, 22 Aug 2023 15:25:34 +0300:
> >   
> >> The first 4 bytes are reserved for bad block data.  
> > 
> > Are you sure about that? I've never seen 4-bytes BBM.  
> 
> Yes, I'm sure. I have checked in all the relevant datasheets:
> ----------------------------------------------------------------------------------
> |        Name        | Max Byte | Min Byte |  Area   |        Description        |
> |                    | Address  | Address  |         |                           |
> |--------------------+----------+----------+---------+---------------------------|
> | MT29F2G01ABAGD [1] |          |          |         |                           |
> |--------------------|          |          |         |                           |
> | MT29F2G01ABBGD [2] |          |          |         |                           |
> |--------------------|   803h   |   800h   |         |                           |
> | MT29F1G01ABAFD [3] |          |          |         |                           |
> |--------------------|          |          |         |                           |
> | MT29F4G01ADAGD [4] |          |          |         |                           |
> |--------------------+----------+----------| Spare 0 | Reserved (bad block data) |
> | MT29F4G01ABAFD [5] |          |          |         |                           |
> |--------------------|          |          |         |                           |
> | MT29F4G01ABBFD [6] |          |          |         |                           |
> |--------------------|  1003h   |  1000h   |         |                           |
> | MT29F8G01ADAFD [7] |          |          |         |                           |
> |--------------------|          |          |         |                           |
> | MT29F8G01ADBFD [8] |          |          |         |                           |
> ----------------------------------------------------------------------------------
> 
> Note: to view the datasheet on the Micron (https://www.micron.com/) website, you need
> to register.

I don't think the four bytes have any "bad block specific" meaning. In
practice, the datasheet states:

	Value programmed for bad block at the first byte of spare
	area: 00h

So only the first byte is used to mark the block bad, the rest is
probably marked "reserved" for simplicity. I believe we should keep the
current layout because it would otherwise break users for no real
reason.

> Links:
>   [1] https://datasheet.lcsc.com/lcsc/1912111437_Micron-Tech-MT29F2G01ABAGDWB-IT-G_C410863.pdf - page 45
>   [2] https://www.micron.com/-/media/client/global/documents/products/data-sheet/nand-flash/70-series/m79a_2gb_1_8v_nand_spi.pdf - page 46
>   [3] https://datasheet.lcsc.com/lcsc/2209201030_Micron-Tech-MT29F1G01ABAFDWB-IT-F_C2905686.pdf - page 46
>   [4] https://www.micron.com/-/media/client/global/documents/products/data-sheet/nand-flash/70-series/m79a_ddp_4gb_3v_nand_spi.pdf - page 46
>   [5] https://www.micron.com/-/media/client/global/documents/products/data-sheet/nand-flash/70-series/m70a_4gb_3v_nand_spi.pdf - page 47
>   [6] https://www.micron.com/-/media/client/global/documents/products/data-sheet/nand-flash/70-series/m70a_4gb_1_8v_nand_spi.pdf - page 49
>   [7] https://www.micron.com/-/media/client/global/documents/products/data-sheet/nand-flash/70-series/m70a_ddp_8gb_3v_nand_spi.pdf - page 48
>   [8] https://www.micron.com/-/media/client/global/documents/products/data-sheet/nand-flash/70-series/m70a_ddp_8gb_1_8v_nand_spi.pdf - page 48
> 

Thanks,
Miquèl

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

Re: [PATCH v2 2/2] mtd: spinand: micron: fixing the offset for OOB

[Martin Kurbanov]

Hi Miquel,

On 23.08.2023 11:41, Miquel Raynal wrote:
> Hi Martin,
>
> I don't think the four bytes have any "bad block specific" meaning. In
> practice, the datasheet states:
> 
> 	Value programmed for bad block at the first byte of spare
> 	area: 00h
> 
> So only the first byte is used to mark the block bad, the rest is
> probably marked "reserved" for simplicity. I believe we should keep the
> current layout because it would otherwise break users for no real
> reason.

I agree with you that this can break the work of users who use OOB.
However, I believe it would be more appropriate to use an offset of 4,
as the micron chip can use all 4 bytes for additional data about the
bad block. So, there is a non-zero probability of losing OOB data in
the reserved area (2 bytes) when the hardware chip attempts to mark
the block as bad.

-- 
Best Regards,
Martin Kurbanov

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

Re: [PATCH v2 2/2] mtd: spinand: micron: fixing the offset for OOB

[Miquel Raynal]

Hi Martin,

mmkurbanov@sberdevices.ru wrote on Wed, 23 Aug 2023 14:33:57 +0300:

> Hi Miquel,
> 
> On 23.08.2023 11:41, Miquel Raynal wrote:
> > Hi Martin,
> >
> > I don't think the four bytes have any "bad block specific" meaning. In
> > practice, the datasheet states:
> > 
> > 	Value programmed for bad block at the first byte of spare
> > 	area: 00h
> > 
> > So only the first byte is used to mark the block bad, the rest is
> > probably marked "reserved" for simplicity. I believe we should keep the
> > current layout because it would otherwise break users for no real
> > reason.  
> 
> I agree with you that this can break the work of users who use OOB.
> However, I believe it would be more appropriate to use an offset of 4,
> as the micron chip can use all 4 bytes for additional data about the
> bad block. So, there is a non-zero probability of losing OOB data in
> the reserved area (2 bytes) when the hardware chip attempts to mark
> the block as bad.

Is this really a process the chip can do? Aren't bad blocks factory
marked only?

Then it's mtd's duty to manage them.

Thanks,
Miquèl

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

Re: [PATCH v2 2/2] mtd: spinand: micron: fixing the offset for OOB

[Martin Kurbanov]

On 23.08.2023 14:39, Miquel Raynal wrote:
> Hi Martin,
> 
> mmkurbanov@sberdevices.ru wrote on Wed, 23 Aug 2023 14:33:57 +0300:
> 
>> Hi Miquel,
>>
>> On 23.08.2023 11:41, Miquel Raynal wrote:
>>> Hi Martin,
>>>
>>> I don't think the four bytes have any "bad block specific" meaning. In
>>> practice, the datasheet states:
>>>
>>> 	Value programmed for bad block at the first byte of spare
>>> 	area: 00h
>>>
>>> So only the first byte is used to mark the block bad, the rest is
>>> probably marked "reserved" for simplicity. I believe we should keep the
>>> current layout because it would otherwise break users for no real
>>> reason.  
>>
>> I agree with you that this can break the work of users who use OOB.
>> However, I believe it would be more appropriate to use an offset of 4,
>> as the micron chip can use all 4 bytes for additional data about the
>> bad block. So, there is a non-zero probability of losing OOB data in
>> the reserved area (2 bytes) when the hardware chip attempts to mark
>> the block as bad.
> 
> Is this really a process the chip can do? Aren't bad blocks factory
> marked only?

Actually, there is my understanding, I’m not sure exactly.

-- 
Best Regards,
Martin Kurbanov

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

Re: [PATCH v2 2/2] mtd: spinand: micron: fixing the offset for OOB

[Martin Kurbanov]

Hi Miquel,

On 24.08.2023 12:35, Martin Kurbanov wrote:
> 
> 
> On 23.08.2023 14:39, Miquel Raynal wrote:
>> Hi Martin,
>>
>> mmkurbanov@sberdevices.ru wrote on Wed, 23 Aug 2023 14:33:57 +0300:
>>
>>> Hi Miquel,
>>>
>>> On 23.08.2023 11:41, Miquel Raynal wrote:
>>>> Hi Martin,
>>>>
>>>> I don't think the four bytes have any "bad block specific" meaning. In
>>>> practice, the datasheet states:
>>>>
>>>> 	Value programmed for bad block at the first byte of spare
>>>> 	area: 00h
>>>>
>>>> So only the first byte is used to mark the block bad, the rest is
>>>> probably marked "reserved" for simplicity. I believe we should keep the
>>>> current layout because it would otherwise break users for no real
>>>> reason.  
>>>
>>> I agree with you that this can break the work of users who use OOB.
>>> However, I believe it would be more appropriate to use an offset of 4,
>>> as the micron chip can use all 4 bytes for additional data about the
>>> bad block. So, there is a non-zero probability of losing OOB data in
>>> the reserved area (2 bytes) when the hardware chip attempts to mark
>>> the block as bad.
>>
>> Is this really a process the chip can do? Aren't bad blocks factory
>> marked only?
> 
> Actually, there is my understanding, I’m not sure exactly.

I tested with an offset of 2, no read/write errors were detected
(including read/write to OOB). But I don't have a flash chip with
factory bad blocks yet, when I find such a flash, I will report the
results.
Do I need to send the v3 of the patch with only first commit ("correct
bitmask for ecc status")?

-- 
Best Regards,
Martin Kurbanov

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

Re: [PATCH v2 2/2] mtd: spinand: micron: fixing the offset for OOB

[Miquel Raynal]

Hi Martin,

mmkurbanov@sberdevices.ru wrote on Mon, 4 Sep 2023 17:20:59 +0300:

> Hi Miquel,
> 
> On 24.08.2023 12:35, Martin Kurbanov wrote:
> > 
> > 
> > On 23.08.2023 14:39, Miquel Raynal wrote:
> >> Hi Martin,
> >>
> >> mmkurbanov@sberdevices.ru wrote on Wed, 23 Aug 2023 14:33:57 +0300:
> >>
> >>> Hi Miquel,
> >>>
> >>> On 23.08.2023 11:41, Miquel Raynal wrote:
> >>>> Hi Martin,
> >>>>
> >>>> I don't think the four bytes have any "bad block specific" meaning. In
> >>>> practice, the datasheet states:
> >>>>
> >>>> 	Value programmed for bad block at the first byte of spare
> >>>> 	area: 00h
> >>>>
> >>>> So only the first byte is used to mark the block bad, the rest is
> >>>> probably marked "reserved" for simplicity. I believe we should keep the
> >>>> current layout because it would otherwise break users for no real
> >>>> reason.  
> >>>
> >>> I agree with you that this can break the work of users who use OOB.
> >>> However, I believe it would be more appropriate to use an offset of 4,
> >>> as the micron chip can use all 4 bytes for additional data about the
> >>> bad block. So, there is a non-zero probability of losing OOB data in
> >>> the reserved area (2 bytes) when the hardware chip attempts to mark
> >>> the block as bad.
> >>
> >> Is this really a process the chip can do? Aren't bad blocks factory
> >> marked only?
> > 
> > Actually, there is my understanding, I’m not sure exactly.
> 
> I tested with an offset of 2, no read/write errors were detected
> (including read/write to OOB). But I don't have a flash chip with
> factory bad blocks yet, when I find such a flash, I will report the
> results.

Ok.

> Do I need to send the v3 of the patch with only first commit ("correct
> bitmask for ecc status")?

Yes please, with Frieder's comments fixed.

Thanks,
Miquèl

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