linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [RFC PATCH 00/16] OMAP: GPMC: Restructure OMAP GPMC driver (NAND)
@ 2014-05-21 11:20 Roger Quadros
  2014-05-21 11:20 ` [RFC PATCH 01/16] ARM: OMAP2+: gpmc: Add platform data Roger Quadros
                   ` (16 more replies)
  0 siblings, 17 replies; 28+ messages in thread
From: Roger Quadros @ 2014-05-21 11:20 UTC (permalink / raw)
  To: tony, computersforpeace
  Cc: pekon, ezequiel.garcia, robertcnelson, jg1.han, dwmw2, javier,
	nsekhar, linux-omap, linux-mtd, devicetree, linux-kernel,
	Roger Quadros

Hi,

The existing OMAP GPMC driver has been suffering from ad-hoc architecture
with no clear separation between GPMC driver, Chip Select timing/settings and
Memory Device driver. The device tree implementation also reflects the same
issues.

The purpose of this series is to clean up the OMAP GPMC driver architecture,
provide a cleaner device tree implementation and finally move the gpmc driver
out of arch/arm/mach-omap2 so that it can be used by other non-OMAP
TI platforms.

This series tries to solve the problem as follows

For non-DT boot:
- Create a GPMC platform data structure that holds chip select
  information as well as child platform device
- Board files will build this structure using a helper function
  gpmc_generic_init()
- GPMC driver will configure the chip select region and create the
  platform device for each chip select region. Introduce gpmc_probe_legacy()
  for this purpose.

For DT boot:
- The GPMC controller node should have a chip select (CS) node for each used
  chip select. The CS node must have a child device node for each device
  attached to that chip select. Properties for that child are GPMC agnostic.

  i.e.
	gpmc {
		cs0 {
			nand0 {
			}
		};

		cs1 {
			nor0 {
			}
		}
		...
	};

- The meaning of ranges and how address mapping is done has changed.
  Ranges should now contain 2 ranges
  - GPMC I/O map. This map will be partitioned among the
    chip select (CS) nodes.
  - GPMC register map. This is common for all the CS nodes.
    
- Chip select (CS) number is no longer specified via reg property. Instead
  it is specified via the gpmc,cs property in the CS node.

For NAND:
- Move interrupt handling from GPMC driver to NAND driver. GPMC driver has nothing to do
  with interrupts.
- Expose GPMC register space to NAND driver. Both GPMC and NAND drivers share the same
  register space but don't use the same registers so it is safe. A clear partitioning couldn't
  be done since the GPMC config registers are interleaved with the NAND registers.
- Add compatible id. Move NAND DT parsing code from GPMC to NAND driver.

For now the series only addresses the OMAP NAND driver. I'm working at the moment
to fix other GPMC tied drivers as well (onenand, smc91x, smsc91xx, usb-tusb6010).

Tested on:
- omap3-beagle

NOTE: This series is only for review and most likely breaks everything apart from NAND on beagle board.
I hope to get review comments early so that I can avoid any major rework.

Thanks.

--
cheers,
-roger

Roger Quadros (16):
  ARM: OMAP2+: gpmc: Add platform data
  ARM: OMAP2+: gpmc: Add gpmc timings and settings to platform data
  ARM: OMAP2+: gmpc: add gpmc_generic_init()
  ARM: OMAP2+: gpmc: use platform data to configure CS space and
    poplulate device
  ARM: OMAP2+: gpmc: Use low level read/write for context save/restore
  ARM: OMAP2+: gpmc: add NAND specific setup
  ARM: OMAP2+: nand: Update gpmc_nand_init() to use generic_gpmc_init()
  mtd: nand: omap: Fix build warning
  mtd: nand: omap: Move IRQ handling from GPMC to NAND driver
  mtd: nand: omap: Move gpmc_update_nand_reg to nand driver
  mtd: nand: omap: Move NAND write protect code from GPMC to NAND driver
  mtd: nand: omap: Copy platform data parameters to omap_nand_info data
  mtd: nand: omap: True device tree support
  ARM: OMAP: gpmc: Update DT binding documentation
  mtd: nand: omap: Update DT binding documentation
  ARM: dts: omap3-beagle: Add NAND device

 Documentation/devicetree/bindings/bus/ti-gpmc.txt  | 109 ++-
 .../devicetree/bindings/mtd/gpmc-nand.txt          |  86 ++-
 arch/arm/boot/dts/omap3-beagle.dts                 |  66 ++
 arch/arm/mach-omap2/gpmc-nand.c                    |  85 +--
 arch/arm/mach-omap2/gpmc.c                         | 781 +++++++++++----------
 arch/arm/mach-omap2/gpmc.h                         | 151 +---
 arch/arm/mach-omap2/io.c                           |   2 +
 drivers/mtd/nand/omap2.c                           | 365 ++++++++--
 include/linux/platform_data/gpmc-omap.h            | 168 +++++
 include/linux/platform_data/mtd-nand-omap2.h       |  10 +-
 10 files changed, 1103 insertions(+), 720 deletions(-)
 create mode 100644 include/linux/platform_data/gpmc-omap.h

-- 
1.8.3.2


^ permalink raw reply	[flat|nested] 28+ messages in thread

* [RFC PATCH 01/16] ARM: OMAP2+: gpmc: Add platform data
  2014-05-21 11:20 [RFC PATCH 00/16] OMAP: GPMC: Restructure OMAP GPMC driver (NAND) Roger Quadros
@ 2014-05-21 11:20 ` Roger Quadros
  2014-05-21 11:20 ` [RFC PATCH 02/16] ARM: OMAP2+: gpmc: Add gpmc timings and settings to " Roger Quadros
                   ` (15 subsequent siblings)
  16 siblings, 0 replies; 28+ messages in thread
From: Roger Quadros @ 2014-05-21 11:20 UTC (permalink / raw)
  To: tony, computersforpeace
  Cc: pekon, ezequiel.garcia, robertcnelson, jg1.han, dwmw2, javier,
	nsekhar, linux-omap, linux-mtd, devicetree, linux-kernel,
	Roger Quadros

Add a platform data structure for GPMC. It contains all the necessary
platform information that needs to be passed from platform init code
to GPMC driver.

Signed-off-by: Roger Quadros <rogerq@ti.com>
---
 arch/arm/mach-omap2/gpmc.h              |  4 +---
 include/linux/platform_data/gpmc-omap.h | 37 +++++++++++++++++++++++++++++++++
 2 files changed, 38 insertions(+), 3 deletions(-)
 create mode 100644 include/linux/platform_data/gpmc-omap.h

diff --git a/arch/arm/mach-omap2/gpmc.h b/arch/arm/mach-omap2/gpmc.h
index 707f6d5..c476712 100644
--- a/arch/arm/mach-omap2/gpmc.h
+++ b/arch/arm/mach-omap2/gpmc.h
@@ -12,9 +12,7 @@
 #define __OMAP2_GPMC_H
 
 #include <linux/platform_data/mtd-nand-omap2.h>
-
-/* Maximum Number of Chip Selects */
-#define GPMC_CS_NUM		8
+#include <linux/platform_data/gpmc-omap.h>
 
 #define GPMC_CS_CONFIG1		0x00
 #define GPMC_CS_CONFIG2		0x04
diff --git a/include/linux/platform_data/gpmc-omap.h b/include/linux/platform_data/gpmc-omap.h
new file mode 100644
index 0000000..bff352f
--- /dev/null
+++ b/include/linux/platform_data/gpmc-omap.h
@@ -0,0 +1,37 @@
+/*
+ * OMAP GPMC Platform data
+ *
+ * Copyright (C) 2014 Texas Instruments, Inc. - http://www.ti.com
+ *	Roger Quadros <rogerq@ti.com>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ */
+
+#ifndef _GPMC_OMAP_H_
+#define _GPMC_OMAP_H_
+
+/* Maximum Number of Chip Selects */
+#define GPMC_CS_NUM		8
+
+enum gpmc_omap_type {
+	GPMC_OMAP_TYPE_NOR,
+	GPMC_OMAP_TYPE_NAND,
+	GPMC_OMAP_TYPE_ONENAND,
+	GPMC_OMAP_TYPE_GENERIC,
+};
+
+/* Data for each chip select */
+struct gpmc_omap_cs_data {
+	bool valid;	/* data is valid */
+	enum gpmc_omap_type type;
+	struct platform_device *pdev;	/* device within this CS region */
+	unsigned pdata_size;
+};
+
+struct gpmc_omap_platform_data {
+	struct gpmc_omap_cs_data cs[GPMC_CS_NUM];
+};
+
+#endif /* _GPMC_OMAP_H */
-- 
1.8.3.2


^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [RFC PATCH 02/16] ARM: OMAP2+: gpmc: Add gpmc timings and settings to platform data
  2014-05-21 11:20 [RFC PATCH 00/16] OMAP: GPMC: Restructure OMAP GPMC driver (NAND) Roger Quadros
  2014-05-21 11:20 ` [RFC PATCH 01/16] ARM: OMAP2+: gpmc: Add platform data Roger Quadros
@ 2014-05-21 11:20 ` Roger Quadros
  2014-05-21 11:20 ` [RFC PATCH 03/16] ARM: OMAP2+: gmpc: add gpmc_generic_init() Roger Quadros
                   ` (14 subsequent siblings)
  16 siblings, 0 replies; 28+ messages in thread
From: Roger Quadros @ 2014-05-21 11:20 UTC (permalink / raw)
  To: tony, computersforpeace
  Cc: pekon, ezequiel.garcia, robertcnelson, jg1.han, dwmw2, javier,
	nsekhar, linux-omap, linux-mtd, devicetree, linux-kernel,
	Roger Quadros

Add device_timings, gpmc_timings and gpmc_setting to
gpmc platform data.

Signed-off-by: Roger Quadros <rogerq@ti.com>
---
 arch/arm/mach-omap2/gpmc.h              | 125 ------------------------------
 include/linux/platform_data/gpmc-omap.h | 131 ++++++++++++++++++++++++++++++++
 2 files changed, 131 insertions(+), 125 deletions(-)

diff --git a/arch/arm/mach-omap2/gpmc.h b/arch/arm/mach-omap2/gpmc.h
index c476712..681977f 100644
--- a/arch/arm/mach-omap2/gpmc.h
+++ b/arch/arm/mach-omap2/gpmc.h
@@ -79,131 +79,6 @@
 #define GPMC_MUX_AAD			1	/* Addr-Addr-Data multiplex */
 #define GPMC_MUX_AD			2	/* Addr-Data multiplex */
 
-/* bool type time settings */
-struct gpmc_bool_timings {
-	bool cycle2cyclediffcsen;
-	bool cycle2cyclesamecsen;
-	bool we_extra_delay;
-	bool oe_extra_delay;
-	bool adv_extra_delay;
-	bool cs_extra_delay;
-	bool time_para_granularity;
-};
-
-/*
- * Note that all values in this struct are in nanoseconds except sync_clk
- * (which is in picoseconds), while the register values are in gpmc_fck cycles.
- */
-struct gpmc_timings {
-	/* Minimum clock period for synchronous mode (in picoseconds) */
-	u32 sync_clk;
-
-	/* Chip-select signal timings corresponding to GPMC_CS_CONFIG2 */
-	u32 cs_on;		/* Assertion time */
-	u32 cs_rd_off;		/* Read deassertion time */
-	u32 cs_wr_off;		/* Write deassertion time */
-
-	/* ADV signal timings corresponding to GPMC_CONFIG3 */
-	u32 adv_on;		/* Assertion time */
-	u32 adv_rd_off;		/* Read deassertion time */
-	u32 adv_wr_off;		/* Write deassertion time */
-
-	/* WE signals timings corresponding to GPMC_CONFIG4 */
-	u32 we_on;		/* WE assertion time */
-	u32 we_off;		/* WE deassertion time */
-
-	/* OE signals timings corresponding to GPMC_CONFIG4 */
-	u32 oe_on;		/* OE assertion time */
-	u32 oe_off;		/* OE deassertion time */
-
-	/* Access time and cycle time timings corresponding to GPMC_CONFIG5 */
-	u32 page_burst_access;	/* Multiple access word delay */
-	u32 access;		/* Start-cycle to first data valid delay */
-	u32 rd_cycle;		/* Total read cycle time */
-	u32 wr_cycle;		/* Total write cycle time */
-
-	u32 bus_turnaround;
-	u32 cycle2cycle_delay;
-
-	u32 wait_monitoring;
-	u32 clk_activation;
-
-	/* The following are only on OMAP3430 */
-	u32 wr_access;		/* WRACCESSTIME */
-	u32 wr_data_mux_bus;	/* WRDATAONADMUXBUS */
-
-	struct gpmc_bool_timings bool_timings;
-};
-
-/* Device timings in picoseconds */
-struct gpmc_device_timings {
-	u32 t_ceasu;	/* address setup to CS valid */
-	u32 t_avdasu;	/* address setup to ADV valid */
-	/* XXX: try to combine t_avdp_r & t_avdp_w. Issue is
-	 * of tusb using these timings even for sync whilst
-	 * ideally for adv_rd/(wr)_off it should have considered
-	 * t_avdh instead. This indirectly necessitates r/w
-	 * variations of t_avdp as it is possible to have one
-	 * sync & other async
-	 */
-	u32 t_avdp_r;	/* ADV low time (what about t_cer ?) */
-	u32 t_avdp_w;
-	u32 t_aavdh;	/* address hold time */
-	u32 t_oeasu;	/* address setup to OE valid */
-	u32 t_aa;	/* access time from ADV assertion */
-	u32 t_iaa;	/* initial access time */
-	u32 t_oe;	/* access time from OE assertion */
-	u32 t_ce;	/* access time from CS asertion */
-	u32 t_rd_cycle;	/* read cycle time */
-	u32 t_cez_r;	/* read CS deassertion to high Z */
-	u32 t_cez_w;	/* write CS deassertion to high Z */
-	u32 t_oez;	/* OE deassertion to high Z */
-	u32 t_weasu;	/* address setup to WE valid */
-	u32 t_wpl;	/* write assertion time */
-	u32 t_wph;	/* write deassertion time */
-	u32 t_wr_cycle;	/* write cycle time */
-
-	u32 clk;
-	u32 t_bacc;	/* burst access valid clock to output delay */
-	u32 t_ces;	/* CS setup time to clk */
-	u32 t_avds;	/* ADV setup time to clk */
-	u32 t_avdh;	/* ADV hold time from clk */
-	u32 t_ach;	/* address hold time from clk */
-	u32 t_rdyo;	/* clk to ready valid */
-
-	u32 t_ce_rdyz;	/* XXX: description ?, or use t_cez instead */
-	u32 t_ce_avd;	/* CS on to ADV on delay */
-
-	/* XXX: check the possibility of combining
-	 * cyc_aavhd_oe & cyc_aavdh_we
-	 */
-	u8 cyc_aavdh_oe;/* read address hold time in cycles */
-	u8 cyc_aavdh_we;/* write address hold time in cycles */
-	u8 cyc_oe;	/* access time from OE assertion in cycles */
-	u8 cyc_wpl;	/* write deassertion time in cycles */
-	u32 cyc_iaa;	/* initial access time in cycles */
-
-	/* extra delays */
-	bool ce_xdelay;
-	bool avd_xdelay;
-	bool oe_xdelay;
-	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 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 mux_add_data;	/* multiplex address & data */
-	u32 wait_pin;		/* wait-pin to be used */
-};
 
 extern int gpmc_calc_timings(struct gpmc_timings *gpmc_t,
 			     struct gpmc_settings *gpmc_s,
diff --git a/include/linux/platform_data/gpmc-omap.h b/include/linux/platform_data/gpmc-omap.h
index bff352f..52b6504 100644
--- a/include/linux/platform_data/gpmc-omap.h
+++ b/include/linux/platform_data/gpmc-omap.h
@@ -22,10 +22,141 @@ enum gpmc_omap_type {
 	GPMC_OMAP_TYPE_GENERIC,
 };
 
+/* bool type time settings */
+struct gpmc_bool_timings {
+	bool cycle2cyclediffcsen;
+	bool cycle2cyclesamecsen;
+	bool we_extra_delay;
+	bool oe_extra_delay;
+	bool adv_extra_delay;
+	bool cs_extra_delay;
+	bool time_para_granularity;
+};
+
+/*
+ * Note that all values in this struct are in nanoseconds except sync_clk
+ * (which is in picoseconds), while the register values are in gpmc_fck cycles.
+ */
+struct gpmc_timings {
+	/* Minimum clock period for synchronous mode (in picoseconds) */
+	u32 sync_clk;
+
+	/* Chip-select signal timings corresponding to GPMC_CS_CONFIG2 */
+	u32 cs_on;		/* Assertion time */
+	u32 cs_rd_off;		/* Read deassertion time */
+	u32 cs_wr_off;		/* Write deassertion time */
+
+	/* ADV signal timings corresponding to GPMC_CONFIG3 */
+	u32 adv_on;		/* Assertion time */
+	u32 adv_rd_off;		/* Read deassertion time */
+	u32 adv_wr_off;		/* Write deassertion time */
+
+	/* WE signals timings corresponding to GPMC_CONFIG4 */
+	u32 we_on;		/* WE assertion time */
+	u32 we_off;		/* WE deassertion time */
+
+	/* OE signals timings corresponding to GPMC_CONFIG4 */
+	u32 oe_on;		/* OE assertion time */
+	u32 oe_off;		/* OE deassertion time */
+
+	/* Access time and cycle time timings corresponding to GPMC_CONFIG5 */
+	u32 page_burst_access;	/* Multiple access word delay */
+	u32 access;		/* Start-cycle to first data valid delay */
+	u32 rd_cycle;		/* Total read cycle time */
+	u32 wr_cycle;		/* Total write cycle time */
+
+	u32 bus_turnaround;
+	u32 cycle2cycle_delay;
+
+	u32 wait_monitoring;
+	u32 clk_activation;
+
+	/* The following are only on OMAP3430 */
+	u32 wr_access;		/* WRACCESSTIME */
+	u32 wr_data_mux_bus;	/* WRDATAONADMUXBUS */
+
+	struct gpmc_bool_timings bool_timings;
+};
+
+
+/* Device timings in picoseconds */
+struct gpmc_device_timings {
+	u32 t_ceasu;	/* address setup to CS valid */
+	u32 t_avdasu;	/* address setup to ADV valid */
+	/* XXX: try to combine t_avdp_r & t_avdp_w. Issue is
+	 * of tusb using these timings even for sync whilst
+	 * ideally for adv_rd/(wr)_off it should have considered
+	 * t_avdh instead. This indirectly necessitates r/w
+	 * variations of t_avdp as it is possible to have one
+	 * sync & other async
+	 */
+	u32 t_avdp_r;	/* ADV low time (what about t_cer ?) */
+	u32 t_avdp_w;
+	u32 t_aavdh;	/* address hold time */
+	u32 t_oeasu;	/* address setup to OE valid */
+	u32 t_aa;	/* access time from ADV assertion */
+	u32 t_iaa;	/* initial access time */
+	u32 t_oe;	/* access time from OE assertion */
+	u32 t_ce;	/* access time from CS asertion */
+	u32 t_rd_cycle;	/* read cycle time */
+	u32 t_cez_r;	/* read CS deassertion to high Z */
+	u32 t_cez_w;	/* write CS deassertion to high Z */
+	u32 t_oez;	/* OE deassertion to high Z */
+	u32 t_weasu;	/* address setup to WE valid */
+	u32 t_wpl;	/* write assertion time */
+	u32 t_wph;	/* write deassertion time */
+	u32 t_wr_cycle;	/* write cycle time */
+
+	u32 clk;
+	u32 t_bacc;	/* burst access valid clock to output delay */
+	u32 t_ces;	/* CS setup time to clk */
+	u32 t_avds;	/* ADV setup time to clk */
+	u32 t_avdh;	/* ADV hold time from clk */
+	u32 t_ach;	/* address hold time from clk */
+	u32 t_rdyo;	/* clk to ready valid */
+
+	u32 t_ce_rdyz;	/* XXX: description ?, or use t_cez instead */
+	u32 t_ce_avd;	/* CS on to ADV on delay */
+
+	/* XXX: check the possibility of combining
+	 * cyc_aavhd_oe & cyc_aavdh_we
+	 */
+	u8 cyc_aavdh_oe;/* read address hold time in cycles */
+	u8 cyc_aavdh_we;/* write address hold time in cycles */
+	u8 cyc_oe;	/* access time from OE assertion in cycles */
+	u8 cyc_wpl;	/* write deassertion time in cycles */
+	u32 cyc_iaa;	/* initial access time in cycles */
+
+	/* extra delays */
+	bool ce_xdelay;
+	bool avd_xdelay;
+	bool oe_xdelay;
+	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 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 mux_add_data;	/* multiplex address & data */
+	u32 wait_pin;		/* wait-pin to be used */
+};
+
+
 /* Data for each chip select */
 struct gpmc_omap_cs_data {
 	bool valid;	/* data is valid */
 	enum gpmc_omap_type type;
+	struct gpmc_settings *settings;
+	struct gpmc_device_timings *device_timings;
+	struct gpmc_timings *gpmc_timings;
 	struct platform_device *pdev;	/* device within this CS region */
 	unsigned pdata_size;
 };
-- 
1.8.3.2


^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [RFC PATCH 03/16] ARM: OMAP2+: gmpc: add gpmc_generic_init()
  2014-05-21 11:20 [RFC PATCH 00/16] OMAP: GPMC: Restructure OMAP GPMC driver (NAND) Roger Quadros
  2014-05-21 11:20 ` [RFC PATCH 01/16] ARM: OMAP2+: gpmc: Add platform data Roger Quadros
  2014-05-21 11:20 ` [RFC PATCH 02/16] ARM: OMAP2+: gpmc: Add gpmc timings and settings to " Roger Quadros
@ 2014-05-21 11:20 ` Roger Quadros
  2014-05-21 11:20 ` [RFC PATCH 04/16] ARM: OMAP2+: gpmc: use platform data to configure CS space and poplulate device Roger Quadros
                   ` (13 subsequent siblings)
  16 siblings, 0 replies; 28+ messages in thread
From: Roger Quadros @ 2014-05-21 11:20 UTC (permalink / raw)
  To: tony, computersforpeace
  Cc: pekon, ezequiel.garcia, robertcnelson, jg1.han, dwmw2, javier,
	nsekhar, linux-omap, linux-mtd, devicetree, linux-kernel,
	Roger Quadros

This function populates platform data for the specified Chip Select.
It should be called by board init code.

Signed-off-by: Roger Quadros <rogerq@ti.com>
---
 arch/arm/mach-omap2/gpmc.c | 69 ++++++++++++++++++++++++++++++++++++++++++++++
 arch/arm/mach-omap2/gpmc.h |  6 ++++
 2 files changed, 75 insertions(+)

diff --git a/arch/arm/mach-omap2/gpmc.c b/arch/arm/mach-omap2/gpmc.c
index 9fe8c94..6b4a322 100644
--- a/arch/arm/mach-omap2/gpmc.c
+++ b/arch/arm/mach-omap2/gpmc.c
@@ -115,6 +115,8 @@
  */
 #define	GPMC_NR_IRQ		2
 
+static struct gpmc_omap_platform_data gpmc_pdata;
+
 struct gpmc_client_irq	{
 	unsigned		irq;
 	u32			bitmask;
@@ -1787,6 +1789,73 @@ static int __init omap_gpmc_init(void)
 }
 omap_postcore_initcall(omap_gpmc_init);
 
+/**
+ * gpmc_generic_init - Initialize platform data for a Chip Select
+ *
+ * @cs		chip select number
+ * @type	GPMC_OMAP_TYPE
+ * @settings	GPMC settings
+ * @device_timings	device timings for device on this CS
+ * @gpmc_timings	GPMC timings
+ * @pdev	platform device for the device on this CS
+ * @pdata_size	platform data size for the platform device
+ */
+int gpmc_generic_init(int cs, enum gpmc_omap_type type,
+		      struct gpmc_settings *settings,
+		      struct gpmc_device_timings *device_timings,
+		      struct gpmc_timings *gpmc_timings,
+		      struct platform_device *pdev, unsigned pdata_size)
+{
+	struct gpmc_settings *gpmc_s;
+	struct gpmc_device_timings *gpmc_dev_t;
+	struct gpmc_timings *gpmc_t;
+
+	if (cs >= GPMC_CS_NUM) {
+		pr_err("%s: Invalid cs specified. Max CS = %d\n",
+		       __func__, GPMC_CS_NUM);
+		return -EINVAL;
+	}
+
+	if (gpmc_pdata.cs[cs].valid) {
+		pr_err("%s: cs %d already requested, ignoring new request\n",
+		       __func__, cs);
+		return -EINVAL;
+	}
+
+	if (settings) {
+		gpmc_s = kmemdup(settings, sizeof(*settings), GFP_KERNEL);
+		if (!gpmc_s)
+			return -ENOMEM;
+
+		gpmc_pdata.cs[cs].settings = gpmc_s;
+	}
+
+	if (device_timings) {
+		gpmc_dev_t = kmemdup(device_timings, sizeof(*device_timings),
+				     GFP_KERNEL);
+		if (!gpmc_dev_t)
+			return -ENOMEM;
+
+		gpmc_pdata.cs[cs].device_timings = gpmc_dev_t;
+	}
+
+	if (gpmc_timings) {
+		gpmc_t = kmemdup(gpmc_timings, sizeof(*gpmc_timings),
+				 GFP_KERNEL);
+		if (!gpmc_t)
+			return -ENOMEM;
+
+		gpmc_pdata.cs[cs].gpmc_timings = gpmc_t;
+	}
+
+	gpmc_pdata.cs[cs].type = type;
+	gpmc_pdata.cs[cs].pdev = pdev;
+	gpmc_pdata.cs[cs].pdata_size = pdata_size;
+	gpmc_pdata.cs[cs].valid = true;
+
+	return 0;
+}
+
 static irqreturn_t gpmc_handle_irq(int irq, void *dev)
 {
 	int i;
diff --git a/arch/arm/mach-omap2/gpmc.h b/arch/arm/mach-omap2/gpmc.h
index 681977f..c18b022 100644
--- a/arch/arm/mach-omap2/gpmc.h
+++ b/arch/arm/mach-omap2/gpmc.h
@@ -100,5 +100,11 @@ 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);
+extern int gpmc_generic_init(int cs, enum gpmc_omap_type type,
+			     struct gpmc_settings *settings,
+			     struct gpmc_device_timings *device_timings,
+			     struct gpmc_timings *gpmc_timings,
+			     struct platform_device *pdev,
+			     unsigned pdata_size);
 
 #endif
-- 
1.8.3.2


^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [RFC PATCH 04/16] ARM: OMAP2+: gpmc: use platform data to configure CS space and poplulate device
  2014-05-21 11:20 [RFC PATCH 00/16] OMAP: GPMC: Restructure OMAP GPMC driver (NAND) Roger Quadros
                   ` (2 preceding siblings ...)
  2014-05-21 11:20 ` [RFC PATCH 03/16] ARM: OMAP2+: gmpc: add gpmc_generic_init() Roger Quadros
@ 2014-05-21 11:20 ` Roger Quadros
  2014-05-21 11:20 ` [RFC PATCH 05/16] ARM: OMAP2+: gpmc: Use low level read/write for context save/restore Roger Quadros
                   ` (12 subsequent siblings)
  16 siblings, 0 replies; 28+ messages in thread
From: Roger Quadros @ 2014-05-21 11:20 UTC (permalink / raw)
  To: tony, computersforpeace
  Cc: pekon, ezequiel.garcia, robertcnelson, jg1.han, dwmw2, javier,
	nsekhar, linux-omap, linux-mtd, devicetree, linux-kernel,
	Roger Quadros

Add gpmc_probe_legacy() that will be called for non DT boots. This function
will use platform data to setup each chip select and populate the child
platform device for each of the chip selects.

Re-arrange init order so that gpmc device is created after
the gpmc platform data is initialized by board files.
i.e. move omap_gpmc_init() to subsys_initcall.

Load gpmc platform driver later in the boot process.
i.e. move gpmc_init() to module_initcall.

NOTE: this will break legacy boot since they call gpmc_cs_*()
functions before gpmc_mem_init() is called. They will eventually
be fixed.

Signed-off-by: Roger Quadros <rogerq@ti.com>
---
 arch/arm/mach-omap2/gpmc.c | 137 ++++++++++++++++++++++++++++++++++++++-------
 1 file changed, 118 insertions(+), 19 deletions(-)

diff --git a/arch/arm/mach-omap2/gpmc.c b/arch/arm/mach-omap2/gpmc.c
index 6b4a322..04bae67 100644
--- a/arch/arm/mach-omap2/gpmc.c
+++ b/arch/arm/mach-omap2/gpmc.c
@@ -160,7 +160,6 @@ static DEFINE_SPINLOCK(gpmc_mem_lock);
 static unsigned int gpmc_cs_map = ((1 << GPMC_CS_NUM) - 1);
 static unsigned int gpmc_cs_num = GPMC_CS_NUM;
 static unsigned int gpmc_nr_waitpins;
-static struct device *gpmc_dev;
 static int gpmc_irq;
 static resource_size_t phys_base, mem_size;
 static unsigned gpmc_capability;
@@ -1630,11 +1629,102 @@ static int gpmc_probe_dt(struct platform_device *pdev)
 }
 #endif
 
+static void gpmc_probe_legacy(struct platform_device *pdev)
+{
+	int i, rc;
+	struct device *dev = &pdev->dev;
+	struct gpmc_omap_platform_data *gpmc_pdata;
+
+	gpmc_pdata = dev->platform_data;
+	gpmc_cs_num = GPMC_CS_NUM;
+	gpmc_nr_waitpins = GPMC_NR_WAITPINS;
+
+	for (i = 0; i < GPMC_CS_NUM; i++) {
+		struct resource *mem_res;
+		unsigned long cs_base;
+		resource_size_t size;
+		struct gpmc_timings gpmc_timings;
+		struct gpmc_omap_cs_data *cs;
+
+		cs = &gpmc_pdata->cs[i];
+		if (!cs->valid)
+			continue;
+
+		/*
+		 * Request a CS space. Use size from
+		 * platform device's MEM resource
+		 */
+		if (!cs->pdev)
+			goto skip_mem;
+
+		mem_res = cs->pdev->resource;
+		if (cs->pdev->num_resources < 1 ||
+		    resource_type(mem_res) != IORESOURCE_MEM) {
+			dev_err(dev, "Invalid IOMEM resource for CS %d\n", i);
+			continue;
+		}
+
+		size = mem_res->end - mem_res->start + 1;
+		if (gpmc_cs_request(i, size, &cs_base)) {
+			dev_err(dev, "Couldn't request resource for CS %d\n",
+				i);
+			continue;
+		}
+
+		mem_res->start = cs_base;
+		mem_res->end = cs_base + size - 1;
+
+		/* FIXME: When do we need to call gpmc_cs_remap()? */
+skip_mem:
+
+		if (cs->settings) {
+			if (gpmc_cs_program_settings(i, cs->settings)) {
+				dev_err(dev,
+					"Couldn't program settings for CS %d\n",
+					i);
+				continue;
+			}
+		}
+
+		/* give device_timings priority over gpmc_timings */
+		if (cs->device_timings) {
+			gpmc_calc_timings(&gpmc_timings, cs->settings,
+					  cs->device_timings);
+
+			if (gpmc_cs_set_timings(i, &gpmc_timings)) {
+				dev_err(dev,
+					"Couldn't program timings for CS %d\n",
+					i);
+				continue;
+			}
+		} else if (cs->gpmc_timings) {
+			if (gpmc_cs_set_timings(i, cs->gpmc_timings)) {
+				dev_err(dev,
+					"Couldn't program timings for CS %d\n",
+					i);
+				continue;
+			}
+		}
+
+		if (!cs->pdev)
+			continue;
+
+		rc = platform_device_register(cs->pdev);
+		if (rc < 0) {
+			dev_err(dev,
+				"Failed to register device %s on CS %d\n",
+				cs->pdev->name, i);
+			continue;
+		}
+	}
+}
+
 static int gpmc_probe(struct platform_device *pdev)
 {
 	int rc;
 	u32 l;
 	struct resource *res;
+	struct device *dev = &pdev->dev;
 
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	if (res == NULL)
@@ -1663,8 +1753,6 @@ static int gpmc_probe(struct platform_device *pdev)
 	pm_runtime_enable(&pdev->dev);
 	pm_runtime_get_sync(&pdev->dev);
 
-	gpmc_dev = &pdev->dev;
-
 	l = gpmc_read_reg(GPMC_REVISION);
 
 	/*
@@ -1683,31 +1771,40 @@ static int gpmc_probe(struct platform_device *pdev)
 		gpmc_capability = GPMC_HAS_WR_ACCESS | GPMC_HAS_WR_DATA_MUX_BUS;
 	if (GPMC_REVISION_MAJOR(l) > 0x5)
 		gpmc_capability |= GPMC_HAS_MUX_AAD;
-	dev_info(gpmc_dev, "GPMC revision %d.%d\n", GPMC_REVISION_MAJOR(l),
+	dev_info(dev, "GPMC revision %d.%d\n", GPMC_REVISION_MAJOR(l),
 		 GPMC_REVISION_MINOR(l));
 
 	gpmc_mem_init();
 
 	if (gpmc_setup_irq() < 0)
-		dev_warn(gpmc_dev, "gpmc_setup_irq failed\n");
+		dev_warn(dev, "gpmc_setup_irq failed\n");
 
 	/* Now the GPMC is initialised, unreserve the chip-selects */
 	gpmc_cs_map = 0;
 
-	if (!pdev->dev.of_node) {
-		gpmc_cs_num	 = GPMC_CS_NUM;
-		gpmc_nr_waitpins = GPMC_NR_WAITPINS;
-	}
+	if (!dev->of_node) {
+		/* Legacy probing based on platform data */
+		if (!dev->platform_data) {
+			dev_err(dev, "missing platform data\n");
+			rc = -EINVAL;
+			goto error;
+		}
+		gpmc_probe_legacy(pdev);
 
-	rc = gpmc_probe_dt(pdev);
-	if (rc < 0) {
-		pm_runtime_put_sync(&pdev->dev);
-		clk_put(gpmc_l3_clk);
-		dev_err(gpmc_dev, "failed to probe DT parameters\n");
-		return rc;
+	} else {
+		rc = gpmc_probe_dt(pdev);
+		if (rc < 0) {
+			dev_err(dev, "failed to probe DT parameters\n");
+			goto error;
+		}
 	}
 
 	return 0;
+
+error:
+	pm_runtime_put_sync(dev);
+	clk_put(gpmc_l3_clk);
+	return rc;
 }
 
 static int gpmc_remove(struct platform_device *pdev)
@@ -1716,7 +1813,6 @@ static int gpmc_remove(struct platform_device *pdev)
 	gpmc_mem_exit();
 	pm_runtime_put_sync(&pdev->dev);
 	pm_runtime_disable(&pdev->dev);
-	gpmc_dev = NULL;
 	return 0;
 }
 
@@ -1760,7 +1856,7 @@ static __exit void gpmc_exit(void)
 
 }
 
-omap_postcore_initcall(gpmc_init);
+module_init(gpmc_init);
 module_exit(gpmc_exit);
 
 static int __init omap_gpmc_init(void)
@@ -1782,12 +1878,15 @@ static int __init omap_gpmc_init(void)
 		return -ENODEV;
 	}
 
-	pdev = omap_device_build(DEVICE_NAME, -1, oh, NULL, 0);
+	pdev = omap_device_build(DEVICE_NAME, -1, oh, (void *)&gpmc_pdata,
+				 sizeof(gpmc_pdata));
 	WARN(IS_ERR(pdev), "could not build omap_device for %s\n", oh_name);
 
 	return PTR_RET(pdev);
 }
-omap_postcore_initcall(omap_gpmc_init);
+
+/* must run after machine_init code. i.e. arch_init */
+omap_subsys_initcall(omap_gpmc_init);
 
 /**
  * gpmc_generic_init - Initialize platform data for a Chip Select
-- 
1.8.3.2


^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [RFC PATCH 05/16] ARM: OMAP2+: gpmc: Use low level read/write for context save/restore
  2014-05-21 11:20 [RFC PATCH 00/16] OMAP: GPMC: Restructure OMAP GPMC driver (NAND) Roger Quadros
                   ` (3 preceding siblings ...)
  2014-05-21 11:20 ` [RFC PATCH 04/16] ARM: OMAP2+: gpmc: use platform data to configure CS space and poplulate device Roger Quadros
@ 2014-05-21 11:20 ` Roger Quadros
  2014-05-21 11:20 ` [RFC PATCH 06/16] ARM: OMAP2+: gpmc: add NAND specific setup Roger Quadros
                   ` (11 subsequent siblings)
  16 siblings, 0 replies; 28+ messages in thread
From: Roger Quadros @ 2014-05-21 11:20 UTC (permalink / raw)
  To: tony, computersforpeace
  Cc: pekon, ezequiel.garcia, robertcnelson, jg1.han, dwmw2, javier,
	nsekhar, linux-omap, linux-mtd, devicetree, linux-kernel,
	Roger Quadros

This will make it easier to move the GPMC driver out of arch/arm/mach-omap2.
Just the context save/restore code can remain there without any dependency
with the GPMC driver.

Signed-off-by: Roger Quadros <rogerq@ti.com>
---
 arch/arm/mach-omap2/gpmc.c | 121 +++++++++++++++++++++++++++++++--------------
 arch/arm/mach-omap2/gpmc.h |   1 +
 arch/arm/mach-omap2/io.c   |   2 +
 3 files changed, 88 insertions(+), 36 deletions(-)

diff --git a/arch/arm/mach-omap2/gpmc.c b/arch/arm/mach-omap2/gpmc.c
index 04bae67..26b26ec 100644
--- a/arch/arm/mach-omap2/gpmc.c
+++ b/arch/arm/mach-omap2/gpmc.c
@@ -1976,34 +1976,80 @@ static irqreturn_t gpmc_handle_irq(int irq, void *dev)
 
 static struct omap3_gpmc_regs gpmc_context;
 
+/*
+ * Below code only for OMAP3 OFF mode support.
+ * This code must be left back in mach-omap2.
+ */
+void __iomem *omap2_gpmc_base;
+
+void __init omap2_set_globals_gpmc(void __iomem *gpmc)
+{
+	omap2_gpmc_base = gpmc;
+}
+
+static u32 _gpmc_read_reg(u16 reg)
+{
+	return __raw_readl(omap2_gpmc_base + reg);
+}
+
+static void _gpmc_write_reg(u32 val, u16 reg)
+{
+	__raw_readl(omap2_gpmc_base + reg);
+}
+
+static u32 _gpmc_cs_read_reg(int cs, int idx)
+{
+	u16 reg;
+
+	reg = GPMC_CS0_OFFSET + (cs * GPMC_CS_SIZE) + idx;
+
+	return _gpmc_read_reg(reg);
+}
+
+static void _gpmc_cs_write_reg(int cs, int idx, u32 val)
+{
+	u16 reg;
+
+	reg = GPMC_CS0_OFFSET + (cs * GPMC_CS_SIZE) + idx;
+	_gpmc_write_reg(val, reg);
+}
+
 void omap3_gpmc_save_context(void)
 {
 	int i;
+	u32 val;
+
+	if (!omap2_gpmc_base)
+		return;
 
-	gpmc_context.sysconfig = gpmc_read_reg(GPMC_SYSCONFIG);
-	gpmc_context.irqenable = gpmc_read_reg(GPMC_IRQENABLE);
-	gpmc_context.timeout_ctrl = gpmc_read_reg(GPMC_TIMEOUT_CONTROL);
-	gpmc_context.config = gpmc_read_reg(GPMC_CONFIG);
-	gpmc_context.prefetch_config1 = gpmc_read_reg(GPMC_PREFETCH_CONFIG1);
-	gpmc_context.prefetch_config2 = gpmc_read_reg(GPMC_PREFETCH_CONFIG2);
-	gpmc_context.prefetch_control = gpmc_read_reg(GPMC_PREFETCH_CONTROL);
+	gpmc_context.sysconfig = _gpmc_read_reg(GPMC_SYSCONFIG);
+	gpmc_context.irqenable = _gpmc_read_reg(GPMC_IRQENABLE);
+	gpmc_context.timeout_ctrl = _gpmc_read_reg(GPMC_TIMEOUT_CONTROL);
+	gpmc_context.config = _gpmc_read_reg(GPMC_CONFIG);
+	gpmc_context.prefetch_config1 = _gpmc_read_reg(GPMC_PREFETCH_CONFIG1);
+	gpmc_context.prefetch_config2 = _gpmc_read_reg(GPMC_PREFETCH_CONFIG2);
+	gpmc_context.prefetch_control = _gpmc_read_reg(GPMC_PREFETCH_CONTROL);
 	for (i = 0; i < gpmc_cs_num; i++) {
-		gpmc_context.cs_context[i].is_valid = gpmc_cs_mem_enabled(i);
+		/* check if valid */
+		val = _gpmc_cs_read_reg(i, GPMC_CS_CONFIG7);
+		gpmc_context.cs_context[i].is_valid =
+						val & GPMC_CONFIG7_CSVALID;
+
 		if (gpmc_context.cs_context[i].is_valid) {
 			gpmc_context.cs_context[i].config1 =
-				gpmc_cs_read_reg(i, GPMC_CS_CONFIG1);
+				_gpmc_cs_read_reg(i, GPMC_CS_CONFIG1);
 			gpmc_context.cs_context[i].config2 =
-				gpmc_cs_read_reg(i, GPMC_CS_CONFIG2);
+				_gpmc_cs_read_reg(i, GPMC_CS_CONFIG2);
 			gpmc_context.cs_context[i].config3 =
-				gpmc_cs_read_reg(i, GPMC_CS_CONFIG3);
+				_gpmc_cs_read_reg(i, GPMC_CS_CONFIG3);
 			gpmc_context.cs_context[i].config4 =
-				gpmc_cs_read_reg(i, GPMC_CS_CONFIG4);
+				_gpmc_cs_read_reg(i, GPMC_CS_CONFIG4);
 			gpmc_context.cs_context[i].config5 =
-				gpmc_cs_read_reg(i, GPMC_CS_CONFIG5);
+				_gpmc_cs_read_reg(i, GPMC_CS_CONFIG5);
 			gpmc_context.cs_context[i].config6 =
-				gpmc_cs_read_reg(i, GPMC_CS_CONFIG6);
+				_gpmc_cs_read_reg(i, GPMC_CS_CONFIG6);
 			gpmc_context.cs_context[i].config7 =
-				gpmc_cs_read_reg(i, GPMC_CS_CONFIG7);
+				_gpmc_cs_read_reg(i, GPMC_CS_CONFIG7);
 		}
 	}
 }
@@ -2012,29 +2058,32 @@ void omap3_gpmc_restore_context(void)
 {
 	int i;
 
-	gpmc_write_reg(GPMC_SYSCONFIG, gpmc_context.sysconfig);
-	gpmc_write_reg(GPMC_IRQENABLE, gpmc_context.irqenable);
-	gpmc_write_reg(GPMC_TIMEOUT_CONTROL, gpmc_context.timeout_ctrl);
-	gpmc_write_reg(GPMC_CONFIG, gpmc_context.config);
-	gpmc_write_reg(GPMC_PREFETCH_CONFIG1, gpmc_context.prefetch_config1);
-	gpmc_write_reg(GPMC_PREFETCH_CONFIG2, gpmc_context.prefetch_config2);
-	gpmc_write_reg(GPMC_PREFETCH_CONTROL, gpmc_context.prefetch_control);
+	if (!omap2_gpmc_base)
+		return;
+
+	_gpmc_write_reg(GPMC_SYSCONFIG, gpmc_context.sysconfig);
+	_gpmc_write_reg(GPMC_IRQENABLE, gpmc_context.irqenable);
+	_gpmc_write_reg(GPMC_TIMEOUT_CONTROL, gpmc_context.timeout_ctrl);
+	_gpmc_write_reg(GPMC_CONFIG, gpmc_context.config);
+	_gpmc_write_reg(GPMC_PREFETCH_CONFIG1, gpmc_context.prefetch_config1);
+	_gpmc_write_reg(GPMC_PREFETCH_CONFIG2, gpmc_context.prefetch_config2);
+	_gpmc_write_reg(GPMC_PREFETCH_CONTROL, gpmc_context.prefetch_control);
 	for (i = 0; i < gpmc_cs_num; i++) {
 		if (gpmc_context.cs_context[i].is_valid) {
-			gpmc_cs_write_reg(i, GPMC_CS_CONFIG1,
-				gpmc_context.cs_context[i].config1);
-			gpmc_cs_write_reg(i, GPMC_CS_CONFIG2,
-				gpmc_context.cs_context[i].config2);
-			gpmc_cs_write_reg(i, GPMC_CS_CONFIG3,
-				gpmc_context.cs_context[i].config3);
-			gpmc_cs_write_reg(i, GPMC_CS_CONFIG4,
-				gpmc_context.cs_context[i].config4);
-			gpmc_cs_write_reg(i, GPMC_CS_CONFIG5,
-				gpmc_context.cs_context[i].config5);
-			gpmc_cs_write_reg(i, GPMC_CS_CONFIG6,
-				gpmc_context.cs_context[i].config6);
-			gpmc_cs_write_reg(i, GPMC_CS_CONFIG7,
-				gpmc_context.cs_context[i].config7);
+			_gpmc_cs_write_reg(i, GPMC_CS_CONFIG1,
+					   gpmc_context.cs_context[i].config1);
+			_gpmc_cs_write_reg(i, GPMC_CS_CONFIG2,
+					   gpmc_context.cs_context[i].config2);
+			_gpmc_cs_write_reg(i, GPMC_CS_CONFIG3,
+					   gpmc_context.cs_context[i].config3);
+			_gpmc_cs_write_reg(i, GPMC_CS_CONFIG4,
+					   gpmc_context.cs_context[i].config4);
+			_gpmc_cs_write_reg(i, GPMC_CS_CONFIG5,
+					   gpmc_context.cs_context[i].config5);
+			_gpmc_cs_write_reg(i, GPMC_CS_CONFIG6,
+					   gpmc_context.cs_context[i].config6);
+			_gpmc_cs_write_reg(i, GPMC_CS_CONFIG7,
+					   gpmc_context.cs_context[i].config7);
 		}
 	}
 }
diff --git a/arch/arm/mach-omap2/gpmc.h b/arch/arm/mach-omap2/gpmc.h
index c18b022..795502f 100644
--- a/arch/arm/mach-omap2/gpmc.h
+++ b/arch/arm/mach-omap2/gpmc.h
@@ -107,4 +107,5 @@ extern int gpmc_generic_init(int cs, enum gpmc_omap_type type,
 			     struct platform_device *pdev,
 			     unsigned pdata_size);
 
+void __init omap2_set_globals_gpmc(void __iomem *gpmc);
 #endif
diff --git a/arch/arm/mach-omap2/io.c b/arch/arm/mach-omap2/io.c
index f14f9ac..e3321fd 100644
--- a/arch/arm/mach-omap2/io.c
+++ b/arch/arm/mach-omap2/io.c
@@ -53,6 +53,7 @@
 #include "prm2xxx.h"
 #include "prm3xxx.h"
 #include "prm44xx.h"
+#include "gpmc.h"
 
 /*
  * omap_clk_soc_init: points to a function that does the SoC-specific
@@ -464,6 +465,7 @@ void __init omap3_init_early(void)
 				  NULL);
 	omap2_set_globals_prm(OMAP2_L4_IO_ADDRESS(OMAP3430_PRM_BASE));
 	omap2_set_globals_cm(OMAP2_L4_IO_ADDRESS(OMAP3430_CM_BASE), NULL);
+	omap2_set_globals_gpmc(OMAP2_L3_IO_ADDRESS(OMAP34XX_GPMC_BASE));
 	omap3xxx_check_revision();
 	omap3xxx_check_features();
 	omap3xxx_prm_init();
-- 
1.8.3.2


^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [RFC PATCH 06/16] ARM: OMAP2+: gpmc: add NAND specific setup
  2014-05-21 11:20 [RFC PATCH 00/16] OMAP: GPMC: Restructure OMAP GPMC driver (NAND) Roger Quadros
                   ` (4 preceding siblings ...)
  2014-05-21 11:20 ` [RFC PATCH 05/16] ARM: OMAP2+: gpmc: Use low level read/write for context save/restore Roger Quadros
@ 2014-05-21 11:20 ` Roger Quadros
  2014-05-21 11:20 ` [RFC PATCH 07/16] ARM: OMAP2+: nand: Update gpmc_nand_init() to use generic_gpmc_init() Roger Quadros
                   ` (10 subsequent siblings)
  16 siblings, 0 replies; 28+ messages in thread
From: Roger Quadros @ 2014-05-21 11:20 UTC (permalink / raw)
  To: tony, computersforpeace
  Cc: pekon, ezequiel.garcia, robertcnelson, jg1.han, dwmw2, javier,
	nsekhar, linux-omap, linux-mtd, devicetree, linux-kernel,
	Roger Quadros

Provide NAND specific resources and platform data.

Signed-off-by: Roger Quadros <rogerq@ti.com>
---
 arch/arm/mach-omap2/gpmc.c | 77 ++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 77 insertions(+)

diff --git a/arch/arm/mach-omap2/gpmc.c b/arch/arm/mach-omap2/gpmc.c
index 26b26ec..f4ee6e9 100644
--- a/arch/arm/mach-omap2/gpmc.c
+++ b/arch/arm/mach-omap2/gpmc.c
@@ -1629,6 +1629,61 @@ static int gpmc_probe_dt(struct platform_device *pdev)
 }
 #endif
 
+static int gpmc_nand_setup(struct platform_device *pdev,
+			   struct gpmc_settings *settings)
+{
+	struct resource *res;
+	int rc;
+	struct omap_nand_platform_data *gpmc_nand_data;
+
+	gpmc_nand_data = pdev->dev.platform_data;
+	if (!gpmc_nand_data)
+		return -EINVAL;
+
+	res = pdev->resource;
+
+	/* setup IRQ resources */
+	res[1].start = gpmc_get_client_irq(GPMC_IRQ_FIFOEVENTENABLE);
+
+	res[2].start = gpmc_get_client_irq(GPMC_IRQ_COUNT_EVENT);
+
+	settings->device_nand = true;
+	/*
+	 * Not sure why WP is explicitly turned OFF. we just moved it here
+	 * as is from mach-omap2/gpmc-nand.c
+	 */
+	rc = gpmc_configure(GPMC_CONFIG_WP, 0);
+	if (rc < 0)
+		return rc;
+
+	/* update register addresses in NAND platform data */
+	gpmc_update_nand_reg(&gpmc_nand_data->reg, gpmc_nand_data->cs);
+
+	return 0;
+}
+
+static int gpmc_nand_check_csdata(struct gpmc_omap_cs_data *cs)
+{
+	struct resource *res;
+
+	if (!cs->pdev)
+		return -EINVAL;
+
+	res = cs->pdev->resource;
+
+	if (cs->pdev->num_resources < 3)
+		return -EINVAL;
+
+	if (resource_type(&res[1]) != IORESOURCE_IRQ ||
+	    resource_type(&res[2]) != IORESOURCE_IRQ)
+		return -EINVAL;
+
+	if (!cs->settings)
+		return -EINVAL;
+
+	return 0;
+}
+
 static void gpmc_probe_legacy(struct platform_device *pdev)
 {
 	int i, rc;
@@ -1677,6 +1732,28 @@ static void gpmc_probe_legacy(struct platform_device *pdev)
 		/* FIXME: When do we need to call gpmc_cs_remap()? */
 skip_mem:
 
+		/* Customized setup based on type */
+		switch (cs->type) {
+		case GPMC_OMAP_TYPE_NAND:
+			if (gpmc_nand_check_csdata(cs)) {
+				dev_err(dev, "Invalid NAND config on CS %d\n",
+					i);
+				continue;
+			}
+
+			rc = gpmc_nand_setup(cs->pdev, cs->settings);
+			if (rc) {
+				dev_err(dev, "Error setting up NAND on CS %d\n",
+					i);
+				continue;
+			}
+			break;
+		case GPMC_OMAP_TYPE_NOR:
+		case GPMC_OMAP_TYPE_ONENAND:
+		case GPMC_OMAP_TYPE_GENERIC:
+			break;
+		}
+
 		if (cs->settings) {
 			if (gpmc_cs_program_settings(i, cs->settings)) {
 				dev_err(dev,
-- 
1.8.3.2


^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [RFC PATCH 07/16] ARM: OMAP2+: nand: Update gpmc_nand_init() to use generic_gpmc_init()
  2014-05-21 11:20 [RFC PATCH 00/16] OMAP: GPMC: Restructure OMAP GPMC driver (NAND) Roger Quadros
                   ` (5 preceding siblings ...)
  2014-05-21 11:20 ` [RFC PATCH 06/16] ARM: OMAP2+: gpmc: add NAND specific setup Roger Quadros
@ 2014-05-21 11:20 ` Roger Quadros
  2014-05-21 11:20 ` [RFC PATCH 08/16] mtd: nand: omap: Fix build warning Roger Quadros
                   ` (9 subsequent siblings)
  16 siblings, 0 replies; 28+ messages in thread
From: Roger Quadros @ 2014-05-21 11:20 UTC (permalink / raw)
  To: tony, computersforpeace
  Cc: pekon, ezequiel.garcia, robertcnelson, jg1.han, dwmw2, javier,
	nsekhar, linux-omap, linux-mtd, devicetree, linux-kernel,
	Roger Quadros

This function should only be called by board init code for
legacy boot.

Signed-off-by: Roger Quadros <rogerq@ti.com>
---
 arch/arm/mach-omap2/gpmc-nand.c | 79 ++++++++---------------------------------
 1 file changed, 14 insertions(+), 65 deletions(-)

diff --git a/arch/arm/mach-omap2/gpmc-nand.c b/arch/arm/mach-omap2/gpmc-nand.c
index 4349e82..d536575 100644
--- a/arch/arm/mach-omap2/gpmc-nand.c
+++ b/arch/arm/mach-omap2/gpmc-nand.c
@@ -25,8 +25,11 @@
 #define	NAND_IO_SIZE	4
 
 static struct resource gpmc_nand_resource[] = {
+/* GPMC driver will fixup all the resources, see gpmc_probe_legacy () */
 	{
 		.flags		= IORESOURCE_MEM,
+		.start          = 0,
+		.end            = NAND_IO_SIZE - 1,
 	},
 	{
 		.flags		= IORESOURCE_IRQ,
@@ -72,89 +75,35 @@ static bool gpmc_hwecc_bch_capable(enum omap_ecc ecc_opt)
 		return 0;
 }
 
-/* This function will go away once the device-tree convertion is complete */
-static void gpmc_set_legacy(struct omap_nand_platform_data *gpmc_nand_data,
-			    struct gpmc_settings *s)
-{
-	/* Enable RD PIN Monitoring Reg */
-	if (gpmc_nand_data->dev_ready) {
-		s->wait_on_read = true;
-		s->wait_on_write = true;
-	}
-
-	if (gpmc_nand_data->devsize == NAND_BUSWIDTH_16)
-		s->device_width = GPMC_DEVWIDTH_16BIT;
-	else
-		s->device_width = GPMC_DEVWIDTH_8BIT;
-}
-
 int gpmc_nand_init(struct omap_nand_platform_data *gpmc_nand_data,
 		   struct gpmc_timings *gpmc_t)
 {
 	int err	= 0;
 	struct gpmc_settings s;
-	struct device *dev = &gpmc_nand_device.dev;
 
 	memset(&s, 0, sizeof(struct gpmc_settings));
-
 	gpmc_nand_device.dev.platform_data = gpmc_nand_data;
 
-	err = gpmc_cs_request(gpmc_nand_data->cs, NAND_IO_SIZE,
-				(unsigned long *)&gpmc_nand_resource[0].start);
-	if (err < 0) {
-		dev_err(dev, "Cannot request GPMC CS %d, error %d\n",
-			gpmc_nand_data->cs, err);
-		return err;
-	}
-
-	gpmc_nand_resource[0].end = gpmc_nand_resource[0].start +
-							NAND_IO_SIZE - 1;
-
-	gpmc_nand_resource[1].start =
-				gpmc_get_client_irq(GPMC_IRQ_FIFOEVENTENABLE);
-	gpmc_nand_resource[2].start =
-				gpmc_get_client_irq(GPMC_IRQ_COUNT_EVENT);
-
-	if (gpmc_t) {
-		err = gpmc_cs_set_timings(gpmc_nand_data->cs, gpmc_t);
-		if (err < 0) {
-			dev_err(dev, "Unable to set gpmc timings: %d\n", err);
-			return err;
-		}
-	}
-
-	if (gpmc_nand_data->of_node)
-		gpmc_read_settings_dt(gpmc_nand_data->of_node, &s);
+	/* GPMC settings */
+	if (gpmc_nand_data->devsize == NAND_BUSWIDTH_16)
+		s.device_width = GPMC_DEVWIDTH_16BIT;
 	else
-		gpmc_set_legacy(gpmc_nand_data, &s);
+		s.device_width = GPMC_DEVWIDTH_8BIT;
 
 	s.device_nand = true;
 
-	err = gpmc_cs_program_settings(gpmc_nand_data->cs, &s);
-	if (err < 0)
-		goto out_free_cs;
-
-	err = gpmc_configure(GPMC_CONFIG_WP, 0);
-	if (err < 0)
-		goto out_free_cs;
-
-	gpmc_update_nand_reg(&gpmc_nand_data->reg, gpmc_nand_data->cs);
-
 	if (!gpmc_hwecc_bch_capable(gpmc_nand_data->ecc_opt)) {
-		dev_err(dev, "Unsupported NAND ECC scheme selected\n");
+		pr_err("%s: Unsupported NAND ECC scheme selected\n", __func__);
 		return -EINVAL;
 	}
 
-	err = platform_device_register(&gpmc_nand_device);
-	if (err < 0) {
-		dev_err(dev, "Unable to register NAND device\n");
-		goto out_free_cs;
+	err = gpmc_generic_init(gpmc_nand_data->cs, GPMC_OMAP_TYPE_NAND, &s,
+				NULL, gpmc_t, &gpmc_nand_device,
+				sizeof(*gpmc_nand_data));
+	if (err) {
+		pr_err("%s: gpmc_generic_init() failed %d\n", __func__, err);
+		return err;
 	}
 
 	return 0;
-
-out_free_cs:
-	gpmc_cs_free(gpmc_nand_data->cs);
-
-	return err;
 }
-- 
1.8.3.2


^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [RFC PATCH 08/16] mtd: nand: omap: Fix build warning
  2014-05-21 11:20 [RFC PATCH 00/16] OMAP: GPMC: Restructure OMAP GPMC driver (NAND) Roger Quadros
                   ` (6 preceding siblings ...)
  2014-05-21 11:20 ` [RFC PATCH 07/16] ARM: OMAP2+: nand: Update gpmc_nand_init() to use generic_gpmc_init() Roger Quadros
@ 2014-05-21 11:20 ` Roger Quadros
  2014-05-22  0:54   ` Jingoo Han
  2014-05-21 11:20 ` [RFC PATCH 09/16] mtd: nand: omap: Move IRQ handling from GPMC to NAND driver Roger Quadros
                   ` (8 subsequent siblings)
  16 siblings, 1 reply; 28+ messages in thread
From: Roger Quadros @ 2014-05-21 11:20 UTC (permalink / raw)
  To: tony, computersforpeace
  Cc: pekon, ezequiel.garcia, robertcnelson, jg1.han, dwmw2, javier,
	nsekhar, linux-omap, linux-mtd, devicetree, linux-kernel,
	Roger Quadros

Fix the following warning when CONFIG_MTD_NAND_OMAP_BCH is disabled.
warning: ‘erased_sector_bitflips’ defined but not used [-Wunused-function]

Signed-off-by: Roger Quadros <rogerq@ti.com>
---
 drivers/mtd/nand/omap2.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mtd/nand/omap2.c b/drivers/mtd/nand/omap2.c
index 1ff49b8..1b800bc 100644
--- a/drivers/mtd/nand/omap2.c
+++ b/drivers/mtd/nand/omap2.c
@@ -1237,6 +1237,7 @@ static int __maybe_unused omap_calculate_ecc_bch(struct mtd_info *mtd,
 	return 0;
 }
 
+#ifdef CONFIG_MTD_NAND_OMAP_BCH
 /**
  * erased_sector_bitflips - count bit flips
  * @data:	data sector buffer
@@ -1276,7 +1277,6 @@ static int erased_sector_bitflips(u_char *data, u_char *oob,
 	return flip_bits;
 }
 
-#ifdef CONFIG_MTD_NAND_OMAP_BCH
 /**
  * omap_elm_correct_data - corrects page data area in case error reported
  * @mtd:	MTD device structure
-- 
1.8.3.2


^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [RFC PATCH 09/16] mtd: nand: omap: Move IRQ handling from GPMC to NAND driver
  2014-05-21 11:20 [RFC PATCH 00/16] OMAP: GPMC: Restructure OMAP GPMC driver (NAND) Roger Quadros
                   ` (7 preceding siblings ...)
  2014-05-21 11:20 ` [RFC PATCH 08/16] mtd: nand: omap: Fix build warning Roger Quadros
@ 2014-05-21 11:20 ` Roger Quadros
  2014-05-21 11:20 ` [RFC PATCH 10/16] mtd: nand: omap: Move gpmc_update_nand_reg to nand driver Roger Quadros
                   ` (7 subsequent siblings)
  16 siblings, 0 replies; 28+ messages in thread
From: Roger Quadros @ 2014-05-21 11:20 UTC (permalink / raw)
  To: tony, computersforpeace
  Cc: pekon, ezequiel.garcia, robertcnelson, jg1.han, dwmw2, javier,
	nsekhar, linux-omap, linux-mtd, devicetree, linux-kernel,
	Roger Quadros

Since the Interrupt Events are used only by the NAND driver,
there is no point in managing the Interrupt registers
in the GPMC driver and complicating it with irqchip modeling.

Let's manage the interrupt registers directly in the NAND driver
and get rid of irqchip model from GPMC driver.

Get rid of IRQ commands and unused commands from gpmc_configure() in
GPMC driver.

Signed-off-by: Roger Quadros <rogerq@ti.com>
---
 arch/arm/mach-omap2/gpmc-nand.c              |   3 -
 arch/arm/mach-omap2/gpmc.c                   | 166 +--------------------------
 arch/arm/mach-omap2/gpmc.h                   |   6 -
 drivers/mtd/nand/omap2.c                     |  76 ++++++------
 include/linux/platform_data/mtd-nand-omap2.h |   2 +
 5 files changed, 48 insertions(+), 205 deletions(-)

diff --git a/arch/arm/mach-omap2/gpmc-nand.c b/arch/arm/mach-omap2/gpmc-nand.c
index d536575..775b263 100644
--- a/arch/arm/mach-omap2/gpmc-nand.c
+++ b/arch/arm/mach-omap2/gpmc-nand.c
@@ -34,9 +34,6 @@ static struct resource gpmc_nand_resource[] = {
 	{
 		.flags		= IORESOURCE_IRQ,
 	},
-	{
-		.flags		= IORESOURCE_IRQ,
-	},
 };
 
 static struct platform_device gpmc_nand_device = {
diff --git a/arch/arm/mach-omap2/gpmc.c b/arch/arm/mach-omap2/gpmc.c
index f4ee6e9..e28864d 100644
--- a/arch/arm/mach-omap2/gpmc.c
+++ b/arch/arm/mach-omap2/gpmc.c
@@ -111,17 +111,8 @@
 
 #define GPMC_NR_WAITPINS		4
 
-/* XXX: Only NAND irq has been considered,currently these are the only ones used
- */
-#define	GPMC_NR_IRQ		2
-
 static struct gpmc_omap_platform_data gpmc_pdata;
 
-struct gpmc_client_irq	{
-	unsigned		irq;
-	u32			bitmask;
-};
-
 /* Structure to save gpmc cs context */
 struct gpmc_cs_config {
 	u32 config1;
@@ -149,10 +140,6 @@ struct omap3_gpmc_regs {
 	struct gpmc_cs_config cs_context[GPMC_CS_NUM];
 };
 
-static struct gpmc_client_irq gpmc_client_irq[GPMC_NR_IRQ];
-static struct irq_chip gpmc_irq_chip;
-static int gpmc_irq_start;
-
 static struct resource	gpmc_mem_root;
 static struct resource	gpmc_cs_mem[GPMC_CS_NUM];
 static DEFINE_SPINLOCK(gpmc_mem_lock);
@@ -167,8 +154,6 @@ static void __iomem *gpmc_base;
 
 static struct clk *gpmc_l3_clk;
 
-static irqreturn_t gpmc_handle_irq(int irq, void *dev);
-
 static void gpmc_write_reg(int idx, u32 val)
 {
 	__raw_writel(val, gpmc_base + idx);
@@ -623,14 +608,6 @@ int gpmc_configure(int cmd, int wval)
 	u32 regval;
 
 	switch (cmd) {
-	case GPMC_ENABLE_IRQ:
-		gpmc_write_reg(GPMC_IRQENABLE, wval);
-		break;
-
-	case GPMC_SET_IRQ_STATUS:
-		gpmc_write_reg(GPMC_IRQSTATUS, wval);
-		break;
-
 	case GPMC_CONFIG_WP:
 		regval = gpmc_read_reg(GPMC_CONFIG);
 		if (wval)
@@ -654,6 +631,8 @@ void gpmc_update_nand_reg(struct gpmc_nand_regs *reg, int cs)
 	int i;
 
 	reg->gpmc_status = gpmc_base + GPMC_STATUS;
+	reg->gpmc_irqstatus = gpmc_base + GPMC_IRQSTATUS;
+	reg->gpmc_irqenable = gpmc_base + GPMC_IRQENABLE;
 	reg->gpmc_nand_command = gpmc_base + GPMC_CS0_OFFSET +
 				GPMC_CS_NAND_COMMAND + GPMC_CS_SIZE * cs;
 	reg->gpmc_nand_address = gpmc_base + GPMC_CS0_OFFSET +
@@ -681,115 +660,6 @@ void gpmc_update_nand_reg(struct gpmc_nand_regs *reg, int cs)
 	}
 }
 
-int gpmc_get_client_irq(unsigned irq_config)
-{
-	int i;
-
-	if (hweight32(irq_config) > 1)
-		return 0;
-
-	for (i = 0; i < GPMC_NR_IRQ; i++)
-		if (gpmc_client_irq[i].bitmask & irq_config)
-			return gpmc_client_irq[i].irq;
-
-	return 0;
-}
-
-static int gpmc_irq_endis(unsigned irq, bool endis)
-{
-	int i;
-	u32 regval;
-
-	for (i = 0; i < GPMC_NR_IRQ; i++)
-		if (irq == gpmc_client_irq[i].irq) {
-			regval = gpmc_read_reg(GPMC_IRQENABLE);
-			if (endis)
-				regval |= gpmc_client_irq[i].bitmask;
-			else
-				regval &= ~gpmc_client_irq[i].bitmask;
-			gpmc_write_reg(GPMC_IRQENABLE, regval);
-			break;
-		}
-
-	return 0;
-}
-
-static void gpmc_irq_disable(struct irq_data *p)
-{
-	gpmc_irq_endis(p->irq, false);
-}
-
-static void gpmc_irq_enable(struct irq_data *p)
-{
-	gpmc_irq_endis(p->irq, true);
-}
-
-static void gpmc_irq_noop(struct irq_data *data) { }
-
-static unsigned int gpmc_irq_noop_ret(struct irq_data *data) { return 0; }
-
-static int gpmc_setup_irq(void)
-{
-	int i;
-	u32 regval;
-
-	if (!gpmc_irq)
-		return -EINVAL;
-
-	gpmc_irq_start = irq_alloc_descs(-1, 0, GPMC_NR_IRQ, 0);
-	if (gpmc_irq_start < 0) {
-		pr_err("irq_alloc_descs failed\n");
-		return gpmc_irq_start;
-	}
-
-	gpmc_irq_chip.name = "gpmc";
-	gpmc_irq_chip.irq_startup = gpmc_irq_noop_ret;
-	gpmc_irq_chip.irq_enable = gpmc_irq_enable;
-	gpmc_irq_chip.irq_disable = gpmc_irq_disable;
-	gpmc_irq_chip.irq_shutdown = gpmc_irq_noop;
-	gpmc_irq_chip.irq_ack = gpmc_irq_noop;
-	gpmc_irq_chip.irq_mask = gpmc_irq_noop;
-	gpmc_irq_chip.irq_unmask = gpmc_irq_noop;
-
-	gpmc_client_irq[0].bitmask = GPMC_IRQ_FIFOEVENTENABLE;
-	gpmc_client_irq[1].bitmask = GPMC_IRQ_COUNT_EVENT;
-
-	for (i = 0; i < GPMC_NR_IRQ; i++) {
-		gpmc_client_irq[i].irq = gpmc_irq_start + i;
-		irq_set_chip_and_handler(gpmc_client_irq[i].irq,
-					&gpmc_irq_chip, handle_simple_irq);
-		set_irq_flags(gpmc_client_irq[i].irq,
-				IRQF_VALID | IRQF_NOAUTOEN);
-	}
-
-	/* Disable interrupts */
-	gpmc_write_reg(GPMC_IRQENABLE, 0);
-
-	/* clear interrupts */
-	regval = gpmc_read_reg(GPMC_IRQSTATUS);
-	gpmc_write_reg(GPMC_IRQSTATUS, regval);
-
-	return request_irq(gpmc_irq, gpmc_handle_irq, 0, "gpmc", NULL);
-}
-
-static int gpmc_free_irq(void)
-{
-	int i;
-
-	if (gpmc_irq)
-		free_irq(gpmc_irq, NULL);
-
-	for (i = 0; i < GPMC_NR_IRQ; i++) {
-		irq_set_handler(gpmc_client_irq[i].irq, NULL);
-		irq_set_chip(gpmc_client_irq[i].irq, &no_irq_chip);
-		irq_modify_status(gpmc_client_irq[i].irq, 0, 0);
-	}
-
-	irq_free_descs(gpmc_irq_start, GPMC_NR_IRQ);
-
-	return 0;
-}
-
 static void gpmc_mem_exit(void)
 {
 	int cs;
@@ -1643,9 +1513,7 @@ static int gpmc_nand_setup(struct platform_device *pdev,
 	res = pdev->resource;
 
 	/* setup IRQ resources */
-	res[1].start = gpmc_get_client_irq(GPMC_IRQ_FIFOEVENTENABLE);
-
-	res[2].start = gpmc_get_client_irq(GPMC_IRQ_COUNT_EVENT);
+	res[1].start = gpmc_irq;
 
 	settings->device_nand = true;
 	/*
@@ -1671,11 +1539,10 @@ static int gpmc_nand_check_csdata(struct gpmc_omap_cs_data *cs)
 
 	res = cs->pdev->resource;
 
-	if (cs->pdev->num_resources < 3)
+	if (cs->pdev->num_resources < 2)
 		return -EINVAL;
 
-	if (resource_type(&res[1]) != IORESOURCE_IRQ ||
-	    resource_type(&res[2]) != IORESOURCE_IRQ)
+	if (resource_type(&res[1]) != IORESOURCE_IRQ)
 		return -EINVAL;
 
 	if (!cs->settings)
@@ -1853,9 +1720,6 @@ static int gpmc_probe(struct platform_device *pdev)
 
 	gpmc_mem_init();
 
-	if (gpmc_setup_irq() < 0)
-		dev_warn(dev, "gpmc_setup_irq failed\n");
-
 	/* Now the GPMC is initialised, unreserve the chip-selects */
 	gpmc_cs_map = 0;
 
@@ -1886,7 +1750,6 @@ error:
 
 static int gpmc_remove(struct platform_device *pdev)
 {
-	gpmc_free_irq();
 	gpmc_mem_exit();
 	pm_runtime_put_sync(&pdev->dev);
 	pm_runtime_disable(&pdev->dev);
@@ -2032,25 +1895,6 @@ int gpmc_generic_init(int cs, enum gpmc_omap_type type,
 	return 0;
 }
 
-static irqreturn_t gpmc_handle_irq(int irq, void *dev)
-{
-	int i;
-	u32 regval;
-
-	regval = gpmc_read_reg(GPMC_IRQSTATUS);
-
-	if (!regval)
-		return IRQ_NONE;
-
-	for (i = 0; i < GPMC_NR_IRQ; i++)
-		if (regval & gpmc_client_irq[i].bitmask)
-			generic_handle_irq(gpmc_client_irq[i].irq);
-
-	gpmc_write_reg(GPMC_IRQSTATUS, regval);
-
-	return IRQ_HANDLED;
-}
-
 static struct omap3_gpmc_regs gpmc_context;
 
 /*
diff --git a/arch/arm/mach-omap2/gpmc.h b/arch/arm/mach-omap2/gpmc.h
index 795502f..906eefe 100644
--- a/arch/arm/mach-omap2/gpmc.h
+++ b/arch/arm/mach-omap2/gpmc.h
@@ -26,14 +26,8 @@
 #define GPMC_CS_NAND_DATA	0x24
 
 /* Control Commands */
-#define GPMC_CONFIG_RDY_BSY	0x00000001
-#define GPMC_CONFIG_DEV_SIZE	0x00000002
-#define GPMC_CONFIG_DEV_TYPE	0x00000003
-#define GPMC_SET_IRQ_STATUS	0x00000004
 #define GPMC_CONFIG_WP		0x00000005
 
-#define GPMC_ENABLE_IRQ		0x0000000d
-
 /* ECC commands */
 #define GPMC_ECC_READ		0 /* Reset Hardware ECC for read */
 #define GPMC_ECC_WRITE		1 /* Reset Hardware ECC for write */
diff --git a/drivers/mtd/nand/omap2.c b/drivers/mtd/nand/omap2.c
index 1b800bc..ad9d28f 100644
--- a/drivers/mtd/nand/omap2.c
+++ b/drivers/mtd/nand/omap2.c
@@ -136,6 +136,10 @@
 
 #define BADBLOCK_MARKER_LENGTH		2
 
+/* GPMC IRQ REGISTER bits */
+#define GPMC_IRQ_FIFOEVENT	BIT(0)
+#define GPMC_IRQ_TERMCOUNT	BIT(1)
+
 #ifdef CONFIG_MTD_NAND_OMAP_BCH
 static u_char bch8_vector[] = {0xf3, 0xdb, 0x14, 0x16, 0x8b, 0xd2, 0xbe, 0xcc,
 	0xac, 0x6b, 0xff, 0x99, 0x7b};
@@ -157,8 +161,7 @@ struct omap_nand_info {
 	enum omap_ecc			ecc_opt;
 	struct completion		comp;
 	struct dma_chan			*dma;
-	int				gpmc_irq_fifo;
-	int				gpmc_irq_count;
+	int				gpmc_irq;
 	enum {
 		OMAP_NAND_IO_READ = 0,	/* read */
 		OMAP_NAND_IO_WRITE,	/* write */
@@ -572,12 +575,16 @@ static irqreturn_t omap_nand_irq(int this_irq, void *dev)
 {
 	struct omap_nand_info *info = (struct omap_nand_info *) dev;
 	u32 bytes;
+	u32 irqstatus;
+	u32 irqenable;
+
+	irqstatus = readl(info->reg.gpmc_irqstatus);
 
 	bytes = readl(info->reg.gpmc_prefetch_status);
 	bytes = PREFETCH_STATUS_FIFO_CNT(bytes);
 	bytes = bytes  & 0xFFFC; /* io in multiple of 4 bytes */
 	if (info->iomode == OMAP_NAND_IO_WRITE) { /* checks for write io */
-		if (this_irq == info->gpmc_irq_count)
+		if (irqstatus & GPMC_IRQ_TERMCOUNT)
 			goto done;
 
 		if (info->buf_len && (info->buf_len < bytes))
@@ -594,17 +601,27 @@ static irqreturn_t omap_nand_irq(int this_irq, void *dev)
 						(u32 *)info->buf, bytes >> 2);
 		info->buf = info->buf + bytes;
 
-		if (this_irq == info->gpmc_irq_count)
+		if (irqstatus & GPMC_IRQ_TERMCOUNT)
 			goto done;
 	}
 
+	/* Clear FIFOEVENT STATUS */
+	irqstatus &= ~GPMC_IRQ_FIFOEVENT;
+	writel(irqstatus, info->reg.gpmc_irqstatus);
+
 	return IRQ_HANDLED;
 
 done:
 	complete(&info->comp);
 
-	disable_irq_nosync(info->gpmc_irq_fifo);
-	disable_irq_nosync(info->gpmc_irq_count);
+	/* Clear FIFOEVENT and TERMCOUNT STATUS */
+	irqstatus &= ~(GPMC_IRQ_TERMCOUNT | GPMC_IRQ_FIFOEVENT);
+	writel(irqstatus, info->reg.gpmc_irqstatus);
+
+	/* Disable Interrupt generation */
+	irqenable = readl(info->reg.gpmc_irqenable);
+	irqenable &= ~(GPMC_IRQ_TERMCOUNT | GPMC_IRQ_FIFOEVENT);
+	writel(irqenable, info->reg.gpmc_irqenable);
 
 	return IRQ_HANDLED;
 }
@@ -620,6 +637,7 @@ static void omap_read_buf_irq_pref(struct mtd_info *mtd, u_char *buf, int len)
 	struct omap_nand_info *info = container_of(mtd,
 						struct omap_nand_info, mtd);
 	int ret = 0;
+	u32 irqenable;
 
 	if (len <= mtd->oobsize) {
 		omap_read_buf_pref(mtd, buf, len);
@@ -639,8 +657,10 @@ static void omap_read_buf_irq_pref(struct mtd_info *mtd, u_char *buf, int len)
 
 	info->buf_len = len;
 
-	enable_irq(info->gpmc_irq_count);
-	enable_irq(info->gpmc_irq_fifo);
+	/* Enable Interrupt generation */
+	irqenable = readl(info->reg.gpmc_irqenable);
+	irqenable |= (GPMC_IRQ_FIFOEVENT | GPMC_IRQ_TERMCOUNT);
+	writel(irqenable, info->reg.gpmc_irqenable);
 
 	/* waiting for read to complete */
 	wait_for_completion(&info->comp);
@@ -670,6 +690,7 @@ static void omap_write_buf_irq_pref(struct mtd_info *mtd,
 	int ret = 0;
 	unsigned long tim, limit;
 	u32 val;
+	u32 irqenable;
 
 	if (len <= mtd->oobsize) {
 		omap_write_buf_pref(mtd, buf, len);
@@ -689,8 +710,10 @@ static void omap_write_buf_irq_pref(struct mtd_info *mtd,
 
 	info->buf_len = len;
 
-	enable_irq(info->gpmc_irq_count);
-	enable_irq(info->gpmc_irq_fifo);
+	/* Enable Interrupt generation */
+	irqenable = readl(info->reg.gpmc_irqenable);
+	irqenable |= (GPMC_IRQ_FIFOEVENT | GPMC_IRQ_TERMCOUNT);
+	writel(irqenable, info->reg.gpmc_irqenable);
 
 	/* waiting for write to complete */
 	wait_for_completion(&info->comp);
@@ -1689,35 +1712,18 @@ static int omap_nand_probe(struct platform_device *pdev)
 		break;
 
 	case NAND_OMAP_PREFETCH_IRQ:
-		info->gpmc_irq_fifo = platform_get_irq(pdev, 0);
-		if (info->gpmc_irq_fifo <= 0) {
-			dev_err(&pdev->dev, "error getting fifo irq\n");
-			err = -ENODEV;
-			goto return_error;
-		}
-		err = devm_request_irq(&pdev->dev, info->gpmc_irq_fifo,
-					omap_nand_irq, IRQF_SHARED,
-					"gpmc-nand-fifo", info);
-		if (err) {
-			dev_err(&pdev->dev, "requesting irq(%d) error:%d",
-						info->gpmc_irq_fifo, err);
-			info->gpmc_irq_fifo = 0;
-			goto return_error;
-		}
-
-		info->gpmc_irq_count = platform_get_irq(pdev, 1);
-		if (info->gpmc_irq_count <= 0) {
-			dev_err(&pdev->dev, "error getting count irq\n");
-			err = -ENODEV;
+		info->gpmc_irq = platform_get_irq(pdev, 0);
+		if (info->gpmc_irq < 0) {
+			dev_err(&pdev->dev, "error getting GPMC irq\n");
+			err = info->gpmc_irq;
 			goto return_error;
 		}
-		err = devm_request_irq(&pdev->dev, info->gpmc_irq_count,
-					omap_nand_irq, IRQF_SHARED,
-					"gpmc-nand-count", info);
+		err = devm_request_irq(&pdev->dev, info->gpmc_irq,
+				       omap_nand_irq, IRQF_SHARED,
+				       DRIVER_NAME, info);
 		if (err) {
 			dev_err(&pdev->dev, "requesting irq(%d) error:%d",
-						info->gpmc_irq_count, err);
-			info->gpmc_irq_count = 0;
+				info->gpmc_irq, err);
 			goto return_error;
 		}
 
diff --git a/include/linux/platform_data/mtd-nand-omap2.h b/include/linux/platform_data/mtd-nand-omap2.h
index 3e9dd66..97c9852 100644
--- a/include/linux/platform_data/mtd-nand-omap2.h
+++ b/include/linux/platform_data/mtd-nand-omap2.h
@@ -35,6 +35,8 @@ enum omap_ecc {
 
 struct gpmc_nand_regs {
 	void __iomem	*gpmc_status;
+	void __iomem	*gpmc_irqstatus;
+	void __iomem	*gpmc_irqenable;
 	void __iomem	*gpmc_nand_command;
 	void __iomem	*gpmc_nand_address;
 	void __iomem	*gpmc_nand_data;
-- 
1.8.3.2


^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [RFC PATCH 10/16] mtd: nand: omap: Move gpmc_update_nand_reg to nand driver
  2014-05-21 11:20 [RFC PATCH 00/16] OMAP: GPMC: Restructure OMAP GPMC driver (NAND) Roger Quadros
                   ` (8 preceding siblings ...)
  2014-05-21 11:20 ` [RFC PATCH 09/16] mtd: nand: omap: Move IRQ handling from GPMC to NAND driver Roger Quadros
@ 2014-05-21 11:20 ` Roger Quadros
  2014-05-21 11:20 ` [RFC PATCH 11/16] mtd: nand: omap: Move NAND write protect code from GPMC to NAND driver Roger Quadros
                   ` (6 subsequent siblings)
  16 siblings, 0 replies; 28+ messages in thread
From: Roger Quadros @ 2014-05-21 11:20 UTC (permalink / raw)
  To: tony, computersforpeace
  Cc: pekon, ezequiel.garcia, robertcnelson, jg1.han, dwmw2, javier,
	nsekhar, linux-omap, linux-mtd, devicetree, linux-kernel,
	Roger Quadros

GPMC and NAND drivers share the same register space but never use the
same registers. As there is no clear address seperation between the
registers for GPMC and NAND, we can't easily split it up into 2 regions
i.e. one for GPMC and other for NAND. Instead, we simply remap the entire
register space in both the drivers. The NAND driver doesn't re-request
the region as it is already requested by the GPMC driver (parent).

Signed-off-by: Roger Quadros <rogerq@ti.com>
---
 arch/arm/mach-omap2/gpmc-nand.c              |   7 +-
 arch/arm/mach-omap2/gpmc.c                   |  60 +++----------
 arch/arm/mach-omap2/gpmc.h                   |   3 -
 drivers/mtd/nand/omap2.c                     | 123 ++++++++++++++++++++++++---
 include/linux/platform_data/mtd-nand-omap2.h |   3 +-
 5 files changed, 131 insertions(+), 65 deletions(-)

diff --git a/arch/arm/mach-omap2/gpmc-nand.c b/arch/arm/mach-omap2/gpmc-nand.c
index 775b263..d916ec0 100644
--- a/arch/arm/mach-omap2/gpmc-nand.c
+++ b/arch/arm/mach-omap2/gpmc-nand.c
@@ -27,13 +27,18 @@
 static struct resource gpmc_nand_resource[] = {
 /* GPMC driver will fixup all the resources, see gpmc_probe_legacy () */
 	{
+		/* GPMC I/O space */
 		.flags		= IORESOURCE_MEM,
 		.start          = 0,
 		.end            = NAND_IO_SIZE - 1,
 	},
 	{
-		.flags		= IORESOURCE_IRQ,
+		/* GPMC register space */
+		.flags		= IORESOURCE_MEM,
 	},
+	{
+		.flags		= IORESOURCE_IRQ,
+	}
 };
 
 static struct platform_device gpmc_nand_device = {
diff --git a/arch/arm/mach-omap2/gpmc.c b/arch/arm/mach-omap2/gpmc.c
index e28864d..4207dc9 100644
--- a/arch/arm/mach-omap2/gpmc.c
+++ b/arch/arm/mach-omap2/gpmc.c
@@ -626,40 +626,6 @@ int gpmc_configure(int cmd, int wval)
 }
 EXPORT_SYMBOL(gpmc_configure);
 
-void gpmc_update_nand_reg(struct gpmc_nand_regs *reg, int cs)
-{
-	int i;
-
-	reg->gpmc_status = gpmc_base + GPMC_STATUS;
-	reg->gpmc_irqstatus = gpmc_base + GPMC_IRQSTATUS;
-	reg->gpmc_irqenable = gpmc_base + GPMC_IRQENABLE;
-	reg->gpmc_nand_command = gpmc_base + GPMC_CS0_OFFSET +
-				GPMC_CS_NAND_COMMAND + GPMC_CS_SIZE * cs;
-	reg->gpmc_nand_address = gpmc_base + GPMC_CS0_OFFSET +
-				GPMC_CS_NAND_ADDRESS + GPMC_CS_SIZE * cs;
-	reg->gpmc_nand_data = gpmc_base + GPMC_CS0_OFFSET +
-				GPMC_CS_NAND_DATA + GPMC_CS_SIZE * cs;
-	reg->gpmc_prefetch_config1 = gpmc_base + GPMC_PREFETCH_CONFIG1;
-	reg->gpmc_prefetch_config2 = gpmc_base + GPMC_PREFETCH_CONFIG2;
-	reg->gpmc_prefetch_control = gpmc_base + GPMC_PREFETCH_CONTROL;
-	reg->gpmc_prefetch_status = gpmc_base + GPMC_PREFETCH_STATUS;
-	reg->gpmc_ecc_config = gpmc_base + GPMC_ECC_CONFIG;
-	reg->gpmc_ecc_control = gpmc_base + GPMC_ECC_CONTROL;
-	reg->gpmc_ecc_size_config = gpmc_base + GPMC_ECC_SIZE_CONFIG;
-	reg->gpmc_ecc1_result = gpmc_base + GPMC_ECC1_RESULT;
-
-	for (i = 0; i < GPMC_BCH_NUM_REMAINDER; i++) {
-		reg->gpmc_bch_result0[i] = gpmc_base + GPMC_ECC_BCH_RESULT_0 +
-					   GPMC_BCH_SIZE * i;
-		reg->gpmc_bch_result1[i] = gpmc_base + GPMC_ECC_BCH_RESULT_1 +
-					   GPMC_BCH_SIZE * i;
-		reg->gpmc_bch_result2[i] = gpmc_base + GPMC_ECC_BCH_RESULT_2 +
-					   GPMC_BCH_SIZE * i;
-		reg->gpmc_bch_result3[i] = gpmc_base + GPMC_ECC_BCH_RESULT_3 +
-					   GPMC_BCH_SIZE * i;
-	}
-}
-
 static void gpmc_mem_exit(void)
 {
 	int cs;
@@ -1499,21 +1465,25 @@ static int gpmc_probe_dt(struct platform_device *pdev)
 }
 #endif
 
-static int gpmc_nand_setup(struct platform_device *pdev,
+/* Configure nand device resources */
+static int gpmc_nand_setup(struct platform_device *parent_pdev,
+			   struct platform_device *pdev,
 			   struct gpmc_settings *settings)
 {
 	struct resource *res;
 	int rc;
-	struct omap_nand_platform_data *gpmc_nand_data;
+	struct resource *res_mem;
 
-	gpmc_nand_data = pdev->dev.platform_data;
-	if (!gpmc_nand_data)
+	/* GPMC register space */
+	res_mem = platform_get_resource(parent_pdev, IORESOURCE_MEM, 0);
+	if (!res_mem)
 		return -EINVAL;
 
 	res = pdev->resource;
+	res[1] = *res_mem;
 
-	/* setup IRQ resources */
-	res[1].start = gpmc_irq;
+	/* IRQ resource */
+	res[2].start = gpmc_irq;
 
 	settings->device_nand = true;
 	/*
@@ -1524,9 +1494,6 @@ static int gpmc_nand_setup(struct platform_device *pdev,
 	if (rc < 0)
 		return rc;
 
-	/* update register addresses in NAND platform data */
-	gpmc_update_nand_reg(&gpmc_nand_data->reg, gpmc_nand_data->cs);
-
 	return 0;
 }
 
@@ -1539,10 +1506,11 @@ static int gpmc_nand_check_csdata(struct gpmc_omap_cs_data *cs)
 
 	res = cs->pdev->resource;
 
-	if (cs->pdev->num_resources < 2)
+	if (cs->pdev->num_resources < 3)
 		return -EINVAL;
 
-	if (resource_type(&res[1]) != IORESOURCE_IRQ)
+	if (resource_type(&res[1]) != IORESOURCE_MEM ||
+	    resource_type(&res[2]) != IORESOURCE_IRQ)
 		return -EINVAL;
 
 	if (!cs->settings)
@@ -1608,7 +1576,7 @@ skip_mem:
 				continue;
 			}
 
-			rc = gpmc_nand_setup(cs->pdev, cs->settings);
+			rc = gpmc_nand_setup(pdev, cs->pdev, cs->settings);
 			if (rc) {
 				dev_err(dev, "Error setting up NAND on CS %d\n",
 					i);
diff --git a/arch/arm/mach-omap2/gpmc.h b/arch/arm/mach-omap2/gpmc.h
index 906eefe..05974f0 100644
--- a/arch/arm/mach-omap2/gpmc.h
+++ b/arch/arm/mach-omap2/gpmc.h
@@ -21,9 +21,6 @@
 #define GPMC_CS_CONFIG5		0x10
 #define GPMC_CS_CONFIG6		0x14
 #define GPMC_CS_CONFIG7		0x18
-#define GPMC_CS_NAND_COMMAND	0x1c
-#define GPMC_CS_NAND_ADDRESS	0x20
-#define GPMC_CS_NAND_DATA	0x24
 
 /* Control Commands */
 #define GPMC_CONFIG_WP		0x00000005
diff --git a/drivers/mtd/nand/omap2.c b/drivers/mtd/nand/omap2.c
index ad9d28f..7d12d92 100644
--- a/drivers/mtd/nand/omap2.c
+++ b/drivers/mtd/nand/omap2.c
@@ -140,6 +140,36 @@
 #define GPMC_IRQ_FIFOEVENT	BIT(0)
 #define GPMC_IRQ_TERMCOUNT	BIT(1)
 
+/* GPMC register offsets */
+#define GPMC_REVISION		0x00
+#define GPMC_SYSCONFIG		0x10
+#define GPMC_SYSSTATUS		0x14
+#define GPMC_IRQSTATUS		0x18
+#define GPMC_IRQENABLE		0x1c
+#define GPMC_TIMEOUT_CONTROL	0x40
+#define GPMC_ERR_ADDRESS	0x44
+#define GPMC_ERR_TYPE		0x48
+#define GPMC_CONFIG		0x50
+#define GPMC_STATUS		0x54
+#define GPMC_CS_NAND_COMMAND	0x7c
+#define GPMC_CS_NAND_ADDRESS	0x80
+#define GPMC_CS_NAND_DATA	0x84
+#define GPMC_PREFETCH_CONFIG1	0x1e0
+#define GPMC_PREFETCH_CONFIG2	0x1e4
+#define GPMC_PREFETCH_CONTROL	0x1ec
+#define GPMC_PREFETCH_STATUS	0x1f0
+#define GPMC_ECC_CONFIG		0x1f4
+#define GPMC_ECC_CONTROL	0x1f8
+#define GPMC_ECC_SIZE_CONFIG	0x1fc
+#define GPMC_ECC1_RESULT        0x200
+#define GPMC_ECC_BCH_RESULT_0   0x240   /* not available on OMAP2 */
+#define	GPMC_ECC_BCH_RESULT_1	0x244	/* not available on OMAP2 */
+#define	GPMC_ECC_BCH_RESULT_2	0x248	/* not available on OMAP2 */
+#define	GPMC_ECC_BCH_RESULT_3	0x24c	/* not available on OMAP2 */
+
+#define GPMC_CS_SIZE		0x30
+#define	GPMC_BCH_SIZE		0x10
+
 #ifdef CONFIG_MTD_NAND_OMAP_BCH
 static u_char bch8_vector[] = {0xf3, 0xdb, 0x14, 0x16, 0x8b, 0xd2, 0xbe, 0xcc,
 	0xac, 0x6b, 0xff, 0x99, 0x7b};
@@ -158,6 +188,7 @@ struct omap_nand_info {
 
 	int				gpmc_cs;
 	unsigned long			phys_base;
+	void __iomem			*gpmc_base;
 	enum omap_ecc			ecc_opt;
 	struct completion		comp;
 	struct dma_chan			*dma;
@@ -1584,20 +1615,58 @@ static int is_elm_present(struct omap_nand_info *info,
 }
 #endif /* CONFIG_MTD_NAND_ECC_BCH */
 
+static void gpmc_update_nand_reg(struct omap_nand_info *info)
+{
+	int i;
+	struct gpmc_nand_regs *reg = &info->reg;
+	int cs = info->gpmc_cs;
+	void __iomem *gpmc_base = info->gpmc_base;
+
+	reg->gpmc_status = gpmc_base + GPMC_STATUS;
+	reg->gpmc_irqstatus = gpmc_base + GPMC_IRQSTATUS;
+	reg->gpmc_irqenable = gpmc_base + GPMC_IRQENABLE;
+	reg->gpmc_nand_command = gpmc_base + GPMC_CS_NAND_COMMAND +
+				 GPMC_CS_SIZE * cs;
+	reg->gpmc_nand_address = gpmc_base + GPMC_CS_NAND_ADDRESS +
+				 GPMC_CS_SIZE * cs;
+	reg->gpmc_nand_data = gpmc_base + GPMC_CS_NAND_DATA +
+				 GPMC_CS_SIZE * cs;
+	reg->gpmc_prefetch_config1 = gpmc_base + GPMC_PREFETCH_CONFIG1;
+	reg->gpmc_prefetch_config2 = gpmc_base + GPMC_PREFETCH_CONFIG2;
+	reg->gpmc_prefetch_control = gpmc_base + GPMC_PREFETCH_CONTROL;
+	reg->gpmc_prefetch_status = gpmc_base + GPMC_PREFETCH_STATUS;
+	reg->gpmc_ecc_config = gpmc_base + GPMC_ECC_CONFIG;
+	reg->gpmc_ecc_control = gpmc_base + GPMC_ECC_CONTROL;
+	reg->gpmc_ecc_size_config = gpmc_base + GPMC_ECC_SIZE_CONFIG;
+	reg->gpmc_ecc1_result = gpmc_base + GPMC_ECC1_RESULT;
+
+	for (i = 0; i < GPMC_BCH_NUM_REMAINDER; i++) {
+		reg->gpmc_bch_result0[i] = gpmc_base + GPMC_ECC_BCH_RESULT_0 +
+					   GPMC_BCH_SIZE * i;
+		reg->gpmc_bch_result1[i] = gpmc_base + GPMC_ECC_BCH_RESULT_1 +
+					   GPMC_BCH_SIZE * i;
+		reg->gpmc_bch_result2[i] = gpmc_base + GPMC_ECC_BCH_RESULT_2 +
+					   GPMC_BCH_SIZE * i;
+		reg->gpmc_bch_result3[i] = gpmc_base + GPMC_ECC_BCH_RESULT_3 +
+					   GPMC_BCH_SIZE * i;
+	}
+}
+
 static int omap_nand_probe(struct platform_device *pdev)
 {
-	struct omap_nand_info		*info;
-	struct omap_nand_platform_data	*pdata;
-	struct mtd_info			*mtd;
-	struct nand_chip		*nand_chip;
-	struct nand_ecclayout		*ecclayout;
-	int				err;
-	int				i;
-	dma_cap_mask_t			mask;
-	unsigned			sig;
-	unsigned			oob_index;
-	struct resource			*res;
-	struct mtd_part_parser_data	ppdata = {};
+	struct omap_nand_info *info;
+	struct omap_nand_platform_data *pdata;
+	struct mtd_info	*mtd;
+	struct nand_chip *nand_chip;
+	struct nand_ecclayout *ecclayout;
+	int err;
+	int i;
+	dma_cap_mask_t mask;
+	unsigned sig;
+	unsigned oob_index;
+	struct resource	*res;
+	struct mtd_part_parser_data ppdata = {};
+	struct device *dev = &pdev->dev;
 
 	pdata = dev_get_platdata(&pdev->dev);
 	if (pdata == NULL) {
@@ -1617,7 +1686,6 @@ static int omap_nand_probe(struct platform_device *pdev)
 
 	info->pdev		= pdev;
 	info->gpmc_cs		= pdata->cs;
-	info->reg		= pdata->reg;
 	info->of_node		= pdata->of_node;
 	info->ecc_opt		= pdata->ecc_opt;
 	mtd			= &info->mtd;
@@ -1628,8 +1696,14 @@ static int omap_nand_probe(struct platform_device *pdev)
 	nand_chip->ecc.priv	= NULL;
 	nand_chip->options	|= NAND_SKIP_BBTSCAN;
 
+	/* GPMC external I/O space */
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	nand_chip->IO_ADDR_R = devm_ioremap_resource(&pdev->dev, res);
+	if (!res) {
+		dev_err(dev, "Can't get memory resource 0\n");
+		return -EINVAL;
+	}
+
+	nand_chip->IO_ADDR_R = devm_ioremap_resource(dev, res);
 	if (IS_ERR(nand_chip->IO_ADDR_R))
 		return PTR_ERR(nand_chip->IO_ADDR_R);
 
@@ -1640,6 +1714,27 @@ static int omap_nand_probe(struct platform_device *pdev)
 	nand_chip->IO_ADDR_W = nand_chip->IO_ADDR_R;
 	nand_chip->cmd_ctrl  = omap_hwcontrol;
 
+	/* GPMC internal registers */
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
+	if (!res) {
+		dev_err(dev, "Can't get memory resource 1\n");
+		return -EINVAL;
+	}
+
+	/*
+	 * This resource is already requested by the GPMC driver
+	 * so we can't request it again. Instead, we just ioremap it.
+	 * This driver doesn't access the same registers as the GPMC
+	 * driver so it is safe.
+	 */
+	info->gpmc_base = devm_ioremap(dev, res->start, resource_size(res));
+	if (!info->gpmc_base) {
+		dev_err(dev, "Can't ioremap resource 1\n");
+		return -EADDRNOTAVAIL;
+	}
+
+	gpmc_update_nand_reg(info);
+
 	/*
 	 * If RDY/BSY line is connected to OMAP then use the omap ready
 	 * function and the generic nand_wait function which reads the status
diff --git a/include/linux/platform_data/mtd-nand-omap2.h b/include/linux/platform_data/mtd-nand-omap2.h
index 97c9852..b71cfbd 100644
--- a/include/linux/platform_data/mtd-nand-omap2.h
+++ b/include/linux/platform_data/mtd-nand-omap2.h
@@ -62,10 +62,11 @@ struct omap_nand_platform_data {
 	enum nand_io		xfer_type;
 	int			devsize;
 	enum omap_ecc           ecc_opt;
-	struct gpmc_nand_regs	reg;
 
 	/* for passing the partitions */
 	struct device_node	*of_node;
 	struct device_node	*elm_of_node;
+
+	struct gpmc_nand_regs	reg;		/* deprecated */
 };
 #endif
-- 
1.8.3.2


^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [RFC PATCH 11/16] mtd: nand: omap: Move NAND write protect code from GPMC to NAND driver
  2014-05-21 11:20 [RFC PATCH 00/16] OMAP: GPMC: Restructure OMAP GPMC driver (NAND) Roger Quadros
                   ` (9 preceding siblings ...)
  2014-05-21 11:20 ` [RFC PATCH 10/16] mtd: nand: omap: Move gpmc_update_nand_reg to nand driver Roger Quadros
@ 2014-05-21 11:20 ` Roger Quadros
  2014-05-21 11:21 ` [RFC PATCH 12/16] mtd: nand: omap: Copy platform data parameters to omap_nand_info data Roger Quadros
                   ` (5 subsequent siblings)
  16 siblings, 0 replies; 28+ messages in thread
From: Roger Quadros @ 2014-05-21 11:20 UTC (permalink / raw)
  To: tony, computersforpeace
  Cc: pekon, ezequiel.garcia, robertcnelson, jg1.han, dwmw2, javier,
	nsekhar, linux-omap, linux-mtd, devicetree, linux-kernel,
	Roger Quadros

The write protect (WP) pin is only used for NAND devices. So move
the code into the NAND driver.

Get rid of gpmc_configure() and gpmc_write_reg() as they are no longer
used.

Signed-off-by: Roger Quadros <rogerq@ti.com>
---
 arch/arm/mach-omap2/gpmc.c                   | 42 ----------------------------
 arch/arm/mach-omap2/gpmc.h                   |  4 ---
 drivers/mtd/nand/omap2.c                     | 23 +++++++++++++++
 include/linux/platform_data/mtd-nand-omap2.h |  1 +
 4 files changed, 24 insertions(+), 46 deletions(-)

diff --git a/arch/arm/mach-omap2/gpmc.c b/arch/arm/mach-omap2/gpmc.c
index 4207dc9..132f786 100644
--- a/arch/arm/mach-omap2/gpmc.c
+++ b/arch/arm/mach-omap2/gpmc.c
@@ -154,11 +154,6 @@ static void __iomem *gpmc_base;
 
 static struct clk *gpmc_l3_clk;
 
-static void gpmc_write_reg(int idx, u32 val)
-{
-	__raw_writel(val, gpmc_base + idx);
-}
-
 static u32 gpmc_read_reg(int idx)
 {
 	return __raw_readl(gpmc_base + idx);
@@ -597,35 +592,6 @@ void gpmc_cs_free(int cs)
 }
 EXPORT_SYMBOL(gpmc_cs_free);
 
-/**
- * gpmc_configure - write request to configure gpmc
- * @cmd: command type
- * @wval: value to write
- * @return status of the operation
- */
-int gpmc_configure(int cmd, int wval)
-{
-	u32 regval;
-
-	switch (cmd) {
-	case GPMC_CONFIG_WP:
-		regval = gpmc_read_reg(GPMC_CONFIG);
-		if (wval)
-			regval &= ~GPMC_CONFIG_WRITEPROTECT; /* WP is ON */
-		else
-			regval |= GPMC_CONFIG_WRITEPROTECT;  /* WP is OFF */
-		gpmc_write_reg(GPMC_CONFIG, regval);
-		break;
-
-	default:
-		pr_err("%s: command not supported\n", __func__);
-		return -EINVAL;
-	}
-
-	return 0;
-}
-EXPORT_SYMBOL(gpmc_configure);
-
 static void gpmc_mem_exit(void)
 {
 	int cs;
@@ -1471,7 +1437,6 @@ static int gpmc_nand_setup(struct platform_device *parent_pdev,
 			   struct gpmc_settings *settings)
 {
 	struct resource *res;
-	int rc;
 	struct resource *res_mem;
 
 	/* GPMC register space */
@@ -1486,13 +1451,6 @@ static int gpmc_nand_setup(struct platform_device *parent_pdev,
 	res[2].start = gpmc_irq;
 
 	settings->device_nand = true;
-	/*
-	 * Not sure why WP is explicitly turned OFF. we just moved it here
-	 * as is from mach-omap2/gpmc-nand.c
-	 */
-	rc = gpmc_configure(GPMC_CONFIG_WP, 0);
-	if (rc < 0)
-		return rc;
 
 	return 0;
 }
diff --git a/arch/arm/mach-omap2/gpmc.h b/arch/arm/mach-omap2/gpmc.h
index 05974f0..8ebadf5 100644
--- a/arch/arm/mach-omap2/gpmc.h
+++ b/arch/arm/mach-omap2/gpmc.h
@@ -22,9 +22,6 @@
 #define GPMC_CS_CONFIG6		0x14
 #define GPMC_CS_CONFIG7		0x18
 
-/* Control Commands */
-#define GPMC_CONFIG_WP		0x00000005
-
 /* ECC commands */
 #define GPMC_ECC_READ		0 /* Reset Hardware ECC for read */
 #define GPMC_ECC_WRITE		1 /* Reset Hardware ECC for write */
@@ -57,7 +54,6 @@
 
 #define GPMC_DEVICETYPE_NOR		0
 #define GPMC_DEVICETYPE_NAND		2
-#define GPMC_CONFIG_WRITEPROTECT	0x00000010
 #define WR_RD_PIN_MONITORING		0x00600000
 #define GPMC_IRQ_FIFOEVENTENABLE	0x01
 #define GPMC_IRQ_COUNT_EVENT		0x02
diff --git a/drivers/mtd/nand/omap2.c b/drivers/mtd/nand/omap2.c
index 7d12d92..fee0458 100644
--- a/drivers/mtd/nand/omap2.c
+++ b/drivers/mtd/nand/omap2.c
@@ -140,6 +140,9 @@
 #define GPMC_IRQ_FIFOEVENT	BIT(0)
 #define GPMC_IRQ_TERMCOUNT	BIT(1)
 
+/* GPMC_CONFIG register bits */
+#define GPMC_CONFIG_WRITEPROTECT	BIT(4)
+
 /* GPMC register offsets */
 #define GPMC_REVISION		0x00
 #define GPMC_SYSCONFIG		0x10
@@ -206,6 +209,22 @@ struct omap_nand_info {
 };
 
 /**
+ * omap_nand_writeprotect - Control the WP line to the NAND chip
+ */
+static void omap_nand_writeprotect(struct omap_nand_info *info, bool on)
+{
+	u32 val;
+
+	val = readl(info->reg.gpmc_config);
+	if (on)
+		val |= GPMC_CONFIG_WRITEPROTECT;
+	else
+		val &= GPMC_CONFIG_WRITEPROTECT;
+
+	writel(val, info->reg.gpmc_config);
+}
+
+/**
  * omap_prefetch_enable - configures and starts prefetch transfer
  * @cs: cs (chip select) number
  * @fifo_th: fifo threshold to be used for read/ write
@@ -1622,6 +1641,7 @@ static void gpmc_update_nand_reg(struct omap_nand_info *info)
 	int cs = info->gpmc_cs;
 	void __iomem *gpmc_base = info->gpmc_base;
 
+	reg->gpmc_config = gpmc_base + GPMC_CONFIG;
 	reg->gpmc_status = gpmc_base + GPMC_STATUS;
 	reg->gpmc_irqstatus = gpmc_base + GPMC_IRQSTATUS;
 	reg->gpmc_irqenable = gpmc_base + GPMC_IRQENABLE;
@@ -2029,6 +2049,9 @@ static int omap_nand_probe(struct platform_device *pdev)
 		goto return_error;
 	}
 
+	/* turn off write protect */
+	omap_nand_writeprotect(info, false);
+
 	/* second phase scan */
 	if (nand_scan_tail(mtd)) {
 		err = -ENXIO;
diff --git a/include/linux/platform_data/mtd-nand-omap2.h b/include/linux/platform_data/mtd-nand-omap2.h
index b71cfbd..62a855e 100644
--- a/include/linux/platform_data/mtd-nand-omap2.h
+++ b/include/linux/platform_data/mtd-nand-omap2.h
@@ -34,6 +34,7 @@ enum omap_ecc {
 };
 
 struct gpmc_nand_regs {
+	void __iomem	*gpmc_config;
 	void __iomem	*gpmc_status;
 	void __iomem	*gpmc_irqstatus;
 	void __iomem	*gpmc_irqenable;
-- 
1.8.3.2


^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [RFC PATCH 12/16] mtd: nand: omap: Copy platform data parameters to omap_nand_info data
  2014-05-21 11:20 [RFC PATCH 00/16] OMAP: GPMC: Restructure OMAP GPMC driver (NAND) Roger Quadros
                   ` (10 preceding siblings ...)
  2014-05-21 11:20 ` [RFC PATCH 11/16] mtd: nand: omap: Move NAND write protect code from GPMC to NAND driver Roger Quadros
@ 2014-05-21 11:21 ` Roger Quadros
  2014-05-21 11:21 ` [RFC PATCH 13/16] mtd: nand: omap: True device tree support Roger Quadros
                   ` (4 subsequent siblings)
  16 siblings, 0 replies; 28+ messages in thread
From: Roger Quadros @ 2014-05-21 11:21 UTC (permalink / raw)
  To: tony, computersforpeace
  Cc: pekon, ezequiel.garcia, robertcnelson, jg1.han, dwmw2, javier,
	nsekhar, linux-omap, linux-mtd, devicetree, linux-kernel,
	Roger Quadros

Copy all the platform data parameters to the driver's local data
structure 'omap_nand_info' and use it in the entire driver. This will
make it easer for device tree migration.

Signed-off-by: Roger Quadros <rogerq@ti.com>
---
 drivers/mtd/nand/omap2.c | 27 ++++++++++++++++++---------
 1 file changed, 18 insertions(+), 9 deletions(-)

diff --git a/drivers/mtd/nand/omap2.c b/drivers/mtd/nand/omap2.c
index fee0458..7ff5cb3 100644
--- a/drivers/mtd/nand/omap2.c
+++ b/drivers/mtd/nand/omap2.c
@@ -184,15 +184,19 @@ static struct nand_ecclayout omap_oobinfo;
 
 struct omap_nand_info {
 	struct nand_hw_control		controller;
-	struct omap_nand_platform_data	*pdata;
 	struct mtd_info			mtd;
 	struct nand_chip		nand;
 	struct platform_device		*pdev;
 
 	int				gpmc_cs;
+	bool				dev_ready;
+	enum nand_io			xfer_type;
+	int				devsize;
+	enum omap_ecc			ecc_opt;
+	struct device_node		*elm_of_node;
+
 	unsigned long			phys_base;
 	void __iomem			*gpmc_base;
-	enum omap_ecc			ecc_opt;
 	struct completion		comp;
 	struct dma_chan			*dma;
 	int				gpmc_irq;
@@ -1708,6 +1712,11 @@ static int omap_nand_probe(struct platform_device *pdev)
 	info->gpmc_cs		= pdata->cs;
 	info->of_node		= pdata->of_node;
 	info->ecc_opt		= pdata->ecc_opt;
+	info->dev_ready	= pdata->dev_ready;
+	info->xfer_type = pdata->xfer_type;
+	info->devsize = pdata->devsize;
+	info->elm_of_node = pdata->elm_of_node;
+
 	mtd			= &info->mtd;
 	mtd->priv		= &info->nand;
 	mtd->name		= dev_name(&pdev->dev);
@@ -1762,7 +1771,7 @@ static int omap_nand_probe(struct platform_device *pdev)
 	 * chip delay which is slightly more than tR (AC Timing) of the NAND
 	 * device and read status register until you get a failure or success
 	 */
-	if (pdata->dev_ready) {
+	if (info->dev_ready) {
 		nand_chip->dev_ready = omap_dev_ready;
 		nand_chip->chip_delay = 0;
 	} else {
@@ -1771,7 +1780,7 @@ static int omap_nand_probe(struct platform_device *pdev)
 	}
 
 	/* scan NAND device connected to chip controller */
-	nand_chip->options |= pdata->devsize & NAND_BUSWIDTH_16;
+	nand_chip->options |= info->devsize & NAND_BUSWIDTH_16;
 	if (nand_scan_ident(mtd, 1, NULL)) {
 		pr_err("nand device scan failed, may be bus-width mismatch\n");
 		err = -ENXIO;
@@ -1779,14 +1788,14 @@ static int omap_nand_probe(struct platform_device *pdev)
 	}
 
 	/* check for small page devices */
-	if ((mtd->oobsize < 64) && (pdata->ecc_opt != OMAP_ECC_HAM1_CODE_HW)) {
+	if ((mtd->oobsize < 64) && (info->ecc_opt != OMAP_ECC_HAM1_CODE_HW)) {
 		pr_err("small page devices are not supported\n");
 		err = -EINVAL;
 		goto return_error;
 	}
 
 	/* re-populate low-level callbacks based on xfer modes */
-	switch (pdata->xfer_type) {
+	switch (info->xfer_type) {
 	case NAND_OMAP_PREFETCH_POLLED:
 		nand_chip->read_buf   = omap_read_buf_pref;
 		nand_chip->write_buf  = omap_write_buf_pref;
@@ -1849,7 +1858,7 @@ static int omap_nand_probe(struct platform_device *pdev)
 
 	default:
 		dev_err(&pdev->dev,
-			"xfer_type(%d) not supported!\n", pdata->xfer_type);
+			"xfer_type(%d) not supported!\n", info->xfer_type);
 		err = -EINVAL;
 		goto return_error;
 	}
@@ -1945,7 +1954,7 @@ static int omap_nand_probe(struct platform_device *pdev)
 		ecclayout->oobfree->offset	=
 				ecclayout->eccpos[ecclayout->eccbytes - 1] + 1;
 		/* This ECC scheme requires ELM H/W block */
-		if (is_elm_present(info, pdata->elm_of_node, BCH4_ECC) < 0) {
+		if (is_elm_present(info, info->elm_of_node, BCH4_ECC) < 0) {
 			pr_err("nand: error: could not initialize ELM\n");
 			err = -ENODEV;
 			goto return_error;
@@ -2011,7 +2020,7 @@ static int omap_nand_probe(struct platform_device *pdev)
 		nand_chip->ecc.read_page	= omap_read_page_bch;
 		nand_chip->ecc.write_page	= omap_write_page_bch;
 		/* This ECC scheme requires ELM H/W block */
-		err = is_elm_present(info, pdata->elm_of_node, BCH8_ECC);
+		err = is_elm_present(info, info->elm_of_node, BCH8_ECC);
 		if (err < 0) {
 			pr_err("nand: error: could not initialize ELM\n");
 			goto return_error;
-- 
1.8.3.2


^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [RFC PATCH 13/16] mtd: nand: omap: True device tree support
  2014-05-21 11:20 [RFC PATCH 00/16] OMAP: GPMC: Restructure OMAP GPMC driver (NAND) Roger Quadros
                   ` (11 preceding siblings ...)
  2014-05-21 11:21 ` [RFC PATCH 12/16] mtd: nand: omap: Copy platform data parameters to omap_nand_info data Roger Quadros
@ 2014-05-21 11:21 ` Roger Quadros
  2014-05-21 11:21 ` [RFC PATCH 14/16] ARM: OMAP: gpmc: Update DT binding documentation Roger Quadros
                   ` (3 subsequent siblings)
  16 siblings, 0 replies; 28+ messages in thread
From: Roger Quadros @ 2014-05-21 11:21 UTC (permalink / raw)
  To: tony, computersforpeace
  Cc: pekon, ezequiel.garcia, robertcnelson, jg1.han, dwmw2, javier,
	nsekhar, linux-omap, linux-mtd, devicetree, linux-kernel,
	Roger Quadros

The NAND controller must be a child node of the Chip select (CS) node.
NAND specific parameters are moved to the NAND node.

Move NAND specific device tree parsing to NAND driver.

Signed-off-by: Roger Quadros <rogerq@ti.com>
---
 arch/arm/mach-omap2/gpmc.c                   | 183 +++++++++++++--------------
 drivers/mtd/nand/omap2.c                     | 124 +++++++++++++++---
 include/linux/platform_data/mtd-nand-omap2.h |   6 +-
 3 files changed, 195 insertions(+), 118 deletions(-)

diff --git a/arch/arm/mach-omap2/gpmc.c b/arch/arm/mach-omap2/gpmc.c
index 132f786..6493010 100644
--- a/arch/arm/mach-omap2/gpmc.c
+++ b/arch/arm/mach-omap2/gpmc.c
@@ -27,9 +27,7 @@
 #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>
 #include <linux/pm_runtime.h>
 
 #include <linux/platform_data/mtd-nand-omap2.h>
@@ -40,7 +38,6 @@
 #include "common.h"
 #include "omap_device.h"
 #include "gpmc.h"
-#include "gpmc-nand.h"
 #include "gpmc-onenand.h"
 
 #define	DEVICE_NAME		"omap-gpmc"
@@ -1153,96 +1150,6 @@ static void __maybe_unused gpmc_read_timings_dt(struct device_node *np,
 		of_property_read_bool(np, "gpmc,time-para-granularity");
 }
 
-#if IS_ENABLED(CONFIG_MTD_NAND)
-
-static const char * const nand_xfer_types[] = {
-	[NAND_OMAP_PREFETCH_POLLED]		= "prefetch-polled",
-	[NAND_OMAP_POLLED]			= "polled",
-	[NAND_OMAP_PREFETCH_DMA]		= "prefetch-dma",
-	[NAND_OMAP_PREFETCH_IRQ]		= "prefetch-irq",
-};
-
-static int gpmc_probe_nand_child(struct platform_device *pdev,
-				 struct device_node *child)
-{
-	u32 val;
-	const char *s;
-	struct gpmc_timings gpmc_t;
-	struct omap_nand_platform_data *gpmc_nand_data;
-
-	if (of_property_read_u32(child, "reg", &val) < 0) {
-		dev_err(&pdev->dev, "%s has no 'reg' property\n",
-			child->full_name);
-		return -ENODEV;
-	}
-
-	gpmc_nand_data = devm_kzalloc(&pdev->dev, sizeof(*gpmc_nand_data),
-				      GFP_KERNEL);
-	if (!gpmc_nand_data)
-		return -ENOMEM;
-
-	gpmc_nand_data->cs = val;
-	gpmc_nand_data->of_node = child;
-
-	/* Detect availability of ELM module */
-	gpmc_nand_data->elm_of_node = of_parse_phandle(child, "ti,elm-id", 0);
-	if (gpmc_nand_data->elm_of_node == NULL)
-		gpmc_nand_data->elm_of_node =
-					of_parse_phandle(child, "elm_id", 0);
-	if (gpmc_nand_data->elm_of_node == NULL)
-		pr_warn("%s: ti,elm-id property not found\n", __func__);
-
-	/* select ecc-scheme for NAND */
-	if (of_property_read_string(child, "ti,nand-ecc-opt", &s)) {
-		pr_err("%s: ti,nand-ecc-opt not found\n", __func__);
-		return -ENODEV;
-	}
-	if (!strcmp(s, "ham1") || !strcmp(s, "sw") ||
-		!strcmp(s, "hw") || !strcmp(s, "hw-romcode"))
-		gpmc_nand_data->ecc_opt =
-				OMAP_ECC_HAM1_CODE_HW;
-	else if (!strcmp(s, "bch4"))
-		if (gpmc_nand_data->elm_of_node)
-			gpmc_nand_data->ecc_opt =
-				OMAP_ECC_BCH4_CODE_HW;
-		else
-			gpmc_nand_data->ecc_opt =
-				OMAP_ECC_BCH4_CODE_HW_DETECTION_SW;
-	else if (!strcmp(s, "bch8"))
-		if (gpmc_nand_data->elm_of_node)
-			gpmc_nand_data->ecc_opt =
-				OMAP_ECC_BCH8_CODE_HW;
-		else
-			gpmc_nand_data->ecc_opt =
-				OMAP_ECC_BCH8_CODE_HW_DETECTION_SW;
-	else
-		pr_err("%s: ti,nand-ecc-opt invalid value\n", __func__);
-
-	/* select data transfer mode for NAND controller */
-	if (!of_property_read_string(child, "ti,nand-xfer-type", &s))
-		for (val = 0; val < ARRAY_SIZE(nand_xfer_types); val++)
-			if (!strcasecmp(s, nand_xfer_types[val])) {
-				gpmc_nand_data->xfer_type = val;
-				break;
-			}
-
-	val = of_get_nand_bus_width(child);
-	if (val == 16)
-		gpmc_nand_data->devsize = NAND_BUSWIDTH_16;
-
-	gpmc_read_timings_dt(child, &gpmc_t);
-	gpmc_nand_init(gpmc_nand_data, &gpmc_t);
-
-	return 0;
-}
-#else
-static int gpmc_probe_nand_child(struct platform_device *pdev,
-				 struct device_node *child)
-{
-	return 0;
-}
-#endif
-
 #if IS_ENABLED(CONFIG_MTD_ONENAND)
 static int gpmc_probe_onenand_child(struct platform_device *pdev,
 				 struct device_node *child)
@@ -1372,6 +1279,90 @@ err:
 	return ret;
 }
 
+static int gpmc_probe_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;
+	struct device *dev = &pdev->dev;
+	const char *child_name = child->full_name;
+	u32 val;
+
+	if (of_property_read_u32(child, "gpmc,cs", &cs) < 0) {
+		dev_err(dev, "%s: has no 'gpmc,cs' property\n", child_name);
+		return -EINVAL;
+	}
+
+	if (of_address_to_resource(child, 0, &res) < 0) {
+		dev_err(dev, "%s: has malformed 'reg' property\n", child_name);
+		return -EINVAL;
+	}
+
+	ret = gpmc_cs_request(cs, resource_size(&res), &base);
+	if (ret < 0) {
+		dev_err(dev, "%s: cannot request GPMC CS %d\n", child_name, cs);
+		return ret;
+	}
+
+	ret = gpmc_cs_remap(cs, res.start);
+	if (ret < 0) {
+		dev_err(dev, "%s: cannot remap GPMC CS %d to %pa\n",
+			child_name, cs, (void *)res.start);
+		goto err;
+	}
+
+	gpmc_read_settings_dt(child, &gpmc_s);
+
+	ret = of_property_read_u32(child, "gpmc,device-width", &val);
+	if (ret) {
+		dev_err(dev, "%s: has no 'gpmc,device-width' property\n",
+			child_name);
+		goto err;
+	}
+
+	switch (val) {
+	case 1:
+		gpmc_s.device_width = GPMC_DEVWIDTH_8BIT;
+		break;
+	case 2:
+		gpmc_s.device_width = GPMC_DEVWIDTH_16BIT;
+		break;
+	default:
+		dev_err(dev, "%s: invalid 'gpmc,device-width'\n", child_name);
+		ret = -EINVAL;
+		goto err;
+	}
+
+	gpmc_s.device_nand = of_property_read_bool(child, "gpmc,nand");
+
+	ret = gpmc_cs_program_settings(cs, &gpmc_s);
+	if (ret < 0) {
+		dev_err(dev, "%s: failed to program GPMC settings\n",
+			child_name);
+		goto err;
+	}
+
+	gpmc_read_timings_dt(child, &gpmc_t);
+	gpmc_cs_set_timings(cs, &gpmc_t);
+
+	ret = of_platform_populate(child, NULL, NULL, &pdev->dev);
+	if (ret) {
+		dev_err(&pdev->dev, "failed to create gpmc child %s\n",
+			child_name);
+		goto err;
+	}
+
+	return 0;
+
+err:
+	gpmc_cs_free(cs);
+
+	return ret;
+}
+
 static int gpmc_probe_dt(struct platform_device *pdev)
 {
 	int ret;
@@ -1408,14 +1399,14 @@ static int gpmc_probe_dt(struct platform_device *pdev)
 		if (!child->name)
 			continue;
 
-		if (of_node_cmp(child->name, "nand") == 0)
-			ret = gpmc_probe_nand_child(pdev, child);
-		else if (of_node_cmp(child->name, "onenand") == 0)
+		if (of_node_cmp(child->name, "onenand") == 0)
 			ret = gpmc_probe_onenand_child(pdev, child);
 		else if (of_node_cmp(child->name, "ethernet") == 0 ||
 			 of_node_cmp(child->name, "nor") == 0 ||
 			 of_node_cmp(child->name, "uart") == 0)
 			ret = gpmc_probe_generic_child(pdev, child);
+		else
+			ret = gpmc_probe_child(pdev, child);
 
 		if (WARN(ret < 0, "%s: probing gpmc child %s failed\n",
 			 __func__, child->full_name))
diff --git a/drivers/mtd/nand/omap2.c b/drivers/mtd/nand/omap2.c
index 7ff5cb3..4ca1f48 100644
--- a/drivers/mtd/nand/omap2.c
+++ b/drivers/mtd/nand/omap2.c
@@ -24,6 +24,7 @@
 #include <linux/slab.h>
 #include <linux/of.h>
 #include <linux/of_device.h>
+#include <linux/of_mtd.h>
 
 #include <linux/mtd/nand_bch.h>
 #include <linux/platform_data/elm.h>
@@ -1676,10 +1677,81 @@ static void gpmc_update_nand_reg(struct omap_nand_info *info)
 	}
 }
 
+static const char * const nand_xfer_types[] = {
+	[NAND_OMAP_PREFETCH_POLLED] = "prefetch-polled",
+	[NAND_OMAP_POLLED] = "polled",
+	[NAND_OMAP_PREFETCH_DMA] = "prefetch-dma",
+	[NAND_OMAP_PREFETCH_IRQ] = "prefetch-irq",
+};
+
+static int omap_get_dt_info(struct device *dev, struct omap_nand_info *info)
+{
+	struct device_node *child = dev->of_node;
+	int i;
+	const char *s;
+
+	if (of_property_read_u32(child, "ti,nand-cs", &info->gpmc_cs)) {
+		dev_err(dev, "ti,nand-cs not found in DT\n");
+		return -EINVAL;
+	}
+
+	/* detect availability of ELM module. Won't be present pre-OMAP4 */
+	info->elm_of_node = of_parse_phandle(child, "ti,elm-id", 0);
+	if (info->elm_of_node == NULL)
+		dev_dbg(dev, "ti,elm-id not in DT\n");
+
+	/* select ecc-scheme */
+	if (of_property_read_string(child, "ti,nand-ecc-opt", &s)) {
+		/* default to HAM1 */
+		info->ecc_opt = OMAP_ECC_HAM1_CODE_HW;
+		goto ecc_done;
+	}
+
+	if (!strcmp(s, "ham1") || !strcmp(s, "sw") ||
+	    !strcmp(s, "hw") || !strcmp(s, "hw-romcode")) {
+		info->ecc_opt = OMAP_ECC_HAM1_CODE_HW;
+
+	} else if (!strcmp(s, "bch4")) {
+		if (info->elm_of_node)
+			info->ecc_opt = OMAP_ECC_BCH4_CODE_HW;
+		else
+			info->ecc_opt = OMAP_ECC_BCH4_CODE_HW_DETECTION_SW;
+
+	} else if (!strcmp(s, "bch8")) {
+		if (info->elm_of_node)
+			info->ecc_opt = OMAP_ECC_BCH8_CODE_HW;
+		else
+			info->ecc_opt = OMAP_ECC_BCH8_CODE_HW_DETECTION_SW;
+	} else {
+		dev_err(dev, "unrecognized value for ti,nand-ecc-opt\n");
+		return -EINVAL;
+	}
+
+ecc_done:
+	/* select data transfer mode */
+	if (!of_property_read_string(child, "ti,nand-xfer-type", &s)) {
+		for (i = 0; i < ARRAY_SIZE(nand_xfer_types); i++) {
+			if (!strcasecmp(s, nand_xfer_types[i])) {
+				info->xfer_type = i;
+				goto found;
+			}
+		}
+
+		dev_err(dev, "unrecognized value for ti,nand-xfer-type\n");
+		return -EINVAL;
+	}
+
+found:
+	if (of_get_nand_bus_width(child) == 16)
+		info->devsize = NAND_BUSWIDTH_16;
+
+	return 0;
+}
+
 static int omap_nand_probe(struct platform_device *pdev)
 {
 	struct omap_nand_info *info;
-	struct omap_nand_platform_data *pdata;
+	struct omap_nand_platform_data *pdata = NULL;
 	struct mtd_info	*mtd;
 	struct nand_chip *nand_chip;
 	struct nand_ecclayout *ecclayout;
@@ -1692,31 +1764,37 @@ static int omap_nand_probe(struct platform_device *pdev)
 	struct mtd_part_parser_data ppdata = {};
 	struct device *dev = &pdev->dev;
 
-	pdata = dev_get_platdata(&pdev->dev);
-	if (pdata == NULL) {
-		dev_err(&pdev->dev, "platform data missing\n");
-		return -ENODEV;
-	}
 
 	info = devm_kzalloc(&pdev->dev, sizeof(struct omap_nand_info),
 				GFP_KERNEL);
 	if (!info)
 		return -ENOMEM;
 
+	info->pdev = pdev;
+
+	if (dev->of_node) {
+		if (omap_get_dt_info(dev, info))
+			return -EINVAL;
+	} else {
+		pdata = dev_get_platdata(&pdev->dev);
+		if (pdata == NULL) {
+			dev_err(&pdev->dev, "platform data missing\n");
+			return -EINVAL;
+		}
+
+		info->gpmc_cs = pdata->cs;
+		info->of_node = pdata->of_node;
+		info->ecc_opt = pdata->ecc_opt;
+		info->dev_ready	= pdata->dev_ready;
+		info->xfer_type = pdata->xfer_type;
+		info->devsize = pdata->devsize;
+	}
+
 	platform_set_drvdata(pdev, info);
 
 	spin_lock_init(&info->controller.lock);
 	init_waitqueue_head(&info->controller.wq);
 
-	info->pdev		= pdev;
-	info->gpmc_cs		= pdata->cs;
-	info->of_node		= pdata->of_node;
-	info->ecc_opt		= pdata->ecc_opt;
-	info->dev_ready	= pdata->dev_ready;
-	info->xfer_type = pdata->xfer_type;
-	info->devsize = pdata->devsize;
-	info->elm_of_node = pdata->elm_of_node;
-
 	mtd			= &info->mtd;
 	mtd->priv		= &info->nand;
 	mtd->name		= dev_name(&pdev->dev);
@@ -2067,9 +2145,13 @@ static int omap_nand_probe(struct platform_device *pdev)
 		goto return_error;
 	}
 
-	ppdata.of_node = pdata->of_node;
-	mtd_device_parse_register(mtd, NULL, &ppdata, pdata->parts,
-				  pdata->nr_parts);
+	if (dev->of_node) {
+		ppdata.of_node = dev->of_node;
+		mtd_device_parse_register(mtd, NULL, &ppdata, NULL, 0);
+
+	} else {
+		mtd_device_register(mtd, pdata->parts, pdata->nr_parts);
+	}
 
 	platform_set_drvdata(pdev, mtd);
 
@@ -2101,12 +2183,18 @@ static int omap_nand_remove(struct platform_device *pdev)
 	return 0;
 }
 
+static const struct of_device_id omap_nand_ids[] = {
+	{ .compatible = "ti,omap2-nand", },
+	{},
+};
+
 static struct platform_driver omap_nand_driver = {
 	.probe		= omap_nand_probe,
 	.remove		= omap_nand_remove,
 	.driver		= {
 		.name	= DRIVER_NAME,
 		.owner	= THIS_MODULE,
+		.of_match_table = of_match_ptr(omap_nand_ids),
 	},
 };
 
diff --git a/include/linux/platform_data/mtd-nand-omap2.h b/include/linux/platform_data/mtd-nand-omap2.h
index 62a855e..45203a5 100644
--- a/include/linux/platform_data/mtd-nand-omap2.h
+++ b/include/linux/platform_data/mtd-nand-omap2.h
@@ -64,10 +64,8 @@ struct omap_nand_platform_data {
 	int			devsize;
 	enum omap_ecc           ecc_opt;
 
-	/* for passing the partitions */
-	struct device_node	*of_node;
-	struct device_node	*elm_of_node;
-
 	struct gpmc_nand_regs	reg;		/* deprecated */
+	struct device_node	*elm_of_node;	/* deprecated */
+	struct device_node	*of_node;	/* deprecated */
 };
 #endif
-- 
1.8.3.2


^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [RFC PATCH 14/16] ARM: OMAP: gpmc: Update DT binding documentation
  2014-05-21 11:20 [RFC PATCH 00/16] OMAP: GPMC: Restructure OMAP GPMC driver (NAND) Roger Quadros
                   ` (12 preceding siblings ...)
  2014-05-21 11:21 ` [RFC PATCH 13/16] mtd: nand: omap: True device tree support Roger Quadros
@ 2014-05-21 11:21 ` Roger Quadros
  2014-05-21 11:21 ` [RFC PATCH 15/16] mtd: nand: omap: " Roger Quadros
                   ` (2 subsequent siblings)
  16 siblings, 0 replies; 28+ messages in thread
From: Roger Quadros @ 2014-05-21 11:21 UTC (permalink / raw)
  To: tony, computersforpeace
  Cc: pekon, ezequiel.garcia, robertcnelson, jg1.han, dwmw2, javier,
	nsekhar, linux-omap, linux-mtd, devicetree, linux-kernel,
	Roger Quadros

The meaning of ranges and how address mapping is done has changed.
Ranges should now contain 2 ranges
  - GPMC I/O map. ~1GB. This map will be partitioned among the
    chip select (CS) nodes
  - GPMC register map. This is common for all the CS nodes
    and is shared only by the NAND controller.

Chip select (CS) number is no longer specified via reg property. Instead
it is specified via the gpmc,cs property in the CS node.

gpmc,nand boolean property added to specify whether a NAND device is
interfaced to the CS.
gpmc,device-width property is made mandatory.

The CS node must have a child device node for each device attached to
that chip select. Properties for that child are GPMC agnostic.

Signed-off-by: Roger Quadros <rogerq@ti.com>
---
 Documentation/devicetree/bindings/bus/ti-gpmc.txt | 109 ++++++++++++++++------
 1 file changed, 81 insertions(+), 28 deletions(-)

diff --git a/Documentation/devicetree/bindings/bus/ti-gpmc.txt b/Documentation/devicetree/bindings/bus/ti-gpmc.txt
index 704be93..ca92efb 100644
--- a/Documentation/devicetree/bindings/bus/ti-gpmc.txt
+++ b/Documentation/devicetree/bindings/bus/ti-gpmc.txt
@@ -1,8 +1,6 @@
 Device tree bindings for OMAP general purpose memory controllers (GPMC)
 
-The actual devices are instantiated from the child nodes of a GPMC node.
-
-Required properties:
+GPMC node - Required properties:
 
  - compatible:		Should be set to one of the following:
 
@@ -16,22 +14,39 @@ Required properties:
 			(see the example below)
  - ti,hwmods:		Should be set to "ti,gpmc" until the DT transition is
 			completed.
- - #address-cells:	Must be set to 2 to allow memory address translation
- - #size-cells:		Must be set to 1 to allow CS address passing
+ - interrupts:		Interrupt resource specifier for GPMC interrupt.
+ - #address-cells:	Must be set to 2 to allow memory address translation.
+			1 for range selection and 1 for child's resource address.
+ - #size-cells:		Must be set to 1 to allow child's resource size.
  - gpmc,num-cs:		The maximum number of chip-select lines that controller
 			can support.
  - gpmc,num-waitpins:	The maximum number of wait pins that controller can
 			support.
- - ranges:		Must be set up to reflect the memory layout with four
-			integer values for each chip-select line in use:
-
-			   <cs-number> 0 <physical address of mapping> <size>
-
-			Currently, calculated values derived from the contents
-			of the per-CS register GPMC_CONFIG7 (as set up by the
-			bootloader) are used for the physical address decoding.
-			As this will change in the future, filling correct
-			values here is a requirement.
+ - ranges:		Should contain 2 ranges
+			- GPMC I/O map. ~1GB. This map will be partitioned among
+			the chip select (CS) nodes
+			- GPMC register map. This is common for all the CS nodes
+			and is shared only by the NAND controller.
+
+The GPMC node must contain Chip Select (CS) child nodes representing the
+chip selects in use.
+
+CS node - Required properties:
+
+ - reg:			Resource specifier specifying the CS partition start address
+			and size. Start address must be 16MB aligned. Size has to be
+			one of 16MB, 32MB, 64MB, 128MB and 256MB.
+ - #address-cells:	Must be set to 2 to allow memory address translation.
+			1 for range selection and 1 for child's resource address.
+ - #size-cells:		Must be set to 1 to allow child's resource size.
+ - ranges:		Must be present for 1:1 address translation of child nodes.
+
+ - gpmc,cs:		Chip select number. 0 to (gpmc,num-cs - 1)
+ - gpmc,device-width:	Total width of device(s) connected to a GPMC
+			chip-select in bytes. The GPMC supports 8-bit
+			and 16-bit devices and so this property must be
+			1 or 2.
+ - gpmc,nand:		Boolean. Must be present if CS contains NAND device child.
 
 Timing properties for child nodes. All are optional and default to 0.
 
@@ -95,10 +110,6 @@ GPMC chip-select settings properties for child nodes. All are optional.
 - gpmc,burst-wrap	Enables wrap bursting
 - gpmc,burst-read	Enables read page/burst mode
 - gpmc,burst-write	Enables write page/burst mode
-- gpmc,device-width	Total width of device(s) connected to a GPMC
-			chip-select in bytes. The GPMC supports 8-bit
-			and 16-bit devices and so this property must be
-			1 or 2.
 - gpmc,mux-add-data	Address and data multiplexing configuration.
 			Valid values are 1 for address-address-data
 			multiplexing mode and 2 for address-data
@@ -114,17 +125,59 @@ GPMC chip-select settings properties for child nodes. All are optional.
 
 Example for an AM33xx board:
 
-	gpmc: gpmc@50000000 {
-		compatible = "ti,am3352-gpmc";
-		ti,hwmods = "gpmc";
-		reg = <0x50000000 0x2000>;
-		interrupts = <100>;
+gpmc@50000000 {
+	compatible = "ti,am3352-gpmc";
+	ti,hwmods = "gpmc";
+	reg = <0x50000000 0x36b>;
+	interrupts = <100>;
 
-		gpmc,num-cs = <8>;
-		gpmc,num-waitpins = <2>;
+	gpmc,num-cs = <8>;
+	gpmc,num-waitpins = <2>;
+	#address-cells = <2>;
+	#size-cells = <1>;
+
+	ranges = <0 0 0x00000000 0x1FFFFFFF	/* GPMC I/O space 512MB */
+		  1 0 0x50000000 0x36b>;	/* register space */
+
+	cs0 {
 		#address-cells = <2>;
 		#size-cells = <1>;
-		ranges = <0 0 0x08000000 0x10000000>; /* CS0 @addr 0x8000000, size 0x10000000 */
+		reg = <0 0 0x1000000>;  /* CS0 partition, 16 MB */
+		ranges;
+
+		gpmc,cs = <0>;
+		gpmc,device-width = <2>;	/* 16-bit */
+		gpmc,nand;
+
+		gpmc,cs-on-ns = <0>;
+		gpmc,cs-rd-off-ns = <36>;
+		gpmc,cs-wr-off-ns = <36>;
+		gpmc,adv-on-ns = <6>;
+		gpmc,adv-rd-off-ns = <24>;
+		gpmc,adv-wr-off-ns = <36>;
+		gpmc,oe-on-ns = <6>;
+		gpmc,oe-off-ns = <48>;
+		gpmc,we-on-ns = <6>;
+		gpmc,we-off-ns = <30>;
+		gpmc,rd-cycle-ns = <72>;
+		gpmc,wr-cycle-ns = <72>;
+		gpmc,access-ns = <54>;
+		gpmc,wr-access-ns = <30>;
+
+		nand@0,0 {
+			compatible = "ti,omap2-nand";
+			#address-cells = <1>;
+			#size-cells = <1>;
+			reg = <0 0 4		/* Nand I/O */
+			       1 0 0x36b>;	/* GPMC registers */
+			interrupts = <20>;
+
+			ti,nand-cs = <0>;
+			nand-bus-width = <16>;
+			...
+		};
 
-		/* child nodes go here */
 	};
+
+	/* Other CS nodes and their children if present */
+};
-- 
1.8.3.2


^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [RFC PATCH 15/16] mtd: nand: omap: Update DT binding documentation
  2014-05-21 11:20 [RFC PATCH 00/16] OMAP: GPMC: Restructure OMAP GPMC driver (NAND) Roger Quadros
                   ` (13 preceding siblings ...)
  2014-05-21 11:21 ` [RFC PATCH 14/16] ARM: OMAP: gpmc: Update DT binding documentation Roger Quadros
@ 2014-05-21 11:21 ` Roger Quadros
  2014-05-21 11:21 ` [RFC PATCH 16/16] ARM: dts: omap3-beagle: Add NAND device Roger Quadros
  2014-05-21 16:08 ` [RFC PATCH 00/16] OMAP: GPMC: Restructure OMAP GPMC driver (NAND) Ezequiel Garcia
  16 siblings, 0 replies; 28+ messages in thread
From: Roger Quadros @ 2014-05-21 11:21 UTC (permalink / raw)
  To: tony, computersforpeace
  Cc: pekon, ezequiel.garcia, robertcnelson, jg1.han, dwmw2, javier,
	nsekhar, linux-omap, linux-mtd, devicetree, linux-kernel,
	Roger Quadros

Add compatible id and other mandatory properties.
Remove deprecated 'elm_id' property as it is no longer used.

Update usage example.

Signed-off-by: Roger Quadros <rogerq@ti.com>
---
 .../devicetree/bindings/mtd/gpmc-nand.txt          | 86 +++++++++++-----------
 1 file changed, 42 insertions(+), 44 deletions(-)

diff --git a/Documentation/devicetree/bindings/mtd/gpmc-nand.txt b/Documentation/devicetree/bindings/mtd/gpmc-nand.txt
index 5e1f31b..49ef190 100644
--- a/Documentation/devicetree/bindings/mtd/gpmc-nand.txt
+++ b/Documentation/devicetree/bindings/mtd/gpmc-nand.txt
@@ -1,19 +1,22 @@
-Device tree bindings for GPMC connected NANDs
+Device tree bindings for Texas Instruments GPMC connected NANDs
 
-GPMC connected NAND (found on OMAP boards) are represented as child nodes of
-the GPMC controller with a name of "nand".
-
-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
+The TI GPMC NAND node is usually present as a child node of the GPMC controller
+node's Chip Select (CS) node. Documentation/devicetree/bindings/bus/ti-gpmc.txt
 
 For NAND specific properties such as ECC modes or bus width, please refer to
 Documentation/devicetree/bindings/mtd/nand.txt
 
-
 Required properties:
 
- - reg:		The CS line the peripheral is connected to
+ - compatible:		"ti,omap2-nand"
+ - reg:			Should contain 2 resource specifiers
+			- range, base offset and length of the NAND I/O space
+			The CS line the peripheral is connected to.
+			- range, base offset and length of the GPMC register
+			space.
+ - ti,nand-cs:		Chip select number used for the NAND device. Must match
+			the chip select used for the parent Chip Select node.
+ - interrupts:		Interrupt resource specifier for GPMC interrupt.
 
 Optional properties:
 
@@ -36,7 +39,6 @@ Optional properties:
 		"prefetch-dma"		Prefetch enabled sDMA mode
 		"prefetch-irq"		Prefetch enabled irq mode
 
- - elm_id:	<deprecated> use "ti,elm-id" instead
  - ti,elm-id:	Specifies phandle of the ELM devicetree node.
 		ELM is an on-chip hardware engine on TI SoC which is used for
 		locating ECC errors for BCHx algorithms. SoC devices which have
@@ -48,45 +50,41 @@ For inline partiton table parsing (optional):
  - #address-cells: should be set to 1
  - #size-cells: should be set to 1
 
-Example for an AM33xx board:
+Example for an OMAP3 board:
+
+gpmc {
+	...
+	ranges = <0 0 0x30000000 0x3FFFFFFF     /* I/O space */
+		  1 0 0x6e000000 0x02d4>;       /* register space */
+	...
 
-	gpmc: gpmc@50000000 {
-		compatible = "ti,am3352-gpmc";
-		ti,hwmods = "gpmc";
-		reg = <0x50000000 0x1000000>;
-		interrupts = <100>;
-		gpmc,num-cs = <8>;
-		gpmc,num-waitpins = <2>;
-		#address-cells = <2>;
-		#size-cells = <1>;
-		ranges = <0 0 0x08000000 0x2000>;	/* CS0: NAND */
-		elm_id = <&elm>;
+	cs0 {	/* chip select 0 */
+		...
+		reg = <0 0 0x1000000>;  /* CS0 partition, 16 MB min */
+		ranges;
+		...
 
 		nand@0,0 {
-			reg = <0 0 0>; /* CS0, offset 0 */
+			compatible = "ti,omap2-nand";
+			reg = <0 0 4            /* Nand I/O */
+			       1 0 0x2d4>;      /* GPMC registers */
+			interrupts = <20>;
+
+			ti,nand-cs = <0>;
 			nand-bus-width = <16>;
-			ti,nand-ecc-opt = "bch8";
-			ti,nand-xfer-type = "polled";
-
-			gpmc,sync-clk-ps = <0>;
-			gpmc,cs-on-ns = <0>;
-			gpmc,cs-rd-off-ns = <44>;
-			gpmc,cs-wr-off-ns = <44>;
-			gpmc,adv-on-ns = <6>;
-			gpmc,adv-rd-off-ns = <34>;
-			gpmc,adv-wr-off-ns = <44>;
-			gpmc,we-off-ns = <40>;
-			gpmc,oe-off-ns = <54>;
-			gpmc,access-ns = <64>;
-			gpmc,rd-cycle-ns = <82>;
-			gpmc,wr-cycle-ns = <82>;
-			gpmc,wr-access-ns = <40>;
-			gpmc,wr-data-mux-bus-ns = <0>;
+			ti,nand-ecc-opt = "ham1";
 
 			#address-cells = <1>;
 			#size-cells = <1>;
 
-			/* partitions go here */
-		};
-	};
-
+			partition@0 {
+				label = "X-Loader";
+				reg = <0 0x80000>;
+			};
+			partition@80000 {
+				label = "U-Boot";
+				reg = <0x80000 0x1e0000>;
+                        };
+		}
+	}
+}
-- 
1.8.3.2


^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [RFC PATCH 16/16] ARM: dts: omap3-beagle: Add NAND device
  2014-05-21 11:20 [RFC PATCH 00/16] OMAP: GPMC: Restructure OMAP GPMC driver (NAND) Roger Quadros
                   ` (14 preceding siblings ...)
  2014-05-21 11:21 ` [RFC PATCH 15/16] mtd: nand: omap: " Roger Quadros
@ 2014-05-21 11:21 ` Roger Quadros
  2014-05-21 16:08 ` [RFC PATCH 00/16] OMAP: GPMC: Restructure OMAP GPMC driver (NAND) Ezequiel Garcia
  16 siblings, 0 replies; 28+ messages in thread
From: Roger Quadros @ 2014-05-21 11:21 UTC (permalink / raw)
  To: tony, computersforpeace
  Cc: pekon, ezequiel.garcia, robertcnelson, jg1.han, dwmw2, javier,
	nsekhar, linux-omap, linux-mtd, devicetree, linux-kernel,
	Roger Quadros

The beagle board contains a 16-bit NAND device connected to
chip select 0 of the GPMC controller.

Signed-off-by: Roger Quadros <rogerq@ti.com>
---
 arch/arm/boot/dts/omap3-beagle.dts | 66 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 66 insertions(+)

diff --git a/arch/arm/boot/dts/omap3-beagle.dts b/arch/arm/boot/dts/omap3-beagle.dts
index 3c3e6da..825b364 100644
--- a/arch/arm/boot/dts/omap3-beagle.dts
+++ b/arch/arm/boot/dts/omap3-beagle.dts
@@ -350,3 +350,69 @@
 		};
 	};
 };
+
+&gpmc {
+	ranges = <0 0 0x30000000 0x3FFFFFFF     /* I/O space */
+		  1 0 0x6e000000 0x02d4>;       /* register space */
+
+	/* Chip select 0 */
+	cs0 {
+		#address-cells = <2>;
+		#size-cells = <1>;
+		reg = <0 0 0x1000000>;	/* CS0 partition, 16 MB min */
+		ranges;
+
+		gpmc,cs = <0>;
+		gpmc,device-width = <2>;	/* 16-bit */
+		gpmc,nand;
+
+		gpmc,cs-on-ns = <0>;
+		gpmc,cs-rd-off-ns = <36>;
+		gpmc,cs-wr-off-ns = <36>;
+		gpmc,adv-on-ns = <6>;
+		gpmc,adv-rd-off-ns = <24>;
+		gpmc,adv-wr-off-ns = <36>;
+		gpmc,oe-on-ns = <6>;
+		gpmc,oe-off-ns = <48>;
+		gpmc,we-on-ns = <6>;
+		gpmc,we-off-ns = <30>;
+		gpmc,rd-cycle-ns = <72>;
+		gpmc,wr-cycle-ns = <72>;
+		gpmc,access-ns = <54>;
+		gpmc,wr-access-ns = <30>;
+
+		nand@0,0 {
+			compatible = "ti,omap2-nand";
+			#address-cells = <1>;
+			#size-cells = <1>;
+			reg = <0 0 4		/* Nand I/O */
+			       1 0 0x2d4>;	/* GPMC registers */
+			interrupts = <20>;
+
+			ti,nand-cs = <0>;
+			nand-bus-width = <16>;
+			ti,nand-ecc-opt = "ham1";
+
+			partition@0 {
+				label = "X-Loader";
+				reg = <0 0x80000>;
+			};
+			partition@80000 {
+				label = "U-Boot";
+				reg = <0x80000 0x1e0000>;
+			};
+			partition@1c0000 {
+				label = "U-Boot Env";
+				reg = <0x260000 0x20000>;
+			};
+			partition@280000 {
+				label = "Kernel";
+				reg = <0x280000 0x400000>;
+			};
+			partition@780000 {
+				label = "Filesystem";
+				reg = <0x680000 0xf980000>;
+			};
+		};
+	};
+};
-- 
1.8.3.2


^ permalink raw reply related	[flat|nested] 28+ messages in thread

* Re: [RFC PATCH 00/16] OMAP: GPMC: Restructure OMAP GPMC driver (NAND)
  2014-05-21 11:20 [RFC PATCH 00/16] OMAP: GPMC: Restructure OMAP GPMC driver (NAND) Roger Quadros
                   ` (15 preceding siblings ...)
  2014-05-21 11:21 ` [RFC PATCH 16/16] ARM: dts: omap3-beagle: Add NAND device Roger Quadros
@ 2014-05-21 16:08 ` Ezequiel Garcia
  2014-05-22  8:12   ` [RFC PATCH 00/16] OMAP: GPMC: Restructure OMAP GPMC driver (NAND) : DT binding change proposal Roger Quadros
  16 siblings, 1 reply; 28+ messages in thread
From: Ezequiel Garcia @ 2014-05-21 16:08 UTC (permalink / raw)
  To: Roger Quadros
  Cc: tony, computersforpeace, pekon, robertcnelson, jg1.han, dwmw2,
	javier, nsekhar, linux-omap, linux-mtd, devicetree, linux-kernel

Hi Roger,

On 21 May 02:20 PM, Roger Quadros wrote:
> 
> For DT boot:
> - The GPMC controller node should have a chip select (CS) node for each used
>   chip select. The CS node must have a child device node for each device
>   attached to that chip select. Properties for that child are GPMC agnostic.
> 
>   i.e.
> 	gpmc {
> 		cs0 {
> 			nand0 {
> 			}
> 		};
> 
> 		cs1 {
> 			nor0 {
> 			}
> 		}
> 		...
> 	};
> 

While I agree that the GPMC driver is a bit messy, I'm not sure it's possible
to go through such a complete devicetree binding re-design (breaking backwards
compatibility) now that the binding is already in production.

AFAIK, TI's SDK 7.0 is released, with a v3.8.x kernel which uses this GPMC
binding. And then you have the ISEE board too, using this binding.

Also, what's the problem with the current devicetree binding (not that I'm fan
of it)?
-- 
Ezequiel García, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [RFC PATCH 08/16] mtd: nand: omap: Fix build warning
  2014-05-21 11:20 ` [RFC PATCH 08/16] mtd: nand: omap: Fix build warning Roger Quadros
@ 2014-05-22  0:54   ` Jingoo Han
  2014-05-22  8:17     ` Roger Quadros
  0 siblings, 1 reply; 28+ messages in thread
From: Jingoo Han @ 2014-05-22  0:54 UTC (permalink / raw)
  To: 'Roger Quadros'
  Cc: tony, computersforpeace, pekon, ezequiel.garcia, robertcnelson,
	dwmw2, javier, nsekhar, linux-omap, linux-mtd, devicetree,
	linux-kernel, 'Jingoo Han',
	'Christian Engelmayer'

On Wednesday, May 21, 2014 8:21 PM, Roger Quadros wrote:
> 
> Fix the following warning when CONFIG_MTD_NAND_OMAP_BCH is disabled.
> warning: ‘erased_sector_bitflips’ defined but not used [-Wunused-function]
> 
> Signed-off-by: Roger Quadros <rogerq@ti.com>

(+cc Christian Engelmayer)

The same patch was already sent by Christian Engelmayer. [1]
Also, it was applied to mtd tree by Brian Norris. [2]
Thank you.

[1] http://lists.infradead.org/pipermail/linux-mtd/2014-April/053330.html
[2] http://git.infradead.org/users/dedekind/l2-mtd.git/commit/9fd6c6c18c1a4a3220473c76fd447c5708b5ecf9

Best regards,
Jingoo Han

> ---
>  drivers/mtd/nand/omap2.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/mtd/nand/omap2.c b/drivers/mtd/nand/omap2.c
> index 1ff49b8..1b800bc 100644
> --- a/drivers/mtd/nand/omap2.c
> +++ b/drivers/mtd/nand/omap2.c
> @@ -1237,6 +1237,7 @@ static int __maybe_unused omap_calculate_ecc_bch(struct mtd_info *mtd,
>  	return 0;
>  }
> 
> +#ifdef CONFIG_MTD_NAND_OMAP_BCH
>  /**
>   * erased_sector_bitflips - count bit flips
>   * @data:	data sector buffer
> @@ -1276,7 +1277,6 @@ static int erased_sector_bitflips(u_char *data, u_char *oob,
>  	return flip_bits;
>  }
> 
> -#ifdef CONFIG_MTD_NAND_OMAP_BCH
>  /**
>   * omap_elm_correct_data - corrects page data area in case error reported
>   * @mtd:	MTD device structure
> --
> 1.8.3.2


^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [RFC PATCH 00/16] OMAP: GPMC: Restructure OMAP GPMC driver (NAND) : DT binding change proposal
  2014-05-21 16:08 ` [RFC PATCH 00/16] OMAP: GPMC: Restructure OMAP GPMC driver (NAND) Ezequiel Garcia
@ 2014-05-22  8:12   ` Roger Quadros
  2014-05-22 11:51     ` Javier Martinez Canillas
  0 siblings, 1 reply; 28+ messages in thread
From: Roger Quadros @ 2014-05-22  8:12 UTC (permalink / raw)
  To: Ezequiel Garcia
  Cc: tony, computersforpeace, pekon, robertcnelson, jg1.han, dwmw2,
	javier, nsekhar, linux-omap, linux-mtd, devicetree, linux-kernel,
	Grant Likely, Rob Herring

Hi Ezequiel,

On 05/21/2014 07:08 PM, Ezequiel Garcia wrote:
> Hi Roger,
> 
> On 21 May 02:20 PM, Roger Quadros wrote:
>>
>> For DT boot:
>> - The GPMC controller node should have a chip select (CS) node for each used
>>   chip select. The CS node must have a child device node for each device
>>   attached to that chip select. Properties for that child are GPMC agnostic.
>>
>>   i.e.
>> 	gpmc {
>> 		cs0 {
>> 			nand0 {
>> 			}
>> 		};
>>
>> 		cs1 {
>> 			nor0 {
>> 			}
>> 		}
>> 		...
>> 	};
>>
> 
> While I agree that the GPMC driver is a bit messy, I'm not sure it's possible
> to go through such a complete devicetree binding re-design (breaking backwards
> compatibility) now that the binding is already in production.

Why not? especially if the existing bindings are poorly dones. Is anyone using these
bindings burning the DT into ROM and can't change it when they update the kernel?

I wouldn't bother much about backward compatibility but just focus on not breaking
functionality with all GPMC users while cleaning up the existing bindings.

> 
> AFAIK, TI's SDK 7.0 is released, with a v3.8.x kernel which uses this GPMC
> binding. And then you have the ISEE board too, using this binding.

How does this prevent them from not using the new bindings when they update the kernel?

> 
> Also, what's the problem with the current devicetree binding (not that I'm fan
> of it)?
> 

The existing binding uses this format

	gpmc {
		ranges <cs-num 0 IO_partition_start IO_partition_size,
			cs-num 0 IO_partition_start IO_partition_size,
			...>

		node-name0 {
			compatible = "<id for device on this chip select>";
			reg = <cs-num IO_offset IO_size>;

			<gpmc cs properties>;

			<device specific properties>;
		};

		node-name1 {
			...
		};
	};

with requirements that
- chip select number (cs-num) is encoded in range id
- child's node-name is used to identify device type (NAND, Onenand, etc) and driver expects that.

All this results in the following issues
- No way to define entire GPMC I/O map (typically 1st 1GB), without assigning them to a Chip select. i.e. incomplete hardware description.
  TI SoCs variants can have different GPMC I/O sizes, some can have 512MB others can have 1GB. There needs to be a way to specify that.
- No clean way to specify GPMC register map for use by child nodes. NAND controller which can be one of the children needs to use the GPMC register map.
- Tricky to define multiple devices within a single chip select region.
- Uses node name to identify device type like nand and onenand. Doesn't use compatible id for them.
- GPMC CS properties are mixed with device properties, resulting in unnecessary binding documents like
http://lxr.free-electrons.com/source/Documentation/devicetree/bindings/net/gpmc-eth.txt
http://lxr.free-electrons.com/source/Documentation/devicetree/bindings/mtd/gpmc-nor.txt

To solve these issues, I'm proposing the following format

	gpmc {
		ranges <0 0 IO_start IO_size		/* entire GPMC I/O space e.g. 1GB or 512MB */
			1 0 Reg_start Reg_size>;	/* GPMC register space */

		cs0 {
			ranges <0 0 IO_partition_start IO_partition_size>;	/* CS0 IO partition e.g. 16MB */

			gpmc,cs-num = <0>;		/* pass chip select number explicitly */
			<gpmc cs properties>;

			dev0 {
				compatible = "<id for device on this chip select>";
				reg = <0 IO_offset IO_size			/* Device IO region e.g. 1KB */
					1 Reg_offset Reg_size>;

				<device specific properties>;
			};

			dev1 {
			...
			};
		};
		
		cs1 {
			...
		};
	};

All I'm doing is splitting up the CS node and the device node and removing the cs-num encoding from the ranges property.
This results in a much cleaner DT binding and code.

The format is similar to the one used by the ti-aemif driver.
http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/memory-controllers/ti-aemif.txt

Having a unified format for all TI memory controllers will make life much easier for us.

cheers,
-roger

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [RFC PATCH 08/16] mtd: nand: omap: Fix build warning
  2014-05-22  0:54   ` Jingoo Han
@ 2014-05-22  8:17     ` Roger Quadros
  0 siblings, 0 replies; 28+ messages in thread
From: Roger Quadros @ 2014-05-22  8:17 UTC (permalink / raw)
  To: Jingoo Han
  Cc: tony, computersforpeace, pekon, ezequiel.garcia, robertcnelson,
	dwmw2, javier, nsekhar, linux-omap, linux-mtd, devicetree,
	linux-kernel, 'Christian Engelmayer'

On 05/22/2014 03:54 AM, Jingoo Han wrote:
> On Wednesday, May 21, 2014 8:21 PM, Roger Quadros wrote:
>>
>> Fix the following warning when CONFIG_MTD_NAND_OMAP_BCH is disabled.
>> warning: ‘erased_sector_bitflips’ defined but not used [-Wunused-function]
>>
>> Signed-off-by: Roger Quadros <rogerq@ti.com>
> 
> (+cc Christian Engelmayer)
> 
> The same patch was already sent by Christian Engelmayer. [1]
> Also, it was applied to mtd tree by Brian Norris. [2]
> Thank you.
> 
> [1] http://lists.infradead.org/pipermail/linux-mtd/2014-April/053330.html
> [2] http://git.infradead.org/users/dedekind/l2-mtd.git/commit/9fd6c6c18c1a4a3220473c76fd447c5708b5ecf9

Great! thanks for the information.

I'll drop this patch from my series.

cheers,
-roger

> 
>> ---
>>  drivers/mtd/nand/omap2.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/mtd/nand/omap2.c b/drivers/mtd/nand/omap2.c
>> index 1ff49b8..1b800bc 100644
>> --- a/drivers/mtd/nand/omap2.c
>> +++ b/drivers/mtd/nand/omap2.c
>> @@ -1237,6 +1237,7 @@ static int __maybe_unused omap_calculate_ecc_bch(struct mtd_info *mtd,
>>  	return 0;
>>  }
>>
>> +#ifdef CONFIG_MTD_NAND_OMAP_BCH
>>  /**
>>   * erased_sector_bitflips - count bit flips
>>   * @data:	data sector buffer
>> @@ -1276,7 +1277,6 @@ static int erased_sector_bitflips(u_char *data, u_char *oob,
>>  	return flip_bits;
>>  }
>>
>> -#ifdef CONFIG_MTD_NAND_OMAP_BCH
>>  /**
>>   * omap_elm_correct_data - corrects page data area in case error reported
>>   * @mtd:	MTD device structure
>> --
>> 1.8.3.2
> 


^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [RFC PATCH 00/16] OMAP: GPMC: Restructure OMAP GPMC driver (NAND) : DT binding change proposal
  2014-05-22  8:12   ` [RFC PATCH 00/16] OMAP: GPMC: Restructure OMAP GPMC driver (NAND) : DT binding change proposal Roger Quadros
@ 2014-05-22 11:51     ` Javier Martinez Canillas
  2014-05-22 14:46       ` Ezequiel Garcia
  0 siblings, 1 reply; 28+ messages in thread
From: Javier Martinez Canillas @ 2014-05-22 11:51 UTC (permalink / raw)
  To: Roger Quadros
  Cc: Ezequiel Garcia, Tony Lindgren, Brian Norris, Gupta, Pekon,
	Robert Nelson, Jingoo Han, dwmw2, nsekhar, linux-omap, linux-mtd,
	devicetree, Linux Kernel, Grant Likely, Rob Herring

Hello Roger,

On Thu, May 22, 2014 at 10:12 AM, Roger Quadros <rogerq@ti.com> wrote:
> Hi Ezequiel,
>
> On 05/21/2014 07:08 PM, Ezequiel Garcia wrote:
>> Hi Roger,
>>
>> On 21 May 02:20 PM, Roger Quadros wrote:
>>>
>>> For DT boot:
>>> - The GPMC controller node should have a chip select (CS) node for each used
>>>   chip select. The CS node must have a child device node for each device
>>>   attached to that chip select. Properties for that child are GPMC agnostic.
>>>
>>>   i.e.
>>>      gpmc {
>>>              cs0 {
>>>                      nand0 {
>>>                      }
>>>              };
>>>
>>>              cs1 {
>>>                      nor0 {
>>>                      }
>>>              }
>>>              ...
>>>      };
>>>
>>
>> While I agree that the GPMC driver is a bit messy, I'm not sure it's possible
>> to go through such a complete devicetree binding re-design (breaking backwards
>> compatibility) now that the binding is already in production.
>
> Why not? especially if the existing bindings are poorly dones. Is anyone using these
> bindings burning the DT into ROM and can't change it when they update the kernel?
>

While I do agree that your DT bindings are much better than the
current ones, there is a policy that DT bindings are an external API
and once are released with a kernel are set in stone and can't be
changed.

The rationale here is that DT only describes hardware and since
hardware does not change, there is no need to change the DT on
subsequent kernel releases.

While that may be true in theory, in practice our understanding of the
hardware keeps evolving. It may be possible that the person adding
those binding didn't understand the hardware fully or did not have
access to all the documentation.

So given that the GPMC bindings are really awful maybe we can make an
exception here? I don't even know who should decide this, if Tony as
OMAP maintainer or the DT maintainers.

> I wouldn't bother much about backward compatibility but just focus on not breaking
> functionality with all GPMC users while cleaning up the existing bindings.
>
>>
>> AFAIK, TI's SDK 7.0 is released, with a v3.8.x kernel which uses this GPMC
>> binding. And then you have the ISEE board too, using this binding.
>
> How does this prevent them from not using the new bindings when they update the kernel?
>

In the particular case of ISEE boards, the vendor still ships a very
old kernel that uses board files and platform data so probably is not
an issue for mainline users to update their DTB but I see that there
are many users of the GPMC binding in mainline:

$ git grep "gpmc[:@_a-zA-Z0-9]* {" arch/arm/boot/dts/ | wc -l
36

>>
>> Also, what's the problem with the current devicetree binding (not that I'm fan
>> of it)?
>>

Like Ezequiel said, I'm not a big fan of the current binding neither
but I don't know how safe is to break backward compatibility at this
point.

Best regards,
Javier

>
> The existing binding uses this format
>
>         gpmc {
>                 ranges <cs-num 0 IO_partition_start IO_partition_size,
>                         cs-num 0 IO_partition_start IO_partition_size,
>                         ...>
>
>                 node-name0 {
>                         compatible = "<id for device on this chip select>";
>                         reg = <cs-num IO_offset IO_size>;
>
>                         <gpmc cs properties>;
>
>                         <device specific properties>;
>                 };
>
>                 node-name1 {
>                         ...
>                 };
>         };
>
> with requirements that
> - chip select number (cs-num) is encoded in range id
> - child's node-name is used to identify device type (NAND, Onenand, etc) and driver expects that.
>
> All this results in the following issues
> - No way to define entire GPMC I/O map (typically 1st 1GB), without assigning them to a Chip select. i.e. incomplete hardware description.
>   TI SoCs variants can have different GPMC I/O sizes, some can have 512MB others can have 1GB. There needs to be a way to specify that.
> - No clean way to specify GPMC register map for use by child nodes. NAND controller which can be one of the children needs to use the GPMC register map.
> - Tricky to define multiple devices within a single chip select region.
> - Uses node name to identify device type like nand and onenand. Doesn't use compatible id for them.
> - GPMC CS properties are mixed with device properties, resulting in unnecessary binding documents like
> http://lxr.free-electrons.com/source/Documentation/devicetree/bindings/net/gpmc-eth.txt
> http://lxr.free-electrons.com/source/Documentation/devicetree/bindings/mtd/gpmc-nor.txt
>
> To solve these issues, I'm proposing the following format
>
>         gpmc {
>                 ranges <0 0 IO_start IO_size            /* entire GPMC I/O space e.g. 1GB or 512MB */
>                         1 0 Reg_start Reg_size>;        /* GPMC register space */
>
>                 cs0 {
>                         ranges <0 0 IO_partition_start IO_partition_size>;      /* CS0 IO partition e.g. 16MB */
>
>                         gpmc,cs-num = <0>;              /* pass chip select number explicitly */
>                         <gpmc cs properties>;
>
>                         dev0 {
>                                 compatible = "<id for device on this chip select>";
>                                 reg = <0 IO_offset IO_size                      /* Device IO region e.g. 1KB */
>                                         1 Reg_offset Reg_size>;
>
>                                 <device specific properties>;
>                         };
>
>                         dev1 {
>                         ...
>                         };
>                 };
>
>                 cs1 {
>                         ...
>                 };
>         };
>
> All I'm doing is splitting up the CS node and the device node and removing the cs-num encoding from the ranges property.
> This results in a much cleaner DT binding and code.
>
> The format is similar to the one used by the ti-aemif driver.
> http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/memory-controllers/ti-aemif.txt
>
> Having a unified format for all TI memory controllers will make life much easier for us.
>
> cheers,
> -roger

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [RFC PATCH 00/16] OMAP: GPMC: Restructure OMAP GPMC driver (NAND) : DT binding change proposal
  2014-05-22 11:51     ` Javier Martinez Canillas
@ 2014-05-22 14:46       ` Ezequiel Garcia
  2014-05-23  8:16         ` Roger Quadros
  0 siblings, 1 reply; 28+ messages in thread
From: Ezequiel Garcia @ 2014-05-22 14:46 UTC (permalink / raw)
  To: Roger Quadros, Tony Lindgren, Javier Martinez Canillas
  Cc: Brian Norris, Gupta, Pekon, Robert Nelson, Jingoo Han, dwmw2,
	nsekhar, linux-omap, linux-mtd, devicetree, Linux Kernel,
	Grant Likely, Rob Herring

On 22 May 01:51 PM, Javier Martinez Canillas wrote:
> On Thu, May 22, 2014 at 10:12 AM, Roger Quadros <rogerq@ti.com> wrote:
> >> On 21 May 02:20 PM, Roger Quadros wrote:
> >>>
> >>> For DT boot:
> >>> - The GPMC controller node should have a chip select (CS) node for each used
> >>>   chip select. The CS node must have a child device node for each device
> >>>   attached to that chip select. Properties for that child are GPMC agnostic.
> >>>
> >>>   i.e.
> >>>      gpmc {
> >>>              cs0 {
> >>>                      nand0 {
> >>>                      }
> >>>              };
> >>>
> >>>              cs1 {
> >>>                      nor0 {
> >>>                      }
> >>>              }
> >>>              ...
> >>>      };
> >>>
> >>
> >> While I agree that the GPMC driver is a bit messy, I'm not sure it's possible
> >> to go through such a complete devicetree binding re-design (breaking backwards
> >> compatibility) now that the binding is already in production.
> >
> > Why not? especially if the existing bindings are poorly dones. Is anyone using these
> > bindings burning the DT into ROM and can't change it when they update the kernel?
> >
> 
> While I do agree that your DT bindings are much better than the
> current ones, there is a policy that DT bindings are an external API
> and once are released with a kernel are set in stone and can't be
> changed.
> 

Exactly. The DT binding is considered an ABI. Thus, invariant across kernel
versions. Users can't be coherced into a DTB update after a kernel update.

That said, I don't really care if you break compatilibity in this case.
Rather, I'm suggesting that you make sure this change is going to be accepted
upstream, before doing any more work. The DT maintainers are reluctant to do
so.

On the other side, I guess you will also break bisectability while breaking
backward compatibility. Doesn't sound very nice.
-- 
Ezequiel García, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [RFC PATCH 00/16] OMAP: GPMC: Restructure OMAP GPMC driver (NAND) : DT binding change proposal
  2014-05-22 14:46       ` Ezequiel Garcia
@ 2014-05-23  8:16         ` Roger Quadros
  2014-05-23  9:40           ` Javier Martinez Canillas
  2014-05-23 14:53           ` Tony Lindgren
  0 siblings, 2 replies; 28+ messages in thread
From: Roger Quadros @ 2014-05-23  8:16 UTC (permalink / raw)
  To: Ezequiel Garcia, Tony Lindgren, Javier Martinez Canillas
  Cc: Brian Norris, Gupta, Pekon, Robert Nelson, Jingoo Han, dwmw2,
	nsekhar, linux-omap, linux-mtd, devicetree, Linux Kernel,
	Grant Likely, Rob Herring

Ezequiel & Javier,

On 05/22/2014 05:46 PM, Ezequiel Garcia wrote:
> On 22 May 01:51 PM, Javier Martinez Canillas wrote:
>> On Thu, May 22, 2014 at 10:12 AM, Roger Quadros <rogerq@ti.com> wrote:
>>>> On 21 May 02:20 PM, Roger Quadros wrote:
>>>>>
>>>>> For DT boot:
>>>>> - The GPMC controller node should have a chip select (CS) node for each used
>>>>>   chip select. The CS node must have a child device node for each device
>>>>>   attached to that chip select. Properties for that child are GPMC agnostic.
>>>>>
>>>>>   i.e.
>>>>>      gpmc {
>>>>>              cs0 {
>>>>>                      nand0 {
>>>>>                      }
>>>>>              };
>>>>>
>>>>>              cs1 {
>>>>>                      nor0 {
>>>>>                      }
>>>>>              }
>>>>>              ...
>>>>>      };
>>>>>
>>>>
>>>> While I agree that the GPMC driver is a bit messy, I'm not sure it's possible
>>>> to go through such a complete devicetree binding re-design (breaking backwards
>>>> compatibility) now that the binding is already in production.
>>>
>>> Why not? especially if the existing bindings are poorly dones. Is anyone using these
>>> bindings burning the DT into ROM and can't change it when they update the kernel?
>>>
>>
>> While I do agree that your DT bindings are much better than the
>> current ones, there is a policy that DT bindings are an external API
>> and once are released with a kernel are set in stone and can't be
>> changed.
>>
> 
> Exactly. The DT binding is considered an ABI. Thus, invariant across kernel
> versions. Users can't be coherced into a DTB update after a kernel update.
> 
> That said, I don't really care if you break compatilibity in this case.
> Rather, I'm suggesting that you make sure this change is going to be accepted
> upstream, before doing any more work. The DT maintainers are reluctant to do
> so.

Appreciate your concern.

Would be really nice if you can review patches 1-12. They have nothing to do with DT changes.
Thanks.

cheers,
-roger

> 
> On the other side, I guess you will also break bisectability while breaking
> backward compatibility. Doesn't sound very nice.
> 


^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [RFC PATCH 00/16] OMAP: GPMC: Restructure OMAP GPMC driver (NAND) : DT binding change proposal
  2014-05-23  8:16         ` Roger Quadros
@ 2014-05-23  9:40           ` Javier Martinez Canillas
  2014-05-26  7:23             ` Roger Quadros
  2014-05-23 14:53           ` Tony Lindgren
  1 sibling, 1 reply; 28+ messages in thread
From: Javier Martinez Canillas @ 2014-05-23  9:40 UTC (permalink / raw)
  To: Roger Quadros
  Cc: Ezequiel Garcia, Tony Lindgren, Brian Norris, Gupta, Pekon,
	Robert Nelson, Jingoo Han, David Woodhouse, nsekhar, linux-omap,
	linux-mtd, devicetree, Linux Kernel, Grant Likely, Rob Herring

Hello Roger,

On Fri, May 23, 2014 at 10:16 AM, Roger Quadros <rogerq@ti.com> wrote:
> Ezequiel & Javier,
>
> On 05/22/2014 05:46 PM, Ezequiel Garcia wrote:
>> On 22 May 01:51 PM, Javier Martinez Canillas wrote:
>>> On Thu, May 22, 2014 at 10:12 AM, Roger Quadros <rogerq@ti.com> wrote:
>>>>> On 21 May 02:20 PM, Roger Quadros wrote:
>>>>>>
>>>>>> For DT boot:
>>>>>> - The GPMC controller node should have a chip select (CS) node for each used
>>>>>>   chip select. The CS node must have a child device node for each device
>>>>>>   attached to that chip select. Properties for that child are GPMC agnostic.
>>>>>>
>>>>>>   i.e.
>>>>>>      gpmc {
>>>>>>              cs0 {
>>>>>>                      nand0 {
>>>>>>                      }
>>>>>>              };
>>>>>>
>>>>>>              cs1 {
>>>>>>                      nor0 {
>>>>>>                      }
>>>>>>              }
>>>>>>              ...
>>>>>>      };
>>>>>>
>>>>>
>>>>> While I agree that the GPMC driver is a bit messy, I'm not sure it's possible
>>>>> to go through such a complete devicetree binding re-design (breaking backwards
>>>>> compatibility) now that the binding is already in production.
>>>>
>>>> Why not? especially if the existing bindings are poorly dones. Is anyone using these
>>>> bindings burning the DT into ROM and can't change it when they update the kernel?
>>>>
>>>
>>> While I do agree that your DT bindings are much better than the
>>> current ones, there is a policy that DT bindings are an external API
>>> and once are released with a kernel are set in stone and can't be
>>> changed.
>>>
>>
>> Exactly. The DT binding is considered an ABI. Thus, invariant across kernel
>> versions. Users can't be coherced into a DTB update after a kernel update.
>>
>> That said, I don't really care if you break compatilibity in this case.
>> Rather, I'm suggesting that you make sure this change is going to be accepted
>> upstream, before doing any more work. The DT maintainers are reluctant to do
>> so.
>
> Appreciate your concern.
>
> Would be really nice if you can review patches 1-12. They have nothing to do with DT changes.
> Thanks.
>

Overall your patches looks good to me. But I think it's better to wait
until Tony removes the legacy board files for OMAP2+ since AFAIU at
least the following patches could be dropped or trimmed down when
board files are gone:

[RFC PATCH 04/16] ARM: OMAP2+: gpmc: use platform data to configure CS
space and poplulate
[RFC PATCH 06/16] ARM: OMAP2+: gpmc: add NAND specific setup
[RFC PATCH 07/16] ARM: OMAP2+: nand: Update gpmc_nand_init() to use
generic_gpmc_init()

Patches 1-3 and 5 are independent and can be applied in the meantime
as a preparation for further changes following board files removal.

I really like patches 9-12 since those moves some NAND add-hoc code to
the NAND driver where it really belongs. I think that similar changes
can be made for OneNAND and push the special case handling code from
GPMC driver to drivers/mtd/onenand/omap2.c.

Other devices (nor, ethernet, uart, etc) are already using
gpmc_probe_generic_child() so I hope we can isolate the NAND and
OneNAND specific changes and just use a single probe function for all
child devices and possibly get even need the enum gpmc_omap_type you
are adding on your struct gpmc_omap_cs_data.

So what do you think if as a first step we add the platform data as
you propose with all the commons timings and settings there, move all
the possible code to NAND and OneNAND drivers and try to use a single
configuration function for all child devices?

Then once board files are gone we can do further cleanup in the driver
and then we can discuss about changing the DT bindings. Maybe we can
even change it while keeping backwards compatibility? Although I'm not
sure about the last point I think that at least is worth to discuss
it.

> cheers,
> -roger
>

Thanks a lot and best regards,
Javier

>>
>> On the other side, I guess you will also break bisectability while breaking
>> backward compatibility. Doesn't sound very nice.
>>
>

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [RFC PATCH 00/16] OMAP: GPMC: Restructure OMAP GPMC driver (NAND) : DT binding change proposal
  2014-05-23  8:16         ` Roger Quadros
  2014-05-23  9:40           ` Javier Martinez Canillas
@ 2014-05-23 14:53           ` Tony Lindgren
  2014-05-26  7:33             ` Roger Quadros
  1 sibling, 1 reply; 28+ messages in thread
From: Tony Lindgren @ 2014-05-23 14:53 UTC (permalink / raw)
  To: Roger Quadros
  Cc: Ezequiel Garcia, Javier Martinez Canillas, Brian Norris, Gupta,
	Pekon, Robert Nelson, Jingoo Han, dwmw2, nsekhar, linux-omap,
	linux-mtd, devicetree, Linux Kernel, Grant Likely, Rob Herring

* Roger Quadros <rogerq@ti.com> [140523 01:17]:
> On 05/22/2014 05:46 PM, Ezequiel Garcia wrote:
> > On 22 May 01:51 PM, Javier Martinez Canillas wrote:
> >> On Thu, May 22, 2014 at 10:12 AM, Roger Quadros <rogerq@ti.com> wrote:
> >>>> On 21 May 02:20 PM, Roger Quadros wrote:
> >>>> While I agree that the GPMC driver is a bit messy, I'm not sure it's possible
> >>>> to go through such a complete devicetree binding re-design (breaking backwards
> >>>> compatibility) now that the binding is already in production.
> >>>
> >>> Why not? especially if the existing bindings are poorly dones. Is anyone using these
> >>> bindings burning the DT into ROM and can't change it when they update the kernel?
> >>>
> >>
> >> While I do agree that your DT bindings are much better than the
> >> current ones, there is a policy that DT bindings are an external API
> >> and once are released with a kernel are set in stone and can't be
> >> changed.
> >>
> > 
> > Exactly. The DT binding is considered an ABI. Thus, invariant across kernel
> > versions. Users can't be coherced into a DTB update after a kernel update.
> > 
> > That said, I don't really care if you break compatilibity in this case.
> > Rather, I'm suggesting that you make sure this change is going to be accepted
> > upstream, before doing any more work. The DT maintainers are reluctant to do
> > so.
> 
> Appreciate your concern.
> 
> Would be really nice if you can review patches 1-12. They have nothing to do with DT changes.
> Thanks.

I'm mostly concerned about keeping things working. I think the
only way we can keep things working is to keep support for
the old binding around in addition to the new one. That way
we can update devices one at a time.

Regards,

Tony

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [RFC PATCH 00/16] OMAP: GPMC: Restructure OMAP GPMC driver (NAND) : DT binding change proposal
  2014-05-23  9:40           ` Javier Martinez Canillas
@ 2014-05-26  7:23             ` Roger Quadros
  0 siblings, 0 replies; 28+ messages in thread
From: Roger Quadros @ 2014-05-26  7:23 UTC (permalink / raw)
  To: Javier Martinez Canillas
  Cc: Ezequiel Garcia, Tony Lindgren, Brian Norris, Gupta, Pekon,
	Robert Nelson, Jingoo Han, David Woodhouse, nsekhar, linux-omap,
	linux-mtd, devicetree, Linux Kernel, Grant Likely, Rob Herring

Javier,

On 05/23/2014 12:40 PM, Javier Martinez Canillas wrote:
> Hello Roger,
> 
> On Fri, May 23, 2014 at 10:16 AM, Roger Quadros <rogerq@ti.com> wrote:
>> Ezequiel & Javier,
>>
>> On 05/22/2014 05:46 PM, Ezequiel Garcia wrote:
>>> On 22 May 01:51 PM, Javier Martinez Canillas wrote:
>>>> On Thu, May 22, 2014 at 10:12 AM, Roger Quadros <rogerq@ti.com> wrote:
>>>>>> On 21 May 02:20 PM, Roger Quadros wrote:
>>>>>>>
>>>>>>> For DT boot:
>>>>>>> - The GPMC controller node should have a chip select (CS) node for each used
>>>>>>>   chip select. The CS node must have a child device node for each device
>>>>>>>   attached to that chip select. Properties for that child are GPMC agnostic.
>>>>>>>
>>>>>>>   i.e.
>>>>>>>      gpmc {
>>>>>>>              cs0 {
>>>>>>>                      nand0 {
>>>>>>>                      }
>>>>>>>              };
>>>>>>>
>>>>>>>              cs1 {
>>>>>>>                      nor0 {
>>>>>>>                      }
>>>>>>>              }
>>>>>>>              ...
>>>>>>>      };
>>>>>>>
>>>>>>
>>>>>> While I agree that the GPMC driver is a bit messy, I'm not sure it's possible
>>>>>> to go through such a complete devicetree binding re-design (breaking backwards
>>>>>> compatibility) now that the binding is already in production.
>>>>>
>>>>> Why not? especially if the existing bindings are poorly dones. Is anyone using these
>>>>> bindings burning the DT into ROM and can't change it when they update the kernel?
>>>>>
>>>>
>>>> While I do agree that your DT bindings are much better than the
>>>> current ones, there is a policy that DT bindings are an external API
>>>> and once are released with a kernel are set in stone and can't be
>>>> changed.
>>>>
>>>
>>> Exactly. The DT binding is considered an ABI. Thus, invariant across kernel
>>> versions. Users can't be coherced into a DTB update after a kernel update.
>>>
>>> That said, I don't really care if you break compatilibity in this case.
>>> Rather, I'm suggesting that you make sure this change is going to be accepted
>>> upstream, before doing any more work. The DT maintainers are reluctant to do
>>> so.
>>
>> Appreciate your concern.
>>
>> Would be really nice if you can review patches 1-12. They have nothing to do with DT changes.
>> Thanks.
>>
> 
> Overall your patches looks good to me. But I think it's better to wait
> until Tony removes the legacy board files for OMAP2+ since AFAIU at
> least the following patches could be dropped or trimmed down when
> board files are gone:
> 
> [RFC PATCH 04/16] ARM: OMAP2+: gpmc: use platform data to configure CS
> space and poplulate
> [RFC PATCH 06/16] ARM: OMAP2+: gpmc: add NAND specific setup
> [RFC PATCH 07/16] ARM: OMAP2+: nand: Update gpmc_nand_init() to use
> generic_gpmc_init()
> 
> Patches 1-3 and 5 are independent and can be applied in the meantime
> as a preparation for further changes following board files removal.
> 
> I really like patches 9-12 since those moves some NAND add-hoc code to
> the NAND driver where it really belongs. I think that similar changes
> can be made for OneNAND and push the special case handling code from
> GPMC driver to drivers/mtd/onenand/omap2.c.
> 
> Other devices (nor, ethernet, uart, etc) are already using
> gpmc_probe_generic_child() so I hope we can isolate the NAND and
> OneNAND specific changes and just use a single probe function for all
> child devices and possibly get even need the enum gpmc_omap_type you
> are adding on your struct gpmc_omap_cs_data.

Yes, I was thinking the same.

> 
> So what do you think if as a first step we add the platform data as
> you propose with all the commons timings and settings there, move all
> the possible code to NAND and OneNAND drivers and try to use a single
> configuration function for all child devices?

Yes, I agree.
> 
> Then once board files are gone we can do further cleanup in the driver
> and then we can discuss about changing the DT bindings. Maybe we can
> even change it while keeping backwards compatibility? Although I'm not
> sure about the last point I think that at least is worth to discuss
> it.

OK.

cheers,
-roger


^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [RFC PATCH 00/16] OMAP: GPMC: Restructure OMAP GPMC driver (NAND) : DT binding change proposal
  2014-05-23 14:53           ` Tony Lindgren
@ 2014-05-26  7:33             ` Roger Quadros
  0 siblings, 0 replies; 28+ messages in thread
From: Roger Quadros @ 2014-05-26  7:33 UTC (permalink / raw)
  To: Tony Lindgren
  Cc: Ezequiel Garcia, Javier Martinez Canillas, Brian Norris, Gupta,
	Pekon, Robert Nelson, Jingoo Han, dwmw2, nsekhar, linux-omap,
	linux-mtd, devicetree, Linux Kernel, Grant Likely, Rob Herring

On 05/23/2014 05:53 PM, Tony Lindgren wrote:
> * Roger Quadros <rogerq@ti.com> [140523 01:17]:
>> On 05/22/2014 05:46 PM, Ezequiel Garcia wrote:
>>> On 22 May 01:51 PM, Javier Martinez Canillas wrote:
>>>> On Thu, May 22, 2014 at 10:12 AM, Roger Quadros <rogerq@ti.com> wrote:
>>>>>> On 21 May 02:20 PM, Roger Quadros wrote:
>>>>>> While I agree that the GPMC driver is a bit messy, I'm not sure it's possible
>>>>>> to go through such a complete devicetree binding re-design (breaking backwards
>>>>>> compatibility) now that the binding is already in production.
>>>>>
>>>>> Why not? especially if the existing bindings are poorly dones. Is anyone using these
>>>>> bindings burning the DT into ROM and can't change it when they update the kernel?
>>>>>
>>>>
>>>> While I do agree that your DT bindings are much better than the
>>>> current ones, there is a policy that DT bindings are an external API
>>>> and once are released with a kernel are set in stone and can't be
>>>> changed.
>>>>
>>>
>>> Exactly. The DT binding is considered an ABI. Thus, invariant across kernel
>>> versions. Users can't be coherced into a DTB update after a kernel update.
>>>
>>> That said, I don't really care if you break compatilibity in this case.
>>> Rather, I'm suggesting that you make sure this change is going to be accepted
>>> upstream, before doing any more work. The DT maintainers are reluctant to do
>>> so.
>>
>> Appreciate your concern.
>>
>> Would be really nice if you can review patches 1-12. They have nothing to do with DT changes.
>> Thanks.
> 
> I'm mostly concerned about keeping things working. I think the
> only way we can keep things working is to keep support for
> the old binding around in addition to the new one. That way
> we can update devices one at a time.

Good to hear that you are not keen on keeping the old bindings forever. I understand
that we need to keep things working during the transition. I'll think of something to
maintain backward compatibility while supporting the new binding.

cheers,
-roger

^ permalink raw reply	[flat|nested] 28+ messages in thread

end of thread, other threads:[~2014-05-26  7:34 UTC | newest]

Thread overview: 28+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-05-21 11:20 [RFC PATCH 00/16] OMAP: GPMC: Restructure OMAP GPMC driver (NAND) Roger Quadros
2014-05-21 11:20 ` [RFC PATCH 01/16] ARM: OMAP2+: gpmc: Add platform data Roger Quadros
2014-05-21 11:20 ` [RFC PATCH 02/16] ARM: OMAP2+: gpmc: Add gpmc timings and settings to " Roger Quadros
2014-05-21 11:20 ` [RFC PATCH 03/16] ARM: OMAP2+: gmpc: add gpmc_generic_init() Roger Quadros
2014-05-21 11:20 ` [RFC PATCH 04/16] ARM: OMAP2+: gpmc: use platform data to configure CS space and poplulate device Roger Quadros
2014-05-21 11:20 ` [RFC PATCH 05/16] ARM: OMAP2+: gpmc: Use low level read/write for context save/restore Roger Quadros
2014-05-21 11:20 ` [RFC PATCH 06/16] ARM: OMAP2+: gpmc: add NAND specific setup Roger Quadros
2014-05-21 11:20 ` [RFC PATCH 07/16] ARM: OMAP2+: nand: Update gpmc_nand_init() to use generic_gpmc_init() Roger Quadros
2014-05-21 11:20 ` [RFC PATCH 08/16] mtd: nand: omap: Fix build warning Roger Quadros
2014-05-22  0:54   ` Jingoo Han
2014-05-22  8:17     ` Roger Quadros
2014-05-21 11:20 ` [RFC PATCH 09/16] mtd: nand: omap: Move IRQ handling from GPMC to NAND driver Roger Quadros
2014-05-21 11:20 ` [RFC PATCH 10/16] mtd: nand: omap: Move gpmc_update_nand_reg to nand driver Roger Quadros
2014-05-21 11:20 ` [RFC PATCH 11/16] mtd: nand: omap: Move NAND write protect code from GPMC to NAND driver Roger Quadros
2014-05-21 11:21 ` [RFC PATCH 12/16] mtd: nand: omap: Copy platform data parameters to omap_nand_info data Roger Quadros
2014-05-21 11:21 ` [RFC PATCH 13/16] mtd: nand: omap: True device tree support Roger Quadros
2014-05-21 11:21 ` [RFC PATCH 14/16] ARM: OMAP: gpmc: Update DT binding documentation Roger Quadros
2014-05-21 11:21 ` [RFC PATCH 15/16] mtd: nand: omap: " Roger Quadros
2014-05-21 11:21 ` [RFC PATCH 16/16] ARM: dts: omap3-beagle: Add NAND device Roger Quadros
2014-05-21 16:08 ` [RFC PATCH 00/16] OMAP: GPMC: Restructure OMAP GPMC driver (NAND) Ezequiel Garcia
2014-05-22  8:12   ` [RFC PATCH 00/16] OMAP: GPMC: Restructure OMAP GPMC driver (NAND) : DT binding change proposal Roger Quadros
2014-05-22 11:51     ` Javier Martinez Canillas
2014-05-22 14:46       ` Ezequiel Garcia
2014-05-23  8:16         ` Roger Quadros
2014-05-23  9:40           ` Javier Martinez Canillas
2014-05-26  7:23             ` Roger Quadros
2014-05-23 14:53           ` Tony Lindgren
2014-05-26  7:33             ` Roger Quadros

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).