[PATCH] mtd: rawnand: pl353: make sure optimal timings are applied

[PATCH] mtd: rawnand: pl353: make sure optimal timings are applied

[Olivier Sobrie]

Timings of the nand are adjusted by pl35x_nfc_setup_interface() but
actually applied by the pl35x_nand_select_target() function.
If there is only one nand chip, the pl35x_nand_select_target() will only
apply the timings once since the test at its beginning will always be true
after the first call to this function. As a result, the hardware will
keep using the default timings set at boot to detect the nand chip, not
the optimal ones.

With this patch, we program directly the new timings when
pl35x_nfc_setup_interface() is called.

Signed-off-by: Olivier Sobrie <olivier@sobrie.be>
---
 drivers/mtd/nand/raw/pl35x-nand-controller.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/mtd/nand/raw/pl35x-nand-controller.c b/drivers/mtd/nand/raw/pl35x-nand-controller.c
index 947fd86ac5fa..f2c65eb7a8d9 100644
--- a/drivers/mtd/nand/raw/pl35x-nand-controller.c
+++ b/drivers/mtd/nand/raw/pl35x-nand-controller.c
@@ -862,6 +862,9 @@ static int pl35x_nfc_setup_interface(struct nand_chip *chip, int cs,
 			  PL35X_SMC_NAND_TAR_CYCLES(tmgs.t_ar) |
 			  PL35X_SMC_NAND_TRR_CYCLES(tmgs.t_rr);
 
+	writel(plnand->timings, nfc->conf_regs + PL35X_SMC_CYCLES);
+	pl35x_smc_update_regs(nfc);
+
 	return 0;
 }
 
-- 
2.53.0


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH] mtd: rawnand: pl353: make sure optimal timings are applied

[Miquel Raynal]

Hello Olivier,

+ Andrea

On 04/03/2026 at 22:18:23 +01, Olivier Sobrie <olivier@sobrie.be> wrote:

> Timings of the nand are adjusted by pl35x_nfc_setup_interface() but
> actually applied by the pl35x_nand_select_target() function.
> If there is only one nand chip, the pl35x_nand_select_target() will only
> apply the timings once since the test at its beginning will always be true
> after the first call to this function. As a result, the hardware will
> keep using the default timings set at boot to detect the nand chip, not
> the optimal ones.
>
> With this patch, we program directly the new timings when
> pl35x_nfc_setup_interface() is called.
>
> Signed-off-by: Olivier Sobrie <olivier@sobrie.be>

Nice, this probably will deserve a Fixes tag and Cc: stable.

Andrea, this might be the reason why your NAND chip misbehaves after a
set_feature. Would you mind testing it?

Thanks,
Miquèl

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH] mtd: rawnand: pl353: make sure optimal timings are applied

[Olivier Sobrie]

Hello Miquel,

On Mon, Mar 16, 2026 at 09:46:39AM +0100, Miquel Raynal wrote:
> Hello Olivier,
> 
> + Andrea
> 
> On 04/03/2026 at 22:18:23 +01, Olivier Sobrie <olivier@sobrie.be> wrote:
> 
> > Timings of the nand are adjusted by pl35x_nfc_setup_interface() but
> > actually applied by the pl35x_nand_select_target() function.
> > If there is only one nand chip, the pl35x_nand_select_target() will only
> > apply the timings once since the test at its beginning will always be true
> > after the first call to this function. As a result, the hardware will
> > keep using the default timings set at boot to detect the nand chip, not
> > the optimal ones.
> >
> > With this patch, we program directly the new timings when
> > pl35x_nfc_setup_interface() is called.
> >
> > Signed-off-by: Olivier Sobrie <olivier@sobrie.be>
> 
> Nice, this probably will deserve a Fixes tag and Cc: stable.

Do you implicitely suggest that I should send a v2 of this patch
with Fixes tag and stable in CC? Or do you take care of that?

Thanks,

Olivier

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH] mtd: rawnand: pl353: make sure optimal timings are applied

[Miquel Raynal]

>> > With this patch, we program directly the new timings when
>> > pl35x_nfc_setup_interface() is called.
>> >
>> > Signed-off-by: Olivier Sobrie <olivier@sobrie.be>
>> 
>> Nice, this probably will deserve a Fixes tag and Cc: stable.
>
> Do you implicitely suggest that I should send a v2 of this patch
> with Fixes tag and stable in CC? Or do you take care of that?

Please send a v2, indeed!

Thanks,
Miquèl

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

RE: [PATCH] mtd: rawnand: pl353: make sure optimal timings are applied

[Andrea Scian]

Dear Miquel,

> -----Original Message-----
> From: Miquel Raynal <miquel.raynal@bootlin.com>
> Sent: lunedì 16 marzo 2026 09:47
> [snip] 
> Andrea, this might be the reason why your NAND chip misbehaves after a
> set_feature. Would you mind testing it?

Thanks for pointing me to this patch.
I got a similar one in my tree (sorry for not publishing it before, but ECC was a bigger problem ;-) ), that applies new timing like on Olivier patch but only when they change


commit 35ba37433ebef77ab1f0e47e79e69bbc0d21cf90
Author: Andrea Scian <andrea.scian@dave.eu>
Date:   Mon Jan 12 17:49:37 2026 +0100

    mtd: rawnand: pl353: update timings on changes
    
    It's not enough to wait for selecting another chip to
    update the timings, because this also happens without
    changing CS (e.g. when switching between different SDR modes)

diff --git a/drivers/mtd/nand/raw/pl35x-nand-controller.c b/drivers/mtd/nand/raw/pl35x-nand-controller.c
index 74cc37e7b7da..72463638148d 100644
--- a/drivers/mtd/nand/raw/pl35x-nand-controller.c
+++ b/drivers/mtd/nand/raw/pl35x-nand-controller.c
@@ -790,6 +790,7 @@ static int pl35x_nfc_setup_interface(struct nand_chip *chip, int cs,
 	const struct nand_sdr_timings *sdr;
 	unsigned int period_ns, val;
 	struct clk *mclk;
+	u32 new_timings;
 
 	sdr = nand_get_sdr_timings(conf);
 	if (IS_ERR(sdr))
@@ -857,13 +858,18 @@ static int pl35x_nfc_setup_interface(struct nand_chip *chip, int cs,
 	if (cs == NAND_DATA_IFACE_CHECK_ONLY)
 		return 0;
 
-	plnand->timings = PL35X_SMC_NAND_TRC_CYCLES(tmgs.t_rc) |
+	new_timings =  PL35X_SMC_NAND_TRC_CYCLES(tmgs.t_rc) |
 			  PL35X_SMC_NAND_TWC_CYCLES(tmgs.t_wc) |
 			  PL35X_SMC_NAND_TREA_CYCLES(tmgs.t_rea) |
 			  PL35X_SMC_NAND_TWP_CYCLES(tmgs.t_wp) |
 			  PL35X_SMC_NAND_TCLR_CYCLES(tmgs.t_clr) |
 			  PL35X_SMC_NAND_TAR_CYCLES(tmgs.t_ar) |
 			  PL35X_SMC_NAND_TRR_CYCLES(tmgs.t_rr);
+	if (plnand->timings != new_timings) {
+		plnand->timings = new_timings;
+		writel(plnand->timings, nfc->conf_regs + PL35X_SMC_CYCLES);
+		pl35x_smc_update_regs(nfc);
+	}
 
 	dev_info(nfc->dev,
 		 "NAND timings: tRC=%u, tWC=%u, tREA=%u, tWP=%u, tCLR=%u, tAR=%u, tRR=%u (in cycles) (reg 0x%08x)\n",


This works on my setup (NAND now run a lot faster) but it doesn't solve the set_feature problem.
However, let me try this again with a cleaner setup

Kind Regards,

Andrea Scian
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/