* [PATCH 0/2] mtd: rawnand: atmel: Fix ECC error on SAM9
@ 2025-11-06 11:12 Zixun LI
2025-11-06 11:12 ` [PATCH 1/2] mtd: rawnand: atmel: set pmecc data setup time Zixun LI
` (2 more replies)
0 siblings, 3 replies; 12+ messages in thread
From: Zixun LI @ 2025-11-06 11:12 UTC (permalink / raw)
To: Balamanikandan Gunasundar, u-boot
Cc: Dario Binacchi, Michael Trimarchi, Tom Rini, Alexander Dahl,
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>
^ permalink raw reply [flat|nested] 12+ messages in thread* [PATCH 1/2] mtd: rawnand: atmel: set pmecc data setup time 2025-11-06 11:12 [PATCH 0/2] mtd: rawnand: atmel: Fix ECC error on SAM9 Zixun LI @ 2025-11-06 11:12 ` Zixun LI 2025-11-07 10:52 ` Eugen Hristev 2025-11-06 11:12 ` [PATCH 2/2] mtd: rawnand: atmel: Remove timing from atmel_pmecc struct Zixun LI 2025-11-07 10:48 ` [PATCH 0/2] mtd: rawnand: atmel: Fix ECC error on SAM9 Eugen Hristev 2 siblings, 1 reply; 12+ messages in thread From: Zixun LI @ 2025-11-06 11:12 UTC (permalink / raw) To: Balamanikandan Gunasundar, u-boot Cc: Dario Binacchi, Michael Trimarchi, Tom Rini, Alexander Dahl, 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 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH 1/2] mtd: rawnand: atmel: set pmecc data setup time 2025-11-06 11:12 ` [PATCH 1/2] mtd: rawnand: atmel: set pmecc data setup time Zixun LI @ 2025-11-07 10:52 ` Eugen Hristev 2025-11-07 11:01 ` Alexander Dahl 0 siblings, 1 reply; 12+ messages in thread From: Eugen Hristev @ 2025-11-07 10:52 UTC (permalink / raw) To: Zixun LI, Alexander Dahl Cc: Dario Binacchi, u-boot, Michael Trimarchi, Tom Rini, Balamanikandan Gunasundar 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 = { > ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1/2] mtd: rawnand: atmel: set pmecc data setup time 2025-11-07 10:52 ` Eugen Hristev @ 2025-11-07 11:01 ` Alexander Dahl 2025-11-07 12:53 ` Zixun LI 2025-11-11 10:29 ` Eugen Hristev 0 siblings, 2 replies; 12+ messages in thread From: Alexander Dahl @ 2025-11-07 11:01 UTC (permalink / raw) To: Eugen Hristev Cc: Zixun LI, Alexander Dahl, Dario Binacchi, u-boot, Michael Trimarchi, Tom Rini, Balamanikandan Gunasundar 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 = { > > > ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1/2] mtd: rawnand: atmel: set pmecc data setup time 2025-11-07 11:01 ` Alexander Dahl @ 2025-11-07 12:53 ` Zixun LI 2025-11-11 10:29 ` Eugen Hristev 1 sibling, 0 replies; 12+ messages in thread From: Zixun LI @ 2025-11-07 12:53 UTC (permalink / raw) To: Eugen Hristev, Zixun LI, Dario Binacchi, u-boot, Michael Trimarchi, Tom Rini, Balamanikandan Gunasundar Cc: Alexander Dahl 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 ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1/2] mtd: rawnand: atmel: set pmecc data setup time 2025-11-07 11:01 ` Alexander Dahl 2025-11-07 12:53 ` Zixun LI @ 2025-11-11 10:29 ` Eugen Hristev 1 sibling, 0 replies; 12+ messages in thread From: Eugen Hristev @ 2025-11-11 10:29 UTC (permalink / raw) To: Zixun LI, Dario Binacchi, u-boot, Michael Trimarchi, Tom Rini, Balamanikandan Gunasundar 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 = { >>> >> ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 2/2] mtd: rawnand: atmel: Remove timing from atmel_pmecc struct 2025-11-06 11:12 [PATCH 0/2] mtd: rawnand: atmel: Fix ECC error on SAM9 Zixun LI 2025-11-06 11:12 ` [PATCH 1/2] mtd: rawnand: atmel: set pmecc data setup time Zixun LI @ 2025-11-06 11:12 ` Zixun LI 2025-11-06 16:27 ` Alexander Dahl 2025-11-07 10:56 ` Eugen Hristev 2025-11-07 10:48 ` [PATCH 0/2] mtd: rawnand: atmel: Fix ECC error on SAM9 Eugen Hristev 2 siblings, 2 replies; 12+ messages in thread From: Zixun LI @ 2025-11-06 11:12 UTC (permalink / raw) To: Balamanikandan Gunasundar, u-boot Cc: Dario Binacchi, Michael Trimarchi, Tom Rini, Alexander Dahl, 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 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH 2/2] mtd: rawnand: atmel: Remove timing from atmel_pmecc struct 2025-11-06 11:12 ` [PATCH 2/2] mtd: rawnand: atmel: Remove timing from atmel_pmecc struct Zixun LI @ 2025-11-06 16:27 ` Alexander Dahl 2025-11-07 10:56 ` Eugen Hristev 1 sibling, 0 replies; 12+ messages in thread From: Alexander Dahl @ 2025-11-06 16:27 UTC (permalink / raw) To: Zixun LI Cc: Balamanikandan Gunasundar, u-boot, Dario Binacchi, Michael Trimarchi, Tom Rini, 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 > ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH 2/2] mtd: rawnand: atmel: Remove timing from atmel_pmecc struct 2025-11-06 11:12 ` [PATCH 2/2] mtd: rawnand: atmel: Remove timing from atmel_pmecc struct Zixun LI 2025-11-06 16:27 ` Alexander Dahl @ 2025-11-07 10:56 ` Eugen Hristev 2025-11-07 11:04 ` Michael Nazzareno Trimarchi 1 sibling, 1 reply; 12+ messages in thread From: Eugen Hristev @ 2025-11-07 10:56 UTC (permalink / raw) To: Zixun LI, Balamanikandan Gunasundar, u-boot Cc: Dario Binacchi, Michael Trimarchi, Tom Rini, Alexander Dahl 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 */ > ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 2/2] mtd: rawnand: atmel: Remove timing from atmel_pmecc struct 2025-11-07 10:56 ` Eugen Hristev @ 2025-11-07 11:04 ` Michael Nazzareno Trimarchi 2025-11-07 11:17 ` Alexander Dahl 0 siblings, 1 reply; 12+ messages in thread From: Michael Nazzareno Trimarchi @ 2025-11-07 11:04 UTC (permalink / raw) To: Eugen Hristev Cc: Zixun LI, Balamanikandan Gunasundar, u-boot, Dario Binacchi, Tom Rini, Alexander Dahl 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 ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 2/2] mtd: rawnand: atmel: Remove timing from atmel_pmecc struct 2025-11-07 11:04 ` Michael Nazzareno Trimarchi @ 2025-11-07 11:17 ` Alexander Dahl 0 siblings, 0 replies; 12+ messages in thread From: Alexander Dahl @ 2025-11-07 11:17 UTC (permalink / raw) To: Michael Nazzareno Trimarchi Cc: Eugen Hristev, Zixun LI, Balamanikandan Gunasundar, u-boot, Dario Binacchi, Tom Rini, 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 ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 0/2] mtd: rawnand: atmel: Fix ECC error on SAM9 2025-11-06 11:12 [PATCH 0/2] mtd: rawnand: atmel: Fix ECC error on SAM9 Zixun LI 2025-11-06 11:12 ` [PATCH 1/2] mtd: rawnand: atmel: set pmecc data setup time Zixun LI 2025-11-06 11:12 ` [PATCH 2/2] mtd: rawnand: atmel: Remove timing from atmel_pmecc struct Zixun LI @ 2025-11-07 10:48 ` Eugen Hristev 2 siblings, 0 replies; 12+ messages in thread From: Eugen Hristev @ 2025-11-07 10:48 UTC (permalink / raw) To: Zixun LI, Balamanikandan Gunasundar, u-boot Cc: Dario Binacchi, Michael Trimarchi, Tom Rini, Alexander Dahl 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 ^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2025-11-11 10:29 UTC | newest] Thread overview: 12+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2025-11-06 11:12 [PATCH 0/2] mtd: rawnand: atmel: Fix ECC error on SAM9 Zixun LI 2025-11-06 11:12 ` [PATCH 1/2] mtd: rawnand: atmel: set pmecc data setup time Zixun LI 2025-11-07 10:52 ` Eugen Hristev 2025-11-07 11:01 ` Alexander Dahl 2025-11-07 12:53 ` Zixun LI 2025-11-11 10:29 ` Eugen Hristev 2025-11-06 11:12 ` [PATCH 2/2] mtd: rawnand: atmel: Remove timing from atmel_pmecc struct Zixun LI 2025-11-06 16:27 ` Alexander Dahl 2025-11-07 10:56 ` Eugen Hristev 2025-11-07 11:04 ` Michael Nazzareno Trimarchi 2025-11-07 11:17 ` Alexander Dahl 2025-11-07 10:48 ` [PATCH 0/2] mtd: rawnand: atmel: Fix ECC error on SAM9 Eugen Hristev
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox