linux-mtd.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
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, 19 Feb 2020 19:42:28 +0100	[thread overview]
Message-ID: <311cdc3c-59b5-a46b-62f0-e78fc970134a@denx.de> (raw)
In-Reply-To: <CAK7LNAT3EG0XocC0xT0f=6MBpXLga3FehOjEYbRyP6AJUbqb2Q@mail.gmail.com>

[-- Attachment #1: Type: text/plain, Size: 1368 bytes --]

On 2/18/20 6:55 AM, Masahiro Yamada wrote:
> Hi

Hi,

[...]

>> 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?

Considering that the NAND works if denali_setup_data_interface() is not
called, would it rather make sense to first read and print what's
programmed into the controller and then print what the code calculated
and intends to program into the controller ?

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

[-- Attachment #2: 0001-denali-dump-timing-parameters.patch --]
[-- Type: text/x-patch, Size: 6264 bytes --]

>From a2a1300041979f183a5a85ddada63faa80b68983 Mon Sep 17 00:00:00 2001
From: Masahiro Yamada <masahiroy@kernel.org>
Date: Mon, 17 Feb 2020 16:48:06 +0900
Subject: [PATCH] denali: dump timing parameters

:'<,'>s@iowrite32(\([^,]\+\), \([^)]\+\));@pr_err("\2 = 0x%08x -> 0x%08x\\n", ioread32(\2), \1);\r\t&

Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
---
 drivers/mtd/nand/raw/denali.c | 44 +++++++++++++++++++++++++++++++++--
 1 file changed, 42 insertions(+), 2 deletions(-)

diff --git a/drivers/mtd/nand/raw/denali.c b/drivers/mtd/nand/raw/denali.c
index b6c463d02167..4241b47d92a6 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);
 }
 
@@ -795,6 +802,8 @@ static int denali_setup_data_interface(struct nand_chip *chip, int chipnr,
 	if (chipnr == NAND_DATA_IFACE_CHECK_ONLY)
 		return 0;
 
+	printk("Denali: clk_rate=%ld, clk_x_rate=%ld\n", denali->clk_rate, denali->clk_x_rate);
+
 	sel = &to_denali_chip(chip)->sels[chipnr];
 
 	/* tREA -> ACC_CLKS */
@@ -806,10 +815,16 @@ static int denali_setup_data_interface(struct nand_chip *chip, int chipnr,
 	tmp |= FIELD_PREP(ACC_CLKS__VALUE, acc_clks);
 	sel->acc_clks = tmp;
 
+	printk("Denali: tREA=%d\n", timings->tREA_max);
+	printk("Denali: acc_clks=%d\n", acc_clks);
+
 	/* 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);
 
+	printk("Denali: tRHW=%d\n", timings->tRHW_min);
+	printk("Denali: re_2_we=%d\n", re_2_we);
+
 	tmp = ioread32(denali->reg + RE_2_WE);
 	tmp &= ~RE_2_WE__VALUE;
 	tmp |= FIELD_PREP(RE_2_WE__VALUE, re_2_we);
@@ -819,6 +834,9 @@ static int denali_setup_data_interface(struct nand_chip *chip, int chipnr,
 	re_2_re = DIV_ROUND_UP(timings->tRHZ_max, t_x);
 	re_2_re = min_t(int, re_2_re, RE_2_RE__VALUE);
 
+	printk("Denali: tRHZ=%d\n", timings->tRHZ_max);
+	printk("Denali: re_2_re=%d\n", re_2_re);
+
 	tmp = ioread32(denali->reg + RE_2_RE);
 	tmp &= ~RE_2_RE__VALUE;
 	tmp |= FIELD_PREP(RE_2_RE__VALUE, re_2_re);
@@ -833,6 +851,10 @@ static int denali_setup_data_interface(struct nand_chip *chip, int chipnr,
 	we_2_re = DIV_ROUND_UP(max(timings->tCCS_min, timings->tWHR_min), t_x);
 	we_2_re = min_t(int, we_2_re, TWHR2_AND_WE_2_RE__WE_2_RE);
 
+	printk("Denali: tCCS=%d\n", timings->tCCS_min);
+	printk("Denali: tWHR=%d\n", timings->tWHR_min);
+	printk("Denali: we_2_re=%d\n", we_2_re);
+
 	tmp = ioread32(denali->reg + TWHR2_AND_WE_2_RE);
 	tmp &= ~TWHR2_AND_WE_2_RE__WE_2_RE;
 	tmp |= FIELD_PREP(TWHR2_AND_WE_2_RE__WE_2_RE, we_2_re);
@@ -848,6 +870,9 @@ static int denali_setup_data_interface(struct nand_chip *chip, int chipnr,
 	addr_2_data = DIV_ROUND_UP(timings->tADL_min, t_x);
 	addr_2_data = min_t(int, addr_2_data, addr_2_data_mask);
 
+	printk("Denali: tADL=%d\n", timings->tADL_min);
+	printk("Denali: addr_2_data=%d\n", addr_2_data);
+
 	tmp = ioread32(denali->reg + TCWAW_AND_ADDR_2_DATA);
 	tmp &= ~TCWAW_AND_ADDR_2_DATA__ADDR_2_DATA;
 	tmp |= FIELD_PREP(TCWAW_AND_ADDR_2_DATA__ADDR_2_DATA, addr_2_data);
@@ -858,6 +883,10 @@ static int denali_setup_data_interface(struct nand_chip *chip, int chipnr,
 				  t_x);
 	rdwr_en_hi = min_t(int, rdwr_en_hi, RDWR_EN_HI_CNT__VALUE);
 
+	printk("Denali: tREH=%d\n", timings->tREH_min);
+	printk("Denali: tWH=%d\n", timings->tWH_min);
+	printk("Denali: rdwr_en_hi=%d\n", rdwr_en_hi);
+
 	tmp = ioread32(denali->reg + RDWR_EN_HI_CNT);
 	tmp &= ~RDWR_EN_HI_CNT__VALUE;
 	tmp |= FIELD_PREP(RDWR_EN_HI_CNT__VALUE, rdwr_en_hi);
@@ -871,6 +900,13 @@ static int denali_setup_data_interface(struct nand_chip *chip, int chipnr,
 	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);
 
+	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: rdwr_en_lo_hi=%d\n", rdwr_en_lo_hi);
+	printk("Denali: rdwr_en_lo=%d\n", rdwr_en_lo);
+
 	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);
@@ -882,6 +918,10 @@ static int denali_setup_data_interface(struct nand_chip *chip, int chipnr,
 			0);
 	cs_setup = min_t(int, cs_setup, CS_SETUP_CNT__VALUE);
 
+	printk("Denali: tCS=%d\n", timings->tCS_min);
+	printk("Denali: tCEA=%d\n", timings->tCEA_max);
+	printk("Denali: cs_setup=%d\n", cs_setup);
+
 	tmp = ioread32(denali->reg + CS_SETUP_CNT);
 	tmp &= ~CS_SETUP_CNT__VALUE;
 	tmp |= FIELD_PREP(CS_SETUP_CNT__VALUE, cs_setup);
-- 
2.24.1


[-- Attachment #3: Type: text/plain, Size: 144 bytes --]

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

  reply	other threads:[~2020-02-19 18:45 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 [this message]
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
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=311cdc3c-59b5-a46b-62f0-e78fc970134a@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).