| Message ID | 1232559685-2268-3-git-send-email-w.sang@pengutronix.de |
|---|---|
| State | New, archived |
| Headers | show |
Hello, found one question thing myself... On Wed, Jan 21, 2009 at 06:41:25PM +0100, Wolfram Sang wrote: > Create an of-aware driver using the now exported generic functions from > plat-ram.c. A typical oftree snipplet for it looks like this: > > sram@04e00000 { > compatible = "mtd-ram"; > reg = <0x04e00000 0x00200000>; > bank-width = <2>; > }; > > Partitions on this device are not yet supported. > > Signed-off-by: Wolfram Sang <w.sang@pengutronix.de> > --- > drivers/mtd/maps/Kconfig | 6 ++ > drivers/mtd/maps/Makefile | 1 + > drivers/mtd/maps/of-ram.c | 120 +++++++++++++++++++++++++++++++++++++++++++++ > 3 files changed, 127 insertions(+), 0 deletions(-) > create mode 100644 drivers/mtd/maps/of-ram.c > > diff --git a/drivers/mtd/maps/Kconfig b/drivers/mtd/maps/Kconfig > index 0225cbb..b238b6d 100644 > --- a/drivers/mtd/maps/Kconfig > +++ b/drivers/mtd/maps/Kconfig > @@ -551,5 +551,11 @@ config MTD_PLATRAM > > This selection automatically selects the map_ram driver. > > +config MTD_OFRAM > + tristate "Map driver for of-device RAM (mtd-ram)" > + depends on MTD_PLATRAM && OF > + help > + Map driver for RAM areas described via the of-device > + system. > endmenu > > diff --git a/drivers/mtd/maps/Makefile b/drivers/mtd/maps/Makefile > index 6d9ba35..f8e3908 100644 > --- a/drivers/mtd/maps/Makefile > +++ b/drivers/mtd/maps/Makefile > @@ -58,6 +58,7 @@ obj-$(CONFIG_MTD_WRSBC8260) += wr_sbc82xx_flash.o > obj-$(CONFIG_MTD_DMV182) += dmv182.o > obj-$(CONFIG_MTD_SHARP_SL) += sharpsl-flash.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..4d334a9 > --- /dev/null > +++ b/drivers/mtd/maps/of-ram.c > @@ -0,0 +1,120 @@ > +/* > + * drivers/mtd/maps/of-ram.c > + * > + * 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 int of_ram_probe(struct of_device *op, const struct of_device_id *match) > +{ > + struct platdata_mtd_ram *pdata; > + struct resource *resource; > + const char *name; > + const u32 *bankwidth; > + int ret = -ENOMEM; > + > + pdata = kzalloc(sizeof(*pdata), GFP_KERNEL); > + if (!pdata) > + goto bad0; > + op->dev.platform_data = pdata; > + > + name = of_get_property(op->node, "name", NULL); As this pointer gets passed on and used, I must probably add something like of_node_get(op->node) here and put the node later in the remove function? Not 100% sure, though... > + if (!name) { > + ret = -ENOENT; > + dev_dbg(&op->dev, "could not get node name\n"); > + goto bad1; > + } > + > + resource = kzalloc(sizeof(struct resource), GFP_KERNEL); > + if (!resource) > + goto bad1; > + > + if (of_address_to_resource(op->node, 0, resource)) { > + ret = -ENXIO; > + dev_dbg(&op->dev, "could not create resource\n"); > + goto bad2; > + } > + > + bankwidth = of_get_property(op->node, "bank-width", NULL); > + if (*bankwidth == 0) { > + ret = -ENOENT; > + dev_dbg(&op->dev, "bank width not set\n"); > + goto bad2; > + } > + pdata->bankwidth = *bankwidth; > + > + ret = __platram_probe(&op->dev, name, resource, pdata); > + kfree(resource); > + > + if (ret) > + goto bad1; > + > + return 0; > + > + bad2: > + kfree(resource); > + bad1: > + op->dev.platform_data = NULL; > + kfree(pdata); > + bad0: > + return ret; > +} > + > +static 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 struct of_device_id 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-ram", > + .match_table = of_ram_match, > + .probe = of_ram_probe, > + .remove = of_ram_remove, > +}; > + > +static int __init of_ram_init(void) > +{ > + return of_register_platform_driver(&of_ram_driver); > +} > + > +static void __exit of_ram_exit(void) > +{ > + of_unregister_platform_driver(&of_ram_driver); > +} > + > +module_init(of_ram_init); > +module_exit(of_ram_exit); > + > +MODULE_AUTHOR("Wolfram Sang"); > +MODULE_DESCRIPTION("MTD OF RAM map driver"); > +MODULE_LICENSE("GPL v2"); > -- > 1.5.6.5 >
On Wed, Jan 21, 2009 at 11:41 AM, Wolfram Sang <w.sang@pengutronix.de> wrote: > Create an of-aware driver using the now exported generic functions from > plat-ram.c. A typical oftree snipplet for it looks like this: > > sram@04e00000 { > compatible = "mtd-ram"; > reg = <0x04e00000 0x00200000>; > bank-width = <2>; > }; > > Partitions on this device are not yet supported. This is a new device tree binding. It must be documented in Documentation/powerpc/dts-bindings and posted to devicetree-discuss@ozlabs.org for review. > diff --git a/drivers/mtd/maps/Makefile b/drivers/mtd/maps/Makefile > index 6d9ba35..f8e3908 100644 > --- a/drivers/mtd/maps/Makefile > +++ b/drivers/mtd/maps/Makefile > @@ -58,6 +58,7 @@ obj-$(CONFIG_MTD_WRSBC8260) += wr_sbc82xx_flash.o > obj-$(CONFIG_MTD_DMV182) += dmv182.o > obj-$(CONFIG_MTD_SHARP_SL) += sharpsl-flash.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..4d334a9 > --- /dev/null > +++ b/drivers/mtd/maps/of-ram.c > @@ -0,0 +1,120 @@ > +/* > + * drivers/mtd/maps/of-ram.c > + * > + * 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 int of_ram_probe(struct of_device *op, const struct of_device_id *match) __devinit > +{ > + struct platdata_mtd_ram *pdata; > + struct resource *resource; > + const char *name; > + const u32 *bankwidth; > + int ret = -ENOMEM; > + > + pdata = kzalloc(sizeof(*pdata), GFP_KERNEL); > + if (!pdata) > + goto bad0; > + op->dev.platform_data = pdata; > + > + name = of_get_property(op->node, "name", NULL); > + if (!name) { > + ret = -ENOENT; > + dev_dbg(&op->dev, "could not get node name\n"); > + goto bad1; > + } > + > + resource = kzalloc(sizeof(struct resource), GFP_KERNEL); > + if (!resource) > + goto bad1; Why isn't resource on the stack? > + > + if (of_address_to_resource(op->node, 0, resource)) { > + ret = -ENXIO; > + dev_dbg(&op->dev, "could not create resource\n"); > + goto bad2; > + } > + > + bankwidth = of_get_property(op->node, "bank-width", NULL); > + if (*bankwidth == 0) { > + ret = -ENOENT; > + dev_dbg(&op->dev, "bank width not set\n"); > + goto bad2; > + } > + pdata->bankwidth = *bankwidth; > + > + ret = __platram_probe(&op->dev, name, resource, pdata); > + kfree(resource); > + > + if (ret) > + goto bad1; > + > + return 0; > + > + bad2: > + kfree(resource); > + bad1: > + op->dev.platform_data = NULL; > + kfree(pdata); > + bad0: > + return ret; I'd rather see more meaningful labels. ie "err_kalloc:" and "err_platram_probe:". > +} > + > +static int of_ram_remove(struct of_device *op) __devexit > +{ > + 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 struct of_device_id of_ram_match[] = { > + { .compatible = "mtd-ram", }, > + {}, > +}; > +MODULE_DEVICE_TABLE(of, of_ram_match); > + > +static struct of_platform_driver of_ram_driver = { __devinitdata > + .owner = THIS_MODULE, > + .name = "of-ram", This name is too generic for an of_platform device. 'mtd' needs to be in there somewhere. > + .match_table = of_ram_match, > + .probe = of_ram_probe, > + .remove = of_ram_remove, __devexit_p() > +}; > + > +static int __init of_ram_init(void) > +{ > + return of_register_platform_driver(&of_ram_driver); > +} > + > +static void __exit of_ram_exit(void) > +{ > + of_unregister_platform_driver(&of_ram_driver); > +} > + > +module_init(of_ram_init); Put the module_init() statement directly after the of_ram_init() definition. > +module_exit(of_ram_exit); > + > +MODULE_AUTHOR("Wolfram Sang"); > +MODULE_DESCRIPTION("MTD OF RAM map driver"); > +MODULE_LICENSE("GPL v2"); > -- > 1.5.6.5 > > _______________________________________________ > Linuxppc-dev mailing list > Linuxppc-dev@ozlabs.org > https://ozlabs.org/mailman/listinfo/linuxppc-dev >
Hi Grant, thanks a lot for the review! On Tue, May 19, 2009 at 08:57:15AM -0600, Grant Likely wrote: > On Wed, Jan 21, 2009 at 11:41 AM, Wolfram Sang <w.sang@pengutronix.de> wrote: > > Create an of-aware driver using the now exported generic functions from > > plat-ram.c. A typical oftree snipplet for it looks like this: > > > > sram@04e00000 { > > compatible = "mtd-ram"; > > reg = <0x04e00000 0x00200000>; > > bank-width = <2>; > > }; > > > > Partitions on this device are not yet supported. > > This is a new device tree binding. It must be documented in > Documentation/powerpc/dts-bindings and posted to > devicetree-discuss@ozlabs.org for review. Yeah, noticed that myself. I wanted to start another try to mainline this driver next week, then also asking devicetree-discuss; will do it happily this week, too :) > > > diff --git a/drivers/mtd/maps/Makefile b/drivers/mtd/maps/Makefile > > index 6d9ba35..f8e3908 100644 > > --- a/drivers/mtd/maps/Makefile > > +++ b/drivers/mtd/maps/Makefile > > @@ -58,6 +58,7 @@ obj-$(CONFIG_MTD_WRSBC8260) += wr_sbc82xx_flash.o > > obj-$(CONFIG_MTD_DMV182) += dmv182.o > > obj-$(CONFIG_MTD_SHARP_SL) += sharpsl-flash.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..4d334a9 > > --- /dev/null > > +++ b/drivers/mtd/maps/of-ram.c > > @@ -0,0 +1,120 @@ > > +/* > > + * drivers/mtd/maps/of-ram.c > > + * > > + * 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 int of_ram_probe(struct of_device *op, const struct of_device_id *match) > > __devinit Oops, missed all of those. > > > +{ > > + struct platdata_mtd_ram *pdata; > > + struct resource *resource; > > + const char *name; > > + const u32 *bankwidth; > > + int ret = -ENOMEM; > > + > > + pdata = kzalloc(sizeof(*pdata), GFP_KERNEL); > > + if (!pdata) > > + goto bad0; > > + op->dev.platform_data = pdata; > > + > > + name = of_get_property(op->node, "name", NULL); > > + if (!name) { > > + ret = -ENOENT; > > + dev_dbg(&op->dev, "could not get node name\n"); > > + goto bad1; > > + } > > + > > + resource = kzalloc(sizeof(struct resource), GFP_KERNEL); > > + if (!resource) > > + goto bad1; > > Why isn't resource on the stack? Point taken. > > > + > > + if (of_address_to_resource(op->node, 0, resource)) { > > + ret = -ENXIO; > > + dev_dbg(&op->dev, "could not create resource\n"); > > + goto bad2; > > + } > > + > > + bankwidth = of_get_property(op->node, "bank-width", NULL); > > + if (*bankwidth == 0) { > > + ret = -ENOENT; > > + dev_dbg(&op->dev, "bank width not set\n"); > > + goto bad2; > > + } > > + pdata->bankwidth = *bankwidth; > > + > > + ret = __platram_probe(&op->dev, name, resource, pdata); > > + kfree(resource); > > + > > + if (ret) > > + goto bad1; > > + > > + return 0; > > + > > + bad2: > > + kfree(resource); > > + bad1: > > + op->dev.platform_data = NULL; > > + kfree(pdata); > > + bad0: > > + return ret; > > I'd rather see more meaningful labels. ie "err_kalloc:" and > "err_platram_probe:". Okay. > > > +} > > + > > +static int of_ram_remove(struct of_device *op) > > __devexit > > > +{ > > + 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 struct of_device_id of_ram_match[] = { > > + { .compatible = "mtd-ram", }, > > + {}, > > +}; > > +MODULE_DEVICE_TABLE(of, of_ram_match); > > + > > +static struct of_platform_driver of_ram_driver = { > > __devinitdata > > > + .owner = THIS_MODULE, > > + .name = "of-ram", > > This name is too generic for an of_platform device. 'mtd' needs to be > in there somewhere. Will "of-mtd-ram" do? (I'd think so) > > > + .match_table = of_ram_match, > > + .probe = of_ram_probe, > > + .remove = of_ram_remove, > > __devexit_p() > > > +}; > > + > > +static int __init of_ram_init(void) > > +{ > > + return of_register_platform_driver(&of_ram_driver); > > +} > > + > > +static void __exit of_ram_exit(void) > > +{ > > + of_unregister_platform_driver(&of_ram_driver); > > +} > > + > > +module_init(of_ram_init); > > Put the module_init() statement directly after the of_ram_init() definition. > > > +module_exit(of_ram_exit); > > + > > +MODULE_AUTHOR("Wolfram Sang"); > > +MODULE_DESCRIPTION("MTD OF RAM map driver"); > > +MODULE_LICENSE("GPL v2"); > > -- > > 1.5.6.5 > > > > _______________________________________________ > > Linuxppc-dev mailing list > > Linuxppc-dev@ozlabs.org > > https://ozlabs.org/mailman/listinfo/linuxppc-dev > > > > > > -- > Grant Likely, B.Sc., P.Eng. > Secret Lab Technologies Ltd. Will send V2 after some tests tomorrow... Thanks, Wolfram
On Tue, May 19, 2009 at 2:30 PM, Wolfram Sang <w.sang@pengutronix.de> wrote: >> This name is too generic for an of_platform device. 'mtd' needs to be >> in there somewhere. > > Will "of-mtd-ram" do? (I'd think so) fine by me. g
Hello Grant, as I am just working on V2 (fixed even some additional things), two more questions came up: On Tue, May 19, 2009 at 08:57:15AM -0600, Grant Likely wrote: > On Wed, Jan 21, 2009 at 11:41 AM, Wolfram Sang <w.sang@pengutronix.de> wrote: > > Create an of-aware driver using the now exported generic functions from > > plat-ram.c. A typical oftree snipplet for it looks like this: > > > > sram@04e00000 { > > compatible = "mtd-ram"; > > reg = <0x04e00000 0x00200000>; > > bank-width = <2>; > > }; > > > > Partitions on this device are not yet supported. > > This is a new device tree binding. It must be documented in > Documentation/powerpc/dts-bindings and posted to > devicetree-discuss@ozlabs.org for review. > > > diff --git a/drivers/mtd/maps/Makefile b/drivers/mtd/maps/Makefile > > index 6d9ba35..f8e3908 100644 > > --- a/drivers/mtd/maps/Makefile > > +++ b/drivers/mtd/maps/Makefile > > @@ -58,6 +58,7 @@ obj-$(CONFIG_MTD_WRSBC8260) += wr_sbc82xx_flash.o > > obj-$(CONFIG_MTD_DMV182) += dmv182.o > > obj-$(CONFIG_MTD_SHARP_SL) += sharpsl-flash.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..4d334a9 > > --- /dev/null > > +++ b/drivers/mtd/maps/of-ram.c > > @@ -0,0 +1,120 @@ > > +/* > > + * drivers/mtd/maps/of-ram.c > > + * > > + * 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 int of_ram_probe(struct of_device *op, const struct of_device_id *match) > > __devinit > > > +{ > > + struct platdata_mtd_ram *pdata; > > + struct resource *resource; > > + const char *name; > > + const u32 *bankwidth; > > + int ret = -ENOMEM; > > + > > + pdata = kzalloc(sizeof(*pdata), GFP_KERNEL); > > + if (!pdata) > > + goto bad0; > > + op->dev.platform_data = pdata; > > + > > + name = of_get_property(op->node, "name", NULL); > > + if (!name) { > > + ret = -ENOENT; > > + dev_dbg(&op->dev, "could not get node name\n"); > > + goto bad1; > > + } Can I just use name = op->node->name here? I wonder because of_device in asm/of_device.h states 'node' as "to be obsoleted". And could I safely assume that all architectures will have the node entry? At least, I could drop the error check, if I understand booting-without-of correctly? A name property will always be constructed, even if it is not explicitly given. > > + > > + resource = kzalloc(sizeof(struct resource), GFP_KERNEL); > > + if (!resource) > > + goto bad1; > > Why isn't resource on the stack? > > > + > > + if (of_address_to_resource(op->node, 0, resource)) { > > + ret = -ENXIO; > > + dev_dbg(&op->dev, "could not create resource\n"); > > + goto bad2; > > + } > > + > > + bankwidth = of_get_property(op->node, "bank-width", NULL); > > + if (*bankwidth == 0) { > > + ret = -ENOENT; > > + dev_dbg(&op->dev, "bank width not set\n"); > > + goto bad2; > > + } > > + pdata->bankwidth = *bankwidth; > > + > > + ret = __platram_probe(&op->dev, name, resource, pdata); > > + kfree(resource); > > + > > + if (ret) > > + goto bad1; > > + > > + return 0; > > + > > + bad2: > > + kfree(resource); > > + bad1: > > + op->dev.platform_data = NULL; > > + kfree(pdata); > > + bad0: > > + return ret; > > I'd rather see more meaningful labels. ie "err_kalloc:" and > "err_platram_probe:". > > > +} > > + > > +static int of_ram_remove(struct of_device *op) > > __devexit > > > +{ > > + 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 struct of_device_id of_ram_match[] = { > > + { .compatible = "mtd-ram", }, > > + {}, > > +}; > > +MODULE_DEVICE_TABLE(of, of_ram_match); > > + > > +static struct of_platform_driver of_ram_driver = { > > __devinitdata I assume you mean the match_table and not the of_platform_driver. Shouldn't it even better be const and using __devinitconst? > > > + .owner = THIS_MODULE, > > + .name = "of-ram", > > This name is too generic for an of_platform device. 'mtd' needs to be > in there somewhere. > > > + .match_table = of_ram_match, > > + .probe = of_ram_probe, > > + .remove = of_ram_remove, > > __devexit_p() > > > +}; > > + > > +static int __init of_ram_init(void) > > +{ > > + return of_register_platform_driver(&of_ram_driver); > > +} > > + > > +static void __exit of_ram_exit(void) > > +{ > > + of_unregister_platform_driver(&of_ram_driver); > > +} > > + > > +module_init(of_ram_init); > > Put the module_init() statement directly after the of_ram_init() definition. > > > +module_exit(of_ram_exit); > > + > > +MODULE_AUTHOR("Wolfram Sang"); > > +MODULE_DESCRIPTION("MTD OF RAM map driver"); > > +MODULE_LICENSE("GPL v2"); > > -- > > 1.5.6.5 > > > > _______________________________________________ > > Linuxppc-dev mailing list > > Linuxppc-dev@ozlabs.org > > https://ozlabs.org/mailman/listinfo/linuxppc-dev > > > > > > -- > Grant Likely, B.Sc., P.Eng. > Secret Lab Technologies Ltd. > > ______________________________________________________ > Linux MTD discussion mailing list > http://lists.infradead.org/mailman/listinfo/linux-mtd/
On Wed, May 20, 2009 at 6:46 PM, Wolfram Sang <w.sang@pengutronix.de> wrote: >> > + name = of_get_property(op->node, "name", NULL); >> > + if (!name) { >> > + ret = -ENOENT; >> > + dev_dbg(&op->dev, "could not get node name\n"); >> > + goto bad1; >> > + } > > Can I just use > > name = op->node->name > > here? I wonder because of_device in asm/of_device.h states 'node' as "to be > obsoleted" It may be labeled as such, but it's been so for over 2 years now, and op->node is used all over the place. I think Ben would like to eventually move to using the node pointer in archdata, but that will require a fair bit of refactoring. Ben will kick me if I'm wrong, but I'll go out on a limb and say that I think you're okay to use it. >. And could I safely assume that all architectures will have the node > entry? All three current users do. >> > +static struct of_device_id of_ram_match[] = { >> > + { .compatible = "mtd-ram", }, >> > + {}, >> > +}; >> > +MODULE_DEVICE_TABLE(of, of_ram_match); >> > + >> > +static struct of_platform_driver of_ram_driver = { >> >> __devinitdata > > I assume you mean the match_table and not the of_platform_driver. Shouldn't it > even better be const and using __devinitconst? correct on both counts. g.
diff --git a/drivers/mtd/maps/Kconfig b/drivers/mtd/maps/Kconfig index 0225cbb..b238b6d 100644 --- a/drivers/mtd/maps/Kconfig +++ b/drivers/mtd/maps/Kconfig @@ -551,5 +551,11 @@ config MTD_PLATRAM This selection automatically selects the map_ram driver. +config MTD_OFRAM + tristate "Map driver for of-device RAM (mtd-ram)" + depends on MTD_PLATRAM && OF + help + Map driver for RAM areas described via the of-device + system. endmenu diff --git a/drivers/mtd/maps/Makefile b/drivers/mtd/maps/Makefile index 6d9ba35..f8e3908 100644 --- a/drivers/mtd/maps/Makefile +++ b/drivers/mtd/maps/Makefile @@ -58,6 +58,7 @@ obj-$(CONFIG_MTD_WRSBC8260) += wr_sbc82xx_flash.o obj-$(CONFIG_MTD_DMV182) += dmv182.o obj-$(CONFIG_MTD_SHARP_SL) += sharpsl-flash.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..4d334a9 --- /dev/null +++ b/drivers/mtd/maps/of-ram.c @@ -0,0 +1,120 @@ +/* + * drivers/mtd/maps/of-ram.c + * + * 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 int of_ram_probe(struct of_device *op, const struct of_device_id *match) +{ + struct platdata_mtd_ram *pdata; + struct resource *resource; + const char *name; + const u32 *bankwidth; + int ret = -ENOMEM; + + pdata = kzalloc(sizeof(*pdata), GFP_KERNEL); + if (!pdata) + goto bad0; + op->dev.platform_data = pdata; + + name = of_get_property(op->node, "name", NULL); + if (!name) { + ret = -ENOENT; + dev_dbg(&op->dev, "could not get node name\n"); + goto bad1; + } + + resource = kzalloc(sizeof(struct resource), GFP_KERNEL); + if (!resource) + goto bad1; + + if (of_address_to_resource(op->node, 0, resource)) { + ret = -ENXIO; + dev_dbg(&op->dev, "could not create resource\n"); + goto bad2; + } + + bankwidth = of_get_property(op->node, "bank-width", NULL); + if (*bankwidth == 0) { + ret = -ENOENT; + dev_dbg(&op->dev, "bank width not set\n"); + goto bad2; + } + pdata->bankwidth = *bankwidth; + + ret = __platram_probe(&op->dev, name, resource, pdata); + kfree(resource); + + if (ret) + goto bad1; + + return 0; + + bad2: + kfree(resource); + bad1: + op->dev.platform_data = NULL; + kfree(pdata); + bad0: + return ret; +} + +static 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 struct of_device_id 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-ram", + .match_table = of_ram_match, + .probe = of_ram_probe, + .remove = of_ram_remove, +}; + +static int __init of_ram_init(void) +{ + return of_register_platform_driver(&of_ram_driver); +} + +static void __exit of_ram_exit(void) +{ + of_unregister_platform_driver(&of_ram_driver); +} + +module_init(of_ram_init); +module_exit(of_ram_exit); + +MODULE_AUTHOR("Wolfram Sang"); +MODULE_DESCRIPTION("MTD OF RAM map driver"); +MODULE_LICENSE("GPL v2");
Create an of-aware driver using the now exported generic functions from plat-ram.c. A typical oftree snipplet for it looks like this: sram@04e00000 { compatible = "mtd-ram"; reg = <0x04e00000 0x00200000>; bank-width = <2>; }; Partitions on this device are not yet supported. Signed-off-by: Wolfram Sang <w.sang@pengutronix.de> --- drivers/mtd/maps/Kconfig | 6 ++ drivers/mtd/maps/Makefile | 1 + drivers/mtd/maps/of-ram.c | 120 +++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 127 insertions(+), 0 deletions(-) create mode 100644 drivers/mtd/maps/of-ram.c