[PATCH v3 1/3] mtd: omap-onenand: pass device_node in platform data

[PATCH v3 1/3] mtd: omap-onenand: pass device_node in platform data

[Ezequiel Garcia]

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.

Acked-by: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>
Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
---
 drivers/mtd/onenand/omap2.c                     |    4 +++-
 include/linux/platform_data/mtd-onenand-omap2.h |    3 +++
 2 files changed, 6 insertions(+), 1 deletions(-)

diff --git a/drivers/mtd/onenand/omap2.c b/drivers/mtd/onenand/omap2.c
index 065f3fe..eec2aed 100644
--- a/drivers/mtd/onenand/omap2.c
+++ b/drivers/mtd/onenand/omap2.c
@@ -637,6 +637,7 @@ static int omap2_onenand_probe(struct platform_device *pdev)
 	struct onenand_chip *this;
 	int r;
 	struct resource *res;
+	struct mtd_part_parser_data ppdata = {};
 
 	pdata = pdev->dev.platform_data;
 	if (pdata == NULL) {
@@ -767,7 +768,8 @@ static int omap2_onenand_probe(struct platform_device *pdev)
 	if ((r = onenand_scan(&c->mtd, 1)) < 0)
 		goto err_release_regulator;
 
-	r = mtd_device_parse_register(&c->mtd, NULL, NULL,
+	ppdata.of_node = pdata->of_node;
+	r = mtd_device_parse_register(&c->mtd, NULL, &ppdata,
 				      pdata ? pdata->parts : NULL,
 				      pdata ? pdata->nr_parts : 0);
 	if (r)
diff --git a/include/linux/platform_data/mtd-onenand-omap2.h b/include/linux/platform_data/mtd-onenand-omap2.h
index 685af7e..e9a9fb1 100644
--- a/include/linux/platform_data/mtd-onenand-omap2.h
+++ b/include/linux/platform_data/mtd-onenand-omap2.h
@@ -29,5 +29,8 @@ struct omap_onenand_platform_data {
 	u8			flags;
 	u8			regulator_can_sleep;
 	u8			skip_initial_unlocking;
+
+	/* for passing the partitions */
+	struct device_node	*of_node;
 };
 #endif
-- 
1.7.8.6

[PATCH v3 1/3] mtd: omap-onenand: pass device_node in platform data

[Ezequiel Garcia]

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.

Acked-by: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>
Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
---
 drivers/mtd/onenand/omap2.c                     |    4 +++-
 include/linux/platform_data/mtd-onenand-omap2.h |    3 +++
 2 files changed, 6 insertions(+), 1 deletions(-)

diff --git a/drivers/mtd/onenand/omap2.c b/drivers/mtd/onenand/omap2.c
index 065f3fe..eec2aed 100644
--- a/drivers/mtd/onenand/omap2.c
+++ b/drivers/mtd/onenand/omap2.c
@@ -637,6 +637,7 @@ static int omap2_onenand_probe(struct platform_device *pdev)
 	struct onenand_chip *this;
 	int r;
 	struct resource *res;
+	struct mtd_part_parser_data ppdata = {};
 
 	pdata = pdev->dev.platform_data;
 	if (pdata == NULL) {
@@ -767,7 +768,8 @@ static int omap2_onenand_probe(struct platform_device *pdev)
 	if ((r = onenand_scan(&c->mtd, 1)) < 0)
 		goto err_release_regulator;
 
-	r = mtd_device_parse_register(&c->mtd, NULL, NULL,
+	ppdata.of_node = pdata->of_node;
+	r = mtd_device_parse_register(&c->mtd, NULL, &ppdata,
 				      pdata ? pdata->parts : NULL,
 				      pdata ? pdata->nr_parts : 0);
 	if (r)
diff --git a/include/linux/platform_data/mtd-onenand-omap2.h b/include/linux/platform_data/mtd-onenand-omap2.h
index 685af7e..e9a9fb1 100644
--- a/include/linux/platform_data/mtd-onenand-omap2.h
+++ b/include/linux/platform_data/mtd-onenand-omap2.h
@@ -29,5 +29,8 @@ struct omap_onenand_platform_data {
 	u8			flags;
 	u8			regulator_can_sleep;
 	u8			skip_initial_unlocking;
+
+	/* for passing the partitions */
+	struct device_node	*of_node;
 };
 #endif
-- 
1.7.8.6

[PATCH v3 1/3] mtd: omap-onenand: pass device_node in platform data

[Ezequiel Garcia]

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.

Acked-by: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>
Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
---
 drivers/mtd/onenand/omap2.c                     |    4 +++-
 include/linux/platform_data/mtd-onenand-omap2.h |    3 +++
 2 files changed, 6 insertions(+), 1 deletions(-)

diff --git a/drivers/mtd/onenand/omap2.c b/drivers/mtd/onenand/omap2.c
index 065f3fe..eec2aed 100644
--- a/drivers/mtd/onenand/omap2.c
+++ b/drivers/mtd/onenand/omap2.c
@@ -637,6 +637,7 @@ static int omap2_onenand_probe(struct platform_device *pdev)
 	struct onenand_chip *this;
 	int r;
 	struct resource *res;
+	struct mtd_part_parser_data ppdata = {};
 
 	pdata = pdev->dev.platform_data;
 	if (pdata == NULL) {
@@ -767,7 +768,8 @@ static int omap2_onenand_probe(struct platform_device *pdev)
 	if ((r = onenand_scan(&c->mtd, 1)) < 0)
 		goto err_release_regulator;
 
-	r = mtd_device_parse_register(&c->mtd, NULL, NULL,
+	ppdata.of_node = pdata->of_node;
+	r = mtd_device_parse_register(&c->mtd, NULL, &ppdata,
 				      pdata ? pdata->parts : NULL,
 				      pdata ? pdata->nr_parts : 0);
 	if (r)
diff --git a/include/linux/platform_data/mtd-onenand-omap2.h b/include/linux/platform_data/mtd-onenand-omap2.h
index 685af7e..e9a9fb1 100644
--- a/include/linux/platform_data/mtd-onenand-omap2.h
+++ b/include/linux/platform_data/mtd-onenand-omap2.h
@@ -29,5 +29,8 @@ struct omap_onenand_platform_data {
 	u8			flags;
 	u8			regulator_can_sleep;
 	u8			skip_initial_unlocking;
+
+	/* for passing the partitions */
+	struct device_node	*of_node;
 };
 #endif
-- 
1.7.8.6

[PATCH v3 2/3] arm: omap2: gpmc-onenand: drop __init annotation

[Ezequiel Garcia]

gpmc_onenand_init() will be called from another driver's probe() function,
so drop the __init annotation, in order to prevent section mismatches.

Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
---
 arch/arm/mach-omap2/gpmc-onenand.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/arch/arm/mach-omap2/gpmc-onenand.c b/arch/arm/mach-omap2/gpmc-onenand.c
index 94a349e..fadd8743 100644
--- a/arch/arm/mach-omap2/gpmc-onenand.c
+++ b/arch/arm/mach-omap2/gpmc-onenand.c
@@ -356,7 +356,7 @@ static int gpmc_onenand_setup(void __iomem *onenand_base, int *freq_ptr)
 	return ret;
 }
 
-void __init gpmc_onenand_init(struct omap_onenand_platform_data *_onenand_data)
+void gpmc_onenand_init(struct omap_onenand_platform_data *_onenand_data)
 {
 	int err;
 
-- 
1.7.8.6

[PATCH v3 2/3] arm: omap2: gpmc-onenand: drop __init annotation

[Ezequiel Garcia]

gpmc_onenand_init() will be called from another driver's probe() function,
so drop the __init annotation, in order to prevent section mismatches.

Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
---
 arch/arm/mach-omap2/gpmc-onenand.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/arch/arm/mach-omap2/gpmc-onenand.c b/arch/arm/mach-omap2/gpmc-onenand.c
index 94a349e..fadd8743 100644
--- a/arch/arm/mach-omap2/gpmc-onenand.c
+++ b/arch/arm/mach-omap2/gpmc-onenand.c
@@ -356,7 +356,7 @@ static int gpmc_onenand_setup(void __iomem *onenand_base, int *freq_ptr)
 	return ret;
 }
 
-void __init gpmc_onenand_init(struct omap_onenand_platform_data *_onenand_data)
+void gpmc_onenand_init(struct omap_onenand_platform_data *_onenand_data)
 {
 	int err;
 
-- 
1.7.8.6

[PATCH v3 2/3] arm: omap2: gpmc-onenand: drop __init annotation

[Ezequiel Garcia]

gpmc_onenand_init() will be called from another driver's probe() function,
so drop the __init annotation, in order to prevent section mismatches.

Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
---
 arch/arm/mach-omap2/gpmc-onenand.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/arch/arm/mach-omap2/gpmc-onenand.c b/arch/arm/mach-omap2/gpmc-onenand.c
index 94a349e..fadd8743 100644
--- a/arch/arm/mach-omap2/gpmc-onenand.c
+++ b/arch/arm/mach-omap2/gpmc-onenand.c
@@ -356,7 +356,7 @@ static int gpmc_onenand_setup(void __iomem *onenand_base, int *freq_ptr)
 	return ret;
 }
 
-void __init gpmc_onenand_init(struct omap_onenand_platform_data *_onenand_data)
+void gpmc_onenand_init(struct omap_onenand_platform_data *_onenand_data)
 {
 	int err;
 
-- 
1.7.8.6

[PATCH v3 3/3] arm: omap2: gpmc: add DT bindings for OneNAND

[Ezequiel Garcia]

This patch adds device tree bindings for OMAP OneNAND devices.
Tested on an OMAP3 3430 IGEPv2 board.

Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
---
Changes from v2:
 * Remove unneeded of_node_put() as reported by Mark Rutland

Changes from v1:
 * Fix typo in Documentation/devicetree/bindings/mtd/gpmc-onenand.txt

 .../devicetree/bindings/mtd/gpmc-onenand.txt       |   43 +++++++++++++++++++
 arch/arm/mach-omap2/gpmc.c                         |   45 ++++++++++++++++++++
 2 files changed, 88 insertions(+), 0 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/mtd/gpmc-onenand.txt

diff --git a/Documentation/devicetree/bindings/mtd/gpmc-onenand.txt b/Documentation/devicetree/bindings/mtd/gpmc-onenand.txt
new file mode 100644
index 0000000..deec9da
--- /dev/null
+++ b/Documentation/devicetree/bindings/mtd/gpmc-onenand.txt
@@ -0,0 +1,43 @@
+Device tree bindings for GPMC connected OneNANDs
+
+GPMC connected OneNAND (found on OMAP boards) are represented as child nodes of
+the GPMC controller with a name of "onenand".
+
+All timing relevant properties as well as generic gpmc child properties are
+explained in a separate documents - please refer to
+Documentation/devicetree/bindings/bus/ti-gpmc.txt
+
+Required properties:
+
+ - reg:			The CS line the peripheral is connected to
+
+Optional properties:
+
+ - dma-channel:		DMA Channel index
+
+For inline partiton table parsing (optional):
+
+ - #address-cells: should be set to 1
+ - #size-cells: should be set to 1
+
+Example for an OMAP3430 board:
+
+	gpmc: gpmc@6e000000 {
+		compatible = "ti,omap3430-gpmc";
+		ti,hwmods = "gpmc";
+		reg = <0x6e000000 0x1000000>;
+		interrupts = <20>;
+		gpmc,num-cs = <8>;
+		gpmc,num-waitpins = <4>;
+		#address-cells = <2>;
+		#size-cells = <1>;
+
+		onenand@0 {
+			reg = <0 0 0>; /* CS0, offset 0 */
+
+			#address-cells = <1>;
+			#size-cells = <1>;
+
+			/* partitions go here */
+		};
+	};
diff --git a/arch/arm/mach-omap2/gpmc.c b/arch/arm/mach-omap2/gpmc.c
index c6255f7..0636d0a 100644
--- a/arch/arm/mach-omap2/gpmc.c
+++ b/arch/arm/mach-omap2/gpmc.c
@@ -39,6 +39,7 @@
 #include "omap_device.h"
 #include "gpmc.h"
 #include "gpmc-nand.h"
+#include "gpmc-onenand.h"
 
 #define	DEVICE_NAME		"omap-gpmc"
 
@@ -1259,6 +1260,43 @@ static int gpmc_probe_nand_child(struct platform_device *pdev,
 }
 #endif
 
+#ifdef CONFIG_MTD_ONENAND
+static int gpmc_probe_onenand_child(struct platform_device *pdev,
+				 struct device_node *child)
+{
+	u32 val;
+	struct omap_onenand_platform_data *gpmc_onenand_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_onenand_data = devm_kzalloc(&pdev->dev, sizeof(*gpmc_onenand_data),
+					 GFP_KERNEL);
+	if (!gpmc_onenand_data)
+		return -ENOMEM;
+
+	gpmc_onenand_data->cs = val;
+	gpmc_onenand_data->of_node = child;
+	gpmc_onenand_data->dma_channel = -1;
+
+	if (!of_property_read_u32(child, "dma-channel", &val))
+		gpmc_onenand_data->dma_channel = val;
+
+	gpmc_onenand_init(gpmc_onenand_data);
+
+	return 0;
+}
+#else
+static int gpmc_probe_onenand_child(struct platform_device *pdev,
+				    struct device_node *child)
+{
+	return 0;
+}
+#endif
+
 static int gpmc_probe_dt(struct platform_device *pdev)
 {
 	int ret;
@@ -1277,6 +1315,13 @@ static int gpmc_probe_dt(struct platform_device *pdev)
 		}
 	}
 
+	for_each_node_by_name(child, "onenand") {
+		ret = gpmc_probe_onenand_child(pdev, child);
+		if (ret < 0) {
+			of_node_put(child);
+			return ret;
+		}
+	}
 	return 0;
 }
 #else
-- 
1.7.8.6

[PATCH v3 3/3] arm: omap2: gpmc: add DT bindings for OneNAND

[Ezequiel Garcia]

This patch adds device tree bindings for OMAP OneNAND devices.
Tested on an OMAP3 3430 IGEPv2 board.

Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
---
Changes from v2:
 * Remove unneeded of_node_put() as reported by Mark Rutland

Changes from v1:
 * Fix typo in Documentation/devicetree/bindings/mtd/gpmc-onenand.txt

 .../devicetree/bindings/mtd/gpmc-onenand.txt       |   43 +++++++++++++++++++
 arch/arm/mach-omap2/gpmc.c                         |   45 ++++++++++++++++++++
 2 files changed, 88 insertions(+), 0 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/mtd/gpmc-onenand.txt

diff --git a/Documentation/devicetree/bindings/mtd/gpmc-onenand.txt b/Documentation/devicetree/bindings/mtd/gpmc-onenand.txt
new file mode 100644
index 0000000..deec9da
--- /dev/null
+++ b/Documentation/devicetree/bindings/mtd/gpmc-onenand.txt
@@ -0,0 +1,43 @@
+Device tree bindings for GPMC connected OneNANDs
+
+GPMC connected OneNAND (found on OMAP boards) are represented as child nodes of
+the GPMC controller with a name of "onenand".
+
+All timing relevant properties as well as generic gpmc child properties are
+explained in a separate documents - please refer to
+Documentation/devicetree/bindings/bus/ti-gpmc.txt
+
+Required properties:
+
+ - reg:			The CS line the peripheral is connected to
+
+Optional properties:
+
+ - dma-channel:		DMA Channel index
+
+For inline partiton table parsing (optional):
+
+ - #address-cells: should be set to 1
+ - #size-cells: should be set to 1
+
+Example for an OMAP3430 board:
+
+	gpmc: gpmc@6e000000 {
+		compatible = "ti,omap3430-gpmc";
+		ti,hwmods = "gpmc";
+		reg = <0x6e000000 0x1000000>;
+		interrupts = <20>;
+		gpmc,num-cs = <8>;
+		gpmc,num-waitpins = <4>;
+		#address-cells = <2>;
+		#size-cells = <1>;
+
+		onenand@0 {
+			reg = <0 0 0>; /* CS0, offset 0 */
+
+			#address-cells = <1>;
+			#size-cells = <1>;
+
+			/* partitions go here */
+		};
+	};
diff --git a/arch/arm/mach-omap2/gpmc.c b/arch/arm/mach-omap2/gpmc.c
index c6255f7..0636d0a 100644
--- a/arch/arm/mach-omap2/gpmc.c
+++ b/arch/arm/mach-omap2/gpmc.c
@@ -39,6 +39,7 @@
 #include "omap_device.h"
 #include "gpmc.h"
 #include "gpmc-nand.h"
+#include "gpmc-onenand.h"
 
 #define	DEVICE_NAME		"omap-gpmc"
 
@@ -1259,6 +1260,43 @@ static int gpmc_probe_nand_child(struct platform_device *pdev,
 }
 #endif
 
+#ifdef CONFIG_MTD_ONENAND
+static int gpmc_probe_onenand_child(struct platform_device *pdev,
+				 struct device_node *child)
+{
+	u32 val;
+	struct omap_onenand_platform_data *gpmc_onenand_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_onenand_data = devm_kzalloc(&pdev->dev, sizeof(*gpmc_onenand_data),
+					 GFP_KERNEL);
+	if (!gpmc_onenand_data)
+		return -ENOMEM;
+
+	gpmc_onenand_data->cs = val;
+	gpmc_onenand_data->of_node = child;
+	gpmc_onenand_data->dma_channel = -1;
+
+	if (!of_property_read_u32(child, "dma-channel", &val))
+		gpmc_onenand_data->dma_channel = val;
+
+	gpmc_onenand_init(gpmc_onenand_data);
+
+	return 0;
+}
+#else
+static int gpmc_probe_onenand_child(struct platform_device *pdev,
+				    struct device_node *child)
+{
+	return 0;
+}
+#endif
+
 static int gpmc_probe_dt(struct platform_device *pdev)
 {
 	int ret;
@@ -1277,6 +1315,13 @@ static int gpmc_probe_dt(struct platform_device *pdev)
 		}
 	}
 
+	for_each_node_by_name(child, "onenand") {
+		ret = gpmc_probe_onenand_child(pdev, child);
+		if (ret < 0) {
+			of_node_put(child);
+			return ret;
+		}
+	}
 	return 0;
 }
 #else
-- 
1.7.8.6

[PATCH v3 3/3] arm: omap2: gpmc: add DT bindings for OneNAND

[Ezequiel Garcia]

This patch adds device tree bindings for OMAP OneNAND devices.
Tested on an OMAP3 3430 IGEPv2 board.

Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
---
Changes from v2:
 * Remove unneeded of_node_put() as reported by Mark Rutland

Changes from v1:
 * Fix typo in Documentation/devicetree/bindings/mtd/gpmc-onenand.txt

 .../devicetree/bindings/mtd/gpmc-onenand.txt       |   43 +++++++++++++++++++
 arch/arm/mach-omap2/gpmc.c                         |   45 ++++++++++++++++++++
 2 files changed, 88 insertions(+), 0 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/mtd/gpmc-onenand.txt

diff --git a/Documentation/devicetree/bindings/mtd/gpmc-onenand.txt b/Documentation/devicetree/bindings/mtd/gpmc-onenand.txt
new file mode 100644
index 0000000..deec9da
--- /dev/null
+++ b/Documentation/devicetree/bindings/mtd/gpmc-onenand.txt
@@ -0,0 +1,43 @@
+Device tree bindings for GPMC connected OneNANDs
+
+GPMC connected OneNAND (found on OMAP boards) are represented as child nodes of
+the GPMC controller with a name of "onenand".
+
+All timing relevant properties as well as generic gpmc child properties are
+explained in a separate documents - please refer to
+Documentation/devicetree/bindings/bus/ti-gpmc.txt
+
+Required properties:
+
+ - reg:			The CS line the peripheral is connected to
+
+Optional properties:
+
+ - dma-channel:		DMA Channel index
+
+For inline partiton table parsing (optional):
+
+ - #address-cells: should be set to 1
+ - #size-cells: should be set to 1
+
+Example for an OMAP3430 board:
+
+	gpmc: gpmc at 6e000000 {
+		compatible = "ti,omap3430-gpmc";
+		ti,hwmods = "gpmc";
+		reg = <0x6e000000 0x1000000>;
+		interrupts = <20>;
+		gpmc,num-cs = <8>;
+		gpmc,num-waitpins = <4>;
+		#address-cells = <2>;
+		#size-cells = <1>;
+
+		onenand at 0 {
+			reg = <0 0 0>; /* CS0, offset 0 */
+
+			#address-cells = <1>;
+			#size-cells = <1>;
+
+			/* partitions go here */
+		};
+	};
diff --git a/arch/arm/mach-omap2/gpmc.c b/arch/arm/mach-omap2/gpmc.c
index c6255f7..0636d0a 100644
--- a/arch/arm/mach-omap2/gpmc.c
+++ b/arch/arm/mach-omap2/gpmc.c
@@ -39,6 +39,7 @@
 #include "omap_device.h"
 #include "gpmc.h"
 #include "gpmc-nand.h"
+#include "gpmc-onenand.h"
 
 #define	DEVICE_NAME		"omap-gpmc"
 
@@ -1259,6 +1260,43 @@ static int gpmc_probe_nand_child(struct platform_device *pdev,
 }
 #endif
 
+#ifdef CONFIG_MTD_ONENAND
+static int gpmc_probe_onenand_child(struct platform_device *pdev,
+				 struct device_node *child)
+{
+	u32 val;
+	struct omap_onenand_platform_data *gpmc_onenand_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_onenand_data = devm_kzalloc(&pdev->dev, sizeof(*gpmc_onenand_data),
+					 GFP_KERNEL);
+	if (!gpmc_onenand_data)
+		return -ENOMEM;
+
+	gpmc_onenand_data->cs = val;
+	gpmc_onenand_data->of_node = child;
+	gpmc_onenand_data->dma_channel = -1;
+
+	if (!of_property_read_u32(child, "dma-channel", &val))
+		gpmc_onenand_data->dma_channel = val;
+
+	gpmc_onenand_init(gpmc_onenand_data);
+
+	return 0;
+}
+#else
+static int gpmc_probe_onenand_child(struct platform_device *pdev,
+				    struct device_node *child)
+{
+	return 0;
+}
+#endif
+
 static int gpmc_probe_dt(struct platform_device *pdev)
 {
 	int ret;
@@ -1277,6 +1315,13 @@ static int gpmc_probe_dt(struct platform_device *pdev)
 		}
 	}
 
+	for_each_node_by_name(child, "onenand") {
+		ret = gpmc_probe_onenand_child(pdev, child);
+		if (ret < 0) {
+			of_node_put(child);
+			return ret;
+		}
+	}
 	return 0;
 }
 #else
-- 
1.7.8.6

Re: [PATCH v3 3/3] arm: omap2: gpmc: add DT bindings for OneNAND

[Mark Rutland]

Hi,

I have a couple more comments after looking though this a bit more thoroughly.

On Fri, Jan 25, 2013 at 12:23:11PM +0000, Ezequiel Garcia wrote:
> This patch adds device tree bindings for OMAP OneNAND devices.
> Tested on an OMAP3 3430 IGEPv2 board.
> 
> Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
> ---
> Changes from v2:
>  * Remove unneeded of_node_put() as reported by Mark Rutland
> 
> Changes from v1:
>  * Fix typo in Documentation/devicetree/bindings/mtd/gpmc-onenand.txt
> 
>  .../devicetree/bindings/mtd/gpmc-onenand.txt       |   43 +++++++++++++++++++
>  arch/arm/mach-omap2/gpmc.c                         |   45 ++++++++++++++++++++
>  2 files changed, 88 insertions(+), 0 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/mtd/gpmc-onenand.txt
> 
> diff --git a/Documentation/devicetree/bindings/mtd/gpmc-onenand.txt b/Documentation/devicetree/bindings/mtd/gpmc-onenand.txt
> new file mode 100644
> index 0000000..deec9da
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mtd/gpmc-onenand.txt
> @@ -0,0 +1,43 @@
> +Device tree bindings for GPMC connected OneNANDs
> +
> +GPMC connected OneNAND (found on OMAP boards) are represented as child nodes of
> +the GPMC controller with a name of "onenand".
> +
> +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

Which tree can I find this in?

> +
> +Required properties:
> +
> + - reg:			The CS line the peripheral is connected to
> +
> +Optional properties:
> +
> + - dma-channel:		DMA Channel index
> +
> +For inline partiton table parsing (optional):
> +
> + - #address-cells: should be set to 1
> + - #size-cells: should be set to 1
> +
> +Example for an OMAP3430 board:
> +
> +	gpmc: gpmc@6e000000 {
> +		compatible = "ti,omap3430-gpmc";
> +		ti,hwmods = "gpmc";
> +		reg = <0x6e000000 0x1000000>;
> +		interrupts = <20>;
> +		gpmc,num-cs = <8>;
> +		gpmc,num-waitpins = <4>;
> +		#address-cells = <2>;
> +		#size-cells = <1>;
> +
> +		onenand@0 {
> +			reg = <0 0 0>; /* CS0, offset 0 */
> +
> +			#address-cells = <1>;
> +			#size-cells = <1>;
> +
> +			/* partitions go here */
> +		};
> +	};
> diff --git a/arch/arm/mach-omap2/gpmc.c b/arch/arm/mach-omap2/gpmc.c
> index c6255f7..0636d0a 100644
> --- a/arch/arm/mach-omap2/gpmc.c
> +++ b/arch/arm/mach-omap2/gpmc.c
> @@ -39,6 +39,7 @@
>  #include "omap_device.h"
>  #include "gpmc.h"
>  #include "gpmc-nand.h"
> +#include "gpmc-onenand.h"
>  
>  #define	DEVICE_NAME		"omap-gpmc"
>  
> @@ -1259,6 +1260,43 @@ static int gpmc_probe_nand_child(struct platform_device *pdev,
>  }
>  #endif
>  
> +#ifdef CONFIG_MTD_ONENAND
> +static int gpmc_probe_onenand_child(struct platform_device *pdev,
> +				 struct device_node *child)
> +{
> +	u32 val;
> +	struct omap_onenand_platform_data *gpmc_onenand_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;
> +	}

I don't understand the format of the reg property, but it seems odd that you
only need to read one cell from it. Are the remaining address cell and size
cell used anywhere?

> +
> +	gpmc_onenand_data = devm_kzalloc(&pdev->dev, sizeof(*gpmc_onenand_data),
> +					 GFP_KERNEL);
> +	if (!gpmc_onenand_data)
> +		return -ENOMEM;
> +
> +	gpmc_onenand_data->cs = val;
> +	gpmc_onenand_data->of_node = child;
> +	gpmc_onenand_data->dma_channel = -1;
> +
> +	if (!of_property_read_u32(child, "dma-channel", &val))
> +		gpmc_onenand_data->dma_channel = val;
> +
> +	gpmc_onenand_init(gpmc_onenand_data);
> +
> +	return 0;
> +}

[...]

Otherwise looks good.

Thanks,
Mark.

Re: [PATCH v3 3/3] arm: omap2: gpmc: add DT bindings for OneNAND

[Mark Rutland]

Hi,

I have a couple more comments after looking though this a bit more thoroughly.

On Fri, Jan 25, 2013 at 12:23:11PM +0000, Ezequiel Garcia wrote:
> This patch adds device tree bindings for OMAP OneNAND devices.
> Tested on an OMAP3 3430 IGEPv2 board.
> 
> Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
> ---
> Changes from v2:
>  * Remove unneeded of_node_put() as reported by Mark Rutland
> 
> Changes from v1:
>  * Fix typo in Documentation/devicetree/bindings/mtd/gpmc-onenand.txt
> 
>  .../devicetree/bindings/mtd/gpmc-onenand.txt       |   43 +++++++++++++++++++
>  arch/arm/mach-omap2/gpmc.c                         |   45 ++++++++++++++++++++
>  2 files changed, 88 insertions(+), 0 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/mtd/gpmc-onenand.txt
> 
> diff --git a/Documentation/devicetree/bindings/mtd/gpmc-onenand.txt b/Documentation/devicetree/bindings/mtd/gpmc-onenand.txt
> new file mode 100644
> index 0000000..deec9da
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mtd/gpmc-onenand.txt
> @@ -0,0 +1,43 @@
> +Device tree bindings for GPMC connected OneNANDs
> +
> +GPMC connected OneNAND (found on OMAP boards) are represented as child nodes of
> +the GPMC controller with a name of "onenand".
> +
> +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

Which tree can I find this in?

> +
> +Required properties:
> +
> + - reg:			The CS line the peripheral is connected to
> +
> +Optional properties:
> +
> + - dma-channel:		DMA Channel index
> +
> +For inline partiton table parsing (optional):
> +
> + - #address-cells: should be set to 1
> + - #size-cells: should be set to 1
> +
> +Example for an OMAP3430 board:
> +
> +	gpmc: gpmc@6e000000 {
> +		compatible = "ti,omap3430-gpmc";
> +		ti,hwmods = "gpmc";
> +		reg = <0x6e000000 0x1000000>;
> +		interrupts = <20>;
> +		gpmc,num-cs = <8>;
> +		gpmc,num-waitpins = <4>;
> +		#address-cells = <2>;
> +		#size-cells = <1>;
> +
> +		onenand@0 {
> +			reg = <0 0 0>; /* CS0, offset 0 */
> +
> +			#address-cells = <1>;
> +			#size-cells = <1>;
> +
> +			/* partitions go here */
> +		};
> +	};
> diff --git a/arch/arm/mach-omap2/gpmc.c b/arch/arm/mach-omap2/gpmc.c
> index c6255f7..0636d0a 100644
> --- a/arch/arm/mach-omap2/gpmc.c
> +++ b/arch/arm/mach-omap2/gpmc.c
> @@ -39,6 +39,7 @@
>  #include "omap_device.h"
>  #include "gpmc.h"
>  #include "gpmc-nand.h"
> +#include "gpmc-onenand.h"
>  
>  #define	DEVICE_NAME		"omap-gpmc"
>  
> @@ -1259,6 +1260,43 @@ static int gpmc_probe_nand_child(struct platform_device *pdev,
>  }
>  #endif
>  
> +#ifdef CONFIG_MTD_ONENAND
> +static int gpmc_probe_onenand_child(struct platform_device *pdev,
> +				 struct device_node *child)
> +{
> +	u32 val;
> +	struct omap_onenand_platform_data *gpmc_onenand_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;
> +	}

I don't understand the format of the reg property, but it seems odd that you
only need to read one cell from it. Are the remaining address cell and size
cell used anywhere?

> +
> +	gpmc_onenand_data = devm_kzalloc(&pdev->dev, sizeof(*gpmc_onenand_data),
> +					 GFP_KERNEL);
> +	if (!gpmc_onenand_data)
> +		return -ENOMEM;
> +
> +	gpmc_onenand_data->cs = val;
> +	gpmc_onenand_data->of_node = child;
> +	gpmc_onenand_data->dma_channel = -1;
> +
> +	if (!of_property_read_u32(child, "dma-channel", &val))
> +		gpmc_onenand_data->dma_channel = val;
> +
> +	gpmc_onenand_init(gpmc_onenand_data);
> +
> +	return 0;
> +}

[...]

Otherwise looks good.

Thanks,
Mark.

[PATCH v3 3/3] arm: omap2: gpmc: add DT bindings for OneNAND

[Mark Rutland]

Hi,

I have a couple more comments after looking though this a bit more thoroughly.

On Fri, Jan 25, 2013 at 12:23:11PM +0000, Ezequiel Garcia wrote:
> This patch adds device tree bindings for OMAP OneNAND devices.
> Tested on an OMAP3 3430 IGEPv2 board.
> 
> Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
> ---
> Changes from v2:
>  * Remove unneeded of_node_put() as reported by Mark Rutland
> 
> Changes from v1:
>  * Fix typo in Documentation/devicetree/bindings/mtd/gpmc-onenand.txt
> 
>  .../devicetree/bindings/mtd/gpmc-onenand.txt       |   43 +++++++++++++++++++
>  arch/arm/mach-omap2/gpmc.c                         |   45 ++++++++++++++++++++
>  2 files changed, 88 insertions(+), 0 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/mtd/gpmc-onenand.txt
> 
> diff --git a/Documentation/devicetree/bindings/mtd/gpmc-onenand.txt b/Documentation/devicetree/bindings/mtd/gpmc-onenand.txt
> new file mode 100644
> index 0000000..deec9da
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mtd/gpmc-onenand.txt
> @@ -0,0 +1,43 @@
> +Device tree bindings for GPMC connected OneNANDs
> +
> +GPMC connected OneNAND (found on OMAP boards) are represented as child nodes of
> +the GPMC controller with a name of "onenand".
> +
> +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

Which tree can I find this in?

> +
> +Required properties:
> +
> + - reg:			The CS line the peripheral is connected to
> +
> +Optional properties:
> +
> + - dma-channel:		DMA Channel index
> +
> +For inline partiton table parsing (optional):
> +
> + - #address-cells: should be set to 1
> + - #size-cells: should be set to 1
> +
> +Example for an OMAP3430 board:
> +
> +	gpmc: gpmc at 6e000000 {
> +		compatible = "ti,omap3430-gpmc";
> +		ti,hwmods = "gpmc";
> +		reg = <0x6e000000 0x1000000>;
> +		interrupts = <20>;
> +		gpmc,num-cs = <8>;
> +		gpmc,num-waitpins = <4>;
> +		#address-cells = <2>;
> +		#size-cells = <1>;
> +
> +		onenand at 0 {
> +			reg = <0 0 0>; /* CS0, offset 0 */
> +
> +			#address-cells = <1>;
> +			#size-cells = <1>;
> +
> +			/* partitions go here */
> +		};
> +	};
> diff --git a/arch/arm/mach-omap2/gpmc.c b/arch/arm/mach-omap2/gpmc.c
> index c6255f7..0636d0a 100644
> --- a/arch/arm/mach-omap2/gpmc.c
> +++ b/arch/arm/mach-omap2/gpmc.c
> @@ -39,6 +39,7 @@
>  #include "omap_device.h"
>  #include "gpmc.h"
>  #include "gpmc-nand.h"
> +#include "gpmc-onenand.h"
>  
>  #define	DEVICE_NAME		"omap-gpmc"
>  
> @@ -1259,6 +1260,43 @@ static int gpmc_probe_nand_child(struct platform_device *pdev,
>  }
>  #endif
>  
> +#ifdef CONFIG_MTD_ONENAND
> +static int gpmc_probe_onenand_child(struct platform_device *pdev,
> +				 struct device_node *child)
> +{
> +	u32 val;
> +	struct omap_onenand_platform_data *gpmc_onenand_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;
> +	}

I don't understand the format of the reg property, but it seems odd that you
only need to read one cell from it. Are the remaining address cell and size
cell used anywhere?

> +
> +	gpmc_onenand_data = devm_kzalloc(&pdev->dev, sizeof(*gpmc_onenand_data),
> +					 GFP_KERNEL);
> +	if (!gpmc_onenand_data)
> +		return -ENOMEM;
> +
> +	gpmc_onenand_data->cs = val;
> +	gpmc_onenand_data->of_node = child;
> +	gpmc_onenand_data->dma_channel = -1;
> +
> +	if (!of_property_read_u32(child, "dma-channel", &val))
> +		gpmc_onenand_data->dma_channel = val;
> +
> +	gpmc_onenand_init(gpmc_onenand_data);
> +
> +	return 0;
> +}

[...]

Otherwise looks good.

Thanks,
Mark.

Re: [PATCH v3 3/3] arm: omap2: gpmc: add DT bindings for OneNAND

[Ezequiel Garcia]

Hi Mark,

First of all: thanks for reviewing.

On Fri, Jan 25, 2013 at 12:56 PM, Mark Rutland <mark.rutland@arm.com> wrote:
> Hi,
>
> I have a couple more comments after looking though this a bit more thoroughly.
>
> On Fri, Jan 25, 2013 at 12:23:11PM +0000, Ezequiel Garcia wrote:
>> This patch adds device tree bindings for OMAP OneNAND devices.
>> Tested on an OMAP3 3430 IGEPv2 board.
>>
>> Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
>> ---
>> Changes from v2:
>>  * Remove unneeded of_node_put() as reported by Mark Rutland
>>
>> Changes from v1:
>>  * Fix typo in Documentation/devicetree/bindings/mtd/gpmc-onenand.txt
>>
>>  .../devicetree/bindings/mtd/gpmc-onenand.txt       |   43 +++++++++++++++++++
>>  arch/arm/mach-omap2/gpmc.c                         |   45 ++++++++++++++++++++
>>  2 files changed, 88 insertions(+), 0 deletions(-)
>>  create mode 100644 Documentation/devicetree/bindings/mtd/gpmc-onenand.txt
>>
>> diff --git a/Documentation/devicetree/bindings/mtd/gpmc-onenand.txt b/Documentation/devicetree/bindings/mtd/gpmc-onenand.txt
>> new file mode 100644
>> index 0000000..deec9da
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/mtd/gpmc-onenand.txt
>> @@ -0,0 +1,43 @@
>> +Device tree bindings for GPMC connected OneNANDs
>> +
>> +GPMC connected OneNAND (found on OMAP boards) are represented as child nodes of
>> +the GPMC controller with a name of "onenand".
>> +
>> +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
>
> Which tree can I find this in?
>

GPMC binding was posted by Daniel Mack a while ago.
Tony has recently pushed it to his master branch here:
git://git.kernel.org/pub/scm/linux/kernel/git/tmlind/linux-omap.git

>> +
>> +Required properties:
>> +
>> + - reg:                      The CS line the peripheral is connected to
>> +
>> +Optional properties:
>> +
>> + - dma-channel:              DMA Channel index
>> +
>> +For inline partiton table parsing (optional):
>> +
>> + - #address-cells: should be set to 1
>> + - #size-cells: should be set to 1
>> +
>> +Example for an OMAP3430 board:
>> +
>> +     gpmc: gpmc@6e000000 {
>> +             compatible = "ti,omap3430-gpmc";
>> +             ti,hwmods = "gpmc";
>> +             reg = <0x6e000000 0x1000000>;
>> +             interrupts = <20>;
>> +             gpmc,num-cs = <8>;
>> +             gpmc,num-waitpins = <4>;
>> +             #address-cells = <2>;
>> +             #size-cells = <1>;
>> +
>> +             onenand@0 {
>> +                     reg = <0 0 0>; /* CS0, offset 0 */
>> +
>> +                     #address-cells = <1>;
>> +                     #size-cells = <1>;
>> +
>> +                     /* partitions go here */
>> +             };
>> +     };
>> diff --git a/arch/arm/mach-omap2/gpmc.c b/arch/arm/mach-omap2/gpmc.c
>> index c6255f7..0636d0a 100644
>> --- a/arch/arm/mach-omap2/gpmc.c
>> +++ b/arch/arm/mach-omap2/gpmc.c
>> @@ -39,6 +39,7 @@
>>  #include "omap_device.h"
>>  #include "gpmc.h"
>>  #include "gpmc-nand.h"
>> +#include "gpmc-onenand.h"
>>
>>  #define      DEVICE_NAME             "omap-gpmc"
>>
>> @@ -1259,6 +1260,43 @@ static int gpmc_probe_nand_child(struct platform_device *pdev,
>>  }
>>  #endif
>>
>> +#ifdef CONFIG_MTD_ONENAND
>> +static int gpmc_probe_onenand_child(struct platform_device *pdev,
>> +                              struct device_node *child)
>> +{
>> +     u32 val;
>> +     struct omap_onenand_platform_data *gpmc_onenand_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;
>> +     }
>
> I don't understand the format of the reg property, but it seems odd that you
> only need to read one cell from it. Are the remaining address cell and size
> cell used anywhere?
>

Okey, I'll give a shot and try to explain this myself:

As you can see by Daniel's first patch [1]
the reg property originally contained the chip select only, and
after some discussion in that same thread, and in this one [2]
It was decided to use a reg property that would also describe
the base address and size of the gpmc sub-device,
and use ranges for the address translation.
This was reflected in Daniel's changelog when he submitted
the v2 patch series [3].

[1] http://www.spinics.net/lists/arm-kernel/msg202169.html
[2] http://web.archiveorange.com/archive/v/vEQ2yFI0tmpQJdigvAog
[3] http://lists.infradead.org/pipermail/linux-arm-kernel/2012-November/129426.html

>> +
>> +     gpmc_onenand_data = devm_kzalloc(&pdev->dev, sizeof(*gpmc_onenand_data),
>> +                                      GFP_KERNEL);
>> +     if (!gpmc_onenand_data)
>> +             return -ENOMEM;
>> +
>> +     gpmc_onenand_data->cs = val;
>> +     gpmc_onenand_data->of_node = child;
>> +     gpmc_onenand_data->dma_channel = -1;
>> +
>> +     if (!of_property_read_u32(child, "dma-channel", &val))
>> +             gpmc_onenand_data->dma_channel = val;
>> +
>> +     gpmc_onenand_init(gpmc_onenand_data);
>> +
>> +     return 0;
>> +}
>
> [...]
>
> Otherwise looks good.
>

Thanks,

-- 
    Ezequiel

Re: [PATCH v3 3/3] arm: omap2: gpmc: add DT bindings for OneNAND

[Ezequiel Garcia]

Hi Mark,

First of all: thanks for reviewing.

On Fri, Jan 25, 2013 at 12:56 PM, Mark Rutland <mark.rutland@arm.com> wrote:
> Hi,
>
> I have a couple more comments after looking though this a bit more thoroughly.
>
> On Fri, Jan 25, 2013 at 12:23:11PM +0000, Ezequiel Garcia wrote:
>> This patch adds device tree bindings for OMAP OneNAND devices.
>> Tested on an OMAP3 3430 IGEPv2 board.
>>
>> Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
>> ---
>> Changes from v2:
>>  * Remove unneeded of_node_put() as reported by Mark Rutland
>>
>> Changes from v1:
>>  * Fix typo in Documentation/devicetree/bindings/mtd/gpmc-onenand.txt
>>
>>  .../devicetree/bindings/mtd/gpmc-onenand.txt       |   43 +++++++++++++++++++
>>  arch/arm/mach-omap2/gpmc.c                         |   45 ++++++++++++++++++++
>>  2 files changed, 88 insertions(+), 0 deletions(-)
>>  create mode 100644 Documentation/devicetree/bindings/mtd/gpmc-onenand.txt
>>
>> diff --git a/Documentation/devicetree/bindings/mtd/gpmc-onenand.txt b/Documentation/devicetree/bindings/mtd/gpmc-onenand.txt
>> new file mode 100644
>> index 0000000..deec9da
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/mtd/gpmc-onenand.txt
>> @@ -0,0 +1,43 @@
>> +Device tree bindings for GPMC connected OneNANDs
>> +
>> +GPMC connected OneNAND (found on OMAP boards) are represented as child nodes of
>> +the GPMC controller with a name of "onenand".
>> +
>> +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
>
> Which tree can I find this in?
>

GPMC binding was posted by Daniel Mack a while ago.
Tony has recently pushed it to his master branch here:
git://git.kernel.org/pub/scm/linux/kernel/git/tmlind/linux-omap.git

>> +
>> +Required properties:
>> +
>> + - reg:                      The CS line the peripheral is connected to
>> +
>> +Optional properties:
>> +
>> + - dma-channel:              DMA Channel index
>> +
>> +For inline partiton table parsing (optional):
>> +
>> + - #address-cells: should be set to 1
>> + - #size-cells: should be set to 1
>> +
>> +Example for an OMAP3430 board:
>> +
>> +     gpmc: gpmc@6e000000 {
>> +             compatible = "ti,omap3430-gpmc";
>> +             ti,hwmods = "gpmc";
>> +             reg = <0x6e000000 0x1000000>;
>> +             interrupts = <20>;
>> +             gpmc,num-cs = <8>;
>> +             gpmc,num-waitpins = <4>;
>> +             #address-cells = <2>;
>> +             #size-cells = <1>;
>> +
>> +             onenand@0 {
>> +                     reg = <0 0 0>; /* CS0, offset 0 */
>> +
>> +                     #address-cells = <1>;
>> +                     #size-cells = <1>;
>> +
>> +                     /* partitions go here */
>> +             };
>> +     };
>> diff --git a/arch/arm/mach-omap2/gpmc.c b/arch/arm/mach-omap2/gpmc.c
>> index c6255f7..0636d0a 100644
>> --- a/arch/arm/mach-omap2/gpmc.c
>> +++ b/arch/arm/mach-omap2/gpmc.c
>> @@ -39,6 +39,7 @@
>>  #include "omap_device.h"
>>  #include "gpmc.h"
>>  #include "gpmc-nand.h"
>> +#include "gpmc-onenand.h"
>>
>>  #define      DEVICE_NAME             "omap-gpmc"
>>
>> @@ -1259,6 +1260,43 @@ static int gpmc_probe_nand_child(struct platform_device *pdev,
>>  }
>>  #endif
>>
>> +#ifdef CONFIG_MTD_ONENAND
>> +static int gpmc_probe_onenand_child(struct platform_device *pdev,
>> +                              struct device_node *child)
>> +{
>> +     u32 val;
>> +     struct omap_onenand_platform_data *gpmc_onenand_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;
>> +     }
>
> I don't understand the format of the reg property, but it seems odd that you
> only need to read one cell from it. Are the remaining address cell and size
> cell used anywhere?
>

Okey, I'll give a shot and try to explain this myself:

As you can see by Daniel's first patch [1]
the reg property originally contained the chip select only, and
after some discussion in that same thread, and in this one [2]
It was decided to use a reg property that would also describe
the base address and size of the gpmc sub-device,
and use ranges for the address translation.
This was reflected in Daniel's changelog when he submitted
the v2 patch series [3].

[1] http://www.spinics.net/lists/arm-kernel/msg202169.html
[2] http://web.archiveorange.com/archive/v/vEQ2yFI0tmpQJdigvAog
[3] http://lists.infradead.org/pipermail/linux-arm-kernel/2012-November/129426.html

>> +
>> +     gpmc_onenand_data = devm_kzalloc(&pdev->dev, sizeof(*gpmc_onenand_data),
>> +                                      GFP_KERNEL);
>> +     if (!gpmc_onenand_data)
>> +             return -ENOMEM;
>> +
>> +     gpmc_onenand_data->cs = val;
>> +     gpmc_onenand_data->of_node = child;
>> +     gpmc_onenand_data->dma_channel = -1;
>> +
>> +     if (!of_property_read_u32(child, "dma-channel", &val))
>> +             gpmc_onenand_data->dma_channel = val;
>> +
>> +     gpmc_onenand_init(gpmc_onenand_data);
>> +
>> +     return 0;
>> +}
>
> [...]
>
> Otherwise looks good.
>

Thanks,

-- 
    Ezequiel

[PATCH v3 3/3] arm: omap2: gpmc: add DT bindings for OneNAND

[Ezequiel Garcia]

Hi Mark,

First of all: thanks for reviewing.

On Fri, Jan 25, 2013 at 12:56 PM, Mark Rutland <mark.rutland@arm.com> wrote:
> Hi,
>
> I have a couple more comments after looking though this a bit more thoroughly.
>
> On Fri, Jan 25, 2013 at 12:23:11PM +0000, Ezequiel Garcia wrote:
>> This patch adds device tree bindings for OMAP OneNAND devices.
>> Tested on an OMAP3 3430 IGEPv2 board.
>>
>> Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
>> ---
>> Changes from v2:
>>  * Remove unneeded of_node_put() as reported by Mark Rutland
>>
>> Changes from v1:
>>  * Fix typo in Documentation/devicetree/bindings/mtd/gpmc-onenand.txt
>>
>>  .../devicetree/bindings/mtd/gpmc-onenand.txt       |   43 +++++++++++++++++++
>>  arch/arm/mach-omap2/gpmc.c                         |   45 ++++++++++++++++++++
>>  2 files changed, 88 insertions(+), 0 deletions(-)
>>  create mode 100644 Documentation/devicetree/bindings/mtd/gpmc-onenand.txt
>>
>> diff --git a/Documentation/devicetree/bindings/mtd/gpmc-onenand.txt b/Documentation/devicetree/bindings/mtd/gpmc-onenand.txt
>> new file mode 100644
>> index 0000000..deec9da
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/mtd/gpmc-onenand.txt
>> @@ -0,0 +1,43 @@
>> +Device tree bindings for GPMC connected OneNANDs
>> +
>> +GPMC connected OneNAND (found on OMAP boards) are represented as child nodes of
>> +the GPMC controller with a name of "onenand".
>> +
>> +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
>
> Which tree can I find this in?
>

GPMC binding was posted by Daniel Mack a while ago.
Tony has recently pushed it to his master branch here:
git://git.kernel.org/pub/scm/linux/kernel/git/tmlind/linux-omap.git

>> +
>> +Required properties:
>> +
>> + - reg:                      The CS line the peripheral is connected to
>> +
>> +Optional properties:
>> +
>> + - dma-channel:              DMA Channel index
>> +
>> +For inline partiton table parsing (optional):
>> +
>> + - #address-cells: should be set to 1
>> + - #size-cells: should be set to 1
>> +
>> +Example for an OMAP3430 board:
>> +
>> +     gpmc: gpmc at 6e000000 {
>> +             compatible = "ti,omap3430-gpmc";
>> +             ti,hwmods = "gpmc";
>> +             reg = <0x6e000000 0x1000000>;
>> +             interrupts = <20>;
>> +             gpmc,num-cs = <8>;
>> +             gpmc,num-waitpins = <4>;
>> +             #address-cells = <2>;
>> +             #size-cells = <1>;
>> +
>> +             onenand at 0 {
>> +                     reg = <0 0 0>; /* CS0, offset 0 */
>> +
>> +                     #address-cells = <1>;
>> +                     #size-cells = <1>;
>> +
>> +                     /* partitions go here */
>> +             };
>> +     };
>> diff --git a/arch/arm/mach-omap2/gpmc.c b/arch/arm/mach-omap2/gpmc.c
>> index c6255f7..0636d0a 100644
>> --- a/arch/arm/mach-omap2/gpmc.c
>> +++ b/arch/arm/mach-omap2/gpmc.c
>> @@ -39,6 +39,7 @@
>>  #include "omap_device.h"
>>  #include "gpmc.h"
>>  #include "gpmc-nand.h"
>> +#include "gpmc-onenand.h"
>>
>>  #define      DEVICE_NAME             "omap-gpmc"
>>
>> @@ -1259,6 +1260,43 @@ static int gpmc_probe_nand_child(struct platform_device *pdev,
>>  }
>>  #endif
>>
>> +#ifdef CONFIG_MTD_ONENAND
>> +static int gpmc_probe_onenand_child(struct platform_device *pdev,
>> +                              struct device_node *child)
>> +{
>> +     u32 val;
>> +     struct omap_onenand_platform_data *gpmc_onenand_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;
>> +     }
>
> I don't understand the format of the reg property, but it seems odd that you
> only need to read one cell from it. Are the remaining address cell and size
> cell used anywhere?
>

Okey, I'll give a shot and try to explain this myself:

As you can see by Daniel's first patch [1]
the reg property originally contained the chip select only, and
after some discussion in that same thread, and in this one [2]
It was decided to use a reg property that would also describe
the base address and size of the gpmc sub-device,
and use ranges for the address translation.
This was reflected in Daniel's changelog when he submitted
the v2 patch series [3].

[1] http://www.spinics.net/lists/arm-kernel/msg202169.html
[2] http://web.archiveorange.com/archive/v/vEQ2yFI0tmpQJdigvAog
[3] http://lists.infradead.org/pipermail/linux-arm-kernel/2012-November/129426.html

>> +
>> +     gpmc_onenand_data = devm_kzalloc(&pdev->dev, sizeof(*gpmc_onenand_data),
>> +                                      GFP_KERNEL);
>> +     if (!gpmc_onenand_data)
>> +             return -ENOMEM;
>> +
>> +     gpmc_onenand_data->cs = val;
>> +     gpmc_onenand_data->of_node = child;
>> +     gpmc_onenand_data->dma_channel = -1;
>> +
>> +     if (!of_property_read_u32(child, "dma-channel", &val))
>> +             gpmc_onenand_data->dma_channel = val;
>> +
>> +     gpmc_onenand_init(gpmc_onenand_data);
>> +
>> +     return 0;
>> +}
>
> [...]
>
> Otherwise looks good.
>

Thanks,

-- 
    Ezequiel

Re: [PATCH v3 3/3] arm: omap2: gpmc: add DT bindings for OneNAND

[Mark Rutland]

On Fri, Jan 25, 2013 at 06:11:28PM +0000, Ezequiel Garcia wrote:
> Hi Mark,
> 
> First of all: thanks for reviewing.
> 
> On Fri, Jan 25, 2013 at 12:56 PM, Mark Rutland <mark.rutland@arm.com> wrote:
> > Hi,
> >
> > I have a couple more comments after looking though this a bit more thoroughly.
> >
> > On Fri, Jan 25, 2013 at 12:23:11PM +0000, Ezequiel Garcia wrote:
> >> This patch adds device tree bindings for OMAP OneNAND devices.
> >> Tested on an OMAP3 3430 IGEPv2 board.
> >>
> >> Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
> >> ---
> >> Changes from v2:
> >>  * Remove unneeded of_node_put() as reported by Mark Rutland
> >>
> >> Changes from v1:
> >>  * Fix typo in Documentation/devicetree/bindings/mtd/gpmc-onenand.txt
> >>
> >>  .../devicetree/bindings/mtd/gpmc-onenand.txt       |   43 +++++++++++++++++++
> >>  arch/arm/mach-omap2/gpmc.c                         |   45 ++++++++++++++++++++
> >>  2 files changed, 88 insertions(+), 0 deletions(-)
> >>  create mode 100644 Documentation/devicetree/bindings/mtd/gpmc-onenand.txt
> >>
> >> diff --git a/Documentation/devicetree/bindings/mtd/gpmc-onenand.txt b/Documentation/devicetree/bindings/mtd/gpmc-onenand.txt
> >> new file mode 100644
> >> index 0000000..deec9da
> >> --- /dev/null
> >> +++ b/Documentation/devicetree/bindings/mtd/gpmc-onenand.txt
> >> @@ -0,0 +1,43 @@
> >> +Device tree bindings for GPMC connected OneNANDs
> >> +
> >> +GPMC connected OneNAND (found on OMAP boards) are represented as child nodes of
> >> +the GPMC controller with a name of "onenand".
> >> +
> >> +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
> >
> > Which tree can I find this in?
> >
> 
> GPMC binding was posted by Daniel Mack a while ago.
> Tony has recently pushed it to his master branch here:
> git://git.kernel.org/pub/scm/linux/kernel/git/tmlind/linux-omap.git

Aha. Thanks, it's far easier to understand with the gpmc doc!

> 
> >> +
> >> +Required properties:
> >> +
> >> + - reg:                      The CS line the peripheral is connected to
> >> +
> >> +Optional properties:
> >> +
> >> + - dma-channel:              DMA Channel index
> >> +
> >> +For inline partiton table parsing (optional):
> >> +
> >> + - #address-cells: should be set to 1
> >> + - #size-cells: should be set to 1
> >> +
> >> +Example for an OMAP3430 board:
> >> +
> >> +     gpmc: gpmc@6e000000 {
> >> +             compatible = "ti,omap3430-gpmc";
> >> +             ti,hwmods = "gpmc";
> >> +             reg = <0x6e000000 0x1000000>;
> >> +             interrupts = <20>;
> >> +             gpmc,num-cs = <8>;
> >> +             gpmc,num-waitpins = <4>;
> >> +             #address-cells = <2>;
> >> +             #size-cells = <1>;
> >> +
> >> +             onenand@0 {
> >> +                     reg = <0 0 0>; /* CS0, offset 0 */
> >> +
> >> +                     #address-cells = <1>;
> >> +                     #size-cells = <1>;
> >> +
> >> +                     /* partitions go here */
> >> +             };
> >> +     };
> >> diff --git a/arch/arm/mach-omap2/gpmc.c b/arch/arm/mach-omap2/gpmc.c
> >> index c6255f7..0636d0a 100644
> >> --- a/arch/arm/mach-omap2/gpmc.c
> >> +++ b/arch/arm/mach-omap2/gpmc.c
> >> @@ -39,6 +39,7 @@
> >>  #include "omap_device.h"
> >>  #include "gpmc.h"
> >>  #include "gpmc-nand.h"
> >> +#include "gpmc-onenand.h"
> >>
> >>  #define      DEVICE_NAME             "omap-gpmc"
> >>
> >> @@ -1259,6 +1260,43 @@ static int gpmc_probe_nand_child(struct platform_device *pdev,
> >>  }
> >>  #endif
> >>
> >> +#ifdef CONFIG_MTD_ONENAND
> >> +static int gpmc_probe_onenand_child(struct platform_device *pdev,
> >> +                              struct device_node *child)
> >> +{
> >> +     u32 val;
> >> +     struct omap_onenand_platform_data *gpmc_onenand_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;
> >> +     }
> >
> > I don't understand the format of the reg property, but it seems odd that you
> > only need to read one cell from it. Are the remaining address cell and size
> > cell used anywhere?
> >
> 
> Okey, I'll give a shot and try to explain this myself:
> 
> As you can see by Daniel's first patch [1]
> the reg property originally contained the chip select only, and
> after some discussion in that same thread, and in this one [2]
> It was decided to use a reg property that would also describe
> the base address and size of the gpmc sub-device,
> and use ranges for the address translation.
> This was reflected in Daniel's changelog when he submitted
> the v2 patch series [3].
> 
> [1] http://www.spinics.net/lists/arm-kernel/msg202169.html
> [2] http://web.archiveorange.com/archive/v/vEQ2yFI0tmpQJdigvAog
> [3] http://lists.infradead.org/pipermail/linux-arm-kernel/2012-November/129426.html

So if I've understood correctly, the first address cell is the CS, and the
second the offset within this (as the comment in the onenand@0 node hints)?

If so, the code now makes sense to me :)

I was having difficulty seeing where the base address of the child got
translated via ranges, but I see in [3] that this doesn't currently happen.
That answers my question about the remaining cells.

Might it be worth sanity checking the runtime calculated address with the one
translated from dt? That way we can print a warning if the two don't match,
which would help to discover broken dts (or invalid runtime calculation if
future hardware breaks it somewhow).

Thanks,
Mark.

Re: [PATCH v3 3/3] arm: omap2: gpmc: add DT bindings for OneNAND

[Mark Rutland]

On Fri, Jan 25, 2013 at 06:11:28PM +0000, Ezequiel Garcia wrote:
> Hi Mark,
> 
> First of all: thanks for reviewing.
> 
> On Fri, Jan 25, 2013 at 12:56 PM, Mark Rutland <mark.rutland@arm.com> wrote:
> > Hi,
> >
> > I have a couple more comments after looking though this a bit more thoroughly.
> >
> > On Fri, Jan 25, 2013 at 12:23:11PM +0000, Ezequiel Garcia wrote:
> >> This patch adds device tree bindings for OMAP OneNAND devices.
> >> Tested on an OMAP3 3430 IGEPv2 board.
> >>
> >> Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
> >> ---
> >> Changes from v2:
> >>  * Remove unneeded of_node_put() as reported by Mark Rutland
> >>
> >> Changes from v1:
> >>  * Fix typo in Documentation/devicetree/bindings/mtd/gpmc-onenand.txt
> >>
> >>  .../devicetree/bindings/mtd/gpmc-onenand.txt       |   43 +++++++++++++++++++
> >>  arch/arm/mach-omap2/gpmc.c                         |   45 ++++++++++++++++++++
> >>  2 files changed, 88 insertions(+), 0 deletions(-)
> >>  create mode 100644 Documentation/devicetree/bindings/mtd/gpmc-onenand.txt
> >>
> >> diff --git a/Documentation/devicetree/bindings/mtd/gpmc-onenand.txt b/Documentation/devicetree/bindings/mtd/gpmc-onenand.txt
> >> new file mode 100644
> >> index 0000000..deec9da
> >> --- /dev/null
> >> +++ b/Documentation/devicetree/bindings/mtd/gpmc-onenand.txt
> >> @@ -0,0 +1,43 @@
> >> +Device tree bindings for GPMC connected OneNANDs
> >> +
> >> +GPMC connected OneNAND (found on OMAP boards) are represented as child nodes of
> >> +the GPMC controller with a name of "onenand".
> >> +
> >> +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
> >
> > Which tree can I find this in?
> >
> 
> GPMC binding was posted by Daniel Mack a while ago.
> Tony has recently pushed it to his master branch here:
> git://git.kernel.org/pub/scm/linux/kernel/git/tmlind/linux-omap.git

Aha. Thanks, it's far easier to understand with the gpmc doc!

> 
> >> +
> >> +Required properties:
> >> +
> >> + - reg:                      The CS line the peripheral is connected to
> >> +
> >> +Optional properties:
> >> +
> >> + - dma-channel:              DMA Channel index
> >> +
> >> +For inline partiton table parsing (optional):
> >> +
> >> + - #address-cells: should be set to 1
> >> + - #size-cells: should be set to 1
> >> +
> >> +Example for an OMAP3430 board:
> >> +
> >> +     gpmc: gpmc@6e000000 {
> >> +             compatible = "ti,omap3430-gpmc";
> >> +             ti,hwmods = "gpmc";
> >> +             reg = <0x6e000000 0x1000000>;
> >> +             interrupts = <20>;
> >> +             gpmc,num-cs = <8>;
> >> +             gpmc,num-waitpins = <4>;
> >> +             #address-cells = <2>;
> >> +             #size-cells = <1>;
> >> +
> >> +             onenand@0 {
> >> +                     reg = <0 0 0>; /* CS0, offset 0 */
> >> +
> >> +                     #address-cells = <1>;
> >> +                     #size-cells = <1>;
> >> +
> >> +                     /* partitions go here */
> >> +             };
> >> +     };
> >> diff --git a/arch/arm/mach-omap2/gpmc.c b/arch/arm/mach-omap2/gpmc.c
> >> index c6255f7..0636d0a 100644
> >> --- a/arch/arm/mach-omap2/gpmc.c
> >> +++ b/arch/arm/mach-omap2/gpmc.c
> >> @@ -39,6 +39,7 @@
> >>  #include "omap_device.h"
> >>  #include "gpmc.h"
> >>  #include "gpmc-nand.h"
> >> +#include "gpmc-onenand.h"
> >>
> >>  #define      DEVICE_NAME             "omap-gpmc"
> >>
> >> @@ -1259,6 +1260,43 @@ static int gpmc_probe_nand_child(struct platform_device *pdev,
> >>  }
> >>  #endif
> >>
> >> +#ifdef CONFIG_MTD_ONENAND
> >> +static int gpmc_probe_onenand_child(struct platform_device *pdev,
> >> +                              struct device_node *child)
> >> +{
> >> +     u32 val;
> >> +     struct omap_onenand_platform_data *gpmc_onenand_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;
> >> +     }
> >
> > I don't understand the format of the reg property, but it seems odd that you
> > only need to read one cell from it. Are the remaining address cell and size
> > cell used anywhere?
> >
> 
> Okey, I'll give a shot and try to explain this myself:
> 
> As you can see by Daniel's first patch [1]
> the reg property originally contained the chip select only, and
> after some discussion in that same thread, and in this one [2]
> It was decided to use a reg property that would also describe
> the base address and size of the gpmc sub-device,
> and use ranges for the address translation.
> This was reflected in Daniel's changelog when he submitted
> the v2 patch series [3].
> 
> [1] http://www.spinics.net/lists/arm-kernel/msg202169.html
> [2] http://web.archiveorange.com/archive/v/vEQ2yFI0tmpQJdigvAog
> [3] http://lists.infradead.org/pipermail/linux-arm-kernel/2012-November/129426.html

So if I've understood correctly, the first address cell is the CS, and the
second the offset within this (as the comment in the onenand@0 node hints)?

If so, the code now makes sense to me :)

I was having difficulty seeing where the base address of the child got
translated via ranges, but I see in [3] that this doesn't currently happen.
That answers my question about the remaining cells.

Might it be worth sanity checking the runtime calculated address with the one
translated from dt? That way we can print a warning if the two don't match,
which would help to discover broken dts (or invalid runtime calculation if
future hardware breaks it somewhow).

Thanks,
Mark.

[PATCH v3 3/3] arm: omap2: gpmc: add DT bindings for OneNAND

[Mark Rutland]

On Fri, Jan 25, 2013 at 06:11:28PM +0000, Ezequiel Garcia wrote:
> Hi Mark,
> 
> First of all: thanks for reviewing.
> 
> On Fri, Jan 25, 2013 at 12:56 PM, Mark Rutland <mark.rutland@arm.com> wrote:
> > Hi,
> >
> > I have a couple more comments after looking though this a bit more thoroughly.
> >
> > On Fri, Jan 25, 2013 at 12:23:11PM +0000, Ezequiel Garcia wrote:
> >> This patch adds device tree bindings for OMAP OneNAND devices.
> >> Tested on an OMAP3 3430 IGEPv2 board.
> >>
> >> Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
> >> ---
> >> Changes from v2:
> >>  * Remove unneeded of_node_put() as reported by Mark Rutland
> >>
> >> Changes from v1:
> >>  * Fix typo in Documentation/devicetree/bindings/mtd/gpmc-onenand.txt
> >>
> >>  .../devicetree/bindings/mtd/gpmc-onenand.txt       |   43 +++++++++++++++++++
> >>  arch/arm/mach-omap2/gpmc.c                         |   45 ++++++++++++++++++++
> >>  2 files changed, 88 insertions(+), 0 deletions(-)
> >>  create mode 100644 Documentation/devicetree/bindings/mtd/gpmc-onenand.txt
> >>
> >> diff --git a/Documentation/devicetree/bindings/mtd/gpmc-onenand.txt b/Documentation/devicetree/bindings/mtd/gpmc-onenand.txt
> >> new file mode 100644
> >> index 0000000..deec9da
> >> --- /dev/null
> >> +++ b/Documentation/devicetree/bindings/mtd/gpmc-onenand.txt
> >> @@ -0,0 +1,43 @@
> >> +Device tree bindings for GPMC connected OneNANDs
> >> +
> >> +GPMC connected OneNAND (found on OMAP boards) are represented as child nodes of
> >> +the GPMC controller with a name of "onenand".
> >> +
> >> +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
> >
> > Which tree can I find this in?
> >
> 
> GPMC binding was posted by Daniel Mack a while ago.
> Tony has recently pushed it to his master branch here:
> git://git.kernel.org/pub/scm/linux/kernel/git/tmlind/linux-omap.git

Aha. Thanks, it's far easier to understand with the gpmc doc!

> 
> >> +
> >> +Required properties:
> >> +
> >> + - reg:                      The CS line the peripheral is connected to
> >> +
> >> +Optional properties:
> >> +
> >> + - dma-channel:              DMA Channel index
> >> +
> >> +For inline partiton table parsing (optional):
> >> +
> >> + - #address-cells: should be set to 1
> >> + - #size-cells: should be set to 1
> >> +
> >> +Example for an OMAP3430 board:
> >> +
> >> +     gpmc: gpmc at 6e000000 {
> >> +             compatible = "ti,omap3430-gpmc";
> >> +             ti,hwmods = "gpmc";
> >> +             reg = <0x6e000000 0x1000000>;
> >> +             interrupts = <20>;
> >> +             gpmc,num-cs = <8>;
> >> +             gpmc,num-waitpins = <4>;
> >> +             #address-cells = <2>;
> >> +             #size-cells = <1>;
> >> +
> >> +             onenand at 0 {
> >> +                     reg = <0 0 0>; /* CS0, offset 0 */
> >> +
> >> +                     #address-cells = <1>;
> >> +                     #size-cells = <1>;
> >> +
> >> +                     /* partitions go here */
> >> +             };
> >> +     };
> >> diff --git a/arch/arm/mach-omap2/gpmc.c b/arch/arm/mach-omap2/gpmc.c
> >> index c6255f7..0636d0a 100644
> >> --- a/arch/arm/mach-omap2/gpmc.c
> >> +++ b/arch/arm/mach-omap2/gpmc.c
> >> @@ -39,6 +39,7 @@
> >>  #include "omap_device.h"
> >>  #include "gpmc.h"
> >>  #include "gpmc-nand.h"
> >> +#include "gpmc-onenand.h"
> >>
> >>  #define      DEVICE_NAME             "omap-gpmc"
> >>
> >> @@ -1259,6 +1260,43 @@ static int gpmc_probe_nand_child(struct platform_device *pdev,
> >>  }
> >>  #endif
> >>
> >> +#ifdef CONFIG_MTD_ONENAND
> >> +static int gpmc_probe_onenand_child(struct platform_device *pdev,
> >> +                              struct device_node *child)
> >> +{
> >> +     u32 val;
> >> +     struct omap_onenand_platform_data *gpmc_onenand_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;
> >> +     }
> >
> > I don't understand the format of the reg property, but it seems odd that you
> > only need to read one cell from it. Are the remaining address cell and size
> > cell used anywhere?
> >
> 
> Okey, I'll give a shot and try to explain this myself:
> 
> As you can see by Daniel's first patch [1]
> the reg property originally contained the chip select only, and
> after some discussion in that same thread, and in this one [2]
> It was decided to use a reg property that would also describe
> the base address and size of the gpmc sub-device,
> and use ranges for the address translation.
> This was reflected in Daniel's changelog when he submitted
> the v2 patch series [3].
> 
> [1] http://www.spinics.net/lists/arm-kernel/msg202169.html
> [2] http://web.archiveorange.com/archive/v/vEQ2yFI0tmpQJdigvAog
> [3] http://lists.infradead.org/pipermail/linux-arm-kernel/2012-November/129426.html

So if I've understood correctly, the first address cell is the CS, and the
second the offset within this (as the comment in the onenand at 0 node hints)?

If so, the code now makes sense to me :)

I was having difficulty seeing where the base address of the child got
translated via ranges, but I see in [3] that this doesn't currently happen.
That answers my question about the remaining cells.

Might it be worth sanity checking the runtime calculated address with the one
translated from dt? That way we can print a warning if the two don't match,
which would help to discover broken dts (or invalid runtime calculation if
future hardware breaks it somewhow).

Thanks,
Mark.

Re: [PATCH v3 3/3] arm: omap2: gpmc: add DT bindings for OneNAND

[Tony Lindgren]

* Mark Rutland <mark.rutland@arm.com> [130128 03:57]:
> 
> So if I've understood correctly, the first address cell is the CS, and the
> second the offset within this (as the comment in the onenand@0 node hints)?
> 
> If so, the code now makes sense to me :)

Sounds like further checking can be added as needed, so applying this
series into omap-for-v3.9/gpmc.

Regards,

Tony

Re: [PATCH v3 3/3] arm: omap2: gpmc: add DT bindings for OneNAND

[Tony Lindgren]

* Mark Rutland <mark.rutland@arm.com> [130128 03:57]:
> 
> So if I've understood correctly, the first address cell is the CS, and the
> second the offset within this (as the comment in the onenand@0 node hints)?
> 
> If so, the code now makes sense to me :)

Sounds like further checking can be added as needed, so applying this
series into omap-for-v3.9/gpmc.

Regards,

Tony

[PATCH v3 3/3] arm: omap2: gpmc: add DT bindings for OneNAND

[Tony Lindgren]

* Mark Rutland <mark.rutland@arm.com> [130128 03:57]:
> 
> So if I've understood correctly, the first address cell is the CS, and the
> second the offset within this (as the comment in the onenand at 0 node hints)?
> 
> If so, the code now makes sense to me :)

Sounds like further checking can be added as needed, so applying this
series into omap-for-v3.9/gpmc.

Regards,

Tony