| Message ID | 1244203514-12516-3-git-send-email-w.sang@pengutronix.de (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
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ß <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" > > 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 > >
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.
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
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/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. Question: why is bank-width even relevant for a RAM device? g.
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.
> 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
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
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
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ß 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. 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.
> > 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
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-----
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");
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