Skip to contentSkip to conversation

OMAP GPMC DT bindings

PATCH Superseded Daniel Mack 4 participants 20 messages Dec 5, 2012 – Dec 15, 2012

Subsystems (8)

v7 of 8 Superseded Latest is v8 →

lore
Share & tools
lore
#0 lore raw

v7 cover OMAP GPMC DT bindings

This is a series of patches to support GPMC peripherals on OMAP boards.

Grant, Rob, could you have a look and give your Acked-by if appropriate?

Many thanks again,
Daniel

Tested on Linus' master +
omap-next (branch omap-for-v3.8/cleanup-headers-gpmc)

Generated from linux-next as of today, resolving one trivial include file rebase conflict.

The only supported peripheral for now is NAND, but other types would be easy to add.

Version 2 addresses details pointed out by Jon Hunter, Afzal Mohammed and Rob Herring:

- add "reg" and "ti,hwmod" properties to Documentation
- use generic of_mtd functions and the property names defined by them, namely "nand-bus-width" and "nand-ecc-mode"
- reduce the default register space size in the Documentation to 8K, as found in the hwmod code
- switch to a DT layout based on ranges and address translation. Although this property is not currently looked at as long as the handling code still uses the runtime calculation methods, we now have these values in the bindings, eventually allowing us to switch the implementation with less pain.

Version 3 includes fixes pointed out by Jon Hunter:

- better documentation of the 'ranges' property to describe the fact that it's representing the CS lines
- GPMC_CS_CONFIGx -> GPMC_CONFIGx in comments
- drop interrupt-parent from example bindings
- add of_node_put() at the end of the child iteration

Version 4 fixes compilation for !CONFIG_MTD_NAND and includes more details from Jon Hunter and Avinash, Philip:

- Add "num-cs" and "num-waitpins" properties, which will eventually be used to get rid of GPMC_CS_NUM
- Better description of generic nand DT properties
- Dropped patch 3/4 as an equivalent fix was already merged
- Added ti,nand-ecc-use-elm property

Version 5 with regards to Avinash, Philip and Peter Korsgaard:

- Re-add accidentially forgotten Documentation/devicetree/bindings/bus/ti-gpmc.txt
- Rename "software" ecc mode to "sw"
- Initialize gpmc_nand_data->is_elm_used to 'true' rather than 1
- Drop ti,nand-ecc-use-elm binding in favor of a new ecc mode named "bch8-am335xrbl-compatible"
- Add two more patches for section mismatch fixups

Version 6:

- Dropped "bch8-am335xrbl-compatible" mode again. As discussed with Avinash, the ELM issue will be solved subsequently in s separate series.
- re-added a patch to bail out of automatic GPMC instanciation in case of DT boot.
- re-added the "of_node" addition in mtd_nand_omap2.h in 2/5

Version 7: comments from Jon Hunter, all affecting the documentation:

- add num-wait pins properties as they are marked 'required'
- make reg sizes consistent
- AM335x only has 2 wait-pins

Again, many thanks to everybody, in particular Avinash, for the long disussion about how to the these details right in the first place.

Daniel

Daniel Mack (5):
ARM: OMAP: gpmc: don't create devices from initcall on DT
mtd: omap-nand: pass device_node in platform data
ARM: OMAP: gpmc-nand: drop __init annotation
ARM: OMAP: gpmc: enable hwecc for AM33xx SoCs
ARM: OMAP: gpmc: add DT bindings for GPMC timings and NAND

 Documentation/devicetree/bindings/bus/ti-gpmc.txt  |  76 +++++++++
 .../devicetree/bindings/mtd/gpmc-nand.txt          |  76 +++++++++
 arch/arm/mach-omap2/gpmc-nand.c                    |  15 +-
 arch/arm/mach-omap2/gpmc.c                         | 178 ++++++++++++++++++++-
 drivers/mtd/nand/omap2.c                           |   4 +-
 include/linux/platform_data/mtd-nand-omap2.h       |   4 +-
 6 files changed, 343 insertions(+), 10 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/bus/ti-gpmc.txt
 create mode 100644 Documentation/devicetree/bindings/mtd/gpmc-nand.txt
Signature · 1 line
1.7.11.7
#1 lore raw

PATCH v7 1/5 ARM: OMAP: gpmc: don't create devices from initcall on DT

On DT driven boards, the gpmc node will match the driver. Hence, there's no need to do that unconditionally from the initcall.

---
 arch/arm/mach-omap2/gpmc.c | 7 +++++++
 1 file changed, 7 insertions(+)
arch/arm/mach-omap2/gpmc.c +7 -0
index 65468f6..d5cbd29 100644
@@ -1214,6 +1214,13 @@ static int __init omap_gpmc_init(void)
 	struct platform_device *pdev;
 	char *oh_name = "gpmc";
 
+	/*
+	 * if the board boots up with a populated DT, do not
+	 * manually add the device from this initcall
+	 */
+	if (of_have_populated_dt())
+		return -ENODEV;
+
 	oh = omap_hwmod_lookup(oh_name);
 	if (!oh) {
 		pr_err("Could not look up %s\n", oh_name);
Signature · 1 line
1.7.11.7
#2 lore raw

PATCH v7 2/5 mtd: omap-nand: pass device_node in platform data

In reply to Daniel Mack

Pass an optional device_node pointer in the platform data, which in turn will be put into a mtd_part_parser_data. This way, code that sets up the platform devices can pass along the node from DT so that the partitions can be parsed.

For non-DT boards, this change has no effect.

---
 drivers/mtd/nand/omap2.c                     | 4 +++-
 include/linux/platform_data/mtd-nand-omap2.h | 4 +++-
 2 files changed, 6 insertions(+), 2 deletions(-)
drivers/mtd/nand/omap2.c +3 -1
index 0002d5e..1d333497c 100644
@@ -1332,6 +1332,7 @@ static int omap_nand_probe(struct platform_device *pdev)
 	dma_cap_mask_t mask;
 	unsigned sig;
 	struct resource			*res;
+	struct mtd_part_parser_data	ppdata = {};
 
 	pdata = pdev->dev.platform_data;
 	if (pdata == NULL) {
@@ -1557,7 +1558,8 @@ static int omap_nand_probe(struct platform_device *pdev)
 		goto out_release_mem_region;
 	}
 
-	mtd_device_parse_register(&info->mtd, NULL, NULL, pdata->parts,
+	ppdata.of_node = pdata->of_node;
+	mtd_device_parse_register(&info->mtd, NULL, &ppdata, pdata->parts,
 				  pdata->nr_parts);
 
 	platform_set_drvdata(pdev, &info->mtd);
include/linux/platform_data/mtd-nand-omap2.h +3 -1
index 24d32ca..6bf9ef4 100644
@@ -60,6 +60,8 @@ struct omap_nand_platform_data {
 	int			devsize;
 	enum omap_ecc           ecc_opt;
 	struct gpmc_nand_regs	reg;
-};
 
+	/* for passing the partitions */
+	struct device_node	*of_node;
+};
 #endif
Signature · 1 line
1.7.11.7
#3 lore raw

PATCH v7 3/5 ARM: OMAP: gpmc-nand: drop __init annotation

In reply to Daniel Mack

gpmc_nand_init() will be called from another driver's probe() function, so the easiest way to prevent section mismatches is to drop the annotation here.

---
 arch/arm/mach-omap2/gpmc-nand.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)
arch/arm/mach-omap2/gpmc-nand.c +3 -3
index db969a5..3059f5e 100644
@@ -89,7 +89,7 @@ static int omap2_nand_gpmc_retime(
 	return 0;
 }
 
-static bool __init gpmc_hwecc_bch_capable(enum omap_ecc ecc_opt)
+static bool gpmc_hwecc_bch_capable(enum omap_ecc ecc_opt)
 {
 	/* support only OMAP3 class */
 	if (!cpu_is_omap34xx()) {
@@ -110,8 +110,8 @@ static bool __init gpmc_hwecc_bch_capable(enum omap_ecc ecc_opt)
 	return 1;
 }
 
-int __init gpmc_nand_init(struct omap_nand_platform_data *gpmc_nand_data,
-			  struct gpmc_timings *gpmc_t)
+int gpmc_nand_init(struct omap_nand_platform_data *gpmc_nand_data,
+		   struct gpmc_timings *gpmc_t)
 {
 	int err	= 0;
 	struct device *dev = &gpmc_nand_device.dev;
Signature · 1 line
1.7.11.7
#4 lore raw

PATCH v7 4/5 ARM: OMAP: gpmc: enable hwecc for AM33xx SoCs

In reply to Daniel Mack

The am33xx is capable of handling bch error correction modes, so enable that feature in the driver.

---
 arch/arm/mach-omap2/gpmc-nand.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)
arch/arm/mach-omap2/gpmc-nand.c +5 -4
index 3059f5e..afc1e8c 100644
@@ -92,17 +92,18 @@ static int omap2_nand_gpmc_retime(
 static bool gpmc_hwecc_bch_capable(enum omap_ecc ecc_opt)
 {
 	/* support only OMAP3 class */
-	if (!cpu_is_omap34xx()) {
+	if (!cpu_is_omap34xx() && !soc_is_am33xx()) {
 		pr_err("BCH ecc is not supported on this CPU\n");
 		return 0;
 	}
 
 	/*
-	 * For now, assume 4-bit mode is only supported on OMAP3630 ES1.x, x>=1.
-	 * Other chips may be added if confirmed to work.
+	 * For now, assume 4-bit mode is only supported on OMAP3630 ES1.x, x>=1
+	 * and AM33xx derivates. Other chips may be added if confirmed to work.
 	 */
 	if ((ecc_opt == OMAP_ECC_BCH4_CODE_HW) &&
-	    (!cpu_is_omap3630() || (GET_OMAP_REVISION() == 0))) {
+	    (!cpu_is_omap3630() || (GET_OMAP_REVISION() == 0)) &&
+	    (!soc_is_am33xx())) {
 		pr_err("BCH 4-bit mode is not supported on this CPU\n");
 		return 0;
 	}
Signature · 1 line
1.7.11.7
#5 lore raw

PATCH v7 5/5 ARM: OMAP: gpmc: add DT bindings for GPMC timings and NAND

In reply to Daniel Mack

This patch adds basic DT bindings for OMAP GPMC.

The actual peripherals are instantiated from child nodes within the GPMC node, and the only type of device that is currently supported is NAND.

Code was added to parse the generic GPMC timing parameters and some documentation with examples on how to use them.

Successfully tested on an AM33xx board.

---
 Documentation/devicetree/bindings/bus/ti-gpmc.txt  |  77 ++++++++++
 .../devicetree/bindings/mtd/gpmc-nand.txt          |  76 +++++++++
 arch/arm/mach-omap2/gpmc.c                         | 171 ++++++++++++++++++++-
 3 files changed, 323 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/devicetree/bindings/bus/ti-gpmc.txt
 create mode 100644 Documentation/devicetree/bindings/mtd/gpmc-nand.txt
Documentation/devicetree/bindings/bus/ti-gpmc.txt new file +77 -0
new file mode 100644
index 0000000..7d2a645
@@ -0,0 +1,77 @@
+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:
+
+ - compatible:		Should be set to "ti,gpmc"
+ - reg:			A resource specifier for the register space
+			(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
+ - num-cs:		The maximum number of chip-select lines that controller
+			can support.
+ - 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>
+
+			Note that this property is not currently parsed.
+			Calculated values derived from the contents of the
+			per-CS register GPMC_CONFIG7 (as set up by the
+			bootloader) are used. That will change in the future,
+			so be sure to fill the correct values here.
+
+Timing properties for child nodes. All are optional and default to 0.
+
+ - gpmc,sync-clk:	Minimum clock period for synchronous mode, in picoseconds
+
+ Chip-select signal timings corresponding to GPMC_CONFIG2:
+ - gpmc,cs-on:		Assertion time
+ - gpmc,cs-rd-off:	Read deassertion time
+ - gpmc,cs-wr-off:	Write deassertion time
+
+ ADV signal timings corresponding to GPMC_CONFIG3:
+ - gpmc,adv-on:		Assertion time
+ - gpmc,adv-rd-off:	Read deassertion time
+ - gpmc,adv-wr-off:	Write deassertion time
+
+ WE signals timings corresponding to GPMC_CONFIG4:
+ - gpmc,we-on:		Assertion time
+ - gpmc,we-off:		Deassertion time
+
+ OE signals timings corresponding to GPMC_CONFIG4:
+ - gpmc,oe-on:		Assertion time
+ - gpmc,oe-off:		Deassertion time
+
+ Access time and cycle time timings corresponding to GPMC_CONFIG5:
+ - gpmc,page-burst-access: Multiple access word delay
+ - gpmc,access:		Start-cycle to first data valid delay
+ - gpmc,rd-cycle:	Total read cycle time
+ - gpmc,wr-cycle:	Total write cycle time
+
+The following are only applicable to OMAP3+ and AM335x:
+ - gpmc,wr-access
+ - gpmc,wr-data-mux-bus
+
+
+Example for an AM33xx board:
+
+	gpmc: gpmc at 50000000 {
+		compatible = "ti,gpmc";
+		ti,hwmods = "gpmc";
+		reg = <0x50000000 0x2000>;
+		interrupts = <100>;
+
+		num-cs = <8>;
+		num-waitpins = <2>;
+		#address-cells = <2>;
+		#size-cells = <1>;
+		ranges = <0 0 0x08000000 0x10000000>; /* CS0 @addr 0x8000000, size 0x10000000 */
+
+		/* child nodes go here */
+	};
Documentation/devicetree/bindings/mtd/gpmc-nand.txt new file +76 -0

76 lines not on this page · raw message

arch/arm/mach-omap2/gpmc.c +170 -1

201 lines not on this page · raw message

Signature · 1 line
1.7.11.7
#6 lore raw

PATCH v7 5/5 ARM: OMAP: gpmc: add DT bindings for GPMC timings and NAND

Quoted 25 lines · Daniel Mack

This patch adds basic DT bindings for OMAP GPMC.

The actual peripherals are instantiated from child nodes within the GPMC node, and the only type of device that is currently supported is NAND.

Code was added to parse the generic GPMC timing parameters and some documentation with examples on how to use them.

Successfully tested on an AM33xx board.

---
 Documentation/devicetree/bindings/bus/ti-gpmc.txt  |  77 ++++++++++
 .../devicetree/bindings/mtd/gpmc-nand.txt          |  76 +++++++++
 arch/arm/mach-omap2/gpmc.c                         | 171 ++++++++++++++++++++-
 3 files changed, 323 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/devicetree/bindings/bus/ti-gpmc.txt
 create mode 100644 Documentation/devicetree/bindings/mtd/gpmc-nand.txt
Documentation/devicetree/bindings/bus/ti-gpmc.txt new file +0 -0
new file mode 100644
index 0000000..7d2a645

Documentation/devicetree/bindings/bus/ti-gpmc.txt line 7

Quoted 7 lines
Documentation/devicetree/bindings/bus/ti-gpmc.txt new file +7 -0
@@ -0,0 +1,77 @@
+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:
+
+ - compatible:		Should be set to "ti,gpmc"

Please, be specific. Use something like "ti,am3340-gpmc" or "ti,omap3430-gpmc". The compatible property is a list so that new devices can claim compatibility with old. Compatible strings that are overly generic are a pet-peave of mine.

Documentation/devicetree/bindings/bus/ti-gpmc.txt line 15

Quoted 8 lines
Documentation/devicetree/bindings/bus/ti-gpmc.txt new file +8 -0
+ - reg:			A resource specifier for the register space
+			(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
+ - num-cs:		The maximum number of chip-select lines that controller
+			can support.

gpmc,num-cs

Documentation/devicetree/bindings/bus/ti-gpmc.txt line 17

Quoted 2 lines
Documentation/devicetree/bindings/bus/ti-gpmc.txt new file +2 -0
+ - num-waitpins:	The maximum number of wait pins that controller can
+			support.

gpmc,num-waitpins

Documentation/devicetree/bindings/bus/ti-gpmc.txt line 27

Quoted 10 lines
Documentation/devicetree/bindings/bus/ti-gpmc.txt new file +10 -0
+ - 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>
+
+			Note that this property is not currently parsed.
+			Calculated values derived from the contents of the
+			per-CS register GPMC_CONFIG7 (as set up by the
+			bootloader) are used. That will change in the future,
+			so be sure to fill the correct values here.

The core linux code will use ranges to decode the reg property of child devices, even if the gpmc driver doesn't use it, so the comment here is misleading.

Otherwise the binding looks fine to me.

Quoted 295 lines
Documentation/devicetree/bindings/mtd/gpmc-nand.txt new file +76 -0
new file mode 100644
index 0000000..b3fafb1
@@ -0,0 +1,76 @@
+Device tree bindings for 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
+
+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
+
+Optional properties:
+
+ - nand-bus-width: 		Set this numeric value to 16 if the hardware
+				is wired that way. If not specified, a bus
+				width of 8 is assumed.
+
+ - ti,nand-ecc-opt:		A string setting the ECC layout to use. One of:
+
+		"sw"		Software method (default)
+		"hw"		Hardware method
+		"hw-romcode"	gpmc hamming mode method & romcode layout
+		"bch4"		4-bit BCH ecc code
+		"bch8"		8-bit BCH ecc code
+
+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:
+
+	gpmc: gpmc at 50000000 {
+		compatible = "ti,gpmc";
+		ti,hwmods = "gpmc";
+		reg = <0x50000000 0x1000000>;
+		interrupts = <100>;
+		num-cs = <8>;
+		num-waitpins = <2>;
+		#address-cells = <2>;
+		#size-cells = <1>;
+		ranges = <0 0 0x08000000 0x2000>;	/* CS0: NAND */
+
+		nand at 0,0 {
+			reg = <0 0 0>; /* CS0, offset 0 */
+			nand-bus-width = <16>;
+			ti,nand-ecc-opt = "bch8";
+
+			gpmc,sync-clk = <0>;
+			gpmc,cs-on = <0>;
+			gpmc,cs-rd-off = <44>;
+			gpmc,cs-wr-off = <44>;
+			gpmc,adv-on = <6>;
+			gpmc,adv-rd-off = <34>;
+			gpmc,adv-wr-off = <44>;
+			gpmc,we-off = <40>;
+			gpmc,oe-off = <54>;
+			gpmc,access = <64>;
+			gpmc,rd-cycle = <82>;
+			gpmc,wr-cycle = <82>;
+			gpmc,wr-access = <40>;
+			gpmc,wr-data-mux-bus = <0>;
+
+			#address-cells = <1>;
+			#size-cells = <1>;
+
+			/* partitions go here */
+		};
+	};
+
arch/arm/mach-omap2/gpmc.c +170 -1
index d5cbd29..0baf9df 100644
@@ -25,6 +25,10 @@
 #include <linux/module.h>
 #include <linux/interrupt.h>
 #include <linux/platform_device.h>
+#include <linux/of.h>
+#include <linux/of_mtd.h>
+#include <linux/of_device.h>
+#include <linux/mtd/nand.h>
 
 #include <linux/platform_data/mtd-nand-omap2.h>
 
@@ -34,6 +38,7 @@
 #include "common.h"
 #include "omap_device.h"
 #include "gpmc.h"
+#include "gpmc-nand.h"
 
 #define	DEVICE_NAME		"omap-gpmc"
 
@@ -1121,7 +1126,162 @@ int gpmc_calc_timings(struct gpmc_timings *gpmc_t,
 	return 0;
 }
 
-static __devinit int gpmc_probe(struct platform_device *pdev)
+#ifdef CONFIG_OF
+static struct of_device_id gpmc_dt_ids[] = {
+	{ .compatible = "ti,gpmc" },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, gpmc_dt_ids);
+
+static void __maybe_unused gpmc_read_timings_dt(struct device_node *np,
+						struct gpmc_timings *gpmc_t)
+{
+	u32 val;
+
+	memset(gpmc_t, 0, sizeof(*gpmc_t));
+
+	/* minimum clock period for syncronous mode */
+	if (!of_property_read_u32(np, "gpmc,sync-clk", &val))
+		gpmc_t->sync_clk = val;
+
+	/* chip select timtings */
+	if (!of_property_read_u32(np, "gpmc,cs-on", &val))
+		gpmc_t->cs_on = val;
+
+	if (!of_property_read_u32(np, "gpmc,cs-rd-off", &val))
+		gpmc_t->cs_rd_off = val;
+
+	if (!of_property_read_u32(np, "gpmc,cs-wr-off", &val))
+		gpmc_t->cs_wr_off = val;
+
+	/* ADV signal timings */
+	if (!of_property_read_u32(np, "gpmc,adv-on", &val))
+		gpmc_t->adv_on = val;
+
+	if (!of_property_read_u32(np, "gpmc,adv-rd-off", &val))
+		gpmc_t->adv_rd_off = val;
+
+	if (!of_property_read_u32(np, "gpmc,adv-wr-off", &val))
+		gpmc_t->adv_wr_off = val;
+
+	/* WE signal timings */
+	if (!of_property_read_u32(np, "gpmc,we-on", &val))
+		gpmc_t->we_on = val;
+
+	if (!of_property_read_u32(np, "gpmc,we-off", &val))
+		gpmc_t->we_off = val;
+
+	/* OE signal timings */
+	if (!of_property_read_u32(np, "gpmc,oe-on", &val))
+		gpmc_t->oe_on = val;
+
+	if (!of_property_read_u32(np, "gpmc,oe-off", &val))
+		gpmc_t->oe_off = val;
+
+	/* access and cycle timings */
+	if (!of_property_read_u32(np, "gpmc,page-burst-access", &val))
+		gpmc_t->page_burst_access = val;
+
+	if (!of_property_read_u32(np, "gpmc,access", &val))
+		gpmc_t->access = val;
+
+	if (!of_property_read_u32(np, "gpmc,rd-cycle", &val))
+		gpmc_t->rd_cycle = val;
+
+	if (!of_property_read_u32(np, "gpmc,wr-cycle", &val))
+		gpmc_t->wr_cycle = val;
+
+	/* only for OMAP3430 */
+	if (!of_property_read_u32(np, "gpmc,wr-access", &val))
+		gpmc_t->wr_access = val;
+
+	if (!of_property_read_u32(np, "gpmc,wr-data-mux-bus", &val))
+		gpmc_t->wr_data_mux_bus = val;
+}
+
+#ifdef CONFIG_MTD_NAND
+
+static const char * const nand_ecc_opts[] = {
+	[OMAP_ECC_HAMMING_CODE_DEFAULT]		= "sw",
+	[OMAP_ECC_HAMMING_CODE_HW]		= "hw",
+	[OMAP_ECC_HAMMING_CODE_HW_ROMCODE]	= "hw-romcode",
+	[OMAP_ECC_BCH4_CODE_HW]			= "bch4",
+	[OMAP_ECC_BCH8_CODE_HW]			= "bch8",
+};
+
+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;
+
+	if (!of_property_read_string(child, "ti,nand-ecc-opt", &s))
+		for (val = 0; val < ARRAY_SIZE(nand_ecc_opts); val++)
+			if (!strcasecmp(s, nand_ecc_opts[val])) {
+				gpmc_nand_data->ecc_opt = 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
+
+static int gpmc_probe_dt(struct platform_device *pdev)
+{
+	int ret;
+	struct device_node *child;
+	const struct of_device_id *of_id =
+		of_match_device(gpmc_dt_ids, &pdev->dev);
+
+	if (!of_id)
+		return 0;
+
+	for_each_node_by_name(child, "nand") {
+		ret = gpmc_probe_nand_child(pdev, child);
+		of_node_put(child);
+		if (ret < 0)
+			return ret;
+	}
+
+	return 0;
+}
+#else
+static int gpmc_probe_dt(struct platform_device *pdev)
+{
+	return 0;
+}
+#endif
+
+static int __devinit gpmc_probe(struct platform_device *pdev)
 {
 	int rc;
 	u32 l;
@@ -1174,6 +1334,14 @@ static __devinit int gpmc_probe(struct platform_device *pdev)
 	if (IS_ERR_VALUE(gpmc_setup_irq()))
 		dev_warn(gpmc_dev, "gpmc_setup_irq failed\n");
 
+	rc = gpmc_probe_dt(pdev);
+	if (rc < 0) {
+		clk_disable_unprepare(gpmc_l3_clk);
+		clk_put(gpmc_l3_clk);
+		dev_err(gpmc_dev, "failed to probe DT parameters\n");
+		return rc;
+	}
+
 	return 0;
 }
 
@@ -1191,6 +1359,7 @@ static struct platform_driver gpmc_driver = {
 	.driver		= {
 		.name	= DEVICE_NAME,
 		.owner	= THIS_MODULE,
+		.of_match_table = of_match_ptr(gpmc_dt_ids),
 	},
 };
 
Signature · 1 line
1.7.11.7
Signature · 2 lines
Grant Likely, B.Sc, P.Eng.
Secret Lab Technologies, Ltd.
#7 lore raw

PATCH v7 5/5 ARM: OMAP: gpmc: add DT bindings for GPMC timings and NAND

Hi Grant,

Quoted 30 lines · Grant Likely
Quoted 29 lines · Daniel Mack

This patch adds basic DT bindings for OMAP GPMC.

The actual peripherals are instantiated from child nodes within the GPMC node, and the only type of device that is currently supported is NAND.

Code was added to parse the generic GPMC timing parameters and some documentation with examples on how to use them.

Successfully tested on an AM33xx board.

---
 Documentation/devicetree/bindings/bus/ti-gpmc.txt  |  77 ++++++++++
 .../devicetree/bindings/mtd/gpmc-nand.txt          |  76 +++++++++
 arch/arm/mach-omap2/gpmc.c                         | 171 ++++++++++++++++++++-
 3 files changed, 323 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/devicetree/bindings/bus/ti-gpmc.txt
 create mode 100644 Documentation/devicetree/bindings/mtd/gpmc-nand.txt
Documentation/devicetree/bindings/bus/ti-gpmc.txt new file +4 -0
new file mode 100644
index 0000000..7d2a645
@@ -0,0 +1,77 @@
+Device tree bindings for OMAP general purpose memory controllers (GPMC)
+
+The actual devices are instantiated from the child nodes of a GPMC node.
+
Quoted 8 lines

Documentation/devicetree/bindings/bus/ti-gpmc.txt line 7

Quoted 3 lines
Documentation/devicetree/bindings/bus/ti-gpmc.txt new file +3 -0
+Required properties:
+
+ - compatible:		Should be set to "ti,gpmc"

Please, be specific. Use something like "ti,am3340-gpmc" or "ti,omap3430-gpmc". The compatible property is a list so that new devices can claim compatibility with old. Compatible strings that are overly generic are a pet-peave of mine.

We aim to use the binding for omap2,3,4,5 as well as the am33xx devices (which are omap based). Would it be sufficient to have "ti,omap2-gpmc" implying all omap2+ based devices or should we have a compatible string for each device supported?

Thanks
Jon

#8 lore raw

PATCH v7 5/5 ARM: OMAP: gpmc: add DT bindings for GPMC timings and NAND

Quoted 42 lines · Jon Hunter

Hi Grant,

Quoted 30 lines · Grant Likely
Quoted 29 lines · Daniel Mack

This patch adds basic DT bindings for OMAP GPMC.

The actual peripherals are instantiated from child nodes within the GPMC node, and the only type of device that is currently supported is NAND.

Code was added to parse the generic GPMC timing parameters and some documentation with examples on how to use them.

Successfully tested on an AM33xx board.

---
 Documentation/devicetree/bindings/bus/ti-gpmc.txt  |  77 ++++++++++
 .../devicetree/bindings/mtd/gpmc-nand.txt          |  76 +++++++++
 arch/arm/mach-omap2/gpmc.c                         | 171 ++++++++++++++++++++-
 3 files changed, 323 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/devicetree/bindings/bus/ti-gpmc.txt
 create mode 100644 Documentation/devicetree/bindings/mtd/gpmc-nand.txt
Documentation/devicetree/bindings/bus/ti-gpmc.txt new file +4 -0
new file mode 100644
index 0000000..7d2a645
@@ -0,0 +1,77 @@
+Device tree bindings for OMAP general purpose memory controllers (GPMC)
+
+The actual devices are instantiated from the child nodes of a GPMC node.
+
Quoted 8 lines

Documentation/devicetree/bindings/bus/ti-gpmc.txt line 7

Quoted 3 lines
Documentation/devicetree/bindings/bus/ti-gpmc.txt new file +3 -0
+Required properties:
+
+ - compatible:		Should be set to "ti,gpmc"

Please, be specific. Use something like "ti,am3340-gpmc" or "ti,omap3430-gpmc". The compatible property is a list so that new devices can claim compatibility with old. Compatible strings that are overly generic are a pet-peave of mine.

Quoted 4 lines

We aim to use the binding for omap2,3,4,5 as well as the am33xx devices (which are omap based). Would it be sufficient to have "ti,omap2-gpmc" implying all omap2+ based devices or should we have a compatible string for each device supported?

Are they each register-level compatible with one another?

The general recommended approach here is to make subsequent silicon claim compatibility with the first compatible implementation.

So, for an am3358 board:

	compatible = "ti,am3358-gpmc", "ti,omap2420-gpmc";

Essentially, what this means is that "ti,omap2420-gpmc" is the generic value instead of "omap2-gpmc". The reason for this is so that the value is anchored against a specific implementation, and not against something completely imaginary or idealized. If a newer version isn't quite compatible with the omap2420-gpmc, then it can drop the compatible claim and the driver really should be told about the new device.

g.

Quoted 8 lines

Thanks
Jon

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Signature · 2 lines
Grant Likely, B.Sc, P.Eng.
Secret Lab Technologies, Ltd.
maintainer
#9 lore raw

PATCH v7 5/5 ARM: OMAP: gpmc: add DT bindings for GPMC timings and NAND

Quoted 19 lines · Grant Likely
Quoted 11 lines · Jon Hunter
Quoted 5 lines · Grant Likely

Please, be specific. Use something like "ti,am3340-gpmc" or "ti,omap3430-gpmc". The compatible property is a list so that new devices can claim compatibility with old. Compatible strings that are overly generic are a pet-peave of mine.

We aim to use the binding for omap2,3,4,5 as well as the am33xx devices (which are omap based). Would it be sufficient to have "ti,omap2-gpmc" implying all omap2+ based devices or should we have a compatible string for each device supported?

Are they each register-level compatible with one another?

The general recommended approach here is to make subsequent silicon claim compatibility with the first compatible implementation.

So, for an am3358 board:

Quoted 8 lines
	compatible = "ti,am3358-gpmc", "ti,omap2420-gpmc";

Essentially, what this means is that "ti,omap2420-gpmc" is the generic value instead of "omap2-gpmc". The reason for this is so that the value is anchored against a specific implementation, and not against something completely imaginary or idealized. If a newer version isn't quite compatible with the omap2420-gpmc, then it can drop the compatible claim and the driver really should be told about the new device.

The compatible property can also be used to figure out which ones need the workarounds in patch #4 of this series for the DT case. So we should be specific with the compatible.

Regards,

Tony

#10 lore raw

PATCH v7 5/5 ARM: OMAP: gpmc: add DT bindings for GPMC timings and NAND

Quoted 29 lines · Tony Lindgren
Quoted 19 lines · Grant Likely
Quoted 11 lines · Jon Hunter
Quoted 5 lines · Grant Likely

Please, be specific. Use something like "ti,am3340-gpmc" or "ti,omap3430-gpmc". The compatible property is a list so that new devices can claim compatibility with old. Compatible strings that are overly generic are a pet-peave of mine.

We aim to use the binding for omap2,3,4,5 as well as the am33xx devices (which are omap based). Would it be sufficient to have "ti,omap2-gpmc" implying all omap2+ based devices or should we have a compatible string for each device supported?

Are they each register-level compatible with one another?

The general recommended approach here is to make subsequent silicon claim compatibility with the first compatible implementation.

So, for an am3358 board:

Quoted 8 lines
	compatible = "ti,am3358-gpmc", "ti,omap2420-gpmc";

Essentially, what this means is that "ti,omap2420-gpmc" is the generic value instead of "omap2-gpmc". The reason for this is so that the value is anchored against a specific implementation, and not against something completely imaginary or idealized. If a newer version isn't quite compatible with the omap2420-gpmc, then it can drop the compatible claim and the driver really should be told about the new device.

Quoted 3 lines

The compatible property can also be used to figure out which ones need the workarounds in patch #4 of this series for the DT case. So we should be specific with the compatible.

We should not merged patch #4. Daniel included this here because he is using this on the current mainline, however, this is not needed for linux-next and so we should drop it.

Cheers
Jon

#11 lore raw

PATCH v7 5/5 ARM: OMAP: gpmc: add DT bindings for GPMC timings and NAND

Quoted 31 lines · Jon Hunter
Quoted 29 lines · Tony Lindgren
Quoted 27 lines · Grant Likely
Quoted 11 lines · Jon Hunter
Quoted 5 lines · Grant Likely

Please, be specific. Use something like "ti,am3340-gpmc" or "ti,omap3430-gpmc". The compatible property is a list so that new devices can claim compatibility with old. Compatible strings that are overly generic are a pet-peave of mine.

We aim to use the binding for omap2,3,4,5 as well as the am33xx devices (which are omap based). Would it be sufficient to have "ti,omap2-gpmc" implying all omap2+ based devices or should we have a compatible string for each device supported?

Are they each register-level compatible with one another?

The general recommended approach here is to make subsequent silicon claim compatibility with the first compatible implementation.

So, for an am3358 board:

	compatible = "ti,am3358-gpmc", "ti,omap2420-gpmc";

Essentially, what this means is that "ti,omap2420-gpmc" is the generic value instead of "omap2-gpmc". The reason for this is so that the value is anchored against a specific implementation, and not against something completely imaginary or idealized. If a newer version isn't quite compatible with the omap2420-gpmc, then it can drop the compatible claim and the driver really should be told about the new device.

Quoted 7 lines
Quoted 3 lines

The compatible property can also be used to figure out which ones need the workarounds in patch #4 of this series for the DT case. So we should be specific with the compatible.

We should not merged patch #4. Daniel included this here because he is using this on the current mainline, however, this is not needed for linux-next and so we should drop it.

I think we're talking about different things here since awhile.

The patch I pointed you which is in mainline and which removes the reference to <plat/gpmc.h> from drivers/mtd/nand/omap2.c has nothing to do with my patch #4. It just solves Tony's concern that regarding the multi-arch zImages.

My code in gpmc.c calls gpmc_nand_init() which in turn calls gpmc_hwecc_bch_capable(). Without path #4, gpmc_hwecc_bch_capable() will return 0, and the nand init will fail consequently, in mainline as well as in linux-next.

I understood Tony that he wanted to remove the entiry function and do the check based on DT properties, which will then solve the problem on a different level. However, that change is planned for *after* the merge window.

Daniel

#12 lore raw

PATCH v7 5/5 ARM: OMAP: gpmc: add DT bindings for GPMC timings and NAND

Quoted 47 lines · Daniel Mack
Quoted 31 lines · Jon Hunter
Quoted 29 lines · Tony Lindgren
Quoted 27 lines · Grant Likely
Quoted 11 lines · Jon Hunter
Quoted 5 lines · Grant Likely

Please, be specific. Use something like "ti,am3340-gpmc" or "ti,omap3430-gpmc". The compatible property is a list so that new devices can claim compatibility with old. Compatible strings that are overly generic are a pet-peave of mine.

We aim to use the binding for omap2,3,4,5 as well as the am33xx devices (which are omap based). Would it be sufficient to have "ti,omap2-gpmc" implying all omap2+ based devices or should we have a compatible string for each device supported?

Are they each register-level compatible with one another?

The general recommended approach here is to make subsequent silicon claim compatibility with the first compatible implementation.

So, for an am3358 board:

	compatible = "ti,am3358-gpmc", "ti,omap2420-gpmc";

Essentially, what this means is that "ti,omap2420-gpmc" is the generic value instead of "omap2-gpmc". The reason for this is so that the value is anchored against a specific implementation, and not against something completely imaginary or idealized. If a newer version isn't quite compatible with the omap2420-gpmc, then it can drop the compatible claim and the driver really should be told about the new device.

Quoted 7 lines
Quoted 3 lines

The compatible property can also be used to figure out which ones need the workarounds in patch #4 of this series for the DT case. So we should be specific with the compatible.

We should not merged patch #4. Daniel included this here because he is using this on the current mainline, however, this is not needed for linux-next and so we should drop it.

I think we're talking about different things here since awhile.

The patch I pointed you which is in mainline and which removes the reference to <plat/gpmc.h> from drivers/mtd/nand/omap2.c has nothing to do with my patch #4. It just solves Tony's concern that regarding the multi-arch zImages.

Quoted 4 lines

My code in gpmc.c calls gpmc_nand_init() which in turn calls gpmc_hwecc_bch_capable(). Without path #4, gpmc_hwecc_bch_capable() will return 0, and the nand init will fail consequently, in mainline as well as in linux-next.

Ok, yes I see that now. I should have looked more closely at linux-next.

Quoted 4 lines

I understood Tony that he wanted to remove the entiry function and do the check based on DT properties, which will then solve the problem on a different level. However, that change is planned for *after* the merge window.

Well now that it is only being called from within the platform code and not from drivers, it is ok.

Cheers
Jon

#13 lore raw

PATCH v7 5/5 ARM: OMAP: gpmc: add DT bindings for GPMC timings and NAND

In reply to Grant Likely
Quoted 43 lines · Grant Likely
Quoted 42 lines · Jon Hunter

Hi Grant,

Quoted 38 lines · Grant Likely
Quoted 25 lines · Daniel Mack

This patch adds basic DT bindings for OMAP GPMC.

The actual peripherals are instantiated from child nodes within the GPMC node, and the only type of device that is currently supported is NAND.

Code was added to parse the generic GPMC timing parameters and some documentation with examples on how to use them.

Successfully tested on an AM33xx board.

---
 Documentation/devicetree/bindings/bus/ti-gpmc.txt  |  77 ++++++++++
 .../devicetree/bindings/mtd/gpmc-nand.txt          |  76 +++++++++
 arch/arm/mach-omap2/gpmc.c                         | 171 ++++++++++++++++++++-
 3 files changed, 323 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/devicetree/bindings/bus/ti-gpmc.txt
 create mode 100644 Documentation/devicetree/bindings/mtd/gpmc-nand.txt
Documentation/devicetree/bindings/bus/ti-gpmc.txt new file +0 -0
new file mode 100644
index 0000000..7d2a645

Documentation/devicetree/bindings/bus/ti-gpmc.txt line 7

Quoted 7 lines
Documentation/devicetree/bindings/bus/ti-gpmc.txt new file +7 -0
@@ -0,0 +1,77 @@
+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:
+
+ - compatible:		Should be set to "ti,gpmc"

Please, be specific. Use something like "ti,am3340-gpmc" or "ti,omap3430-gpmc". The compatible property is a list so that new devices can claim compatibility with old. Compatible strings that are overly generic are a pet-peave of mine.

Quoted 6 lines
Quoted 4 lines

We aim to use the binding for omap2,3,4,5 as well as the am33xx devices (which are omap based). Would it be sufficient to have "ti,omap2-gpmc" implying all omap2+ based devices or should we have a compatible string for each device supported?

Are they each register-level compatible with one another?

They are not 100% register compatible. There are a couple fields in the binding that are only applicable to OMAP3+ devices.

Quoted 4 lines

The general recommended approach here is to make subsequent silicon claim compatibility with the first compatible implementation.

So, for an am3358 board:

Quoted 8 lines
	compatible = "ti,am3358-gpmc", "ti,omap2420-gpmc";

Essentially, what this means is that "ti,omap2420-gpmc" is the generic value instead of "omap2-gpmc". The reason for this is so that the value is anchored against a specific implementation, and not against something completely imaginary or idealized. If a newer version isn't quite compatible with the omap2420-gpmc, then it can drop the compatible claim and the driver really should be told about the new device.

Ok, gotcha! I will do a register comparison and may be recommend to Daniel which compatible strings we will need.

Thanks!
Jon

#14 lore raw

PATCH v7 5/5 ARM: OMAP: gpmc: add DT bindings for GPMC timings and NAND

Hi Jon,

Quoted 68 lines · Jon Hunter
Quoted 43 lines · Grant Likely
Quoted 42 lines · Jon Hunter

Hi Grant,

Quoted 38 lines · Grant Likely
Quoted 25 lines · Daniel Mack

This patch adds basic DT bindings for OMAP GPMC.

The actual peripherals are instantiated from child nodes within the GPMC node, and the only type of device that is currently supported is NAND.

Code was added to parse the generic GPMC timing parameters and some documentation with examples on how to use them.

Successfully tested on an AM33xx board.

---
 Documentation/devicetree/bindings/bus/ti-gpmc.txt  |  77 ++++++++++
 .../devicetree/bindings/mtd/gpmc-nand.txt          |  76 +++++++++
 arch/arm/mach-omap2/gpmc.c                         | 171 ++++++++++++++++++++-
 3 files changed, 323 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/devicetree/bindings/bus/ti-gpmc.txt
 create mode 100644 Documentation/devicetree/bindings/mtd/gpmc-nand.txt
Documentation/devicetree/bindings/bus/ti-gpmc.txt new file +0 -0
new file mode 100644
index 0000000..7d2a645

Documentation/devicetree/bindings/bus/ti-gpmc.txt line 7

Quoted 7 lines
Documentation/devicetree/bindings/bus/ti-gpmc.txt new file +7 -0
@@ -0,0 +1,77 @@
+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:
+
+ - compatible:		Should be set to "ti,gpmc"

Please, be specific. Use something like "ti,am3340-gpmc" or "ti,omap3430-gpmc". The compatible property is a list so that new devices can claim compatibility with old. Compatible strings that are overly generic are a pet-peave of mine.

Quoted 6 lines
Quoted 4 lines

We aim to use the binding for omap2,3,4,5 as well as the am33xx devices (which are omap based). Would it be sufficient to have "ti,omap2-gpmc" implying all omap2+ based devices or should we have a compatible string for each device supported?

Are they each register-level compatible with one another?

They are not 100% register compatible. There are a couple fields in the binding that are only applicable to OMAP3+ devices.

Quoted 4 lines

The general recommended approach here is to make subsequent silicon claim compatibility with the first compatible implementation.

So, for an am3358 board:

Quoted 8 lines
	compatible = "ti,am3358-gpmc", "ti,omap2420-gpmc";

Essentially, what this means is that "ti,omap2420-gpmc" is the generic value instead of "omap2-gpmc". The reason for this is so that the value is anchored against a specific implementation, and not against something completely imaginary or idealized. If a newer version isn't quite compatible with the omap2420-gpmc, then it can drop the compatible claim and the driver really should be told about the new device.

Quoted 2 lines

Ok, gotcha! I will do a register comparison and may be recommend to Daniel which compatible strings we will need.

Ok, thanks a lot!

However, I don't know whether we should treat different models differently in the code yet, distiguished by their 'compatible' string.

We should register the driver correctly so we can use the power of that feature once we get rid of the runtime-hacks (cpu_is_xxx() macros), but I think until we're there, we can well live with multiple compatible-entries that all map to the same driver, right?

OTOH, we could also think about tieing num-waitpins and num-cs to a specific 'compatible' entry (by passing a struct along in .data members of of_device_id), but I'm not sure whether that's really better here.

Daniel

#15 lore raw

PATCH v7 5/5 ARM: OMAP: gpmc: add DT bindings for GPMC timings and NAND

In reply to Jon Hunter
Quoted 68 lines · Jon Hunter
Quoted 43 lines · Grant Likely
Quoted 42 lines · Jon Hunter

Hi Grant,

Quoted 38 lines · Grant Likely
Quoted 25 lines · Daniel Mack

This patch adds basic DT bindings for OMAP GPMC.

The actual peripherals are instantiated from child nodes within the GPMC node, and the only type of device that is currently supported is NAND.

Code was added to parse the generic GPMC timing parameters and some documentation with examples on how to use them.

Successfully tested on an AM33xx board.

---
 Documentation/devicetree/bindings/bus/ti-gpmc.txt  |  77 ++++++++++
 .../devicetree/bindings/mtd/gpmc-nand.txt          |  76 +++++++++
 arch/arm/mach-omap2/gpmc.c                         | 171 ++++++++++++++++++++-
 3 files changed, 323 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/devicetree/bindings/bus/ti-gpmc.txt
 create mode 100644 Documentation/devicetree/bindings/mtd/gpmc-nand.txt
Documentation/devicetree/bindings/bus/ti-gpmc.txt new file +0 -0
new file mode 100644
index 0000000..7d2a645

Documentation/devicetree/bindings/bus/ti-gpmc.txt line 7

Quoted 7 lines
Documentation/devicetree/bindings/bus/ti-gpmc.txt new file +7 -0
@@ -0,0 +1,77 @@
+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:
+
+ - compatible:		Should be set to "ti,gpmc"

Please, be specific. Use something like "ti,am3340-gpmc" or "ti,omap3430-gpmc". The compatible property is a list so that new devices can claim compatibility with old. Compatible strings that are overly generic are a pet-peave of mine.

Quoted 6 lines
Quoted 4 lines

We aim to use the binding for omap2,3,4,5 as well as the am33xx devices (which are omap based). Would it be sufficient to have "ti,omap2-gpmc" implying all omap2+ based devices or should we have a compatible string for each device supported?

Are they each register-level compatible with one another?

They are not 100% register compatible. There are a couple fields in the binding that are only applicable to OMAP3+ devices.

Quoted 4 lines

The general recommended approach here is to make subsequent silicon claim compatibility with the first compatible implementation.

So, for an am3358 board:

Quoted 8 lines
	compatible = "ti,am3358-gpmc", "ti,omap2420-gpmc";

Essentially, what this means is that "ti,omap2420-gpmc" is the generic value instead of "omap2-gpmc". The reason for this is so that the value is anchored against a specific implementation, and not against something completely imaginary or idealized. If a newer version isn't quite compatible with the omap2420-gpmc, then it can drop the compatible claim and the driver really should be told about the new device.

Quoted 2 lines

Ok, gotcha! I will do a register comparison and may be recommend to Daniel which compatible strings we will need.

Any idea yet how we want to continue on this? I'm asking because I'm leaving for a longer trip by the end of this week, and so anything I haven't finished until then will have to be postponed until February or be taken over by someone else :)

Thanks,
Daniel

#16 lore raw

PATCH v7 5/5 ARM: OMAP: gpmc: add DT bindings for GPMC timings and NAND

Quoted 69 lines · Daniel Mack
Quoted 68 lines · Jon Hunter
Quoted 43 lines · Grant Likely
Quoted 42 lines · Jon Hunter

Hi Grant,

Quoted 38 lines · Grant Likely
Quoted 25 lines · Daniel Mack

This patch adds basic DT bindings for OMAP GPMC.

The actual peripherals are instantiated from child nodes within the GPMC node, and the only type of device that is currently supported is NAND.

Code was added to parse the generic GPMC timing parameters and some documentation with examples on how to use them.

Successfully tested on an AM33xx board.

---
 Documentation/devicetree/bindings/bus/ti-gpmc.txt  |  77 ++++++++++
 .../devicetree/bindings/mtd/gpmc-nand.txt          |  76 +++++++++
 arch/arm/mach-omap2/gpmc.c                         | 171 ++++++++++++++++++++-
 3 files changed, 323 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/devicetree/bindings/bus/ti-gpmc.txt
 create mode 100644 Documentation/devicetree/bindings/mtd/gpmc-nand.txt
Documentation/devicetree/bindings/bus/ti-gpmc.txt new file +0 -0
new file mode 100644
index 0000000..7d2a645

Documentation/devicetree/bindings/bus/ti-gpmc.txt line 7

Quoted 7 lines
Documentation/devicetree/bindings/bus/ti-gpmc.txt new file +7 -0
@@ -0,0 +1,77 @@
+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:
+
+ - compatible:		Should be set to "ti,gpmc"

Please, be specific. Use something like "ti,am3340-gpmc" or "ti,omap3430-gpmc". The compatible property is a list so that new devices can claim compatibility with old. Compatible strings that are overly generic are a pet-peave of mine.

Quoted 6 lines
Quoted 4 lines

We aim to use the binding for omap2,3,4,5 as well as the am33xx devices (which are omap based). Would it be sufficient to have "ti,omap2-gpmc" implying all omap2+ based devices or should we have a compatible string for each device supported?

Are they each register-level compatible with one another?

They are not 100% register compatible. There are a couple fields in the binding that are only applicable to OMAP3+ devices.

Quoted 12 lines

The general recommended approach here is to make subsequent silicon claim compatibility with the first compatible implementation.

So, for an am3358 board:

	compatible = "ti,am3358-gpmc", "ti,omap2420-gpmc";

Essentially, what this means is that "ti,omap2420-gpmc" is the generic value instead of "omap2-gpmc". The reason for this is so that the value is anchored against a specific implementation, and not against something completely imaginary or idealized. If a newer version isn't quite compatible with the omap2420-gpmc, then it can drop the compatible claim and the driver really should be told about the new device.

Quoted 7 lines
Quoted 2 lines

Ok, gotcha! I will do a register comparison and may be recommend to Daniel which compatible strings we will need.

Any idea yet how we want to continue on this? I'm asking because I'm leaving for a longer trip by the end of this week, and so anything I haven't finished until then will have to be postponed until February or be taken over by someone else :)

Thanks for the reminder!

So looking at this today, here is what I see when comparing the registers ...

omap2430 != omap2420
omap3430 != omap2430
omap3630 == omap3430
omap4430 != omap3430
omap4460 == omap4430
omap543x == omap4430
am335x != omap4430

Therefore, I believe that we need to have the following compatible strings ...

ti,omap2420-gpmc
ti,omap2430-gpmc
ti,omap3430-gpmc (omap3430 & omap3630)
ti,omap4430-gpmc (omap4430 & omap4460 & omap543x)
ti,am3352-gpmc (am335x devices)

Probably worth adding a comment to the Documentation what should be used for which device.

Cheers
Jon

#17 lore raw

PATCH v7 5/5 ARM: OMAP: gpmc: add DT bindings for GPMC timings and NAND

Quoted 15 lines · Jon Hunter

So looking at this today, here is what I see when comparing the registers ...

omap2430 != omap2420
omap3430 != omap2430
omap3630 == omap3430
omap4430 != omap3430
omap4460 == omap4430
omap543x == omap4430
am335x != omap4430

Therefore, I believe that we need to have the following compatible strings ...

Quoted 8 lines

ti,omap2420-gpmc
ti,omap2430-gpmc
ti,omap3430-gpmc (omap3430 & omap3630)
ti,omap4430-gpmc (omap4430 & omap4460 & omap543x)
ti,am3352-gpmc (am335x devices)

Probably worth adding a comment to the Documentation what should be used for which device.

Yes, it is completely appropriate to add a comment to the documentation here so this doesn't get lost.

g.

#18 lore raw

v7 cover OMAP GPMC DT bindings

In reply to Daniel Mack
Quoted 79 lines · Daniel Mack

This is a series of patches to support GPMC peripherals on OMAP boards.

Grant, Rob, could you have a look and give your Acked-by if appropriate?

Many thanks again,
Daniel

Tested on Linus' master +
omap-next (branch omap-for-v3.8/cleanup-headers-gpmc)

Generated from linux-next as of today, resolving one trivial include file rebase conflict.

The only supported peripheral for now is NAND, but other types would be easy to add.

Version 2 addresses details pointed out by Jon Hunter, Afzal Mohammed and Rob Herring:

- add "reg" and "ti,hwmod" properties to Documentation
- use generic of_mtd functions and the property names defined by them, namely "nand-bus-width" and "nand-ecc-mode"
- reduce the default register space size in the Documentation to 8K, as found in the hwmod code
- switch to a DT layout based on ranges and address translation. Although this property is not currently looked at as long as the handling code still uses the runtime calculation methods, we now have these values in the bindings, eventually allowing us to switch the implementation with less pain.

Version 3 includes fixes pointed out by Jon Hunter:

- better documentation of the 'ranges' property to describe the fact that it's representing the CS lines
- GPMC_CS_CONFIGx -> GPMC_CONFIGx in comments
- drop interrupt-parent from example bindings
- add of_node_put() at the end of the child iteration

Version 4 fixes compilation for !CONFIG_MTD_NAND and includes more details from Jon Hunter and Avinash, Philip:

- Add "num-cs" and "num-waitpins" properties, which will eventually be used to get rid of GPMC_CS_NUM
- Better description of generic nand DT properties
- Dropped patch 3/4 as an equivalent fix was already merged
- Added ti,nand-ecc-use-elm property

Version 5 with regards to Avinash, Philip and Peter Korsgaard:

- Re-add accidentially forgotten Documentation/devicetree/bindings/bus/ti-gpmc.txt
- Rename "software" ecc mode to "sw"
- Initialize gpmc_nand_data->is_elm_used to 'true' rather than 1
- Drop ti,nand-ecc-use-elm binding in favor of a new ecc mode named "bch8-am335xrbl-compatible"
- Add two more patches for section mismatch fixups

Version 6:

- Dropped "bch8-am335xrbl-compatible" mode again. As discussed with Avinash, the ELM issue will be solved subsequently in s separate series.
- re-added a patch to bail out of automatic GPMC instanciation in case of DT boot.
- re-added the "of_node" addition in mtd_nand_omap2.h in 2/5

Version 7: comments from Jon Hunter, all affecting the documentation:

- add num-wait pins properties as they are marked 'required'
- make reg sizes consistent
- AM335x only has 2 wait-pins

Again, many thanks to everybody, in particular Avinash, for the long disussion about how to the these details right in the first place.

Quoted 7 lines

Daniel

Daniel Mack (5):
ARM: OMAP: gpmc: don't create devices from initcall on DT
mtd: omap-nand: pass device_node in platform data
ARM: OMAP: gpmc-nand: drop __init annotation
ARM: OMAP: gpmc: enable hwecc for AM33xx SoCs

Do you still need patch #4 now? Can't we drop this?

May be worth just including what is required for linux-next as this is 3.9 material.

Cheers
Jon

#19 lore raw

v7 cover OMAP GPMC DT bindings

Quoted 89 lines · Jon Hunter
Quoted 79 lines · Daniel Mack

This is a series of patches to support GPMC peripherals on OMAP boards.

Grant, Rob, could you have a look and give your Acked-by if appropriate?

Many thanks again,
Daniel

Tested on Linus' master +
omap-next (branch omap-for-v3.8/cleanup-headers-gpmc)

Generated from linux-next as of today, resolving one trivial include file rebase conflict.

The only supported peripheral for now is NAND, but other types would be easy to add.

Version 2 addresses details pointed out by Jon Hunter, Afzal Mohammed and Rob Herring:

- add "reg" and "ti,hwmod" properties to Documentation
- use generic of_mtd functions and the property names defined by them, namely "nand-bus-width" and "nand-ecc-mode"
- reduce the default register space size in the Documentation to 8K, as found in the hwmod code
- switch to a DT layout based on ranges and address translation. Although this property is not currently looked at as long as the handling code still uses the runtime calculation methods, we now have these values in the bindings, eventually allowing us to switch the implementation with less pain.

Version 3 includes fixes pointed out by Jon Hunter:

- better documentation of the 'ranges' property to describe the fact that it's representing the CS lines
- GPMC_CS_CONFIGx -> GPMC_CONFIGx in comments
- drop interrupt-parent from example bindings
- add of_node_put() at the end of the child iteration

Version 4 fixes compilation for !CONFIG_MTD_NAND and includes more details from Jon Hunter and Avinash, Philip:

- Add "num-cs" and "num-waitpins" properties, which will eventually be used to get rid of GPMC_CS_NUM
- Better description of generic nand DT properties
- Dropped patch 3/4 as an equivalent fix was already merged
- Added ti,nand-ecc-use-elm property

Version 5 with regards to Avinash, Philip and Peter Korsgaard:

- Re-add accidentially forgotten Documentation/devicetree/bindings/bus/ti-gpmc.txt
- Rename "software" ecc mode to "sw"
- Initialize gpmc_nand_data->is_elm_used to 'true' rather than 1
- Drop ti,nand-ecc-use-elm binding in favor of a new ecc mode named "bch8-am335xrbl-compatible"
- Add two more patches for section mismatch fixups

Version 6:

- Dropped "bch8-am335xrbl-compatible" mode again. As discussed with Avinash, the ELM issue will be solved subsequently in s separate series.
- re-added a patch to bail out of automatic GPMC instanciation in case of DT boot.
- re-added the "of_node" addition in mtd_nand_omap2.h in 2/5

Version 7: comments from Jon Hunter, all affecting the documentation:

- add num-wait pins properties as they are marked 'required'
- make reg sizes consistent
- AM335x only has 2 wait-pins

Again, many thanks to everybody, in particular Avinash, for the long disussion about how to the these details right in the first place.

Quoted 7 lines

Daniel

Daniel Mack (5):
ARM: OMAP: gpmc: don't create devices from initcall on DT
mtd: omap-nand: pass device_node in platform data
ARM: OMAP: gpmc-nand: drop __init annotation
ARM: OMAP: gpmc: enable hwecc for AM33xx SoCs

Quoted 4 lines

Do you still need patch #4 now? Can't we drop this?

May be worth just including what is required for linux-next as this is 3.9 material.

Well, yes, otherwise the driver init will bail of course, so I need it locally.

Feel free to drop it when that's not needed at the time of merging the series, but for now I resubmitted everything you need to test and use these patches :)

Daniel