All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCHv6] mtd: gpio-nand: add device tree bindings
@ 2011-10-11 23:10 ` Jamie Iles
  0 siblings, 0 replies; 14+ messages in thread
From: Jamie Iles @ 2011-10-11 23:10 UTC (permalink / raw)
  To: linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
  Cc: Artem Bityutskiy, devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
	Scott Wood, David Woodhouse

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.

v6:	- convert to mtd_device_parse_register()
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>
---

This is pretty much a repost of v5, but updated to use
mtd_device_parse_register().  This has a dependency on the 64-bit device tree
property reading helper that has now been merged into Grant's tree.

 .../devicetree/bindings/mtd/gpio-control-nand.txt  |   44 +++++++++
 drivers/mtd/nand/gpio.c                            |   94 ++++++++++++++++++--
 2 files changed, 131 insertions(+), 7 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..e12968f 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,10 @@ static int __devinit gpio_nand_probe(struct platform_device *dev)
 	struct gpiomtd *gpiomtd;
 	struct nand_chip *this;
 	struct resource *res0, *res1;
-	int ret;
+	struct mtd_part_parser_data ppdata;
+	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 +320,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 +329,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)
@@ -316,8 +390,13 @@ static int __devinit gpio_nand_probe(struct platform_device *dev)
 		gpiomtd->plat.adjust_parts(&gpiomtd->plat,
 					   gpiomtd->mtd_info.size);
 
-	mtd_device_register(&gpiomtd->mtd_info, gpiomtd->plat.parts,
-			    gpiomtd->plat.num_parts);
+	memset(&ppdata, 0, sizeof(ppdata));
+	ppdata.of_node = dev->dev.of_node;
+	ret = mtd_device_parse_register(&gpiomtd->mtd_info, NULL, &ppdata,
+					gpiomtd->plat.parts,
+					gpiomtd->plat.num_parts);
+	if (ret < 0)
+		goto err_wp;
 	platform_set_drvdata(dev, gpiomtd);
 
 	return 0;
@@ -352,6 +431,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

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

* [PATCHv6] mtd: gpio-nand: add device tree bindings
@ 2011-10-11 23:10 ` Jamie Iles
  0 siblings, 0 replies; 14+ messages in thread
From: Jamie Iles @ 2011-10-11 23:10 UTC (permalink / raw)
  To: linux-mtd
  Cc: Artem Bityutskiy, devicetree-discuss, Grant Likely, Scott Wood,
	Jamie Iles, David Woodhouse

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.

v6:	- convert to mtd_device_parse_register()
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>
---

This is pretty much a repost of v5, but updated to use
mtd_device_parse_register().  This has a dependency on the 64-bit device tree
property reading helper that has now been merged into Grant's tree.

 .../devicetree/bindings/mtd/gpio-control-nand.txt  |   44 +++++++++
 drivers/mtd/nand/gpio.c                            |   94 ++++++++++++++++++--
 2 files changed, 131 insertions(+), 7 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..e12968f 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,10 @@ static int __devinit gpio_nand_probe(struct platform_device *dev)
 	struct gpiomtd *gpiomtd;
 	struct nand_chip *this;
 	struct resource *res0, *res1;
-	int ret;
+	struct mtd_part_parser_data ppdata;
+	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 +320,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 +329,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)
@@ -316,8 +390,13 @@ static int __devinit gpio_nand_probe(struct platform_device *dev)
 		gpiomtd->plat.adjust_parts(&gpiomtd->plat,
 					   gpiomtd->mtd_info.size);
 
-	mtd_device_register(&gpiomtd->mtd_info, gpiomtd->plat.parts,
-			    gpiomtd->plat.num_parts);
+	memset(&ppdata, 0, sizeof(ppdata));
+	ppdata.of_node = dev->dev.of_node;
+	ret = mtd_device_parse_register(&gpiomtd->mtd_info, NULL, &ppdata,
+					gpiomtd->plat.parts,
+					gpiomtd->plat.num_parts);
+	if (ret < 0)
+		goto err_wp;
 	platform_set_drvdata(dev, gpiomtd);
 
 	return 0;
@@ -352,6 +431,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

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

* Re: [PATCHv6] mtd: gpio-nand: add device tree bindings
  2011-10-11 23:10 ` Jamie Iles
@ 2011-10-14 13:48   ` Artem Bityutskiy
  -1 siblings, 0 replies; 14+ messages in thread
From: Artem Bityutskiy @ 2011-10-14 13:48 UTC (permalink / raw)
  To: Jamie Iles
  Cc: Grant Likely, Scott Wood, devicetree-discuss, linux-mtd, David Woodhouse

On Wed, 2011-10-12 at 00:10 +0100, Jamie Iles wrote:
> +#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);
...
> +#else /* CONFIG_OF */
...
> +#endif /* CONFIG_OF */

I wonder, why it is either OF of platform data? What if I want my kernel
to fall-back to platform data if device tree data is absent? What is the
general policy? Sorry, I am not very well aware of the DT stuff. But off
the top of my head, it is logical when things go like this: I have a
kernel with working platform data, but I can change that dynamically by
feeding it a device tree configuration. Hmm?

-- 
Best Regards,
Artem Bityutskiy


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

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

* Re: [PATCHv6] mtd: gpio-nand: add device tree bindings
@ 2011-10-14 13:48   ` Artem Bityutskiy
  0 siblings, 0 replies; 14+ messages in thread
From: Artem Bityutskiy @ 2011-10-14 13:48 UTC (permalink / raw)
  To: Jamie Iles
  Cc: Grant Likely, Scott Wood, devicetree-discuss, linux-mtd, David Woodhouse

On Wed, 2011-10-12 at 00:10 +0100, Jamie Iles wrote:
> +#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);
...
> +#else /* CONFIG_OF */
...
> +#endif /* CONFIG_OF */

I wonder, why it is either OF of platform data? What if I want my kernel
to fall-back to platform data if device tree data is absent? What is the
general policy? Sorry, I am not very well aware of the DT stuff. But off
the top of my head, it is logical when things go like this: I have a
kernel with working platform data, but I can change that dynamically by
feeding it a device tree configuration. Hmm?

-- 
Best Regards,
Artem Bityutskiy

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

* Re: [PATCHv6] mtd: gpio-nand: add device tree bindings
  2011-10-14 13:48   ` Artem Bityutskiy
@ 2011-10-14 14:21     ` Jamie Iles
  -1 siblings, 0 replies; 14+ messages in thread
From: Jamie Iles @ 2011-10-14 14:21 UTC (permalink / raw)
  To: Artem Bityutskiy
  Cc: devicetree-discuss, Grant Likely, linux-mtd, Scott Wood,
	Jamie Iles, David Woodhouse

Hi Artem,

On Fri, Oct 14, 2011 at 04:48:20PM +0300, Artem Bityutskiy wrote:
> On Wed, 2011-10-12 at 00:10 +0100, Jamie Iles wrote:
> > +#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);
> ...
> > +#else /* CONFIG_OF */
> ...
> > +#endif /* CONFIG_OF */
> 
> I wonder, why it is either OF of platform data? What if I want my kernel
> to fall-back to platform data if device tree data is absent? What is the
> general policy? Sorry, I am not very well aware of the DT stuff. But off

I think the general policy is that for device tree everything should be 
in the device tree.  There is a mechanism for device tree platforms to 
pass platform data too, but I believe this is more as a tool for 
migrating existing platforms to device tree.

Also, the device tree binding should be well documented - if the device 
is present in the tree it should have all of the required properties.  
If the device isn't there at all then it won't get registered.

> the top of my head, it is logical when things go like this: I have a
> kernel with working platform data, but I can change that dynamically by
> feeding it a device tree configuration. Hmm?

I think in general platform data and device tree should be mutually 
exclusive.

Jamie

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

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

* Re: [PATCHv6] mtd: gpio-nand: add device tree bindings
@ 2011-10-14 14:21     ` Jamie Iles
  0 siblings, 0 replies; 14+ messages in thread
From: Jamie Iles @ 2011-10-14 14:21 UTC (permalink / raw)
  To: Artem Bityutskiy
  Cc: devicetree-discuss, Grant Likely, linux-mtd, Scott Wood,
	Jamie Iles, David Woodhouse

Hi Artem,

On Fri, Oct 14, 2011 at 04:48:20PM +0300, Artem Bityutskiy wrote:
> On Wed, 2011-10-12 at 00:10 +0100, Jamie Iles wrote:
> > +#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);
> ...
> > +#else /* CONFIG_OF */
> ...
> > +#endif /* CONFIG_OF */
> 
> I wonder, why it is either OF of platform data? What if I want my kernel
> to fall-back to platform data if device tree data is absent? What is the
> general policy? Sorry, I am not very well aware of the DT stuff. But off

I think the general policy is that for device tree everything should be 
in the device tree.  There is a mechanism for device tree platforms to 
pass platform data too, but I believe this is more as a tool for 
migrating existing platforms to device tree.

Also, the device tree binding should be well documented - if the device 
is present in the tree it should have all of the required properties.  
If the device isn't there at all then it won't get registered.

> the top of my head, it is logical when things go like this: I have a
> kernel with working platform data, but I can change that dynamically by
> feeding it a device tree configuration. Hmm?

I think in general platform data and device tree should be mutually 
exclusive.

Jamie

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

* Re: [PATCHv6] mtd: gpio-nand: add device tree bindings
  2011-10-14 14:21     ` Jamie Iles
@ 2011-10-14 14:25       ` Artem Bityutskiy
  -1 siblings, 0 replies; 14+ messages in thread
From: Artem Bityutskiy @ 2011-10-14 14:25 UTC (permalink / raw)
  To: Jamie Iles
  Cc: Scott Wood, devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
	linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, David Woodhouse

On Fri, 2011-10-14 at 15:21 +0100, Jamie Iles wrote:
> > the top of my head, it is logical when things go like this: I have a
> > kernel with working platform data, but I can change that dynamically by
> > feeding it a device tree configuration. Hmm?
> 
> I think in general platform data and device tree should be mutually 
> exclusive.

OK, but would you please confirm this by pointing to some docs or
discussions, or may be people from devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
could confirm that?

-- 
Best Regards,
Artem Bityutskiy

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

* Re: [PATCHv6] mtd: gpio-nand: add device tree bindings
@ 2011-10-14 14:25       ` Artem Bityutskiy
  0 siblings, 0 replies; 14+ messages in thread
From: Artem Bityutskiy @ 2011-10-14 14:25 UTC (permalink / raw)
  To: Jamie Iles
  Cc: Grant Likely, Scott Wood, devicetree-discuss, linux-mtd, David Woodhouse

On Fri, 2011-10-14 at 15:21 +0100, Jamie Iles wrote:
> > the top of my head, it is logical when things go like this: I have a
> > kernel with working platform data, but I can change that dynamically by
> > feeding it a device tree configuration. Hmm?
> 
> I think in general platform data and device tree should be mutually 
> exclusive.

OK, but would you please confirm this by pointing to some docs or
discussions, or may be people from devicetree-discuss@lists.ozlabs.org
could confirm that?

-- 
Best Regards,
Artem Bityutskiy

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

* Re: [PATCHv6] mtd: gpio-nand: add device tree bindings
  2011-10-14 14:25       ` Artem Bityutskiy
@ 2011-10-14 14:43         ` Jamie Iles
  -1 siblings, 0 replies; 14+ messages in thread
From: Jamie Iles @ 2011-10-14 14:43 UTC (permalink / raw)
  To: Artem Bityutskiy
  Cc: devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
	linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Scott Wood,
	David Woodhouse

On Fri, Oct 14, 2011 at 05:25:21PM +0300, Artem Bityutskiy wrote:
> On Fri, 2011-10-14 at 15:21 +0100, Jamie Iles wrote:
> > > the top of my head, it is logical when things go like this: I have a
> > > kernel with working platform data, but I can change that dynamically by
> > > feeding it a device tree configuration. Hmm?
> > 
> > I think in general platform data and device tree should be mutually 
> > exclusive.
> 
> OK, but would you please confirm this by pointing to some docs or
> discussions, or may be people from devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
> could confirm that?

I found https://lwn.net/Articles/449058/ from Grant that I think covers 
it, but that's the best I could find written down!

Jamie

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

* Re: [PATCHv6] mtd: gpio-nand: add device tree bindings
@ 2011-10-14 14:43         ` Jamie Iles
  0 siblings, 0 replies; 14+ messages in thread
From: Jamie Iles @ 2011-10-14 14:43 UTC (permalink / raw)
  To: Artem Bityutskiy
  Cc: devicetree-discuss, Grant Likely, linux-mtd, Scott Wood,
	Jamie Iles, David Woodhouse

On Fri, Oct 14, 2011 at 05:25:21PM +0300, Artem Bityutskiy wrote:
> On Fri, 2011-10-14 at 15:21 +0100, Jamie Iles wrote:
> > > the top of my head, it is logical when things go like this: I have a
> > > kernel with working platform data, but I can change that dynamically by
> > > feeding it a device tree configuration. Hmm?
> > 
> > I think in general platform data and device tree should be mutually 
> > exclusive.
> 
> OK, but would you please confirm this by pointing to some docs or
> discussions, or may be people from devicetree-discuss@lists.ozlabs.org
> could confirm that?

I found https://lwn.net/Articles/449058/ from Grant that I think covers 
it, but that's the best I could find written down!

Jamie

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

* Re: [PATCHv6] mtd: gpio-nand: add device tree bindings
  2011-10-14 14:21     ` Jamie Iles
@ 2011-10-15  3:04       ` Grant Likely
  -1 siblings, 0 replies; 14+ messages in thread
From: Grant Likely @ 2011-10-15  3:04 UTC (permalink / raw)
  To: Jamie Iles
  Cc: Scott Wood, David Woodhouse,
	devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
	linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Artem Bityutskiy

On Fri, Oct 14, 2011 at 03:21:49PM +0100, Jamie Iles wrote:
> Hi Artem,
> 
> On Fri, Oct 14, 2011 at 04:48:20PM +0300, Artem Bityutskiy wrote:
> > On Wed, 2011-10-12 at 00:10 +0100, Jamie Iles wrote:
> > > +#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);
> > ...
> > > +#else /* CONFIG_OF */
> > ...
> > > +#endif /* CONFIG_OF */
> > 
> > I wonder, why it is either OF of platform data? What if I want my kernel
> > to fall-back to platform data if device tree data is absent? What is the
> > general policy? Sorry, I am not very well aware of the DT stuff. But off
> 
> I think the general policy is that for device tree everything should be 
> in the device tree.  There is a mechanism for device tree platforms to 
> pass platform data too, but I believe this is more as a tool for 
> migrating existing platforms to device tree.
> 
> Also, the device tree binding should be well documented - if the device 
> is present in the tree it should have all of the required properties.  
> If the device isn't there at all then it won't get registered.
> 
> > the top of my head, it is logical when things go like this: I have a
> > kernel with working platform data, but I can change that dynamically by
> > feeding it a device tree configuration. Hmm?
> 
> I think in general platform data and device tree should be mutually 
> exclusive.

No, Artem is correct.  It is *not* okay to break platform_data support
simply by turning on CONFIG_OF.  The driver must be able to support
both, and to choose its data source at runtime.

Essentially, turning on CONFIG_OF adds the ability to boot with a
device tree while still being able to boot on legacy platforms.

g.

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

* Re: [PATCHv6] mtd: gpio-nand: add device tree bindings
@ 2011-10-15  3:04       ` Grant Likely
  0 siblings, 0 replies; 14+ messages in thread
From: Grant Likely @ 2011-10-15  3:04 UTC (permalink / raw)
  To: Jamie Iles
  Cc: Scott Wood, David Woodhouse, devicetree-discuss, linux-mtd,
	Artem Bityutskiy

On Fri, Oct 14, 2011 at 03:21:49PM +0100, Jamie Iles wrote:
> Hi Artem,
> 
> On Fri, Oct 14, 2011 at 04:48:20PM +0300, Artem Bityutskiy wrote:
> > On Wed, 2011-10-12 at 00:10 +0100, Jamie Iles wrote:
> > > +#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);
> > ...
> > > +#else /* CONFIG_OF */
> > ...
> > > +#endif /* CONFIG_OF */
> > 
> > I wonder, why it is either OF of platform data? What if I want my kernel
> > to fall-back to platform data if device tree data is absent? What is the
> > general policy? Sorry, I am not very well aware of the DT stuff. But off
> 
> I think the general policy is that for device tree everything should be 
> in the device tree.  There is a mechanism for device tree platforms to 
> pass platform data too, but I believe this is more as a tool for 
> migrating existing platforms to device tree.
> 
> Also, the device tree binding should be well documented - if the device 
> is present in the tree it should have all of the required properties.  
> If the device isn't there at all then it won't get registered.
> 
> > the top of my head, it is logical when things go like this: I have a
> > kernel with working platform data, but I can change that dynamically by
> > feeding it a device tree configuration. Hmm?
> 
> I think in general platform data and device tree should be mutually 
> exclusive.

No, Artem is correct.  It is *not* okay to break platform_data support
simply by turning on CONFIG_OF.  The driver must be able to support
both, and to choose its data source at runtime.

Essentially, turning on CONFIG_OF adds the ability to boot with a
device tree while still being able to boot on legacy platforms.

g.

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

* Re: [PATCHv6] mtd: gpio-nand: add device tree bindings
  2011-10-15  3:04       ` Grant Likely
@ 2011-10-15  5:09           ` Jamie Iles
  -1 siblings, 0 replies; 14+ messages in thread
From: Jamie Iles @ 2011-10-15  5:09 UTC (permalink / raw)
  To: Grant Likely, Artem Bityutskiy
  Cc: Scott Wood, David Woodhouse,
	devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
	linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r

On 15 October 2011 04:04, Grant Likely <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org> wrote:
> On Fri, Oct 14, 2011 at 03:21:49PM +0100, Jamie Iles wrote:
>> Hi Artem,
>>
>> On Fri, Oct 14, 2011 at 04:48:20PM +0300, Artem Bityutskiy wrote:
>> > On Wed, 2011-10-12 at 00:10 +0100, Jamie Iles wrote:
>> > > +#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);
>> > ...
>> > > +#else /* CONFIG_OF */
>> > ...
>> > > +#endif /* CONFIG_OF */
>> >
>> > I wonder, why it is either OF of platform data? What if I want my kernel
>> > to fall-back to platform data if device tree data is absent? What is the
>> > general policy? Sorry, I am not very well aware of the DT stuff. But off
>>
>> I think the general policy is that for device tree everything should be
>> in the device tree.  There is a mechanism for device tree platforms to
>> pass platform data too, but I believe this is more as a tool for
>> migrating existing platforms to device tree.
>>
>> Also, the device tree binding should be well documented - if the device
>> is present in the tree it should have all of the required properties.
>> If the device isn't there at all then it won't get registered.
>>
>> > the top of my head, it is logical when things go like this: I have a
>> > kernel with working platform data, but I can change that dynamically by
>> > feeding it a device tree configuration. Hmm?
>>
>> I think in general platform data and device tree should be mutually
>> exclusive.
>
> No, Artem is correct.  It is *not* okay to break platform_data support
> simply by turning on CONFIG_OF.  The driver must be able to support
> both, and to choose its data source at runtime.
>
> Essentially, turning on CONFIG_OF adds the ability to boot with a
> device tree while still being able to boot on legacy platforms.

Gah!  Double fail.  I completely misunderstood Artem's question and my
own code.  I did have this ability in previous versions but somehow
managed to kill it in v5 against my best intentions.

Artem, Grant, thanks for spotting this and steering me in the right direction!

Jamie

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

* Re: [PATCHv6] mtd: gpio-nand: add device tree bindings
@ 2011-10-15  5:09           ` Jamie Iles
  0 siblings, 0 replies; 14+ messages in thread
From: Jamie Iles @ 2011-10-15  5:09 UTC (permalink / raw)
  To: Grant Likely, Artem Bityutskiy
  Cc: Scott Wood, David Woodhouse, devicetree-discuss, linux-mtd

On 15 October 2011 04:04, Grant Likely <grant.likely@secretlab.ca> wrote:
> On Fri, Oct 14, 2011 at 03:21:49PM +0100, Jamie Iles wrote:
>> Hi Artem,
>>
>> On Fri, Oct 14, 2011 at 04:48:20PM +0300, Artem Bityutskiy wrote:
>> > On Wed, 2011-10-12 at 00:10 +0100, Jamie Iles wrote:
>> > > +#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);
>> > ...
>> > > +#else /* CONFIG_OF */
>> > ...
>> > > +#endif /* CONFIG_OF */
>> >
>> > I wonder, why it is either OF of platform data? What if I want my kernel
>> > to fall-back to platform data if device tree data is absent? What is the
>> > general policy? Sorry, I am not very well aware of the DT stuff. But off
>>
>> I think the general policy is that for device tree everything should be
>> in the device tree.  There is a mechanism for device tree platforms to
>> pass platform data too, but I believe this is more as a tool for
>> migrating existing platforms to device tree.
>>
>> Also, the device tree binding should be well documented - if the device
>> is present in the tree it should have all of the required properties.
>> If the device isn't there at all then it won't get registered.
>>
>> > the top of my head, it is logical when things go like this: I have a
>> > kernel with working platform data, but I can change that dynamically by
>> > feeding it a device tree configuration. Hmm?
>>
>> I think in general platform data and device tree should be mutually
>> exclusive.
>
> No, Artem is correct.  It is *not* okay to break platform_data support
> simply by turning on CONFIG_OF.  The driver must be able to support
> both, and to choose its data source at runtime.
>
> Essentially, turning on CONFIG_OF adds the ability to boot with a
> device tree while still being able to boot on legacy platforms.

Gah!  Double fail.  I completely misunderstood Artem's question and my
own code.  I did have this ability in previous versions but somehow
managed to kill it in v5 against my best intentions.

Artem, Grant, thanks for spotting this and steering me in the right direction!

Jamie

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

end of thread, other threads:[~2011-10-15  5:09 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-10-11 23:10 [PATCHv6] mtd: gpio-nand: add device tree bindings Jamie Iles
2011-10-11 23:10 ` Jamie Iles
2011-10-14 13:48 ` Artem Bityutskiy
2011-10-14 13:48   ` Artem Bityutskiy
2011-10-14 14:21   ` Jamie Iles
2011-10-14 14:21     ` Jamie Iles
2011-10-14 14:25     ` Artem Bityutskiy
2011-10-14 14:25       ` Artem Bityutskiy
2011-10-14 14:43       ` Jamie Iles
2011-10-14 14:43         ` Jamie Iles
2011-10-15  3:04     ` Grant Likely
2011-10-15  3:04       ` Grant Likely
     [not found]       ` <20111015030415.GA8274-e0URQFbLeQY2iJbIjFUEsiwD8/FfD2ys@public.gmane.org>
2011-10-15  5:09         ` Jamie Iles
2011-10-15  5:09           ` Jamie Iles

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.