omap: gpmc-nand: add ability to keep timings defined by the bootloader

PATCH In review Mike Rapoport 3 participants 9 messages Apr 30, 2010 – May 4, 2010

Subsystems (5)

Arm (32-bit) Arm sub-architectures OMAP2+ support OMAP power management support +1 more subsystems

lore

Share & tools

lore

Mike Rapoport

author

2010-04-30 21:02 UTC #0 lore raw

v2 cover omap: gpmc-nand: add ability to keep timings defined by the bootloader

These patches add ability to keep GPMC timings configured by the bootloader. The platforms have to either define NAND GPMC timinings explicitly via gpmc_t field of omap_nand_platform_data or set keep_timings flag to allow detection of current timing configuration and its subsequent use in omap2_nand_gpmc_retime.

v2 changes:
- remove debug leftovers
- make timing rounding depend on .keep_timing flag and introduce omap2_nand_gpmc_round_timings to facilitate this change

Mike Rapoport (3):
omap: gpmc: add gpmc_cs_get_timings
omap: gpmc-nand: introduce omap2_nand_gpmc_round_timings helper
omap: gpmc-nand: add ability to keep timings defined by the
bootloader

 arch/arm/mach-omap2/gpmc-nand.c        |   73 +++++++++++++++++++------------
 arch/arm/mach-omap2/gpmc.c             |   76 ++++++++++++++++++++++++++++++++
 arch/arm/plat-omap/include/plat/gpmc.h |    1 +
 arch/arm/plat-omap/include/plat/nand.h |    1 +
 4 files changed, 123 insertions(+), 28 deletions(-)

Mike Rapoport

author

2010-04-30 21:03 UTC #1 lore raw

PATCH v2 1/3 omap: gpmc: add gpmc_cs_get_timings

Add gpmc_cs_get_timings counterpart of gpmc_cs_set_timings and convinience macros to read particular timing configuration fields

Signed-off-by Mike Rapoport <[email protected]>

---
 arch/arm/mach-omap2/gpmc.c             |   76 ++++++++++++++++++++++++++++++++
 arch/arm/plat-omap/include/plat/gpmc.h |    1 +
 2 files changed, 77 insertions(+), 0 deletions(-)

arch/arm/mach-omap2/gpmc.c +76 -0

index 5bc3ca0..527a0da 100644
@@ -163,6 +163,36 @@ unsigned int gpmc_round_ns_to_ticks(unsigned int time_ns)
 }
 
 #ifdef DEBUG
+static int get_gpmc_timing_reg(int cs, int reg, int st_bit, int end_bit,
+			       const char *name)
+#else
+static int get_gpmc_timing_reg(int cs, int reg, int st_bit, int end_bit)
+#endif
+{
+	u32 l;
+	int ticks, mask, nr_bits, time;
+
+	nr_bits = end_bit - st_bit + 1;
+	mask = ((1 << nr_bits) - 1);
+
+	l = gpmc_cs_read_reg(cs, reg);
+	ticks = (l >> st_bit) & mask;
+
+	if (ticks == 0)
+		time = 0;
+	else
+		time = gpmc_ticks_to_ns(ticks);
+
+#ifdef DEBUG
+	printk(KERN_INFO
+		"GPMC CS%d: %-10s: %3d ticks, %3d ns\n",
+	       cs, name, ticks, time);
+#endif
+
+	return time;
+}
+
+#ifdef DEBUG
 static int set_gpmc_timing_reg(int cs, int reg, int st_bit, int end_bit,
 			       int time, const char *name)
 #else

@@ -206,10 +236,14 @@ static int set_gpmc_timing_reg(int cs, int reg, int st_bit, int end_bit,
 	if (set_gpmc_timing_reg(cs, (reg), (st), (end),		\
 			t->field, #field) < 0)			\
 		return -1
+#define GPMC_GET_ONE(reg, st, end, field) \
+	t->field = get_gpmc_timing_reg(cs, (reg), (st), (end), #field)
 #else
 #define GPMC_SET_ONE(reg, st, end, field) \
 	if (set_gpmc_timing_reg(cs, (reg), (st), (end), t->field) < 0) \
 		return -1
+#define GPMC_GET_ONE(reg, st, end, field) \
+	t->field = get_gpmc_timing_reg(cs, (reg), (st), (end))
 #endif
 
 int gpmc_cs_calc_divider(int cs, unsigned int sync_clk)

@@ -227,6 +261,48 @@ int gpmc_cs_calc_divider(int cs, unsigned int sync_clk)
 	return div;
 }
 
+void gpmc_cs_get_timings(int cs, struct gpmc_timings *t)
+{
+	int div;
+	u32 l;
+
+	GPMC_GET_ONE(GPMC_CS_CONFIG2,  0,  3, cs_on);
+	GPMC_GET_ONE(GPMC_CS_CONFIG2,  8, 12, cs_rd_off);
+	GPMC_GET_ONE(GPMC_CS_CONFIG2, 16, 20, cs_wr_off);
+
+	GPMC_GET_ONE(GPMC_CS_CONFIG3,  0,  3, adv_on);
+	GPMC_GET_ONE(GPMC_CS_CONFIG3,  8, 12, adv_rd_off);
+	GPMC_GET_ONE(GPMC_CS_CONFIG3, 16, 20, adv_wr_off);
+
+	GPMC_GET_ONE(GPMC_CS_CONFIG4,  0,  3, oe_on);
+	GPMC_GET_ONE(GPMC_CS_CONFIG4,  8, 12, oe_off);
+	GPMC_GET_ONE(GPMC_CS_CONFIG4, 16, 19, we_on);
+	GPMC_GET_ONE(GPMC_CS_CONFIG4, 24, 28, we_off);
+
+	GPMC_GET_ONE(GPMC_CS_CONFIG5,  0,  4, rd_cycle);
+	GPMC_GET_ONE(GPMC_CS_CONFIG5,  8, 12, wr_cycle);
+	GPMC_GET_ONE(GPMC_CS_CONFIG5, 16, 20, access);
+
+	GPMC_GET_ONE(GPMC_CS_CONFIG5, 24, 27, page_burst_access);
+
+	if (cpu_is_omap34xx()) {
+		GPMC_GET_ONE(GPMC_CS_CONFIG6, 16, 19, wr_data_mux_bus);
+		GPMC_GET_ONE(GPMC_CS_CONFIG6, 24, 28, wr_access);
+	}
+
+	l = gpmc_cs_read_reg(cs, GPMC_CS_CONFIG1);
+	if (l & (GPMC_CONFIG1_READTYPE_SYNC | GPMC_CONFIG1_WRITETYPE_SYNC)) {
+		div = (l & 0x03) + 1;
+#ifdef DEBUG
+		printk(KERN_INFO "GPMC CS%d CLK period is %lu ns (div %d)\n",
+				cs, (div * gpmc_get_fclk_period()) / 1000, div);
+#endif
+		t->sync_clk = (div * gpmc_get_fclk_period()) / 1000;
+	} else {
+		t->sync_clk = 0;
+	}
+}
+
 int gpmc_cs_set_timings(int cs, const struct gpmc_timings *t)
 {
 	int div;

arch/arm/plat-omap/include/plat/gpmc.h +1 -0

index 145838a..5c345f1 100644
@@ -102,6 +102,7 @@ extern void gpmc_cs_write_reg(int cs, int idx, u32 val);
 extern u32 gpmc_cs_read_reg(int cs, int idx);
 extern int gpmc_cs_calc_divider(int cs, unsigned int sync_clk);
 extern int gpmc_cs_set_timings(int cs, const struct gpmc_timings *t);
+extern void gpmc_cs_get_timings(int cs, struct gpmc_timings *t);
 extern int gpmc_cs_request(int cs, unsigned long size, unsigned long *base);
 extern void gpmc_cs_free(int cs);
 extern int gpmc_cs_set_reserved(int cs, int reserved);

Signature · 1 line

1.6.6.2

Mike Rapoport

author

2010-04-30 21:02 UTC #2 lore raw

PATCH v2 2/3 omap: gpmc-nand: introduce omap2_nand_gpmc_round_timings helper

In reply to Mike Rapoport

Signed-off-by Mike Rapoport <[email protected]>

---
 arch/arm/mach-omap2/gpmc-nand.c |   56 +++++++++++++++++++++------------------
 1 files changed, 30 insertions(+), 26 deletions(-)

arch/arm/mach-omap2/gpmc-nand.c +30 ~~-26~~

index e57fb29..9434c80 100644
@@ -34,6 +34,35 @@ static struct platform_device gpmc_nand_device = {
 	.resource	= &gpmc_nand_resource,
 };
 
+static void omap2_nand_gpmc_round_timings(struct gpmc_timings *src,
+					  struct gpmc_timings *dst)
+{
+	dst->sync_clk = gpmc_round_ns_to_ticks(src->sync_clk);
+	dst->cs_on = gpmc_round_ns_to_ticks(src->cs_on);
+	dst->adv_on = gpmc_round_ns_to_ticks(src->adv_on);
+
+	/* Read */
+	dst->adv_rd_off = gpmc_round_ns_to_ticks(src->adv_rd_off);
+	dst->oe_on  = dst->adv_on;
+	dst->access = gpmc_round_ns_to_ticks(src->access);
+	dst->oe_off = gpmc_round_ns_to_ticks(src->oe_off);
+	dst->cs_rd_off = gpmc_round_ns_to_ticks(src->cs_rd_off);
+	dst->rd_cycle  = gpmc_round_ns_to_ticks(src->rd_cycle);
+
+	/* Write */
+	dst->adv_wr_off = gpmc_round_ns_to_ticks(src->adv_wr_off);
+	dst->we_on  = dst->oe_on;
+	if (cpu_is_omap34xx()) {
+		dst->wr_data_mux_bus =	gpmc_round_ns_to_ticks(
+				src->wr_data_mux_bus);
+		dst->wr_access = gpmc_round_ns_to_ticks(
+				src->wr_access);
+	}
+	dst->we_off = gpmc_round_ns_to_ticks(src->we_off);
+	dst->cs_wr_off = gpmc_round_ns_to_ticks(src->cs_wr_off);
+	dst->wr_cycle  = gpmc_round_ns_to_ticks(src->wr_cycle);
+}
+
 static int omap2_nand_gpmc_retime(void)
 {
 	struct gpmc_timings t;

@@ -43,32 +72,7 @@ static int omap2_nand_gpmc_retime(void)
 		return 0;
 
 	memset(&t, 0, sizeof(t));
-	t.sync_clk = gpmc_round_ns_to_ticks(gpmc_nand_data->gpmc_t->sync_clk);
-	t.cs_on = gpmc_round_ns_to_ticks(gpmc_nand_data->gpmc_t->cs_on);
-	t.adv_on = gpmc_round_ns_to_ticks(gpmc_nand_data->gpmc_t->adv_on);
-
-	/* Read */
-	t.adv_rd_off = gpmc_round_ns_to_ticks(
-				gpmc_nand_data->gpmc_t->adv_rd_off);
-	t.oe_on  = t.adv_on;
-	t.access = gpmc_round_ns_to_ticks(gpmc_nand_data->gpmc_t->access);
-	t.oe_off = gpmc_round_ns_to_ticks(gpmc_nand_data->gpmc_t->oe_off);
-	t.cs_rd_off = gpmc_round_ns_to_ticks(gpmc_nand_data->gpmc_t->cs_rd_off);
-	t.rd_cycle  = gpmc_round_ns_to_ticks(gpmc_nand_data->gpmc_t->rd_cycle);
-
-	/* Write */
-	t.adv_wr_off = gpmc_round_ns_to_ticks(
-				gpmc_nand_data->gpmc_t->adv_wr_off);
-	t.we_on  = t.oe_on;
-	if (cpu_is_omap34xx()) {
-	    t.wr_data_mux_bus =	gpmc_round_ns_to_ticks(
-				gpmc_nand_data->gpmc_t->wr_data_mux_bus);
-	    t.wr_access = gpmc_round_ns_to_ticks(
-				gpmc_nand_data->gpmc_t->wr_access);
-	}
-	t.we_off = gpmc_round_ns_to_ticks(gpmc_nand_data->gpmc_t->we_off);
-	t.cs_wr_off = gpmc_round_ns_to_ticks(gpmc_nand_data->gpmc_t->cs_wr_off);
-	t.wr_cycle  = gpmc_round_ns_to_ticks(gpmc_nand_data->gpmc_t->wr_cycle);
+	omap2_nand_gpmc_round_timings(gpmc_nand_data->gpmc_t, &t);
 
 	/* Configure GPMC */
 	gpmc_cs_write_reg(gpmc_nand_data->cs, GPMC_CS_CONFIG1,

Signature · 1 line

1.6.6.2

Mike Rapoport

author

2010-04-30 21:03 UTC #3 lore raw

PATCH v2 3/3 omap: gpmc-nand: add ability to keep timings defined by the bootloader

In reply to Mike Rapoport

Signed-off-by Mike Rapoport <[email protected]>

---
 arch/arm/mach-omap2/gpmc-nand.c        |   21 +++++++++++++++++----
 arch/arm/plat-omap/include/plat/nand.h |    1 +
 2 files changed, 18 insertions(+), 4 deletions(-)

arch/arm/mach-omap2/gpmc-nand.c +17 -4

index 9434c80..96f0c7f 100644
@@ -22,6 +22,7 @@
 #define WR_RD_PIN_MONITORING	0x00600000
 
 static struct omap_nand_platform_data *gpmc_nand_data;
+static struct gpmc_timings gpmc_default_timings;
 
 static struct resource gpmc_nand_resource = {
 	.flags		= IORESOURCE_MEM,

@@ -65,21 +66,28 @@ static void omap2_nand_gpmc_round_timings(struct gpmc_timings *src,
 
 static int omap2_nand_gpmc_retime(void)
 {
+	struct device *dev = &gpmc_nand_device.dev;
+	struct gpmc_timings *gpmc_t = gpmc_nand_data->gpmc_t;
 	struct gpmc_timings t;
 	int err;
 
-	if (!gpmc_nand_data->gpmc_t)
+	if (!gpmc_t) {
+		dev_warn(dev, "No timings provided, skipping retime\n");
 		return 0;
+	}
 
-	memset(&t, 0, sizeof(t));
-	omap2_nand_gpmc_round_timings(gpmc_nand_data->gpmc_t, &t);
+	if (!gpmc_nand_data->keep_timings) {
+		memset(&t, 0, sizeof(t));
+		omap2_nand_gpmc_round_timings(gpmc_nand_data->gpmc_t, &t);
+		gpmc_t = &t;
+	}
 
 	/* Configure GPMC */
 	gpmc_cs_write_reg(gpmc_nand_data->cs, GPMC_CS_CONFIG1,
 			GPMC_CONFIG1_DEVICESIZE(gpmc_nand_data->devsize) |
 			GPMC_CONFIG1_DEVICETYPE_NAND);
 
-	err = gpmc_cs_set_timings(gpmc_nand_data->cs, &t);
+	err = gpmc_cs_set_timings(gpmc_nand_data->cs, gpmc_t);
 	if (err)
 		return err;

@@ -116,6 +124,11 @@ int __init gpmc_nand_init(struct omap_nand_platform_data *_nand_data)
 		return err;
 	}
 
+	if (gpmc_nand_data->keep_timings) {
+		gpmc_cs_get_timings(gpmc_nand_data->cs, &gpmc_default_timings);
+		gpmc_nand_data->gpmc_t = &gpmc_default_timings;
+	}
+
 	err = gpmc_nand_setup();
 	if (err < 0) {
 		dev_err(dev, "NAND platform setup failed: %d\n", err);

arch/arm/plat-omap/include/plat/nand.h +1 -0

index f8efd54..0f727ea 100644
@@ -24,6 +24,7 @@ struct omap_nand_platform_data {
 	void __iomem		*gpmc_cs_baseaddr;
 	void __iomem		*gpmc_baseaddr;
 	int			devsize;
+	bool			keep_timings;
 };
 
 /* size (4 KiB) for IO mapping */

Signature · 1 line

1.6.6.2

Tony Lindgren

maintainer

2010-05-03 18:24 UTC #4 lore raw

Quoted 1 line · Mike Rapoport

Signed-off-by Mike Rapoport <[email protected]>

Please add a proper description to all the patches.

Quoted 4 lines

arch/arm/mach-omap2/gpmc-nand.c +0 -0

@@ -116,6 +124,11 @@ int __init gpmc_nand_init(struct omap_nand_platform_data *_nand_data)
 		return err;

arch/arm/mach-omap2/gpmc-nand.c int __init gpmc_nand_init(struct omap_nand_platform_data *_nand_data) · line 134

Quoted 10 lines

arch/arm/mach-omap2/gpmc-nand.c +5 -0

 	}
 
+	if (gpmc_nand_data->keep_timings) {
+		gpmc_cs_get_timings(gpmc_nand_data->cs, &gpmc_default_timings);
+		gpmc_nand_data->gpmc_t = &gpmc_default_timings;
+	}
+
 	err = gpmc_nand_setup();
 	if (err < 0) {
 		dev_err(dev, "NAND platform setup failed: %d\n", err);

Hmm, so you're setting the timings based on the bootloader values?

I' think the problem with that is that chances are that it still won't work for other L3 frequencies because of rounding errors.

With gpmc_cs_get_timings() you're already using tick rounded timings, so you won't get the required accuracy out of those for the other L3 frequencies.

So maybe just not do anything, and print a warning on gpmc L3 changes if the timings are not set?

Regards,

Tony

Mike Rapoport

2010-05-03 20:32 UTC #5 lore raw

Quoted 22 lines · Tony Lindgren

Quoted 1 line · Mike Rapoport
Signed-off-by Mike Rapoport <[email protected]>
Please add a proper description to all the patches.
Quoted 4 lines
arch/arm/mach-omap2/gpmc-nand.c +0 -0
@@ -116,6 +124,11 @@ int __init gpmc_nand_init(struct omap_nand_platform_data *_nand_data)
               return err;
arch/arm/mach-omap2/gpmc-nand.c int __init gpmc_nand_init(struct omap_nand_platform_data *_nand_data) · line 134
Quoted 10 lines
arch/arm/mach-omap2/gpmc-nand.c +5 -0
       }
 
+     if (gpmc_nand_data->keep_timings) {
+             gpmc_cs_get_timings(gpmc_nand_data->cs, &gpmc_default_timings);
+             gpmc_nand_data->gpmc_t = &gpmc_default_timings;
+     }
+
       err = gpmc_nand_setup();
       if (err < 0) {
               dev_err(dev, "NAND platform setup failed: %d\n", err);
Hmm, so you're setting the timings based on the bootloader values?

Quoted 6 lines

I' think the problem with that is that chances are that it still won't work for other L3 frequencies because of rounding errors.
With gpmc_cs_get_timings() you're already using tick rounded timings, so you won't get the required accuracy out of those for the other L3 frequencies.

Agree. But even if the timings are in nanoseconds there are rounding errors, and there are still chances that L3 frequency change will break NAND
So it comes down to what provides better tolerance, the explicit NAND timings in nanosecs or (rounded) timings in ticks derived from bootloader settings...

Quoted 2 lines

So maybe just not do anything, and print a warning on gpmc L3 changes if the timings are not set?

I don't quite understand where exactly this warning should go. I haven't found any treatment of L3 frequency changes in gpmc related code neither in linux-omap nor in linux-omap-pm...

Quoted 8 lines

Regards,
Tony
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Signature · 6 lines

	Sincerely Yours,
		Mike.
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Tony Lindgren

maintainer

2010-05-03 21:16 UTC #6 lore raw

Quoted 30 lines · Mike Rapoport

Quoted 22 lines · Tony Lindgren
Quoted 1 line · Mike Rapoport
Signed-off-by Mike Rapoport <[email protected]>
Please add a proper description to all the patches.
Quoted 4 lines
arch/arm/mach-omap2/gpmc-nand.c +0 -0
@@ -116,6 +124,11 @@ int __init gpmc_nand_init(struct omap_nand_platform_data *_nand_data)
               return err;
arch/arm/mach-omap2/gpmc-nand.c int __init gpmc_nand_init(struct omap_nand_platform_data *_nand_data) · line 134
Quoted 10 lines
arch/arm/mach-omap2/gpmc-nand.c +5 -0
       }
 
+     if (gpmc_nand_data->keep_timings) {
+             gpmc_cs_get_timings(gpmc_nand_data->cs, &gpmc_default_timings);
+             gpmc_nand_data->gpmc_t = &gpmc_default_timings;
+     }
+
       err = gpmc_nand_setup();
       if (err < 0) {
               dev_err(dev, "NAND platform setup failed: %d\n", err);
Hmm, so you're setting the timings based on the bootloader values?
Quoted 6 lines
I' think the problem with that is that chances are that it still won't work for other L3 frequencies because of rounding errors.
With gpmc_cs_get_timings() you're already using tick rounded timings, so you won't get the required accuracy out of those for the other L3 frequencies.

Quoted 6 lines

Agree. But even if the timings are in nanoseconds there are rounding errors, and there are still chances that L3 frequency change will break NAND
So it comes down to what provides better tolerance, the explicit NAND timings in nanosecs or (rounded) timings in ticks derived from bootloader settings...

My experience is that you can get the nanosec timings from the device datasheet(s) and that just should work for any L3 frequency. My experience is also that trying to do it the other way around won't work because of rounding errors. Trying to produce nanosecond values out of the tick values just is not accurate enough.

That's why gpmc-onenand.c and usb-tusb6010.c timings are done the way they are, and they're known to work at various L3 frequencies.

Quoted 6 lines

Quoted 2 lines
So maybe just not do anything, and print a warning on gpmc L3 changes if the timings are not set?
I don't quite understand where exactly this warning should go. I haven't found any treatment of L3 frequency changes in gpmc related code neither in linux-omap nor in linux-omap-pm...

Ah, right. There's currently nothing doing that.. That would have to be done based on cpufreq notifiers (or clock notifiers). But we don't have any of that at least in the mainline yet. Hmm I don't even think we currently scale the L3 for cpufreq.. Right now the best way to test would be by booting at different L3 frequencies.

Anyways, my point is that setting gpmc_default_timings based on the bootloader after doing the gpmc_cs_get_timings is most likely unsafe for other L3 frequencies.

Regards,

Tony
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Mike Rapoport

author

2010-05-04 13:23 UTC #7 lore raw

Quoted 8 lines · Tony Lindgren

Quoted 3 lines · Mike Rapoport
So it comes down to what provides better tolerance, the explicit NAND timings in nanosecs or (rounded) timings in ticks derived from bootloader settings...
My experience is that you can get the nanosec timings from the device datasheet(s) and that just should work for any L3 frequency.

And what about boards that can have different NAND flash chips assembled? What datasheet should be used to get the nanosecs? Note, that detecting NAND ID in the bootloader and adjusting timings appropriately is not that big deal, and doing it in the kernel seems to me really impractical.

Quoted 3 lines

My experience is also that trying to do it the other way around won't work because of rounding errors. Trying to produce nanosecond values out of the tick values just is not accurate enough.

I'm still not convinced. Similar approach worked for me with several devices attached to sort of GPMC controllers on different SoC. There always was a way to set timings once in the bootloader and then detect the timings in the kernel and update them during cpu-freq transitions...

Quoted 2 lines

That's why gpmc-onenand.c and usb-tusb6010.c timings are done the way they are, and they're known to work at various L3 frequencies.

I'm not really familiar with OneNAND, but looking at gpmc-onenand.c I
see hardcoded timings. Moreover, the nanosecs values seem to get adjusted for different L3 frequencies. So, for NAND it would mean that platform would have to supply several timing sets for different L3 freqs?

Quoted 19 lines

Quoted 5 lines
Quoted 2 lines
So maybe just not do anything, and print a warning on gpmc L3 changes if the timings are not set?
I don't quite understand where exactly this warning should go. I haven't found any treatment of L3 frequency changes in gpmc related code neither in linux-omap nor in linux-omap-pm...
Ah, right. There's currently nothing doing that.. That would have to be done based on cpufreq notifiers (or clock notifiers). But we don't have any of that at least in the mainline yet. Hmm I don't even think we currently scale the L3 for cpufreq.. Right now the best way to test would be by booting at different L3 frequencies.
Anyways, my point is that setting gpmc_default_timings based on the bootloader after doing the gpmc_cs_get_timings is most likely unsafe for other L3 frequencies.
Regards,
Tony

Signature · 2 lines

Sincerely yours,
Mike.

Tony Lindgren

maintainer

2010-05-04 21:46 UTC #8 lore raw

Quoted 7 lines · Mike Rapoport

Quoted 6 lines · Tony Lindgren
Quoted 3 lines · Mike Rapoport
So it comes down to what provides better tolerance, the explicit NAND timings in nanosecs or (rounded) timings in ticks derived from bootloader settings...

Quoted 8 lines

Quoted 2 lines
My experience is that you can get the nanosec timings from the device datasheet(s) and that just should work for any L3 frequency.
And what about boards that can have different NAND flash chips assembled? What datasheet should be used to get the nanosecs? Note, that detecting NAND ID in the bootloader and adjusting timings appropriately is not that big deal, and doing it in the kernel seems to me really impractical.

Hmm, if there are different variations of the same board, the best way is to pass ATAG_REVISION from the bootloader. Then you can set the selected NAND platform data based on the revision.

Quoted 9 lines

Quoted 3 lines
My experience is also that trying to do it the other way around won't work because of rounding errors. Trying to produce nanosecond values out of the tick values just is not accurate enough.
I'm still not convinced. Similar approach worked for me with several devices attached to sort of GPMC controllers on different SoC. There always was a way to set timings once in the bootloader and then detect the timings in the kernel and update them during cpu-freq transitions...

Not based on my experience with GPMC and L3.. When converting from GPMC ticks to nanosecond timings, you're losing accuracy so things won't scale the right way if you scale L3 frequency.

Quoted 6 lines

Quoted 2 lines
That's why gpmc-onenand.c and usb-tusb6010.c timings are done the way they are, and they're known to work at various L3 frequencies.
I'm not really familiar with OneNAND, but looking at gpmc-onenand.c I see hardcoded timings. Moreover, the nanosecs values seem to get adjusted for different L3 frequencies.

Yeah, you need to look at the L3 frequency to calculate the timings.

Quoted 2 lines

So, for NAND it would mean that platform would have to supply several timing sets for different L3 freqs?

Not needed if the GPMC tick timings are calculated based on the device datasheet nanosecond timings and omap L3 frequency. Then it scales for all L3 frequencies.

In any case, we still need to also allow using hardcoded bootloader timings for people who don't care about the L3 scaling. But there are other more serious things badly broken with the whole gpmc-nand and drivers/mtd/nand/omap2.c, I'll send another email about that shortly.

Regards,

Tony