[PATCHv4] mtd: gpio-nand: add device tree bindings

[PATCHv4] mtd: gpio-nand: add device tree bindings

[Jamie Iles]

Add device tree bindings so that the gpio-nand driver may be
instantiated from the device tree.  This also allows the partitions
to be specified in the device tree.

v4:
	- get io sync address from gpio-control-nand,io-sync-reg
	  property rather than a resource
	- clarified a few details in the binding
v3:
	- remove redundant cast and a couple of whitespace/naming
	  changes
v2:
	- add CONFIG_OF guards for non-dt platforms
	- compatible becomes gpio-control-nand
	- clarify some binding details

Cc: David Woodhouse <dwmw2-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org>
Cc: Artem Bityutskiy <dedekind1-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Cc: Scott Wood <scottwood-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
Cc: Grant Likely <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org>
Signed-off-by: Jamie Iles <jamie-wmLquQDDieKakBO8gow8eQ@public.gmane.org>
---

Grant, I dropped your Acked-by as I thought the change to the io-sync
stuff was significant enough for that.  

 .../devicetree/bindings/mtd/gpio-control-nand.txt  |   44 ++++++
 drivers/mtd/nand/gpio.c                            |  139 ++++++++++++++++++-
 2 files changed, 175 insertions(+), 8 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/mtd/gpio-control-nand.txt

diff --git a/Documentation/devicetree/bindings/mtd/gpio-control-nand.txt b/Documentation/devicetree/bindings/mtd/gpio-control-nand.txt
new file mode 100644
index 0000000..8654294
--- /dev/null
+++ b/Documentation/devicetree/bindings/mtd/gpio-control-nand.txt
@@ -0,0 +1,44 @@
+GPIO assisted NAND flash
+
+The GPIO assisted NAND flash uses a memory mapped interface to
+read/write the NAND commands and data and GPIO pins for the control
+signals.
+
+Required properties:
+- compatible : "gpio-control-nand"
+- reg : should specify localbus chip select and size used for the chip.  The
+  resource describes the data bus connected to the NAND flash and all accesses
+  are made in native endianness.
+- #address-cells, #size-cells : Must be present if the device has sub-nodes
+  representing partitions.
+- gpios : specifies the gpio pins to control the NAND device.  nwp is an
+  optional gpio and may be set to 0 if not present.
+
+Optional properties:
+- bank-width : Width (in bytes) of the device.  If not present, the width
+  defaults to 8 bits.
+- chip-delay : chip dependent delay for transferring data from array to
+  read registers (tR).  If not present then a default of 0 is used.
+- gpio-control-nand,io-sync-reg : A 64-bit physical address for a read
+  location used to guard against bus reordering with regards to accesses to
+  the GPIO's and the NAND flash data bus.  If present, then after changing
+  GPIO state, this register will be read to ensure that the accesses have
+  completed.
+
+Examples:
+
+gpio-nand@1,0 {
+	compatible = "gpio-control-nand";
+	reg = <1 0x0000 0x2>;
+	#address-cells = <1>;
+	#size-cells = <1>;
+	gpios = <&banka 1 0	/* rdy */
+		 &banka 2 0 	/* nce */
+		 &banka 3 0 	/* ale */
+		 &banka 4 0 	/* cle */
+		 0		/* nwp */>;
+
+	partition@0 {
+	...
+	};
+};
diff --git a/drivers/mtd/nand/gpio.c b/drivers/mtd/nand/gpio.c
index 2c2060b..89b819b 100644
--- a/drivers/mtd/nand/gpio.c
+++ b/drivers/mtd/nand/gpio.c
@@ -27,6 +27,9 @@
 #include <linux/mtd/nand.h>
 #include <linux/mtd/partitions.h>
 #include <linux/mtd/nand-gpio.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/of_gpio.h>
 
 struct gpiomtd {
 	void __iomem		*io_sync;
@@ -171,6 +174,99 @@ static int gpio_nand_devready(struct mtd_info *mtd)
 	return gpio_get_value(gpiomtd->plat.gpio_rdy);
 }
 
+#ifdef CONFIG_OF
+static const struct of_device_id gpio_nand_id_table[] = {
+	{ .compatible = "gpio-control-nand" },
+	{}
+};
+MODULE_DEVICE_TABLE(of, gpio_nand_id_table);
+
+static int gpio_nand_of_get_options(struct device *dev,
+				    struct gpio_nand_platdata *plat)
+{
+	u32 width;
+
+	if (!of_property_read_u32(dev->of_node, "bank-width", &width)) {
+		if (width == 2) {
+			plat->options |= NAND_BUSWIDTH_16;
+		} else if (width != 1) {
+			dev_err(dev, "invalid bank-width %u\n", width);
+			return -EINVAL;
+		}
+	}
+
+	return 0;
+}
+
+static void gpio_nand_of_get_gpio(struct device *dev,
+				  struct gpio_nand_platdata *plat)
+{
+	plat->gpio_rdy = of_get_gpio(dev->of_node, 0);
+	plat->gpio_nce = of_get_gpio(dev->of_node, 1);
+	plat->gpio_ale = of_get_gpio(dev->of_node, 2);
+	plat->gpio_cle = of_get_gpio(dev->of_node, 3);
+	plat->gpio_nwp = of_get_gpio(dev->of_node, 4);
+}
+
+static void gpio_nand_of_get_chip_delay(struct device *dev,
+					struct gpio_nand_platdata *plat)
+{
+	u32 chip_delay;
+
+	if (!of_property_read_u32(dev->of_node, "chip-delay", &chip_delay))
+		plat->chip_delay = (int)chip_delay;
+}
+
+static int gpio_nand_of_get_config(struct device *dev,
+				   struct gpio_nand_platdata *plat)
+{
+	int ret = gpio_nand_of_get_options(dev, plat);
+
+	if (ret < 0)
+		return ret;
+
+	gpio_nand_of_get_gpio(dev, plat);
+	gpio_nand_of_get_chip_delay(dev, plat);
+
+	return 0;
+}
+
+static struct resource *gpio_nand_of_get_io_sync(struct device *dev)
+{
+	struct resource *r = kzalloc(sizeof(*r), GFP_KERNEL);
+	u64 addr;
+
+	if (!r)
+		return NULL;
+
+	if (of_property_read_u64(dev->of_node,
+				 "gpio-control-nand,io-sync-reg", &addr))
+		goto out_free;
+
+	r->start = addr;
+	r->end = r->start + 0x3;
+	r->flags = IORESOURCE_MEM;
+
+	return r;
+
+out_free:
+	kfree(r);
+
+	return NULL;
+}
+#else /* CONFIG_OF */
+#define gpio_nand_id_table NULL
+static inline int gpio_nand_of_get_config(struct device *dev,
+					  struct gpio_nand_platdata *plat)
+{
+	return -ENODEV;
+}
+static inline struct resource *gpio_nand_of_get_io_sync(struct device *dev)
+{
+	return NULL;
+}
+#endif /* CONFIG_OF */
+
 static int __devexit gpio_nand_remove(struct platform_device *dev)
 {
 	struct gpiomtd *gpiomtd = platform_get_drvdata(dev);
@@ -178,10 +274,15 @@ static int __devexit gpio_nand_remove(struct platform_device *dev)
 
 	nand_release(&gpiomtd->mtd_info);
 
-	res = platform_get_resource(dev, IORESOURCE_MEM, 1);
+	if (!dev->dev.of_node)
+		res = platform_get_resource(dev, IORESOURCE_MEM, 1);
+	else
+		res = gpio_nand_of_get_io_sync(&dev->dev);
 	iounmap(gpiomtd->io_sync);
 	if (res)
 		release_mem_region(res->start, resource_size(res));
+	if (!dev->dev.of_node)
+		kfree(res);
 
 	res = platform_get_resource(dev, IORESOURCE_MEM, 0);
 	iounmap(gpiomtd->nand_chip.IO_ADDR_R);
@@ -226,9 +327,9 @@ static int __devinit gpio_nand_probe(struct platform_device *dev)
 	struct gpiomtd *gpiomtd;
 	struct nand_chip *this;
 	struct resource *res0, *res1;
-	int ret;
+	int ret = 0;
 
-	if (!dev->dev.platform_data)
+	if (!dev->dev.of_node && !dev->dev.platform_data)
 		return -EINVAL;
 
 	res0 = platform_get_resource(dev, IORESOURCE_MEM, 0);
@@ -248,7 +349,11 @@ static int __devinit gpio_nand_probe(struct platform_device *dev)
 		goto err_map;
 	}
 
-	res1 = platform_get_resource(dev, IORESOURCE_MEM, 1);
+	if (!dev->dev.of_node)
+		res1 = platform_get_resource(dev, IORESOURCE_MEM, 1);
+	else
+		res1 = gpio_nand_of_get_io_sync(&dev->dev);
+
 	if (res1) {
 		gpiomtd->io_sync = request_and_remap(res1, 4, "NAND sync", &ret);
 		if (!gpiomtd->io_sync) {
@@ -257,7 +362,16 @@ static int __devinit gpio_nand_probe(struct platform_device *dev)
 		}
 	}
 
-	memcpy(&gpiomtd->plat, dev->dev.platform_data, sizeof(gpiomtd->plat));
+	if (dev->dev.of_node)
+		kfree(res1);
+
+	if (dev->dev.platform_data)
+		memcpy(&gpiomtd->plat, dev->dev.platform_data,
+		       sizeof(gpiomtd->plat));
+	else
+		ret = gpio_nand_of_get_config(&dev->dev, &gpiomtd->plat);
+	if (ret)
+		goto err_nce;
 
 	ret = gpio_request(gpiomtd->plat.gpio_nce, "NAND NCE");
 	if (ret)
@@ -312,10 +426,18 @@ static int __devinit gpio_nand_probe(struct platform_device *dev)
 		goto err_wp;
 	}
 
-	if (gpiomtd->plat.adjust_parts)
-		gpiomtd->plat.adjust_parts(&gpiomtd->plat,
-					   gpiomtd->mtd_info.size);
+	if (dev->dev.platform_data) {
+		if (gpiomtd->plat.adjust_parts)
+			gpiomtd->plat.adjust_parts(&gpiomtd->plat,
+						   gpiomtd->mtd_info.size);
+	} else {
+		ret = of_mtd_parse_partitions(&dev->dev, dev->dev.of_node,
+					      &gpiomtd->plat.parts);
+		if (ret < 0)
+			goto err_wp;
 
+		gpiomtd->plat.num_parts = ret;
+	}
 	mtd_device_register(&gpiomtd->mtd_info, gpiomtd->plat.parts,
 			    gpiomtd->plat.num_parts);
 	platform_set_drvdata(dev, gpiomtd);
@@ -352,6 +474,7 @@ static struct platform_driver gpio_nand_driver = {
 	.remove		= gpio_nand_remove,
 	.driver		= {
 		.name	= "gpio-nand",
+		.of_match_table = gpio_nand_id_table,
 	},
 };
 
-- 
1.7.4.1

Re: [PATCHv4] mtd: gpio-nand: add device tree bindings

[Scott Wood]

On 08/09/2011 10:12 AM, Jamie Iles wrote:
> +Optional properties:
> +- bank-width : Width (in bytes) of the device.  If not present, the width
> +  defaults to 8 bits.

"in bytes" versus "defaults to 8 bits"...

> +- chip-delay : chip dependent delay for transferring data from array to
> +  read registers (tR).  If not present then a default of 0 is used.

nand_set_defaults() will set this to 20 us if you pass in zero.

> +- gpio-control-nand,io-sync-reg : A 64-bit physical address for a read
> +  location used to guard against bus reordering with regards to accesses to
> +  the GPIO's and the NAND flash data bus.  If present, then after changing
> +  GPIO state, this register will be read to ensure that the accesses have
> +  completed.

The driver does it before and after all cmd_ctrl byte writes, in
addition to after changing the GPIO state.

-Scott

Re: [PATCHv4] mtd: gpio-nand: add device tree bindings

[Jamie Iles]

On Wed, Aug 10, 2011 at 10:13:27AM -0500, Scott Wood wrote:
> On 08/09/2011 10:12 AM, Jamie Iles wrote:
> > +Optional properties:
> > +- bank-width : Width (in bytes) of the device.  If not present, the width
> > +  defaults to 8 bits.
> 
> "in bytes" versus "defaults to 8 bits"...
> 
> > +- chip-delay : chip dependent delay for transferring data from array to
> > +  read registers (tR).  If not present then a default of 0 is used.
> 
> nand_set_defaults() will set this to 20 us if you pass in zero.
> 
> > +- gpio-control-nand,io-sync-reg : A 64-bit physical address for a read
> > +  location used to guard against bus reordering with regards to accesses to
> > +  the GPIO's and the NAND flash data bus.  If present, then after changing
> > +  GPIO state, this register will be read to ensure that the accesses have
> > +  completed.
> 
> The driver does it before and after all cmd_ctrl byte writes, in
> addition to after changing the GPIO state.

Hmm, I haven't done a great job documenting this binding!  Revised patch 
below.

Thanks Scott!

8<----

From: Jamie Iles <jamie-wmLquQDDieKakBO8gow8eQ@public.gmane.org>
Subject: [PATCH] mtd: gpio-nand: add device tree bindings

Add device tree bindings so that the gpio-nand driver may be
instantiated from the device tree.  This also allows the partitions
to be specified in the device tree.

v4:
	- get io sync address from gpio-control-nand,io-sync-reg
	  property rather than a resource
	- clarified a few details in the binding
v3:
	- remove redundant cast and a couple of whitespace/naming
	  changes
v2:
	- add CONFIG_OF guards for non-dt platforms
	- compatible becomes gpio-control-nand
	- clarify some binding details

Cc: David Woodhouse <dwmw2-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org>
Cc: Artem Bityutskiy <dedekind1-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Cc: Scott Wood <scottwood-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
Cc: Grant Likely <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org>
Signed-off-by: Jamie Iles <jamie-wmLquQDDieKakBO8gow8eQ@public.gmane.org>
---
 .../devicetree/bindings/mtd/gpio-control-nand.txt  |   44 ++++++
 drivers/mtd/nand/gpio.c                            |  139 ++++++++++++++++++-
 2 files changed, 175 insertions(+), 8 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/mtd/gpio-control-nand.txt

diff --git a/Documentation/devicetree/bindings/mtd/gpio-control-nand.txt b/Documentation/devicetree/bindings/mtd/gpio-control-nand.txt
new file mode 100644
index 0000000..719f4dc
--- /dev/null
+++ b/Documentation/devicetree/bindings/mtd/gpio-control-nand.txt
@@ -0,0 +1,44 @@
+GPIO assisted NAND flash
+
+The GPIO assisted NAND flash uses a memory mapped interface to
+read/write the NAND commands and data and GPIO pins for the control
+signals.
+
+Required properties:
+- compatible : "gpio-control-nand"
+- reg : should specify localbus chip select and size used for the chip.  The
+  resource describes the data bus connected to the NAND flash and all accesses
+  are made in native endianness.
+- #address-cells, #size-cells : Must be present if the device has sub-nodes
+  representing partitions.
+- gpios : specifies the gpio pins to control the NAND device.  nwp is an
+  optional gpio and may be set to 0 if not present.
+
+Optional properties:
+- bank-width : Width (in bytes) of the device.  If not present, the width
+  defaults to 1 byte.
+- chip-delay : chip dependent delay for transferring data from array to
+  read registers (tR).  If not present then a default of 20us is used.
+- gpio-control-nand,io-sync-reg : A 64-bit physical address for a read
+  location used to guard against bus reordering with regards to accesses to
+  the GPIO's and the NAND flash data bus.  If present, then after changing
+  GPIO state and before and after command byte writes, this register will be
+  read to ensure that the GPIO accesses have completed.
+
+Examples:
+
+gpio-nand@1,0 {
+	compatible = "gpio-control-nand";
+	reg = <1 0x0000 0x2>;
+	#address-cells = <1>;
+	#size-cells = <1>;
+	gpios = <&banka 1 0	/* rdy */
+		 &banka 2 0 	/* nce */
+		 &banka 3 0 	/* ale */
+		 &banka 4 0 	/* cle */
+		 0		/* nwp */>;
+
+	partition@0 {
+	...
+	};
+};
diff --git a/drivers/mtd/nand/gpio.c b/drivers/mtd/nand/gpio.c
index 2c2060b..89b819b 100644
--- a/drivers/mtd/nand/gpio.c
+++ b/drivers/mtd/nand/gpio.c
@@ -27,6 +27,9 @@
 #include <linux/mtd/nand.h>
 #include <linux/mtd/partitions.h>
 #include <linux/mtd/nand-gpio.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/of_gpio.h>
 
 struct gpiomtd {
 	void __iomem		*io_sync;
@@ -171,6 +174,99 @@ static int gpio_nand_devready(struct mtd_info *mtd)
 	return gpio_get_value(gpiomtd->plat.gpio_rdy);
 }
 
+#ifdef CONFIG_OF
+static const struct of_device_id gpio_nand_id_table[] = {
+	{ .compatible = "gpio-control-nand" },
+	{}
+};
+MODULE_DEVICE_TABLE(of, gpio_nand_id_table);
+
+static int gpio_nand_of_get_options(struct device *dev,
+				    struct gpio_nand_platdata *plat)
+{
+	u32 width;
+
+	if (!of_property_read_u32(dev->of_node, "bank-width", &width)) {
+		if (width == 2) {
+			plat->options |= NAND_BUSWIDTH_16;
+		} else if (width != 1) {
+			dev_err(dev, "invalid bank-width %u\n", width);
+			return -EINVAL;
+		}
+	}
+
+	return 0;
+}
+
+static void gpio_nand_of_get_gpio(struct device *dev,
+				  struct gpio_nand_platdata *plat)
+{
+	plat->gpio_rdy = of_get_gpio(dev->of_node, 0);
+	plat->gpio_nce = of_get_gpio(dev->of_node, 1);
+	plat->gpio_ale = of_get_gpio(dev->of_node, 2);
+	plat->gpio_cle = of_get_gpio(dev->of_node, 3);
+	plat->gpio_nwp = of_get_gpio(dev->of_node, 4);
+}
+
+static void gpio_nand_of_get_chip_delay(struct device *dev,
+					struct gpio_nand_platdata *plat)
+{
+	u32 chip_delay;
+
+	if (!of_property_read_u32(dev->of_node, "chip-delay", &chip_delay))
+		plat->chip_delay = (int)chip_delay;
+}
+
+static int gpio_nand_of_get_config(struct device *dev,
+				   struct gpio_nand_platdata *plat)
+{
+	int ret = gpio_nand_of_get_options(dev, plat);
+
+	if (ret < 0)
+		return ret;
+
+	gpio_nand_of_get_gpio(dev, plat);
+	gpio_nand_of_get_chip_delay(dev, plat);
+
+	return 0;
+}
+
+static struct resource *gpio_nand_of_get_io_sync(struct device *dev)
+{
+	struct resource *r = kzalloc(sizeof(*r), GFP_KERNEL);
+	u64 addr;
+
+	if (!r)
+		return NULL;
+
+	if (of_property_read_u64(dev->of_node,
+				 "gpio-control-nand,io-sync-reg", &addr))
+		goto out_free;
+
+	r->start = addr;
+	r->end = r->start + 0x3;
+	r->flags = IORESOURCE_MEM;
+
+	return r;
+
+out_free:
+	kfree(r);
+
+	return NULL;
+}
+#else /* CONFIG_OF */
+#define gpio_nand_id_table NULL
+static inline int gpio_nand_of_get_config(struct device *dev,
+					  struct gpio_nand_platdata *plat)
+{
+	return -ENODEV;
+}
+static inline struct resource *gpio_nand_of_get_io_sync(struct device *dev)
+{
+	return NULL;
+}
+#endif /* CONFIG_OF */
+
 static int __devexit gpio_nand_remove(struct platform_device *dev)
 {
 	struct gpiomtd *gpiomtd = platform_get_drvdata(dev);
@@ -178,10 +274,15 @@ static int __devexit gpio_nand_remove(struct platform_device *dev)
 
 	nand_release(&gpiomtd->mtd_info);
 
-	res = platform_get_resource(dev, IORESOURCE_MEM, 1);
+	if (!dev->dev.of_node)
+		res = platform_get_resource(dev, IORESOURCE_MEM, 1);
+	else
+		res = gpio_nand_of_get_io_sync(&dev->dev);
 	iounmap(gpiomtd->io_sync);
 	if (res)
 		release_mem_region(res->start, resource_size(res));
+	if (!dev->dev.of_node)
+		kfree(res);
 
 	res = platform_get_resource(dev, IORESOURCE_MEM, 0);
 	iounmap(gpiomtd->nand_chip.IO_ADDR_R);
@@ -226,9 +327,9 @@ static int __devinit gpio_nand_probe(struct platform_device *dev)
 	struct gpiomtd *gpiomtd;
 	struct nand_chip *this;
 	struct resource *res0, *res1;
-	int ret;
+	int ret = 0;
 
-	if (!dev->dev.platform_data)
+	if (!dev->dev.of_node && !dev->dev.platform_data)
 		return -EINVAL;
 
 	res0 = platform_get_resource(dev, IORESOURCE_MEM, 0);
@@ -248,7 +349,11 @@ static int __devinit gpio_nand_probe(struct platform_device *dev)
 		goto err_map;
 	}
 
-	res1 = platform_get_resource(dev, IORESOURCE_MEM, 1);
+	if (!dev->dev.of_node)
+		res1 = platform_get_resource(dev, IORESOURCE_MEM, 1);
+	else
+		res1 = gpio_nand_of_get_io_sync(&dev->dev);
+
 	if (res1) {
 		gpiomtd->io_sync = request_and_remap(res1, 4, "NAND sync", &ret);
 		if (!gpiomtd->io_sync) {
@@ -257,7 +362,16 @@ static int __devinit gpio_nand_probe(struct platform_device *dev)
 		}
 	}
 
-	memcpy(&gpiomtd->plat, dev->dev.platform_data, sizeof(gpiomtd->plat));
+	if (dev->dev.of_node)
+		kfree(res1);
+
+	if (dev->dev.platform_data)
+		memcpy(&gpiomtd->plat, dev->dev.platform_data,
+		       sizeof(gpiomtd->plat));
+	else
+		ret = gpio_nand_of_get_config(&dev->dev, &gpiomtd->plat);
+	if (ret)
+		goto err_nce;
 
 	ret = gpio_request(gpiomtd->plat.gpio_nce, "NAND NCE");
 	if (ret)
@@ -312,10 +426,18 @@ static int __devinit gpio_nand_probe(struct platform_device *dev)
 		goto err_wp;
 	}
 
-	if (gpiomtd->plat.adjust_parts)
-		gpiomtd->plat.adjust_parts(&gpiomtd->plat,
-					   gpiomtd->mtd_info.size);
+	if (dev->dev.platform_data) {
+		if (gpiomtd->plat.adjust_parts)
+			gpiomtd->plat.adjust_parts(&gpiomtd->plat,
+						   gpiomtd->mtd_info.size);
+	} else {
+		ret = of_mtd_parse_partitions(&dev->dev, dev->dev.of_node,
+					      &gpiomtd->plat.parts);
+		if (ret < 0)
+			goto err_wp;
 
+		gpiomtd->plat.num_parts = ret;
+	}
 	mtd_device_register(&gpiomtd->mtd_info, gpiomtd->plat.parts,
 			    gpiomtd->plat.num_parts);
 	platform_set_drvdata(dev, gpiomtd);
@@ -352,6 +474,7 @@ static struct platform_driver gpio_nand_driver = {
 	.remove		= gpio_nand_remove,
 	.driver		= {
 		.name	= "gpio-nand",
+		.of_match_table = gpio_nand_id_table,
 	},
 };
 
-- 
1.7.4.1

Re: [PATCHv4] mtd: gpio-nand: add device tree bindings

[Artem Bityutskiy]

On Tue, 2011-08-09 at 16:12 +0100, Jamie Iles wrote:
> +static int gpio_nand_of_get_options(struct device *dev,
> +				    struct gpio_nand_platdata *plat)
> +{
> +	u32 width;
> +
> +	if (!of_property_read_u32(dev->of_node, "bank-width", &width)) {
> +		if (width == 2) {
> +			plat->options |= NAND_BUSWIDTH_16;
> +		} else if (width != 1) {
> +			dev_err(dev, "invalid bank-width %u\n", width);
> +			return -EINVAL;
> +		}
> +	}
> +
> +	return 0;
> +}
> +
> +static void gpio_nand_of_get_gpio(struct device *dev,
> +				  struct gpio_nand_platdata *plat)
> +{
> +	plat->gpio_rdy = of_get_gpio(dev->of_node, 0);
> +	plat->gpio_nce = of_get_gpio(dev->of_node, 1);
> +	plat->gpio_ale = of_get_gpio(dev->of_node, 2);
> +	plat->gpio_cle = of_get_gpio(dev->of_node, 3);
> +	plat->gpio_nwp = of_get_gpio(dev->of_node, 4);
> +}
> +
> +static void gpio_nand_of_get_chip_delay(struct device *dev,
> +					struct gpio_nand_platdata *plat)
> +{
> +	u32 chip_delay;
> +
> +	if (!of_property_read_u32(dev->of_node, "chip-delay", &chip_delay))
> +		plat->chip_delay = (int)chip_delay;
> +}
> +
> +static int gpio_nand_of_get_config(struct device *dev,
> +				   struct gpio_nand_platdata *plat)
> +{
> +	int ret = gpio_nand_of_get_options(dev, plat);
> +
> +	if (ret < 0)
> +		return ret;
> +
> +	gpio_nand_of_get_gpio(dev, plat);
> +	gpio_nand_of_get_chip_delay(dev, plat);
> +
> +	return 0;
> +}

Do we really need to have 3 additional helper functions for
'gpio_nand_of_get_config()' ?

-- 
Best Regards,
Artem Bityutskiy

Re: [PATCHv4] mtd: gpio-nand: add device tree bindings

[Jamie Iles]

On Mon, Aug 15, 2011 at 04:57:56PM +0300, Artem Bityutskiy wrote:
> On Tue, 2011-08-09 at 16:12 +0100, Jamie Iles wrote:
> > +static int gpio_nand_of_get_options(struct device *dev,
> > +				    struct gpio_nand_platdata *plat)
> > +{
> > +	u32 width;
> > +
> > +	if (!of_property_read_u32(dev->of_node, "bank-width", &width)) {
> > +		if (width == 2) {
> > +			plat->options |= NAND_BUSWIDTH_16;
> > +		} else if (width != 1) {
> > +			dev_err(dev, "invalid bank-width %u\n", width);
> > +			return -EINVAL;
> > +		}
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> > +static void gpio_nand_of_get_gpio(struct device *dev,
> > +				  struct gpio_nand_platdata *plat)
> > +{
> > +	plat->gpio_rdy = of_get_gpio(dev->of_node, 0);
> > +	plat->gpio_nce = of_get_gpio(dev->of_node, 1);
> > +	plat->gpio_ale = of_get_gpio(dev->of_node, 2);
> > +	plat->gpio_cle = of_get_gpio(dev->of_node, 3);
> > +	plat->gpio_nwp = of_get_gpio(dev->of_node, 4);
> > +}
> > +
> > +static void gpio_nand_of_get_chip_delay(struct device *dev,
> > +					struct gpio_nand_platdata *plat)
> > +{
> > +	u32 chip_delay;
> > +
> > +	if (!of_property_read_u32(dev->of_node, "chip-delay", &chip_delay))
> > +		plat->chip_delay = (int)chip_delay;
> > +}
> > +
> > +static int gpio_nand_of_get_config(struct device *dev,
> > +				   struct gpio_nand_platdata *plat)
> > +{
> > +	int ret = gpio_nand_of_get_options(dev, plat);
> > +
> > +	if (ret < 0)
> > +		return ret;
> > +
> > +	gpio_nand_of_get_gpio(dev, plat);
> > +	gpio_nand_of_get_chip_delay(dev, plat);
> > +
> > +	return 0;
> > +}
> 
> Do we really need to have 3 additional helper functions for
> 'gpio_nand_of_get_config()' ?

Not really, I'm happy to fold those all into one if you prefer.

Jamie

Re: [PATCHv4] mtd: gpio-nand: add device tree bindings

[Artem Bityutskiy]

On Tue, 2011-08-09 at 16:12 +0100, Jamie Iles wrote:
> -	res1 = platform_get_resource(dev, IORESOURCE_MEM, 1);
> +	if (!dev->dev.of_node)
> +		res1 = platform_get_resource(dev, IORESOURCE_MEM, 1);
> +	else
> +		res1 = gpio_nand_of_get_io_sync(&dev->dev);
> +
>  	if (res1) {
>  		gpiomtd->io_sync = request_and_remap(res1, 4, "NAND sync", &ret);
>  		if (!gpiomtd->io_sync) {
> @@ -257,7 +362,16 @@ static int __devinit gpio_nand_probe(struct platform_device *dev)
>  		}
>  	}
>  
> -	memcpy(&gpiomtd->plat, dev->dev.platform_data, sizeof(gpiomtd->plat));
> +	if (dev->dev.of_node)
> +		kfree(res1);
> +
> +	if (dev->dev.platform_data)
> +		memcpy(&gpiomtd->plat, dev->dev.platform_data,
> +		       sizeof(gpiomtd->plat));
> +	else
> +		ret = gpio_nand_of_get_config(&dev->dev, &gpiomtd->plat);
> +	if (ret)
> +		goto err_nce;
>  

So with this code you can mix platform data and DT? Say, io_sync may
come from platform data and the rest from the DT? Is this normal
practice?

-- 
Best Regards,
Artem Bityutskiy

Re: [PATCHv4] mtd: gpio-nand: add device tree bindings

[Jamie Iles]

On Mon, Aug 15, 2011 at 05:28:57PM +0300, Artem Bityutskiy wrote:
> On Tue, 2011-08-09 at 16:12 +0100, Jamie Iles wrote:
> > -	res1 = platform_get_resource(dev, IORESOURCE_MEM, 1);
> > +	if (!dev->dev.of_node)
> > +		res1 = platform_get_resource(dev, IORESOURCE_MEM, 1);
> > +	else
> > +		res1 = gpio_nand_of_get_io_sync(&dev->dev);
> > +
> >  	if (res1) {
> >  		gpiomtd->io_sync = request_and_remap(res1, 4, "NAND sync", &ret);
> >  		if (!gpiomtd->io_sync) {
> > @@ -257,7 +362,16 @@ static int __devinit gpio_nand_probe(struct platform_device *dev)
> >  		}
> >  	}
> >  
> > -	memcpy(&gpiomtd->plat, dev->dev.platform_data, sizeof(gpiomtd->plat));
> > +	if (dev->dev.of_node)
> > +		kfree(res1);
> > +
> > +	if (dev->dev.platform_data)
> > +		memcpy(&gpiomtd->plat, dev->dev.platform_data,
> > +		       sizeof(gpiomtd->plat));
> > +	else
> > +		ret = gpio_nand_of_get_config(&dev->dev, &gpiomtd->plat);
> > +	if (ret)
> > +		goto err_nce;
> >  
> 
> So with this code you can mix platform data and DT? Say, io_sync may
> come from platform data and the rest from the DT? Is this normal
> practice?

Well you can use platform_data with DT - it's the only way to pass 
function pointers for example, but I'm not convinced it's required in 
this case (there is the adjust_parts callback, but I can't see a user of 
it) so I'd be inclined to change the conditionals to:

	if (!dev->dev.of_node)
		memcpy(&gpiomtd->plat, dev->dev.platform_data, ...);
	else
		gpio_nand_of_get_config(...);

so that we don't use platform_data for the DT case.

Jamie

Re: [PATCHv4] mtd: gpio-nand: add device tree bindings

[Artem Bityutskiy]

On Mon, 2011-08-15 at 15:38 +0100, Jamie Iles wrote:
> On Mon, Aug 15, 2011 at 05:28:57PM +0300, Artem Bityutskiy wrote:
> > On Tue, 2011-08-09 at 16:12 +0100, Jamie Iles wrote:
> > > -	res1 = platform_get_resource(dev, IORESOURCE_MEM, 1);
> > > +	if (!dev->dev.of_node)
> > > +		res1 = platform_get_resource(dev, IORESOURCE_MEM, 1);
> > > +	else
> > > +		res1 = gpio_nand_of_get_io_sync(&dev->dev);
> > > +
> > >  	if (res1) {
> > >  		gpiomtd->io_sync = request_and_remap(res1, 4, "NAND sync", &ret);
> > >  		if (!gpiomtd->io_sync) {
> > > @@ -257,7 +362,16 @@ static int __devinit gpio_nand_probe(struct platform_device *dev)
> > >  		}
> > >  	}
> > >  
> > > -	memcpy(&gpiomtd->plat, dev->dev.platform_data, sizeof(gpiomtd->plat));
> > > +	if (dev->dev.of_node)
> > > +		kfree(res1);
> > > +
> > > +	if (dev->dev.platform_data)
> > > +		memcpy(&gpiomtd->plat, dev->dev.platform_data,
> > > +		       sizeof(gpiomtd->plat));
> > > +	else
> > > +		ret = gpio_nand_of_get_config(&dev->dev, &gpiomtd->plat);
> > > +	if (ret)
> > > +		goto err_nce;
> > >  
> > 
> > So with this code you can mix platform data and DT? Say, io_sync may
> > come from platform data and the rest from the DT? Is this normal
> > practice?
> 
> Well you can use platform_data with DT - it's the only way to pass 
> function pointers for example, but I'm not convinced it's required in 
> this case (there is the adjust_parts callback, but I can't see a user of 
> it) so I'd be inclined to change the conditionals to:
> 
> 	if (!dev->dev.of_node)
> 		memcpy(&gpiomtd->plat, dev->dev.platform_data, ...);
> 	else
> 		gpio_nand_of_get_config(...);
> 
> so that we don't use platform_data for the DT case.

Yeah, would be nice to have only one "read platform data" and only one
"read DT information" call - this is just cleaner design.

-- 
Best Regards,
Artem Bityutskiy


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCHv4] mtd: gpio-nand: add device tree bindings

[Jamie Iles]

On Mon, Aug 15, 2011 at 05:45:31PM +0300, Artem Bityutskiy wrote:
> On Mon, 2011-08-15 at 15:38 +0100, Jamie Iles wrote:
> > On Mon, Aug 15, 2011 at 05:28:57PM +0300, Artem Bityutskiy wrote:
> > > On Tue, 2011-08-09 at 16:12 +0100, Jamie Iles wrote:
> > > > -	res1 = platform_get_resource(dev, IORESOURCE_MEM, 1);
> > > > +	if (!dev->dev.of_node)
> > > > +		res1 = platform_get_resource(dev, IORESOURCE_MEM, 1);
> > > > +	else
> > > > +		res1 = gpio_nand_of_get_io_sync(&dev->dev);
> > > > +
> > > >  	if (res1) {
> > > >  		gpiomtd->io_sync = request_and_remap(res1, 4, "NAND sync", &ret);
> > > >  		if (!gpiomtd->io_sync) {
> > > > @@ -257,7 +362,16 @@ static int __devinit gpio_nand_probe(struct platform_device *dev)
> > > >  		}
> > > >  	}
> > > >  
> > > > -	memcpy(&gpiomtd->plat, dev->dev.platform_data, sizeof(gpiomtd->plat));
> > > > +	if (dev->dev.of_node)
> > > > +		kfree(res1);
> > > > +
> > > > +	if (dev->dev.platform_data)
> > > > +		memcpy(&gpiomtd->plat, dev->dev.platform_data,
> > > > +		       sizeof(gpiomtd->plat));
> > > > +	else
> > > > +		ret = gpio_nand_of_get_config(&dev->dev, &gpiomtd->plat);
> > > > +	if (ret)
> > > > +		goto err_nce;
> > > >  
> > > 
> > > So with this code you can mix platform data and DT? Say, io_sync may
> > > come from platform data and the rest from the DT? Is this normal
> > > practice?
> > 
> > Well you can use platform_data with DT - it's the only way to pass 
> > function pointers for example, but I'm not convinced it's required in 
> > this case (there is the adjust_parts callback, but I can't see a user of 
> > it) so I'd be inclined to change the conditionals to:
> > 
> > 	if (!dev->dev.of_node)
> > 		memcpy(&gpiomtd->plat, dev->dev.platform_data, ...);
> > 	else
> > 		gpio_nand_of_get_config(...);
> > 
> > so that we don't use platform_data for the DT case.
> 
> Yeah, would be nice to have only one "read platform data" and only one
> "read DT information" call - this is just cleaner design.

OK, how about this?  I think it looks a lot neater.

Jamie

8<--

From: Jamie Iles <jamie-wmLquQDDieKakBO8gow8eQ@public.gmane.org>
Subject: [PATCH] mtd: gpio-nand: add device tree bindings

Add device tree bindings so that the gpio-nand driver may be
instantiated from the device tree.  This also allows the partitions
to be specified in the device tree.

v5:	- fold dt config helpers into a single gpio_nand_of_get_config()
v4:	- get io sync address from gpio-control-nand,io-sync-reg
	  property rather than a resource
	- clarified a few details in the binding
v3:	- remove redundant cast and a couple of whitespace/naming
	  changes
v2:	- add CONFIG_OF guards for non-dt platforms
	- compatible becomes gpio-control-nand
	- clarify some binding details

Cc: David Woodhouse <dwmw2-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org>
Cc: Artem Bityutskiy <dedekind1-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Cc: Scott Wood <scottwood-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
Cc: Grant Likely <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org>
Signed-off-by: Jamie Iles <jamie-wmLquQDDieKakBO8gow8eQ@public.gmane.org>
---
 .../devicetree/bindings/mtd/gpio-control-nand.txt  |   44 +++++++++
 drivers/mtd/nand/gpio.c                            |   93 ++++++++++++++++++--
 2 files changed, 131 insertions(+), 6 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/mtd/gpio-control-nand.txt

diff --git a/Documentation/devicetree/bindings/mtd/gpio-control-nand.txt b/Documentation/devicetree/bindings/mtd/gpio-control-nand.txt
new file mode 100644
index 0000000..719f4dc
--- /dev/null
+++ b/Documentation/devicetree/bindings/mtd/gpio-control-nand.txt
@@ -0,0 +1,44 @@
+GPIO assisted NAND flash
+
+The GPIO assisted NAND flash uses a memory mapped interface to
+read/write the NAND commands and data and GPIO pins for the control
+signals.
+
+Required properties:
+- compatible : "gpio-control-nand"
+- reg : should specify localbus chip select and size used for the chip.  The
+  resource describes the data bus connected to the NAND flash and all accesses
+  are made in native endianness.
+- #address-cells, #size-cells : Must be present if the device has sub-nodes
+  representing partitions.
+- gpios : specifies the gpio pins to control the NAND device.  nwp is an
+  optional gpio and may be set to 0 if not present.
+
+Optional properties:
+- bank-width : Width (in bytes) of the device.  If not present, the width
+  defaults to 1 byte.
+- chip-delay : chip dependent delay for transferring data from array to
+  read registers (tR).  If not present then a default of 20us is used.
+- gpio-control-nand,io-sync-reg : A 64-bit physical address for a read
+  location used to guard against bus reordering with regards to accesses to
+  the GPIO's and the NAND flash data bus.  If present, then after changing
+  GPIO state and before and after command byte writes, this register will be
+  read to ensure that the GPIO accesses have completed.
+
+Examples:
+
+gpio-nand@1,0 {
+	compatible = "gpio-control-nand";
+	reg = <1 0x0000 0x2>;
+	#address-cells = <1>;
+	#size-cells = <1>;
+	gpios = <&banka 1 0	/* rdy */
+		 &banka 2 0 	/* nce */
+		 &banka 3 0 	/* ale */
+		 &banka 4 0 	/* cle */
+		 0		/* nwp */>;
+
+	partition@0 {
+	...
+	};
+};
diff --git a/drivers/mtd/nand/gpio.c b/drivers/mtd/nand/gpio.c
index 2c2060b..961f4eb 100644
--- a/drivers/mtd/nand/gpio.c
+++ b/drivers/mtd/nand/gpio.c
@@ -27,6 +27,9 @@
 #include <linux/mtd/nand.h>
 #include <linux/mtd/partitions.h>
 #include <linux/mtd/nand-gpio.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/of_gpio.h>
 
 struct gpiomtd {
 	void __iomem		*io_sync;
@@ -171,6 +174,74 @@ static int gpio_nand_devready(struct mtd_info *mtd)
 	return gpio_get_value(gpiomtd->plat.gpio_rdy);
 }
 
+#ifdef CONFIG_OF
+static const struct of_device_id gpio_nand_id_table[] = {
+	{ .compatible = "gpio-control-nand" },
+	{}
+};
+MODULE_DEVICE_TABLE(of, gpio_nand_id_table);
+
+static int gpio_nand_get_config(const struct device *dev,
+				struct gpio_nand_platdata *plat)
+{
+	u32 val;
+
+	if (!of_property_read_u32(dev->of_node, "bank-width", &val)) {
+		if (val == 2) {
+			plat->options |= NAND_BUSWIDTH_16;
+		} else if (val != 1) {
+			dev_err(dev, "invalid bank-width %u\n", val);
+			return -EINVAL;
+		}
+	}
+
+	plat->gpio_rdy = of_get_gpio(dev->of_node, 0);
+	plat->gpio_nce = of_get_gpio(dev->of_node, 1);
+	plat->gpio_ale = of_get_gpio(dev->of_node, 2);
+	plat->gpio_cle = of_get_gpio(dev->of_node, 3);
+	plat->gpio_nwp = of_get_gpio(dev->of_node, 4);
+
+	if (!of_property_read_u32(dev->of_node, "chip-delay", &val))
+		plat->chip_delay = val;
+
+	return 0;
+}
+
+static struct resource *gpio_nand_get_io_sync(struct platform_device *pdev)
+{
+	struct resource *r = devm_kzalloc(&pdev->dev, sizeof(*r), GFP_KERNEL);
+	u64 addr;
+
+	if (!r || of_property_read_u64(pdev->dev.of_node,
+				       "gpio-control-nand,io-sync-reg", &addr))
+		return NULL;
+
+	r->start = addr;
+	r->end = r->start + 0x3;
+	r->flags = IORESOURCE_MEM;
+
+	return r;
+}
+#else /* CONFIG_OF */
+
+#define gpio_nand_id_table NULL
+
+static inline int gpio_nand_get_config(const struct device *dev,
+					  struct gpio_nand_platdata *plat)
+{
+	if (dev->platform_data)
+		memcpy(plat, dev->platform_data, sizeof(*plat));
+
+	return 0;
+}
+
+static inline struct resource *
+gpio_nand_get_io_sync(struct platform_device *pdev)
+{
+	return platform_get_resource(pdev, IORESOURCE_MEM, 1);
+}
+#endif /* CONFIG_OF */
+
 static int __devexit gpio_nand_remove(struct platform_device *dev)
 {
 	struct gpiomtd *gpiomtd = platform_get_drvdata(dev);
@@ -178,7 +249,7 @@ static int __devexit gpio_nand_remove(struct platform_device *dev)
 
 	nand_release(&gpiomtd->mtd_info);
 
-	res = platform_get_resource(dev, IORESOURCE_MEM, 1);
+	res = gpio_nand_get_io_sync(dev);
 	iounmap(gpiomtd->io_sync);
 	if (res)
 		release_mem_region(res->start, resource_size(res));
@@ -226,9 +297,9 @@ static int __devinit gpio_nand_probe(struct platform_device *dev)
 	struct gpiomtd *gpiomtd;
 	struct nand_chip *this;
 	struct resource *res0, *res1;
-	int ret;
+	int ret = 0;
 
-	if (!dev->dev.platform_data)
+	if (!dev->dev.of_node && !dev->dev.platform_data)
 		return -EINVAL;
 
 	res0 = platform_get_resource(dev, IORESOURCE_MEM, 0);
@@ -248,7 +319,7 @@ static int __devinit gpio_nand_probe(struct platform_device *dev)
 		goto err_map;
 	}
 
-	res1 = platform_get_resource(dev, IORESOURCE_MEM, 1);
+	res1 = gpio_nand_get_io_sync(dev);
 	if (res1) {
 		gpiomtd->io_sync = request_and_remap(res1, 4, "NAND sync", &ret);
 		if (!gpiomtd->io_sync) {
@@ -257,7 +328,9 @@ static int __devinit gpio_nand_probe(struct platform_device *dev)
 		}
 	}
 
-	memcpy(&gpiomtd->plat, dev->dev.platform_data, sizeof(gpiomtd->plat));
+	ret = gpio_nand_get_config(&dev->dev, &gpiomtd->plat);
+	if (ret)
+		goto err_nce;
 
 	ret = gpio_request(gpiomtd->plat.gpio_nce, "NAND NCE");
 	if (ret)
@@ -312,10 +385,17 @@ static int __devinit gpio_nand_probe(struct platform_device *dev)
 		goto err_wp;
 	}
 
-	if (gpiomtd->plat.adjust_parts)
+	if (gpiomtd->plat.adjust_parts) {
 		gpiomtd->plat.adjust_parts(&gpiomtd->plat,
 					   gpiomtd->mtd_info.size);
+	} else {
+		ret = of_mtd_parse_partitions(&dev->dev, dev->dev.of_node,
+					      &gpiomtd->plat.parts);
+		if (ret < 0)
+			goto err_wp;
 
+		gpiomtd->plat.num_parts = ret;
+	}
 	mtd_device_register(&gpiomtd->mtd_info, gpiomtd->plat.parts,
 			    gpiomtd->plat.num_parts);
 	platform_set_drvdata(dev, gpiomtd);
@@ -352,6 +432,7 @@ static struct platform_driver gpio_nand_driver = {
 	.remove		= gpio_nand_remove,
 	.driver		= {
 		.name	= "gpio-nand",
+		.of_match_table = gpio_nand_id_table,
 	},
 };
 
-- 
1.7.4.1

Re: [PATCHv4] mtd: gpio-nand: add device tree bindings

[Artem Bityutskiy]

On Mon, 2011-08-15 at 16:24 +0100, Jamie Iles wrote:
> @@ -178,7 +249,7 @@ static int __devexit gpio_nand_remove(struct platform_device *dev)
>  
>  	nand_release(&gpiomtd->mtd_info);
>  
> -	res = platform_get_resource(dev, IORESOURCE_MEM, 1);
> +	res = gpio_nand_get_io_sync(dev);

Why do you call 'gpio_nand_get_io_sync(dev)' here, in
'gpio_nand_remove()' function? You should have it in gpiomtd->io_sync.
Right?

If this is the case, then you do not need a separate
'gpio_nand_get_io_sync()' function at all, you can make
'gpio_nand_get_config()' to fetch the io_sync information from the DT.
And then you will have one single function which gets data from DT, not
2 -> simpler code.

Do I miss something?

-- 
Best Regards,
Artem Bityutskiy (Битюцкий Артём)

_______________________________________________
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss

Re: [PATCHv4] mtd: gpio-nand: add device tree bindings

[Jamie Iles]

On Fri, Aug 19, 2011 at 10:51:46PM +0300, Artem Bityutskiy wrote:
> On Mon, 2011-08-15 at 16:24 +0100, Jamie Iles wrote:
> > @@ -178,7 +249,7 @@ static int __devexit gpio_nand_remove(struct platform_device *dev)
> >  
> >  	nand_release(&gpiomtd->mtd_info);
> >  
> > -	res = platform_get_resource(dev, IORESOURCE_MEM, 1);
> > +	res = gpio_nand_get_io_sync(dev);
> 
> Why do you call 'gpio_nand_get_io_sync(dev)' here, in
> 'gpio_nand_remove()' function? You should have it in gpiomtd->io_sync.
> Right?
>
> If this is the case, then you do not need a separate
> 'gpio_nand_get_io_sync()' function at all, you can make
> 'gpio_nand_get_config()' to fetch the io_sync information from the DT.
> And then you will have one single function which gets data from DT, not
> 2 -> simpler code.
> 
> Do I miss something?

gpiomtd->io_sync is a void __iomem *, but we need a struct resource here 
so that we can do the release_mem_region().  I could store the struct 
resource pointer in gpiomtd rather than calling gpio_nand_get_io_sync() 
twice though, I'm happy to change if you prefer.

Note that for the device tree case, the iosync register isn't in the reg 
property so we can't do platform_get_resource() to get it.  We do this 
because the io_sync address isn't actually a gpio nand resource and 
can't always be expressed as such in the device tree.

Jamie

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCHv4] mtd: gpio-nand: add device tree bindings

[Artem Bityutskiy]

On Mon, 2011-08-15 at 16:24 +0100, Jamie Iles wrote:
> From: Jamie Iles <jamie@jamieiles.com>
> Subject: [PATCH] mtd: gpio-nand: add device tree bindings
> 
> Add device tree bindings so that the gpio-nand driver may be
> instantiated from the device tree.  This also allows the partitions
> to be specified in the device tree.
> 
> v5:	- fold dt config helpers into a single gpio_nand_of_get_config()
> v4:	- get io sync address from gpio-control-nand,io-sync-reg
> 	  property rather than a resource
> 	- clarified a few details in the binding
> v3:	- remove redundant cast and a couple of whitespace/naming
> 	  changes
> v2:	- add CONFIG_OF guards for non-dt platforms
> 	- compatible becomes gpio-control-nand
> 	- clarify some binding details
> 
> Cc: David Woodhouse <dwmw2@infradead.org>
> Cc: Artem Bityutskiy <dedekind1@gmail.com>
> Cc: Scott Wood <scottwood@freescale.com>
> Cc: Grant Likely <grant.likely@secretlab.ca>
> Signed-off-by: Jamie Iles <jamie@jamieiles.com>

Pushed this patch to l2-mtd-2.6.git, thanks.

P.S. probably it should contain some reviewed-by tags? I can always add
them, though, if needed.

-- 
Best Regards,
Artem Bityutskiy (Битюцкий Артём)

_______________________________________________
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss

Re: [PATCHv4] mtd: gpio-nand: add device tree bindings

[Jamie Iles]

Hi Artem,

On Sat, Aug 20, 2011 at 06:39:10AM +0300, Artem Bityutskiy wrote:
> On Mon, 2011-08-15 at 16:24 +0100, Jamie Iles wrote:
> > From: Jamie Iles <jamie-wmLquQDDieKakBO8gow8eQ@public.gmane.org>
> > Subject: [PATCH] mtd: gpio-nand: add device tree bindings
> > 
> > Add device tree bindings so that the gpio-nand driver may be
> > instantiated from the device tree.  This also allows the partitions
> > to be specified in the device tree.
> > 
> > v5:	- fold dt config helpers into a single gpio_nand_of_get_config()
> > v4:	- get io sync address from gpio-control-nand,io-sync-reg
> > 	  property rather than a resource
> > 	- clarified a few details in the binding
> > v3:	- remove redundant cast and a couple of whitespace/naming
> > 	  changes
> > v2:	- add CONFIG_OF guards for non-dt platforms
> > 	- compatible becomes gpio-control-nand
> > 	- clarify some binding details
> > 
> > Cc: David Woodhouse <dwmw2-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org>
> > Cc: Artem Bityutskiy <dedekind1-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> > Cc: Scott Wood <scottwood-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
> > Cc: Grant Likely <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org>
> > Signed-off-by: Jamie Iles <jamie-wmLquQDDieKakBO8gow8eQ@public.gmane.org>
> 
> Pushed this patch to l2-mtd-2.6.git, thanks.
> 
> P.S. probably it should contain some reviewed-by tags? I can always add
> them, though, if needed.

This patch needs of_read_property_u64() from 
http://www.mail-archive.com/devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org/msg05973.html
which hasn't been merged yet for the CONFIG_OF case so I'm not sure how this 
should be handled.

Thanks,

Jamie

Re: [PATCHv4] mtd: gpio-nand: add device tree bindings

[Artem Bityutskiy]

On Sat, 2011-08-20 at 07:38 +0100, Jamie Iles wrote:
> Hi Artem,
> 
> On Sat, Aug 20, 2011 at 06:39:10AM +0300, Artem Bityutskiy wrote:
> > On Mon, 2011-08-15 at 16:24 +0100, Jamie Iles wrote:
> > > From: Jamie Iles <jamie-wmLquQDDieKakBO8gow8eQ@public.gmane.org>
> > > Subject: [PATCH] mtd: gpio-nand: add device tree bindings
> > > 
> > > Add device tree bindings so that the gpio-nand driver may be
> > > instantiated from the device tree.  This also allows the partitions
> > > to be specified in the device tree.
> > > 
> > > v5:	- fold dt config helpers into a single gpio_nand_of_get_config()
> > > v4:	- get io sync address from gpio-control-nand,io-sync-reg
> > > 	  property rather than a resource
> > > 	- clarified a few details in the binding
> > > v3:	- remove redundant cast and a couple of whitespace/naming
> > > 	  changes
> > > v2:	- add CONFIG_OF guards for non-dt platforms
> > > 	- compatible becomes gpio-control-nand
> > > 	- clarify some binding details
> > > 
> > > Cc: David Woodhouse <dwmw2-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org>
> > > Cc: Artem Bityutskiy <dedekind1-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> > > Cc: Scott Wood <scottwood-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
> > > Cc: Grant Likely <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org>
> > > Signed-off-by: Jamie Iles <jamie-wmLquQDDieKakBO8gow8eQ@public.gmane.org>
> > 
> > Pushed this patch to l2-mtd-2.6.git, thanks.
> > 
> > P.S. probably it should contain some reviewed-by tags? I can always add
> > them, though, if needed.
> 
> This patch needs of_read_property_u64() from 
> http://www.mail-archive.com/devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org/msg05973.html
> which hasn't been merged yet for the CONFIG_OF case so I'm not sure how this 
> should be handled.

Oh, I did not realize that. Well, let's wait when it is include. I'll
drop your patch for now then.