[PATCH 0/2] mtd: rawnand: atmel: Fix ECC error on SAM9

[PATCH 0/2] mtd: rawnand: atmel: Fix ECC error on SAM9

[Zixun LI]

PMECC setup time config was lost in the porting, this patchset backports the
fix from Linux.

Signed-off-by: Zixun LI <admin@hifiphile.com>
---
Zixun LI (2):
      mtd: rawnand: atmel: set pmecc data setup time
      mtd: rawnand: atmel: Remove timing from atmel_pmecc struct

 drivers/mtd/nand/raw/atmel/pmecc.c | 6 +++++-
 drivers/mtd/nand/raw/atmel/pmecc.h | 1 -
 2 files changed, 5 insertions(+), 2 deletions(-)
---
base-commit: 1c250e444ad3b15315ee8b0fcb3fc3acc26449e2
change-id: 20251106-pmecc-a3841c81c782

Best regards,
-- 
Zixun LI <admin@hifiphile.com>

[PATCH 1/2] mtd: rawnand: atmel: set pmecc data setup time

[Zixun LI]

Setup the pmecc data setup time as 3 clock cycles for 133MHz as
recommended by the datasheet.

Backported from Linux: f552a7c7 ("mtd: rawnand: atmel: set pmecc data
setup time")

Fixes: a490e1b7c017c ("nand: atmel: Add pmecc driver")

Signed-off-by: Zixun LI <admin@hifiphile.com>
---
 drivers/mtd/nand/raw/atmel/pmecc.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/mtd/nand/raw/atmel/pmecc.c b/drivers/mtd/nand/raw/atmel/pmecc.c
index e500a0fe3f8671ff08195c94c3383696a7731eb3..7c4e9bd5f9967b119239adf2b39e683ea62b71fd 100644
--- a/drivers/mtd/nand/raw/atmel/pmecc.c
+++ b/drivers/mtd/nand/raw/atmel/pmecc.c
@@ -142,6 +142,7 @@ struct atmel_pmecc_caps {
 	int nstrengths;
 	int el_offset;
 	bool correct_erased_chunks;
+	bool clk_ctrl;
 };
 
 struct atmel_pmecc_user_conf_cache {
@@ -840,6 +841,10 @@ atmel_pmecc_create(struct udevice *dev,
 
 	pmecc->regs.timing = 0;
 
+	/* pmecc data setup time */
+	if (caps->clk_ctrl)
+		writel(PMECC_CLK_133MHZ, pmecc->regs.base + ATMEL_PMECC_CLK);
+
 	/* Disable all interrupts before registering the PMECC handler. */
 	writel(0xffffffff, pmecc->regs.base + ATMEL_PMECC_IDR);
 	atmel_pmecc_reset(pmecc);
@@ -884,6 +889,7 @@ static struct atmel_pmecc_caps at91sam9g45_caps = {
 	.strengths = atmel_pmecc_strengths,
 	.nstrengths = 5,
 	.el_offset = 0x8c,
+	.clk_ctrl = true,
 };
 
 static struct atmel_pmecc_caps sama5d4_caps = {

-- 
2.51.0

[PATCH 2/2] mtd: rawnand: atmel: Remove timing from atmel_pmecc struct

[Zixun LI]

"timing" is unused in atmel_pmecc struct, also it's not exist in Linux
driver, let's remove it.

Signed-off-by: Zixun LI <admin@hifiphile.com>
---
 drivers/mtd/nand/raw/atmel/pmecc.c | 2 --
 drivers/mtd/nand/raw/atmel/pmecc.h | 1 -
 2 files changed, 3 deletions(-)

diff --git a/drivers/mtd/nand/raw/atmel/pmecc.c b/drivers/mtd/nand/raw/atmel/pmecc.c
index 7c4e9bd5f9967b119239adf2b39e683ea62b71fd..8fc5614cc0091d3e0f4b9280beee9e684fe2a57d 100644
--- a/drivers/mtd/nand/raw/atmel/pmecc.c
+++ b/drivers/mtd/nand/raw/atmel/pmecc.c
@@ -839,8 +839,6 @@ atmel_pmecc_create(struct udevice *dev,
 	ofnode_read_resource(dev->node_, 1, &res);
 	pmecc->regs.errloc = (void *)res.start;
 
-	pmecc->regs.timing = 0;
-
 	/* pmecc data setup time */
 	if (caps->clk_ctrl)
 		writel(PMECC_CLK_133MHZ, pmecc->regs.base + ATMEL_PMECC_CLK);
diff --git a/drivers/mtd/nand/raw/atmel/pmecc.h b/drivers/mtd/nand/raw/atmel/pmecc.h
index 43f96b2f168c7b88e0c907996f2ae64f8554ed58..88b0d18040e7c48f226c74ce9b5caf93b5760360 100644
--- a/drivers/mtd/nand/raw/atmel/pmecc.h
+++ b/drivers/mtd/nand/raw/atmel/pmecc.h
@@ -65,7 +65,6 @@ struct atmel_pmecc {
 	struct {
 		void __iomem *base;
 		void __iomem *errloc;
-		void __iomem *timing;
 	} regs;
 
 	/* Mutex used for pmecc enable/disable */

-- 
2.51.0

Re: [PATCH 2/2] mtd: rawnand: atmel: Remove timing from atmel_pmecc struct

[Alexander Dahl]

Hello,

this is a nice coincident, I'm currently working on sam9x60 u-boot
support anyways. ^^

Am Thu, Nov 06, 2025 at 12:12:02PM +0100 schrieb Zixun LI:
> "timing" is unused in atmel_pmecc struct, also it's not exist in Linux
> driver, let's remove it.
> 
> Signed-off-by: Zixun LI <admin@hifiphile.com>
> ---
>  drivers/mtd/nand/raw/atmel/pmecc.c | 2 --
>  drivers/mtd/nand/raw/atmel/pmecc.h | 1 -
>  2 files changed, 3 deletions(-)
> 
> diff --git a/drivers/mtd/nand/raw/atmel/pmecc.c b/drivers/mtd/nand/raw/atmel/pmecc.c
> index 7c4e9bd5f9967b119239adf2b39e683ea62b71fd..8fc5614cc0091d3e0f4b9280beee9e684fe2a57d 100644
> --- a/drivers/mtd/nand/raw/atmel/pmecc.c
> +++ b/drivers/mtd/nand/raw/atmel/pmecc.c
> @@ -839,8 +839,6 @@ atmel_pmecc_create(struct udevice *dev,
>  	ofnode_read_resource(dev->node_, 1, &res);
>  	pmecc->regs.errloc = (void *)res.start;
>  
> -	pmecc->regs.timing = 0;
> -
>  	/* pmecc data setup time */
>  	if (caps->clk_ctrl)
>  		writel(PMECC_CLK_133MHZ, pmecc->regs.base + ATMEL_PMECC_CLK);
> diff --git a/drivers/mtd/nand/raw/atmel/pmecc.h b/drivers/mtd/nand/raw/atmel/pmecc.h
> index 43f96b2f168c7b88e0c907996f2ae64f8554ed58..88b0d18040e7c48f226c74ce9b5caf93b5760360 100644
> --- a/drivers/mtd/nand/raw/atmel/pmecc.h
> +++ b/drivers/mtd/nand/raw/atmel/pmecc.h
> @@ -65,7 +65,6 @@ struct atmel_pmecc {
>  	struct {
>  		void __iomem *base;
>  		void __iomem *errloc;
> -		void __iomem *timing;

Acked-by: Alexander Dahl <ada@thorsis.com>

Tested both patches on a sam9x60-curiosity rev 4 board.

Note: the NAND flash is terribly slow without the following additional
patch, I reported this before, someone at Microchip confirmed and
wanted to investigate, never got an answer, you might find the
discussion in the mailing list archive:

diff --git a/arch/arm/dts/at91-sam9x60_curiosity-u-boot.dtsi b/arch/arm/dts/at91-sam9x60_curiosity-u-boot.dtsi
index 9144387861e..3dd2309be10 100644
--- a/arch/arm/dts/at91-sam9x60_curiosity-u-boot.dtsi
+++ b/arch/arm/dts/at91-sam9x60_curiosity-u-boot.dtsi
@@ -56,6 +56,12 @@
         bootph-all;
 };
 
+&nand_controller {
+        nand@3 {
+                /delete-property/ rb-gpios;
+        };
+};
+
 &pinctrl {
         bootph-all;
 };

Want to give your patches another try on a sam9x60 custom board with a
different NAND flash. I'm confident it will work, already does on
Linux.  Will report tomorrow.

Greets
Alex

>  	} regs;
>  
>  	/* Mutex used for pmecc enable/disable */
> 
> -- 
> 2.51.0
> 

Re: [PATCH 0/2] mtd: rawnand: atmel: Fix ECC error on SAM9

[Eugen Hristev]

On 11/6/25 13:12, Zixun LI wrote:
> PMECC setup time config was lost in the porting, this patchset backports the
> fix from Linux.
> 
> Signed-off-by: Zixun LI <admin@hifiphile.com>
> ---
> Zixun LI (2):
>       mtd: rawnand: atmel: set pmecc data setup time
>       mtd: rawnand: atmel: Remove timing from atmel_pmecc struct
> 
>  drivers/mtd/nand/raw/atmel/pmecc.c | 6 +++++-
>  drivers/mtd/nand/raw/atmel/pmecc.h | 1 -
>  2 files changed, 5 insertions(+), 2 deletions(-)
> ---
> base-commit: 1c250e444ad3b15315ee8b0fcb3fc3acc26449e2
> change-id: 20251106-pmecc-a3841c81c782
> 
> Best regards,


Hi Zixun,

Can you please detail "Fix ECC error", what is the error? you are not
explaining about it.

Eugen

Re: [PATCH 1/2] mtd: rawnand: atmel: set pmecc data setup time

[Eugen Hristev]

Hello Zixun, Alexander,

On 11/6/25 13:12, Zixun LI wrote:
> Setup the pmecc data setup time as 3 clock cycles for 133MHz as
> recommended by the datasheet.
> 
> Backported from Linux: f552a7c7 ("mtd: rawnand: atmel: set pmecc data
> setup time")

Maybe use 12 digits commit hash ? I cannot find this commit in Linux

> 
> Fixes: a490e1b7c017c ("nand: atmel: Add pmecc driver")
> 
> Signed-off-by: Zixun LI <admin@hifiphile.com>
> ---
>  drivers/mtd/nand/raw/atmel/pmecc.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/drivers/mtd/nand/raw/atmel/pmecc.c b/drivers/mtd/nand/raw/atmel/pmecc.c
> index e500a0fe3f8671ff08195c94c3383696a7731eb3..7c4e9bd5f9967b119239adf2b39e683ea62b71fd 100644
> --- a/drivers/mtd/nand/raw/atmel/pmecc.c
> +++ b/drivers/mtd/nand/raw/atmel/pmecc.c
> @@ -142,6 +142,7 @@ struct atmel_pmecc_caps {
>  	int nstrengths;
>  	int el_offset;
>  	bool correct_erased_chunks;
> +	bool clk_ctrl;
>  };
>  
>  struct atmel_pmecc_user_conf_cache {
> @@ -840,6 +841,10 @@ atmel_pmecc_create(struct udevice *dev,
>  
>  	pmecc->regs.timing = 0;
>  
> +	/* pmecc data setup time */
> +	if (caps->clk_ctrl)
> +		writel(PMECC_CLK_133MHZ, pmecc->regs.base + ATMEL_PMECC_CLK);
> +
>  	/* Disable all interrupts before registering the PMECC handler. */
>  	writel(0xffffffff, pmecc->regs.base + ATMEL_PMECC_IDR);
>  	atmel_pmecc_reset(pmecc);
> @@ -884,6 +889,7 @@ static struct atmel_pmecc_caps at91sam9g45_caps = {
>  	.strengths = atmel_pmecc_strengths,
>  	.nstrengths = 5,
>  	.el_offset = 0x8c,
> +	.clk_ctrl = true,

This appears to be done only for sam9g45.

So, Alexander, this should not impact sam9x60 .

I looked a bit in the Linux driver and this appears to be just as bad:
133 Mhz clock config hardcoded. I would expect this clock to be taken
from a clock input and not hardcoded. But oh well, if this clock is to
be changed in another device, it would be exposed somewhere...

Eugen

>  };
>  
>  static struct atmel_pmecc_caps sama5d4_caps = {
> 

Re: [PATCH 2/2] mtd: rawnand: atmel: Remove timing from atmel_pmecc struct

[Eugen Hristev]

On 11/6/25 13:12, Zixun LI wrote:
> "timing" is unused in atmel_pmecc struct, also it's not exist in Linux
> driver, let's remove it.
> 
> Signed-off-by: Zixun LI <admin@hifiphile.com>
> ---
>  drivers/mtd/nand/raw/atmel/pmecc.c | 2 --
>  drivers/mtd/nand/raw/atmel/pmecc.h | 1 -
>  2 files changed, 3 deletions(-)
> 
> diff --git a/drivers/mtd/nand/raw/atmel/pmecc.c b/drivers/mtd/nand/raw/atmel/pmecc.c
> index 7c4e9bd5f9967b119239adf2b39e683ea62b71fd..8fc5614cc0091d3e0f4b9280beee9e684fe2a57d 100644
> --- a/drivers/mtd/nand/raw/atmel/pmecc.c
> +++ b/drivers/mtd/nand/raw/atmel/pmecc.c
> @@ -839,8 +839,6 @@ atmel_pmecc_create(struct udevice *dev,
>  	ofnode_read_resource(dev->node_, 1, &res);
>  	pmecc->regs.errloc = (void *)res.start;
>  
> -	pmecc->regs.timing = 0;
> -

I see there is also an unused parameter to atmel_pmecc_create() called
timing_res_idx .
Maybe this parameter was intended to be passed to the timing mapped memory ?

If the timing_res_idx is related and unused as well, isn't it supposed
to be removed together ?

I see it's being called with a hardcoded "2" , does this have any
significance that you know of ? Or it can be safely removed.

Eugen

>  	/* pmecc data setup time */
>  	if (caps->clk_ctrl)
>  		writel(PMECC_CLK_133MHZ, pmecc->regs.base + ATMEL_PMECC_CLK);
> diff --git a/drivers/mtd/nand/raw/atmel/pmecc.h b/drivers/mtd/nand/raw/atmel/pmecc.h
> index 43f96b2f168c7b88e0c907996f2ae64f8554ed58..88b0d18040e7c48f226c74ce9b5caf93b5760360 100644
> --- a/drivers/mtd/nand/raw/atmel/pmecc.h
> +++ b/drivers/mtd/nand/raw/atmel/pmecc.h
> @@ -65,7 +65,6 @@ struct atmel_pmecc {
>  	struct {
>  		void __iomem *base;
>  		void __iomem *errloc;
> -		void __iomem *timing;
>  	} regs;
>  
>  	/* Mutex used for pmecc enable/disable */
> 

Re: [PATCH 1/2] mtd: rawnand: atmel: set pmecc data setup time

[Alexander Dahl]

Hello Eugen,

Am Fri, Nov 07, 2025 at 12:52:05PM +0200 schrieb Eugen Hristev:
> Hello Zixun, Alexander,
> 
> On 11/6/25 13:12, Zixun LI wrote:
> > Setup the pmecc data setup time as 3 clock cycles for 133MHz as
> > recommended by the datasheet.
> > 
> > Backported from Linux: f552a7c7 ("mtd: rawnand: atmel: set pmecc data
> > setup time")
> 
> Maybe use 12 digits commit hash ? I cannot find this commit in Linux

v6.16-rc4-10-gf552a7c7e0a14 aka
f552a7c7e0a14215cb8a6fd89e60fa3932a74786 from July 2025.

> 
> > 
> > Fixes: a490e1b7c017c ("nand: atmel: Add pmecc driver")
> > 
> > Signed-off-by: Zixun LI <admin@hifiphile.com>
> > ---
> >  drivers/mtd/nand/raw/atmel/pmecc.c | 6 ++++++
> >  1 file changed, 6 insertions(+)
> > 
> > diff --git a/drivers/mtd/nand/raw/atmel/pmecc.c b/drivers/mtd/nand/raw/atmel/pmecc.c
> > index e500a0fe3f8671ff08195c94c3383696a7731eb3..7c4e9bd5f9967b119239adf2b39e683ea62b71fd 100644
> > --- a/drivers/mtd/nand/raw/atmel/pmecc.c
> > +++ b/drivers/mtd/nand/raw/atmel/pmecc.c
> > @@ -142,6 +142,7 @@ struct atmel_pmecc_caps {
> >  	int nstrengths;
> >  	int el_offset;
> >  	bool correct_erased_chunks;
> > +	bool clk_ctrl;
> >  };
> >  
> >  struct atmel_pmecc_user_conf_cache {
> > @@ -840,6 +841,10 @@ atmel_pmecc_create(struct udevice *dev,
> >  
> >  	pmecc->regs.timing = 0;
> >  
> > +	/* pmecc data setup time */
> > +	if (caps->clk_ctrl)
> > +		writel(PMECC_CLK_133MHZ, pmecc->regs.base + ATMEL_PMECC_CLK);
> > +
> >  	/* Disable all interrupts before registering the PMECC handler. */
> >  	writel(0xffffffff, pmecc->regs.base + ATMEL_PMECC_IDR);
> >  	atmel_pmecc_reset(pmecc);
> > @@ -884,6 +889,7 @@ static struct atmel_pmecc_caps at91sam9g45_caps = {
> >  	.strengths = atmel_pmecc_strengths,
> >  	.nstrengths = 5,
> >  	.el_offset = 0x8c,
> > +	.clk_ctrl = true,
> 
> This appears to be done only for sam9g45.
> 
> So, Alexander, this should not impact sam9x60 .

Yes it should.  According to arch/arm/boot/dts/microchip/sam9x60.dtsi
the compatible used on sam9x60 is "atmel,at91sam9g45-pmecc", see:

	pmecc: ecc-engine@ffffe000 {
			compatible = "microchip,sam9x60-pmecc", "atmel,at91sam9g45-pmecc";
			reg = <0xffffe000 0x300>,
					<0xffffe600 0x100>;
	};

> I looked a bit in the Linux driver and this appears to be just as bad:
> 133 Mhz clock config hardcoded. I would expect this clock to be taken
> from a clock input and not hardcoded. But oh well, if this clock is to
> be changed in another device, it would be exposed somewhere...

I did not look into that deeply, but if it is like you said, someone
should.  This sounds bad.

Greets
Alex

> 
> Eugen
> 
> >  };
> >  
> >  static struct atmel_pmecc_caps sama5d4_caps = {
> > 
> 

Re: [PATCH 2/2] mtd: rawnand: atmel: Remove timing from atmel_pmecc struct

[Michael Nazzareno Trimarchi]

Hi

On Fri, Nov 7, 2025 at 11:56 AM Eugen Hristev <eugen.hristev@linaro.org> wrote:
>
>
>
> On 11/6/25 13:12, Zixun LI wrote:
> > "timing" is unused in atmel_pmecc struct, also it's not exist in Linux
> > driver, let's remove it.
> >
> > Signed-off-by: Zixun LI <admin@hifiphile.com>
> > ---
> >  drivers/mtd/nand/raw/atmel/pmecc.c | 2 --
> >  drivers/mtd/nand/raw/atmel/pmecc.h | 1 -
> >  2 files changed, 3 deletions(-)
> >
> > diff --git a/drivers/mtd/nand/raw/atmel/pmecc.c b/drivers/mtd/nand/raw/atmel/pmecc.c
> > index 7c4e9bd5f9967b119239adf2b39e683ea62b71fd..8fc5614cc0091d3e0f4b9280beee9e684fe2a57d 100644
> > --- a/drivers/mtd/nand/raw/atmel/pmecc.c
> > +++ b/drivers/mtd/nand/raw/atmel/pmecc.c
> > @@ -839,8 +839,6 @@ atmel_pmecc_create(struct udevice *dev,
> >       ofnode_read_resource(dev->node_, 1, &res);
> >       pmecc->regs.errloc = (void *)res.start;
> >
> > -     pmecc->regs.timing = 0;
> > -
>
> I see there is also an unused parameter to atmel_pmecc_create() called
> timing_res_idx .
> Maybe this parameter was intended to be passed to the timing mapped memory ?
>
> If the timing_res_idx is related and unused as well, isn't it supposed
> to be removed together ?
>
> I see it's being called with a hardcoded "2" , does this have any
> significance that you know of ? Or it can be safely removed.
>
> Eugen
>
> >       /* pmecc data setup time */
> >       if (caps->clk_ctrl)
> >               writel(PMECC_CLK_133MHZ, pmecc->regs.base + ATMEL_PMECC_CLK);
> > diff --git a/drivers/mtd/nand/raw/atmel/pmecc.h b/drivers/mtd/nand/raw/atmel/pmecc.h
> > index 43f96b2f168c7b88e0c907996f2ae64f8554ed58..88b0d18040e7c48f226c74ce9b5caf93b5760360 100644
> > --- a/drivers/mtd/nand/raw/atmel/pmecc.h
> > +++ b/drivers/mtd/nand/raw/atmel/pmecc.h
> > @@ -65,7 +65,6 @@ struct atmel_pmecc {
> >       struct {
> >               void __iomem *base;
> >               void __iomem *errloc;
> > -             void __iomem *timing;
> >       } regs;
> >
> >       /* Mutex used for pmecc enable/disable */
> >
>
I'm willing to help to clock part of nand, can someone point where I
can buy the board?

Michael

-- 
Michael Nazzareno Trimarchi
Co-Founder & Chief Executive Officer
M. +39 347 913 2170
michael@amarulasolutions.com
__________________________________

Amarula Solutions BV
Joop Geesinkweg 125, 1114 AB, Amsterdam, NL
T. +31 (0)85 111 9172
info@amarulasolutions.com
www.amarulasolutions.com

Re: [PATCH 2/2] mtd: rawnand: atmel: Remove timing from atmel_pmecc struct

[Alexander Dahl]

Hello Michael,

Am Fri, Nov 07, 2025 at 12:04:16PM +0100 schrieb Michael Nazzareno Trimarchi:
> Hi
> 
> On Fri, Nov 7, 2025 at 11:56 AM Eugen Hristev <eugen.hristev@linaro.org> wrote:
> >
> >
> >
> > On 11/6/25 13:12, Zixun LI wrote:
> > > "timing" is unused in atmel_pmecc struct, also it's not exist in Linux
> > > driver, let's remove it.
> > >
> > > Signed-off-by: Zixun LI <admin@hifiphile.com>
> > > ---
> > >  drivers/mtd/nand/raw/atmel/pmecc.c | 2 --
> > >  drivers/mtd/nand/raw/atmel/pmecc.h | 1 -
> > >  2 files changed, 3 deletions(-)
> > >
> > > diff --git a/drivers/mtd/nand/raw/atmel/pmecc.c b/drivers/mtd/nand/raw/atmel/pmecc.c
> > > index 7c4e9bd5f9967b119239adf2b39e683ea62b71fd..8fc5614cc0091d3e0f4b9280beee9e684fe2a57d 100644
> > > --- a/drivers/mtd/nand/raw/atmel/pmecc.c
> > > +++ b/drivers/mtd/nand/raw/atmel/pmecc.c
> > > @@ -839,8 +839,6 @@ atmel_pmecc_create(struct udevice *dev,
> > >       ofnode_read_resource(dev->node_, 1, &res);
> > >       pmecc->regs.errloc = (void *)res.start;
> > >
> > > -     pmecc->regs.timing = 0;
> > > -
> >
> > I see there is also an unused parameter to atmel_pmecc_create() called
> > timing_res_idx .
> > Maybe this parameter was intended to be passed to the timing mapped memory ?
> >
> > If the timing_res_idx is related and unused as well, isn't it supposed
> > to be removed together ?
> >
> > I see it's being called with a hardcoded "2" , does this have any
> > significance that you know of ? Or it can be safely removed.
> >
> > Eugen
> >
> > >       /* pmecc data setup time */
> > >       if (caps->clk_ctrl)
> > >               writel(PMECC_CLK_133MHZ, pmecc->regs.base + ATMEL_PMECC_CLK);
> > > diff --git a/drivers/mtd/nand/raw/atmel/pmecc.h b/drivers/mtd/nand/raw/atmel/pmecc.h
> > > index 43f96b2f168c7b88e0c907996f2ae64f8554ed58..88b0d18040e7c48f226c74ce9b5caf93b5760360 100644
> > > --- a/drivers/mtd/nand/raw/atmel/pmecc.h
> > > +++ b/drivers/mtd/nand/raw/atmel/pmecc.h
> > > @@ -65,7 +65,6 @@ struct atmel_pmecc {
> > >       struct {
> > >               void __iomem *base;
> > >               void __iomem *errloc;
> > > -             void __iomem *timing;
> > >       } regs;
> > >
> > >       /* Mutex used for pmecc enable/disable */
> > >
> >
> I'm willing to help to clock part of nand, can someone point where I
> can buy the board?

I use the Microchip SAM9X60 Curiosity:

https://www.microchip.com/en-us/development-tool/ev40e67a

It's also available from Mouser, RS, Digikey, and probably more.

The NAND flash populated on that board is an MX30LF4G28AD-XKI [1].

Greets
Alex

[1] https://www.mxic.com.tw/en-us/products/NAND-Flash/SLC-NAND-Flash/Pages/spec.aspx?p=MX30LF4G28AD&m=SLC+NAND&n=PM2579

Re: [PATCH 1/2] mtd: rawnand: atmel: set pmecc data setup time

[Zixun LI]

Hi Eugen, Alexander,

On Fri, Nov 7, 2025 at 12:01 PM Alexander Dahl <ada@thorsis.com> wrote:
>
> Hello Eugen,
>
> Am Fri, Nov 07, 2025 at 12:52:05PM +0200 schrieb Eugen Hristev:
> > Hello Zixun, Alexander,
> >
> > On 11/6/25 13:12, Zixun LI wrote:
> > > Setup the pmecc data setup time as 3 clock cycles for 133MHz as
> > > recommended by the datasheet.
> > >
> > > Backported from Linux: f552a7c7 ("mtd: rawnand: atmel: set pmecc data
> > > setup time")
> >
> > Maybe use 12 digits commit hash ? I cannot find this commit in Linux
>
> v6.16-rc4-10-gf552a7c7e0a14 aka
> f552a7c7e0a14215cb8a6fd89e60fa3932a74786 from July 2025.
>

I'll update the cover to include the issue and use a 12 digits hash.

Also remove timing_res_idx in atmel_pmecc_create.

Zixun

Re: [PATCH 1/2] mtd: rawnand: atmel: set pmecc data setup time

[Eugen Hristev]

On 11/7/25 13:01, Alexander Dahl wrote:
> Hello Eugen,
> 
> Am Fri, Nov 07, 2025 at 12:52:05PM +0200 schrieb Eugen Hristev:
>> Hello Zixun, Alexander,
>>
>> On 11/6/25 13:12, Zixun LI wrote:
>>> Setup the pmecc data setup time as 3 clock cycles for 133MHz as
>>> recommended by the datasheet.
>>>
>>> Backported from Linux: f552a7c7 ("mtd: rawnand: atmel: set pmecc data
>>> setup time")
>>
>> Maybe use 12 digits commit hash ? I cannot find this commit in Linux
> 
> v6.16-rc4-10-gf552a7c7e0a14 aka
> f552a7c7e0a14215cb8a6fd89e60fa3932a74786 from July 2025.
> 
>>
>>>
>>> Fixes: a490e1b7c017c ("nand: atmel: Add pmecc driver")
>>>
>>> Signed-off-by: Zixun LI <admin@hifiphile.com>
>>> ---
>>>  drivers/mtd/nand/raw/atmel/pmecc.c | 6 ++++++
>>>  1 file changed, 6 insertions(+)
>>>
>>> diff --git a/drivers/mtd/nand/raw/atmel/pmecc.c b/drivers/mtd/nand/raw/atmel/pmecc.c
>>> index e500a0fe3f8671ff08195c94c3383696a7731eb3..7c4e9bd5f9967b119239adf2b39e683ea62b71fd 100644
>>> --- a/drivers/mtd/nand/raw/atmel/pmecc.c
>>> +++ b/drivers/mtd/nand/raw/atmel/pmecc.c
>>> @@ -142,6 +142,7 @@ struct atmel_pmecc_caps {
>>>  	int nstrengths;
>>>  	int el_offset;
>>>  	bool correct_erased_chunks;
>>> +	bool clk_ctrl;
>>>  };
>>>  
>>>  struct atmel_pmecc_user_conf_cache {
>>> @@ -840,6 +841,10 @@ atmel_pmecc_create(struct udevice *dev,
>>>  
>>>  	pmecc->regs.timing = 0;
>>>  
>>> +	/* pmecc data setup time */
>>> +	if (caps->clk_ctrl)
>>> +		writel(PMECC_CLK_133MHZ, pmecc->regs.base + ATMEL_PMECC_CLK);
>>> +
>>>  	/* Disable all interrupts before registering the PMECC handler. */
>>>  	writel(0xffffffff, pmecc->regs.base + ATMEL_PMECC_IDR);
>>>  	atmel_pmecc_reset(pmecc);
>>> @@ -884,6 +889,7 @@ static struct atmel_pmecc_caps at91sam9g45_caps = {
>>>  	.strengths = atmel_pmecc_strengths,
>>>  	.nstrengths = 5,
>>>  	.el_offset = 0x8c,
>>> +	.clk_ctrl = true,
>>
>> This appears to be done only for sam9g45.
>>
>> So, Alexander, this should not impact sam9x60 .
> 
> Yes it should.  According to arch/arm/boot/dts/microchip/sam9x60.dtsi
> the compatible used on sam9x60 is "atmel,at91sam9g45-pmecc", see:
> 
> 	pmecc: ecc-engine@ffffe000 {
> 			compatible = "microchip,sam9x60-pmecc", "atmel,at91sam9g45-pmecc";
> 			reg = <0xffffe000 0x300>,
> 					<0xffffe600 0x100>;
> 	};
> 

Yes it looks you are right. sam9x60 does not have a dedicated caps but
reusing the sam9g45 one, and I missed that.

Thanks

>> I looked a bit in the Linux driver and this appears to be just as bad:
>> 133 Mhz clock config hardcoded. I would expect this clock to be taken
>> from a clock input and not hardcoded. But oh well, if this clock is to
>> be changed in another device, it would be exposed somewhere...
> 
> I did not look into that deeply, but if it is like you said, someone
> should.  This sounds bad.
> 
> Greets
> Alex
> 
>>
>> Eugen
>>
>>>  };
>>>  
>>>  static struct atmel_pmecc_caps sama5d4_caps = {
>>>
>>