Hi Paul,

thank you for your comments.

Am Freitag, den 26.10.2012, 12:07 -0400 schrieb Paul Gortmaker:

On Thu, Oct 18, 2012 at 10:27 AM, Philipp Zabel [off-list ref] wrote:

quoted

This driver requests and remaps a memory region as configured in the
device tree. It serves memory from this region via the genalloc API.

Other drivers can retrieve the genalloc pool from a phandle pointing
to this drivers' device node in the device tree.

Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
Reviewed-by: Shawn Guo <redacted>
---
 Documentation/devicetree/bindings/misc/sram.txt |   17 ++++
 drivers/misc/Kconfig                            |    9 ++
 drivers/misc/Makefile                           |    1 +
 drivers/misc/sram.c                             |  111 +++++++++++++++++++++++
 4 files changed, 138 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/misc/sram.txt
 create mode 100644 drivers/misc/sram.c

diff --git a/Documentation/devicetree/bindings/misc/sram.txt b/Documentation/devicetree/bindings/misc/sram.txt
new file mode 100644
index 0000000..b64136c
--- /dev/null
+++ b/Documentation/devicetree/bindings/misc/sram.txt

@@ -0,0 +1,17 @@
+Generic on-chip SRAM
+
+Simple IO memory regions to be managed by the genalloc API.
+
+Required properties:
+
+- compatible : sram
+
+- reg : SRAM iomem address range
+
+Example:
+
+sram: sram@5c000000 {
+       compatible = "sram";
+       reg = <0x5c000000 0x40000>; /* 256 KiB SRAM at address 0x5c000000 */
+};
+

diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
index b151b7c..c5bbd00 100644
--- a/drivers/misc/Kconfig
+++ b/drivers/misc/Kconfig

@@ -499,6 +499,15 @@ config USB_SWITCH_FSA9480
          stereo and mono audio, video, microphone and UART data to use
          a common connector port.

+config SRAM
+       bool "Generic on-chip SRAM driver"

If it is bool, then why bother with module.h and all the
MODULE_AUTHOR and similar stuff?

Yes. This driver was a module before I noticed that I then would have to
keep track in genalloc to avoid module unloading while some SRAM is
allocated. It seemed a bit too invasive.

quoted

+       depends on HAS_IOMEM
+       select GENERIC_ALLOCATOR
+       help
+         This driver allows to declare a memory region to be managed
+         by the genalloc API. It is supposed to be used for small
+         on-chip SRAM areas found on many ARM SoCs.

Probably no need to call out ARM specifically here.  I'm pretty sure
quite a few of the powerpc parts have SRAM too.

Sure, I'll just s/ARM // then.

quoted

+
 source "drivers/misc/c2port/Kconfig"
 source "drivers/misc/eeprom/Kconfig"
 source "drivers/misc/cb710/Kconfig"

diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
index 2129377..d845690 100644
--- a/drivers/misc/Makefile
+++ b/drivers/misc/Makefile

@@ -49,3 +49,4 @@ obj-y                         += carma/
 obj-$(CONFIG_USB_SWITCH_FSA9480) += fsa9480.o
 obj-$(CONFIG_ALTERA_STAPL)     +=altera-stapl/
 obj-$(CONFIG_INTEL_MEI)                += mei/
+obj-$(CONFIG_SRAM)             += sram.o

diff --git a/drivers/misc/sram.c b/drivers/misc/sram.c
new file mode 100644
index 0000000..7a363f2
--- /dev/null
+++ b/drivers/misc/sram.c

@@ -0,0 +1,111 @@
+/*
+ * Generic on-chip SRAM allocation driver
+ *
+ * Copyright (C) 2012 Philipp Zabel, Pengutronix
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version 2
+ * of the License, or (at your option) any later version.
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston,
+ * MA 02110-1301, USA.
+ */
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/io.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/spinlock.h>
+#include <linux/genalloc.h>
+
+struct sram_dev {
+       struct gen_pool *pool;
+};

Assuming you don't ever intend adding more to the struct, then:

See the clock patch. I'm not sure if further functionality will be
needed on any architecture.

static struct gen_pool *sram_pool;

instead?

No, a global variable would mean that only one instance of the SRAM
driver can be used. What about chips that have separate SRAM regions?

And you'll lose the needless indirections in the
remaining code below, and the kzalloc too (which you
currently leak, btw).

Memory allocated by devm_kzalloc should be freed by the devres
framework. If I am missing something, could you please elaborate?

I'd also delete the "/* sentinel */"
comment too, since it is self evident. (The disease has
spread by copying from other drivers I see).

I can do that.

P.
--

quoted

+
+static int __devinit sram_probe(struct platform_device *pdev)
+{
+       void __iomem *virt_base;
+       struct sram_dev *sram;
+       struct resource *res;
+       unsigned long size;
+       int ret;
+
+       res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+       if (!res)
+               return -EINVAL;
+
+       size = resource_size(res);
+
+       virt_base = devm_request_and_ioremap(&pdev->dev, res);
+       if (!virt_base)
+               return -EADDRNOTAVAIL;
+
+       sram = devm_kzalloc(&pdev->dev, sizeof(*sram), GFP_KERNEL);
+       if (!sram)
+               return -ENOMEM;
+
+       sram->pool = gen_pool_create(PAGE_SHIFT, -1);
+       if (!sram->pool)
+               return -ENOMEM;
+
+       ret = gen_pool_add_virt(sram->pool, (unsigned long)virt_base,
+                               res->start, size, -1);
+       if (ret < 0) {
+               gen_pool_destroy(sram->pool);
+               return ret;
+       }
+
+       platform_set_drvdata(pdev, sram);
+
+       dev_dbg(&pdev->dev, "SRAM pool: %ld KiB @ 0x%p\n", size / 1024, virt_base);
+
+       return 0;
+}
+
+static int __devexit sram_remove(struct platform_device *pdev)
+{
+       struct sram_dev *sram = platform_get_drvdata(pdev);
+
+       if (gen_pool_avail(sram->pool) < gen_pool_size(sram->pool))
+               dev_dbg(&pdev->dev, "removed while SRAM allocated\n");
+
+       gen_pool_destroy(sram->pool);
+
+       return 0;
+}
+
+#ifdef CONFIG_OF
+static struct of_device_id sram_dt_ids[] = {
+       { .compatible = "sram" },
+       { /* sentinel */ }
+};
+#endif
+
+static struct platform_driver sram_driver = {
+       .driver = {
+               .name = "sram",
+               .of_match_table = of_match_ptr(sram_dt_ids),
+       },
+       .probe = sram_probe,
+       .remove = __devexit_p(sram_remove),
+};
+
+int __init sram_init(void)
+{
+       return platform_driver_register(&sram_driver);
+}
+
+postcore_initcall(sram_init);
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Philipp Zabel, Pengutronix");
+MODULE_DESCRIPTION("Generic SRAM allocator driver");
--
1.7.10.4

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

regards
Philipp