Re: [PATCH] Revert "mtd: rawnand: denali: get ->setup_data_interface() working again"

From: Masahiro Yamada <masahiroy@kernel.org>
To: Marek Vasut <marex@denx.de>
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: Mon, 9 Mar 2020 19:27:31 +0900	[thread overview]
Message-ID: <CAK7LNARDp1Zru3_XAQyBZHwsRov-QWOnrrwodS8pjszsKy2ysw@mail.gmail.com> (raw)
In-Reply-To: <e86925d0-805c-d364-57ca-6400b77cffd7@denx.de>

Hi.

On Wed, Mar 4, 2020 at 2:13 AM Marek Vasut <marex@denx.de> wrote:
>
> On 2/25/20 1:41 AM, Masahiro Yamada wrote:
> > Hi.
>
> Hi,
>
> > On Thu, Feb 20, 2020 at 3:45 AM Marek Vasut <marex@denx.de> wrote:
> >>
> >> 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?
> >
> >
> >
> > I am still waiting for you to let me know
> > the result of my patches.
>
> Neither patch works, sorry.
>
> >> 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 ?
> >
> > denali_select_target() is called every operation.
> > So, if you dumped this function for a working platform,
> > it might flood the printk buffer.
> >
> > denali_setup_data_interface() is called just twice.
> > That's why I injected the debug code there.
> >
> >
> >>
> >> 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);


-- 
Best Regards
Masahiro Yamada

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