From: Miquel Raynal <miquel.raynal@bootlin.com>
To: Masahiro Yamada <yamada.masahiro@socionext.com>
Cc: Marek Vasut <marex@denx.de>, Richard Weinberger <richard@nod.at>,
linux-mtd@lists.infradead.org,
Vignesh Raghavendra <vigneshr@ti.com>,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH] mtd: rawnand: denali: add more delays before latching incoming data
Date: Fri, 20 Mar 2020 18:11:59 +0100 [thread overview]
Message-ID: <20200320181159.5004099f@xps13> (raw)
In-Reply-To: <20200316104307.1891-1-yamada.masahiro@socionext.com>
Hi Marek,
Masahiro Yamada <yamada.masahiro@socionext.com> wrote on Mon, 16 Mar
2020 19:43:07 +0900:
> The Denali IP have several registers to specify how many clock cycles
> should be waited between falling/rising signals. You can improve the
> NAND access performance by programming these registers with optimized
> values.
>
> Because struct nand_sdr_timings represents the device requirement
> in pico seconds, denali_setup_data_interface() computes the register
> values by dividing the device timings with the clock period.
>
> Marek Vasut reported this driver in the latest kernel does not work
> on his SOCFPGA board. (The on-board NAND chip is mode 5)
>
> The suspicious parameter is acc_clks, so this commit relaxes it.
>
> The Denali NAND Flash Memory Controller User's Guide describes this
> register as follows:
>
> acc_clks
> signifies the number of bus interface clk_x clock cycles,
> controller should wait from read enable going low to sending
> out a strobe of clk_x for capturing of incoming data.
>
> Currently, acc_clks is calculated only based on tREA, the delay on the
> chip side. This does not include additional delays that come from the
> data path on the PCB and in the SoC, load capacity of the pins, etc.
>
> This relatively becomes a big factor on faster timing modes like mode 5.
>
> Before supporting the ->setup_data_interface() hook (e.g. Linux 4.12),
> the Denali driver hacks acc_clks in a couple of ways [1] [2] to support
> the timing mode 5.
>
> We would not go back to the hard-coded acc_clks, but we need to include
> this factor into the delay somehow. Let's say the amount of the additional
> delay is 10000 pico sec.
>
> In the new calculation, acc_clks is determined by timings->tREA_max +
> data_setup_on_host.
>
> Also, prolong the RE# low period to make sure the data hold is met.
>
> Finally, re-center the data latch timing for extra safety.
>
> [1] https://github.com/torvalds/linux/blob/v4.12/drivers/mtd/nand/denali.c#L276
> [2] https://github.com/torvalds/linux/blob/v4.12/drivers/mtd/nand/denali.c#L282
>
> Reported-by: Marek Vasut <marex@denx.de>
> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
> ---
Can you please give this patch a try and report the result?
Thanks,
Miquèl
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
WARNING: multiple messages have this Message-ID (diff)
From: Miquel Raynal <miquel.raynal@bootlin.com>
To: Masahiro Yamada <yamada.masahiro@socionext.com>
Cc: linux-mtd@lists.infradead.org, Marek Vasut <marex@denx.de>,
Richard Weinberger <richard@nod.at>,
Vignesh Raghavendra <vigneshr@ti.com>,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH] mtd: rawnand: denali: add more delays before latching incoming data
Date: Fri, 20 Mar 2020 18:11:59 +0100 [thread overview]
Message-ID: <20200320181159.5004099f@xps13> (raw)
In-Reply-To: <20200316104307.1891-1-yamada.masahiro@socionext.com>
Hi Marek,
Masahiro Yamada <yamada.masahiro@socionext.com> wrote on Mon, 16 Mar
2020 19:43:07 +0900:
> The Denali IP have several registers to specify how many clock cycles
> should be waited between falling/rising signals. You can improve the
> NAND access performance by programming these registers with optimized
> values.
>
> Because struct nand_sdr_timings represents the device requirement
> in pico seconds, denali_setup_data_interface() computes the register
> values by dividing the device timings with the clock period.
>
> Marek Vasut reported this driver in the latest kernel does not work
> on his SOCFPGA board. (The on-board NAND chip is mode 5)
>
> The suspicious parameter is acc_clks, so this commit relaxes it.
>
> The Denali NAND Flash Memory Controller User's Guide describes this
> register as follows:
>
> acc_clks
> signifies the number of bus interface clk_x clock cycles,
> controller should wait from read enable going low to sending
> out a strobe of clk_x for capturing of incoming data.
>
> Currently, acc_clks is calculated only based on tREA, the delay on the
> chip side. This does not include additional delays that come from the
> data path on the PCB and in the SoC, load capacity of the pins, etc.
>
> This relatively becomes a big factor on faster timing modes like mode 5.
>
> Before supporting the ->setup_data_interface() hook (e.g. Linux 4.12),
> the Denali driver hacks acc_clks in a couple of ways [1] [2] to support
> the timing mode 5.
>
> We would not go back to the hard-coded acc_clks, but we need to include
> this factor into the delay somehow. Let's say the amount of the additional
> delay is 10000 pico sec.
>
> In the new calculation, acc_clks is determined by timings->tREA_max +
> data_setup_on_host.
>
> Also, prolong the RE# low period to make sure the data hold is met.
>
> Finally, re-center the data latch timing for extra safety.
>
> [1] https://github.com/torvalds/linux/blob/v4.12/drivers/mtd/nand/denali.c#L276
> [2] https://github.com/torvalds/linux/blob/v4.12/drivers/mtd/nand/denali.c#L282
>
> Reported-by: Marek Vasut <marex@denx.de>
> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
> ---
Can you please give this patch a try and report the result?
Thanks,
Miquèl
next prev parent reply other threads:[~2020-03-20 17:12 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-03-16 10:43 [PATCH] mtd: rawnand: denali: add more delays before latching incoming data Masahiro Yamada
2020-03-16 10:43 ` Masahiro Yamada
2020-03-20 17:11 ` Miquel Raynal [this message]
2020-03-20 17:11 ` Miquel Raynal
2020-03-20 17:14 ` Marek Vasut
2020-03-20 17:14 ` Marek Vasut
2020-03-22 17:42 ` Masahiro Yamada
2020-03-22 17:42 ` Masahiro Yamada
2020-03-22 17:49 ` Marek Vasut
2020-03-22 17:49 ` Marek Vasut
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=20200320181159.5004099f@xps13 \
--to=miquel.raynal@bootlin.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mtd@lists.infradead.org \
--cc=marex@denx.de \
--cc=richard@nod.at \
--cc=vigneshr@ti.com \
--cc=yamada.masahiro@socionext.com \
/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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.