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 <[email protected]>
>>> ---
>>> 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 = {
>>>
>>
