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

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

[Zixun LI]

Hi,

After updating our SAM9G25 device from kernel 3.16 to 6.6, we have
encountered UBIFS failures with ECC errors:

  ubi0 warning: ubi_io_read: error -74 (ECC error) while reading
126976 bytes from PEB 8:4096, read only 126976 bytes, retry

On the old system (kernel 3.16), nandtest passed successfully:

  nandtest -p 1 -l 0x40000 /dev/mtd2
  ECC corrections: 1
  ECC failures : 0
  Bad blocks : 0
  BBT blocks : 0
  00020000: checking...
  Finished pass 1 successfully

But on the new system (kernel 6.6), nandtest reports many errors:

  nandtest -p 1 -l 0x40000 /dev/mtd2
  ECC corrections: 1667
  ECC failures : 1205
  Bad blocks : 1
  BBT blocks : 0
  00000000: reading (1 of 4)...
  218 bit(s) ECC corrected at 00000000

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 ?

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

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 = {

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

[Zixun LI]

On Tue, Jul 1, 2025 at 5:49 PM Ada Couprie Diaz <ada.coupriediaz@arm.com> wrote:
>
> Hi,
>
> 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
>

Hi Ada,
Thank you for looking into this. Internally I've written the same patch as
yours and it works fine.

What's more interesting is the issue happens depending on chip individual
difference or aging. Among 3 chips tested, two with date code of 1933 and
one with 2223. The 1st one has many ECC errors as in the mail, 2nd one
has less errors, while the 3rd one passed the nandtest without error.
Maybe that's why this issue is overlooked.

Best regards,
Zixun LI

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

[Ada Couprie Diaz]

On 01/07/2025 19:04, Zixun LI wrote:

> On Tue, Jul 1, 2025 at 5:49 PM Ada Couprie Diaz <ada.coupriediaz@arm.com> wrote:
>> Hi,
>>
>> 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
>>
> Hi Ada,
> Thank you for looking into this. Internally I've written the same patch as
> yours and it works fine.

Glad to know that you already have a working solution and that I wasn't too far off !
I got interested and wanted to dig into it, I hope it didn't come off wrong.

> What's more interesting is the issue happens depending on chip individual
> difference or aging. Among 3 chips tested, two with date code of 1933 and
> one with 2223. The 1st one has many ECC errors as in the mail, 2nd one
> has less errors, while the 3rd one passed the nandtest without error.
> Maybe that's why this issue is overlooked.

Really interesting behaviour, that could indeed explain how it might 
have been missed during the original refactor as it seems it was tested 
with similar SoCs.

> Best regards,
> Zixun LI
Best regards,
Ada

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

[Balamanikandan.Gunasundar]

Hi,

Thanks Ada for the quick investigation. I too checked the history of the 
change. As you mentioned this could have been lost in the porting.

This patch is required for SAM9x60, SAM9x7, SAM9G25 as it is recommended 
by the datasheet. For 133MHz this must be set to 2, indicating the setup 
time as 3 clock cycles. I will investigate this more and send a patch asap.

I will also try to get more missing details in the datasheet.

Thanks,
Bala

On 01/07/25 9:18 pm, Ada Couprie Diaz wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know 
> the content is safe
> 
> 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 = {
> 
>