[PATCH V2 0/2] mtd/maps/mtd-ram: Make driver usable for the device tree

[PATCH V2 0/2] mtd/maps/mtd-ram: Make driver usable for the device tree

[Wolfram Sang]

Hello,

this is the new series to adapt the mtd-ram-driver for use with the device
tree. All comments have been addressed and it has been tested on a
phyCORE-MPC5200B-IO (with latest linus-git).

Albrecht: As you used the patches recently, maybe you want to ack them?

Looking forward to comments, especially on the binding. Thanks!

Regards,

   Wolfram

[PATCH V2 1/2] mtd/maps/mtd-ram: refactor probe and remove

[Wolfram Sang]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 7285 bytes --]

Refactor the probe and remove routines of the mtd-ram driver to export a
generic part which can later be accessed by an of-counterpart of this
driver. Tested with a phyCORE-i.MX27 (ARM, non-of) and a
phyCORE-MPC5200B-IO (PPC, of).

Signed-off-by: Wolfram Sang <w.sang@pengutronix.de>
Cc: Albrecht Dreß <albrecht.dress@arcor.de>
Cc: Grant Likely <grant.likely@secretlab.ca>
Cc: Ben Dooks <ben-linux@fluff.org>
Cc: David Woodhouse <dwmw2@infradead.org>
Cc: linuxppc-dev@ozlabs.org
Cc: devicetree-discuss@ozlabs.org
Cc: linux-mtd@lists.infradead.org
---
changes in V2:

also converted mtd->dev->parent which got inserted meanwhile

 drivers/mtd/maps/plat-ram.c  |  125 +++++++++++++++++++++---------------------
 include/linux/mtd/plat-ram.h |    4 +
 2 files changed, 67 insertions(+), 62 deletions(-)

diff --git a/drivers/mtd/maps/plat-ram.c b/drivers/mtd/maps/plat-ram.c
index 49c9ece..35731cc 100644
--- a/drivers/mtd/maps/plat-ram.c
+++ b/drivers/mtd/maps/plat-ram.c
@@ -50,16 +50,6 @@ struct platram_info {
 	struct platdata_mtd_ram	*pdata;
 };
 
-/* to_platram_info()
- *
- * device private data to struct platram_info conversion
-*/
-
-static inline struct platram_info *to_platram_info(struct platform_device *dev)
-{
-	return (struct platram_info *)platform_get_drvdata(dev);
-}
-
 /* platram_setrw
  *
  * call the platform device's set rw/ro control
@@ -77,18 +67,14 @@ static inline void platram_setrw(struct platram_info *info, int to)
 		(info->pdata->set_rw)(info->dev, to);
 }
 
-/* platram_remove
- *
- * called to remove the device from the driver's control
-*/
-
-static int platram_remove(struct platform_device *pdev)
+int __platram_remove(struct device *dev)
 {
-	struct platram_info *info = to_platram_info(pdev);
+	struct platram_info *info;
 
-	platform_set_drvdata(pdev, NULL);
+	dev_dbg(dev, "removing device\n");
 
-	dev_dbg(&pdev->dev, "removing device\n");
+	info = dev_get_drvdata(dev);
+	dev_set_drvdata(dev, NULL);
 
 	if (info == NULL)
 		return 0;
@@ -123,68 +109,49 @@ static int platram_remove(struct platform_device *pdev)
 
 	return 0;
 }
+EXPORT_SYMBOL_GPL(__platram_remove);
 
-/* platram_probe
+/* platram_remove
  *
- * called from device drive system when a device matching our
- * driver is found.
+ * called to remove the device from the driver's control
 */
 
-static int platram_probe(struct platform_device *pdev)
+static int platram_remove(struct platform_device *pdev)
+{
+	return __platram_remove(&pdev->dev);
+}
+
+int __platram_probe(struct device *dev, const char *name,
+			struct resource *res, struct platdata_mtd_ram *pdata)
 {
-	struct platdata_mtd_ram	*pdata;
 	struct platram_info *info;
-	struct resource *res;
 	int err = 0;
 
-	dev_dbg(&pdev->dev, "probe entered\n");
-
-	if (pdev->dev.platform_data == NULL) {
-		dev_err(&pdev->dev, "no platform data supplied\n");
-		err = -ENOENT;
-		goto exit_error;
-	}
-
-	pdata = pdev->dev.platform_data;
-
 	info = kzalloc(sizeof(*info), GFP_KERNEL);
 	if (info == NULL) {
-		dev_err(&pdev->dev, "no memory for flash info\n");
+		dev_err(dev, "no memory for flash info\n");
 		err = -ENOMEM;
 		goto exit_error;
 	}
 
-	platform_set_drvdata(pdev, info);
+	dev_set_drvdata(dev, info);
 
-	info->dev = &pdev->dev;
+	info->dev = dev;
 	info->pdata = pdata;
 
-	/* get the resource for the memory mapping */
-
-	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-
-	if (res == NULL) {
-		dev_err(&pdev->dev, "no memory resource specified\n");
-		err = -ENOENT;
-		goto exit_free;
-	}
-
-	dev_dbg(&pdev->dev, "got platform resource %p (0x%llx)\n", res,
-		(unsigned long long)res->start);
-
 	/* setup map parameters */
 
 	info->map.phys = res->start;
 	info->map.size = (res->end - res->start) + 1;
 	info->map.name = pdata->mapname != NULL ?
-			(char *)pdata->mapname : (char *)pdev->name;
+			(char *)pdata->mapname : (char *)name;
 	info->map.bankwidth = pdata->bankwidth;
 
 	/* register our usage of the memory area */
 
-	info->area = request_mem_region(res->start, info->map.size, pdev->name);
+	info->area = request_mem_region(res->start, info->map.size, name);
 	if (info->area == NULL) {
-		dev_err(&pdev->dev, "failed to request memory region\n");
+		dev_err(dev, "failed to request memory region\n");
 		err = -EIO;
 		goto exit_free;
 	}
@@ -192,17 +159,17 @@ static int platram_probe(struct platform_device *pdev)
 	/* remap the memory area */
 
 	info->map.virt = ioremap(res->start, info->map.size);
-	dev_dbg(&pdev->dev, "virt %p, %lu bytes\n", info->map.virt, info->map.size);
+	dev_dbg(dev, "virt %p, %lu bytes\n", info->map.virt, info->map.size);
 
 	if (info->map.virt == NULL) {
-		dev_err(&pdev->dev, "failed to ioremap() region\n");
+		dev_err(dev, "failed to ioremap() region\n");
 		err = -EIO;
 		goto exit_free;
 	}
 
 	simple_map_init(&info->map);
 
-	dev_dbg(&pdev->dev, "initialised map, probing for mtd\n");
+	dev_dbg(dev, "initialised map, probing for mtd\n");
 
 	/* probe for the right mtd map driver
 	 * supplied by the platform_data struct */
@@ -218,13 +185,13 @@ static int platram_probe(struct platform_device *pdev)
 		info->mtd = do_map_probe("map_ram", &info->map);
 
 	if (info->mtd == NULL) {
-		dev_err(&pdev->dev, "failed to probe for map_ram\n");
+		dev_err(dev, "failed to probe for map_ram\n");
 		err = -ENOMEM;
 		goto exit_free;
 	}
 
 	info->mtd->owner = THIS_MODULE;
-	info->mtd->dev.parent = &pdev->dev;
+	info->mtd->dev.parent = dev;
 
 	platram_setrw(info, PLATRAM_RW);
 
@@ -250,20 +217,54 @@ static int platram_probe(struct platform_device *pdev)
 #endif /* CONFIG_MTD_PARTITIONS */
 
 	if (add_mtd_device(info->mtd)) {
-		dev_err(&pdev->dev, "add_mtd_device() failed\n");
+		dev_err(dev, "add_mtd_device() failed\n");
 		err = -ENOMEM;
 	}
 
 	if (!err)
-		dev_info(&pdev->dev, "registered mtd device\n");
+		dev_info(dev, "registered mtd device\n");
 
 	return err;
 
  exit_free:
-	platram_remove(pdev);
+	__platram_remove(dev);
  exit_error:
 	return err;
 }
+EXPORT_SYMBOL_GPL(__platram_probe);
+
+/* platram_probe
+ *
+ * called from device drive system when a device matching our
+ * driver is found.
+*/
+
+static int platram_probe(struct platform_device *pdev)
+{
+	struct platdata_mtd_ram	*pdata;
+	struct resource *res;
+
+	dev_dbg(&pdev->dev, "probe entered\n");
+
+	pdata = pdev->dev.platform_data;
+	if (pdata == NULL) {
+		dev_err(&pdev->dev, "no platform data supplied\n");
+		return -ENOENT;
+	}
+
+	/* get the resource for the memory mapping */
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (res == NULL) {
+		dev_err(&pdev->dev, "no memory resource specified\n");
+		return -ENOENT;
+	}
+
+	dev_dbg(&pdev->dev, "got platform resource %p (0x%llx)\n", res,
+		(unsigned long long)res->start);
+
+	return __platram_probe(&pdev->dev, pdev->name, res, pdata);
+}
 
 /* device driver info */
 
diff --git a/include/linux/mtd/plat-ram.h b/include/linux/mtd/plat-ram.h
index e07890a..c522562 100644
--- a/include/linux/mtd/plat-ram.h
+++ b/include/linux/mtd/plat-ram.h
@@ -31,4 +31,8 @@ struct platdata_mtd_ram {
 	void	(*set_rw)(struct device *dev, int to);
 };
 
+extern int __platram_probe(struct device *dev, const char *name,
+			struct resource *res, struct platdata_mtd_ram *pdata);
+extern int __platram_remove(struct device *dev);
+
 #endif /* __LINUX_MTD_PLATRAM_H */
-- 
1.6.2

[PATCH V2 2/2] mtd/maps/mtd-ram: add an of-platform driver

[Wolfram Sang]

Create an of-aware driver using the now exported generic functions from
plat-ram.c. Also add the documentation for the binding. Partitions are
not yet supported. Tested on a phyCORE-MPC5200B-IO.

Signed-off-by: Wolfram Sang <w.sang@pengutronix.de>
Cc: Albrecht Dreß <albrecht.dress@arcor.de>
Cc: Grant Likely <grant.likely@secretlab.ca>
Cc: Ben Dooks <ben-linux@fluff.org>
Cc: David Woodhouse <dwmw2@infradead.org>
Cc: linuxppc-dev@ozlabs.org
Cc: devicetree-discuss@ozlabs.org
Cc: linux-mtd@lists.infradead.org
---
Changes in V2:

- added binding documentation
- added __devinit & friends
- put struct resource on stack during probe
- simplified getting the name of the of-device
- made error path more readable & add check for valid bank-width-pointer
- rename driver to "of-mtd-ram"

 Documentation/powerpc/dts-bindings/mtd-ram.txt |   18 ++++
 drivers/mtd/maps/Kconfig                       |    7 ++
 drivers/mtd/maps/Makefile                      |    1 +
 drivers/mtd/maps/of-ram.c                      |  102 ++++++++++++++++++++++++
 4 files changed, 128 insertions(+), 0 deletions(-)
 create mode 100644 Documentation/powerpc/dts-bindings/mtd-ram.txt
 create mode 100644 drivers/mtd/maps/of-ram.c

diff --git a/Documentation/powerpc/dts-bindings/mtd-ram.txt b/Documentation/powerpc/dts-bindings/mtd-ram.txt
new file mode 100644
index 0000000..a2e9932
--- /dev/null
+++ b/Documentation/powerpc/dts-bindings/mtd-ram.txt
@@ -0,0 +1,18 @@
+RAM emulating an MTD
+
+An external RAM like SRAM or FRAM can also be treated as an MTD.
+
+ - compatible : should contain the specific model of the RAM chip
+   used, if known, followed by "mtd-ram".
+ - reg : Address range of the RAM chip
+ - bank-width : Width (in bytes) of the RAM bank.
+
+Partitions are not yet supported.
+
+Example:
+
+	sram@2,0 {
+		compatible = "samsung,k6f1616u6a", "mtd-ram";
+		reg = <2 0 0x00200000>;
+		bank-width = <2>;
+	};
diff --git a/drivers/mtd/maps/Kconfig b/drivers/mtd/maps/Kconfig
index 82923bd..bdd9ebc 100644
--- a/drivers/mtd/maps/Kconfig
+++ b/drivers/mtd/maps/Kconfig
@@ -551,6 +551,13 @@ config MTD_PLATRAM
 
 	  This selection automatically selects the map_ram driver.
 
+config MTD_OFRAM
+	tristate "Map driver for of-device RAM (of-mtd-ram)"
+	depends on MTD_PLATRAM && OF
+	help
+	  Map driver for RAM areas described via the of-device
+	  system.
+
 config MTD_VMU
 	tristate "Map driver for Dreamcast VMU"
 	depends on MAPLE
diff --git a/drivers/mtd/maps/Makefile b/drivers/mtd/maps/Makefile
index 2dbc1be..a7a2db3 100644
--- a/drivers/mtd/maps/Makefile
+++ b/drivers/mtd/maps/Makefile
@@ -57,6 +57,7 @@ obj-$(CONFIG_MTD_IXP2000)	+= ixp2000.o
 obj-$(CONFIG_MTD_WRSBC8260)	+= wr_sbc82xx_flash.o
 obj-$(CONFIG_MTD_DMV182)	+= dmv182.o
 obj-$(CONFIG_MTD_PLATRAM)	+= plat-ram.o
+obj-$(CONFIG_MTD_OFRAM)		+= of-ram.o
 obj-$(CONFIG_MTD_OMAP_NOR)	+= omap_nor.o
 obj-$(CONFIG_MTD_INTEL_VR_NOR)	+= intel_vr_nor.o
 obj-$(CONFIG_MTD_BFIN_ASYNC)	+= bfin-async-flash.o
diff --git a/drivers/mtd/maps/of-ram.c b/drivers/mtd/maps/of-ram.c
new file mode 100644
index 0000000..e2f4476
--- /dev/null
+++ b/drivers/mtd/maps/of-ram.c
@@ -0,0 +1,102 @@
+/*
+ * Generic of device based RAM map
+ *
+ * Copyright (C) 2009 Wolfram Sang, Pengutronix
+ *
+ * Using plat-ram.c by Ben Dooks
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 as published by
+ * the Free Software Foundation.
+ */
+
+#include <linux/module.h>
+#include <linux/types.h>
+#include <linux/init.h>
+#include <linux/kernel.h>
+#include <linux/device.h>
+#include <linux/platform_device.h>
+#include <linux/of_device.h>
+#include <linux/of_platform.h>
+#include <linux/mtd/plat-ram.h>
+
+static __devinit int of_ram_probe(struct of_device *op,
+			const struct of_device_id *match)
+{
+	struct platdata_mtd_ram *pdata;
+	struct resource res;
+	const u32 *bankwidth;
+	int ret = -ENOENT;
+
+	pdata = kzalloc(sizeof(*pdata), GFP_KERNEL);
+	if (!pdata)
+		return -ENOMEM;
+	op->dev.platform_data = pdata;
+
+	ret = of_address_to_resource(op->node, 0, &res);
+	if (ret) {
+		dev_dbg(&op->dev, "could not create resource (%d)\n", ret);
+		goto out_free;
+	}
+
+	bankwidth = of_get_property(op->node, "bank-width", NULL);
+	if (!bankwidth || *bankwidth == 0) {
+		dev_dbg(&op->dev, "bank width not set\n");
+		goto out_free;
+	}
+	pdata->bankwidth = *bankwidth;
+
+	ret = __platram_probe(&op->dev, op->node->name, &res, pdata);
+	if (ret) {
+		dev_dbg(&op->dev, "error probing device (%d)\n", ret);
+		goto out_free;
+	}
+
+	return 0;
+
+out_free:
+	op->dev.platform_data = NULL;
+	kfree(pdata);
+	return ret;
+}
+
+static __devexit int of_ram_remove(struct of_device *op)
+{
+	int ret;
+	struct platdata_mtd_ram *pdata = op->dev.platform_data;
+
+	ret = __platram_remove(&op->dev);
+	op->dev.platform_data = NULL;
+	kfree(pdata);
+	return ret;
+}
+
+static const struct of_device_id __devinitconst of_ram_match[] = {
+	{ .compatible = "mtd-ram", },
+	{},
+};
+MODULE_DEVICE_TABLE(of, of_ram_match);
+
+static struct of_platform_driver of_ram_driver = {
+	.owner = THIS_MODULE,
+	.name = "of-mtd-ram",
+	.match_table = of_ram_match,
+	.probe = of_ram_probe,
+	.remove = __devexit_p(of_ram_remove),
+};
+
+static int __init of_ram_init(void)
+{
+	return of_register_platform_driver(&of_ram_driver);
+}
+module_init(of_ram_init);
+
+static void __exit of_ram_exit(void)
+{
+	of_unregister_platform_driver(&of_ram_driver);
+}
+module_exit(of_ram_exit);
+
+MODULE_AUTHOR("Wolfram Sang");
+MODULE_DESCRIPTION("OF-MTD-RAM map driver");
+MODULE_LICENSE("GPL v2");
-- 
1.6.2

Re: [PATCH V2 2/2] mtd/maps/mtd-ram: add an of-platform driver

[Grant Likely]

On Fri, Jun 5, 2009 at 6:05 AM, Wolfram Sang <w.sang@pengutronix.de> wrote:
> Create an of-aware driver using the now exported generic functions from
> plat-ram.c. Also add the documentation for the binding. Partitions are
> not yet supported. Tested on a phyCORE-MPC5200B-IO.
>
> Signed-off-by: Wolfram Sang <w.sang@pengutronix.de>
> Cc: Albrecht Dre=DF <albrecht.dress@arcor.de>
> Cc: Grant Likely <grant.likely@secretlab.ca>
> Cc: Ben Dooks <ben-linux@fluff.org>
> Cc: David Woodhouse <dwmw2@infradead.org>
> Cc: linuxppc-dev@ozlabs.org
> Cc: devicetree-discuss@ozlabs.org
> Cc: linux-mtd@lists.infradead.org

On brief review, looks good to me (for both patches).
Acked-by: Grant Likely <grant.likely@secretlab.ca>

> ---
> Changes in V2:
>
> - added binding documentation
> - added __devinit & friends
> - put struct resource on stack during probe
> - simplified getting the name of the of-device
> - made error path more readable & add check for valid bank-width-pointer
> - rename driver to "of-mtd-ram"
>
> =A0Documentation/powerpc/dts-bindings/mtd-ram.txt | =A0 18 ++++
> =A0drivers/mtd/maps/Kconfig =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 |=
 =A0 =A07 ++
> =A0drivers/mtd/maps/Makefile =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0|=
 =A0 =A01 +
> =A0drivers/mtd/maps/of-ram.c =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0|=
 =A0102 ++++++++++++++++++++++++
> =A04 files changed, 128 insertions(+), 0 deletions(-)
> =A0create mode 100644 Documentation/powerpc/dts-bindings/mtd-ram.txt
> =A0create mode 100644 drivers/mtd/maps/of-ram.c
>
> diff --git a/Documentation/powerpc/dts-bindings/mtd-ram.txt b/Documentati=
on/powerpc/dts-bindings/mtd-ram.txt
> new file mode 100644
> index 0000000..a2e9932
> --- /dev/null
> +++ b/Documentation/powerpc/dts-bindings/mtd-ram.txt
> @@ -0,0 +1,18 @@
> +RAM emulating an MTD
> +
> +An external RAM like SRAM or FRAM can also be treated as an MTD.
> +
> + - compatible : should contain the specific model of the RAM chip
> + =A0 used, if known, followed by "mtd-ram".
> + - reg : Address range of the RAM chip
> + - bank-width : Width (in bytes) of the RAM bank.
> +
> +Partitions are not yet supported.
> +
> +Example:
> +
> + =A0 =A0 =A0 sram@2,0 {
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 compatible =3D "samsung,k6f1616u6a", "mtd-r=
am";
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 reg =3D <2 0 0x00200000>;
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 bank-width =3D <2>;
> + =A0 =A0 =A0 };
> diff --git a/drivers/mtd/maps/Kconfig b/drivers/mtd/maps/Kconfig
> index 82923bd..bdd9ebc 100644
> --- a/drivers/mtd/maps/Kconfig
> +++ b/drivers/mtd/maps/Kconfig
> @@ -551,6 +551,13 @@ config MTD_PLATRAM
>
> =A0 =A0 =A0 =A0 =A0This selection automatically selects the map_ram drive=
r.
>
> +config MTD_OFRAM
> + =A0 =A0 =A0 tristate "Map driver for of-device RAM (of-mtd-ram)"
> + =A0 =A0 =A0 depends on MTD_PLATRAM && OF
> + =A0 =A0 =A0 help
> + =A0 =A0 =A0 =A0 Map driver for RAM areas described via the of-device
> + =A0 =A0 =A0 =A0 system.
> +
> =A0config MTD_VMU
> =A0 =A0 =A0 =A0tristate "Map driver for Dreamcast VMU"
> =A0 =A0 =A0 =A0depends on MAPLE
> diff --git a/drivers/mtd/maps/Makefile b/drivers/mtd/maps/Makefile
> index 2dbc1be..a7a2db3 100644
> --- a/drivers/mtd/maps/Makefile
> +++ b/drivers/mtd/maps/Makefile
> @@ -57,6 +57,7 @@ obj-$(CONFIG_MTD_IXP2000) =A0 =A0 +=3D ixp2000.o
> =A0obj-$(CONFIG_MTD_WRSBC8260) =A0 =A0+=3D wr_sbc82xx_flash.o
> =A0obj-$(CONFIG_MTD_DMV182) =A0 =A0 =A0 +=3D dmv182.o
> =A0obj-$(CONFIG_MTD_PLATRAM) =A0 =A0 =A0+=3D plat-ram.o
> +obj-$(CONFIG_MTD_OFRAM) =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0+=3D of-ram.o
> =A0obj-$(CONFIG_MTD_OMAP_NOR) =A0 =A0 +=3D omap_nor.o
> =A0obj-$(CONFIG_MTD_INTEL_VR_NOR) +=3D intel_vr_nor.o
> =A0obj-$(CONFIG_MTD_BFIN_ASYNC) =A0 +=3D bfin-async-flash.o
> diff --git a/drivers/mtd/maps/of-ram.c b/drivers/mtd/maps/of-ram.c
> new file mode 100644
> index 0000000..e2f4476
> --- /dev/null
> +++ b/drivers/mtd/maps/of-ram.c
> @@ -0,0 +1,102 @@
> +/*
> + * Generic of device based RAM map
> + *
> + * Copyright (C) 2009 Wolfram Sang, Pengutronix
> + *
> + * Using plat-ram.c by Ben Dooks
> + *
> + * This program is free software; you can redistribute it and/or modify =
it
> + * under the terms of the GNU General Public License version 2 as publis=
hed by
> + * the Free Software Foundation.
> + */
> +
> +#include <linux/module.h>
> +#include <linux/types.h>
> +#include <linux/init.h>
> +#include <linux/kernel.h>
> +#include <linux/device.h>
> +#include <linux/platform_device.h>
> +#include <linux/of_device.h>
> +#include <linux/of_platform.h>
> +#include <linux/mtd/plat-ram.h>
> +
> +static __devinit int of_ram_probe(struct of_device *op,
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 const struct of_device_id *=
match)
> +{
> + =A0 =A0 =A0 struct platdata_mtd_ram *pdata;
> + =A0 =A0 =A0 struct resource res;
> + =A0 =A0 =A0 const u32 *bankwidth;
> + =A0 =A0 =A0 int ret =3D -ENOENT;
> +
> + =A0 =A0 =A0 pdata =3D kzalloc(sizeof(*pdata), GFP_KERNEL);
> + =A0 =A0 =A0 if (!pdata)
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 return -ENOMEM;
> + =A0 =A0 =A0 op->dev.platform_data =3D pdata;
> +
> + =A0 =A0 =A0 ret =3D of_address_to_resource(op->node, 0, &res);
> + =A0 =A0 =A0 if (ret) {
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 dev_dbg(&op->dev, "could not create resourc=
e (%d)\n", ret);
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 goto out_free;
> + =A0 =A0 =A0 }
> +
> + =A0 =A0 =A0 bankwidth =3D of_get_property(op->node, "bank-width", NULL)=
;
> + =A0 =A0 =A0 if (!bankwidth || *bankwidth =3D=3D 0) {
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 dev_dbg(&op->dev, "bank width not set\n");
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 goto out_free;
> + =A0 =A0 =A0 }
> + =A0 =A0 =A0 pdata->bankwidth =3D *bankwidth;
> +
> + =A0 =A0 =A0 ret =3D __platram_probe(&op->dev, op->node->name, &res, pda=
ta);
> + =A0 =A0 =A0 if (ret) {
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 dev_dbg(&op->dev, "error probing device (%d=
)\n", ret);
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 goto out_free;
> + =A0 =A0 =A0 }
> +
> + =A0 =A0 =A0 return 0;
> +
> +out_free:
> + =A0 =A0 =A0 op->dev.platform_data =3D NULL;
> + =A0 =A0 =A0 kfree(pdata);
> + =A0 =A0 =A0 return ret;
> +}
> +
> +static __devexit int of_ram_remove(struct of_device *op)
> +{
> + =A0 =A0 =A0 int ret;
> + =A0 =A0 =A0 struct platdata_mtd_ram *pdata =3D op->dev.platform_data;
> +
> + =A0 =A0 =A0 ret =3D __platram_remove(&op->dev);
> + =A0 =A0 =A0 op->dev.platform_data =3D NULL;
> + =A0 =A0 =A0 kfree(pdata);
> + =A0 =A0 =A0 return ret;
> +}
> +
> +static const struct of_device_id __devinitconst of_ram_match[] =3D {
> + =A0 =A0 =A0 { .compatible =3D "mtd-ram", },
> + =A0 =A0 =A0 {},
> +};
> +MODULE_DEVICE_TABLE(of, of_ram_match);
> +
> +static struct of_platform_driver of_ram_driver =3D {
> + =A0 =A0 =A0 .owner =3D THIS_MODULE,
> + =A0 =A0 =A0 .name =3D "of-mtd-ram",
> + =A0 =A0 =A0 .match_table =3D of_ram_match,
> + =A0 =A0 =A0 .probe =3D of_ram_probe,
> + =A0 =A0 =A0 .remove =3D __devexit_p(of_ram_remove),
> +};
> +
> +static int __init of_ram_init(void)
> +{
> + =A0 =A0 =A0 return of_register_platform_driver(&of_ram_driver);
> +}
> +module_init(of_ram_init);
> +
> +static void __exit of_ram_exit(void)
> +{
> + =A0 =A0 =A0 of_unregister_platform_driver(&of_ram_driver);
> +}
> +module_exit(of_ram_exit);
> +
> +MODULE_AUTHOR("Wolfram Sang");
> +MODULE_DESCRIPTION("OF-MTD-RAM map driver");
> +MODULE_LICENSE("GPL v2");
> --
> 1.6.2
>
>



--=20
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.

Re: [PATCH V2 2/2] mtd/maps/mtd-ram: add an of-platform driver

[Albrecht Dreß]

[-- Attachment #1: Type: text/plain, Size: 938 bytes --]

Am 05.06.09 14:05 schrieb(en) Wolfram Sang:
> Create an of-aware driver using the now exported generic functions  
> from plat-ram.c. Also add the documentation for the binding.  
> Partitions are not yet supported. Tested on a phyCORE-MPC5200B-IO.
> 
> Signed-off-by: Wolfram Sang <w.sang@pengutronix.de>
> Cc: Albrecht Dreß <albrecht.dress@arcor.de>
> Cc: Grant Likely <grant.likely@secretlab.ca>
> Cc: Ben Dooks <ben-linux@fluff.org>
> Cc: David Woodhouse <dwmw2@infradead.org>
> Cc: linuxppc-dev@ozlabs.org
> Cc: devicetree-discuss@ozlabs.org
> Cc: linux-mtd@lists.infradead.org

These patches work great for me on a self-designed, roughly Icecube  
based, 5200B board, with a 512kB NV Ram.  As I use 16-bit accesses via  
the Local Plus bus to the chip, I had to add some more tweaks to other  
files, which I promise to post next week...

Thanks, Albrecht.

Acked-by: Albrecht Dreß <albrecht.dress@arcor.de>


[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH V2 2/2] mtd/maps/mtd-ram: add an of-platform driver

[David Woodhouse]

On Fri, 2009-06-05 at 14:05 +0200, Wolfram Sang wrote:
> Create an of-aware driver using the now exported generic functions from
> plat-ram.c. Also add the documentation for the binding. Partitions are
> not yet supported. Tested on a phyCORE-MPC5200B-IO.

Do we have an ack for the device-tree bindings? 

It _would_ be possible to hook up RAM through the existing of_physmap
driver, I think -- although it would be slightly less efficient that
way.

Maybe cleaner from the device-tree POV though. And if we want to put a
special case in the _code_ to make it more efficient, we can do that.

-- 
David Woodhouse                            Open Source Technology Centre
David.Woodhouse@intel.com                              Intel Corporation

Re: [PATCH V2 2/2] mtd/maps/mtd-ram: add an of-platform driver

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 1762 bytes --]

On Sat, Jun 06, 2009 at 09:14:08AM +0100, David Woodhouse wrote:

> On Fri, 2009-06-05 at 14:05 +0200, Wolfram Sang wrote:
> > Create an of-aware driver using the now exported generic functions from
> > plat-ram.c. Also add the documentation for the binding. Partitions are
> > not yet supported. Tested on a phyCORE-MPC5200B-IO.
> 
> Do we have an ack for the device-tree bindings? 
> 
> It _would_ be possible to hook up RAM through the existing of_physmap
> driver, I think -- although it would be slightly less efficient that
> way.
> 
> Maybe cleaner from the device-tree POV though. And if we want to put a
> special case in the _code_ to make it more efficient, we can do that.

During development, I also checked physmap_of.c and found this binding:

        {
                .type           = "rom",
                .compatible     = "direct-mapped"
        },

which made some sense to me and I thought about .type = "ram". However, I then
found this in the code:

/* Helper function to handle probing of the obsolete "direct-mapped"
 * compatible binding, which has an extra "probe-type" property
 * describing the type of flash probe necessary. */
static struct mtd_info * __devinit obsolete_probe(struct of_device *dev,
                                                  struct map_info *map)
{
[...]

        dev_warn(&dev->dev, "Device tree uses obsolete \"direct-mapped\" "
                 "flash binding\n");

My conclusion was then that a mtd-ram binding wouldn't belong here, but I have
maybe misinterpreted things...

Regards,

   Wolfram

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

Re: [PATCH V2 2/2] mtd/maps/mtd-ram: add an of-platform driver

[Grant Likely]

On Fri, Jun 5, 2009 at 6:05 AM, Wolfram Sang<w.sang@pengutronix.de> wrote:
> diff --git a/Documentation/powerpc/dts-bindings/mtd-ram.txt b/Documentati=
on/powerpc/dts-bindings/mtd-ram.txt
> new file mode 100644
> index 0000000..a2e9932
> --- /dev/null
> +++ b/Documentation/powerpc/dts-bindings/mtd-ram.txt
> @@ -0,0 +1,18 @@
> +RAM emulating an MTD
> +
> +An external RAM like SRAM or FRAM can also be treated as an MTD.
> +
> + - compatible : should contain the specific model of the RAM chip
> + =A0 used, if known, followed by "mtd-ram".
> + - reg : Address range of the RAM chip
> + - bank-width : Width (in bytes) of the RAM bank.

Question: why is bank-width even relevant for a RAM device?

g.

--=20
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.

Re: [PATCH V2 2/2] mtd/maps/mtd-ram: add an of-platform driver

[Albrecht Dreß]

[-- Attachment #1: Type: text/plain, Size: 584 bytes --]

Am 06.06.09 18:05 schrieb(en) Grant Likely:
> > + - bank-width : Width (in bytes) of the RAM bank.
> 
> Question: why is bank-width even relevant for a RAM device?

At least if the RAM is attached to the 5200's Local Plus Bus in 16-bit  
mode, no byte write accesses are allowed (byte /reads/ work, though).   
I have a tweak (which I will post next week) to address this case,  
which depends upon this setting.  No idea if the same happens on other  
chips and/or on 5200 in 32-bit (muxed) mode, but I guess similar  
effects will pop up there, too.

Cheers, Albrecht.

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH V2 2/2] mtd/maps/mtd-ram: add an of-platform driver

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 505 bytes --]

> Question: why is bank-width even relevant for a RAM device?

The underlying map_ram-driver uses it once while erasing. The question remains
if this is really needed? And: Does this need to get solved before merging my
patches because changing the binding afterwards is hard? Or can they go
mainline nevertheless?

Thanks,

   Wolfram

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

Re: [PATCH V2 2/2] mtd/maps/mtd-ram: add an of-platform driver

[Albrecht Dreß]

[-- Attachment #1: Type: text/plain, Size: 1036 bytes --]

Am 08.06.09 18:35 schrieb(en) Wolfram Sang:
>> Question: why is bank-width even relevant for a RAM device?
> 
> The underlying map_ram-driver uses it once while erasing. The  
> question remains if this is really needed?

Am 06.06.09 18:16 schrieb(en) Albrecht Dreß:
> At least if the RAM is attached to the 5200's Local Plus Bus in  
> 16-bit mode, no byte write accesses are allowed (byte /reads/ work,  
> though).  I have a tweak (which I will post next week) to address  
> this case, which depends upon this setting.

To put this clearer: on '5200 based systems, the driver (more specific:  
the function inline_map_copy_to()) *must* know whether the hardware is  
connected in 8-bit or 16-bit mode to the Local Plus Bus, as byte writes  
(issued by memcpy_toio()) will fail for the latter setup (probably the  
same applies for byte and word writes in 32-bit mode).

IMHO, this information should be passed using the device tree.  The  
"bank-width" seems to be an obvious choice for that.

Best, Albrecht.

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH V2 2/2] mtd/maps/mtd-ram: add an of-platform driver

[Albrecht Dreß]

[-- Attachment #1: Type: text/plain, Size: 608 bytes --]

Am 05.06.09 14:05 schrieb(en) Wolfram Sang:
> Create an of-aware driver using the now exported generic functions  
> from plat-ram.c. Also add the documentation for the binding.  
> Partitions are not yet supported. Tested on a phyCORE-MPC5200B-IO.

Dumb question: what is the current status of this patch?  I ask because  
I re-wrote my '5200 16-bit Local Plus Bus patch' according to Grant's  
suggestions, and I believe of_ram_probe() of your patch is the right  
place to check if it's attached to "fsl,mpc5200-lpb", and to change the  
write cb if the bankwidth equals 2.

Thanks, Albrecht.

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH V2 2/2] mtd/maps/mtd-ram: add an of-platform driver

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 698 bytes --]

On Sat, Jun 06, 2009 at 09:14:08AM +0100, David Woodhouse wrote:
> On Fri, 2009-06-05 at 14:05 +0200, Wolfram Sang wrote:
> > Create an of-aware driver using the now exported generic functions from
> > plat-ram.c. Also add the documentation for the binding. Partitions are
> > not yet supported. Tested on a phyCORE-MPC5200B-IO.
> 
> Do we have an ack for the device-tree bindings? 

Well, Grant acked so far and he surely has a voice in the device-tree-world :)
Or do you want a specific ack for that?

Regards,

   Wolfram

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

Re: [PATCH V2 2/2] mtd/maps/mtd-ram: add an of-platform driver

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 860 bytes --]

On Mon, Jun 15, 2009 at 07:43:20PM +0200, Albrecht Dreß wrote:
> Am 05.06.09 14:05 schrieb(en) Wolfram Sang:
>> Create an of-aware driver using the now exported generic functions  
>> from plat-ram.c. Also add the documentation for the binding.  
>> Partitions are not yet supported. Tested on a phyCORE-MPC5200B-IO.
>
> Dumb question: what is the current status of this patch?  I ask because  

I don't know.

David wondered if 'of_physmap' could be used, so I wrote what I experienced
during development. I can't tell if this helped the case.

Grant wondered if we need a bankwidth. IMHO it is needed for now, but I don't
know if this is a common agreement.

Regards,

   Wolfram

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

Re: [PATCH V2 2/2] mtd/maps/mtd-ram: add an of-platform driver

[Grant Likely]

On Tue, Jun 16, 2009 at 3:18 AM, Wolfram Sang<w.sang@pengutronix.de> wrote:
> On Mon, Jun 15, 2009 at 07:43:20PM +0200, Albrecht Dre=DF wrote:
>> Am 05.06.09 14:05 schrieb(en) Wolfram Sang:
>>> Create an of-aware driver using the now exported generic functions
>>> from plat-ram.c. Also add the documentation for the binding.
>>> Partitions are not yet supported. Tested on a phyCORE-MPC5200B-IO.
>>
>> Dumb question: what is the current status of this patch? =A0I ask becaus=
e
>
> I don't know.
>
> David wondered if 'of_physmap' could be used, so I wrote what I experienc=
ed
> during development. I can't tell if this helped the case.
>
> Grant wondered if we need a bankwidth. IMHO it is needed for now, but I d=
on't
> know if this is a common agreement.

I'm not happy about the use case though.  It probably shouldn't appear
in this binding, or if it does it should be tagged as an optional
property.  It is only in the 5200 localplus case that bank-width is
needed to figure out how to apply the workaround.

g.

--=20
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.

Re: [PATCH V2 2/2] mtd/maps/mtd-ram: add an of-platform driver

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 1051 bytes --]

> > Grant wondered if we need a bankwidth. IMHO it is needed for now, but I don't
> > know if this is a common agreement.
> 
> I'm not happy about the use case though.  It probably shouldn't appear
> in this binding, or if it does it should be tagged as an optional
> property.  It is only in the 5200 localplus case that bank-width is
> needed to figure out how to apply the workaround.

Maybe there is a misunderstanding here. I am not talking about Albrecht's case.
What I replied to your concern is that bankwidth is used(!) in the underlying
map-ram-driver in mapram_erase() at the moment. Whether this is really needed
could be discussed perhaps, but is beyond the scope of this patch series IMHO.
I'd think this can be addressed in a later series, if needed, although this
could mean that the binding will change (bank-width becoming optional).

Regards,

   Wolfram

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

Re: [PATCH V2 2/2] mtd/maps/mtd-ram: add an of-platform driver

[Grant Likely]

[-- Attachment #1: Type: text/plain, Size: 1054 bytes --]

Okay, fair enough.  I wasn't paying very close attention when I replied.  It
still seems awkward to me, but not enough to object (ie. It's not
dangerous).

g.

On Jun 16, 2009 7:20 AM, "Wolfram Sang" <w.sang@pengutronix.de> wrote:

> > Grant wondered if we need a bankwidth. IMHO it is needed for now, but I
don't > > know if this i...
Maybe there is a misunderstanding here. I am not talking about Albrecht's
case.
What I replied to your concern is that bankwidth is used(!) in the
underlying
map-ram-driver in mapram_erase() at the moment. Whether this is really
needed
could be discussed perhaps, but is beyond the scope of this patch series
IMHO.
I'd think this can be addressed in a later series, if needed, although this
could mean that the binding will change (bank-width becoming optional).

Regards, Wolfram -- Pengutronix e.K. | Wolfram Sang ...

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (GNU/Linux)

iEYEARECAAYFAko3nAUACgkQD27XaX1/VRtTkACfW0aUMJHrU3m4DCel0pm5fA6J
WaQAnjGo5fn6JvMHt3Ke/xFTGB1uYT6p
=V9t5
-----END PGP SIGNATURE-----

[-- Attachment #2: Type: text/html, Size: 1407 bytes --]

Re: [PATCH V2 2/2] mtd/maps/mtd-ram: add an of-platform driver

[Albrecht Dreß]

[-- Attachment #1: Type: text/plain, Size: 874 bytes --]

Am 16.06.09 14:53 schrieb(en) Grant Likely:
> I'm not happy about the use case though.  It probably shouldn't  
> appear in this binding, or if it does it should be tagged as an  
> optional property.

I agree with you that the naming is really misleading - other devices  
which are not mtd's may suffer from the same problem of the lpb (in my  
case, I have an extra memory-mapped Ethernet chip which I didn't try to  
access yet...).

As it is actually a chip select property, what about defining an  
optional property like "cs-width = (8|16|32)" which defaults to 8 and  
may be added to each 5200 lpb child?

> It is only in the 5200 localplus case that bank-width is needed to  
> figure out how to apply the workaround.

Just out of curiosity: what about the "localbus" of other Freescale  
chips (82xx? 83xx? Maybe others?)?

Thanks, Albrecht.

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]