Re: mtd: rawnand: atmel: ECC error after update to kernel 6.6

[Ada Couprie Diaz]

Hi,

I'm not really involved in this part of the kernel or drivers,
but I took some time to look into it. Hopefully I'm not completely
missing something !

On 30/06/2025 15:38, Zixun LI wrote:
> Hi,
>
> After updating our SAM9G25 device from kernel 3.16 to 6.6, we have
> encountered UBIFS failures with ECC errors
>
> [...]
>
> After some diagnostics, I used devmem2 to compare SMC and PMECC
> registers between the two kernels. All values match except for the
> PMECC_CLK register, which is 2 in kernel 3.16 [1] and 0 in kernel 6.6
> [2]. It appears the clock setting is missing since the kernel 4.14
> refactor.
>
> According to the SAM9G25 datasheet this field must be programmed with 2.
>
> Manually setting PMECC_CLK to 2 (devmem2 0xffffe010 w 2) resolves the
> nandtest issue.
>
> Is the clock setting moved to elsewhere after the refactor ?

As far as I can tell, the setting of the *PMECC_CLK* was lost in
f88fc122cc34 (mtd: nand: Cleanup/rework the atmel_nand driver),
which was merged for 4.12.

The register offset and values are  part of the defines it introduces
but are unused, so they might have been forgotten.
I couldn't find another place where this was done, and I think it would
make sense for it to be done here still.

The tricky part is that now the driver handles other PMECCs that do not have
this register at all in their datasheets :
 - SAMA5D2 series[3], page 713, 36.20 Register summary :
     0x80 (0x10 offset) is marked reserved,
 - SAMA5D4 series[4], page 512, Table 30-20 :
     0x080 (0x10 offset) is marked reserved,

So I think it's best not to add this write back for all cases.

I also checked, as some other SoCs' PMECCs are marked compatible with
the one used in your SAM9G25 (`atmel,at91sam9g45-pmecc`) :
 - SAM9x60 series[5], page 256, 21.6 Register Summary :
     0x10 offset has PMECC_CLK present, recommends setting to 2 at 133MHz,
 - SAM9x7 series[6], page 263, 21.6 Register Summary :
     0x10 offset has PMECC_CLK present, reccomends setting to 2 at 133MHz.

So it should be safe to write 2 to this register for all those matching
this /compatible/ string.

The datasheet itself is not clear as to what to do for other MCK frequencies,
though as 133MHz is the highest supported frequency and it worked before,
hopefully it just means a cycle or two of useless delay at worse,
but at least it keeps it simple.

I've written a small patch below that I think should fix the issue, but again
I don't know if that's upstreamable as-is.

> Best regards,
> Zixun LI
>
> [1] https://github.com/torvalds/linux/blob/19583ca584d6f574384e17fe7613dfaeadcdc4a6/drivers/mtd/nand/atmel_nand.c#L1058
> [2] https://github.com/torvalds/linux/blob/ffc253263a1375a65fa6c9f62a893e9767fbebfa/drivers/mtd/nand/raw/atmel/pmecc.c#L772
Hopefully that can help !
Best regards,
Ada


[3]: https://ww1.microchip.com/downloads/en/DeviceDoc/SAMA5D2-Series-Data-Sheet-DS60001476C.pdf
[4]: https://ww1.microchip.com/downloads/aemDocuments/documents/MPU32/ProductDocuments/DataSheets/SAMA5D4-Series-Data-Sheet-DS60001525.pdf
[5]: https://ww1.microchip.com/downloads/aemDocuments/documents/MPU32/ProductDocuments/DataSheets/SAM9X60-Data-Sheet-DS60001579.pdf
[6]: https://ww1.microchip.com/downloads/aemDocuments/documents/MPU32/ProductDocuments/DataSheets/SAM9X7-Series-Data-Sheet-DS60001813.pdf

------------------------ >8 ------------------------

diff --git a/drivers/mtd/nand/raw/atmel/pmecc.c b/drivers/mtd/nand/raw/atmel/pmecc.c
index 3c7dee1be21d..cd96b44f3f30 100644
--- a/drivers/mtd/nand/raw/atmel/pmecc.c
+++ b/drivers/mtd/nand/raw/atmel/pmecc.c
@@ -143,6 +143,7 @@ struct atmel_pmecc_caps {
 	int nstrengths;
 	int el_offset;
 	bool correct_erased_chunks;
+	bool need_clk_config;
 };
 
 struct atmel_pmecc {
@@ -787,6 +788,9 @@ int atmel_pmecc_enable(struct atmel_pmecc_user *user, int op)
 	writel(PMECC_CTRL_ENABLE, pmecc->regs.base + ATMEL_PMECC_CTRL);
 	writel(PMECC_CTRL_DATA, pmecc->regs.base + ATMEL_PMECC_CTRL);
 
+	if (pmecc->caps->need_clk_config)
+		writel(PMECC_CLK_133MHZ, pmecc->regs.base + ATMEL_PMECC_CLK);
+
 	return 0;
 }
 EXPORT_SYMBOL_GPL(atmel_pmecc_enable);
@@ -896,6 +900,7 @@ static struct atmel_pmecc_caps at91sam9g45_caps = {
 	.strengths = atmel_pmecc_strengths,
 	.nstrengths = 5,
 	.el_offset = 0x8c,
+	.need_clk_config = true,
 };
 
 static struct atmel_pmecc_caps sama5d4_caps = {