* [PATCH] mtd: mxc_nand: Set timing for v2 controllers
@ 2016-08-23 10:34 Sascha Hauer
2016-08-26 7:46 ` Boris Brezillon
0 siblings, 1 reply; 6+ messages in thread
From: Sascha Hauer @ 2016-08-23 10:34 UTC (permalink / raw)
To: linux-arm-kernel
So far we relied on reset default or the bootloader to configure a
suitable clk rate for the Nand controller. This works but we can
optimize the timing for better performance. This sets the clk rate for
v2 controllers (i.MX25/35) based on the timing mode read from the ONFI
parameter page. This may also enable the symmetric mode (aks EDO mode)
if necessary which reads one word per clock cycle.
Tested on an i.MX25 with a Micron MT29F4G08ABBDAHC attached.
Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
drivers/mtd/nand/mxc_nand.c | 41 +++++++++++++++++++++++++++++++++++++++++
1 file changed, 41 insertions(+)
diff --git a/drivers/mtd/nand/mxc_nand.c b/drivers/mtd/nand/mxc_nand.c
index 5173fad..725223c 100644
--- a/drivers/mtd/nand/mxc_nand.c
+++ b/drivers/mtd/nand/mxc_nand.c
@@ -1012,6 +1012,45 @@ static void preset_v1(struct mtd_info *mtd)
writew(0x4, NFC_V1_V2_WRPROT);
}
+static void mxc_nand_v2_set_timing(struct mtd_info *mtd, uint16_t *config1)
+{
+ struct nand_chip *nand_chip = mtd->priv;
+ struct mxc_nand_host *host = nand_chip->priv;
+ int mode, ret, tRC_min_ns;
+ unsigned long rate;
+ const struct nand_sdr_timings *timings;
+
+ mode = onfi_get_async_timing_mode(nand_chip);
+ if (mode == ONFI_TIMING_MODE_UNKNOWN)
+ return;
+
+ mode = fls(mode) - 1;
+ if (mode < 0)
+ mode = 0;
+
+ timings = onfi_async_timing_mode_to_sdr_timings(mode);
+ if (IS_ERR(timings))
+ return;
+
+ tRC_min_ns = timings->tRC_min / 1000;
+ rate = 1000000000 / tRC_min_ns;
+
+ if (clk_round_rate(host->clk, rate) > 33333333)
+ /*
+ * If tRC is smaller than 30ns (that is, the rate is higher
+ * than 33.333MHz) we have to use EDO timing.
+ */
+ *config1 |= NFC_V2_CONFIG1_ONE_CYCLE;
+ else
+ /* Otherwise we have two clock cycles per access. */
+ rate *= 2;
+
+ ret = clk_set_rate(host->clk, rate);
+ if (ret)
+ /* Don't worry, continue with old rate */
+ dev_err(host->dev, "failed to set clk rate: %d\n", ret);
+}
+
static void preset_v2(struct mtd_info *mtd)
{
struct nand_chip *nand_chip = mtd_to_nand(mtd);
@@ -1020,6 +1059,8 @@ static void preset_v2(struct mtd_info *mtd)
config1 |= NFC_V2_CONFIG1_FP_INT;
+ mxc_nand_v2_set_timing(mtd, &config1);
+
if (!host->devtype_data->irqpending_quirk)
config1 |= NFC_V1_V2_CONFIG1_INT_MSK;
--
2.8.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH] mtd: mxc_nand: Set timing for v2 controllers
2016-08-23 10:34 [PATCH] mtd: mxc_nand: Set timing for v2 controllers Sascha Hauer
@ 2016-08-26 7:46 ` Boris Brezillon
2016-08-29 8:35 ` Sascha Hauer
0 siblings, 1 reply; 6+ messages in thread
From: Boris Brezillon @ 2016-08-26 7:46 UTC (permalink / raw)
To: linux-arm-kernel
Hi Sascha,
On Tue, 23 Aug 2016 12:34:05 +0200
Sascha Hauer <s.hauer@pengutronix.de> wrote:
> So far we relied on reset default or the bootloader to configure a
> suitable clk rate for the Nand controller. This works but we can
> optimize the timing for better performance. This sets the clk rate for
> v2 controllers (i.MX25/35) based on the timing mode read from the ONFI
> parameter page. This may also enable the symmetric mode (aks EDO mode)
> if necessary which reads one word per clock cycle.
> Tested on an i.MX25 with a Micron MT29F4G08ABBDAHC attached.
>
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> ---
> drivers/mtd/nand/mxc_nand.c | 41 +++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 41 insertions(+)
>
> diff --git a/drivers/mtd/nand/mxc_nand.c b/drivers/mtd/nand/mxc_nand.c
> index 5173fad..725223c 100644
> --- a/drivers/mtd/nand/mxc_nand.c
> +++ b/drivers/mtd/nand/mxc_nand.c
> @@ -1012,6 +1012,45 @@ static void preset_v1(struct mtd_info *mtd)
> writew(0x4, NFC_V1_V2_WRPROT);
> }
>
> +static void mxc_nand_v2_set_timing(struct mtd_info *mtd, uint16_t *config1)
> +{
> + struct nand_chip *nand_chip = mtd->priv;
> + struct mxc_nand_host *host = nand_chip->priv;
> + int mode, ret, tRC_min_ns;
> + unsigned long rate;
> + const struct nand_sdr_timings *timings;
> +
> + mode = onfi_get_async_timing_mode(nand_chip);
> + if (mode == ONFI_TIMING_MODE_UNKNOWN)
> + return;
> +
> + mode = fls(mode) - 1;
> + if (mode < 0)
> + mode = 0;
> +
> + timings = onfi_async_timing_mode_to_sdr_timings(mode);
> + if (IS_ERR(timings))
> + return;
> +
AFAIR, you should apply the new mode on the NAND side
(ONFI_FEATURE_ADDR_TIMING_MODE) before applying the new config to the
controller.
See what's done here [1].
Note that on all the NANDs I tested, it seems to work even if you don't
set ONFI_FEATURE_ADDR_TIMING_MODE, but the ONFI Spec describes the
procedure as:
1/ detect supported modes
2/ select one
3/ apply it to the NAND using set(ONFI_FEATURE_ADDR_TIMING_MODE)
4/ release the CS
5/ adjust the controller setting to match the new config
On a side note, I really think we should handle this timing selection in
the core, and only ask the NAND controller drivers to adapt the
controller settings.
Thanks,
Boris
> + tRC_min_ns = timings->tRC_min / 1000;
> + rate = 1000000000 / tRC_min_ns;
> +
> + if (clk_round_rate(host->clk, rate) > 33333333)
> + /*
> + * If tRC is smaller than 30ns (that is, the rate is higher
> + * than 33.333MHz) we have to use EDO timing.
> + */
> + *config1 |= NFC_V2_CONFIG1_ONE_CYCLE;
> + else
> + /* Otherwise we have two clock cycles per access. */
> + rate *= 2;
> +
> + ret = clk_set_rate(host->clk, rate);
> + if (ret)
> + /* Don't worry, continue with old rate */
> + dev_err(host->dev, "failed to set clk rate: %d\n", ret);
> +}
> +
> static void preset_v2(struct mtd_info *mtd)
> {
> struct nand_chip *nand_chip = mtd_to_nand(mtd);
> @@ -1020,6 +1059,8 @@ static void preset_v2(struct mtd_info *mtd)
>
> config1 |= NFC_V2_CONFIG1_FP_INT;
>
> + mxc_nand_v2_set_timing(mtd, &config1);
> +
> if (!host->devtype_data->irqpending_quirk)
> config1 |= NFC_V1_V2_CONFIG1_INT_MSK;
>
[1]http://lxr.free-electrons.com/source/drivers/mtd/nand/sunxi_nand.c#L1406
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH] mtd: mxc_nand: Set timing for v2 controllers
2016-08-26 7:46 ` Boris Brezillon
@ 2016-08-29 8:35 ` Sascha Hauer
2016-08-29 8:50 ` Boris Brezillon
0 siblings, 1 reply; 6+ messages in thread
From: Sascha Hauer @ 2016-08-29 8:35 UTC (permalink / raw)
To: linux-arm-kernel
Hi Boris,
On Fri, Aug 26, 2016 at 09:46:53AM +0200, Boris Brezillon wrote:
> Hi Sascha,
>
> On Tue, 23 Aug 2016 12:34:05 +0200
> Sascha Hauer <s.hauer@pengutronix.de> wrote:
>
> AFAIR, you should apply the new mode on the NAND side
> (ONFI_FEATURE_ADDR_TIMING_MODE) before applying the new config to the
> controller.
>
> See what's done here [1].
>
> Note that on all the NANDs I tested, it seems to work even if you don't
> set ONFI_FEATURE_ADDR_TIMING_MODE, but the ONFI Spec describes the
> procedure as:
Indeed, works here without notifying the NAND chip aswell. I wasn't even
aware that there is a possibility to notify the NAND chip.
>
> 1/ detect supported modes
> 2/ select one
> 3/ apply it to the NAND using set(ONFI_FEATURE_ADDR_TIMING_MODE)
> 4/ release the CS
> 5/ adjust the controller setting to match the new config
>
> On a side note, I really think we should handle this timing selection in
> the core, and only ask the NAND controller drivers to adapt the
> controller settings.
I agree that the core should do this. Do you already have any thoughts
how the API could could look like? The Nand drivers probably have to
propagate somehow which modes they support, maybe we add a onfi_modes
field to struct nand_chip. Then if onfi_modes is nonzero we call a
nand_chip->set_onfi_mode() function?
Sascha
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH] mtd: mxc_nand: Set timing for v2 controllers
2016-08-29 8:35 ` Sascha Hauer
@ 2016-08-29 8:50 ` Boris Brezillon
2016-09-02 12:43 ` Sascha Hauer
0 siblings, 1 reply; 6+ messages in thread
From: Boris Brezillon @ 2016-08-29 8:50 UTC (permalink / raw)
To: linux-arm-kernel
Hi Sascha,
On Mon, 29 Aug 2016 10:35:58 +0200
Sascha Hauer <s.hauer@pengutronix.de> wrote:
> Hi Boris,
>
> On Fri, Aug 26, 2016 at 09:46:53AM +0200, Boris Brezillon wrote:
> > Hi Sascha,
> >
> > On Tue, 23 Aug 2016 12:34:05 +0200
> > Sascha Hauer <s.hauer@pengutronix.de> wrote:
> >
> > AFAIR, you should apply the new mode on the NAND side
> > (ONFI_FEATURE_ADDR_TIMING_MODE) before applying the new config to the
> > controller.
> >
> > See what's done here [1].
> >
> > Note that on all the NANDs I tested, it seems to work even if you don't
> > set ONFI_FEATURE_ADDR_TIMING_MODE, but the ONFI Spec describes the
> > procedure as:
>
> Indeed, works here without notifying the NAND chip aswell. I wasn't even
> aware that there is a possibility to notify the NAND chip.
>
> >
> > 1/ detect supported modes
> > 2/ select one
> > 3/ apply it to the NAND using set(ONFI_FEATURE_ADDR_TIMING_MODE)
> > 4/ release the CS
> > 5/ adjust the controller setting to match the new config
> >
> > On a side note, I really think we should handle this timing selection in
> > the core, and only ask the NAND controller drivers to adapt the
> > controller settings.
>
> I agree that the core should do this. Do you already have any thoughts
> how the API could could look like?
I already provided a simple implementation a while ago [1]. Not sure it
handles all the corner cases though.
> The Nand drivers probably have to
> propagate somehow which modes they support, maybe we add a onfi_modes
> field to struct nand_chip. Then if onfi_modes is nonzero we call a
> nand_chip->set_onfi_mode() function?
The idea was to have some way to ask the controller whether it supports
specific timings or not. I don't remember if this was part of my
initial proposal.
Note that we should pass struct real timings and not ONFI timing
mode.
Regards,
Boris
[1]https://lkml.org/lkml/2015/10/23/179
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH] mtd: mxc_nand: Set timing for v2 controllers
2016-08-29 8:50 ` Boris Brezillon
@ 2016-09-02 12:43 ` Sascha Hauer
2016-09-02 12:59 ` Boris Brezillon
0 siblings, 1 reply; 6+ messages in thread
From: Sascha Hauer @ 2016-09-02 12:43 UTC (permalink / raw)
To: linux-arm-kernel
On Mon, Aug 29, 2016 at 10:50:50AM +0200, Boris Brezillon wrote:
> Hi Sascha,
>
> On Mon, 29 Aug 2016 10:35:58 +0200
> Sascha Hauer <s.hauer@pengutronix.de> wrote:
>
> > Hi Boris,
> >
> > On Fri, Aug 26, 2016 at 09:46:53AM +0200, Boris Brezillon wrote:
> > > Hi Sascha,
> > >
> > > On Tue, 23 Aug 2016 12:34:05 +0200
> > > Sascha Hauer <s.hauer@pengutronix.de> wrote:
> > >
> > > AFAIR, you should apply the new mode on the NAND side
> > > (ONFI_FEATURE_ADDR_TIMING_MODE) before applying the new config to the
> > > controller.
> > >
> > > See what's done here [1].
> > >
> > > Note that on all the NANDs I tested, it seems to work even if you don't
> > > set ONFI_FEATURE_ADDR_TIMING_MODE, but the ONFI Spec describes the
> > > procedure as:
> >
> > Indeed, works here without notifying the NAND chip aswell. I wasn't even
> > aware that there is a possibility to notify the NAND chip.
> >
> > >
> > > 1/ detect supported modes
> > > 2/ select one
> > > 3/ apply it to the NAND using set(ONFI_FEATURE_ADDR_TIMING_MODE)
> > > 4/ release the CS
> > > 5/ adjust the controller setting to match the new config
> > >
> > > On a side note, I really think we should handle this timing selection in
> > > the core, and only ask the NAND controller drivers to adapt the
> > > controller settings.
> >
> > I agree that the core should do this. Do you already have any thoughts
> > how the API could could look like?
>
> I already provided a simple implementation a while ago [1]. Not sure it
> handles all the corner cases though.
I just picked up the patch and ported my mxc_nand patch over to it. See
the result on the mailing list.
Sascha
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH] mtd: mxc_nand: Set timing for v2 controllers
2016-09-02 12:43 ` Sascha Hauer
@ 2016-09-02 12:59 ` Boris Brezillon
0 siblings, 0 replies; 6+ messages in thread
From: Boris Brezillon @ 2016-09-02 12:59 UTC (permalink / raw)
To: linux-arm-kernel
On Fri, 2 Sep 2016 14:43:53 +0200
Sascha Hauer <s.hauer@pengutronix.de> wrote:
> On Mon, Aug 29, 2016 at 10:50:50AM +0200, Boris Brezillon wrote:
> > Hi Sascha,
> >
> > On Mon, 29 Aug 2016 10:35:58 +0200
> > Sascha Hauer <s.hauer@pengutronix.de> wrote:
> >
> > > Hi Boris,
> > >
> > > On Fri, Aug 26, 2016 at 09:46:53AM +0200, Boris Brezillon wrote:
> > > > Hi Sascha,
> > > >
> > > > On Tue, 23 Aug 2016 12:34:05 +0200
> > > > Sascha Hauer <s.hauer@pengutronix.de> wrote:
> > > >
> > > > AFAIR, you should apply the new mode on the NAND side
> > > > (ONFI_FEATURE_ADDR_TIMING_MODE) before applying the new config to the
> > > > controller.
> > > >
> > > > See what's done here [1].
> > > >
> > > > Note that on all the NANDs I tested, it seems to work even if you don't
> > > > set ONFI_FEATURE_ADDR_TIMING_MODE, but the ONFI Spec describes the
> > > > procedure as:
> > >
> > > Indeed, works here without notifying the NAND chip aswell. I wasn't even
> > > aware that there is a possibility to notify the NAND chip.
> > >
> > > >
> > > > 1/ detect supported modes
> > > > 2/ select one
> > > > 3/ apply it to the NAND using set(ONFI_FEATURE_ADDR_TIMING_MODE)
> > > > 4/ release the CS
> > > > 5/ adjust the controller setting to match the new config
> > > >
> > > > On a side note, I really think we should handle this timing selection in
> > > > the core, and only ask the NAND controller drivers to adapt the
> > > > controller settings.
> > >
> > > I agree that the core should do this. Do you already have any thoughts
> > > how the API could could look like?
> >
> > I already provided a simple implementation a while ago [1]. Not sure it
> > handles all the corner cases though.
>
> I just picked up the patch and ported my mxc_nand patch over to it. See
> the result on the mailing list.
Thanks for doing that. I'll try to review the series soon.
Boris
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2016-09-02 12:59 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-08-23 10:34 [PATCH] mtd: mxc_nand: Set timing for v2 controllers Sascha Hauer
2016-08-26 7:46 ` Boris Brezillon
2016-08-29 8:35 ` Sascha Hauer
2016-08-29 8:50 ` Boris Brezillon
2016-09-02 12:43 ` Sascha Hauer
2016-09-02 12:59 ` Boris Brezillon
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).