[PATCH 00/14] ARM: OMAP2+: GPMC clean-up and DT update

[PATCH 00/14] ARM: OMAP2+: GPMC clean-up and DT update

[Jon Hunter]

While adding device-tree support for NOR memories, it became apparent
that there is no common way for configuring various GPMC settings for
devices that interface to the GPMC. These settings include bus-width,
synchronous/asynchronous mode, burst settings, wait monitoring etc.
Therefore, to simplify the GPMC code and add device-tree support for
NOR, it was first necessary to consolidate how these settings are
programmed.

Summary of changes:
- Clean-up/simplification of ONENAND initialisation code.
- Add a new GPMC structure to unify storage of various GPMC settings
  (that are non-timing related) for client devices and add a new
  function to program these settings in a common way.
- Migrate initialisation code for existing flash, usb and networking
  devices to use the new structure and function for GPMC settings.
- Add device-tree support for NOR flash memories.
- Add additional GPMC timing parameters to GPMC device-tree binding.
- Update GPMC NAND and ONENAND device-tree support to retrieve GPMC
  settings from device-tree.

Testing includes:
- Boot testing on OMAP2420 H4, OMAP3430 SDP and OMAP4430 SDP with
  and without device-tree present.
- OMAP2420 H4 board has NOR flash and OMAP3430 SDP has NOR, NAND
  and ONENAND flash. So verified that flash is detected on boot
  as expected. Note additional patches [1] are required for OMAP2420
  H4 and OMAP3430 SDP dts files in order to enable flash memory
  support.
- All of the above boards use GPMC for interfacing to a networking
  chip and so verified that networking is working wit this series.
  However, please note that networking is not currently supported
  on these boards when booting with DT and so networking is only
  tested without DT.

[1] https://github.com/jonhunter/linux/tree/dev-gpmc

Jon Hunter (14):
  ARM: OMAP2+: Simplify code configuring ONENAND devices
  ARM: OMAP2+: Add variable to store number of GPMC waitpins
  ARM: OMAP2+: Add structure for storing GPMC settings
  ARM: OMAP2+: Add function for configuring GPMC settings
  ARM: OMAP2+: Convert ONENAND to use gpmc_cs_program_settings()
  ARM: OMAP2+: Convert NAND to use gpmc_cs_program_settings()
  ARM: OMAP2+: Convert SMC91x to use gpmc_cs_program_settings()
  ARM: OMAP2+: Convert TUSB to use gpmc_cs_program_settings()
  ARM: OMAP2+: Don't configure of chip-select options in
    gpmc_cs_configure()
  ARM: OMAP2+: Add function to read GPMC settings from device-tree
  ARM: OMAP2+: Add device-tree support for NOR flash
  ARM: OMAP2+: Add additional GPMC timing parameters
  ARM: OMAP2+: Convert NAND to retrieve GPMC settings from DT
  ARM: OMAP2+: Convert ONENAND to retrieve GPMC settings from DT

 Documentation/devicetree/bindings/bus/ti-gpmc.txt  |   44 ++-
 Documentation/devicetree/bindings/mtd/gpmc-nor.txt |   98 ++++++
 .../devicetree/bindings/mtd/gpmc-onenand.txt       |    3 +
 arch/arm/mach-omap2/gpmc-nand.c                    |   38 +-
 arch/arm/mach-omap2/gpmc-onenand.c                 |  110 +++---
 arch/arm/mach-omap2/gpmc-smc91x.c                  |   30 +-
 arch/arm/mach-omap2/gpmc.c                         |  367 +++++++++++++++++---
 arch/arm/mach-omap2/gpmc.h                         |   32 +-
 arch/arm/mach-omap2/usb-tusb6010.c                 |   62 ++--
 9 files changed, 605 insertions(+), 179 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/mtd/gpmc-nor.txt

-- 
1.7.10.4

[PATCH 01/14] ARM: OMAP2+: Simplify code configuring ONENAND devices

[Jon Hunter]

The OMAP2+ code that configures the GPMC for ONENAND devices is copying
structures between functions unnecessarily. Avoid this by passing
pointers instead and simplify the code.

A pointer to structure "omap_onenand_platform_data" is passed to the
function omap2_onenand_calc_sync_timings(), but only the flags member
of the structure is used. Simplify the code by only passing the flags
member and not the entire structure.

Signed-off-by: Jon Hunter <jon-hunter@ti.com>
---
 arch/arm/mach-omap2/gpmc-onenand.c |   26 ++++++++++----------------
 1 file changed, 10 insertions(+), 16 deletions(-)

diff --git a/arch/arm/mach-omap2/gpmc-onenand.c b/arch/arm/mach-omap2/gpmc-onenand.c
index fadd8743..758b09c 100644
--- a/arch/arm/mach-omap2/gpmc-onenand.c
+++ b/arch/arm/mach-omap2/gpmc-onenand.c
@@ -47,10 +47,9 @@ static struct platform_device gpmc_onenand_device = {
 	.resource	= &gpmc_onenand_resource,
 };
 
-static struct gpmc_timings omap2_onenand_calc_async_timings(void)
+static void omap2_onenand_calc_async_timings(struct gpmc_timings *t)
 {
 	struct gpmc_device_timings dev_t;
-	struct gpmc_timings t;
 
 	const int t_cer = 15;
 	const int t_avdp = 12;
@@ -76,9 +75,7 @@ static struct gpmc_timings omap2_onenand_calc_async_timings(void)
 	dev_t.t_wpl = t_wpl * 1000;
 	dev_t.t_wph = t_wph * 1000;
 
-	gpmc_calc_timings(&t, &dev_t);
-
-	return t;
+	gpmc_calc_timings(t, &dev_t);
 }
 
 static int gpmc_set_async_mode(int cs, struct gpmc_timings *t)
@@ -158,12 +155,11 @@ static int omap2_onenand_get_freq(struct omap_onenand_platform_data *cfg,
 	return freq;
 }
 
-static struct gpmc_timings
-omap2_onenand_calc_sync_timings(struct omap_onenand_platform_data *cfg,
-				int freq)
+static void omap2_onenand_calc_sync_timings(struct gpmc_timings *t,
+					    unsigned int flags,
+					    int freq)
 {
 	struct gpmc_device_timings dev_t;
-	struct gpmc_timings t;
 	const int t_cer  = 15;
 	const int t_avdp = 12;
 	const int t_cez  = 20; /* max of t_cez, t_oez */
@@ -172,9 +168,9 @@ omap2_onenand_calc_sync_timings(struct omap_onenand_platform_data *cfg,
 	int min_gpmc_clk_period, t_ces, t_avds, t_avdh, t_ach, t_aavdh, t_rdyo;
 	int div, gpmc_clk_ns;
 
-	if (cfg->flags & ONENAND_SYNC_READ)
+	if (flags & ONENAND_SYNC_READ)
 		onenand_flags = ONENAND_FLAG_SYNCREAD;
-	else if (cfg->flags & ONENAND_SYNC_READWRITE)
+	else if (flags & ONENAND_SYNC_READWRITE)
 		onenand_flags = ONENAND_FLAG_SYNCREAD | ONENAND_FLAG_SYNCWRITE;
 
 	switch (freq) {
@@ -265,9 +261,7 @@ omap2_onenand_calc_sync_timings(struct omap_onenand_platform_data *cfg,
 	dev_t.cyc_aavdh_oe = 1;
 	dev_t.t_rdyo = t_rdyo * 1000 + min_gpmc_clk_period;
 
-	gpmc_calc_timings(&t, &dev_t);
-
-	return t;
+	gpmc_calc_timings(t, &dev_t);
 }
 
 static int gpmc_set_sync_mode(int cs, struct gpmc_timings *t)
@@ -300,7 +294,7 @@ static int omap2_onenand_setup_async(void __iomem *onenand_base)
 
 	omap2_onenand_set_async_mode(onenand_base);
 
-	t = omap2_onenand_calc_async_timings();
+	omap2_onenand_calc_async_timings(&t);
 
 	ret = gpmc_set_async_mode(gpmc_onenand_data->cs, &t);
 	if (IS_ERR_VALUE(ret))
@@ -322,7 +316,7 @@ static int omap2_onenand_setup_sync(void __iomem *onenand_base, int *freq_ptr)
 		set_onenand_cfg(onenand_base);
 	}
 
-	t = omap2_onenand_calc_sync_timings(gpmc_onenand_data, freq);
+	omap2_onenand_calc_sync_timings(&t, gpmc_onenand_data->flags, freq);
 
 	ret = gpmc_set_sync_mode(gpmc_onenand_data->cs, &t);
 	if (IS_ERR_VALUE(ret))
-- 
1.7.10.4

[PATCH 02/14] ARM: OMAP2+: Add variable to store number of GPMC waitpins

[Jon Hunter]

The GPMC has wait-pin signals that can be assigned to a chip-select
to monitor the ready signal of an external device. Add a variable to
indicate the total number of wait-pins for a given device. This will
allow us to detect if the wait-pin being selected is valid or not.

When booting with device-tree read the number of wait-pins from the
device-tree blob. When device-tree is not used set the number of
wait-pins to 4 which is valid for OMAP2-5 devices. Newer devices
that have less wait-pins (such as AM335x) only support booting with
device-tree and so hard-coding the wait-pin number when not using
device-tree is fine.

Signed-off-by: Jon Hunter <jon-hunter@ti.com>
---
 arch/arm/mach-omap2/gpmc.c |   14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/arch/arm/mach-omap2/gpmc.c b/arch/arm/mach-omap2/gpmc.c
index 410e1ba..1c63de0 100644
--- a/arch/arm/mach-omap2/gpmc.c
+++ b/arch/arm/mach-omap2/gpmc.c
@@ -108,6 +108,8 @@
 #define	GPMC_HAS_WR_ACCESS		0x1
 #define	GPMC_HAS_WR_DATA_MUX_BUS	0x2
 
+#define GPMC_NR_WAITPINS		4
+
 /* XXX: Only NAND irq has been considered,currently these are the only ones used
  */
 #define	GPMC_NR_IRQ		2
@@ -153,6 +155,7 @@ static struct resource	gpmc_cs_mem[GPMC_CS_NUM];
 static DEFINE_SPINLOCK(gpmc_mem_lock);
 /* Define chip-selects as reserved by default until probe completes */
 static unsigned int gpmc_cs_map = ((1 << GPMC_CS_NUM) - 1);
+static unsigned int gpmc_nr_waitpins;
 static struct device *gpmc_dev;
 static int gpmc_irq;
 static resource_size_t phys_base, mem_size;
@@ -1305,6 +1308,14 @@ static int gpmc_probe_dt(struct platform_device *pdev)
 	if (!of_id)
 		return 0;
 
+	ret = of_property_read_u32(pdev->dev.of_node, "gpmc,num-waitpins",
+				   &gpmc_nr_waitpins);
+	if (ret) {
+		pr_err("%s: number of wait pins not found!\n", __func__);
+		return ret;
+	}
+
+
 	for_each_node_by_name(child, "nand") {
 		ret = gpmc_probe_nand_child(pdev, child);
 		if (ret < 0) {
@@ -1383,6 +1394,9 @@ static int gpmc_probe(struct platform_device *pdev)
 	/* Now the GPMC is initialised, unreserve the chip-selects */
 	gpmc_cs_map = 0;
 
+	if (!pdev->dev.of_node)
+		gpmc_nr_waitpins = GPMC_NR_WAITPINS;
+
 	rc = gpmc_probe_dt(pdev);
 	if (rc < 0) {
 		clk_disable_unprepare(gpmc_l3_clk);
-- 
1.7.10.4

[PATCH 03/14] ARM: OMAP2+: Add structure for storing GPMC settings

[Jon Hunter]

The GPMC has various different configuration options such as bus-width,
synchronous or asychronous mode selection, burst mode options etc.
Currently, there is no central structure for storing all these options
when configuring the GPMC for a given device. Some of the options are
stored in the GPMC timing structure and some are directly programmed
into the GPMC configuration register. Add a new structure to store
these options and convert code to use this structure. Adding this
structure will allow us to create a common function for configuring
these options.

Signed-off-by: Jon Hunter <jon-hunter@ti.com>
---
 arch/arm/mach-omap2/gpmc-onenand.c |   18 ++++++++++-----
 arch/arm/mach-omap2/gpmc-smc91x.c  |    2 +-
 arch/arm/mach-omap2/gpmc.c         |   45 +++++++++++++++++++++---------------
 arch/arm/mach-omap2/gpmc.h         |   22 ++++++++++++++----
 arch/arm/mach-omap2/usb-tusb6010.c |   19 ++++++++-------
 5 files changed, 68 insertions(+), 38 deletions(-)

diff --git a/arch/arm/mach-omap2/gpmc-onenand.c b/arch/arm/mach-omap2/gpmc-onenand.c
index 758b09c..8b1c08e 100644
--- a/arch/arm/mach-omap2/gpmc-onenand.c
+++ b/arch/arm/mach-omap2/gpmc-onenand.c
@@ -47,6 +47,15 @@ static struct platform_device gpmc_onenand_device = {
 	.resource	= &gpmc_onenand_resource,
 };
 
+static struct gpmc_settings onenand_async = {
+	.mux_add_data	= true,
+};
+
+static struct gpmc_settings onenand_sync = {
+	.burst_read	= true,
+	.mux_add_data	= true,
+};
+
 static void omap2_onenand_calc_async_timings(struct gpmc_timings *t)
 {
 	struct gpmc_device_timings dev_t;
@@ -63,7 +72,6 @@ static void omap2_onenand_calc_async_timings(struct gpmc_timings *t)
 
 	memset(&dev_t, 0, sizeof(dev_t));
 
-	dev_t.mux = true;
 	dev_t.t_avdp_r = max_t(int, t_avdp, t_cer) * 1000;
 	dev_t.t_avdp_w = dev_t.t_avdp_r;
 	dev_t.t_aavdh = t_aavdh * 1000;
@@ -75,7 +83,7 @@ static void omap2_onenand_calc_async_timings(struct gpmc_timings *t)
 	dev_t.t_wpl = t_wpl * 1000;
 	dev_t.t_wph = t_wph * 1000;
 
-	gpmc_calc_timings(t, &dev_t);
+	gpmc_calc_timings(t, &onenand_async, &dev_t);
 }
 
 static int gpmc_set_async_mode(int cs, struct gpmc_timings *t)
@@ -235,10 +243,8 @@ static void omap2_onenand_calc_sync_timings(struct gpmc_timings *t,
 	/* Set synchronous read timings */
 	memset(&dev_t, 0, sizeof(dev_t));
 
-	dev_t.mux = true;
-	dev_t.sync_read = true;
 	if (onenand_flags & ONENAND_FLAG_SYNCWRITE) {
-		dev_t.sync_write = true;
+		onenand_sync.sync_write = true;
 	} else {
 		dev_t.t_avdp_w = max(t_avdp, t_cer) * 1000;
 		dev_t.t_wpl = t_wpl * 1000;
@@ -261,7 +267,7 @@ static void omap2_onenand_calc_sync_timings(struct gpmc_timings *t,
 	dev_t.cyc_aavdh_oe = 1;
 	dev_t.t_rdyo = t_rdyo * 1000 + min_gpmc_clk_period;
 
-	gpmc_calc_timings(t, &dev_t);
+	gpmc_calc_timings(t, &onenand_sync, &dev_t);
 }
 
 static int gpmc_set_sync_mode(int cs, struct gpmc_timings *t)
diff --git a/arch/arm/mach-omap2/gpmc-smc91x.c b/arch/arm/mach-omap2/gpmc-smc91x.c
index 11d0b75..4b78338 100644
--- a/arch/arm/mach-omap2/gpmc-smc91x.c
+++ b/arch/arm/mach-omap2/gpmc-smc91x.c
@@ -104,7 +104,7 @@ static int smc91c96_gpmc_retime(void)
 	dev_t.t_cez_w = t4_w * 1000;
 	dev_t.t_wr_cycle = (t20 - t3) * 1000;
 
-	gpmc_calc_timings(&t, &dev_t);
+	gpmc_calc_timings(&t, NULL, &dev_t);
 
 	return gpmc_cs_set_timings(gpmc_cfg->cs, &t);
 }
diff --git a/arch/arm/mach-omap2/gpmc.c b/arch/arm/mach-omap2/gpmc.c
index 1c63de0..465cac9 100644
--- a/arch/arm/mach-omap2/gpmc.c
+++ b/arch/arm/mach-omap2/gpmc.c
@@ -828,9 +828,9 @@ static u32 gpmc_round_ps_to_sync_clk(u32 time_ps, u32 sync_clk)
 
 /* XXX: can the cycles be avoided ? */
 static int gpmc_calc_sync_read_timings(struct gpmc_timings *gpmc_t,
-				struct gpmc_device_timings *dev_t)
+				       struct gpmc_device_timings *dev_t,
+				       bool mux)
 {
-	bool mux = dev_t->mux;
 	u32 temp;
 
 	/* adv_rd_off */
@@ -883,9 +883,9 @@ static int gpmc_calc_sync_read_timings(struct gpmc_timings *gpmc_t,
 }
 
 static int gpmc_calc_sync_write_timings(struct gpmc_timings *gpmc_t,
-				struct gpmc_device_timings *dev_t)
+					struct gpmc_device_timings *dev_t,
+					bool mux)
 {
-	bool mux = dev_t->mux;
 	u32 temp;
 
 	/* adv_wr_off */
@@ -945,9 +945,9 @@ static int gpmc_calc_sync_write_timings(struct gpmc_timings *gpmc_t,
 }
 
 static int gpmc_calc_async_read_timings(struct gpmc_timings *gpmc_t,
-				struct gpmc_device_timings *dev_t)
+					struct gpmc_device_timings *dev_t,
+					bool mux)
 {
-	bool mux = dev_t->mux;
 	u32 temp;
 
 	/* adv_rd_off */
@@ -985,9 +985,9 @@ static int gpmc_calc_async_read_timings(struct gpmc_timings *gpmc_t,
 }
 
 static int gpmc_calc_async_write_timings(struct gpmc_timings *gpmc_t,
-				struct gpmc_device_timings *dev_t)
+					 struct gpmc_device_timings *dev_t,
+					 bool mux)
 {
-	bool mux = dev_t->mux;
 	u32 temp;
 
 	/* adv_wr_off */
@@ -1057,7 +1057,8 @@ static int gpmc_calc_sync_common_timings(struct gpmc_timings *gpmc_t,
 }
 
 static int gpmc_calc_common_timings(struct gpmc_timings *gpmc_t,
-			struct gpmc_device_timings *dev_t)
+				    struct gpmc_device_timings *dev_t,
+				    bool sync)
 {
 	u32 temp;
 
@@ -1071,7 +1072,7 @@ static int gpmc_calc_common_timings(struct gpmc_timings *gpmc_t,
 				gpmc_t->cs_on + dev_t->t_ce_avd);
 	gpmc_t->adv_on = gpmc_round_ps_to_ticks(temp);
 
-	if (dev_t->sync_write || dev_t->sync_read)
+	if (sync)
 		gpmc_calc_sync_common_timings(gpmc_t, dev_t);
 
 	return 0;
@@ -1106,21 +1107,29 @@ static void gpmc_convert_ps_to_ns(struct gpmc_timings *t)
 }
 
 int gpmc_calc_timings(struct gpmc_timings *gpmc_t,
-			struct gpmc_device_timings *dev_t)
+		      struct gpmc_settings *gpmc_s,
+		      struct gpmc_device_timings *dev_t)
 {
+	bool mux = false, sync = false;
+
+	if (gpmc_s) {
+		mux = gpmc_s->mux_add_data;
+		sync = (gpmc_s->sync_read || gpmc_s->sync_write);
+	}
+
 	memset(gpmc_t, 0, sizeof(*gpmc_t));
 
-	gpmc_calc_common_timings(gpmc_t, dev_t);
+	gpmc_calc_common_timings(gpmc_t, dev_t, sync);
 
-	if (dev_t->sync_read)
-		gpmc_calc_sync_read_timings(gpmc_t, dev_t);
+	if (gpmc_s && gpmc_s->sync_read)
+		gpmc_calc_sync_read_timings(gpmc_t, dev_t, mux);
 	else
-		gpmc_calc_async_read_timings(gpmc_t, dev_t);
+		gpmc_calc_async_read_timings(gpmc_t, dev_t, mux);
 
-	if (dev_t->sync_write)
-		gpmc_calc_sync_write_timings(gpmc_t, dev_t);
+	if (gpmc_s && gpmc_s->sync_write)
+		gpmc_calc_sync_write_timings(gpmc_t, dev_t, mux);
 	else
-		gpmc_calc_async_write_timings(gpmc_t, dev_t);
+		gpmc_calc_async_write_timings(gpmc_t, dev_t, mux);
 
 	/* TODO: remove, see function definition */
 	gpmc_convert_ps_to_ns(gpmc_t);
diff --git a/arch/arm/mach-omap2/gpmc.h b/arch/arm/mach-omap2/gpmc.h
index fe0a844..785e9ce 100644
--- a/arch/arm/mach-omap2/gpmc.h
+++ b/arch/arm/mach-omap2/gpmc.h
@@ -178,10 +178,6 @@ struct gpmc_device_timings {
 	u8 cyc_wpl;	/* write deassertion time in cycles */
 	u32 cyc_iaa;	/* initial access time in cycles */
 
-	bool mux;	/* address & data muxed */
-	bool sync_write;/* synchronous write */
-	bool sync_read;	/* synchronous read */
-
 	/* extra delays */
 	bool ce_xdelay;
 	bool avd_xdelay;
@@ -189,8 +185,24 @@ struct gpmc_device_timings {
 	bool we_xdelay;
 };
 
+struct gpmc_settings {
+	bool burst_wrap;	/* enables wrap bursting */
+	bool burst_read;	/* enables read page/burst mode */
+	bool burst_write;	/* enables write page/burst mode */
+	bool device_nand;	/* device is NAND */
+	bool mux_add_data;	/* multiplex address & data */
+	bool sync_read;		/* enables synchronous reads */
+	bool sync_write;	/* enables synchronous writes */
+	bool wait_on_read;	/* monitor wait on reads */
+	bool wait_on_write;	/* monitor wait on writes */
+	u32 burst_len;		/* page/burst length */
+	u32 device_width;	/* device bus width (8 or 16 bit) */
+	u32 wait_pin;		/* wait-pin to be used */
+};
+
 extern int gpmc_calc_timings(struct gpmc_timings *gpmc_t,
-				struct gpmc_device_timings *dev_t);
+			     struct gpmc_settings *gpmc_s,
+			     struct gpmc_device_timings *dev_t);
 
 extern void gpmc_update_nand_reg(struct gpmc_nand_regs *reg, int cs);
 extern int gpmc_get_client_irq(unsigned irq_config);
diff --git a/arch/arm/mach-omap2/usb-tusb6010.c b/arch/arm/mach-omap2/usb-tusb6010.c
index c5a3c6f..940410a 100644
--- a/arch/arm/mach-omap2/usb-tusb6010.c
+++ b/arch/arm/mach-omap2/usb-tusb6010.c
@@ -26,6 +26,15 @@
 static u8		async_cs, sync_cs;
 static unsigned		refclk_psec;
 
+static struct gpmc_settings tusb_async = {
+	.mux_add_data	= true,
+};
+
+static struct gpmc_settings tusb_sync = {
+	.mux_add_data	= true,
+	.sync_read	= true,
+	.sync_write	= true,
+};
 
 /* NOTE:  timings are from tusb 6010 datasheet Rev 1.8, 12-Sept 2006 */
 
@@ -37,8 +46,6 @@ static int tusb_set_async_mode(unsigned sysclk_ps)
 
 	memset(&dev_t, 0, sizeof(dev_t));
 
-	dev_t.mux = true;
-
 	dev_t.t_ceasu = 8 * 1000;
 	dev_t.t_avdasu = t_acsnh_advnh - 7000;
 	dev_t.t_ce_avd = 1000;
@@ -52,7 +59,7 @@ static int tusb_set_async_mode(unsigned sysclk_ps)
 	dev_t.t_wpl = 300;
 	dev_t.cyc_aavdh_we = 1;
 
-	gpmc_calc_timings(&t, &dev_t);
+	gpmc_calc_timings(&t, &tusb_async, &dev_t);
 
 	return gpmc_cs_set_timings(async_cs, &t);
 }
@@ -65,10 +72,6 @@ static int tusb_set_sync_mode(unsigned sysclk_ps)
 
 	memset(&dev_t, 0, sizeof(dev_t));
 
-	dev_t.mux = true;
-	dev_t.sync_read = true;
-	dev_t.sync_write = true;
-
 	dev_t.clk = 11100;
 	dev_t.t_bacc = 1000;
 	dev_t.t_ces = 1000;
@@ -84,7 +87,7 @@ static int tusb_set_sync_mode(unsigned sysclk_ps)
 	dev_t.cyc_wpl = 6;
 	dev_t.t_ce_rdyz = 7000;
 
-	gpmc_calc_timings(&t, &dev_t);
+	gpmc_calc_timings(&t, &tusb_sync, &dev_t);
 
 	return gpmc_cs_set_timings(sync_cs, &t);
 }
-- 
1.7.10.4

[PATCH 04/14] ARM: OMAP2+: Add function for configuring GPMC settings

[Jon Hunter]

The GPMC has various different configuration options such as bus-width,
synchronous or asychronous mode selection, burst mode options etc.
Currently, there is no common function for configuring these options and
various devices set these options by either programming the GPMC CONFIG1
register directly or by calling gpmc_cs_configure() to set some of the
options.

Add a new function for configuring all of the GPMC options. Having a common
function for configuring this options will simplify code and ease the
migration to device-tree.

Signed-off-by: Jon Hunter <jon-hunter@ti.com>
---
 arch/arm/mach-omap2/gpmc.c |   65 ++++++++++++++++++++++++++++++++++++++++++++
 arch/arm/mach-omap2/gpmc.h |    6 ++++
 2 files changed, 71 insertions(+)

diff --git a/arch/arm/mach-omap2/gpmc.c b/arch/arm/mach-omap2/gpmc.c
index 465cac9..fb8dfd2 100644
--- a/arch/arm/mach-omap2/gpmc.c
+++ b/arch/arm/mach-omap2/gpmc.c
@@ -1137,6 +1137,71 @@ int gpmc_calc_timings(struct gpmc_timings *gpmc_t,
 	return 0;
 }
 
+int gpmc_cs_program_settings(int cs, struct gpmc_settings *p)
+{
+	u32 config1;
+
+	if ((!p->device_width) || (p->device_width > GPMC_DEVWIDTH_16BIT)) {
+		pr_err("%s: invalid width %d!", __func__, p->device_width);
+		return -EINVAL;
+	}
+
+	/* Address-data multiplexing not supported for NAND devices */
+	if (p->device_nand && p->mux_add_data) {
+		pr_err("%s: invalid configuration!\n", __func__);
+		return -EINVAL;
+	}
+
+	/* Page/burst mode supports lengths of 4, 8 and 16 bytes */
+	if (p->burst_read || p->burst_write) {
+		switch (p->burst_len) {
+		case GPMC_BURST_4:
+		case GPMC_BURST_8:
+		case GPMC_BURST_16:
+			break;
+		default:
+			pr_err("%s: invalid page/burst-length (%d)\n",
+			       __func__, p->burst_len);
+			return -EINVAL;
+		}
+	}
+
+	if ((p->wait_on_read || p->wait_on_write) &&
+	    (p->wait_pin > gpmc_nr_waitpins)) {
+		pr_err("%s: invalid wait-pin (%d)\n", __func__, p->wait_pin);
+		return -EINVAL;
+	}
+
+	config1 = GPMC_CONFIG1_DEVICESIZE((p->device_width - 1));
+
+	if (p->sync_read)
+		config1 |= GPMC_CONFIG1_READTYPE_SYNC;
+	if (p->sync_write)
+		config1 |= GPMC_CONFIG1_WRITETYPE_SYNC;
+	if (p->wait_on_read)
+		config1 |= GPMC_CONFIG1_WAIT_READ_MON;
+	if (p->wait_on_write)
+		config1 |= GPMC_CONFIG1_WAIT_WRITE_MON;
+	if (p->wait_on_read || p->wait_on_write)
+		config1 |= GPMC_CONFIG1_WAIT_PIN_SEL(p->wait_pin);
+	if (p->device_nand)
+		config1	|= GPMC_CONFIG1_DEVICETYPE(GPMC_DEVICETYPE_NAND);
+	if (p->mux_add_data)
+		config1	|= GPMC_CONFIG1_MUXADDDATA;
+	if (p->burst_read)
+		config1 |= GPMC_CONFIG1_READMULTIPLE_SUPP;
+	if (p->burst_write)
+		config1 |= GPMC_CONFIG1_WRITEMULTIPLE_SUPP;
+	if (p->burst_read || p->burst_write) {
+		config1 |= GPMC_CONFIG1_PAGE_LEN(p->burst_len >> 3);
+		config1 |= p->burst_wrap ? GPMC_CONFIG1_WRAPBURST_SUPP : 0;
+	}
+
+	gpmc_cs_write_reg(cs, GPMC_CS_CONFIG1, config1);
+
+	return 0;
+}
+
 #ifdef CONFIG_OF
 static struct of_device_id gpmc_dt_ids[] = {
 	{ .compatible = "ti,omap2420-gpmc" },
diff --git a/arch/arm/mach-omap2/gpmc.h b/arch/arm/mach-omap2/gpmc.h
index 785e9ce..638f94a 100644
--- a/arch/arm/mach-omap2/gpmc.h
+++ b/arch/arm/mach-omap2/gpmc.h
@@ -73,6 +73,11 @@
 #define GPMC_IRQ_FIFOEVENTENABLE	0x01
 #define GPMC_IRQ_COUNT_EVENT		0x02
 
+#define GPMC_BURST_4			4
+#define GPMC_BURST_8			8
+#define GPMC_BURST_16			16
+#define GPMC_DEVWIDTH_8BIT		1
+#define GPMC_DEVWIDTH_16BIT		2
 
 /* bool type time settings */
 struct gpmc_bool_timings {
@@ -217,6 +222,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_calc_divider(unsigned int sync_clk);
 extern int gpmc_cs_set_timings(int cs, const struct gpmc_timings *t);
+extern int gpmc_cs_program_settings(int cs, struct gpmc_settings *p);
 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);
-- 
1.7.10.4

[PATCH 04/14] ARM: OMAP2+: Add function for configuring GPMC settings

[Philip, Avinash]

On Tue, Feb 26, 2013 at 23:00:31, Hunter, Jon wrote:
> The GPMC has various different configuration options such as bus-width,
> synchronous or asychronous mode selection, burst mode options etc.
> Currently, there is no common function for configuring these options and
> various devices set these options by either programming the GPMC CONFIG1
> register directly or by calling gpmc_cs_configure() to set some of the
> options.
> 
> Add a new function for configuring all of the GPMC options. Having a common
> function for configuring this options will simplify code and ease the
> migration to device-tree.
> 
> Signed-off-by: Jon Hunter <jon-hunter@ti.com>
> ---
>  arch/arm/mach-omap2/gpmc.c |   65 ++++++++++++++++++++++++++++++++++++++++++++
>  arch/arm/mach-omap2/gpmc.h |    6 ++++
>  2 files changed, 71 insertions(+)
> 
> diff --git a/arch/arm/mach-omap2/gpmc.c b/arch/arm/mach-omap2/gpmc.c
> index 465cac9..fb8dfd2 100644
> --- a/arch/arm/mach-omap2/gpmc.c
> +++ b/arch/arm/mach-omap2/gpmc.c
> @@ -1137,6 +1137,71 @@ int gpmc_calc_timings(struct gpmc_timings *gpmc_t,
>  	return 0;
>  }
>  
> +int gpmc_cs_program_settings(int cs, struct gpmc_settings *p)
> +{
> +	u32 config1;
> +
> +	if ((!p->device_width) || (p->device_width > GPMC_DEVWIDTH_16BIT)) {
> +		pr_err("%s: invalid width %d!", __func__, p->device_width);
> +		return -EINVAL;
> +	}
> +
> +	/* Address-data multiplexing not supported for NAND devices */
> +	if (p->device_nand && p->mux_add_data) {
> +		pr_err("%s: invalid configuration!\n", __func__);
> +		return -EINVAL;
> +	}
> +
> +	/* Page/burst mode supports lengths of 4, 8 and 16 bytes */
> +	if (p->burst_read || p->burst_write) {
> +		switch (p->burst_len) {
> +		case GPMC_BURST_4:
> +		case GPMC_BURST_8:
> +		case GPMC_BURST_16:
> +			break;
> +		default:
> +			pr_err("%s: invalid page/burst-length (%d)\n",
> +			       __func__, p->burst_len);
> +			return -EINVAL;
> +		}
> +	}
> +
> +	if ((p->wait_on_read || p->wait_on_write) &&
> +	    (p->wait_pin > gpmc_nr_waitpins)) {
> +		pr_err("%s: invalid wait-pin (%d)\n", __func__, p->wait_pin);
> +		return -EINVAL;
> +	}
> +
> +	config1 = GPMC_CONFIG1_DEVICESIZE((p->device_width - 1));

Can you consider read_modify approach?
Thanks
Avinash 

[PATCH 04/14] ARM: OMAP2+: Add function for configuring GPMC settings

[Jon Hunter]

On 02/28/2013 12:05 AM, Philip, Avinash wrote:
> On Tue, Feb 26, 2013 at 23:00:31, Hunter, Jon wrote:
>> The GPMC has various different configuration options such as bus-width,
>> synchronous or asychronous mode selection, burst mode options etc.
>> Currently, there is no common function for configuring these options and
>> various devices set these options by either programming the GPMC CONFIG1
>> register directly or by calling gpmc_cs_configure() to set some of the
>> options.
>>
>> Add a new function for configuring all of the GPMC options. Having a common
>> function for configuring this options will simplify code and ease the
>> migration to device-tree.
>>
>> Signed-off-by: Jon Hunter <jon-hunter@ti.com>
>> ---
>>  arch/arm/mach-omap2/gpmc.c |   65 ++++++++++++++++++++++++++++++++++++++++++++
>>  arch/arm/mach-omap2/gpmc.h |    6 ++++
>>  2 files changed, 71 insertions(+)
>>
>> diff --git a/arch/arm/mach-omap2/gpmc.c b/arch/arm/mach-omap2/gpmc.c
>> index 465cac9..fb8dfd2 100644
>> --- a/arch/arm/mach-omap2/gpmc.c
>> +++ b/arch/arm/mach-omap2/gpmc.c
>> @@ -1137,6 +1137,71 @@ int gpmc_calc_timings(struct gpmc_timings *gpmc_t,
>>  	return 0;
>>  }
>>  
>> +int gpmc_cs_program_settings(int cs, struct gpmc_settings *p)
>> +{
>> +	u32 config1;
>> +
>> +	if ((!p->device_width) || (p->device_width > GPMC_DEVWIDTH_16BIT)) {
>> +		pr_err("%s: invalid width %d!", __func__, p->device_width);
>> +		return -EINVAL;
>> +	}
>> +
>> +	/* Address-data multiplexing not supported for NAND devices */
>> +	if (p->device_nand && p->mux_add_data) {
>> +		pr_err("%s: invalid configuration!\n", __func__);
>> +		return -EINVAL;
>> +	}
>> +
>> +	/* Page/burst mode supports lengths of 4, 8 and 16 bytes */
>> +	if (p->burst_read || p->burst_write) {
>> +		switch (p->burst_len) {
>> +		case GPMC_BURST_4:
>> +		case GPMC_BURST_8:
>> +		case GPMC_BURST_16:
>> +			break;
>> +		default:
>> +			pr_err("%s: invalid page/burst-length (%d)\n",
>> +			       __func__, p->burst_len);
>> +			return -EINVAL;
>> +		}
>> +	}
>> +
>> +	if ((p->wait_on_read || p->wait_on_write) &&
>> +	    (p->wait_pin > gpmc_nr_waitpins)) {
>> +		pr_err("%s: invalid wait-pin (%d)\n", __func__, p->wait_pin);
>> +		return -EINVAL;
>> +	}
>> +
>> +	config1 = GPMC_CONFIG1_DEVICESIZE((p->device_width - 1));
> 
> Can you consider read_modify approach?

I was purposely trying to avoid that. The intent here is to program all
the settings and get away from any boot-loader dependencies. If we use a
read-modify approach then it will never be clear in the kernel what
should actually be programmed.

Cheers
Jon

[PATCH 04/14] ARM: OMAP2+: Add function for configuring GPMC settings

[Jon Hunter]

On 02/28/2013 09:52 AM, Jon Hunter wrote:
> 
> On 02/28/2013 12:05 AM, Philip, Avinash wrote:
>> On Tue, Feb 26, 2013 at 23:00:31, Hunter, Jon wrote:
>>> The GPMC has various different configuration options such as bus-width,
>>> synchronous or asychronous mode selection, burst mode options etc.
>>> Currently, there is no common function for configuring these options and
>>> various devices set these options by either programming the GPMC CONFIG1
>>> register directly or by calling gpmc_cs_configure() to set some of the
>>> options.
>>>
>>> Add a new function for configuring all of the GPMC options. Having a common
>>> function for configuring this options will simplify code and ease the
>>> migration to device-tree.
>>>
>>> Signed-off-by: Jon Hunter <jon-hunter@ti.com>
>>> ---
>>>  arch/arm/mach-omap2/gpmc.c |   65 ++++++++++++++++++++++++++++++++++++++++++++
>>>  arch/arm/mach-omap2/gpmc.h |    6 ++++
>>>  2 files changed, 71 insertions(+)
>>>
>>> diff --git a/arch/arm/mach-omap2/gpmc.c b/arch/arm/mach-omap2/gpmc.c
>>> index 465cac9..fb8dfd2 100644
>>> --- a/arch/arm/mach-omap2/gpmc.c
>>> +++ b/arch/arm/mach-omap2/gpmc.c
>>> @@ -1137,6 +1137,71 @@ int gpmc_calc_timings(struct gpmc_timings *gpmc_t,
>>>  	return 0;
>>>  }
>>>  
>>> +int gpmc_cs_program_settings(int cs, struct gpmc_settings *p)
>>> +{
>>> +	u32 config1;
>>> +
>>> +	if ((!p->device_width) || (p->device_width > GPMC_DEVWIDTH_16BIT)) {
>>> +		pr_err("%s: invalid width %d!", __func__, p->device_width);
>>> +		return -EINVAL;
>>> +	}
>>> +
>>> +	/* Address-data multiplexing not supported for NAND devices */
>>> +	if (p->device_nand && p->mux_add_data) {
>>> +		pr_err("%s: invalid configuration!\n", __func__);
>>> +		return -EINVAL;
>>> +	}
>>> +
>>> +	/* Page/burst mode supports lengths of 4, 8 and 16 bytes */
>>> +	if (p->burst_read || p->burst_write) {
>>> +		switch (p->burst_len) {
>>> +		case GPMC_BURST_4:
>>> +		case GPMC_BURST_8:
>>> +		case GPMC_BURST_16:
>>> +			break;
>>> +		default:
>>> +			pr_err("%s: invalid page/burst-length (%d)\n",
>>> +			       __func__, p->burst_len);
>>> +			return -EINVAL;
>>> +		}
>>> +	}
>>> +
>>> +	if ((p->wait_on_read || p->wait_on_write) &&
>>> +	    (p->wait_pin > gpmc_nr_waitpins)) {
>>> +		pr_err("%s: invalid wait-pin (%d)\n", __func__, p->wait_pin);
>>> +		return -EINVAL;
>>> +	}
>>> +
>>> +	config1 = GPMC_CONFIG1_DEVICESIZE((p->device_width - 1));
>>
>> Can you consider read_modify approach?
> 
> I was purposely trying to avoid that. The intent here is to program all
> the settings and get away from any boot-loader dependencies. If we use a
> read-modify approach then it will never be clear in the kernel what
> should actually be programmed.

By the way, it would be good to know what your motivation for this is.

My goal is for all gpmc settings to eventually live in the DT blob and
there be no dependency on the bootloader whatsoever. That may mean that
settings are re-programmed again by the kernel but IMO that is best.

Cheers
Jon

[PATCH 04/14] ARM: OMAP2+: Add function for configuring GPMC settings

[Philip, Avinash]

On Thu, Feb 28, 2013 at 22:42:37, Hunter, Jon wrote:
> 
> On 02/28/2013 09:52 AM, Jon Hunter wrote:
> > 
> > On 02/28/2013 12:05 AM, Philip, Avinash wrote:
> >> On Tue, Feb 26, 2013 at 23:00:31, Hunter, Jon wrote:
> >>> The GPMC has various different configuration options such as bus-width,
> >>> synchronous or asychronous mode selection, burst mode options etc.
> >>> Currently, there is no common function for configuring these options and
> >>> various devices set these options by either programming the GPMC CONFIG1
> >>> register directly or by calling gpmc_cs_configure() to set some of the
> >>> options.
> >>>
> >>> Add a new function for configuring all of the GPMC options. Having a common
> >>> function for configuring this options will simplify code and ease the
> >>> migration to device-tree.
> >>>
> >>> Signed-off-by: Jon Hunter <jon-hunter@ti.com>
> >>> ---
> >>>  arch/arm/mach-omap2/gpmc.c |   65 ++++++++++++++++++++++++++++++++++++++++++++
> >>>  arch/arm/mach-omap2/gpmc.h |    6 ++++
> >>>  2 files changed, 71 insertions(+)
> >>>
> >>> diff --git a/arch/arm/mach-omap2/gpmc.c b/arch/arm/mach-omap2/gpmc.c
> >>> index 465cac9..fb8dfd2 100644
> >>> --- a/arch/arm/mach-omap2/gpmc.c
> >>> +++ b/arch/arm/mach-omap2/gpmc.c
> >>> @@ -1137,6 +1137,71 @@ int gpmc_calc_timings(struct gpmc_timings *gpmc_t,
> >>>  	return 0;
> >>>  }
> >>>  
> >>> +int gpmc_cs_program_settings(int cs, struct gpmc_settings *p)
> >>> +{
> >>> +	u32 config1;
> >>> +
> >>> +	if ((!p->device_width) || (p->device_width > GPMC_DEVWIDTH_16BIT)) {
> >>> +		pr_err("%s: invalid width %d!", __func__, p->device_width);
> >>> +		return -EINVAL;
> >>> +	}
> >>> +
> >>> +	/* Address-data multiplexing not supported for NAND devices */
> >>> +	if (p->device_nand && p->mux_add_data) {
> >>> +		pr_err("%s: invalid configuration!\n", __func__);
> >>> +		return -EINVAL;
> >>> +	}
> >>> +
> >>> +	/* Page/burst mode supports lengths of 4, 8 and 16 bytes */
> >>> +	if (p->burst_read || p->burst_write) {
> >>> +		switch (p->burst_len) {
> >>> +		case GPMC_BURST_4:
> >>> +		case GPMC_BURST_8:
> >>> +		case GPMC_BURST_16:
> >>> +			break;
> >>> +		default:
> >>> +			pr_err("%s: invalid page/burst-length (%d)\n",
> >>> +			       __func__, p->burst_len);
> >>> +			return -EINVAL;
> >>> +		}
> >>> +	}
> >>> +
> >>> +	if ((p->wait_on_read || p->wait_on_write) &&
> >>> +	    (p->wait_pin > gpmc_nr_waitpins)) {
> >>> +		pr_err("%s: invalid wait-pin (%d)\n", __func__, p->wait_pin);
> >>> +		return -EINVAL;
> >>> +	}
> >>> +
> >>> +	config1 = GPMC_CONFIG1_DEVICESIZE((p->device_width - 1));
> >>
> >> Can you consider read_modify approach?
> > 
> > I was purposely trying to avoid that. The intent here is to program all
> > the settings and get away from any boot-loader dependencies. If we use a
> > read-modify approach then it will never be clear in the kernel what
> > should actually be programmed.
> 
> By the way, it would be good to know what your motivation for this is.
> 
> My goal is for all gpmc settings to eventually live in the DT blob and
> there be no dependency on the bootloader whatsoever. That may mean that
> settings are re-programmed again by the kernel but IMO that is best.

Yes I understood now and the right thing to do.
Suggested read modify approach because of some confusion as GPMC_CONFIG1 is already
modified in gpmc_cs_set_timings() and is called from omap2_nand_gpmc_retime().
So the data modified in gpmc_cs_set_timings() will lost (not significant)
May be better and cleaner solution is to remove GPMC_CONFIG1 modification in
gpmc_cs_set_timings() also.

Thanks
Avinash
> 
> Cheers
> Jon
> 

[PATCH 04/14] ARM: OMAP2+: Add function for configuring GPMC settings

[Jon Hunter]

On 02/28/2013 11:33 PM, Philip, Avinash wrote:
> On Thu, Feb 28, 2013 at 22:42:37, Hunter, Jon wrote:
>>
>> On 02/28/2013 09:52 AM, Jon Hunter wrote:
>>>
>>> On 02/28/2013 12:05 AM, Philip, Avinash wrote:
>>>> On Tue, Feb 26, 2013 at 23:00:31, Hunter, Jon wrote:
>>>>> The GPMC has various different configuration options such as bus-width,
>>>>> synchronous or asychronous mode selection, burst mode options etc.
>>>>> Currently, there is no common function for configuring these options and
>>>>> various devices set these options by either programming the GPMC CONFIG1
>>>>> register directly or by calling gpmc_cs_configure() to set some of the
>>>>> options.
>>>>>
>>>>> Add a new function for configuring all of the GPMC options. Having a common
>>>>> function for configuring this options will simplify code and ease the
>>>>> migration to device-tree.
>>>>>
>>>>> Signed-off-by: Jon Hunter <jon-hunter@ti.com>
>>>>> ---
>>>>>  arch/arm/mach-omap2/gpmc.c |   65 ++++++++++++++++++++++++++++++++++++++++++++
>>>>>  arch/arm/mach-omap2/gpmc.h |    6 ++++
>>>>>  2 files changed, 71 insertions(+)
>>>>>
>>>>> diff --git a/arch/arm/mach-omap2/gpmc.c b/arch/arm/mach-omap2/gpmc.c
>>>>> index 465cac9..fb8dfd2 100644
>>>>> --- a/arch/arm/mach-omap2/gpmc.c
>>>>> +++ b/arch/arm/mach-omap2/gpmc.c
>>>>> @@ -1137,6 +1137,71 @@ int gpmc_calc_timings(struct gpmc_timings *gpmc_t,
>>>>>  	return 0;
>>>>>  }
>>>>>  
>>>>> +int gpmc_cs_program_settings(int cs, struct gpmc_settings *p)
>>>>> +{
>>>>> +	u32 config1;
>>>>> +
>>>>> +	if ((!p->device_width) || (p->device_width > GPMC_DEVWIDTH_16BIT)) {
>>>>> +		pr_err("%s: invalid width %d!", __func__, p->device_width);
>>>>> +		return -EINVAL;
>>>>> +	}
>>>>> +
>>>>> +	/* Address-data multiplexing not supported for NAND devices */
>>>>> +	if (p->device_nand && p->mux_add_data) {
>>>>> +		pr_err("%s: invalid configuration!\n", __func__);
>>>>> +		return -EINVAL;
>>>>> +	}
>>>>> +
>>>>> +	/* Page/burst mode supports lengths of 4, 8 and 16 bytes */
>>>>> +	if (p->burst_read || p->burst_write) {
>>>>> +		switch (p->burst_len) {
>>>>> +		case GPMC_BURST_4:
>>>>> +		case GPMC_BURST_8:
>>>>> +		case GPMC_BURST_16:
>>>>> +			break;
>>>>> +		default:
>>>>> +			pr_err("%s: invalid page/burst-length (%d)\n",
>>>>> +			       __func__, p->burst_len);
>>>>> +			return -EINVAL;
>>>>> +		}
>>>>> +	}
>>>>> +
>>>>> +	if ((p->wait_on_read || p->wait_on_write) &&
>>>>> +	    (p->wait_pin > gpmc_nr_waitpins)) {
>>>>> +		pr_err("%s: invalid wait-pin (%d)\n", __func__, p->wait_pin);
>>>>> +		return -EINVAL;
>>>>> +	}
>>>>> +
>>>>> +	config1 = GPMC_CONFIG1_DEVICESIZE((p->device_width - 1));
>>>>
>>>> Can you consider read_modify approach?
>>>
>>> I was purposely trying to avoid that. The intent here is to program all
>>> the settings and get away from any boot-loader dependencies. If we use a
>>> read-modify approach then it will never be clear in the kernel what
>>> should actually be programmed.
>>
>> By the way, it would be good to know what your motivation for this is.
>>
>> My goal is for all gpmc settings to eventually live in the DT blob and
>> there be no dependency on the bootloader whatsoever. That may mean that
>> settings are re-programmed again by the kernel but IMO that is best.
> 
> Yes I understood now and the right thing to do.
> Suggested read modify approach because of some confusion as GPMC_CONFIG1 is already
> modified in gpmc_cs_set_timings() and is called from omap2_nand_gpmc_retime().
> So the data modified in gpmc_cs_set_timings() will lost (not significant)
> May be better and cleaner solution is to remove GPMC_CONFIG1 modification in
> gpmc_cs_set_timings() also.

Actually that is a good point. There are some timings in CONFIG1 that
are not being configured in gpmc_cs_program_settings() but
gpmc_cs_set_timings(). I had left it this way intentionally to keep
timings together, which makes sense for where gpmc clock could change at
run time. However, this means that gpmc_cs_program_settings() needs to
be called before gpmc_cs_set_timings(). Really this is no different to
how the code was before, because the code was writing directly to
CONFIG1 and now we have a function.

So the intent is for gpmc_cs_program_settings() to only be called once
for a given device and then gpmc_cs_set_timings() be called whenever the
timings need to be updated. May be I can updated the kernel-doc for
gpmc_cs_program_settings() to reflect this. Let me know if you have any
other concerns with this.

Cheers
Jon

[PATCH 04/14] ARM: OMAP2+: Add function for configuring GPMC settings

[Philip, Avinash]

On Fri, Mar 01, 2013 at 21:13:37, Hunter, Jon wrote:
> 
> On 02/28/2013 11:33 PM, Philip, Avinash wrote:
> > On Thu, Feb 28, 2013 at 22:42:37, Hunter, Jon wrote:
> >>
> >> On 02/28/2013 09:52 AM, Jon Hunter wrote:
> >>>
> >>> On 02/28/2013 12:05 AM, Philip, Avinash wrote:
> >>>> On Tue, Feb 26, 2013 at 23:00:31, Hunter, Jon wrote:
> >>>>> The GPMC has various different configuration options such as bus-width,
> >>>>> synchronous or asychronous mode selection, burst mode options etc.
> >>>>> Currently, there is no common function for configuring these options and
> >>>>> various devices set these options by either programming the GPMC CONFIG1
> >>>>> register directly or by calling gpmc_cs_configure() to set some of the
> >>>>> options.
> >>>>>
> >>>>> Add a new function for configuring all of the GPMC options. Having a common
> >>>>> function for configuring this options will simplify code and ease the
> >>>>> migration to device-tree.
> >>>>>
> >>>>> Signed-off-by: Jon Hunter <jon-hunter@ti.com>
> >>>>> ---
> >>>>>  arch/arm/mach-omap2/gpmc.c |   65 ++++++++++++++++++++++++++++++++++++++++++++
> >>>>>  arch/arm/mach-omap2/gpmc.h |    6 ++++
> >>>>>  2 files changed, 71 insertions(+)
> >>>>>
> >>>>> diff --git a/arch/arm/mach-omap2/gpmc.c b/arch/arm/mach-omap2/gpmc.c
> >>>>> index 465cac9..fb8dfd2 100644
> >>>>> --- a/arch/arm/mach-omap2/gpmc.c
> >>>>> +++ b/arch/arm/mach-omap2/gpmc.c
> >>>>> @@ -1137,6 +1137,71 @@ int gpmc_calc_timings(struct gpmc_timings *gpmc_t,
> >>>>>  	return 0;
> >>>>>  }
> >>>>>  
> >>>>> +int gpmc_cs_program_settings(int cs, struct gpmc_settings *p)
> >>>>> +{
> >>>>> +	u32 config1;
> >>>>> +
> >>>>> +	if ((!p->device_width) || (p->device_width > GPMC_DEVWIDTH_16BIT)) {
> >>>>> +		pr_err("%s: invalid width %d!", __func__, p->device_width);
> >>>>> +		return -EINVAL;
> >>>>> +	}
> >>>>> +
> >>>>> +	/* Address-data multiplexing not supported for NAND devices */
> >>>>> +	if (p->device_nand && p->mux_add_data) {
> >>>>> +		pr_err("%s: invalid configuration!\n", __func__);
> >>>>> +		return -EINVAL;
> >>>>> +	}
> >>>>> +
> >>>>> +	/* Page/burst mode supports lengths of 4, 8 and 16 bytes */
> >>>>> +	if (p->burst_read || p->burst_write) {
> >>>>> +		switch (p->burst_len) {
> >>>>> +		case GPMC_BURST_4:
> >>>>> +		case GPMC_BURST_8:
> >>>>> +		case GPMC_BURST_16:
> >>>>> +			break;
> >>>>> +		default:
> >>>>> +			pr_err("%s: invalid page/burst-length (%d)\n",
> >>>>> +			       __func__, p->burst_len);
> >>>>> +			return -EINVAL;
> >>>>> +		}
> >>>>> +	}
> >>>>> +
> >>>>> +	if ((p->wait_on_read || p->wait_on_write) &&
> >>>>> +	    (p->wait_pin > gpmc_nr_waitpins)) {
> >>>>> +		pr_err("%s: invalid wait-pin (%d)\n", __func__, p->wait_pin);
> >>>>> +		return -EINVAL;
> >>>>> +	}
> >>>>> +
> >>>>> +	config1 = GPMC_CONFIG1_DEVICESIZE((p->device_width - 1));
> >>>>
> >>>> Can you consider read_modify approach?
> >>>
> >>> I was purposely trying to avoid that. The intent here is to program all
> >>> the settings and get away from any boot-loader dependencies. If we use a
> >>> read-modify approach then it will never be clear in the kernel what
> >>> should actually be programmed.
> >>
> >> By the way, it would be good to know what your motivation for this is.
> >>
> >> My goal is for all gpmc settings to eventually live in the DT blob and
> >> there be no dependency on the bootloader whatsoever. That may mean that
> >> settings are re-programmed again by the kernel but IMO that is best.
> > 
> > Yes I understood now and the right thing to do.
> > Suggested read modify approach because of some confusion as GPMC_CONFIG1 is already
> > modified in gpmc_cs_set_timings() and is called from omap2_nand_gpmc_retime().
> > So the data modified in gpmc_cs_set_timings() will lost (not significant)
> > May be better and cleaner solution is to remove GPMC_CONFIG1 modification in
> > gpmc_cs_set_timings() also.
> 
> Actually that is a good point. There are some timings in CONFIG1 that
> are not being configured in gpmc_cs_program_settings() but
> gpmc_cs_set_timings(). I had left it this way intentionally to keep
> timings together, which makes sense for where gpmc clock could change at
> run time. However, this means that gpmc_cs_program_settings() needs to
> be called before gpmc_cs_set_timings(). Really this is no different to
> how the code was before, because the code was writing directly to
> CONFIG1 and now we have a function.
> 
> So the intent is for gpmc_cs_program_settings() to only be called once
> for a given device and then gpmc_cs_set_timings() be called whenever the
> timings need to be updated. May be I can updated the kernel-doc for
> gpmc_cs_program_settings() to reflect this. Let me know if you have any
> other concerns with this.

Fine enough.

Thanks
Avinash
> 
> Cheers
> Jon
> 

[PATCH 05/14] ARM: OMAP2+: Convert ONENAND to use gpmc_cs_program_settings()

[Jon Hunter]

Convert the OMAP2+ ONENAND code to use the gpmc_cs_program_settings()
function for configuring the various GPMC options instead of directly
programming the CONFIG1 register.

Signed-off-by: Jon Hunter <jon-hunter@ti.com>
---
 arch/arm/mach-omap2/gpmc-onenand.c |   61 +++++++++++++++---------------------
 1 file changed, 25 insertions(+), 36 deletions(-)

diff --git a/arch/arm/mach-omap2/gpmc-onenand.c b/arch/arm/mach-omap2/gpmc-onenand.c
index 8b1c08e..1e2191c 100644
--- a/arch/arm/mach-omap2/gpmc-onenand.c
+++ b/arch/arm/mach-omap2/gpmc-onenand.c
@@ -49,17 +49,21 @@ static struct platform_device gpmc_onenand_device = {
 
 static struct gpmc_settings onenand_async = {
 	.mux_add_data	= true,
+	.device_width	= GPMC_DEVWIDTH_16BIT,
 };
 
 static struct gpmc_settings onenand_sync = {
 	.burst_read	= true,
+	.burst_wrap	= true,
 	.mux_add_data	= true,
+	.burst_len	= GPMC_BURST_16,
+	.device_width	= GPMC_DEVWIDTH_16BIT,
+	.wait_pin	= 0,
 };
 
 static void omap2_onenand_calc_async_timings(struct gpmc_timings *t)
 {
 	struct gpmc_device_timings dev_t;
-
 	const int t_cer = 15;
 	const int t_avdp = 12;
 	const int t_aavdh = 7;
@@ -86,16 +90,6 @@ static void omap2_onenand_calc_async_timings(struct gpmc_timings *t)
 	gpmc_calc_timings(t, &onenand_async, &dev_t);
 }
 
-static int gpmc_set_async_mode(int cs, struct gpmc_timings *t)
-{
-	/* Configure GPMC for asynchronous read */
-	gpmc_cs_write_reg(cs, GPMC_CS_CONFIG1,
-			  GPMC_CONFIG1_DEVICESIZE_16 |
-			  GPMC_CONFIG1_MUXADDDATA);
-
-	return gpmc_cs_set_timings(cs, t);
-}
-
 static void omap2_onenand_set_async_mode(void __iomem *onenand_base)
 {
 	u32 reg;
@@ -243,8 +237,11 @@ static void omap2_onenand_calc_sync_timings(struct gpmc_timings *t,
 	/* Set synchronous read timings */
 	memset(&dev_t, 0, sizeof(dev_t));
 
+	if (onenand_flags & ONENAND_FLAG_SYNCREAD)
+		onenand_sync.sync_read = true;
 	if (onenand_flags & ONENAND_FLAG_SYNCWRITE) {
 		onenand_sync.sync_write = true;
+		onenand_sync.burst_write = true;
 	} else {
 		dev_t.t_avdp_w = max(t_avdp, t_cer) * 1000;
 		dev_t.t_wpl = t_wpl * 1000;
@@ -270,29 +267,6 @@ static void omap2_onenand_calc_sync_timings(struct gpmc_timings *t,
 	gpmc_calc_timings(t, &onenand_sync, &dev_t);
 }
 
-static int gpmc_set_sync_mode(int cs, struct gpmc_timings *t)
-{
-	unsigned sync_read = onenand_flags & ONENAND_FLAG_SYNCREAD;
-	unsigned sync_write = onenand_flags & ONENAND_FLAG_SYNCWRITE;
-
-	/* Configure GPMC for synchronous read */
-	gpmc_cs_write_reg(cs, GPMC_CS_CONFIG1,
-			  GPMC_CONFIG1_WRAPBURST_SUPP |
-			  GPMC_CONFIG1_READMULTIPLE_SUPP |
-			  (sync_read ? GPMC_CONFIG1_READTYPE_SYNC : 0) |
-			  (sync_write ? GPMC_CONFIG1_WRITEMULTIPLE_SUPP : 0) |
-			  (sync_write ? GPMC_CONFIG1_WRITETYPE_SYNC : 0) |
-			  GPMC_CONFIG1_PAGE_LEN(2) |
-			  (cpu_is_omap34xx() ? 0 :
-				(GPMC_CONFIG1_WAIT_READ_MON |
-				 GPMC_CONFIG1_WAIT_PIN_SEL(0))) |
-			  GPMC_CONFIG1_DEVICESIZE_16 |
-			  GPMC_CONFIG1_DEVICETYPE_NOR |
-			  GPMC_CONFIG1_MUXADDDATA);
-
-	return gpmc_cs_set_timings(cs, t);
-}
-
 static int omap2_onenand_setup_async(void __iomem *onenand_base)
 {
 	struct gpmc_timings t;
@@ -302,7 +276,11 @@ static int omap2_onenand_setup_async(void __iomem *onenand_base)
 
 	omap2_onenand_calc_async_timings(&t);
 
-	ret = gpmc_set_async_mode(gpmc_onenand_data->cs, &t);
+	ret = gpmc_cs_program_settings(gpmc_onenand_data->cs, &onenand_async);
+	if (IS_ERR_VALUE(ret))
+		return ret;
+
+	ret = gpmc_cs_set_timings(gpmc_onenand_data->cs, &t);
 	if (IS_ERR_VALUE(ret))
 		return ret;
 
@@ -322,9 +300,20 @@ static int omap2_onenand_setup_sync(void __iomem *onenand_base, int *freq_ptr)
 		set_onenand_cfg(onenand_base);
 	}
 
+	/*
+	 * FIXME: Appears to be legacy code from initial ONENAND commit.
+	 * Unclear what boards this is for and if this can be removed.
+	 */
+	if (!cpu_is_omap34xx())
+		onenand_sync.wait_on_read = true;
+
 	omap2_onenand_calc_sync_timings(&t, gpmc_onenand_data->flags, freq);
 
-	ret = gpmc_set_sync_mode(gpmc_onenand_data->cs, &t);
+	ret = gpmc_cs_program_settings(gpmc_onenand_data->cs, &onenand_sync);
+	if (IS_ERR_VALUE(ret))
+		return ret;
+
+	ret = gpmc_cs_set_timings(gpmc_onenand_data->cs, &t);
 	if (IS_ERR_VALUE(ret))
 		return ret;
 
-- 
1.7.10.4

[PATCH 06/14] ARM: OMAP2+: Convert NAND to use gpmc_cs_program_settings()

[Jon Hunter]

Convert the OMAP2+ NAND code to use the gpmc_cs_program_settings()
function for configuring the various GPMC options instead of directly
programming the CONFIG1 register.

This moves the configuration of some GPMC options outside the
nand_gpmc_retime() because these options should only need to be set once
regardless of whether the gpmc timing is changing dynamically at runtime.
The programming of where the wait-pin is also moved slightly, but this
will not have any impact to existing devices as no boards are currently
setting the dev_ready variable.

Signed-off-by: Jon Hunter <jon-hunter@ti.com>
---
 arch/arm/mach-omap2/gpmc-nand.c |   35 +++++++++++++++++++++++------------
 1 file changed, 23 insertions(+), 12 deletions(-)

diff --git a/arch/arm/mach-omap2/gpmc-nand.c b/arch/arm/mach-omap2/gpmc-nand.c
index afc1e8c..4bdfea2 100644
--- a/arch/arm/mach-omap2/gpmc-nand.c
+++ b/arch/arm/mach-omap2/gpmc-nand.c
@@ -43,6 +43,10 @@ static struct platform_device gpmc_nand_device = {
 	.resource	= gpmc_nand_resource,
 };
 
+static struct gpmc_settings nand_settings = {
+	.device_nand = true,
+};
+
 static int omap2_nand_gpmc_retime(
 				struct omap_nand_platform_data *gpmc_nand_data,
 				struct gpmc_timings *gpmc_t)
@@ -74,14 +78,6 @@ static int omap2_nand_gpmc_retime(
 	t.cs_wr_off = gpmc_t->cs_wr_off;
 	t.wr_cycle = gpmc_t->wr_cycle;
 
-	/* Configure GPMC */
-	if (gpmc_nand_data->devsize == NAND_BUSWIDTH_16)
-		gpmc_cs_configure(gpmc_nand_data->cs, GPMC_CONFIG_DEV_SIZE, 1);
-	else
-		gpmc_cs_configure(gpmc_nand_data->cs, GPMC_CONFIG_DEV_SIZE, 0);
-	gpmc_cs_configure(gpmc_nand_data->cs,
-			GPMC_CONFIG_DEV_TYPE, GPMC_DEVICETYPE_NAND);
-	gpmc_cs_configure(gpmc_nand_data->cs, GPMC_CONFIG_WP, 0);
 	err = gpmc_cs_set_timings(gpmc_nand_data->cs, &t);
 	if (err)
 		return err;
@@ -140,11 +136,26 @@ int gpmc_nand_init(struct omap_nand_platform_data *gpmc_nand_data,
 			dev_err(dev, "Unable to set gpmc timings: %d\n", err);
 			return err;
 		}
-	}
 
-	/* Enable RD PIN Monitoring Reg */
-	if (gpmc_nand_data->dev_ready) {
-		gpmc_cs_configure(gpmc_nand_data->cs, GPMC_CONFIG_RDY_BSY, 1);
+		/* Enable RD PIN Monitoring Reg */
+		if (gpmc_nand_data->dev_ready) {
+			nand_settings.wait_on_read = true;
+			nand_settings.wait_on_write = true;
+		}
+
+		if (gpmc_nand_data->devsize == NAND_BUSWIDTH_16)
+			nand_settings.device_width = GPMC_DEVWIDTH_16BIT;
+		else
+			nand_settings.device_width = GPMC_DEVWIDTH_8BIT;
+
+		err = gpmc_cs_program_settings(gpmc_nand_data->cs,
+					       &nand_settings);
+		if (IS_ERR_VALUE(err))
+			goto out_free_cs;
+
+		err = gpmc_cs_configure(gpmc_nand_data->cs, GPMC_CONFIG_WP, 0);
+		if (IS_ERR_VALUE(err))
+			goto out_free_cs;
 	}
 
 	gpmc_update_nand_reg(&gpmc_nand_data->reg, gpmc_nand_data->cs);
-- 
1.7.10.4

[PATCH 06/14] ARM: OMAP2+: Convert NAND to use gpmc_cs_program_settings()

[Philip, Avinash]

On Tue, Feb 26, 2013 at 23:00:33, Hunter, Jon wrote:
> Convert the OMAP2+ NAND code to use the gpmc_cs_program_settings()
> function for configuring the various GPMC options instead of directly
> programming the CONFIG1 register.
> 
> This moves the configuration of some GPMC options outside the
> nand_gpmc_retime() because these options should only need to be set once
> regardless of whether the gpmc timing is changing dynamically at runtime.
> The programming of where the wait-pin is also moved slightly, but this
> will not have any impact to existing devices as no boards are currently
> setting the dev_ready variable.
> 
> Signed-off-by: Jon Hunter <jon-hunter@ti.com>
> ---
>  arch/arm/mach-omap2/gpmc-nand.c |   35 +++++++++++++++++++++++------------
>  1 file changed, 23 insertions(+), 12 deletions(-)
> 
> diff --git a/arch/arm/mach-omap2/gpmc-nand.c b/arch/arm/mach-omap2/gpmc-nand.c
> index afc1e8c..4bdfea2 100644
> --- a/arch/arm/mach-omap2/gpmc-nand.c
> +++ b/arch/arm/mach-omap2/gpmc-nand.c
> @@ -43,6 +43,10 @@ static struct platform_device gpmc_nand_device = {
>  	.resource	= gpmc_nand_resource,
>  };
>  
> +static struct gpmc_settings nand_settings = {
> +	.device_nand = true,
> +};
> +

Is it possible to make it local variable?
It would help GPMC to support NAND device on multiple chip select.

Thanks
Avinash

...
> @@ -140,11 +136,26 @@ int gpmc_nand_init(struct omap_nand_platform_data *gpmc_nand_data,
>  			dev_err(dev, "Unable to set gpmc timings: %d\n", err);
>  			return err;
>  		}
> -	}
>  
> -	/* Enable RD PIN Monitoring Reg */
> -	if (gpmc_nand_data->dev_ready) {
> -		gpmc_cs_configure(gpmc_nand_data->cs, GPMC_CONFIG_RDY_BSY, 1);
> +		/* Enable RD PIN Monitoring Reg */
> +		if (gpmc_nand_data->dev_ready) {
> +			nand_settings.wait_on_read = true;
> +			nand_settings.wait_on_write = true;
> +		}
> +
> +		if (gpmc_nand_data->devsize == NAND_BUSWIDTH_16)
> +			nand_settings.device_width = GPMC_DEVWIDTH_16BIT;
> +		else
> +			nand_settings.device_width = GPMC_DEVWIDTH_8BIT;
> +
> +		err = gpmc_cs_program_settings(gpmc_nand_data->cs,
> +					       &nand_settings);
> +		if (IS_ERR_VALUE(err))
> +			goto out_free_cs;
> +
> +		err = gpmc_cs_configure(gpmc_nand_data->cs, GPMC_CONFIG_WP, 0);
> +		if (IS_ERR_VALUE(err))
> +			goto out_free_cs;
>  	}
>  
>  	gpmc_update_nand_reg(&gpmc_nand_data->reg, gpmc_nand_data->cs);
> -- 
> 1.7.10.4
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

[PATCH 06/14] ARM: OMAP2+: Convert NAND to use gpmc_cs_program_settings()

[Jon Hunter]

On 02/28/2013 04:38 AM, Philip, Avinash wrote:
> On Tue, Feb 26, 2013 at 23:00:33, Hunter, Jon wrote:
>> Convert the OMAP2+ NAND code to use the gpmc_cs_program_settings()
>> function for configuring the various GPMC options instead of directly
>> programming the CONFIG1 register.
>>
>> This moves the configuration of some GPMC options outside the
>> nand_gpmc_retime() because these options should only need to be set once
>> regardless of whether the gpmc timing is changing dynamically at runtime.
>> The programming of where the wait-pin is also moved slightly, but this
>> will not have any impact to existing devices as no boards are currently
>> setting the dev_ready variable.
>>
>> Signed-off-by: Jon Hunter <jon-hunter@ti.com>
>> ---
>>  arch/arm/mach-omap2/gpmc-nand.c |   35 +++++++++++++++++++++++------------
>>  1 file changed, 23 insertions(+), 12 deletions(-)
>>
>> diff --git a/arch/arm/mach-omap2/gpmc-nand.c b/arch/arm/mach-omap2/gpmc-nand.c
>> index afc1e8c..4bdfea2 100644
>> --- a/arch/arm/mach-omap2/gpmc-nand.c
>> +++ b/arch/arm/mach-omap2/gpmc-nand.c
>> @@ -43,6 +43,10 @@ static struct platform_device gpmc_nand_device = {
>>  	.resource	= gpmc_nand_resource,
>>  };
>>  
>> +static struct gpmc_settings nand_settings = {
>> +	.device_nand = true,
>> +};
>> +
> 
> Is it possible to make it local variable?
> It would help GPMC to support NAND device on multiple chip select.

Well gpmc_nand_init() will be called for each NAND device and so I don't
see why this would prevent supporting multiple NANDs on multiple
chip-selects.

Once migration to device-tree is complete we could definitely make it
local as there will be no need for any static initialisations of the
structure as all fields would be read from device-tree.

I can make it local now if that is preferred and seeing that will be the
direction once we have migrated to device-tree, is does make sense.

Cheers
Jon

[PATCH 06/14] ARM: OMAP2+: Convert NAND to use gpmc_cs_program_settings()

[Philip, Avinash]

On Thu, Feb 28, 2013 at 21:32:01, Hunter, Jon wrote:
> 
> On 02/28/2013 04:38 AM, Philip, Avinash wrote:
> > On Tue, Feb 26, 2013 at 23:00:33, Hunter, Jon wrote:
> >> Convert the OMAP2+ NAND code to use the gpmc_cs_program_settings()
> >> function for configuring the various GPMC options instead of directly
> >> programming the CONFIG1 register.
> >>
> >> This moves the configuration of some GPMC options outside the
> >> nand_gpmc_retime() because these options should only need to be set once
> >> regardless of whether the gpmc timing is changing dynamically at runtime.
> >> The programming of where the wait-pin is also moved slightly, but this
> >> will not have any impact to existing devices as no boards are currently
> >> setting the dev_ready variable.
> >>
> >> Signed-off-by: Jon Hunter <jon-hunter@ti.com>
> >> ---
> >>  arch/arm/mach-omap2/gpmc-nand.c |   35 +++++++++++++++++++++++------------
> >>  1 file changed, 23 insertions(+), 12 deletions(-)
> >>
> >> diff --git a/arch/arm/mach-omap2/gpmc-nand.c b/arch/arm/mach-omap2/gpmc-nand.c
> >> index afc1e8c..4bdfea2 100644
> >> --- a/arch/arm/mach-omap2/gpmc-nand.c
> >> +++ b/arch/arm/mach-omap2/gpmc-nand.c
> >> @@ -43,6 +43,10 @@ static struct platform_device gpmc_nand_device = {
> >>  	.resource	= gpmc_nand_resource,
> >>  };
> >>  
> >> +static struct gpmc_settings nand_settings = {
> >> +	.device_nand = true,
> >> +};
> >> +
> > 
> > Is it possible to make it local variable?
> > It would help GPMC to support NAND device on multiple chip select.
> 
> Well gpmc_nand_init() will be called for each NAND device and so I don't
> see why this would prevent supporting multiple NANDs on multiple
> chip-selects.

Problem exactly lies here. Here platform device initialized as statically.
So multiple requests will points to same platform device.

Recently this issue raised in TI e2e forum and solution being proposed.
Details can found from
http://e2e.ti.com/support/arm/sitara_arm/f/791/p/246997/865379.aspx#865379

I doubt the same is the case for other GPMC client devices also.

> 
> Once migration to device-tree is complete we could definitely make it
> local as there will be no need for any static initialisations of the
> structure as all fields would be read from device-tree.
> 
> I can make it local now if that is preferred and seeing that will be the
> direction once we have migrated to device-tree, is does make sense.

As far as I understood, here usage of local variable won't break anything?
If that is the case, better solution would be usage of local variable.

Thanks
Avinash
> 
> Cheers
> Jon
> 

[PATCH 06/14] ARM: OMAP2+: Convert NAND to use gpmc_cs_program_settings()

[Jon Hunter]

On 02/28/2013 11:40 PM, Philip, Avinash wrote:
> On Thu, Feb 28, 2013 at 21:32:01, Hunter, Jon wrote:
>>
>> On 02/28/2013 04:38 AM, Philip, Avinash wrote:
>>> On Tue, Feb 26, 2013 at 23:00:33, Hunter, Jon wrote:
>>>> Convert the OMAP2+ NAND code to use the gpmc_cs_program_settings()
>>>> function for configuring the various GPMC options instead of directly
>>>> programming the CONFIG1 register.
>>>>
>>>> This moves the configuration of some GPMC options outside the
>>>> nand_gpmc_retime() because these options should only need to be set once
>>>> regardless of whether the gpmc timing is changing dynamically at runtime.
>>>> The programming of where the wait-pin is also moved slightly, but this
>>>> will not have any impact to existing devices as no boards are currently
>>>> setting the dev_ready variable.
>>>>
>>>> Signed-off-by: Jon Hunter <jon-hunter@ti.com>
>>>> ---
>>>>  arch/arm/mach-omap2/gpmc-nand.c |   35 +++++++++++++++++++++++------------
>>>>  1 file changed, 23 insertions(+), 12 deletions(-)
>>>>
>>>> diff --git a/arch/arm/mach-omap2/gpmc-nand.c b/arch/arm/mach-omap2/gpmc-nand.c
>>>> index afc1e8c..4bdfea2 100644
>>>> --- a/arch/arm/mach-omap2/gpmc-nand.c
>>>> +++ b/arch/arm/mach-omap2/gpmc-nand.c
>>>> @@ -43,6 +43,10 @@ static struct platform_device gpmc_nand_device = {
>>>>  	.resource	= gpmc_nand_resource,
>>>>  };
>>>>  
>>>> +static struct gpmc_settings nand_settings = {
>>>> +	.device_nand = true,
>>>> +};
>>>> +
>>>
>>> Is it possible to make it local variable?
>>> It would help GPMC to support NAND device on multiple chip select.
>>
>> Well gpmc_nand_init() will be called for each NAND device and so I don't
>> see why this would prevent supporting multiple NANDs on multiple
>> chip-selects.
> 
> Problem exactly lies here. Here platform device initialized as statically.
> So multiple requests will points to same platform device.
>
> Recently this issue raised in TI e2e forum and solution being proposed.
> Details can found from
> http://e2e.ti.com/support/arm/sitara_arm/f/791/p/246997/865379.aspx#865379
> 
> I doubt the same is the case for other GPMC client devices also.

This structure is only passed to gpmc_cs_program_settings() and not
stored in the platform device structure. So I don't think that this is
the same problem. Look at what the code is doing with the nand_settings
structure.

>> Once migration to device-tree is complete we could definitely make it
>> local as there will be no need for any static initialisations of the
>> structure as all fields would be read from device-tree.
>>
>> I can make it local now if that is preferred and seeing that will be the
>> direction once we have migrated to device-tree, is does make sense.
> 
> As far as I understood, here usage of local variable won't break anything?
> If that is the case, better solution would be usage of local variable.

It will not. So yes we can.

Jon

[PATCH 07/14] ARM: OMAP2+: Convert SMC91x to use gpmc_cs_program_settings()

[Jon Hunter]

Convert the OMAP2+ SMC91x code to use the gpmc_cs_program_settings()
function for configuring the various GPMC options instead of directly
programming the CONFIG1 register.

Move configuration of the GPMC settings outside retime function and
this does not need to be done if the timings are changed dynamically
at runtime.

Signed-off-by: Jon Hunter <jon-hunter@ti.com>
---
 arch/arm/mach-omap2/gpmc-smc91x.c |   30 +++++++++++++++++-------------
 1 file changed, 17 insertions(+), 13 deletions(-)

diff --git a/arch/arm/mach-omap2/gpmc-smc91x.c b/arch/arm/mach-omap2/gpmc-smc91x.c
index 4b78338..0871662 100644
--- a/arch/arm/mach-omap2/gpmc-smc91x.c
+++ b/arch/arm/mach-omap2/gpmc-smc91x.c
@@ -49,6 +49,10 @@ static struct platform_device gpmc_smc91x_device = {
 	.resource	= gpmc_smc91x_resources,
 };
 
+static struct gpmc_settings smc91x_settings = {
+	.device_width = GPMC_DEVWIDTH_16BIT,
+};
+
 /*
  * Set the gpmc timings for smc91c96. The timings are taken
  * from the data sheet available at:
@@ -67,18 +71,6 @@ static int smc91c96_gpmc_retime(void)
 	const int t7 = 5;	/* Figure 12.4 write */
 	const int t8 = 5;	/* Figure 12.4 write */
 	const int t20 = 185;	/* Figure 12.2 read and 12.4 write */
-	u32 l;
-
-	l = GPMC_CONFIG1_DEVICESIZE_16;
-	if (gpmc_cfg->flags & GPMC_MUX_ADD_DATA)
-		l |= GPMC_CONFIG1_MUXADDDATA;
-	if (gpmc_cfg->flags & GPMC_READ_MON)
-		l |= GPMC_CONFIG1_WAIT_READ_MON;
-	if (gpmc_cfg->flags & GPMC_WRITE_MON)
-		l |= GPMC_CONFIG1_WAIT_WRITE_MON;
-	if (gpmc_cfg->wait_pin)
-		l |= GPMC_CONFIG1_WAIT_PIN_SEL(gpmc_cfg->wait_pin);
-	gpmc_cs_write_reg(gpmc_cfg->cs, GPMC_CS_CONFIG1, l);
 
 	/*
 	 * FIXME: Calculate the address and data bus muxed timings.
@@ -104,7 +96,7 @@ static int smc91c96_gpmc_retime(void)
 	dev_t.t_cez_w = t4_w * 1000;
 	dev_t.t_wr_cycle = (t20 - t3) * 1000;
 
-	gpmc_calc_timings(&t, NULL, &dev_t);
+	gpmc_calc_timings(&t, &smc91x_settings, &dev_t);
 
 	return gpmc_cs_set_timings(gpmc_cfg->cs, &t);
 }
@@ -133,6 +125,18 @@ void __init gpmc_smc91x_init(struct omap_smc91x_platform_data *board_data)
 	gpmc_smc91x_resources[0].end = cs_mem_base + 0x30f;
 	gpmc_smc91x_resources[1].flags |= (gpmc_cfg->flags & IRQF_TRIGGER_MASK);
 
+	if (gpmc_cfg->flags & GPMC_MUX_ADD_DATA)
+		smc91x_settings.mux_add_data = true;
+	if (gpmc_cfg->flags & GPMC_READ_MON)
+		smc91x_settings.wait_on_read = true;
+	if (gpmc_cfg->flags & GPMC_WRITE_MON)
+		smc91x_settings.wait_on_write = true;
+	if (gpmc_cfg->wait_pin)
+		smc91x_settings.wait_pin = gpmc_cfg->wait_pin;
+	ret = gpmc_cs_program_settings(gpmc_cfg->cs, &smc91x_settings);
+	if (IS_ERR_VALUE(ret))
+		goto free1;
+
 	if (gpmc_cfg->retime) {
 		ret = gpmc_cfg->retime();
 		if (ret != 0)
-- 
1.7.10.4

[PATCH 08/14] ARM: OMAP2+: Convert TUSB to use gpmc_cs_program_settings()

[Jon Hunter]

Convert the OMAP2+ TUSB code to use the gpmc_cs_program_settings()
function for configuring the various GPMC options instead of directly
programming the CONFIG1 register.

Signed-off-by: Jon Hunter <jon-hunter@ti.com>
---
 arch/arm/mach-omap2/usb-tusb6010.c |   43 ++++++++++++++++--------------------
 1 file changed, 19 insertions(+), 24 deletions(-)

diff --git a/arch/arm/mach-omap2/usb-tusb6010.c b/arch/arm/mach-omap2/usb-tusb6010.c
index 940410a..f38efd0 100644
--- a/arch/arm/mach-omap2/usb-tusb6010.c
+++ b/arch/arm/mach-omap2/usb-tusb6010.c
@@ -8,6 +8,7 @@
  * published by the Free Software Foundation.
  */
 
+#include <linux/err.h>
 #include <linux/string.h>
 #include <linux/types.h>
 #include <linux/errno.h>
@@ -28,12 +29,21 @@ static unsigned		refclk_psec;
 
 static struct gpmc_settings tusb_async = {
 	.mux_add_data	= true,
+	.wait_on_read	= true,
+	.wait_on_write	= true,
+	.device_width	= GPMC_DEVWIDTH_16BIT,
 };
 
 static struct gpmc_settings tusb_sync = {
+	.burst_read	= true,
+	.burst_write	= true,
 	.mux_add_data	= true,
 	.sync_read	= true,
 	.sync_write	= true,
+	.wait_on_read	= true,
+	.wait_on_write	= true,
+	.burst_len	= GPMC_BURST_16,
+	.device_width	= GPMC_DEVWIDTH_16BIT,
 };
 
 /* NOTE:  timings are from tusb 6010 datasheet Rev 1.8, 12-Sept 2006 */
@@ -168,18 +178,12 @@ tusb6010_setup_interface(struct musb_hdrc_platform_data *data,
 		return status;
 	}
 	tusb_resources[0].end = tusb_resources[0].start + 0x9ff;
+	tusb_async.wait_pin = waitpin;
 	async_cs = async;
-	gpmc_cs_write_reg(async, GPMC_CS_CONFIG1,
-			  GPMC_CONFIG1_PAGE_LEN(2)
-			| GPMC_CONFIG1_WAIT_READ_MON
-			| GPMC_CONFIG1_WAIT_WRITE_MON
-			| GPMC_CONFIG1_WAIT_PIN_SEL(waitpin)
-			| GPMC_CONFIG1_READTYPE_ASYNC
-			| GPMC_CONFIG1_WRITETYPE_ASYNC
-			| GPMC_CONFIG1_DEVICESIZE_16
-			| GPMC_CONFIG1_DEVICETYPE_NOR
-			| GPMC_CONFIG1_MUXADDDATA);
 
+	status = gpmc_cs_program_settings(async_cs, &tusb_async);
+	if (IS_ERR_VALUE(status))
+		return status;
 
 	/* SYNC region, primarily for DMA */
 	status = gpmc_cs_request(sync, SZ_16M, (unsigned long *)
@@ -189,21 +193,12 @@ tusb6010_setup_interface(struct musb_hdrc_platform_data *data,
 		return status;
 	}
 	tusb_resources[1].end = tusb_resources[1].start + 0x9ff;
+	tusb_sync.wait_pin = waitpin;
 	sync_cs = sync;
-	gpmc_cs_write_reg(sync, GPMC_CS_CONFIG1,
-			  GPMC_CONFIG1_READMULTIPLE_SUPP
-			| GPMC_CONFIG1_READTYPE_SYNC
-			| GPMC_CONFIG1_WRITEMULTIPLE_SUPP
-			| GPMC_CONFIG1_WRITETYPE_SYNC
-			| GPMC_CONFIG1_PAGE_LEN(2)
-			| GPMC_CONFIG1_WAIT_READ_MON
-			| GPMC_CONFIG1_WAIT_WRITE_MON
-			| GPMC_CONFIG1_WAIT_PIN_SEL(waitpin)
-			| GPMC_CONFIG1_DEVICESIZE_16
-			| GPMC_CONFIG1_DEVICETYPE_NOR
-			| GPMC_CONFIG1_MUXADDDATA
-			/* fclk divider gets set later */
-			);
+
+	status = gpmc_cs_program_settings(sync_cs, &tusb_sync);
+	if (IS_ERR_VALUE(status))
+		return status;
 
 	/* IRQ */
 	status = gpio_request_one(irq, GPIOF_IN, "TUSB6010 irq");
-- 
1.7.10.4

[PATCH 09/14] ARM: OMAP2+: Don't configure of chip-select options in gpmc_cs_configure()

[Jon Hunter]

With the addition of the gpmc_cs_program_settings(), we no longer need
or use gpmc_cs_configure() to configure some of the GPMC chip-select
options. So rename the function to gpmc_configure() and remove code that
modifies options in the CONFIG1 register.

Signed-off-by: Jon Hunter <jon-hunter@ti.com>
---
 arch/arm/mach-omap2/gpmc-nand.c |    2 +-
 arch/arm/mach-omap2/gpmc.c      |   45 ++++++---------------------------------
 arch/arm/mach-omap2/gpmc.h      |    2 +-
 3 files changed, 9 insertions(+), 40 deletions(-)

diff --git a/arch/arm/mach-omap2/gpmc-nand.c b/arch/arm/mach-omap2/gpmc-nand.c
index 4bdfea2..a4c8537 100644
--- a/arch/arm/mach-omap2/gpmc-nand.c
+++ b/arch/arm/mach-omap2/gpmc-nand.c
@@ -153,7 +153,7 @@ int gpmc_nand_init(struct omap_nand_platform_data *gpmc_nand_data,
 		if (IS_ERR_VALUE(err))
 			goto out_free_cs;
 
-		err = gpmc_cs_configure(gpmc_nand_data->cs, GPMC_CONFIG_WP, 0);
+		err = gpmc_configure(GPMC_CONFIG_WP, 0);
 		if (IS_ERR_VALUE(err))
 			goto out_free_cs;
 	}
diff --git a/arch/arm/mach-omap2/gpmc.c b/arch/arm/mach-omap2/gpmc.c
index fb8dfd2..7cf3d98 100644
--- a/arch/arm/mach-omap2/gpmc.c
+++ b/arch/arm/mach-omap2/gpmc.c
@@ -564,16 +564,14 @@ void gpmc_cs_free(int cs)
 EXPORT_SYMBOL(gpmc_cs_free);
 
 /**
- * gpmc_cs_configure - write request to configure gpmc
- * @cs: chip select number
+ * gpmc_configure - write request to configure gpmc
  * @cmd: command type
  * @wval: value to write
  * @return status of the operation
  */
-int gpmc_cs_configure(int cs, int cmd, int wval)
+int gpmc_configure(int cmd, int wval)
 {
-	int err = 0;
-	u32 regval = 0;
+	u32 regval;
 
 	switch (cmd) {
 	case GPMC_ENABLE_IRQ:
@@ -593,43 +591,14 @@ int gpmc_cs_configure(int cs, int cmd, int wval)
 		gpmc_write_reg(GPMC_CONFIG, regval);
 		break;
 
-	case GPMC_CONFIG_RDY_BSY:
-		regval  = gpmc_cs_read_reg(cs, GPMC_CS_CONFIG1);
-		if (wval)
-			regval |= WR_RD_PIN_MONITORING;
-		else
-			regval &= ~WR_RD_PIN_MONITORING;
-		gpmc_cs_write_reg(cs, GPMC_CS_CONFIG1, regval);
-		break;
-
-	case GPMC_CONFIG_DEV_SIZE:
-		regval  = gpmc_cs_read_reg(cs, GPMC_CS_CONFIG1);
-
-		/* clear 2 target bits */
-		regval &= ~GPMC_CONFIG1_DEVICESIZE(3);
-
-		/* set the proper value */
-		regval |= GPMC_CONFIG1_DEVICESIZE(wval);
-
-		gpmc_cs_write_reg(cs, GPMC_CS_CONFIG1, regval);
-		break;
-
-	case GPMC_CONFIG_DEV_TYPE:
-		regval  = gpmc_cs_read_reg(cs, GPMC_CS_CONFIG1);
-		regval |= GPMC_CONFIG1_DEVICETYPE(wval);
-		if (wval == GPMC_DEVICETYPE_NOR)
-			regval |= GPMC_CONFIG1_MUXADDDATA;
-		gpmc_cs_write_reg(cs, GPMC_CS_CONFIG1, regval);
-		break;
-
 	default:
-		printk(KERN_ERR "gpmc_configure_cs: Not supported\n");
-		err = -EINVAL;
+		pr_err("%s: command not supported\n", __func__);
+		return -EINVAL;
 	}
 
-	return err;
+	return 0;
 }
-EXPORT_SYMBOL(gpmc_cs_configure);
+EXPORT_SYMBOL(gpmc_configure);
 
 void gpmc_update_nand_reg(struct gpmc_nand_regs *reg, int cs)
 {
diff --git a/arch/arm/mach-omap2/gpmc.h b/arch/arm/mach-omap2/gpmc.h
index 638f94a..d73fda0 100644
--- a/arch/arm/mach-omap2/gpmc.h
+++ b/arch/arm/mach-omap2/gpmc.h
@@ -229,6 +229,6 @@ extern int gpmc_cs_set_reserved(int cs, int reserved);
 extern int gpmc_cs_reserved(int cs);
 extern void omap3_gpmc_save_context(void);
 extern void omap3_gpmc_restore_context(void);
-extern int gpmc_cs_configure(int cs, int cmd, int wval);
+extern int gpmc_configure(int cmd, int wval);
 
 #endif
-- 
1.7.10.4

[PATCH 10/14] ARM: OMAP2+: Add function to read GPMC settings from device-tree

[Jon Hunter]

Adds a function to read the various GPMC chip-select settings from
device-tree and store them in the gpmc_settings structure.

Update the GPMC device-tree binding documentation to describe these
options.

Signed-off-by: Jon Hunter <jon-hunter@ti.com>
---
 Documentation/devicetree/bindings/bus/ti-gpmc.txt |   19 ++++++++
 arch/arm/mach-omap2/gpmc.c                        |   50 +++++++++++++++++++++
 arch/arm/mach-omap2/gpmc.h                        |    2 +
 3 files changed, 71 insertions(+)

diff --git a/Documentation/devicetree/bindings/bus/ti-gpmc.txt b/Documentation/devicetree/bindings/bus/ti-gpmc.txt
index 5ddb2e9..a31c32a 100644
--- a/Documentation/devicetree/bindings/bus/ti-gpmc.txt
+++ b/Documentation/devicetree/bindings/bus/ti-gpmc.txt
@@ -65,6 +65,25 @@ The following are only applicable to OMAP3+ and AM335x:
  - gpmc,wr-access
  - gpmc,wr-data-mux-bus
 
+GPMC chip-select settings properties for child nodes. All are optional.
+
+- gpmc,burst-length	Page/burst length. Must be 4, 8 or 16.
+- gpmc,burst-wrap	Enables wrap bursting
+- gpmc,burst-read	Enables read page/burst mode
+- gpmc,burst-write	Enables write page/burst mode
+- gpmc,device-nand	Device is NAND
+- gpmc,device-width	Width of device connected to the GPMC in bytes. The
+			GPMC supports 8-bit and 16-bit devices and so this
+			property must be 1 or 2.
+- gpmc,mux-add-data	Enables address and data multiplexing
+- gpmc,sync-read	Enables synchronous read. Defaults to asynchronous
+			is this is not set.
+- gpmc,sync-write	Enables synchronous writes. Defaults to asynchronous
+			is this is not set.
+- gpmc,wait-pin		Wait-pin used by client. Must be less than
+			"gpmc,num-waitpins".
+- gpmc,wait-on-read	Enables wait monitoring on reads.
+- gpmc,wait-on-write	Enables wait monitoring on writes.
 
 Example for an AM33xx board:
 
diff --git a/arch/arm/mach-omap2/gpmc.c b/arch/arm/mach-omap2/gpmc.c
index 7cf3d98..e822d2b 100644
--- a/arch/arm/mach-omap2/gpmc.c
+++ b/arch/arm/mach-omap2/gpmc.c
@@ -1182,6 +1182,56 @@ static struct of_device_id gpmc_dt_ids[] = {
 };
 MODULE_DEVICE_TABLE(of, gpmc_dt_ids);
 
+/**
+ * gpmc_read_settings_dt - read gpmc settings from device-tree
+ * @np:		pointer to device-tree node for a gpmc child device
+ * @p:		pointer to gpmc settings structure
+ *
+ * Reads the GPMC settings for a GPMC child device from device-tree and
+ * stores them in the GPMC settings structure passed. The GPMC settings
+ * structure is initialise to zero by this function and so any previously
+ * stored settings will be clearer.
+ */
+void gpmc_read_settings_dt(struct device_node *np, struct gpmc_settings *p)
+{
+	memset(p, 0, sizeof(struct gpmc_settings));
+
+	if (of_find_property(np, "gpmc,sync-read", NULL))
+		p->sync_read = true;
+	if (of_find_property(np, "gpmc,sync-write", NULL))
+		p->sync_write = true;
+
+	if (!of_property_read_u32(np, "gpmc,burst-length", &p->burst_len)) {
+		if (of_find_property(np, "gpmc,burst-wrap", NULL))
+			p->burst_wrap = true;
+		if (of_find_property(np, "gpmc,burst-read", NULL))
+			p->burst_read = true;
+		if (of_find_property(np, "gpmc,burst-write", NULL))
+			p->burst_write = true;
+		if (!p->burst_read && !p->burst_write)
+			pr_warn("%s: page/burst-length set but not used!\n",
+				__func__);
+	}
+
+	if (!of_property_read_u32(np, "gpmc,wait-pin", &p->wait_pin)) {
+		if (of_find_property(np, "gpmc,wait-on-read", NULL))
+			p->wait_on_read = true;
+		if (of_find_property(np, "gpmc,wait-on-write", NULL))
+			p->wait_on_write = true;
+		if (!p->wait_on_read && !p->wait_on_write)
+			pr_warn("%s: read/write wait monitoring not enabled!\n",
+				__func__);
+	}
+
+	of_property_read_u32(np, "gpmc,device-width", &p->device_width);
+
+	if (of_find_property(np, "gpmc,device-nand", NULL))
+		p->device_nand = true;
+
+	if (of_find_property(np, "gpmc,mux-add-data", NULL))
+		p->mux_add_data = true;
+}
+
 static void __maybe_unused gpmc_read_timings_dt(struct device_node *np,
 						struct gpmc_timings *gpmc_t)
 {
diff --git a/arch/arm/mach-omap2/gpmc.h b/arch/arm/mach-omap2/gpmc.h
index d73fda0..c517eaa 100644
--- a/arch/arm/mach-omap2/gpmc.h
+++ b/arch/arm/mach-omap2/gpmc.h
@@ -230,5 +230,7 @@ extern int gpmc_cs_reserved(int cs);
 extern void omap3_gpmc_save_context(void);
 extern void omap3_gpmc_restore_context(void);
 extern int gpmc_configure(int cmd, int wval);
+extern void gpmc_read_settings_dt(struct device_node *np,
+				  struct gpmc_settings *p);
 
 #endif
-- 
1.7.10.4

[PATCH 11/14] ARM: OMAP2+: Add device-tree support for NOR flash

[Jon Hunter]

NOR flash is not currently supported when booting with device-tree
on OMAP2+ devices. Add support to detect and configure NOR devices
when booting with device-tree.

Add documentation for the TI GPMC NOR binding.

Signed-off-by: Jon Hunter <jon-hunter@ti.com>
---
 Documentation/devicetree/bindings/mtd/gpmc-nor.txt |   98 ++++++++++++++++
 arch/arm/mach-omap2/gpmc.c                         |  120 ++++++++++++++++++++
 2 files changed, 218 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mtd/gpmc-nor.txt

diff --git a/Documentation/devicetree/bindings/mtd/gpmc-nor.txt b/Documentation/devicetree/bindings/mtd/gpmc-nor.txt
new file mode 100644
index 0000000..8c638fc
--- /dev/null
+++ b/Documentation/devicetree/bindings/mtd/gpmc-nor.txt
@@ -0,0 +1,98 @@
+Device tree bindings for NOR flash connect to TI GPMC
+
+NOR flash connected to the TI GPMC (found on OMAP boards) are represented as
+child nodes of the GPMC controller with a name of "nor".
+
+All timing relevant properties as well as generic GPMC child properties are
+explained in a separate documents. Please refer to
+Documentation/devicetree/bindings/bus/ti-gpmc.txt
+
+Required properties:
+- bank-width: 		Width of NOR flash in bytes. GPMC supports 8-bit and
+			16-bit devices and so must be either 1 or 2 bytes.
+- compatible:		Documentation/devicetree/bindings/mtd/mtd-physmap.txt
+- gpmc,cs-on:		Chip-select assertion time
+- gpmc,cs-rd-off:	Chip-select de-assertion time for reads
+- gpmc,cs-wr-off:	Chip-select de-assertion time for writes
+- gpmc,oe-on:		Output-enable assertion time
+- gpmc,oe-off		Output-enable de-assertion time
+- gpmc,we-on:		Write-enable assertion time
+- gpmc,we-off:		Write-enable de-assertion time
+- gpmc,access:		Start cycle to first data capture (read access)
+- gpmc,rd-cycle:	Total read cycle time
+- gpmc,wr-cycle:	Total write cycle time
+- linux,mtd-name:	Documentation/devicetree/bindings/mtd/mtd-physmap.txt
+- reg:			Chip-select, base address (relative to chip-select)
+			and size of NOR flash. Note that base address will be
+			typically 0 as this is the start of the chip-select.
+
+Optional properties:
+- gpmc,XXX		Additional GPMC timings and settings parameters. See
+			Documentation/devicetree/bindings/bus/ti-gpmc.txt
+
+Optional properties for partiton table parsing:
+- #address-cells: should be set to 1
+- #size-cells: should be set to 1
+
+Example:
+
+gpmc: gpmc at 6e000000 {
+	compatible = "ti,omap3430-gpmc", "simple-bus";
+	ti,hwmods = "gpmc";
+	reg = <0x6e000000 0x1000>;
+	interrupts = <20>;
+	gpmc,num-cs = <8>;
+	gpmc,num-waitpins = <4>;
+	#address-cells = <2>;
+	#size-cells = <1>;
+
+	ranges = <0 0 0x10000000 0x08000000>;
+
+	nor at 0,0 {
+		compatible = "cfi-flash";
+		linux,mtd-name= "intel,pf48f6000m0y1be";
+		#address-cells = <1>;
+		#size-cells = <1>;
+		reg = <0 0 0x08000000>;
+		bank-width = <2>;
+
+		gpmc,mux-add-data;
+		gpmc,cs-on = <0>;
+		gpmc,cs-rd-off = <186>;
+		gpmc,cs-wr-off = <186>;
+		gpmc,adv-on = <12>;
+		gpmc,adv-rd-off = <48>;
+		gpmc,adv-wr-off = <48>;
+		gpmc,oe-on = <54>;
+		gpmc,oe-off = <168>;
+		gpmc,we-on = <54>;
+		gpmc,we-off = <168>;
+		gpmc,rd-cycle = <186>;
+		gpmc,wr-cycle = <186>;
+		gpmc,access = <114>;
+		gpmc,page-burst-access = <6>;
+		gpmc,bus-turnaround = <12>;
+		gpmc,cycle2cycle-delay = <18>;
+		gpmc,wr-data-mux-bus = <90>;
+		gpmc,wr-access = <186>;
+		gpmc,cycle2cycle-samecsen;
+		gpmc,cycle2cycle-diffcsen;
+
+		partition at 0 {
+			label = "bootloader-nor";
+			reg = <0 0x40000>;
+		};
+		partition at 0x40000 {
+			label = "params-nor";
+			reg = <0x40000 0x40000>;
+		};
+		partition at 0x80000 {
+			label = "kernel-nor";
+			reg = <0x80000 0x200000>;
+		};
+		partition at 0x280000 {
+			label = "filesystem-nor";
+			reg = <0x240000 0x7d80000>;
+		};
+	};
+};
diff --git a/arch/arm/mach-omap2/gpmc.c b/arch/arm/mach-omap2/gpmc.c
index e822d2b..eb167b3 100644
--- a/arch/arm/mach-omap2/gpmc.c
+++ b/arch/arm/mach-omap2/gpmc.c
@@ -26,6 +26,7 @@
 #include <linux/interrupt.h>
 #include <linux/platform_device.h>
 #include <linux/of.h>
+#include <linux/of_address.h>
 #include <linux/of_mtd.h>
 #include <linux/of_device.h>
 #include <linux/mtd/nand.h>
@@ -513,6 +514,37 @@ static int gpmc_cs_delete_mem(int cs)
 	return r;
 }
 
+/**
+ * gpmc_cs_remap - remaps a chip-select physical base address
+ * @cs:		chip-select to remap
+ * @base:	physical base address to re-map chip-select to
+ *
+ * Re-maps a chip-select to a new physical base address specified by
+ * "base". Returns 0 on success and appropriate negative error code
+ * on failure.
+ */
+static int gpmc_cs_remap(int cs, u32 base)
+{
+	int ret;
+	u32 old_base, size;
+
+	if (cs > GPMC_CS_NUM)
+		return -ENODEV;
+	gpmc_cs_get_memconf(cs, &old_base, &size);
+	if (base == old_base)
+		return 0;
+	gpmc_cs_disable_mem(cs);
+	ret = gpmc_cs_delete_mem(cs);
+	if (IS_ERR_VALUE(ret))
+		return ret;
+	ret = gpmc_cs_insert_mem(cs, base, size);
+	if (IS_ERR_VALUE(ret))
+		return ret;
+	gpmc_cs_enable_mem(cs, base, size);
+
+	return 0;
+}
+
 int gpmc_cs_request(int cs, unsigned long size, unsigned long *base)
 {
 	struct resource *res = &gpmc_cs_mem[cs];
@@ -1172,6 +1204,12 @@ int gpmc_cs_program_settings(int cs, struct gpmc_settings *p)
 }
 
 #ifdef CONFIG_OF
+static struct property device_disabled = {
+	.name = "status",
+	.length = sizeof("disabled"),
+	.value = "disabled",
+};
+
 static struct of_device_id gpmc_dt_ids[] = {
 	{ .compatible = "ti,omap2420-gpmc" },
 	{ .compatible = "ti,omap2430-gpmc" },
@@ -1391,6 +1429,79 @@ static int gpmc_probe_onenand_child(struct platform_device *pdev,
 }
 #endif
 
+/**
+ * gpmc_probe_nor_child - configures the gpmc for a nor device
+ * @pdev:	pointer to gpmc platform device
+ * @child:	pointer to device-tree node for nor device
+ *
+ * Allocates and configures a GPMC chip-select for a NOR flash device.
+ * Returns 0 on success and appropriate negative error code on failure.
+ */
+static int gpmc_probe_nor_child(struct platform_device *pdev,
+				struct device_node *child)
+{
+	struct gpmc_settings gpmc_s;
+	struct gpmc_timings gpmc_t;
+	struct resource res;
+	unsigned long base;
+	int ret, cs;
+
+	if (of_property_read_u32(child, "reg", &cs) < 0) {
+		dev_err(&pdev->dev, "%s has no 'reg' property\n",
+			child->full_name);
+		return -ENODEV;
+	}
+
+	if (of_address_to_resource(child, 0, &res)) {
+		dev_err(&pdev->dev, "%s has malformed 'reg' property\n",
+			child->full_name);
+		return -ENODEV;
+	}
+
+	ret = gpmc_cs_request(cs, res.end - res.start, &base);
+	if (IS_ERR_VALUE(ret)) {
+		dev_err(&pdev->dev, "cannot request GPMC CS %d\n", cs);
+		goto err1;
+	}
+
+	/*
+	 * FIXME: gpmc_cs_request() will map the CS to an arbitary
+	 * location in the gpmc address space. When booting with
+	 * device-tree we want the NOR flash to be mapped to the
+	 * location specified in the device-tree blob. So remap the
+	 * CS to this location. Once DT migration is complete should
+	 * just make gpmc_cs_request() map a specific address.
+	 */
+	ret = gpmc_cs_remap(cs, res.start);
+	if (IS_ERR_VALUE(ret)) {
+		dev_err(&pdev->dev, "cannot remap GPMC CS %d to 0x%x\n",
+			cs, res.start);
+		goto err2;
+	}
+
+	gpmc_read_settings_dt(child, &gpmc_s);
+
+	ret = of_property_read_u32(child, "bank-width", &gpmc_s.device_width);
+	if (IS_ERR_VALUE(ret))
+		goto err2;
+
+	ret = gpmc_cs_program_settings(cs, &gpmc_s);
+	if (IS_ERR_VALUE(ret))
+		goto err2;
+
+	gpmc_read_timings_dt(child, &gpmc_t);
+	gpmc_cs_set_timings(cs, &gpmc_t);
+
+	return 0;
+err2:
+	gpmc_cs_free(cs);
+err1:
+	/* Prevent registering child device on error */
+	of_add_property(child, &device_disabled);
+
+	return ret;
+}
+
 static int gpmc_probe_dt(struct platform_device *pdev)
 {
 	int ret;
@@ -1424,6 +1535,15 @@ static int gpmc_probe_dt(struct platform_device *pdev)
 			return ret;
 		}
 	}
+
+	for_each_node_by_name(child, "nor") {
+		ret = gpmc_probe_nor_child(pdev, child);
+		if (ret < 0) {
+			of_node_put(child);
+			return ret;
+		}
+	}
+
 	return 0;
 }
 #else
-- 
1.7.10.4

[PATCH 11/14] ARM: OMAP2+: Add device-tree support for NOR flash

[Ezequiel Garcia]

Hi Jon,

On Tue, Feb 26, 2013 at 2:30 PM, Jon Hunter <jon-hunter@ti.com> wrote:
[...]
> +static int gpmc_probe_nor_child(struct platform_device *pdev,
> +                               struct device_node *child)
> +{
> +       struct gpmc_settings gpmc_s;
> +       struct gpmc_timings gpmc_t;
> +       struct resource res;
> +       unsigned long base;
> +       int ret, cs;
> +
> +       if (of_property_read_u32(child, "reg", &cs) < 0) {
> +               dev_err(&pdev->dev, "%s has no 'reg' property\n",
> +                       child->full_name);
> +               return -ENODEV;
> +       }
> +
> +       if (of_address_to_resource(child, 0, &res)) {
> +               dev_err(&pdev->dev, "%s has malformed 'reg' property\n",
> +                       child->full_name);
> +               return -ENODEV;
> +       }
> +
> +       ret = gpmc_cs_request(cs, res.end - res.start, &base);

How about using resource_size() above?

BTW, I believe it's size = end - start + 1.

-- 
    Ezequiel

[PATCH 11/14] ARM: OMAP2+: Add device-tree support for NOR flash

[Jon Hunter]

On 03/01/2013 03:25 PM, Ezequiel Garcia wrote:
> Hi Jon,
> 
> On Tue, Feb 26, 2013 at 2:30 PM, Jon Hunter <jon-hunter@ti.com> wrote:
> [...]
>> +static int gpmc_probe_nor_child(struct platform_device *pdev,
>> +                               struct device_node *child)
>> +{
>> +       struct gpmc_settings gpmc_s;
>> +       struct gpmc_timings gpmc_t;
>> +       struct resource res;
>> +       unsigned long base;
>> +       int ret, cs;
>> +
>> +       if (of_property_read_u32(child, "reg", &cs) < 0) {
>> +               dev_err(&pdev->dev, "%s has no 'reg' property\n",
>> +                       child->full_name);
>> +               return -ENODEV;
>> +       }
>> +
>> +       if (of_address_to_resource(child, 0, &res)) {
>> +               dev_err(&pdev->dev, "%s has malformed 'reg' property\n",
>> +                       child->full_name);
>> +               return -ENODEV;
>> +       }
>> +
>> +       ret = gpmc_cs_request(cs, res.end - res.start, &base);
> 
> How about using resource_size() above?
> 
> BTW, I believe it's size = end - start + 1.

Thanks, yes I can update.

Cheers
Jon

[PATCH 11/14] ARM: OMAP2+: Add device-tree support for NOR flash

[Ezequiel Garcia]

Hi Jon,

On Tue, Feb 26, 2013 at 2:30 PM, Jon Hunter <jon-hunter@ti.com> wrote:
> +
> +Example:
> +
> +gpmc: gpmc at 6e000000 {
> +       compatible = "ti,omap3430-gpmc", "simple-bus";

I'm concern about using simple-bus, and I'm not entirely sure this will work.

AFAIK, you can't correlate a parent-child relationship in the device tree
to the order in which drivers will be probed,
so it's only a matter of coincidence if this is working for you right now.
The GPMC code is in "arch/arm/mach-omap2", is located *before* the mtd code
in the Makefile, and thus the GPMC driver loads *before* the MTD code.

Morevore, I believe that when we move GPMC from arch/arm/mach-omap2
to drivers/memory where it should be, this 'simple-bus' will stop
working properly.

Of course, I can be wrong, but I think you will have to find some other way
to initialize the GPMC's childs.

-- 
    Ezequiel

[PATCH 11/14] ARM: OMAP2+: Add device-tree support for NOR flash

[Jon Hunter]

On 03/04/2013 05:57 AM, Ezequiel Garcia wrote:
> Hi Jon,
> 
> On Tue, Feb 26, 2013 at 2:30 PM, Jon Hunter <jon-hunter@ti.com> wrote:
>> +
>> +Example:
>> +
>> +gpmc: gpmc at 6e000000 {
>> +       compatible = "ti,omap3430-gpmc", "simple-bus";
> 
> I'm concern about using simple-bus, and I'm not entirely sure this will work.
> 
> AFAIK, you can't correlate a parent-child relationship in the device tree
> to the order in which drivers will be probed,
> so it's only a matter of coincidence if this is working for you right now.

Ummm, I am not sure I am convinced. Have you looked at
of_platform_bus_create() which creates the devices? It is clearly
creating the child devices after creating the parent device.

> The GPMC code is in "arch/arm/mach-omap2", is located *before* the mtd code
> in the Makefile, and thus the GPMC driver loads *before* the MTD code.

But shouldn't it only matter in the order in which devices are
registered? Unless you have a situation in which the devices are
registered in order (parent then child) but only the driver for the
child is registered before the parent and child devices are registered.
I could see that being a problem.

The only other thing that I can think of is if you happen to be creating
a device both via device-tree and may be you are creating a duplicate
device and calling platform_device_register() somewhere else in your code.

> Morevore, I believe that when we move GPMC from arch/arm/mach-omap2
> to drivers/memory where it should be, this 'simple-bus' will stop
> working properly.

Why? I don't see how the location of files would change this.

> Of course, I can be wrong, but I think you will have to find some other way
> to initialize the GPMC's childs.

Well isn't this how device-tree is meant to work?

It would be great if you could debug your problem and fully understand
why this is not working for you, so we can see if there is a real
problem here. Otherwise this appears to be just guess work.

Jon

[PATCH 11/14] ARM: OMAP2+: Add device-tree support for NOR flash

[Jon Hunter]

On 03/04/2013 11:51 AM, Jon Hunter wrote:
> 
> On 03/04/2013 05:57 AM, Ezequiel Garcia wrote:
>> Hi Jon,
>>
>> On Tue, Feb 26, 2013 at 2:30 PM, Jon Hunter <jon-hunter@ti.com> wrote:
>>> +
>>> +Example:
>>> +
>>> +gpmc: gpmc at 6e000000 {
>>> +       compatible = "ti,omap3430-gpmc", "simple-bus";
>>
>> I'm concern about using simple-bus, and I'm not entirely sure this will work.
>>
>> AFAIK, you can't correlate a parent-child relationship in the device tree
>> to the order in which drivers will be probed,
>> so it's only a matter of coincidence if this is working for you right now.
> 
> Ummm, I am not sure I am convinced. Have you looked at
> of_platform_bus_create() which creates the devices? It is clearly
> creating the child devices after creating the parent device.

Sorry you said order by which drivers are probed. Yes I would agree that
you cannot control the order drivers are probed, but just the order
devices are registered.

However, I don't see this as being a device-tree specific issue. Even
when not using device-tree you would need to ensure that probing the
parent happens before the child for the GPMC.

Jon

[PATCH 11/14] ARM: OMAP2+: Add device-tree support for NOR flash

[Mark Jackson]

On 26/02/13 17:30, Jon Hunter wrote:
> NOR flash is not currently supported when booting with device-tree
> on OMAP2+ devices. Add support to detect and configure NOR devices
> when booting with device-tree.
> 
> Add documentation for the TI GPMC NOR binding.
> 
> Signed-off-by: Jon Hunter <jon-hunter@ti.com>

I'm trying to test this series, and am unable get my NOR device recognised.

If I remove all reference to NOR (and keep only my NAND device definition), then
everything seems to boot fine (so I'm assuming I've got at least the basics of
the patch set working).

My GPMC tree looks like this:-

gpmc: gpmc at 50000000 {
	compatible = "ti,am3352-gpmc", "simple-bus";
	ti,hwmods = "gpmc";
	status = "okay";
	gpmc,num-waitpins = <2>;
	pinctrl-names = "default";
	pinctrl-0 = <&gpmc_pins>;

	#address-cells = <2>;
	#size-cells = <1>;
	ranges = <0 0 0x08000000 0x10000000>,	/* CS0: NAND 256M */
		 <3 0 0x1a000000 0x04000000>;	/* CS3: NOR 64M */

	nand at 0,0 {
		linux,mtd-name= "micron,mt29f2g08abaea";
		reg = <0 0x00000000 0x10000000>; /* CS0, offset 0 */
		nand-bus-width = <8>;
		ti,nand-ecc-opt = "bch8";

		gpmc,device-nand;
		gpmc,device-width = <1>;
		gpmc,wait-pin = <0>;

		gpmc,sync-clk = <0>;
		gpmc,cs-on = <0>;
		gpmc,cs-rd-off = <44>;
		gpmc,cs-wr-off = <44>;
		gpmc,adv-on = <6>;
		gpmc,adv-rd-off = <34>;
		gpmc,adv-wr-off = <44>;
		gpmc,we-off = <40>;
		gpmc,oe-off = <54>;
		gpmc,access = <64>;
		gpmc,rd-cycle = <82>;
		gpmc,wr-cycle = <82>;
		gpmc,wr-access = <40>;
		gpmc,wr-data-mux-bus = <0>;

		#address-cells = <1>;
		#size-cells = <1>;
		elm_id = <&elm>;

		/*
		MTD partition table
		===================
		+------------+-->0x00000000-> SPL start         (SPL copy on 1st block)
		|            |
		|            |-->0x0001FFFF-> SPL end
		|            |-->0x00020000-> SPL.backup1 start (SPL copy on 2nd block)
		|            |
		|            |-->0x0003FFFF-> SPL.backup1 end
		|            |-->0x00040000-> SPL.backup2 start (SPL copy on 3rd block)
		|            |
		|            |-->0x0005FFFF-> SPL.backup2 end
		|            |-->0x00060000-> SPL.backup3 start (SPL copy on 4th block)
		|            |
		|            |-->0x0007FFFF-> SPL.backup3 end
		|            |-->0x00080000-> U-Boot start
		|            |                                    
		|            |-->0x001DFFFF-> U-Boot end
		|            |-->0x001E0000-> ENV start
		|            |
		|            |-->0x001FFFFF-> ENV end
		|            |-->0x00200000-> File system start
		|            |
		|            |-->0x041FFFFF-> File system end
		|            |-->0x04200000-> Data storage start
		|            |
		+------------+-->0x10000000-> NAND end (Free end)
		*/
		partition at 0 {
			label = "spl1";
			reg = <0x00000000 0x00020000>; /* 128KB */
		};

		partition at 1 {
			label = "spl2";
			reg = <0x00020000 0x00020000>; /* 128KB */
		};

		partition at 2 {
			label = "spl3";
			reg = <0x00040000 0x00020000>; /* 128KB */
		};

		partition at 3 {
			label = "spl4";
			reg = <0x00060000 0x00020000>; /* 128KB */
		};

		partition at 4 {
			label = "boot";
			reg = <0x00080000 0x00160000>; /* 1408KB */
		};

		partition at 5 {
			label = "env";
			reg = <0x001e0000 0x00020000>; /* 128KB */
		};

		partition at 6 {
			label = "rootfs";
			reg = <0x00200000 0x04000000>; /* 64MB */
		};

		partition at 7 {
			label = "data";
			reg = <0x04200000 0x0be00000>; /* 190MB */
		};
	};

	nor at 1,0 {
		reg = <3 0x00000000 0x04000000>;
		compatible = "cfi-flash";
		linux,mtd-name = "spansion,s29gl064n90t";
		bank-width = <2>;

		gpmc,device-width = <1>;
		gpmc,mux-add-data;

		gpmc,sync-clk = <0>;
		gpmc,cs-on = <10>;
		gpmc,cs-rd-off = <150>;
		gpmc,cs-wr-off = <150>;
		gpmc,adv-on = <10>;
		gpmc,adv-rd-off = <10>;
		gpmc,adv-wr-off = <10>;
		gpmc,oe-on = <30>;
		gpmc,oe-off = <150>;
		gpmc,we-on = <30>;
		gpmc,we-off = <150>;
		gpmc,rd-cycle = <150>;
		gpmc,wr-cycle = <150>;
		gpmc,access = <130>;
		gpmc,page-burst-access = <10>;
		gpmc,cycle2cycle-diff = <1>;
		gpmc,cycle2cycle-same = <1>;
		gpmc,cycle2cycle-delay = <10>;
		gpmc,wr-data-mux-bus = <60>;

		#address-cells = <1>;
		#size-cells = <1>;
		elm_id = <&elm>;

		partition at 0 {
			label = "data";
			reg = <0x00000000 0x04000000>; /* 64MB */
		};
	};
};

Booting with this NOR device produces the following oops:-

[    0.000000] Booting Linux on physical CPU 0x0
[    0.000000] Linux version 3.9.0-rc1-12191-ga00d6d1-dirty (mpfj at mpfj-nanobone) (gcc version 4.5.4 (Buildroot 2012.11) ) #33 Tue Mar 5 13:08:25 GMT 2013
[    0.000000] CPU: ARMv7 Processor [413fc082] revision 2 (ARMv7), cr=10c53c7d
[    0.000000] CPU: PIPT / VIPT nonaliasing data cache, VIPT aliasing instruction cache
[    0.000000] Machine: Generic AM33XX (Flattened Device Tree), model: TI AM335x BeagleBone
[    0.000000] Memory policy: ECC disabled, Data cache writeback
[    0.000000] CPU: All CPU(s) started in SVC mode.
[    0.000000] AM335X ES1.0 (neon )
[    0.000000] Built 1 zonelists in Zone order, mobility grouping on.  Total pages: 64768
[    0.000000] Kernel command line: console=ttyO0,115200n8 noinitrd ip=off mem=256M rootwait=1 ubi.mtd=6,2048 rootfstype=ubifs root=ubi0:rootfs
[    0.000000] PID hash table entries: 1024 (order: 0, 4096 bytes)
[    0.000000] Dentry cache hash table entries: 32768 (order: 5, 131072 bytes)
[    0.000000] Inode-cache hash table entries: 16384 (order: 4, 65536 bytes)
[    0.000000] __ex_table already sorted, skipping sort
[    0.000000] Memory: 255MB = 255MB total
[    0.000000] Memory: 248068k/248068k available, 14076k reserved, 0K highmem
[    0.000000] Virtual kernel memory layout:
[    0.000000]     vector  : 0xffff0000 - 0xffff1000   (   4 kB)
[    0.000000]     fixmap  : 0xfff00000 - 0xfffe0000   ( 896 kB)
[    0.000000]     vmalloc : 0xd0800000 - 0xff000000   ( 744 MB)
[    0.000000]     lowmem  : 0xc0000000 - 0xd0000000   ( 256 MB)
[    0.000000]     pkmap   : 0xbfe00000 - 0xc0000000   (   2 MB)
[    0.000000]       .text : 0xc0008000 - 0xc04d407c   (4913 kB)
[    0.000000]       .init : 0xc04d5000 - 0xc050593c   ( 195 kB)
[    0.000000]       .data : 0xc0506000 - 0xc0554e80   ( 316 kB)
[    0.000000]        .bss : 0xc0554e80 - 0xc0a76b60   (5256 kB)
[    0.000000] NR_IRQS:16 nr_irqs:16 16
[    0.000000] IRQ: Found an INTC at 0xfa200000 (revision 5.0) with 128 interrupts
[    0.000000] Total of 128 interrupts on 1 active controller
[    0.000000] OMAP clockevent source: GPTIMER1 at 26000000 Hz
[    0.000000] sched_clock: 32 bits at 26MHz, resolution 38ns, wraps every 165191ms
[    0.000000] OMAP clocksource: GPTIMER2 at 26000000 Hz
[    0.000000] Console: colour dummy device 80x30
[    0.000000] Lock dependency validator: Copyright (c) 2006 Red Hat, Inc., Ingo Molnar
[    0.000000] ... MAX_LOCKDEP_SUBCLASSES:  8
[    0.000000] ... MAX_LOCK_DEPTH:          48
[    0.000000] ... MAX_LOCKDEP_KEYS:        8191
[    0.000000] ... CLASSHASH_SIZE:          4096
[    0.000000] ... MAX_LOCKDEP_ENTRIES:     16384
[    0.000000] ... MAX_LOCKDEP_CHAINS:      32768
[    0.000000] ... CHAINHASH_SIZE:          16384
[    0.000000]  memory used by lock dependency info: 3695 kB
[    0.000000]  per task-struct memory footprint: 1152 bytes
[    0.000845] Calibrating delay loop... 479.23 BogoMIPS (lpj=2396160)
[    0.109860] pid_max: default: 32768 minimum: 301
[    0.110128] Security Framework initialized
[    0.110234] Mount-cache hash table entries: 512
[    0.120373] CPU: Testing write buffer coherency: ok
[    0.121628] Setting up static identity map for 0xc03caf60 - 0xc03cafb8
[    0.125001] devtmpfs: initialized
[    0.186744] pinctrl core: initialized pinctrl subsystem
[    0.191942] regulator-dummy: no parameters
[    0.194142] NET: Registered protocol family 16
[    0.194891] DMA: preallocated 256 KiB pool for atomic coherent allocations
[    0.213783] OMAP GPIO hardware version 0.1
[    0.236730] omap-gpmc 50000000.gpmc: could not find pctldev for node /pinmux at 44e10800/gpmc_pins, deferring probe
[    0.236781] platform 50000000.gpmc: Driver omap-gpmc requests probe deferral
[    0.237476] No ATAGs?
[    0.237494] hw-breakpoint: debug architecture 0x4 unsupported.
[    0.269694] bio: create slab <bio-0> at 0
[    0.335502] omap-dma-engine omap-dma-engine: OMAP DMA engine driver
[    0.342078] usbcore: registered new interface driver usbfs
[    0.342491] usbcore: registered new interface driver hub
[    0.343136] usbcore: registered new device driver usb
[    0.344082] omap_i2c 44e0b000.i2c: could not find pctldev for node /pinmux at 44e10800/i2c1_pins, deferring probe
[    0.344128] platform 44e0b000.i2c: Driver omap_i2c requests probe deferral
[    0.351421] cfg80211: Calling CRDA to update world regulatory domain
[    0.353089] Switching to clocksource gp_timer
[    0.401911] NET: Registered protocol family 2
[    0.403612] TCP established hash table entries: 2048 (order: 2, 16384 bytes)
[    0.403822] TCP bind hash table entries: 2048 (order: 4, 73728 bytes)
[    0.404761] TCP: Hash tables configured (established 2048 bind 2048)
[    0.404941] TCP: reno registered
[    0.404969] UDP hash table entries: 256 (order: 2, 20480 bytes)
[    0.405226] UDP-Lite hash table entries: 256 (order: 2, 20480 bytes)
[    0.406010] NET: Registered protocol family 1
[    0.406917] NetWinder Floating Point Emulator V0.97 (double precision)
[    0.407357] CPU PMU: probing PMU on CPU 0
[    0.407384] hw perfevents: enabled with ARMv7 Cortex-A8 PMU driver, 5 counters available
[    0.418794] msgmni has been set to 484
[    0.423083] io scheduler noop registered
[    0.423107] io scheduler deadline registered
[    0.423300] io scheduler cfq registered (default)
[    0.424353] pinctrl-single 44e10800.pinmux: 142 pins at pa f9e10800 size 568
[    0.425948] Serial: 8250/16550 driver, 4 ports, IRQ sharing enabled
[    0.433116] 44e09000.serial: ttyO0 at MMIO 0x44e09000 (irq = 88) is a OMAP UART0
[    0.954564] console [ttyO0] enabled
[    0.960651] 48022000.serial: ttyO1 at MMIO 0x48022000 (irq = 89) is a OMAP UART1
[    0.970290] 48024000.serial: ttyO2 at MMIO 0x48024000 (irq = 90) is a OMAP UART2
[    0.979840] 481a6000.serial: ttyO3 at MMIO 0x481a6000 (irq = 60) is a OMAP UART3
[    0.989314] 481a8000.serial: ttyO4 at MMIO 0x481a8000 (irq = 61) is a OMAP UART4
[    0.998713] 481aa000.serial: ttyO5 at MMIO 0x481aa000 (irq = 62) is a OMAP UART5
[    1.033299] brd: module loaded
[    1.052135] loop: module loaded
[    1.060341] Unhandled fault: external abort on non-linefetch (0x1028) at 0xd3000020
[    1.068463] Internal error: : 1028 [#1] ARM
[    1.072895] CPU: 0    Not tainted  (3.9.0-rc1-12191-ga00d6d1-dirty #33)
[    1.079903] PC is at cfi_qry_present+0x294/0x2c0
[    1.084785] LR is at cfi_qry_present+0x60/0x2c0
[    1.089576] pc : [<c0230c94>]    lr : [<c0230a60>]    psr: 60000113
[    1.089576] sp : cf04dd28  ip : 00000006  fp : 00000001
[    1.101716] r10: 00005151  r9 : 00000000  r8 : 00005959
[    1.107238] r7 : 00000002  r6 : 00000002  r5 : 00000008  r4 : cf28d95c
[    1.114140] r3 : 00005252  r2 : d3000020  r1 : 00000020  r0 : d3000000
[    1.121044] Flags: nZCv  IRQs on  FIQs on  Mode SVC_32  ISA ARM  Segment kernel
[    1.128772] Control: 10c5387d  Table: 80004019  DAC: 00000015
[    1.134849] Process swapper (pid: 1, stack limit = 0xcf04c238)
[    1.141018] Stack: (0xcf04dd28 to 0xcf04e000)
[    1.145627] dd20:                   00000000 cf28d95c cf04ddc4 00000000 c050e0a8 00000002
[    1.154277] dd40: 000000aa 00000008 00009898 c0230e38 cf04c000 00000001 cf04c000 00000000
[    1.162928] dd60: 00000000 cf04ddc4 cf28d95c 00000002 cf0bcc00 00000001 c04920a8 c022fe80
[    1.171576] dd80: cf04c000 cf04b7b0 cf04b340 00000000 00000001 000000d0 c06223ac 00000001
[    1.180228] dda0: cf28d95c c05440a0 cf04ddc4 00000002 cf0bcc00 00000001 c04920a8 c023a97c
[    1.188880] ddc0: cf04c000 00000000 00000000 00000002 00000001 00000000 00000000 00000000
[    1.197530] dde0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
[    1.206182] de00: c054405c c0544094 c0544084 c054407c c04920a8 cf28d95c cf0bcc00 00000000
[    1.214833] de20: c04920a8 c022fde8 cf28d950 00000000 c0c81f60 cf28d950 04000000 c023b180
[    1.223483] de40: 00000000 22222222 cf0bcc10 cf294500 00000000 cf04de80 1a000000 1dffffff
[    1.232134] de60: c0c81f9c 00000200 00000000 00000000 00000000 c005a7ac 00000000 c00c3c50
[    1.240784] de80: c0a79aa4 00000001 c0a716b8 cf0bcc10 c0a716b8 c0a716c8 00000000 00000000
[    1.249434] dea0: c0505608 c0544210 00000000 c0213bb0 c0213b98 c0212b08 00000000 cf0bcc10
[    1.258085] dec0: c0544210 cf0bcc44 00000000 c04f16e4 0000005a c0212d30 c0544210 c0212c9c
[    1.266735] dee0: cf04dee8 c02112a8 cf0444a8 cf0af590 c04f16e4 c0544210 c0542430 cf28d9c0
[    1.275386] df00: 00000000 c0211b00 c04928b0 c0544210 c0544210 00000006 c04fcff4 00000000
[    1.284037] df20: c04f16e4 c02132fc 00000000 c04fd014 00000006 c04fcff4 00000000 c04f16e4
[    1.292688] df40: 0000005a c04d581c 00000006 00000006 60000113 c04fd010 c04fd014 00000006
[    1.301341] df60: c04fcff4 c0554e80 c04d5168 0000005a 00000000 c04d59d8 00000006 00000006
[    1.309990] df80: c04d5168 00000000 00000000 c03c6494 00000000 00000000 00000000 00000000
[    1.318640] dfa0: 00000000 c03c649c 00000000 c0013870 00000000 00000000 00000000 00000000
[    1.327291] dfc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
[    1.335943] dfe0: 00000000 00000000 00000000 00000000 00000013 00000000 ff99f7ff f7b97f3f
[    1.344601] [<c0230c94>] (cfi_qry_present+0x294/0x2c0) from [<c0230e38>] (cfi_qry_mode_on+0x178/0xfd8)
[    1.354449] [<c0230e38>] (cfi_qry_mode_on+0x178/0xfd8) from [<c022fe80>] (cfi_probe_chip+0x40/0xbc0)
[    1.364116] [<c022fe80>] (cfi_probe_chip+0x40/0xbc0) from [<c023a97c>] (mtd_do_chip_probe+0xa8/0x388)
[    1.373873] [<c023a97c>] (mtd_do_chip_probe+0xa8/0x388) from [<c022fde8>] (do_map_probe+0x6c/0x8c)
[    1.383354] [<c022fde8>] (do_map_probe+0x6c/0x8c) from [<c023b180>] (of_flash_probe+0x218/0x5a4)
[    1.392658] [<c023b180>] (of_flash_probe+0x218/0x5a4) from [<c0213bb0>] (platform_drv_probe+0x18/0x1c)
[    1.402506] [<c0213bb0>] (platform_drv_probe+0x18/0x1c) from [<c0212b08>] (driver_probe_device+0x94/0x228)
[    1.412720] [<c0212b08>] (driver_probe_device+0x94/0x228) from [<c0212d30>] (__driver_attach+0x94/0x98)
[    1.422671] [<c0212d30>] (__driver_attach+0x94/0x98) from [<c02112a8>] (bus_for_each_dev+0x68/0x8c)
[    1.432244] [<c02112a8>] (bus_for_each_dev+0x68/0x8c) from [<c0211b00>] (bus_add_driver+0x1b0/0x22c)
[    1.441908] [<c0211b00>] (bus_add_driver+0x1b0/0x22c) from [<c02132fc>] (driver_register+0x78/0x144)
[    1.451580] [<c02132fc>] (driver_register+0x78/0x144) from [<c04d581c>] (do_one_initcall+0x98/0x16c)
[    1.461245] [<c04d581c>] (do_one_initcall+0x98/0x16c) from [<c04d59d8>] (kernel_init_freeable+0xe8/0x1ac)
[    1.471369] [<c04d59d8>] (kernel_init_freeable+0xe8/0x1ac) from [<c03c649c>] (kernel_init+0x8/0xe4)
[    1.480956] [<c03c649c>] (kernel_init+0x8/0xe4) from [<c0013870>] (ret_from_fork+0x14/0x24)
[    1.489792] Code: e7d05001 e6ef5075 eaffffcb e0802002 (e1d2c0b0)
[    1.496294] ---[ end trace a9ca4747ade8eff8 ]---
[    1.501297] Kernel panic - not syncing: Attempted to kill init! exitcode=0x0000000b
[    1.501297]


Can anyone point out what I've got wrong ?

[PATCH 11/14] ARM: OMAP2+: Add device-tree support for NOR flash

[Jon Hunter]

On 03/05/2013 08:34 AM, Mark Jackson wrote:
> On 26/02/13 17:30, Jon Hunter wrote:
>> NOR flash is not currently supported when booting with device-tree
>> on OMAP2+ devices. Add support to detect and configure NOR devices
>> when booting with device-tree.
>>
>> Add documentation for the TI GPMC NOR binding.
>>
>> Signed-off-by: Jon Hunter <jon-hunter@ti.com>
> 
> I'm trying to test this series, and am unable get my NOR device recognised.
> 
> If I remove all reference to NOR (and keep only my NAND device definition), then
> everything seems to boot fine (so I'm assuming I've got at least the basics of
> the patch set working).
> 
> My GPMC tree looks like this:-
> 
> gpmc: gpmc at 50000000 {
> 	compatible = "ti,am3352-gpmc", "simple-bus";
> 	ti,hwmods = "gpmc";
> 	status = "okay";
> 	gpmc,num-waitpins = <2>;
> 	pinctrl-names = "default";
> 	pinctrl-0 = <&gpmc_pins>;
> 
> 	#address-cells = <2>;
> 	#size-cells = <1>;
> 	ranges = <0 0 0x08000000 0x10000000>,	/* CS0: NAND 256M */
> 		 <3 0 0x1a000000 0x04000000>;	/* CS3: NOR 64M */
> 
> 	nand at 0,0 {
> 		linux,mtd-name= "micron,mt29f2g08abaea";
> 		reg = <0 0x00000000 0x10000000>; /* CS0, offset 0 */
> 		nand-bus-width = <8>;
> 		ti,nand-ecc-opt = "bch8";
> 
> 		gpmc,device-nand;
> 		gpmc,device-width = <1>;
> 		gpmc,wait-pin = <0>;
> 
> 		gpmc,sync-clk = <0>;
> 		gpmc,cs-on = <0>;
> 		gpmc,cs-rd-off = <44>;
> 		gpmc,cs-wr-off = <44>;
> 		gpmc,adv-on = <6>;
> 		gpmc,adv-rd-off = <34>;
> 		gpmc,adv-wr-off = <44>;
> 		gpmc,we-off = <40>;
> 		gpmc,oe-off = <54>;
> 		gpmc,access = <64>;
> 		gpmc,rd-cycle = <82>;
> 		gpmc,wr-cycle = <82>;
> 		gpmc,wr-access = <40>;
> 		gpmc,wr-data-mux-bus = <0>;
> 
> 		#address-cells = <1>;
> 		#size-cells = <1>;
> 		elm_id = <&elm>;
> 
> 		/*
> 		MTD partition table
> 		===================
> 		+------------+-->0x00000000-> SPL start         (SPL copy on 1st block)
> 		|            |
> 		|            |-->0x0001FFFF-> SPL end
> 		|            |-->0x00020000-> SPL.backup1 start (SPL copy on 2nd block)
> 		|            |
> 		|            |-->0x0003FFFF-> SPL.backup1 end
> 		|            |-->0x00040000-> SPL.backup2 start (SPL copy on 3rd block)
> 		|            |
> 		|            |-->0x0005FFFF-> SPL.backup2 end
> 		|            |-->0x00060000-> SPL.backup3 start (SPL copy on 4th block)
> 		|            |
> 		|            |-->0x0007FFFF-> SPL.backup3 end
> 		|            |-->0x00080000-> U-Boot start
> 		|            |                                    
> 		|            |-->0x001DFFFF-> U-Boot end
> 		|            |-->0x001E0000-> ENV start
> 		|            |
> 		|            |-->0x001FFFFF-> ENV end
> 		|            |-->0x00200000-> File system start
> 		|            |
> 		|            |-->0x041FFFFF-> File system end
> 		|            |-->0x04200000-> Data storage start
> 		|            |
> 		+------------+-->0x10000000-> NAND end (Free end)
> 		*/
> 		partition at 0 {
> 			label = "spl1";
> 			reg = <0x00000000 0x00020000>; /* 128KB */
> 		};
> 
> 		partition at 1 {
> 			label = "spl2";
> 			reg = <0x00020000 0x00020000>; /* 128KB */
> 		};
> 
> 		partition at 2 {
> 			label = "spl3";
> 			reg = <0x00040000 0x00020000>; /* 128KB */
> 		};
> 
> 		partition at 3 {
> 			label = "spl4";
> 			reg = <0x00060000 0x00020000>; /* 128KB */
> 		};
> 
> 		partition at 4 {
> 			label = "boot";
> 			reg = <0x00080000 0x00160000>; /* 1408KB */
> 		};
> 
> 		partition at 5 {
> 			label = "env";
> 			reg = <0x001e0000 0x00020000>; /* 128KB */
> 		};
> 
> 		partition at 6 {
> 			label = "rootfs";
> 			reg = <0x00200000 0x04000000>; /* 64MB */
> 		};
> 
> 		partition at 7 {
> 			label = "data";
> 			reg = <0x04200000 0x0be00000>; /* 190MB */
> 		};
> 	};
> 
> 	nor at 1,0 {
> 		reg = <3 0x00000000 0x04000000>;
> 		compatible = "cfi-flash";
> 		linux,mtd-name = "spansion,s29gl064n90t";
> 		bank-width = <2>;
> 
> 		gpmc,device-width = <1>;

Only bank-width should be necessary for NOR (per the binding
documentation). However, if you do specify both, then they should match.
Do you have two 8-bits devices? If so may be I need to update the
documentation to make it clear this is the total width of all devices
for a given chip-select.

> 		gpmc,mux-add-data;
> 
> 		gpmc,sync-clk = <0>;
> 		gpmc,cs-on = <10>;
> 		gpmc,cs-rd-off = <150>;
> 		gpmc,cs-wr-off = <150>;
> 		gpmc,adv-on = <10>;
> 		gpmc,adv-rd-off = <10>;
> 		gpmc,adv-wr-off = <10>;
> 		gpmc,oe-on = <30>;
> 		gpmc,oe-off = <150>;
> 		gpmc,we-on = <30>;
> 		gpmc,we-off = <150>;
> 		gpmc,rd-cycle = <150>;
> 		gpmc,wr-cycle = <150>;
> 		gpmc,access = <130>;
> 		gpmc,page-burst-access = <10>;
> 		gpmc,cycle2cycle-diff = <1>;
> 		gpmc,cycle2cycle-same = <1>;
> 		gpmc,cycle2cycle-delay = <10>;
> 		gpmc,wr-data-mux-bus = <60>;
> 
> 		#address-cells = <1>;
> 		#size-cells = <1>;
> 		elm_id = <&elm>;
> 
> 		partition at 0 {
> 			label = "data";
> 			reg = <0x00000000 0x04000000>; /* 64MB */
> 		};
> 	};
> };
> 
> Booting with this NOR device produces the following oops:-
> 
> [    0.000000] Booting Linux on physical CPU 0x0
> [    0.000000] Linux version 3.9.0-rc1-12191-ga00d6d1-dirty (mpfj at mpfj-nanobone) (gcc version 4.5.4 (Buildroot 2012.11) ) #33 Tue Mar 5 13:08:25 GMT 2013
> [    0.000000] CPU: ARMv7 Processor [413fc082] revision 2 (ARMv7), cr=10c53c7d
> [    0.000000] CPU: PIPT / VIPT nonaliasing data cache, VIPT aliasing instruction cache
> [    0.000000] Machine: Generic AM33XX (Flattened Device Tree), model: TI AM335x BeagleBone
> [    0.000000] Memory policy: ECC disabled, Data cache writeback
> [    0.000000] CPU: All CPU(s) started in SVC mode.
> [    0.000000] AM335X ES1.0 (neon )
> [    0.000000] Built 1 zonelists in Zone order, mobility grouping on.  Total pages: 64768
> [    0.000000] Kernel command line: console=ttyO0,115200n8 noinitrd ip=off mem=256M rootwait=1 ubi.mtd=6,2048 rootfstype=ubifs root=ubi0:rootfs
> [    0.000000] PID hash table entries: 1024 (order: 0, 4096 bytes)
> [    0.000000] Dentry cache hash table entries: 32768 (order: 5, 131072 bytes)
> [    0.000000] Inode-cache hash table entries: 16384 (order: 4, 65536 bytes)
> [    0.000000] __ex_table already sorted, skipping sort
> [    0.000000] Memory: 255MB = 255MB total
> [    0.000000] Memory: 248068k/248068k available, 14076k reserved, 0K highmem
> [    0.000000] Virtual kernel memory layout:
> [    0.000000]     vector  : 0xffff0000 - 0xffff1000   (   4 kB)
> [    0.000000]     fixmap  : 0xfff00000 - 0xfffe0000   ( 896 kB)
> [    0.000000]     vmalloc : 0xd0800000 - 0xff000000   ( 744 MB)
> [    0.000000]     lowmem  : 0xc0000000 - 0xd0000000   ( 256 MB)
> [    0.000000]     pkmap   : 0xbfe00000 - 0xc0000000   (   2 MB)
> [    0.000000]       .text : 0xc0008000 - 0xc04d407c   (4913 kB)
> [    0.000000]       .init : 0xc04d5000 - 0xc050593c   ( 195 kB)
> [    0.000000]       .data : 0xc0506000 - 0xc0554e80   ( 316 kB)
> [    0.000000]        .bss : 0xc0554e80 - 0xc0a76b60   (5256 kB)
> [    0.000000] NR_IRQS:16 nr_irqs:16 16
> [    0.000000] IRQ: Found an INTC at 0xfa200000 (revision 5.0) with 128 interrupts
> [    0.000000] Total of 128 interrupts on 1 active controller
> [    0.000000] OMAP clockevent source: GPTIMER1 at 26000000 Hz
> [    0.000000] sched_clock: 32 bits at 26MHz, resolution 38ns, wraps every 165191ms
> [    0.000000] OMAP clocksource: GPTIMER2 at 26000000 Hz
> [    0.000000] Console: colour dummy device 80x30
> [    0.000000] Lock dependency validator: Copyright (c) 2006 Red Hat, Inc., Ingo Molnar
> [    0.000000] ... MAX_LOCKDEP_SUBCLASSES:  8
> [    0.000000] ... MAX_LOCK_DEPTH:          48
> [    0.000000] ... MAX_LOCKDEP_KEYS:        8191
> [    0.000000] ... CLASSHASH_SIZE:          4096
> [    0.000000] ... MAX_LOCKDEP_ENTRIES:     16384
> [    0.000000] ... MAX_LOCKDEP_CHAINS:      32768
> [    0.000000] ... CHAINHASH_SIZE:          16384
> [    0.000000]  memory used by lock dependency info: 3695 kB
> [    0.000000]  per task-struct memory footprint: 1152 bytes
> [    0.000845] Calibrating delay loop... 479.23 BogoMIPS (lpj=2396160)
> [    0.109860] pid_max: default: 32768 minimum: 301
> [    0.110128] Security Framework initialized
> [    0.110234] Mount-cache hash table entries: 512
> [    0.120373] CPU: Testing write buffer coherency: ok
> [    0.121628] Setting up static identity map for 0xc03caf60 - 0xc03cafb8
> [    0.125001] devtmpfs: initialized
> [    0.186744] pinctrl core: initialized pinctrl subsystem
> [    0.191942] regulator-dummy: no parameters
> [    0.194142] NET: Registered protocol family 16
> [    0.194891] DMA: preallocated 256 KiB pool for atomic coherent allocations
> [    0.213783] OMAP GPIO hardware version 0.1
> [    0.236730] omap-gpmc 50000000.gpmc: could not find pctldev for node /pinmux at 44e10800/gpmc_pins, deferring probe
> [    0.236781] platform 50000000.gpmc: Driver omap-gpmc requests probe deferral

This look like your problem. I would figure out why this is failing and
try again.

Cheers
Jon

[PATCH 11/14] ARM: OMAP2+: Add device-tree support for NOR flash

[Mark Jackson]

On 05/03/13 14:46, Jon Hunter wrote:
> 
> On 03/05/2013 08:34 AM, Mark Jackson wrote:
>> On 26/02/13 17:30, Jon Hunter wrote:
>>> NOR flash is not currently supported when booting with device-tree
>>> on OMAP2+ devices. Add support to detect and configure NOR devices
>>> when booting with device-tree.
>>>
>>> Add documentation for the TI GPMC NOR binding.
>>>
>>> Signed-off-by: Jon Hunter <jon-hunter@ti.com>
>>
>> I'm trying to test this series, and am unable get my NOR device recognised.
>>
>> If I remove all reference to NOR (and keep only my NAND device definition), then
>> everything seems to boot fine (so I'm assuming I've got at least the basics of
>> the patch set working).
>>
>> My GPMC tree looks like this:-

<snip>

>> 	nor at 1,0 {
>> 		reg = <3 0x00000000 0x04000000>;
>> 		compatible = "cfi-flash";
>> 		linux,mtd-name = "spansion,s29gl064n90t";
>> 		bank-width = <2>;
>>
>> 		gpmc,device-width = <1>;
> 
> Only bank-width should be necessary for NOR (per the binding
> documentation). However, if you do specify both, then they should match.
> Do you have two 8-bits devices? If so may be I need to update the
> documentation to make it clear this is the total width of all devices
> for a given chip-select.

No ... that was wrong, so I've fixed that.

<snip>

>> Booting with this NOR device produces the following oops:-
>>
>> [    0.000000] Booting Linux on physical CPU 0x0
>> [    0.000000] Linux version 3.9.0-rc1-12191-ga00d6d1-dirty (mpfj at mpfj-nanobone) (gcc version 4.5.4 (Buildroot 2012.11) ) #33 Tue Mar 5 13:08:25 GMT 2013
>> [    0.000000] CPU: ARMv7 Processor [413fc082] revision 2 (ARMv7), cr=10c53c7d

<snip>

>> [    0.236730] omap-gpmc 50000000.gpmc: could not find pctldev for node /pinmux at 44e10800/gpmc_pins, deferring probe
>> [    0.236781] platform 50000000.gpmc: Driver omap-gpmc requests probe deferral
> 
> This look like your problem. I would figure out why this is failing and
> try again.

Hmmmm ... I get this even when I've no NOR device defined and the board boots up fine.

But I can see in physmap_of.c that the device gets registered without any call to
devm_pinctrl_get_select_default() and hence no probe deferring takes place is the
pinctrl device hasn't yet been started (which it hasn't).

Does probe deferral need adding to physmap_of.c, or should the pinctrl device really
be registered sooner ?

Cheers
Mark J.

[PATCH 11/14] ARM: OMAP2+: Add device-tree support for NOR flash

[Jon Hunter]

On 03/05/2013 10:20 AM, Mark Jackson wrote:
> On 05/03/13 14:46, Jon Hunter wrote:
>>
>> On 03/05/2013 08:34 AM, Mark Jackson wrote:
>>> On 26/02/13 17:30, Jon Hunter wrote:
>>>> NOR flash is not currently supported when booting with device-tree
>>>> on OMAP2+ devices. Add support to detect and configure NOR devices
>>>> when booting with device-tree.
>>>>
>>>> Add documentation for the TI GPMC NOR binding.
>>>>
>>>> Signed-off-by: Jon Hunter <jon-hunter@ti.com>
>>>
>>> I'm trying to test this series, and am unable get my NOR device recognised.
>>>
>>> If I remove all reference to NOR (and keep only my NAND device definition), then
>>> everything seems to boot fine (so I'm assuming I've got at least the basics of
>>> the patch set working).
>>>
>>> My GPMC tree looks like this:-
> 
> <snip>
> 
>>> 	nor at 1,0 {
>>> 		reg = <3 0x00000000 0x04000000>;
>>> 		compatible = "cfi-flash";
>>> 		linux,mtd-name = "spansion,s29gl064n90t";
>>> 		bank-width = <2>;
>>>
>>> 		gpmc,device-width = <1>;
>>
>> Only bank-width should be necessary for NOR (per the binding
>> documentation). However, if you do specify both, then they should match.
>> Do you have two 8-bits devices? If so may be I need to update the
>> documentation to make it clear this is the total width of all devices
>> for a given chip-select.
> 
> No ... that was wrong, so I've fixed that.

Ok, I may update the documentation to make this clearer anyway.

> <snip>
> 
>>> Booting with this NOR device produces the following oops:-
>>>
>>> [    0.000000] Booting Linux on physical CPU 0x0
>>> [    0.000000] Linux version 3.9.0-rc1-12191-ga00d6d1-dirty (mpfj at mpfj-nanobone) (gcc version 4.5.4 (Buildroot 2012.11) ) #33 Tue Mar 5 13:08:25 GMT 2013
>>> [    0.000000] CPU: ARMv7 Processor [413fc082] revision 2 (ARMv7), cr=10c53c7d
> 
> <snip>
> 
>>> [    0.236730] omap-gpmc 50000000.gpmc: could not find pctldev for node /pinmux at 44e10800/gpmc_pins, deferring probe
>>> [    0.236781] platform 50000000.gpmc: Driver omap-gpmc requests probe deferral
>>
>> This look like your problem. I would figure out why this is failing and
>> try again.
> 
> Hmmmm ... I get this even when I've no NOR device defined and the board boots up fine.

So for NAND, the device does not get registered until the GPMC probe
completes. However, in the case of NOR, DT is registering the GPMC
and its child devices and so it is working slightly different in
this case.

> But I can see in physmap_of.c that the device gets registered without any call to
> devm_pinctrl_get_select_default() and hence no probe deferring takes place is the
> pinctrl device hasn't yet been started (which it hasn't).
>
> Does probe deferral need adding to physmap_of.c, or should the pinctrl device really
> be registered sooner ?

I see, so the pinctrl driver is not getting probed until later.

This really highlights a weakness in the GPMC driver, particularly
for NOR, where the child device can only be probed once the parent
is probed. I don't see this as being DT specific issue, because
even on older OMAP boards we always registered the NOR flash device
independently of the GPMC device. So we have always been susceptible to
this problem AFAICT.

This is admittedly a hack, but I am curious if you add the pinctrl
properties to the NOR node, if this would also defer the probe on the
NOR device?

Ideally it would be great to defer the probing of the child until the
parent has been probed successfully.

Cheers
Jon

[PATCH 11/14] ARM: OMAP2+: Add device-tree support for NOR flash

[Ezequiel Garcia]

Hi Jon,

On Tue, Mar 5, 2013 at 2:30 PM, Jon Hunter <jon-hunter@ti.com> wrote:
>
> This really highlights a weakness in the GPMC driver, particularly
> for NOR, where the child device can only be probed once the parent
> is probed. I don't see this as being DT specific issue, because
> even on older OMAP boards we always registered the NOR flash device
> independently of the GPMC device. So we have always been susceptible to
> this problem AFAICT.
>
> This is admittedly a hack, but I am curious if you add the pinctrl
> properties to the NOR node, if this would also defer the probe on the
> NOR device?
>
> Ideally it would be great to defer the probing of the child until the
> parent has been probed successfully.
>

This seems related to the probing parent-child ordering probing
I pointed out in a previous mail, isn't it?
(which by the way, I'll answer when I can gather some more convincing
feedback).

I don't have OMAP NOR boards, so I'm not entirely familiar to the
way GPMC registers the NOR device. However, by looking
into board-flash.c:board_nor_init() function, it seems to me that:

1. first we request the CS with gpmc_cs_request() and
2. later we register the NOR device explicitly with
    platform_device_register().

So, unless I'm missing something, we force the NOR device
to be probed *after* the GPMC device.

This is definitely the way NAND, OneNAND is handled.

On the otherside, by using 'simple-bus' you create your devices
first, when the whole device-tree is parsed and later the drivers
are probed at module_initcall time.

So, AFAIK, this problem is DT specific.

On the other side, when you say we should defer the probing
of the child. Do you mean changing something in physmap/NAND/etc. drivers?

Please keep in mind, I have nothing against using simple-bus,
since it's a very clean solution. It's just that it seems to me it will
be problematic.
Moreover, the fix shouldn't be too complicated (still working on this).

I'll try to post my Device Bus driver soon (similar to GPMC)
and also I'll try to answer the previous discussion with some
information on why I think this won't work.

(I hope I'm not making too much noise with this)
-- 
    Ezequiel

[PATCH 11/14] ARM: OMAP2+: Add device-tree support for NOR flash

[Jon Hunter]

On 03/05/2013 11:43 AM, Ezequiel Garcia wrote:
> Hi Jon,
> 
> On Tue, Mar 5, 2013 at 2:30 PM, Jon Hunter <jon-hunter@ti.com> wrote:
>>
>> This really highlights a weakness in the GPMC driver, particularly
>> for NOR, where the child device can only be probed once the parent
>> is probed. I don't see this as being DT specific issue, because
>> even on older OMAP boards we always registered the NOR flash device
>> independently of the GPMC device. So we have always been susceptible to
>> this problem AFAICT.
>>
>> This is admittedly a hack, but I am curious if you add the pinctrl
>> properties to the NOR node, if this would also defer the probe on the
>> NOR device?
>>
>> Ideally it would be great to defer the probing of the child until the
>> parent has been probed successfully.
>>
> 
> This seems related to the probing parent-child ordering probing
> I pointed out in a previous mail, isn't it?

Yes in this case. However, I have no idea what the problem you are
having is. So I cannot comment if your problem represents a real
scenario or not.

> (which by the way, I'll answer when I can gather some more convincing
> feedback).

Thanks.

> I don't have OMAP NOR boards, so I'm not entirely familiar to the
> way GPMC registers the NOR device. However, by looking
> into board-flash.c:board_nor_init() function, it seems to me that:
> 
> 1. first we request the CS with gpmc_cs_request() and
> 2. later we register the NOR device explicitly with
>     platform_device_register().
> 
> So, unless I'm missing something, we force the NOR device
> to be probed *after* the GPMC device.
>
> This is definitely the way NAND, OneNAND is handled.
> 
> On the otherside, by using 'simple-bus' you create your devices
> first, when the whole device-tree is parsed and later the drivers
> are probed at module_initcall time.
> 
> So, AFAIK, this problem is DT specific.

This assumes that the GPMC driver is registered (and probed) before 1
and 2 above occur. What is DT specific about when the driver is registered?

What happens in the non-DT case when the GPMC probe is deferred or fails?

We would never register the GPMC devices, right? For the deferred case
that is not so good. So the current implementation is flawed if the
probe is deferred. However, it would not crash which is a plus.

So my point is in Mark's example, if someone were to add pinctrl support
to the existing GPMC driver which caused the GPMC probe to be deferred
then none of the GPMC child devices would ever be registered. I don't
see that being related to DT either.

What you are also missing is that in the probe_nor() function, if we
fail to allocate the required resources, then I disable the child node
and the child is not registered and hence not probed. I have tested this
and for me this works. I was hoping this would be sufficient to handle
failure cases and avoid crashes.

> On the other side, when you say we should defer the probing
> of the child. Do you mean changing something in physmap/NAND/etc. drivers?

Not necessarily.

> Please keep in mind, I have nothing against using simple-bus,
> since it's a very clean solution. It's just that it seems to me it will
> be problematic.
> Moreover, the fix shouldn't be too complicated (still working on this).

I am interested to know what your approach will be. One alternative is
to call of_platform_device_create() from the GPMC driver. I could see
this as being a safe option.

> I'll try to post my Device Bus driver soon (similar to GPMC)
> and also I'll try to answer the previous discussion with some
> information on why I think this won't work.
> 
> (I hope I'm not making too much noise with this)

No it is all good inputs. I am just frustrated to be told that this
implementation is flawed without being able to tell me exactly why or
the scenario where it would not work.

The probe deferred case is a good example where this implementation will
have problems. I see that as legitimate. However, I have no idea if what
you are reporting is also legitimate or not. May be it is, I just don't
know.

Jon

[PATCH 11/14] ARM: OMAP2+: Add device-tree support for NOR flash

[Jon Hunter]

On 03/05/2013 11:30 AM, Jon Hunter wrote:
> On 03/05/2013 10:20 AM, Mark Jackson wrote:

[snip]

>> But I can see in physmap_of.c that the device gets registered without any call to
>> devm_pinctrl_get_select_default() and hence no probe deferring takes place is the
>> pinctrl device hasn't yet been started (which it hasn't).
>>
>> Does probe deferral need adding to physmap_of.c, or should the pinctrl device really
>> be registered sooner ?
> 
> I see, so the pinctrl driver is not getting probed until later.

Can you give this version of the patch a go? I have re-worked the patch
so the NOR device will only be registered after the GPMC probe completes.

By the way, with this version you should remove "simple-bus" from your
gpmc node compatible strings. I now call of_platform_device_create() to
create the child device during the GPMC probe. I think that this is a
safer approach.

Cheers
Jon

>From 278ef0ac538da720fc15896ad6c3666e0f518633 Mon Sep 17 00:00:00 2001
From: Jon Hunter <jon-hunter@ti.com>
Date: Fri, 8 Feb 2013 16:46:13 -0600
Subject: [PATCH] ARM: OMAP2+: Add device-tree support for NOR flash

NOR flash is not currently supported when booting with device-tree
on OMAP2+ devices. Add support to detect and configure NOR devices
when booting with device-tree.

Add documentation for the TI GPMC NOR binding.

Signed-off-by: Jon Hunter <jon-hunter@ti.com>
---
 Documentation/devicetree/bindings/mtd/gpmc-nor.txt |   98 +++++++++++++++++
 arch/arm/mach-omap2/gpmc.c                         |  113 ++++++++++++++++++++
 2 files changed, 211 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mtd/gpmc-nor.txt

diff --git a/Documentation/devicetree/bindings/mtd/gpmc-nor.txt b/Documentation/devicetree/bindings/mtd/gpmc-nor.txt
new file mode 100644
index 0000000..8c638fc
--- /dev/null
+++ b/Documentation/devicetree/bindings/mtd/gpmc-nor.txt
@@ -0,0 +1,98 @@
+Device tree bindings for NOR flash connect to TI GPMC
+
+NOR flash connected to the TI GPMC (found on OMAP boards) are represented as
+child nodes of the GPMC controller with a name of "nor".
+
+All timing relevant properties as well as generic GPMC child properties are
+explained in a separate documents. Please refer to
+Documentation/devicetree/bindings/bus/ti-gpmc.txt
+
+Required properties:
+- bank-width: 		Width of NOR flash in bytes. GPMC supports 8-bit and
+			16-bit devices and so must be either 1 or 2 bytes.
+- compatible:		Documentation/devicetree/bindings/mtd/mtd-physmap.txt
+- gpmc,cs-on:		Chip-select assertion time
+- gpmc,cs-rd-off:	Chip-select de-assertion time for reads
+- gpmc,cs-wr-off:	Chip-select de-assertion time for writes
+- gpmc,oe-on:		Output-enable assertion time
+- gpmc,oe-off		Output-enable de-assertion time
+- gpmc,we-on:		Write-enable assertion time
+- gpmc,we-off:		Write-enable de-assertion time
+- gpmc,access:		Start cycle to first data capture (read access)
+- gpmc,rd-cycle:	Total read cycle time
+- gpmc,wr-cycle:	Total write cycle time
+- linux,mtd-name:	Documentation/devicetree/bindings/mtd/mtd-physmap.txt
+- reg:			Chip-select, base address (relative to chip-select)
+			and size of NOR flash. Note that base address will be
+			typically 0 as this is the start of the chip-select.
+
+Optional properties:
+- gpmc,XXX		Additional GPMC timings and settings parameters. See
+			Documentation/devicetree/bindings/bus/ti-gpmc.txt
+
+Optional properties for partiton table parsing:
+- #address-cells: should be set to 1
+- #size-cells: should be set to 1
+
+Example:
+
+gpmc: gpmc at 6e000000 {
+	compatible = "ti,omap3430-gpmc", "simple-bus";
+	ti,hwmods = "gpmc";
+	reg = <0x6e000000 0x1000>;
+	interrupts = <20>;
+	gpmc,num-cs = <8>;
+	gpmc,num-waitpins = <4>;
+	#address-cells = <2>;
+	#size-cells = <1>;
+
+	ranges = <0 0 0x10000000 0x08000000>;
+
+	nor at 0,0 {
+		compatible = "cfi-flash";
+		linux,mtd-name= "intel,pf48f6000m0y1be";
+		#address-cells = <1>;
+		#size-cells = <1>;
+		reg = <0 0 0x08000000>;
+		bank-width = <2>;
+
+		gpmc,mux-add-data;
+		gpmc,cs-on = <0>;
+		gpmc,cs-rd-off = <186>;
+		gpmc,cs-wr-off = <186>;
+		gpmc,adv-on = <12>;
+		gpmc,adv-rd-off = <48>;
+		gpmc,adv-wr-off = <48>;
+		gpmc,oe-on = <54>;
+		gpmc,oe-off = <168>;
+		gpmc,we-on = <54>;
+		gpmc,we-off = <168>;
+		gpmc,rd-cycle = <186>;
+		gpmc,wr-cycle = <186>;
+		gpmc,access = <114>;
+		gpmc,page-burst-access = <6>;
+		gpmc,bus-turnaround = <12>;
+		gpmc,cycle2cycle-delay = <18>;
+		gpmc,wr-data-mux-bus = <90>;
+		gpmc,wr-access = <186>;
+		gpmc,cycle2cycle-samecsen;
+		gpmc,cycle2cycle-diffcsen;
+
+		partition at 0 {
+			label = "bootloader-nor";
+			reg = <0 0x40000>;
+		};
+		partition at 0x40000 {
+			label = "params-nor";
+			reg = <0x40000 0x40000>;
+		};
+		partition at 0x80000 {
+			label = "kernel-nor";
+			reg = <0x80000 0x200000>;
+		};
+		partition at 0x280000 {
+			label = "filesystem-nor";
+			reg = <0x240000 0x7d80000>;
+		};
+	};
+};
diff --git a/arch/arm/mach-omap2/gpmc.c b/arch/arm/mach-omap2/gpmc.c
index e822d2b..c217a38 100644
--- a/arch/arm/mach-omap2/gpmc.c
+++ b/arch/arm/mach-omap2/gpmc.c
@@ -26,6 +26,7 @@
 #include <linux/interrupt.h>
 #include <linux/platform_device.h>
 #include <linux/of.h>
+#include <linux/of_address.h>
 #include <linux/of_mtd.h>
 #include <linux/of_device.h>
 #include <linux/mtd/nand.h>
@@ -513,6 +514,37 @@ static int gpmc_cs_delete_mem(int cs)
 	return r;
 }
 
+/**
+ * gpmc_cs_remap - remaps a chip-select physical base address
+ * @cs:		chip-select to remap
+ * @base:	physical base address to re-map chip-select to
+ *
+ * Re-maps a chip-select to a new physical base address specified by
+ * "base". Returns 0 on success and appropriate negative error code
+ * on failure.
+ */
+static int gpmc_cs_remap(int cs, u32 base)
+{
+	int ret;
+	u32 old_base, size;
+
+	if (cs > GPMC_CS_NUM)
+		return -ENODEV;
+	gpmc_cs_get_memconf(cs, &old_base, &size);
+	if (base == old_base)
+		return 0;
+	gpmc_cs_disable_mem(cs);
+	ret = gpmc_cs_delete_mem(cs);
+	if (IS_ERR_VALUE(ret))
+		return ret;
+	ret = gpmc_cs_insert_mem(cs, base, size);
+	if (IS_ERR_VALUE(ret))
+		return ret;
+	gpmc_cs_enable_mem(cs, base, size);
+
+	return 0;
+}
+
 int gpmc_cs_request(int cs, unsigned long size, unsigned long *base)
 {
 	struct resource *res = &gpmc_cs_mem[cs];
@@ -1391,6 +1423,78 @@ static int gpmc_probe_onenand_child(struct platform_device *pdev,
 }
 #endif
 
+/**
+ * gpmc_probe_nor_child - configures the gpmc for a nor device
+ * @pdev:	pointer to gpmc platform device
+ * @child:	pointer to device-tree node for nor device
+ *
+ * Allocates and configures a GPMC chip-select for a NOR flash device.
+ * Returns 0 on success and appropriate negative error code on failure.
+ */
+static int gpmc_probe_nor_child(struct platform_device *pdev,
+				struct device_node *child)
+{
+	struct gpmc_settings gpmc_s;
+	struct gpmc_timings gpmc_t;
+	struct resource res;
+	unsigned long base;
+	int ret, cs;
+
+	if (of_property_read_u32(child, "reg", &cs) < 0) {
+		dev_err(&pdev->dev, "%s has no 'reg' property\n",
+			child->full_name);
+		return -ENODEV;
+	}
+
+	if (of_address_to_resource(child, 0, &res)) {
+		dev_err(&pdev->dev, "%s has malformed 'reg' property\n",
+			child->full_name);
+		return -ENODEV;
+	}
+
+	ret = gpmc_cs_request(cs, res.end - res.start, &base);
+	if (IS_ERR_VALUE(ret)) {
+		dev_err(&pdev->dev, "cannot request GPMC CS %d\n", cs);
+		return ret;
+	}
+
+	/*
+	 * FIXME: gpmc_cs_request() will map the CS to an arbitary
+	 * location in the gpmc address space. When booting with
+	 * device-tree we want the NOR flash to be mapped to the
+	 * location specified in the device-tree blob. So remap the
+	 * CS to this location. Once DT migration is complete should
+	 * just make gpmc_cs_request() map a specific address.
+	 */
+	ret = gpmc_cs_remap(cs, res.start);
+	if (IS_ERR_VALUE(ret)) {
+		dev_err(&pdev->dev, "cannot remap GPMC CS %d to 0x%x\n",
+			cs, res.start);
+		goto err1;
+	}
+
+	gpmc_read_settings_dt(child, &gpmc_s);
+
+	ret = of_property_read_u32(child, "bank-width", &gpmc_s.device_width);
+	if (IS_ERR_VALUE(ret))
+		goto err1;
+
+	ret = gpmc_cs_program_settings(cs, &gpmc_s);
+	if (IS_ERR_VALUE(ret))
+		goto err1;
+
+	gpmc_read_timings_dt(child, &gpmc_t);
+	gpmc_cs_set_timings(cs, &gpmc_t);
+
+	of_platform_device_create(child, NULL, &pdev->dev);
+
+	return 0;
+err1:
+	gpmc_cs_free(cs);
+
+	return ret;
+}
+
 static int gpmc_probe_dt(struct platform_device *pdev)
 {
 	int ret;
@@ -1424,6 +1528,15 @@ static int gpmc_probe_dt(struct platform_device *pdev)
 			return ret;
 		}
 	}
+
+	for_each_node_by_name(child, "nor") {
+		ret = gpmc_probe_nor_child(pdev, child);
+		if (ret < 0) {
+			of_node_put(child);
+			return ret;
+		}
+	}
+
 	return 0;
 }
 #else
-- 
1.7.10.4

[PATCH 11/14] ARM: OMAP2+: Add device-tree support for NOR flash

[Mark Jackson]

On 05/03/13 21:34, Jon Hunter wrote:
> 
> On 03/05/2013 11:30 AM, Jon Hunter wrote:
>> On 03/05/2013 10:20 AM, Mark Jackson wrote:
> 
> [snip]
> 
>>> But I can see in physmap_of.c that the device gets registered without any call to
>>> devm_pinctrl_get_select_default() and hence no probe deferring takes place is the
>>> pinctrl device hasn't yet been started (which it hasn't).
>>>
>>> Does probe deferral need adding to physmap_of.c, or should the pinctrl device really
>>> be registered sooner ?
>>
>> I see, so the pinctrl driver is not getting probed until later.
> 
> Can you give this version of the patch a go? I have re-worked the patch
> so the NOR device will only be registered after the GPMC probe completes.
> 
> By the way, with this version you should remove "simple-bus" from your
> gpmc node compatible strings. I now call of_platform_device_create() to
> create the child device during the GPMC probe. I think that this is a
> safer approach.

This is better in that the probe *is* now delayed until the gpmc has been setup,
but I still get an oops.

However, this time it's in the actual cfi query code.

I've traced it down to when it tries to physically access the memory associated
with the chip select in question (in this case CS3 @ 0x1a000000).

I printed some debug info to show that the GPMC CONFIG7 register is being setup
correctly, and I show the physical to virtual memory mapping values, as performed
in of_flash_probe(), but when I try to do a test raw_readw() on the virtual address,
I get the following:-

[    1.222950] omap-gpmc 50000000.gpmc: GPMC revision 6.0
[    1.229002] gpmc_read_settings_dt: read/write wait monitoring not enabled!
[    1.237916] enabling NAND BCH ecc with 8-bit correction
[    1.243843] ONFI param page 0 valid
[    1.247531] ONFI flash detected
[    1.250856] NAND device: Manufacturer ID: 0x2c, Chip ID: 0xda (Micron MT29F2G08ABAEAWP), 256MiB, page size: 2048, OOB size: 64
[    1.263149] 8 ofpart partitions found on MTD device omap2-nand.0
[    1.269492] Creating 8 MTD partitions on "omap2-nand.0":
[    1.275150] 0x000000000000-0x000000020000 : "spl1"
[    1.282593] 0x000000020000-0x000000040000 : "spl2"
[    1.288524] 0x000000040000-0x000000060000 : "spl3"
[    1.294456] 0x000000060000-0x000000080000 : "spl4"
[    1.300270] 0x000000080000-0x0000001e0000 : "boot"
[    1.307224] 0x0000001e0000-0x000000200000 : "env"
[    1.313093] 0x000000200000-0x000004200000 : "rootfs"
[    1.373589] 0x000004200000-0x000010000000 : "data"
[    1.541884] gpmc_probe_nor_child 1
[    1.545483] GPMC_CS_CONFIG7_0 : 00000f48
[    1.549621] GPMC_CS_CONFIG7_1 : 00000f58
[    1.553812] GPMC_CS_CONFIG7_2 : 00000f00
[    1.557951] GPMC_CS_CONFIG7_3 : 00000c5a
[    1.564789] of_flash_probe ioremap : phys 1a000000 -> virt d3000000
[    1.571468] of_flash_probe test read d3000000 ...
[    1.576440] Unhandled fault: external abort on non-linefetch (0x1028) at 0xd3000000
[    1.584525] Internal error: : 1028 [#1] ARM
[    1.588946] CPU: 0    Not tainted  (3.9.0-rc1-12191-ga00d6d1-dirty #57)
[    1.595943] PC is at of_flash_probe+0x22c/0x5dc
[    1.600724] LR is at of_flash_probe+0x228/0x5dc
[    1.605506] pc : [<c023b28c>]    lr : [<c023b288>]    psr: 20000113
[    1.605506] sp : cf077ba0  ip : cf06c080  fp : c0492150
[    1.617621] r10: 04000000  r9 : cf2ac6d0  r8 : 00000000
[    1.623135] r7 : cf2ac6d0  r6 : cf2b9010  r5 : cf2b9000  r4 : c0c81f60
[    1.630027] r3 : d3000000  r2 : 00000000  r1 : cf06c4d8  r0 : 00000025
[    1.636921] Flags: nzCv  IRQs on  FIQs on  Mode SVC_32  ISA ARM  Segment kernel
[    1.644635] Control: 10c5387d  Table: 80004019  DAC: 00000015
[    1.650703] Process kworker/u:0 (pid: 6, stack limit = 0xcf076238)
[    1.657225] Stack: (0xcf077ba0 to 0xcf078000)

Any ideas ?

Cheers
Mark J.

[PATCH 11/14] ARM: OMAP2+: Add device-tree support for NOR flash

[Mark Jackson]

On 06/03/13 10:23, Mark Jackson wrote:
> On 05/03/13 21:34, Jon Hunter wrote:
>>
>> On 03/05/2013 11:30 AM, Jon Hunter wrote:
>>> On 03/05/2013 10:20 AM, Mark Jackson wrote:
>>
>> [snip]
>>
>>>> But I can see in physmap_of.c that the device gets registered without any call to
>>>> devm_pinctrl_get_select_default() and hence no probe deferring takes place is the
>>>> pinctrl device hasn't yet been started (which it hasn't).
>>>>
>>>> Does probe deferral need adding to physmap_of.c, or should the pinctrl device really
>>>> be registered sooner ?
>>>
>>> I see, so the pinctrl driver is not getting probed until later.
>>
>> Can you give this version of the patch a go? I have re-worked the patch
>> so the NOR device will only be registered after the GPMC probe completes.
>>
>> By the way, with this version you should remove "simple-bus" from your
>> gpmc node compatible strings. I now call of_platform_device_create() to
>> create the child device during the GPMC probe. I think that this is a
>> safer approach.
> 
> This is better in that the probe *is* now delayed until the gpmc has been setup,
> but I still get an oops.
> 
> However, this time it's in the actual cfi query code.
> 
> I've traced it down to when it tries to physically access the memory associated
> with the chip select in question (in this case CS3 @ 0x1a000000).
> 
> I printed some debug info to show that the GPMC CONFIG7 register is being setup
> correctly, and I show the physical to virtual memory mapping values, as performed
> in of_flash_probe(), but when I try to do a test raw_readw() on the virtual address,
> I get the following:-
> 
> [    1.222950] omap-gpmc 50000000.gpmc: GPMC revision 6.0
> [    1.229002] gpmc_read_settings_dt: read/write wait monitoring not enabled!
> [    1.237916] enabling NAND BCH ecc with 8-bit correction
> [    1.243843] ONFI param page 0 valid
> [    1.247531] ONFI flash detected
> [    1.250856] NAND device: Manufacturer ID: 0x2c, Chip ID: 0xda (Micron MT29F2G08ABAEAWP), 256MiB, page size: 2048, OOB size: 64
> [    1.263149] 8 ofpart partitions found on MTD device omap2-nand.0
> [    1.269492] Creating 8 MTD partitions on "omap2-nand.0":
> [    1.275150] 0x000000000000-0x000000020000 : "spl1"
> [    1.282593] 0x000000020000-0x000000040000 : "spl2"
> [    1.288524] 0x000000040000-0x000000060000 : "spl3"
> [    1.294456] 0x000000060000-0x000000080000 : "spl4"
> [    1.300270] 0x000000080000-0x0000001e0000 : "boot"
> [    1.307224] 0x0000001e0000-0x000000200000 : "env"
> [    1.313093] 0x000000200000-0x000004200000 : "rootfs"
> [    1.373589] 0x000004200000-0x000010000000 : "data"
> [    1.541884] gpmc_probe_nor_child 1
> [    1.545483] GPMC_CS_CONFIG7_0 : 00000f48
> [    1.549621] GPMC_CS_CONFIG7_1 : 00000f58
> [    1.553812] GPMC_CS_CONFIG7_2 : 00000f00
> [    1.557951] GPMC_CS_CONFIG7_3 : 00000c5a

0x00000c5a is an invalid mode !!

I'm trying to use a 64MB address space but not on a 64MB boundary ... oops.

[PATCH 11/14] ARM: OMAP2+: Add device-tree support for NOR flash

[Jon Hunter]

On 03/06/2013 07:30 AM, Mark Jackson wrote:
> On 06/03/13 10:23, Mark Jackson wrote:
>> On 05/03/13 21:34, Jon Hunter wrote:
>>>
>>> On 03/05/2013 11:30 AM, Jon Hunter wrote:
>>>> On 03/05/2013 10:20 AM, Mark Jackson wrote:
>>>
>>> [snip]
>>>
>>>>> But I can see in physmap_of.c that the device gets registered without any call to
>>>>> devm_pinctrl_get_select_default() and hence no probe deferring takes place is the
>>>>> pinctrl device hasn't yet been started (which it hasn't).
>>>>>
>>>>> Does probe deferral need adding to physmap_of.c, or should the pinctrl device really
>>>>> be registered sooner ?
>>>>
>>>> I see, so the pinctrl driver is not getting probed until later.
>>>
>>> Can you give this version of the patch a go? I have re-worked the patch
>>> so the NOR device will only be registered after the GPMC probe completes.
>>>
>>> By the way, with this version you should remove "simple-bus" from your
>>> gpmc node compatible strings. I now call of_platform_device_create() to
>>> create the child device during the GPMC probe. I think that this is a
>>> safer approach.
>>
>> This is better in that the probe *is* now delayed until the gpmc has been setup,
>> but I still get an oops.
>>
>> However, this time it's in the actual cfi query code.
>>
>> I've traced it down to when it tries to physically access the memory associated
>> with the chip select in question (in this case CS3 @ 0x1a000000).
>>
>> I printed some debug info to show that the GPMC CONFIG7 register is being setup
>> correctly, and I show the physical to virtual memory mapping values, as performed
>> in of_flash_probe(), but when I try to do a test raw_readw() on the virtual address,
>> I get the following:-
>>
>> [    1.222950] omap-gpmc 50000000.gpmc: GPMC revision 6.0
>> [    1.229002] gpmc_read_settings_dt: read/write wait monitoring not enabled!
>> [    1.237916] enabling NAND BCH ecc with 8-bit correction
>> [    1.243843] ONFI param page 0 valid
>> [    1.247531] ONFI flash detected
>> [    1.250856] NAND device: Manufacturer ID: 0x2c, Chip ID: 0xda (Micron MT29F2G08ABAEAWP), 256MiB, page size: 2048, OOB size: 64
>> [    1.263149] 8 ofpart partitions found on MTD device omap2-nand.0
>> [    1.269492] Creating 8 MTD partitions on "omap2-nand.0":
>> [    1.275150] 0x000000000000-0x000000020000 : "spl1"
>> [    1.282593] 0x000000020000-0x000000040000 : "spl2"
>> [    1.288524] 0x000000040000-0x000000060000 : "spl3"
>> [    1.294456] 0x000000060000-0x000000080000 : "spl4"
>> [    1.300270] 0x000000080000-0x0000001e0000 : "boot"
>> [    1.307224] 0x0000001e0000-0x000000200000 : "env"
>> [    1.313093] 0x000000200000-0x000004200000 : "rootfs"
>> [    1.373589] 0x000004200000-0x000010000000 : "data"
>> [    1.541884] gpmc_probe_nor_child 1
>> [    1.545483] GPMC_CS_CONFIG7_0 : 00000f48
>> [    1.549621] GPMC_CS_CONFIG7_1 : 00000f58
>> [    1.553812] GPMC_CS_CONFIG7_2 : 00000f00
>> [    1.557951] GPMC_CS_CONFIG7_3 : 00000c5a
> 
> 0x00000c5a is an invalid mode !!
> 
> I'm trying to use a 64MB address space but not on a 64MB boundary ... oops.

Good catch. So this is now working for you then?

Thanks
Jon

[PATCH 11/14] ARM: OMAP2+: Add device-tree support for NOR flash

[Mark Jackson]

On 06/03/13 16:44, Jon Hunter wrote:
> 
> On 03/06/2013 07:30 AM, Mark Jackson wrote:
>> On 06/03/13 10:23, Mark Jackson wrote:

<snip>

>>> [    1.541884] gpmc_probe_nor_child 1
>>> [    1.545483] GPMC_CS_CONFIG7_0 : 00000f48
>>> [    1.549621] GPMC_CS_CONFIG7_1 : 00000f58
>>> [    1.553812] GPMC_CS_CONFIG7_2 : 00000f00
>>> [    1.557951] GPMC_CS_CONFIG7_3 : 00000c5a
>>
>> 0x00000c5a is an invalid mode !!
>>
>> I'm trying to use a 64MB address space but not on a 64MB boundary ... oops.
> 
> Good catch. So this is now working for you then?

Not yet ... I got distracted by something else !?!

I'll take another look tomorrow.

Do you think it might be worth adding some sanity checking to the cs config
routines to trap similar errors ?

Cheers
Mark J.

[PATCH 11/14] ARM: OMAP2+: Add device-tree support for NOR flash

[Jon Hunter]

On 03/06/2013 10:48 AM, Mark Jackson wrote:
> On 06/03/13 16:44, Jon Hunter wrote:
>>
>> On 03/06/2013 07:30 AM, Mark Jackson wrote:
>>> On 06/03/13 10:23, Mark Jackson wrote:
> 
> <snip>
> 
>>>> [    1.541884] gpmc_probe_nor_child 1
>>>> [    1.545483] GPMC_CS_CONFIG7_0 : 00000f48
>>>> [    1.549621] GPMC_CS_CONFIG7_1 : 00000f58
>>>> [    1.553812] GPMC_CS_CONFIG7_2 : 00000f00
>>>> [    1.557951] GPMC_CS_CONFIG7_3 : 00000c5a
>>>
>>> 0x00000c5a is an invalid mode !!
>>>
>>> I'm trying to use a 64MB address space but not on a 64MB boundary ... oops.
>>
>> Good catch. So this is now working for you then?
> 
> Not yet ... I got distracted by something else !?!
> 
> I'll take another look tomorrow.
> 
> Do you think it might be worth adding some sanity checking to the cs config
> routines to trap similar errors ?

Yes, I see what you mean. We should check to ensure that the the base is
aligned on a boundary that matches the size being configured.

I can add some checking for this case.

Cheers
Jon

[PATCH 11/14] ARM: OMAP2+: Add device-tree support for NOR flash

[Jon Hunter]

On 03/06/2013 11:00 AM, Jon Hunter wrote:
> 
> On 03/06/2013 10:48 AM, Mark Jackson wrote:
>> On 06/03/13 16:44, Jon Hunter wrote:
>>>
>>> On 03/06/2013 07:30 AM, Mark Jackson wrote:
>>>> On 06/03/13 10:23, Mark Jackson wrote:
>>
>> <snip>
>>
>>>>> [    1.541884] gpmc_probe_nor_child 1
>>>>> [    1.545483] GPMC_CS_CONFIG7_0 : 00000f48
>>>>> [    1.549621] GPMC_CS_CONFIG7_1 : 00000f58
>>>>> [    1.553812] GPMC_CS_CONFIG7_2 : 00000f00
>>>>> [    1.557951] GPMC_CS_CONFIG7_3 : 00000c5a
>>>>
>>>> 0x00000c5a is an invalid mode !!
>>>>
>>>> I'm trying to use a 64MB address space but not on a 64MB boundary ... oops.
>>>
>>> Good catch. So this is now working for you then?
>>
>> Not yet ... I got distracted by something else !?!
>>
>> I'll take another look tomorrow.
>>
>> Do you think it might be worth adding some sanity checking to the cs config
>> routines to trap similar errors ?
> 
> Yes, I see what you mean. We should check to ensure that the the base is
> aligned on a boundary that matches the size being configured.
> 
> I can add some checking for this case.

How about something like ...

Cheers
Jon

>From 4d6396ef49e34b06085a4036b795ab6faf29509c Mon Sep 17 00:00:00 2001
From: Jon Hunter <jon-hunter@ti.com>
Date: Wed, 6 Mar 2013 12:00:10 -0600
Subject: [PATCH] ARM: OMAP2+: Detect incorrectly aligned GPMC base address

---
 arch/arm/mach-omap2/gpmc.c |   22 +++++++++++++++++++---
 1 file changed, 19 insertions(+), 3 deletions(-)

diff --git a/arch/arm/mach-omap2/gpmc.c b/arch/arm/mach-omap2/gpmc.c
index 8e6f019..aeef0a2 100644
--- a/arch/arm/mach-omap2/gpmc.c
+++ b/arch/arm/mach-omap2/gpmc.c
@@ -410,11 +410,18 @@ int gpmc_cs_set_timings(int cs, const struct gpmc_timings *t)
 	return 0;
 }
 
-static void gpmc_cs_enable_mem(int cs, u32 base, u32 size)
+static int gpmc_cs_enable_mem(int cs, u32 base, u32 size)
 {
 	u32 l;
 	u32 mask;
 
+	/*
+	 * Ensure that base address is aligned
+	 * on a boundary greater than size.
+	 */
+	if (base & (size - 1))
+		return -EINVAL;
+
 	mask = (1 << GPMC_SECTION_SHIFT) - size;
 	l = gpmc_cs_read_reg(cs, GPMC_CS_CONFIG7);
 	l &= ~0x3f;
@@ -423,6 +430,8 @@ static void gpmc_cs_enable_mem(int cs, u32 base, u32 size)
 	l |= ((mask >> GPMC_CHUNK_SHIFT) & 0x0f) << 8;
 	l |= GPMC_CONFIG7_CSVALID;
 	gpmc_cs_write_reg(cs, GPMC_CS_CONFIG7, l);
+
+	return 0;
 }
 
 static void gpmc_cs_disable_mem(int cs)
@@ -541,7 +550,9 @@ static int gpmc_cs_remap(int cs, u32 base)
 	ret = gpmc_cs_insert_mem(cs, base, size);
 	if (IS_ERR_VALUE(ret))
 		return ret;
-	gpmc_cs_enable_mem(cs, base, size);
+	ret = gpmc_cs_enable_mem(cs, base, size);
+	if (IS_ERR_VALUE(ret))
+		return ret;
 
 	return 0;
 }
@@ -571,7 +582,12 @@ int gpmc_cs_request(int cs, unsigned long size, unsigned long *base)
 	if (r < 0)
 		goto out;
 
-	gpmc_cs_enable_mem(cs, res->start, resource_size(res));
+	r = gpmc_cs_enable_mem(cs, res->start, resource_size(res));
+	if (IS_ERR_VALUE(r)) {
+		release_resource(res);
+		goto out;
+	}
+
 	*base = res->start;
 	gpmc_cs_set_reserved(cs, 1);
 out:
-- 
1.7.10.4

[PATCH 11/14] ARM: OMAP2+: Add device-tree support for NOR flash

[Ezequiel Garcia]

Jon,

On Tue, Mar 5, 2013 at 6:34 PM, Jon Hunter <jon-hunter@ti.com> wrote:
>
> By the way, with this version you should remove "simple-bus" from your
> gpmc node compatible strings. I now call of_platform_device_create() to
> create the child device during the GPMC probe. I think that this is a
> safer approach.
>

FWIW, using of_platform_device_create() works for my driver.
It seems as simple as using a compatible 'simple-bus',
with the advantage that it allows to create and probe the
child device from its parent.

I'll submit this driver by the end of this week, in case you want
to take a look at it.

Thanks a lot,
-- 
    Ezequiel

[PATCH 11/14] ARM: OMAP2+: Add device-tree support for NOR flash

[Jon Hunter]

On 03/06/2013 05:58 AM, Ezequiel Garcia wrote:
> Jon,
> 
> On Tue, Mar 5, 2013 at 6:34 PM, Jon Hunter <jon-hunter@ti.com> wrote:
>>
>> By the way, with this version you should remove "simple-bus" from your
>> gpmc node compatible strings. I now call of_platform_device_create() to
>> create the child device during the GPMC probe. I think that this is a
>> safer approach.
>>
> 
> FWIW, using of_platform_device_create() works for my driver.
> It seems as simple as using a compatible 'simple-bus',
> with the advantage that it allows to create and probe the
> child device from its parent.

Yes and I think that is safer for these type of drivers.

> I'll submit this driver by the end of this week, in case you want
> to take a look at it.

Ok, but this is not for omap right?

Cheers
Jon

[PATCH 11/14] ARM: OMAP2+: Add device-tree support for NOR flash

[Ezequiel Garcia]

On Wed, Mar 6, 2013 at 1:46 PM, Jon Hunter <jon-hunter@ti.com> wrote:
>
> On 03/06/2013 05:58 AM, Ezequiel Garcia wrote:
>> I'll submit this driver by the end of this week, in case you want
>> to take a look at it.
>
> Ok, but this is not for omap right?
>

No, this is a new driver for a memory controller that's available on
some Marvell SoC. It will be located in drivers/memory/.

I don't have any omap boards with a NOR device to test this,
but the discussion and the patches were really helpful!

Thanks,
-- 
    Ezequiel

[PATCH 11/14] ARM: OMAP2+: Add device-tree support for NOR flash

[Ezequiel Garcia]

On Wed, Mar 6, 2013 at 1:54 PM, Ezequiel Garcia <elezegarcia@gmail.com> wrote:
> On Wed, Mar 6, 2013 at 1:46 PM, Jon Hunter <jon-hunter@ti.com> wrote:
>>
>> On 03/06/2013 05:58 AM, Ezequiel Garcia wrote:
>>> I'll submit this driver by the end of this week, in case you want
>>> to take a look at it.
>>
>> Ok, but this is not for omap right?
>>
>
> No, this is a new driver for a memory controller that's available on
> some Marvell SoC. It will be located in drivers/memory/.
>

Slightly OT: if anyone wants to see the memory-controller driver
for mvebu platforms, I just posted in lakml.

It's this patchset:

"[PATCH 0/5] Device Bus support for Marvell EBU SoC"

-- 
    Ezequiel

[PATCH 12/14] ARM: OMAP2+: Add additional GPMC timing parameters

[Jon Hunter]

Some of the GPMC timings parameters are currently missing from the GPMC
device-tree binding. Add these parameters to the binding documentation
as well as code to read them.

Signed-off-by: Jon Hunter <jon-hunter@ti.com>
---
 Documentation/devicetree/bindings/bus/ti-gpmc.txt |   25 +++++++++++++++---
 arch/arm/mach-omap2/gpmc.c                        |   28 +++++++++++++++++++++
 2 files changed, 49 insertions(+), 4 deletions(-)

diff --git a/Documentation/devicetree/bindings/bus/ti-gpmc.txt b/Documentation/devicetree/bindings/bus/ti-gpmc.txt
index a31c32a..53338cf 100644
--- a/Documentation/devicetree/bindings/bus/ti-gpmc.txt
+++ b/Documentation/devicetree/bindings/bus/ti-gpmc.txt
@@ -56,10 +56,27 @@ Timing properties for child nodes. All are optional and default to 0.
  - gpmc,oe-off:		Deassertion time
 
  Access time and cycle time timings corresponding to GPMC_CONFIG5:
- - gpmc,page-burst-access: Multiple access word delay
- - gpmc,access:		Start-cycle to first data valid delay
- - gpmc,rd-cycle:	Total read cycle time
- - gpmc,wr-cycle:	Total write cycle time
+ - gpmc,page-burst-access: 	Multiple access word delay
+ - gpmc,access:			Start-cycle to first data valid delay
+ - gpmc,rd-cycle:		Total read cycle time
+ - gpmc,wr-cycle:		Total write cycle time
+ - gpmc,bus-turnaround:		Turn-around time between successive accesses
+ - gpmc,cycle2cycle-delay:	Delay between chip-select pulses
+ - gpmc,clk-activation: 	GPMC clock activation time
+ - gpmc,wait-monitoring:	Start of wait monitoring with regard to valid
+				data
+
+Boolean timing parameters. If property is present parameter enabled and
+disabled if omitted:
+ - gpmc,adv-extra-delay:	ADV signal is delayed by half GPMC clock
+ - gpmc,cs-extra-delay:		CS signal is delayed by half GPMC clock
+ - gpmc,cycle2cycle-diffcsen:	Add "cycle2cycle-delay" between successive
+				accesses to a different CS
+ - gpmc,cycle2cycle-samecsen:	Add "cycle2cycle-delay" between successive
+				accesses to the same CS
+ - gpmc,oe-extra-delay:		OE signal is delayed by half GPMC clock
+ - gpmc,we-extra-delay:		WE signal is delayed by half GPMC clock
+ - gpmc,time-para-granularity:	Multiply all access times by 2
 
 The following are only applicable to OMAP3+ and AM335x:
  - gpmc,wr-access
diff --git a/arch/arm/mach-omap2/gpmc.c b/arch/arm/mach-omap2/gpmc.c
index eb167b3..78d6c51 100644
--- a/arch/arm/mach-omap2/gpmc.c
+++ b/arch/arm/mach-omap2/gpmc.c
@@ -1328,12 +1328,40 @@ static void __maybe_unused gpmc_read_timings_dt(struct device_node *np,
 	if (!of_property_read_u32(np, "gpmc,wr-cycle", &val))
 		gpmc_t->wr_cycle = val;
 
+	if (!of_property_read_u32(np, "gpmc,bus-turnaround", &val))
+		gpmc_t->bus_turnaround = val;
+
+	if (!of_property_read_u32(np, "gpmc,cycle2cycle-delay", &val))
+		gpmc_t->cycle2cycle_delay = val;
+
+	if (!of_property_read_u32(np, "gpmc,wait-monitoring", &val))
+		gpmc_t->wait_monitoring = val;
+
+	if (!of_property_read_u32(np, "gpmc,clk-activation", &val))
+		gpmc_t->clk_activation = val;
+
 	/* only for OMAP3430 */
 	if (!of_property_read_u32(np, "gpmc,wr-access", &val))
 		gpmc_t->wr_access = val;
 
 	if (!of_property_read_u32(np, "gpmc,wr-data-mux-bus", &val))
 		gpmc_t->wr_data_mux_bus = val;
+
+	/* bool timing parameters */
+	if (of_find_property(np, "gpmc,cycle2cycle-diffcsen", NULL))
+		gpmc_t->bool_timings.cycle2cyclediffcsen = true;
+	if (of_find_property(np, "gpmc,cycle2cycle-samecsen", NULL))
+		gpmc_t->bool_timings.cycle2cyclesamecsen = true;
+	if (of_find_property(np, "gpmc,we-extra-delay", NULL))
+		gpmc_t->bool_timings.we_extra_delay = true;
+	if (of_find_property(np, "gpmc,oe-extra-delay", NULL))
+		gpmc_t->bool_timings.oe_extra_delay = true;
+	if (of_find_property(np, "gpmc,adv-extra-delay", NULL))
+		gpmc_t->bool_timings.adv_extra_delay = true;
+	if (of_find_property(np, "gpmc,cs-extra-delay", NULL))
+		gpmc_t->bool_timings.cs_extra_delay = true;
+	if (of_find_property(np, "gpmc,time-para-granularity", NULL))
+		gpmc_t->bool_timings.time_para_granularity = true;
 }
 
 #ifdef CONFIG_MTD_NAND
-- 
1.7.10.4

[PATCH 12/14] ARM: OMAP2+: Add additional GPMC timing parameters

[Ezequiel Garcia]

Hi Jon,

On Tue, Feb 26, 2013 at 2:30 PM, Jon Hunter <jon-hunter@ti.com> wrote:
[...]
>
> +       if (!of_property_read_u32(np, "gpmc,bus-turnaround", &val))
> +               gpmc_t->bus_turnaround = val;
> +

AFAIK, you don't need to test if the property is defined.
You can just read it straightforward, and if the property is not defined
then nothing will be stored in your variable:

of_property_read_u32(np, "gpmc,bus-turnaround", &gpmc_t->bus_turnaround);

You can simplify *a lot* your patch with this, and I believe you could
also clean
existent code.

-- 
    Ezequiel

[PATCH 12/14] ARM: OMAP2+: Add additional GPMC timing parameters

[Ezequiel Garcia]

On Fri, Mar 1, 2013 at 5:11 PM, Ezequiel Garcia <elezegarcia@gmail.com> wrote:
> Hi Jon,
>
> On Tue, Feb 26, 2013 at 2:30 PM, Jon Hunter <jon-hunter@ti.com> wrote:
> [...]
>>
>> +       if (!of_property_read_u32(np, "gpmc,bus-turnaround", &val))
>> +               gpmc_t->bus_turnaround = val;
>> +
>
> AFAIK, you don't need to test if the property is defined.
> You can just read it straightforward, and if the property is not defined
> then nothing will be stored in your variable:
>
> of_property_read_u32(np, "gpmc,bus-turnaround", &gpmc_t->bus_turnaround);
>
> You can simplify *a lot* your patch with this, and I believe you could
> also clean
> existent code.
>

Also, since this is somewhat large patchset: would you mind pushing it somewhere
so I can just pull a branch?

Thanks,
-- 
    Ezequiel

[PATCH 12/14] ARM: OMAP2+: Add additional GPMC timing parameters

[Jon Hunter]

On 03/01/2013 02:12 PM, Ezequiel Garcia wrote:
> On Fri, Mar 1, 2013 at 5:11 PM, Ezequiel Garcia <elezegarcia@gmail.com> wrote:
>> Hi Jon,
>>
>> On Tue, Feb 26, 2013 at 2:30 PM, Jon Hunter <jon-hunter@ti.com> wrote:
>> [...]
>>>
>>> +       if (!of_property_read_u32(np, "gpmc,bus-turnaround", &val))
>>> +               gpmc_t->bus_turnaround = val;
>>> +
>>
>> AFAIK, you don't need to test if the property is defined.
>> You can just read it straightforward, and if the property is not defined
>> then nothing will be stored in your variable:
>>
>> of_property_read_u32(np, "gpmc,bus-turnaround", &gpmc_t->bus_turnaround);
>>
>> You can simplify *a lot* your patch with this, and I believe you could
>> also clean
>> existent code.
>>
> 
> Also, since this is somewhat large patchset: would you mind pushing it somewhere
> so I can just pull a branch?

See link in cover-letter ;-)

Jon

[PATCH 12/14] ARM: OMAP2+: Add additional GPMC timing parameters

[Jon Hunter]

On 03/01/2013 02:11 PM, Ezequiel Garcia wrote:
> Hi Jon,
> 
> On Tue, Feb 26, 2013 at 2:30 PM, Jon Hunter <jon-hunter@ti.com> wrote:
> [...]
>>
>> +       if (!of_property_read_u32(np, "gpmc,bus-turnaround", &val))
>> +               gpmc_t->bus_turnaround = val;
>> +
> 
> AFAIK, you don't need to test if the property is defined.
> You can just read it straightforward, and if the property is not defined
> then nothing will be stored in your variable:
> 
> of_property_read_u32(np, "gpmc,bus-turnaround", &gpmc_t->bus_turnaround);
> 
> You can simplify *a lot* your patch with this, and I believe you could
> also clean
> existent code.

True. I was just being consistent with the existing code in
gpmc_read_timings_dt(). May be I can clean-up the entire function.

Cheers
Jon

[PATCH 13/14] ARM: OMAP2+: Convert NAND to retrieve GPMC settings from DT

[Jon Hunter]

When booting with device-tree, retrieve GPMC settings for NAND from
the device-tree blob. This will allow us to remove all static settings
stored in the gpmc-nand.c in the future once the migration to
device-tree is complete.

Signed-off-by: Jon Hunter <jon-hunter@ti.com>
---
 arch/arm/mach-omap2/gpmc-nand.c |    7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/arch/arm/mach-omap2/gpmc-nand.c b/arch/arm/mach-omap2/gpmc-nand.c
index a4c8537..e3d4b72 100644
--- a/arch/arm/mach-omap2/gpmc-nand.c
+++ b/arch/arm/mach-omap2/gpmc-nand.c
@@ -137,8 +137,11 @@ int gpmc_nand_init(struct omap_nand_platform_data *gpmc_nand_data,
 			return err;
 		}
 
-		/* Enable RD PIN Monitoring Reg */
-		if (gpmc_nand_data->dev_ready) {
+		if (gpmc_nand_data->of_node) {
+			gpmc_read_settings_dt(gpmc_nand_data->of_node,
+					      &nand_settings);
+		} else if (gpmc_nand_data->dev_ready) {
+			/* Enable RD PIN Monitoring Reg */
 			nand_settings.wait_on_read = true;
 			nand_settings.wait_on_write = true;
 		}
-- 
1.7.10.4

[PATCH 14/14] ARM: OMAP2+: Convert ONENAND to retrieve GPMC settings from DT

[Jon Hunter]

When booting with device-tree, retrieve GPMC settings for ONENAND from
the device-tree blob. This will allow us to remove all static settings
stored in the gpmc-nand.c in the future once the migration to
device-tree is complete.

The user must now specify the ONENAND device width in the device-tree
binding so that the GPMC can be programmed correctly. Therefore, update
the device-tree binding documentation for ONENAND devices connected to
the GPMC to reflect this.

Please note that this does not include GPMC timings for ONENAND. The
timings are being calculated at runtime.

There is some legacy code that only enables read wait monitoring for
non-OMAP3 devices. There are no known OMAP3 device issues that prevent
this feature being enabled and so when booting with device-tree use the
wait-monitoring settings described in the device-tree blob.

Signed-off-by: Jon Hunter <jon-hunter@ti.com>
---
 .../devicetree/bindings/mtd/gpmc-onenand.txt       |    3 +++
 arch/arm/mach-omap2/gpmc-onenand.c                 |   21 ++++++++++++++------
 2 files changed, 18 insertions(+), 6 deletions(-)

diff --git a/Documentation/devicetree/bindings/mtd/gpmc-onenand.txt b/Documentation/devicetree/bindings/mtd/gpmc-onenand.txt
index deec9da..b752942 100644
--- a/Documentation/devicetree/bindings/mtd/gpmc-onenand.txt
+++ b/Documentation/devicetree/bindings/mtd/gpmc-onenand.txt
@@ -10,6 +10,8 @@ Documentation/devicetree/bindings/bus/ti-gpmc.txt
 Required properties:
 
  - reg:			The CS line the peripheral is connected to
+ - gpmc,device-width	Width of the ONENAND device connected to the GPMC
+			in bytes. Must be 1 or 2.
 
 Optional properties:
 
@@ -34,6 +36,7 @@ Example for an OMAP3430 board:
 
 		onenand at 0 {
 			reg = <0 0 0>; /* CS0, offset 0 */
+			gpmc,device-width = <2>;
 
 			#address-cells = <1>;
 			#size-cells = <1>;
diff --git a/arch/arm/mach-omap2/gpmc-onenand.c b/arch/arm/mach-omap2/gpmc-onenand.c
index 1e2191c..ade8ca0 100644
--- a/arch/arm/mach-omap2/gpmc-onenand.c
+++ b/arch/arm/mach-omap2/gpmc-onenand.c
@@ -272,6 +272,10 @@ static int omap2_onenand_setup_async(void __iomem *onenand_base)
 	struct gpmc_timings t;
 	int ret;
 
+	if (gpmc_onenand_data->of_node)
+		gpmc_read_settings_dt(gpmc_onenand_data->of_node,
+				      &onenand_async);
+
 	omap2_onenand_set_async_mode(onenand_base);
 
 	omap2_onenand_calc_async_timings(&t);
@@ -300,12 +304,17 @@ static int omap2_onenand_setup_sync(void __iomem *onenand_base, int *freq_ptr)
 		set_onenand_cfg(onenand_base);
 	}
 
-	/*
-	 * FIXME: Appears to be legacy code from initial ONENAND commit.
-	 * Unclear what boards this is for and if this can be removed.
-	 */
-	if (!cpu_is_omap34xx())
-		onenand_sync.wait_on_read = true;
+	if (gpmc_onenand_data->of_node) {
+		gpmc_read_settings_dt(gpmc_onenand_data->of_node,
+				      &onenand_sync);
+	} else {
+		/*
+		 * FIXME: Appears to be legacy code from initial ONENAND commit.
+		 * Unclear what boards this is for and if this can be removed.
+		 */
+		if (!cpu_is_omap34xx())
+			onenand_sync.wait_on_read = true;
+	}
 
 	omap2_onenand_calc_sync_timings(&t, gpmc_onenand_data->flags, freq);
 
-- 
1.7.10.4