From: Marek Vasut <marex@denx.de>
To: Masahiro Yamada <masahiroy@kernel.org>
Cc: Dinh Nguyen <dinguyen@kernel.org>,
Boris Brezillon <boris.brezillon@collabora.com>,
linux-mtd <linux-mtd@lists.infradead.org>,
Tim Sander <tim@krieglstein.org>,
Miquel Raynal <miquel.raynal@bootlin.com>
Subject: Re: [PATCH] Revert "mtd: rawnand: denali: get ->setup_data_interface() working again"
Date: Wed, 11 Mar 2020 13:52:30 +0100 [thread overview]
Message-ID: <d4b4abf4-1af9-d57c-5b93-2d56a5dc456b@denx.de> (raw)
In-Reply-To: <CAK7LNARDp1Zru3_XAQyBZHwsRov-QWOnrrwodS8pjszsKy2ysw@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 3827 bytes --]
On 3/9/20 11:27 AM, Masahiro Yamada wrote:
> Hi.
Hi,
[...]
>>>> See attached patch, with which (without this revert) you get this:
>>>> denali->reg + TWHR2_AND_WE_2_RE = 0x00001414 -> 0x0000143f
>>>> denali->reg + TCWAW_AND_ADDR_2_DATA = 0x0000143f -> 0x00001432
>>>> denali->reg + RE_2_WE = 0x00000014 -> 0x00000019
>>>> denali->reg + ACC_CLKS = 0x00000004 -> 0x00000005
>>>> denali->reg + RDWR_EN_LO_CNT = 0x00000002 -> 0x00000009
>>>> denali->reg + RDWR_EN_HI_CNT = 0x00000002 -> 0x00000004
>>>> denali->reg + CS_SETUP_CNT = 0x00000001 -> 0x00000008
>>>> denali->reg + RE_2_RE = 0x00000014 -> 0x00000019
>>>
>>> OK, the left-hand side is probably the timing
>>> set up by U-Boot.
>>
>> Yep, the timings that work. So now, how do you get to those working
>> timings using the Linux driver ?
>
>
> How about
> 0001-denali-more-complicated-calculation-for-timings.patch
>
> + following ?
>
> diff --git a/drivers/mtd/nand/raw/denali.c b/drivers/mtd/nand/raw/denali.c
> index b0482108a127..ea38aa42873e 100644
> --- a/drivers/mtd/nand/raw/denali.c
> +++ b/drivers/mtd/nand/raw/denali.c
> @@ -860,9 +860,9 @@ static int denali_setup_data_interface(struct
> nand_chip *chip, int chipnr,
>
> /*
> * Determine the minimum of acc_clks to meet the data setup timing.
> - * (one additional clock cycle just in case)
> + * (two additional clock cycles just in case)
> */
> - acc_clks = DIV_ROUND_UP(timings->tREA_max, t_x) + 1;
> + acc_clks = DIV_ROUND_UP(timings->tREA_max, t_x) + 2;
>
> /* 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);
Like the attached one ?
That seems to work, but -- the calculated timings differ from the ones
which are calculated by U-Boot and which were tested to work well.
That's not good, I would expect both timings to be identical:
Denali: clk_rate=31250000, clk_x_rate=125000000
Denali: tREA=40000
Denali: tRHW=200000
Denali: tRHZ=200000
Denali: tCCS=500000000
Denali: tWHR=120000
Denali: tADL=400000
Denali: tREH=30000
Denali: tWH=30000
Denali: tRP=50000
Denali: tWP=50000
Denali: tRC=100000
Denali: tWC=100000
Denali: tCS=70000
Denali: tCEA=100000
Denali: acc_clks=8
Denali: re_2_we=25
Denali: re_2_re=25
Denali: we_2_re=63
Denali: addr_2_data=50
Denali: rdwr_en_hi=4
Denali: rdwr_en_lo_hi=13
Denali: rdwr_en_lo=9
Denali: cs_setup=5
denali->reg + TWHR2_AND_WE_2_RE = 0x00001414 -> 0x0000143f
denali->reg + TCWAW_AND_ADDR_2_DATA = 0x0000143f -> 0x00001432
denali->reg + RE_2_WE = 0x00000014 -> 0x00000019
denali->reg + ACC_CLKS = 0x00000004 -> 0x00000008
denali->reg + RDWR_EN_LO_CNT = 0x00000002 -> 0x00000009
denali->reg + RDWR_EN_HI_CNT = 0x00000002 -> 0x00000004
denali->reg + CS_SETUP_CNT = 0x00000001 -> 0x00000005
denali->reg + RE_2_RE = 0x00000014 -> 0x00000019
denali->reg + TWHR2_AND_WE_2_RE = 0x0000143f -> 0x0000143f
denali->reg + TCWAW_AND_ADDR_2_DATA = 0x00001432 -> 0x00001432
denali->reg + RE_2_WE = 0x00000019 -> 0x00000019
denali->reg + ACC_CLKS = 0x00000008 -> 0x00000008
denali->reg + RDWR_EN_LO_CNT = 0x00000009 -> 0x00000009
denali->reg + RDWR_EN_HI_CNT = 0x00000004 -> 0x00000004
denali->reg + CS_SETUP_CNT = 0x00000005 -> 0x00000005
denali->reg + RE_2_RE = 0x00000019 -> 0x00000019
denali->reg + TWHR2_AND_WE_2_RE = 0x0000143f -> 0x0000143f
denali->reg + TCWAW_AND_ADDR_2_DATA = 0x00001432 -> 0x00001432
denali->reg + RE_2_WE = 0x00000019 -> 0x00000019
denali->reg + ACC_CLKS = 0x00000008 -> 0x00000008
denali->reg + RDWR_EN_LO_CNT = 0x00000009 -> 0x00000009
denali->reg + RDWR_EN_HI_CNT = 0x00000004 -> 0x00000004
denali->reg + CS_SETUP_CNT = 0x00000005 -> 0x00000005
denali->reg + RE_2_RE = 0x00000019 -> 0x00000019
...
--
Best regards,
Marek Vasut
[-- Attachment #2: 0001-denali-more-complicated-calculation-for-timings.patch --]
[-- Type: text/x-patch, Size: 5855 bytes --]
>From 5fd2ee8e18de2490da347d0388481adb18a04408 Mon Sep 17 00:00:00 2001
From: Masahiro Yamada <yamada.masahiro@socionext.com>
Date: Mon, 17 Feb 2020 21:16:33 +0900
Subject: [PATCH] denali: more complicated calculation for timings
Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---
drivers/mtd/nand/raw/denali.c | 75 +++++++++++++++++++++++++++++------
1 file changed, 62 insertions(+), 13 deletions(-)
diff --git a/drivers/mtd/nand/raw/denali.c b/drivers/mtd/nand/raw/denali.c
index 5fe3c62a756e..ef5ebb8394f3 100644
--- a/drivers/mtd/nand/raw/denali.c
+++ b/drivers/mtd/nand/raw/denali.c
@@ -218,14 +218,21 @@ static void denali_select_target(struct nand_chip *chip, int cs)
return;
/* update timing registers unless NAND_KEEP_TIMINGS is set */
+ pr_err("denali->reg + TWHR2_AND_WE_2_RE = 0x%08x -> 0x%08x\n", ioread32(denali->reg + TWHR2_AND_WE_2_RE), sel->hwhr2_and_we_2_re);
iowrite32(sel->hwhr2_and_we_2_re, denali->reg + TWHR2_AND_WE_2_RE);
- iowrite32(sel->tcwaw_and_addr_2_data,
- denali->reg + TCWAW_AND_ADDR_2_DATA);
+ pr_err("denali->reg + TCWAW_AND_ADDR_2_DATA = 0x%08x -> 0x%08x\n", ioread32(denali->reg + TCWAW_AND_ADDR_2_DATA), sel->tcwaw_and_addr_2_data);
+ iowrite32(sel->tcwaw_and_addr_2_data, denali->reg + TCWAW_AND_ADDR_2_DATA);
+ pr_err("denali->reg + RE_2_WE = 0x%08x -> 0x%08x\n", ioread32(denali->reg + RE_2_WE), sel->re_2_we);
iowrite32(sel->re_2_we, denali->reg + RE_2_WE);
+ pr_err("denali->reg + ACC_CLKS = 0x%08x -> 0x%08x\n", ioread32(denali->reg + ACC_CLKS), sel->acc_clks);
iowrite32(sel->acc_clks, denali->reg + ACC_CLKS);
+ pr_err("denali->reg + RDWR_EN_LO_CNT = 0x%08x -> 0x%08x\n", ioread32(denali->reg + RDWR_EN_LO_CNT), sel->rdwr_en_lo_cnt);
iowrite32(sel->rdwr_en_lo_cnt, denali->reg + RDWR_EN_LO_CNT);
+ pr_err("denali->reg + RDWR_EN_HI_CNT = 0x%08x -> 0x%08x\n", ioread32(denali->reg + RDWR_EN_HI_CNT), sel->rdwr_en_hi_cnt);
iowrite32(sel->rdwr_en_hi_cnt, denali->reg + RDWR_EN_HI_CNT);
+ pr_err("denali->reg + CS_SETUP_CNT = 0x%08x -> 0x%08x\n", ioread32(denali->reg + CS_SETUP_CNT), sel->cs_setup_cnt);
iowrite32(sel->cs_setup_cnt, denali->reg + CS_SETUP_CNT);
+ pr_err("denali->reg + RE_2_RE = 0x%08x -> 0x%08x\n", ioread32(denali->reg + RE_2_RE), sel->re_2_re);
iowrite32(sel->re_2_re, denali->reg + RE_2_RE);
}
@@ -797,15 +804,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);
@@ -863,14 +861,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) + 2;
+
+ /* 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);
@@ -887,6 +910,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.25.0
[-- Attachment #3: Type: text/plain, Size: 144 bytes --]
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
next prev parent reply other threads:[~2020-03-11 12:53 UTC|newest]
Thread overview: 31+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-02-05 7:08 [PATCH] Revert "mtd: rawnand: denali: get ->setup_data_interface() working again" Marek Vasut
2020-02-05 9:12 ` Miquel Raynal
2020-02-05 9:41 ` Marek Vasut
2020-02-05 9:50 ` Miquel Raynal
2020-02-05 10:05 ` Boris Brezillon
2020-02-05 10:08 ` Marek Vasut
2020-02-11 10:04 ` Marek Vasut
2020-02-11 16:07 ` Miquel Raynal
2020-02-11 20:35 ` Marek Vasut
2020-02-12 9:00 ` Masahiro Yamada
2020-02-12 9:37 ` Marek Vasut
2020-02-12 16:56 ` Masahiro Yamada
2020-02-12 17:13 ` Masahiro Yamada
2020-02-12 17:44 ` Marek Vasut
2020-02-17 8:33 ` Masahiro Yamada
2020-02-18 5:55 ` Masahiro Yamada
2020-02-19 18:42 ` Marek Vasut
2020-02-25 0:41 ` Masahiro Yamada
2020-03-03 17:11 ` Marek Vasut
2020-03-09 10:27 ` Masahiro Yamada
2020-03-11 12:52 ` Marek Vasut [this message]
2020-03-11 13:08 ` Miquel Raynal
2020-03-11 13:19 ` Marek Vasut
2020-03-11 13:33 ` Miquel Raynal
2020-03-11 14:07 ` Marek Vasut
2020-03-11 14:39 ` Miquel Raynal
2020-03-14 14:48 ` Marek Vasut
2020-03-17 9:27 ` Masahiro Yamada
2020-03-16 4:36 ` Masahiro Yamada
2020-02-19 18:27 ` Marek Vasut
2020-02-25 0:38 ` Masahiro Yamada
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=d4b4abf4-1af9-d57c-5b93-2d56a5dc456b@denx.de \
--to=marex@denx.de \
--cc=boris.brezillon@collabora.com \
--cc=dinguyen@kernel.org \
--cc=linux-mtd@lists.infradead.org \
--cc=masahiroy@kernel.org \
--cc=miquel.raynal@bootlin.com \
--cc=tim@krieglstein.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).