Linux-mtd Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v0] mtd: rawnand: marvell: fix 8bit ECC layouts
@ 2025-10-22  1:37 Aryan Srivastava
  2025-10-22 13:39 ` Miquel Raynal
  0 siblings, 1 reply; 3+ messages in thread
From: Aryan Srivastava @ 2025-10-22  1:37 UTC (permalink / raw)
  To: Miquel Raynal
  Cc: Aryan Srivastava, Richard Weinberger, Vignesh Raghavendra,
	Elad Nachman, linux-mtd, linux-kernel

These were modified to appease a check in nand_scan_tail. This change led
to the last spare bytes never being written to or read from.

Modify the fix by restoring the layouts and setting ecc->steps to the
full_chunk_cnt value in the 8 bit ECC cases (4k and 8k page size). This
allows the driver to continue reading/writing all chunks while also
passing the check in nand_scan_tail.

Fixes: e6a30d0c48a1 ("mtd: rawnand: marvell: fix layouts")
Signed-off-by: Aryan Srivastava <aryan.srivastava@alliedtelesis.co.nz>
---
 drivers/mtd/nand/raw/marvell_nand.c | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/drivers/mtd/nand/raw/marvell_nand.c b/drivers/mtd/nand/raw/marvell_nand.c
index 303b3016a070..c20feacbaca8 100644
--- a/drivers/mtd/nand/raw/marvell_nand.c
+++ b/drivers/mtd/nand/raw/marvell_nand.c
@@ -292,10 +292,10 @@ static const struct marvell_hw_ecc_layout marvell_nfc_layouts[] = {
 	MARVELL_LAYOUT( 2048,   512,  8,  2,  1, 1024,  0, 30,1024,64, 30),
 	MARVELL_LAYOUT( 2048,   512,  16, 4,  4, 512,   0, 30,  0, 32, 30),
 	MARVELL_LAYOUT( 4096,   512,  4,  2,  2, 2048, 32, 30,  0,  0,  0),
-	MARVELL_LAYOUT( 4096,   512,  8,  4,  4, 1024,  0, 30,  0, 64, 30),
+	MARVELL_LAYOUT( 4096,   512,  8,  5,  4, 1024,  0, 30,  0, 64, 30),
 	MARVELL_LAYOUT( 4096,   512,  16, 8,  8, 512,   0, 30,  0, 32, 30),
 	MARVELL_LAYOUT( 8192,   512,  4,  4,  4, 2048,  0, 30,  0,  0,  0),
-	MARVELL_LAYOUT( 8192,   512,  8,  8,  8, 1024,  0, 30,  0, 160, 30),
+	MARVELL_LAYOUT( 8192,   512,  8,  9,  8, 1024,  0, 30,  0, 160, 30),
 	MARVELL_LAYOUT( 8192,   512,  16, 16, 16, 512,  0, 30,  0,  32, 30),
 };
 
@@ -2286,7 +2286,16 @@ static int marvell_nand_hw_ecc_controller_init(struct mtd_info *mtd,
 	}
 
 	mtd_set_ooblayout(mtd, &marvell_nand_ooblayout_ops);
-	ecc->steps = l->nchunks;
+
+	/* Validity checks in nand_scan_tail assume even sized chunks, but in the case of 8bit
+	 * ECC with 4k/8k page size the last chunk is spare data, which is not sized to the data
+	 * chunks. Overwrite the ecc->steps to pass this validity check, while maintaining the
+	 * correct number of chunks in-driver.
+	 */
+	if (ecc->strength == 8 && mtd->writesize > SZ_2K)
+		ecc->steps = l->full_chunk_cnt;
+	else
+		ecc->steps = l->nchunks;
 	ecc->size = l->data_bytes;
 
 	if (ecc->strength == 1) {
-- 
2.51.0


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

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH v0] mtd: rawnand: marvell: fix 8bit ECC layouts
  2025-10-22  1:37 [PATCH v0] mtd: rawnand: marvell: fix 8bit ECC layouts Aryan Srivastava
@ 2025-10-22 13:39 ` Miquel Raynal
  2025-10-22 20:18   ` Aryan Srivastava
  0 siblings, 1 reply; 3+ messages in thread
From: Miquel Raynal @ 2025-10-22 13:39 UTC (permalink / raw)
  To: Aryan Srivastava
  Cc: Richard Weinberger, Vignesh Raghavendra, Elad Nachman, linux-mtd,
	linux-kernel

On 22/10/2025 at 14:37:52 +13, Aryan Srivastava <aryan.srivastava@alliedtelesis.co.nz> wrote:

> These were modified to appease a check in nand_scan_tail. This change led
> to the last spare bytes never being written to or read from.
>
> Modify the fix by restoring the layouts and setting ecc->steps to the
> full_chunk_cnt value in the 8 bit ECC cases (4k and 8k page size). This
> allows the driver to continue reading/writing all chunks while also
> passing the check in nand_scan_tail.
>
> Fixes: e6a30d0c48a1 ("mtd: rawnand: marvell: fix layouts")
> Signed-off-by: Aryan Srivastava <aryan.srivastava@alliedtelesis.co.nz>
> ---
>  drivers/mtd/nand/raw/marvell_nand.c | 15 ++++++++++++---
>  1 file changed, 12 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/mtd/nand/raw/marvell_nand.c b/drivers/mtd/nand/raw/marvell_nand.c
> index 303b3016a070..c20feacbaca8 100644
> --- a/drivers/mtd/nand/raw/marvell_nand.c
> +++ b/drivers/mtd/nand/raw/marvell_nand.c
> @@ -292,10 +292,10 @@ static const struct marvell_hw_ecc_layout marvell_nfc_layouts[] = {
>  	MARVELL_LAYOUT( 2048,   512,  8,  2,  1, 1024,  0, 30,1024,64, 30),
>  	MARVELL_LAYOUT( 2048,   512,  16, 4,  4, 512,   0, 30,  0, 32, 30),
>  	MARVELL_LAYOUT( 4096,   512,  4,  2,  2, 2048, 32, 30,  0,  0,  0),
> -	MARVELL_LAYOUT( 4096,   512,  8,  4,  4, 1024,  0, 30,  0, 64, 30),
> +	MARVELL_LAYOUT( 4096,   512,  8,  5,  4, 1024,  0, 30,  0, 64, 30),
>  	MARVELL_LAYOUT( 4096,   512,  16, 8,  8, 512,   0, 30,  0, 32, 30),
>  	MARVELL_LAYOUT( 8192,   512,  4,  4,  4, 2048,  0, 30,  0,  0,  0),
> -	MARVELL_LAYOUT( 8192,   512,  8,  8,  8, 1024,  0, 30,  0, 160, 30),
> +	MARVELL_LAYOUT( 8192,   512,  8,  9,  8, 1024,  0, 30,  0, 160, 30),
>  	MARVELL_LAYOUT( 8192,   512,  16, 16, 16, 512,  0, 30,  0,  32,
> 30),
>  };
>  
> @@ -2286,7 +2286,16 @@ static int marvell_nand_hw_ecc_controller_init(struct mtd_info *mtd,
>  	}
>  
>  	mtd_set_ooblayout(mtd, &marvell_nand_ooblayout_ops);
> -	ecc->steps = l->nchunks;
> +
> +	/* Validity checks in nand_scan_tail assume even sized chunks, but in the case of 8bit
> +	 * ECC with 4k/8k page size the last chunk is spare data, which is not sized to the data
> +	 * chunks. Overwrite the ecc->steps to pass this validity check, while maintaining the
> +	 * correct number of chunks in-driver.
> +	 */

I am not a big fan of this approach, I wonder whether we should relax
the check in nand_scan_tail() or not. Maybe we could avoid the error in
the core by just printing a warning, this way the drivers would not need
to lie to the core.

What I would suggest, now that I properly understood the problem, is to:
1. Revert Elad's patchset *entirely* (including the revert of
   existing/valid but with little use layouts).
2. Relax the check in the core.

Thanks,
Miquèl

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

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH v0] mtd: rawnand: marvell: fix 8bit ECC layouts
  2025-10-22 13:39 ` Miquel Raynal
@ 2025-10-22 20:18   ` Aryan Srivastava
  0 siblings, 0 replies; 3+ messages in thread
From: Aryan Srivastava @ 2025-10-22 20:18 UTC (permalink / raw)
  To: miquel.raynal
  Cc: aryan.srivastava, enachman, linux-kernel, linux-mtd, richard,
	vigneshr

Okay, I will upload a new patch series.

Thank you,
Aryan

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

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2025-10-22 20:18 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-10-22  1:37 [PATCH v0] mtd: rawnand: marvell: fix 8bit ECC layouts Aryan Srivastava
2025-10-22 13:39 ` Miquel Raynal
2025-10-22 20:18   ` Aryan Srivastava

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