Hi
On Mon, Feb 17, 2020 at 5:33 PM Masahiro Yamada <masahiroy@xxxxxxxxxx> wrote:
>
> Hi,
>
>
>
> On Thu, Feb 13, 2020 at 2:45 AM Marek Vasut <marex@xxxxxxx> wrote:
> >
> > On 2/12/20 5:56 PM, Masahiro Yamada wrote:
> > > Hi.
> >
> > Hi,
> >
> > [...]
> >
> > >>>>>>>>>>> On SoCFPGA, denali->clk_rate = 31.25 MHz and denali->clk_x_rate = 125 MHz,
> > >>>>>>>>>>> hence the driver sets NAND_KEEP_TIMINGS flag.
> > >>>
> > >>>
> > >>> Interesting.
> > >>> I have never seen such clock rates before.
> > >>>
> > >>>
> > >>> For all known upstream platforms
> > >>> (Altera SOCFPGA, Socionext UniPhier, Intel MRST),
> > >>> the NAND controller core clock is 50 MHz,
> > >>> the NAND bus clock is 200MHz.
> > >>
> > >> You can configure whatever rate you want in the QSys HPS component.
> > >
> > > OK.
> > >
> > > The SOCFPGA maintainer, Dinh Nguyen, said this:
> > > "From the clock controller, it provides a single 200MHz clock to the NAND
> > > IP. Inside the NAND IP, there is a /4 for the clk. The 200MHz clock is
> > > used for the clk_x and ecc_clk."
> >
> > That sounds like some root clock. You can read the entire documentation
> > for the SoCFPGA CV/AV here:
> > http://www.altera.com/literature/hb/cyclone-v/cv_5v4.pdf
> > See section 3 , look for 'nand' there. Note that NAND can be supplied
> > from at least two different PLLs -- main and peripheral.
> >
> > > http://lists.infradead.org/pipermail/linux-arm-kernel/2018-July/592702.html
> > >
> > >
> > >
> > > Maybe, you are using a brand-new,
> > > different type of SOCFPGA?
> >
> > Cyclone V and Arria V , so no.
> >
> > >>> What would happen if you hard-code:
> > >>> denali->clk_rate = 50000000;
> > >>> denali->clk_x_rate = 200000000;
> > >>
> > >> It will not work, because the IP would be using incorrect clock.
> > >
> > > I wanted to see the past tense here instead of
> > > future tense + subjunctive mood.
> > >
> > > I wanted you to try it.
> > >
> > >
> > >
> > >>
> > >>> like I had already suggested to Tim Sander:
> > >>> https://lore.kernel.org/lkml/CAK7LNAQOCoJC0RzOhTEofHdR+zU5sQTxV-t4nERBExW1ddW5hw@xxxxxxxxxxxxxx/
> > >>>
> > >>> Unfortunately, he did not want to do it, but
> > >>> I am still interested in this experiment because
> > >>> I suspect this might be a bug of drivers/clk/socfpga/.
> > >>
> > >> No, this is a feature of the platform, you can configure any clock you
> > >> want pretty much.
> > >
> > >
> > > OK, but if you agree the 4.19.10 is working,
> > >
> > > denali->clk_rate = 50000000; denali->clk_x_rate = 200000000;
> > >
> > > is worth trying.
> >
> > Why would misconfiguring the IP be worth trying ?
>
>
>
> There is no change around the ->setup_data_interface() hook
> after v4.19
> The only difference I could think of is the clock frequency.
>
> But, it is OK if you do not want to test it.
>
> And you are confident.
>
> So, let's suspect the ->setup_data_interface() hook.
>
>
> If possible, can you provide the dump of
> the attached debug code?
>
I attached two experimental patches.
I cannot test them because
the mainline code works fine for my boards.
Does either of them improve something
on your settings?
--
Best Regards
Masahiro Yamada
From 42c3dc3a3e79365b6fb467edbd49630e697fffc9 Mon Sep 17 00:00:00 2001
From: Masahiro Yamada <yamada.masahiro@xxxxxxxxxxxxx>
Date: Mon, 17 Feb 2020 21:16:33 +0900
Subject: [PATCH] denali: more complicated calculation for timings
Signed-off-by: Masahiro Yamada <yamada.masahiro@xxxxxxxxxxxxx>
---
drivers/mtd/nand/raw/denali.c | 64 +++++++++++++++++++++++++++++------
1 file changed, 53 insertions(+), 11 deletions(-)
diff --git a/drivers/mtd/nand/raw/denali.c b/drivers/mtd/nand/raw/denali.c
index fafd0a0aa8e2..a16b2f1f50b5 100644
--- a/drivers/mtd/nand/raw/denali.c
+++ b/drivers/mtd/nand/raw/denali.c
@@ -796,15 +796,6 @@ static int denali_setup_data_interface(struct nand_chip *chip, int chipnr,
sel = &to_denali_chip(chip)->sels[chipnr];
- /* tREA -> ACC_CLKS */
- acc_clks = DIV_ROUND_UP(timings->tREA_max, t_x);
- acc_clks = min_t(int, acc_clks, ACC_CLKS__VALUE);
-
- tmp = ioread32(denali->reg + ACC_CLKS);
- tmp &= ~ACC_CLKS__VALUE;
- tmp |= FIELD_PREP(ACC_CLKS__VALUE, acc_clks);
- sel->acc_clks = tmp;
-
/* tRWH -> RE_2_WE */
re_2_we = DIV_ROUND_UP(timings->tRHW_min, t_x);
re_2_we = min_t(int, re_2_we, RE_2_WE__VALUE);
@@ -862,14 +853,39 @@ static int denali_setup_data_interface(struct nand_chip *chip, int chipnr,
tmp |= FIELD_PREP(RDWR_EN_HI_CNT__VALUE, rdwr_en_hi);
sel->rdwr_en_hi_cnt = tmp;
- /* tRP, tWP -> RDWR_EN_LO_CNT */
+ /*
+ * tREA -> ACC_CLOCKS
+ * tRP, tWP, tRHOH, tRC, tWC -> RDWR_EN_LO_CNT
+ */
+
+ /*
+ * Determine the minimum of acc_clks to meet the data setup timing.
+ * (one additional clock cycle just in case)
+ */
+ acc_clks = DIV_ROUND_UP(timings->tREA_max, t_x) + 1;
+
+ /* Determine the minimum of rdwr_en_lo_cnt from RE#/WE# pulse width */
rdwr_en_lo = DIV_ROUND_UP(max(timings->tRP_min, timings->tWP_min), t_x);
+
+ /* Extend rdwr_en_lo to meet the data hold timing */
+ rdwr_en_lo = max_t(int, rdwr_en_lo, acc_clks - timings->tRHOH_min / t_x);
+
+ /* Extend rdwr_en_lo to meet the requirement for RE#/WE# cycle time */
rdwr_en_lo_hi = DIV_ROUND_UP(max(timings->tRC_min, timings->tWC_min),
t_x);
- rdwr_en_lo_hi = max_t(int, rdwr_en_lo_hi, mult_x);
rdwr_en_lo = max(rdwr_en_lo, rdwr_en_lo_hi - rdwr_en_hi);
rdwr_en_lo = min_t(int, rdwr_en_lo, RDWR_EN_LO_CNT__VALUE);
+ /* Center the data latch timing for extra safety */
+ acc_clks = (acc_clks + rdwr_en_lo +
+ DIV_ROUND_UP(timings->tRHOH_min, t_x)) / 2;
+ acc_clks = min_t(int, acc_clks, ACC_CLKS__VALUE);
+
+ tmp = ioread32(denali->reg + ACC_CLKS);
+ tmp &= ~ACC_CLKS__VALUE;
+ tmp |= FIELD_PREP(ACC_CLKS__VALUE, acc_clks);
+ sel->acc_clks = tmp;
+
tmp = ioread32(denali->reg + RDWR_EN_LO_CNT);
tmp &= ~RDWR_EN_LO_CNT__VALUE;
tmp |= FIELD_PREP(RDWR_EN_LO_CNT__VALUE, rdwr_en_lo);
@@ -886,6 +902,32 @@ static int denali_setup_data_interface(struct nand_chip *chip, int chipnr,
tmp |= FIELD_PREP(CS_SETUP_CNT__VALUE, cs_setup);
sel->cs_setup_cnt = tmp;
+ /* debug */
+ printk("Denali: clk_rate=%ld, clk_x_rate=%ld\n", denali->clk_rate, denali->clk_x_rate);
+ printk("Denali: tREA=%d\n", timings->tREA_max);
+ printk("Denali: tRHW=%d\n", timings->tRHW_min);
+ printk("Denali: tRHZ=%d\n", timings->tRHZ_max);
+ printk("Denali: tCCS=%d\n", timings->tCCS_min);
+ printk("Denali: tWHR=%d\n", timings->tWHR_min);
+ printk("Denali: tADL=%d\n", timings->tADL_min);
+ printk("Denali: tREH=%d\n", timings->tREH_min);
+ printk("Denali: tWH=%d\n", timings->tWH_min);
+ printk("Denali: tRP=%d\n", timings->tRP_min);
+ printk("Denali: tWP=%d\n", timings->tWP_min);
+ printk("Denali: tRC=%d\n", timings->tRC_min);
+ printk("Denali: tWC=%d\n", timings->tWC_min);
+ printk("Denali: tCS=%d\n", timings->tCS_min);
+ printk("Denali: tCEA=%d\n", timings->tCEA_max);
+ printk("Denali: acc_clks=%d\n", acc_clks);
+ printk("Denali: re_2_we=%d\n", re_2_we);
+ printk("Denali: re_2_re=%d\n", re_2_re);
+ printk("Denali: we_2_re=%d\n", we_2_re);
+ printk("Denali: addr_2_data=%d\n", addr_2_data);
+ printk("Denali: rdwr_en_hi=%d\n", rdwr_en_hi);
+ printk("Denali: rdwr_en_lo_hi=%d\n", rdwr_en_lo_hi);
+ printk("Denali: rdwr_en_lo=%d\n", rdwr_en_lo);
+ printk("Denali: cs_setup=%d\n", cs_setup);
+
return 0;
}
--
2.17.1
From ff8eff942017a58e7f4de36cbb71b1de59a1dfbf Mon Sep 17 00:00:00 2001
From: Masahiro Yamada <yamada.masahiro@xxxxxxxxxxxxx>
Date: Mon, 17 Feb 2020 20:22:48 +0900
Subject: [PATCH] denali: timing hack 1
Increase setup time of data.
(but may not meet the hold time requirement)
Signed-off-by: Masahiro Yamada <yamada.masahiro@xxxxxxxxxxxxx>
---
drivers/mtd/nand/raw/denali.c | 28 +++++++++++++++++++++++++++-
1 file changed, 27 insertions(+), 1 deletion(-)
diff --git a/drivers/mtd/nand/raw/denali.c b/drivers/mtd/nand/raw/denali.c
index fafd0a0aa8e2..762fa8bc3e4c 100644
--- a/drivers/mtd/nand/raw/denali.c
+++ b/drivers/mtd/nand/raw/denali.c
@@ -798,7 +798,7 @@ static int denali_setup_data_interface(struct nand_chip *chip, int chipnr,
/* tREA -> ACC_CLKS */
acc_clks = DIV_ROUND_UP(timings->tREA_max, t_x);
- acc_clks = min_t(int, acc_clks, ACC_CLKS__VALUE);
+ acc_clks = min_t(int, acc_clks, ACC_CLKS__VALUE) + 1;
tmp = ioread32(denali->reg + ACC_CLKS);
tmp &= ~ACC_CLKS__VALUE;
@@ -886,6 +886,32 @@ static int denali_setup_data_interface(struct nand_chip *chip, int chipnr,
tmp |= FIELD_PREP(CS_SETUP_CNT__VALUE, cs_setup);
sel->cs_setup_cnt = tmp;
+ /* debug */
+ printk("Denali: clk_rate=%ld, clk_x_rate=%ld\n", denali->clk_rate, denali->clk_x_rate);
+ printk("Denali: tREA=%d\n", timings->tREA_max);
+ printk("Denali: tRHW=%d\n", timings->tRHW_min);
+ printk("Denali: tRHZ=%d\n", timings->tRHZ_max);
+ printk("Denali: tCCS=%d\n", timings->tCCS_min);
+ printk("Denali: tWHR=%d\n", timings->tWHR_min);
+ printk("Denali: tADL=%d\n", timings->tADL_min);
+ printk("Denali: tREH=%d\n", timings->tREH_min);
+ printk("Denali: tWH=%d\n", timings->tWH_min);
+ printk("Denali: tRP=%d\n", timings->tRP_min);
+ printk("Denali: tWP=%d\n", timings->tWP_min);
+ printk("Denali: tRC=%d\n", timings->tRC_min);
+ printk("Denali: tWC=%d\n", timings->tWC_min);
+ printk("Denali: tCS=%d\n", timings->tCS_min);
+ printk("Denali: tCEA=%d\n", timings->tCEA_max);
+ printk("Denali: acc_clks=%d\n", acc_clks);
+ printk("Denali: re_2_we=%d\n", re_2_we);
+ printk("Denali: re_2_re=%d\n", re_2_re);
+ printk("Denali: we_2_re=%d\n", we_2_re);
+ printk("Denali: addr_2_data=%d\n", addr_2_data);
+ printk("Denali: rdwr_en_hi=%d\n", rdwr_en_hi);
+ printk("Denali: rdwr_en_lo_hi=%d\n", rdwr_en_lo_hi);
+ printk("Denali: rdwr_en_lo=%d\n", rdwr_en_lo);
+ printk("Denali: cs_setup=%d\n", cs_setup);
+
return 0;
}
--
2.17.1
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
