LinuxLists.cc - [PATCH v9 RESEND 0/4] Add generic driver for on-chip SRAM

[permalink] [raw]

Subject: [PATCH v9 RESEND 4/4] ARM: dts: add sram for imx53 and imx6q

Signed-off-by: Philipp Zabel <[email protected]>
Reviewed-by: Shawn Guo <[email protected]>
Acked-by: Grant Likely <[email protected]>
---
Changes since v8:
- Changed device tree compatible string to "mmio-sram"
---
arch/arm/boot/dts/imx53.dtsi | 5 +++++
arch/arm/boot/dts/imx6q.dtsi | 6 ++++++
2 files changed, 11 insertions(+)

diff --git a/arch/arm/boot/dts/imx53.dtsi b/arch/arm/boot/dts/imx53.dtsi
index edc3f1e..69d0680 100644
--- a/arch/arm/boot/dts/imx53.dtsi
+++ b/arch/arm/boot/dts/imx53.dtsi
@@ -664,5 +664,10 @@
status = "disabled";
};
};
+
+ ocram: ocram@f8000000 {
+ compatible = "fsl,imx-ocram", "mmio-sram";
+ reg = <0xf8000000 0x20000>;
+ };
};
};
diff --git a/arch/arm/boot/dts/imx6q.dtsi b/arch/arm/boot/dts/imx6q.dtsi
index ff1205e..73302db 100644
--- a/arch/arm/boot/dts/imx6q.dtsi
+++ b/arch/arm/boot/dts/imx6q.dtsi
@@ -124,6 +124,12 @@
status = "disabled";
};

+ ocram: ocram@00900000 {
+ compatible = "fsl,imx-ocram", "mmio-sram";
+ reg = <0x00900000 0x3f000>;
+ clocks = <&clks 142>;
+ };
+
timer@00a00600 {
compatible = "arm,cortex-a9-twd-timer";
reg = <0x00a00600 0x20>;
--
1.7.10.4

2013-03-20 10:53:03

[permalink] [raw]

Subject: [PATCH v9 RESEND 1/4] genalloc: add devres support, allow to find a managed pool by device

This patch adds three exported functions to lib/genalloc.c:
devm_gen_pool_create, dev_get_gen_pool, and of_get_named_gen_pool.

devm_gen_pool_create is a managed version of gen_pool_create that keeps
track of the pool via devres and allows the management code to automatically
destroy it after device removal.

dev_get_gen_pool retrieves the gen_pool for a given device, if it was
created with devm_gen_pool_create, using devres_find.

of_get_named_gen_pool retrieves the gen_pool for a given device node and
property name, where the property must contain a phandle pointing to a
platform device node. The corresponding platform device is then fed into
dev_get_gen_pool and the resulting gen_pool is returned.

Signed-off-by: Philipp Zabel <[email protected]>
Acked-by: Grant Likely <[email protected]>
---
include/linux/genalloc.h | 15 +++++++++
lib/genalloc.c | 81 ++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 96 insertions(+)

diff --git a/include/linux/genalloc.h b/include/linux/genalloc.h
index dd7c569..383e8f4 100644
--- a/include/linux/genalloc.h
+++ b/include/linux/genalloc.h
@@ -105,4 +105,19 @@ extern unsigned long gen_pool_first_fit(unsigned long *map, unsigned long size,
extern unsigned long gen_pool_best_fit(unsigned long *map, unsigned long size,
unsigned long start, unsigned int nr, void *data);

+extern struct gen_pool *devm_gen_pool_create(struct device *dev,
+ int min_alloc_order, int nid);
+extern struct gen_pool *dev_get_gen_pool(struct device *dev);
+
+struct device_node;
+#ifdef CONFIG_OF
+extern struct gen_pool *of_get_named_gen_pool(struct device_node *np,
+ const char *propname, int index);
+#else
+inline struct gen_pool *of_get_named_gen_pool(struct device_node *np,
+ const char *propname, int index)
+{
+ return NULL;
+}
+#endif
#endif /* __GENALLOC_H__ */
diff --git a/lib/genalloc.c b/lib/genalloc.c
index 5492043..b35cfa9 100644
--- a/lib/genalloc.c
+++ b/lib/genalloc.c
@@ -34,6 +34,8 @@
#include <linux/rculist.h>
#include <linux/interrupt.h>
#include <linux/genalloc.h>
+#include <linux/of_address.h>
+#include <linux/of_device.h>

static int set_bits_ll(unsigned long *addr, unsigned long mask_to_set)
{
@@ -480,3 +482,82 @@ unsigned long gen_pool_best_fit(unsigned long *map, unsigned long size,
return start_bit;
}
EXPORT_SYMBOL(gen_pool_best_fit);
+
+static void devm_gen_pool_release(struct device *dev, void *res)
+{
+ gen_pool_destroy(*(struct gen_pool **)res);
+}
+
+/**
+ * devm_gen_pool_create - managed gen_pool_create
+ * @dev: device that provides the gen_pool
+ * @min_alloc_order: log base 2 of number of bytes each bitmap bit represents
+ * @nid: node id of the node the pool structure should be allocated on, or -1
+ *
+ * Create a new special memory pool that can be used to manage special purpose
+ * memory not managed by the regular kmalloc/kfree interface. The pool will be
+ * automatically destroyed by the device management code.
+ */
+struct gen_pool *devm_gen_pool_create(struct device *dev, int min_alloc_order,
+ int nid)
+{
+ struct gen_pool **ptr, *pool;
+
+ ptr = devres_alloc(devm_gen_pool_release, sizeof(*ptr), GFP_KERNEL);
+
+ pool = gen_pool_create(min_alloc_order, nid);
+ if (pool) {
+ *ptr = pool;
+ devres_add(dev, ptr);
+ } else {
+ devres_free(ptr);
+ }
+
+ return pool;
+}
+
+/**
+ * dev_get_gen_pool - Obtain the gen_pool (if any) for a device
+ * @dev: device to retrieve the gen_pool from
+ * @name: Optional name for the gen_pool, usually NULL
+ *
+ * Returns the gen_pool for the device if one is present, or NULL.
+ */
+struct gen_pool *dev_get_gen_pool(struct device *dev)
+{
+ struct gen_pool **p = devres_find(dev, devm_gen_pool_release, NULL,
+ NULL);
+
+ if (!p)
+ return NULL;
+ return *p;
+}
+EXPORT_SYMBOL_GPL(dev_get_gen_pool);
+
+#ifdef CONFIG_OF
+/**
+ * of_get_named_gen_pool - find a pool by phandle property
+ * @np: device node
+ * @propname: property name containing phandle(s)
+ * @index: index into the phandle array
+ *
+ * Returns the pool that contains the chunk starting at the physical
+ * address of the device tree node pointed at by the phandle property,
+ * or NULL if not found.
+ */
+struct gen_pool *of_get_named_gen_pool(struct device_node *np,
+ const char *propname, int index)
+{
+ struct platform_device *pdev;
+ struct device_node *np_pool;
+
+ np_pool = of_parse_phandle(np, propname, index);
+ if (!np_pool)
+ return NULL;
+ pdev = of_find_device_by_node(np_pool);
+ if (!pdev)
+ return NULL;
+ return dev_get_gen_pool(&pdev->dev);
+}
+EXPORT_SYMBOL_GPL(of_get_named_gen_pool);
+#endif /* CONFIG_OF */
--
1.7.10.4

2013-03-20 10:53:47

[permalink] [raw]

Subject: [PATCH v9 RESEND 3/4] media: coda: use genalloc API

This patch depends on "genalloc: add devres support, allow to find
a managed pool by device", which provides the of_get_named_gen_pool
and dev_get_gen_pool functions.

Signed-off-by: Philipp Zabel <[email protected]>
Acked-By: Javier Martin <[email protected]>
Acked-by: Grant Likely <[email protected]>
---
Documentation/devicetree/bindings/media/coda.txt | 30 +++++++++++++++
drivers/media/platform/Kconfig | 1 -
drivers/media/platform/coda.c | 45 +++++++++++++++-------
include/linux/platform_data/coda.h | 18 +++++++++
4 files changed, 79 insertions(+), 15 deletions(-)
create mode 100644 Documentation/devicetree/bindings/media/coda.txt
create mode 100644 include/linux/platform_data/coda.h

diff --git a/Documentation/devicetree/bindings/media/coda.txt b/Documentation/devicetree/bindings/media/coda.txt
new file mode 100644
index 0000000..2865d04
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/coda.txt
@@ -0,0 +1,30 @@
+Chips&Media Coda multi-standard codec IP
+========================================
+
+Coda codec IPs are present in i.MX SoCs in various versions,
+called VPU (Video Processing Unit).
+
+Required properties:
+- compatible : should be "fsl,<chip>-src" for i.MX SoCs:
+ (a) "fsl,imx27-vpu" for CodaDx6 present in i.MX27
+ (b) "fsl,imx53-vpu" for CODA7541 present in i.MX53
+ (c) "fsl,imx6q-vpu" for CODA960 present in i.MX6q
+- reg: should be register base and length as documented in the
+ SoC reference manual
+- interrupts : Should contain the VPU interrupt. For CODA960,
+ a second interrupt is needed for the MJPEG unit.
+- clocks : Should contain the ahb and per clocks, in the order
+ determined by the clock-names property.
+- clock-names : Should be "ahb", "per"
+- iram : phandle pointing to the SRAM device node
+
+Example:
+
+vpu: vpu@63ff4000 {
+ compatible = "fsl,imx53-vpu";
+ reg = <0x63ff4000 0x1000>;
+ interrupts = <9>;
+ clocks = <&clks 63>, <&clks 63>;
+ clock-names = "ahb", "per";
+ iram = <&ocram>;
+};
diff --git a/drivers/media/platform/Kconfig b/drivers/media/platform/Kconfig
index 05d7b63..bbf83c1 100644
--- a/drivers/media/platform/Kconfig
+++ b/drivers/media/platform/Kconfig
@@ -145,7 +145,6 @@ config VIDEO_CODA
depends on VIDEO_DEV && VIDEO_V4L2 && ARCH_MXC
select VIDEOBUF2_DMA_CONTIG
select V4L2_MEM2MEM_DEV
- select IRAM_ALLOC if SOC_IMX53
---help---
Coda is a range of video codec IPs that supports
H.264, MPEG-4, and other video formats.
diff --git a/drivers/media/platform/coda.c b/drivers/media/platform/coda.c
index 20827ba..b931c2a 100644
--- a/drivers/media/platform/coda.c
+++ b/drivers/media/platform/coda.c
@@ -14,6 +14,7 @@
#include <linux/clk.h>
#include <linux/delay.h>
#include <linux/firmware.h>
+#include <linux/genalloc.h>
#include <linux/interrupt.h>
#include <linux/io.h>
#include <linux/irq.h>
@@ -23,7 +24,7 @@
#include <linux/slab.h>
#include <linux/videodev2.h>
#include <linux/of.h>
-#include <linux/platform_data/imx-iram.h>
+#include <linux/platform_data/coda.h>

#include <media/v4l2-ctrls.h>
#include <media/v4l2-device.h>
@@ -43,6 +44,7 @@
#define CODA7_WORK_BUF_SIZE (512 * 1024 + CODA_FMO_BUF_SIZE * 8 * 1024)
#define CODA_PARA_BUF_SIZE (10 * 1024)
#define CODA_ISRAM_SIZE (2048 * 2)
+#define CODADX6_IRAM_SIZE 0xb000
#define CODA7_IRAM_SIZE 0x14000 /* 81920 bytes */

#define CODA_MAX_FRAMEBUFFERS 2
@@ -128,7 +130,10 @@ struct coda_dev {

struct coda_aux_buf codebuf;
struct coda_aux_buf workbuf;
+ struct gen_pool *iram_pool;
+ long unsigned int iram_vaddr;
long unsigned int iram_paddr;
+ unsigned long iram_size;

spinlock_t irqlock;
struct mutex dev_mutex;
@@ -1926,6 +1931,9 @@ static int coda_probe(struct platform_device *pdev)
const struct of_device_id *of_id =
of_match_device(of_match_ptr(coda_dt_ids), &pdev->dev);
const struct platform_device_id *pdev_id;
+ struct coda_platform_data *pdata = pdev->dev.platform_data;
+ struct device_node *np = pdev->dev.of_node;
+ struct gen_pool *pool;
struct coda_dev *dev;
struct resource *res;
int ret, irq;
@@ -1988,6 +1996,16 @@ static int coda_probe(struct platform_device *pdev)
return -ENOENT;
}

+ /* Get IRAM pool from device tree or platform data */
+ pool = of_get_named_gen_pool(np, "iram", 0);
+ if (!pool && pdata)
+ pool = dev_get_gen_pool(pdata->iram_dev);
+ if (!pool) {
+ dev_err(&pdev->dev, "iram pool not available\n");
+ return -ENOMEM;
+ }
+ dev->iram_pool = pool;
+
ret = v4l2_device_register(&pdev->dev, &dev->v4l2_dev);
if (ret)
return ret;
@@ -2022,18 +2040,17 @@ static int coda_probe(struct platform_device *pdev)
return -ENOMEM;
}

- if (dev->devtype->product == CODA_DX6) {
- dev->iram_paddr = 0xffff4c00;
- } else {
- void __iomem *iram_vaddr;
-
- iram_vaddr = iram_alloc(CODA7_IRAM_SIZE,
- &dev->iram_paddr);
- if (!iram_vaddr) {
- dev_err(&pdev->dev, "unable to alloc iram\n");
- return -ENOMEM;
- }
+ if (dev->devtype->product == CODA_DX6)
+ dev->iram_size = CODADX6_IRAM_SIZE;
+ else
+ dev->iram_size = CODA7_IRAM_SIZE;
+ dev->iram_vaddr = gen_pool_alloc(dev->iram_pool, dev->iram_size);
+ if (!dev->iram_vaddr) {
+ dev_err(&pdev->dev, "unable to alloc iram\n");
+ return -ENOMEM;
}
+ dev->iram_paddr = gen_pool_virt_to_phys(dev->iram_pool,
+ dev->iram_vaddr);

platform_set_drvdata(pdev, dev);

@@ -2050,8 +2067,8 @@ static int coda_remove(struct platform_device *pdev)
if (dev->alloc_ctx)
vb2_dma_contig_cleanup_ctx(dev->alloc_ctx);
v4l2_device_unregister(&dev->v4l2_dev);
- if (dev->iram_paddr)
- iram_free(dev->iram_paddr, CODA7_IRAM_SIZE);
+ if (dev->iram_vaddr)
+ gen_pool_free(dev->iram_pool, dev->iram_vaddr, dev->iram_size);
if (dev->codebuf.vaddr)
dma_free_coherent(&pdev->dev, dev->codebuf.size,
&dev->codebuf.vaddr, dev->codebuf.paddr);
diff --git a/include/linux/platform_data/coda.h b/include/linux/platform_data/coda.h
new file mode 100644
index 0000000..6ad4410
--- /dev/null
+++ b/include/linux/platform_data/coda.h
@@ -0,0 +1,18 @@
+/*
+ * Copyright (C) 2013 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.
+ */
+#ifndef PLATFORM_CODA_H
+#define PLATFORM_CODA_H
+
+struct device;
+
+struct coda_platform_data {
+ struct device *iram_dev;
+};
+
+#endif
--
1.7.10.4

2013-03-20 10:54:01

[permalink] [raw]

Subject: [PATCH v9 RESEND 2/4] misc: Generic on-chip SRAM allocation driver

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

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

The allocation granularity is hard-coded to 32 bytes for now,
to make the SRAM driver useful for the 6502 remoteproc driver.
There is overhead for bigger SRAMs, where only a much coarser
allocation granularity is needed: At 32 bytes minimum allocation
size, a 256 KiB SRAM needs a 1 KiB bitmap to track allocations.

Signed-off-by: Philipp Zabel <[email protected]>
Reviewed-by: Shawn Guo <[email protected]>
Acked-by: Grant Likely <[email protected]>
---
Changes since v8:
- Changed device tree compatible string to "mmio-sram"
---
Documentation/devicetree/bindings/misc/sram.txt | 16 +++
drivers/misc/Kconfig | 9 ++
drivers/misc/Makefile | 1 +
drivers/misc/sram.c | 121 +++++++++++++++++++++++
4 files changed, 147 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..4d0a00e
--- /dev/null
+++ b/Documentation/devicetree/bindings/misc/sram.txt
@@ -0,0 +1,16 @@
+Generic on-chip SRAM
+
+Simple IO memory regions to be managed by the genalloc API.
+
+Required properties:
+
+- compatible : mmio-sram
+
+- reg : SRAM iomem address range
+
+Example:
+
+sram: sram@5c000000 {
+ compatible = "mmio-sram";
+ reg = <0x5c000000 0x40000>; /* 256 KiB SRAM at address 0x5c000000 */
+};
diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
index e83fdfe..4878507 100644
--- a/drivers/misc/Kconfig
+++ b/drivers/misc/Kconfig
@@ -510,6 +510,15 @@ config LATTICE_ECP3_CONFIG

If unsure, say N.

+config SRAM
+ bool "Generic on-chip SRAM driver"
+ 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 SoCs.
+
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 35a1463..08e2007 100644
--- a/drivers/misc/Makefile
+++ b/drivers/misc/Makefile
@@ -52,3 +52,4 @@ obj-$(CONFIG_INTEL_MEI) += mei/
obj-$(CONFIG_MAX8997_MUIC) += max8997-muic.o
obj-$(CONFIG_VMWARE_VMCI) += vmw_vmci/
obj-$(CONFIG_LATTICE_ECP3_CONFIG) += lattice-ecp3-config.o
+obj-$(CONFIG_SRAM) += sram.o
diff --git a/drivers/misc/sram.c b/drivers/misc/sram.c
new file mode 100644
index 0000000..7858c62
--- /dev/null
+++ b/drivers/misc/sram.c
@@ -0,0 +1,121 @@
+/*
+ * 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/init.h>
+#include <linux/clk.h>
+#include <linux/err.h>
+#include <linux/io.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
+#include <linux/spinlock.h>
+#include <linux/genalloc.h>
+
+#define SRAM_GRANULARITY 32
+
+struct sram_dev {
+ struct gen_pool *pool;
+ struct clk *clk;
+};
+
+static int 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->clk = devm_clk_get(&pdev->dev, NULL);
+ if (IS_ERR(sram->clk))
+ sram->clk = NULL;
+ else
+ clk_prepare_enable(sram->clk);
+
+ sram->pool = devm_gen_pool_create(&pdev->dev, ilog2(SRAM_GRANULARITY), -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 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);
+
+ if (sram->clk)
+ clk_disable_unprepare(sram->clk);
+
+ return 0;
+}
+
+#ifdef CONFIG_OF
+static struct of_device_id sram_dt_ids[] = {
+ { .compatible = "mmio-sram" },
+ {}
+};
+#endif
+
+static struct platform_driver sram_driver = {
+ .driver = {
+ .name = "sram",
+ .of_match_table = of_match_ptr(sram_dt_ids),
+ },
+ .probe = sram_probe,
+ .remove = sram_remove,
+};
+
+int __init sram_init(void)
+{
+ return platform_driver_register(&sram_driver);
+}
+
+postcore_initcall(sram_init);
--
1.7.10.4

2013-03-22 12:47:26

[permalink] [raw]

Subject: Re: [PATCH v9 RESEND 1/4] genalloc: add devres support, allow to find a managed pool by device

Hi,

2013/3/20 Philipp Zabel <[email protected]>:
> This patch adds three exported functions to lib/genalloc.c:
> devm_gen_pool_create, dev_get_gen_pool, and of_get_named_gen_pool.
>
> devm_gen_pool_create is a managed version of gen_pool_create that keeps
> track of the pool via devres and allows the management code to automatically
> destroy it after device removal.
>
> dev_get_gen_pool retrieves the gen_pool for a given device, if it was
> created with devm_gen_pool_create, using devres_find.
>
> of_get_named_gen_pool retrieves the gen_pool for a given device node and
> property name, where the property must contain a phandle pointing to a
> platform device node. The corresponding platform device is then fed into
> dev_get_gen_pool and the resulting gen_pool is returned.
>
> Signed-off-by: Philipp Zabel <[email protected]>
> Acked-by: Grant Likely <[email protected]>
> ---
> include/linux/genalloc.h | 15 +++++++++
> lib/genalloc.c | 81 ++++++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 96 insertions(+)
>
> diff --git a/include/linux/genalloc.h b/include/linux/genalloc.h
> index dd7c569..383e8f4 100644
> --- a/include/linux/genalloc.h
> +++ b/include/linux/genalloc.h
> @@ -105,4 +105,19 @@ extern unsigned long gen_pool_first_fit(unsigned long *map, unsigned long size,
> extern unsigned long gen_pool_best_fit(unsigned long *map, unsigned long size,
> unsigned long start, unsigned int nr, void *data);
>
> +extern struct gen_pool *devm_gen_pool_create(struct device *dev,
> + int min_alloc_order, int nid);
> +extern struct gen_pool *dev_get_gen_pool(struct device *dev);
> +
> +struct device_node;
> +#ifdef CONFIG_OF
> +extern struct gen_pool *of_get_named_gen_pool(struct device_node *np,
> + const char *propname, int index);
> +#else
> +inline struct gen_pool *of_get_named_gen_pool(struct device_node *np,
> + const char *propname, int index)
> +{
> + return NULL;
> +}
> +#endif
> #endif /* __GENALLOC_H__ */
> diff --git a/lib/genalloc.c b/lib/genalloc.c
> index 5492043..b35cfa9 100644
> --- a/lib/genalloc.c
> +++ b/lib/genalloc.c
> @@ -34,6 +34,8 @@
> #include <linux/rculist.h>
> #include <linux/interrupt.h>
> #include <linux/genalloc.h>
> +#include <linux/of_address.h>
> +#include <linux/of_device.h>
>
> static int set_bits_ll(unsigned long *addr, unsigned long mask_to_set)
> {
> @@ -480,3 +482,82 @@ unsigned long gen_pool_best_fit(unsigned long *map, unsigned long size,
> return start_bit;
> }
> EXPORT_SYMBOL(gen_pool_best_fit);
> +
> +static void devm_gen_pool_release(struct device *dev, void *res)
> +{
> + gen_pool_destroy(*(struct gen_pool **)res);
> +}
> +
> +/**
> + * devm_gen_pool_create - managed gen_pool_create
> + * @dev: device that provides the gen_pool
> + * @min_alloc_order: log base 2 of number of bytes each bitmap bit represents
> + * @nid: node id of the node the pool structure should be allocated on, or -1
> + *
> + * Create a new special memory pool that can be used to manage special purpose
> + * memory not managed by the regular kmalloc/kfree interface. The pool will be
> + * automatically destroyed by the device management code.
> + */
> +struct gen_pool *devm_gen_pool_create(struct device *dev, int min_alloc_order,
> + int nid)
> +{
> + struct gen_pool **ptr, *pool;
> +
> + ptr = devres_alloc(devm_gen_pool_release, sizeof(*ptr), GFP_KERNEL);
> +
> + pool = gen_pool_create(min_alloc_order, nid);
> + if (pool) {
> + *ptr = pool;
> + devres_add(dev, ptr);
> + } else {
> + devres_free(ptr);
> + }
> +
> + return pool;
> +}
> +
> +/**
> + * dev_get_gen_pool - Obtain the gen_pool (if any) for a device
> + * @dev: device to retrieve the gen_pool from
> + * @name: Optional name for the gen_pool, usually NULL
> + *
> + * Returns the gen_pool for the device if one is present, or NULL.
> + */
> +struct gen_pool *dev_get_gen_pool(struct device *dev)
> +{
> + struct gen_pool **p = devres_find(dev, devm_gen_pool_release, NULL,
> + NULL);
> +
> + if (!p)
> + return NULL;
> + return *p;
> +}
> +EXPORT_SYMBOL_GPL(dev_get_gen_pool);
> +
> +#ifdef CONFIG_OF
> +/**
> + * of_get_named_gen_pool - find a pool by phandle property
> + * @np: device node
> + * @propname: property name containing phandle(s)
> + * @index: index into the phandle array
> + *
> + * Returns the pool that contains the chunk starting at the physical
> + * address of the device tree node pointed at by the phandle property,
> + * or NULL if not found.
> + */
> +struct gen_pool *of_get_named_gen_pool(struct device_node *np,
> + const char *propname, int index)
> +{
> + struct platform_device *pdev;
> + struct device_node *np_pool;
> +
> + np_pool = of_parse_phandle(np, propname, index);
> + if (!np_pool)
> + return NULL;
> + pdev = of_find_device_by_node(np_pool);
> + if (!pdev)
> + return NULL;
> + return dev_get_gen_pool(&pdev->dev);
> +}
> +EXPORT_SYMBOL_GPL(of_get_named_gen_pool);
> +#endif /* CONFIG_OF */
> --
> 1.7.10.4

Tested on xilinx-zynq with OCM.

Tested-by: Michal Simek <[email protected]>

Thanks,
Michal

--
Michal Simek, Ing. (M.Eng)
w: http://www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/
Maintainer of Linux kernel - Xilinx Zynq ARM architecture
Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform

2013-03-22 12:47:48

[permalink] [raw]

Subject: Re: [PATCH v9 RESEND 2/4] misc: Generic on-chip SRAM allocation driver

2013/3/20 Philipp Zabel <[email protected]>:
> This driver requests and remaps a memory region as configured in the
> device tree. It serves memory from this region via the genalloc API.
> It optionally enables the SRAM clock.
>
> Other drivers can retrieve the genalloc pool from a phandle pointing
> to this drivers' device node in the device tree.
>
> The allocation granularity is hard-coded to 32 bytes for now,
> to make the SRAM driver useful for the 6502 remoteproc driver.
> There is overhead for bigger SRAMs, where only a much coarser
> allocation granularity is needed: At 32 bytes minimum allocation
> size, a 256 KiB SRAM needs a 1 KiB bitmap to track allocations.
>
> Signed-off-by: Philipp Zabel <[email protected]>
> Reviewed-by: Shawn Guo <[email protected]>
> Acked-by: Grant Likely <[email protected]>
> ---
> Changes since v8:
> - Changed device tree compatible string to "mmio-sram"
> ---
> Documentation/devicetree/bindings/misc/sram.txt | 16 +++
> drivers/misc/Kconfig | 9 ++
> drivers/misc/Makefile | 1 +
> drivers/misc/sram.c | 121 +++++++++++++++++++++++
> 4 files changed, 147 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..4d0a00e
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/misc/sram.txt
> @@ -0,0 +1,16 @@
> +Generic on-chip SRAM
> +
> +Simple IO memory regions to be managed by the genalloc API.
> +
> +Required properties:
> +
> +- compatible : mmio-sram
> +
> +- reg : SRAM iomem address range
> +
> +Example:
> +
> +sram: sram@5c000000 {
> + compatible = "mmio-sram";
> + reg = <0x5c000000 0x40000>; /* 256 KiB SRAM at address 0x5c000000 */
> +};
> diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
> index e83fdfe..4878507 100644
> --- a/drivers/misc/Kconfig
> +++ b/drivers/misc/Kconfig
> @@ -510,6 +510,15 @@ config LATTICE_ECP3_CONFIG
>
> If unsure, say N.
>
> +config SRAM
> + bool "Generic on-chip SRAM driver"
> + 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 SoCs.
> +
> 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 35a1463..08e2007 100644
> --- a/drivers/misc/Makefile
> +++ b/drivers/misc/Makefile
> @@ -52,3 +52,4 @@ obj-$(CONFIG_INTEL_MEI) += mei/
> obj-$(CONFIG_MAX8997_MUIC) += max8997-muic.o
> obj-$(CONFIG_VMWARE_VMCI) += vmw_vmci/
> obj-$(CONFIG_LATTICE_ECP3_CONFIG) += lattice-ecp3-config.o
> +obj-$(CONFIG_SRAM) += sram.o
> diff --git a/drivers/misc/sram.c b/drivers/misc/sram.c
> new file mode 100644
> index 0000000..7858c62
> --- /dev/null
> +++ b/drivers/misc/sram.c
> @@ -0,0 +1,121 @@
> +/*
> + * 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/init.h>
> +#include <linux/clk.h>
> +#include <linux/err.h>
> +#include <linux/io.h>
> +#include <linux/of.h>
> +#include <linux/platform_device.h>
> +#include <linux/slab.h>
> +#include <linux/spinlock.h>
> +#include <linux/genalloc.h>
> +
> +#define SRAM_GRANULARITY 32
> +
> +struct sram_dev {
> + struct gen_pool *pool;
> + struct clk *clk;
> +};
> +
> +static int 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->clk = devm_clk_get(&pdev->dev, NULL);
> + if (IS_ERR(sram->clk))
> + sram->clk = NULL;
> + else
> + clk_prepare_enable(sram->clk);
> +
> + sram->pool = devm_gen_pool_create(&pdev->dev, ilog2(SRAM_GRANULARITY), -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 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);
> +
> + if (sram->clk)
> + clk_disable_unprepare(sram->clk);
> +
> + return 0;
> +}
> +
> +#ifdef CONFIG_OF
> +static struct of_device_id sram_dt_ids[] = {
> + { .compatible = "mmio-sram" },
> + {}
> +};
> +#endif
> +
> +static struct platform_driver sram_driver = {
> + .driver = {
> + .name = "sram",
> + .of_match_table = of_match_ptr(sram_dt_ids),
> + },
> + .probe = sram_probe,
> + .remove = sram_remove,
> +};
> +
> +int __init sram_init(void)
> +{
> + return platform_driver_register(&sram_driver);
> +}
> +
> +postcore_initcall(sram_init);
> --
> 1.7.10.4

Tested on xilinx-zynq with OCM.

Tested-by: Michal Simek <[email protected]>

Thanks,
Michal

--
Michal Simek, Ing. (M.Eng)
w: http://www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/
Maintainer of Linux kernel - Xilinx Zynq ARM architecture
Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform

2013-03-22 12:49:09

[permalink] [raw]

Subject: Re: [PATCH v9 RESEND 0/4] Add generic driver for on-chip SRAM

Hi Andrew,

2013/3/20 Philipp Zabel <[email protected]>:
> Hi, last time I posted was a bit close to the merge window, so I'm
> reposting now. Greg, Arnd, could you take the first two patches?
>
> These patches add support to configure on-chip SRAM via device-tree
> node or platform data and to obtain the resulting genalloc pool from
> the struct device pointer or a phandle pointing at the device tree node.
> This allows drivers to allocate SRAM with the genalloc API without
> hard-coding the genalloc pool pointer.
>
> The on-chip SRAM on i.MX53 and i.MX6q can be registered via device tree
> and changed to use the simple generic SRAM driver:
>
> ocram: ocram@00900000 {
> compatible = "fsl,imx-ocram", "mmio-sram";
> reg = <0x00900000 0x3f000>;
> };
>
> A driver that needs to allocate SRAM buffers, like the video processing
> unit on i.MX53, can retrieve the genalloc pool from a phandle in the
> device tree using of_get_named_gen_pool(node, "iram", 0) from patch 1:
>
> vpu@63ff4000 {
> /* ... */
> iram = <&ocram>;
> };
>
> Changes since v8:
> - The sram driver now matches against the "mmio-sram" compatible string.
> - Removed a whitespace error in the device tree binding documentation.
>
> regards
> Philipp
>
> ---
> Documentation/devicetree/bindings/media/coda.txt | 30 ++++++
> Documentation/devicetree/bindings/misc/sram.txt | 16 +++
> arch/arm/boot/dts/imx53.dtsi | 5 +
> arch/arm/boot/dts/imx6q.dtsi | 6 ++
> drivers/media/platform/Kconfig | 1 -
> drivers/media/platform/coda.c | 45 +++++---
> drivers/misc/Kconfig | 9 ++
> drivers/misc/Makefile | 1 +
> drivers/misc/sram.c | 121 ++++++++++++++++++++++
> include/linux/genalloc.h | 15 +++
> include/linux/platform_data/coda.h | 18 ++++
> lib/genalloc.c | 81 +++++++++++++++
> 12 files changed, 333 insertions(+), 15 deletions(-)

Andrew: 1/4 and 2/4 patches should probably go through your tree.
Can you please handle them?
Or Philipp should send these two patches directly to you?

Thanks,
Michal

--
Michal Simek, Ing. (M.Eng)
w: http://www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/
Maintainer of Linux kernel - Xilinx Zynq ARM architecture
Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform

2013-03-27 08:33:59

[permalink] [raw]

Subject: Re: [PATCH v9 RESEND 0/4] Add generic driver for on-chip SRAM

Am Mittwoch, den 20.03.2013, 11:52 +0100 schrieb Philipp Zabel:
> Hi, last time I posted was a bit close to the merge window, so I'm
> reposting now. Greg, Arnd, could you take the first two patches?

Ping,

can I do anything to help move this along?

regards
Philipp

> These patches add support to configure on-chip SRAM via device-tree
> node or platform data and to obtain the resulting genalloc pool from
> the struct device pointer or a phandle pointing at the device tree node.
> This allows drivers to allocate SRAM with the genalloc API without
> hard-coding the genalloc pool pointer.
>
> The on-chip SRAM on i.MX53 and i.MX6q can be registered via device tree
> and changed to use the simple generic SRAM driver:
>
> ocram: ocram@00900000 {
> compatible = "fsl,imx-ocram", "mmio-sram";
> reg = <0x00900000 0x3f000>;
> };
>
> A driver that needs to allocate SRAM buffers, like the video processing
> unit on i.MX53, can retrieve the genalloc pool from a phandle in the
> device tree using of_get_named_gen_pool(node, "iram", 0) from patch 1:
>
> vpu@63ff4000 {
> /* ... */
> iram = <&ocram>;
> };
>
> Changes since v8:
> - The sram driver now matches against the "mmio-sram" compatible string.
> - Removed a whitespace error in the device tree binding documentation.
>
> regards
> Philipp
>
> ---
> Documentation/devicetree/bindings/media/coda.txt | 30 ++++++
> Documentation/devicetree/bindings/misc/sram.txt | 16 +++
> arch/arm/boot/dts/imx53.dtsi | 5 +
> arch/arm/boot/dts/imx6q.dtsi | 6 ++
> drivers/media/platform/Kconfig | 1 -
> drivers/media/platform/coda.c | 45 +++++---
> drivers/misc/Kconfig | 9 ++
> drivers/misc/Makefile | 1 +
> drivers/misc/sram.c | 121 ++++++++++++++++++++++
> include/linux/genalloc.h | 15 +++
> include/linux/platform_data/coda.h | 18 ++++
> lib/genalloc.c | 81 +++++++++++++++
> 12 files changed, 333 insertions(+), 15 deletions(-)
>
>

2013-03-27 12:00:35

[permalink] [raw]

Subject: Re: [PATCH v9 RESEND 0/4] Add generic driver for on-chip SRAM

On Wed, Mar 27, 2013 at 09:33:34AM +0100, Philipp Zabel wrote:
> Am Mittwoch, den 20.03.2013, 11:52 +0100 schrieb Philipp Zabel:
> > Hi, last time I posted was a bit close to the merge window, so I'm
> > reposting now. Greg, Arnd, could you take the first two patches?
>
> Ping,
>
> can I do anything to help move this along?

You haven't indicated how the series should be merged and by whom. I
think we should have the first two patches get in through either
Andrew's tree or Greg's tree, since Andrew is handling genalloc and Greg
is handling drivers/misc.

Greg, can you please give your ACK on the second patch to have the first
two go via Andrew's tree? Otherwise, we can try to ask for Andrew's
ACK on the first patch to have them go through yours. Please tell.

Shawn

2013-03-27 22:27:27

by Andrew Morton

[permalink] [raw]

Subject: Re: [PATCH v9 RESEND 2/4] misc: Generic on-chip SRAM allocation driver

On Wed, 20 Mar 2013 11:52:45 +0100 Philipp Zabel <[email protected]> wrote:

> This driver requests and remaps a memory region as configured in the
> device tree. It serves memory from this region via the genalloc API.
> It optionally enables the SRAM clock.
>
> Other drivers can retrieve the genalloc pool from a phandle pointing
> to this drivers' device node in the device tree.
>
> The allocation granularity is hard-coded to 32 bytes for now,
> to make the SRAM driver useful for the 6502 remoteproc driver.
> There is overhead for bigger SRAMs, where only a much coarser
> allocation granularity is needed: At 32 bytes minimum allocation
> size, a 256 KiB SRAM needs a 1 KiB bitmap to track allocations.
>
> Documentation/devicetree/bindings/misc/sram.txt | 16 +++
> drivers/misc/Kconfig | 9 ++
> drivers/misc/Makefile | 1 +
> drivers/misc/sram.c | 121 +++++++++++++++++++++++

drivers/misc/sram.c is a pretty generic-sounding thing. Is it really
Linux's One True SRAM driver? How many different sorts of sram devices
do we expect this can be used with? If I don't use DT?

In other words, perhaps this should have a more specific and accurate
name?

> --- a/drivers/misc/Kconfig
> +++ b/drivers/misc/Kconfig
> @@ -510,6 +510,15 @@ config LATTICE_ECP3_CONFIG
>
> If unsure, say N.
>
> ...
>
> +static int 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;

EADDRNOTAVAIL is a networking error. If your users see this error pop
up on their console they'll start wiggling ethernet cables, wondering
why that didn't fix it.

> + sram = devm_kzalloc(&pdev->dev, sizeof(*sram), GFP_KERNEL);
> + if (!sram)
> + return -ENOMEM;
> +
> + sram->clk = devm_clk_get(&pdev->dev, NULL);
> + if (IS_ERR(sram->clk))
> + sram->clk = NULL;
> + else
> + clk_prepare_enable(sram->clk);
> +
> + sram->pool = devm_gen_pool_create(&pdev->dev, ilog2(SRAM_GRANULARITY), -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;
> +}
>
> ...
>
> +int __init sram_init(void)
> +{
> + return platform_driver_register(&sram_driver);
> +}
> +
> +postcore_initcall(sram_init);

Why is it postcore_initcall()?

Fixlets:

From: Andrew Morton <[email protected]>
Subject: misc-generic-on-chip-sram-allocation-driver-fix

fix Kconfig text, make sram_init static

Cc: Dong Aisheng <[email protected]>
Cc: Fabio Estevam <[email protected]>
Cc: Grant Likely <[email protected]>
Cc: Greg Kroah-Hartman <[email protected]>
Cc: Huang Shijie <[email protected]>
Cc: Javier Martin <[email protected]>
Cc: Matt Porter <[email protected]>
Cc: Michal Simek <[email protected]>
Cc: Paul Gortmaker <[email protected]>
Cc: Philipp Zabel <[email protected]>
Cc: Rob Herring <[email protected]>
Cc: Shawn Guo <[email protected]>
Signed-off-by: Andrew Morton <[email protected]>
---

drivers/misc/Kconfig | 6 +++---
drivers/misc/sram.c | 2 +-
2 files changed, 4 insertions(+), 4 deletions(-)

diff -puN Documentation/devicetree/bindings/misc/sram.txt~misc-generic-on-chip-sram-allocation-driver-fix Documentation/devicetree/bindings/misc/sram.txt
diff -puN drivers/misc/Kconfig~misc-generic-on-chip-sram-allocation-driver-fix drivers/misc/Kconfig
--- a/drivers/misc/Kconfig~misc-generic-on-chip-sram-allocation-driver-fix
+++ a/drivers/misc/Kconfig
@@ -523,9 +523,9 @@ config SRAM
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 SoCs.
+ This driver allows you 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 SoCs.

source "drivers/misc/c2port/Kconfig"
source "drivers/misc/eeprom/Kconfig"
diff -puN drivers/misc/Makefile~misc-generic-on-chip-sram-allocation-driver-fix drivers/misc/Makefile
diff -puN drivers/misc/sram.c~misc-generic-on-chip-sram-allocation-driver-fix drivers/misc/sram.c
--- a/drivers/misc/sram.c~misc-generic-on-chip-sram-allocation-driver-fix
+++ a/drivers/misc/sram.c
@@ -113,7 +113,7 @@ static struct platform_driver sram_drive
.remove = sram_remove,
};

-int __init sram_init(void)
+static int __init sram_init(void)
{
return platform_driver_register(&sram_driver);
}
_

2013-03-27 22:29:13

by Andrew Morton

[permalink] [raw]

Subject: Re: [PATCH v9 RESEND 4/4] ARM: dts: add sram for imx53 and imx6q

On Wed, 20 Mar 2013 11:52:47 +0100 Philipp Zabel <[email protected]> wrote:

> Signed-off-by: Philipp Zabel <[email protected]>
> Reviewed-by: Shawn Guo <[email protected]>
> Acked-by: Grant Likely <[email protected]>
> ---
> Changes since v8:
> - Changed device tree compatible string to "mmio-sram"
> ---
> arch/arm/boot/dts/imx53.dtsi | 5 +++++
> arch/arm/boot/dts/imx6q.dtsi | 6 ++++++

This doesn't apply, due to stuff which is pending in linux-next. I had
a go at fixing it but ran out of confidence - those files have changed
significantly.

Redo and resend, please?

2013-03-28 01:15:26

[permalink] [raw]

Subject: Re: [PATCH v9 RESEND 3/4] media: coda: use genalloc API

On Wed, Mar 20, 2013 at 11:52:46AM +0100, Philipp Zabel wrote:
> This patch depends on "genalloc: add devres support, allow to find
> a managed pool by device", which provides the of_get_named_gen_pool
> and dev_get_gen_pool functions.
>
> Signed-off-by: Philipp Zabel <[email protected]>
> Acked-By: Javier Martin <[email protected]>
> Acked-by: Grant Likely <[email protected]>
> ---
> Documentation/devicetree/bindings/media/coda.txt | 30 +++++++++++++++
> drivers/media/platform/Kconfig | 1 -
> drivers/media/platform/coda.c | 45 +++++++++++++++-------
> include/linux/platform_data/coda.h | 18 +++++++++
> 4 files changed, 79 insertions(+), 15 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/media/coda.txt
> create mode 100644 include/linux/platform_data/coda.h
>
> diff --git a/Documentation/devicetree/bindings/media/coda.txt b/Documentation/devicetree/bindings/media/coda.txt
> new file mode 100644
> index 0000000..2865d04
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/coda.txt
> @@ -0,0 +1,30 @@
> +Chips&Media Coda multi-standard codec IP
> +========================================
> +
> +Coda codec IPs are present in i.MX SoCs in various versions,
> +called VPU (Video Processing Unit).
> +
> +Required properties:
> +- compatible : should be "fsl,<chip>-src" for i.MX SoCs:

fsl,<chip>-vpu

Shawn

> + (a) "fsl,imx27-vpu" for CodaDx6 present in i.MX27
> + (b) "fsl,imx53-vpu" for CODA7541 present in i.MX53
> + (c) "fsl,imx6q-vpu" for CODA960 present in i.MX6q

2013-03-28 01:16:50

[permalink] [raw]

Subject: Re: [PATCH v9 RESEND 4/4] ARM: dts: add sram for imx53 and imx6q

On Wed, Mar 27, 2013 at 03:29:10PM -0700, Andrew Morton wrote:
> On Wed, 20 Mar 2013 11:52:47 +0100 Philipp Zabel <[email protected]> wrote:
>
> > Signed-off-by: Philipp Zabel <[email protected]>
> > Reviewed-by: Shawn Guo <[email protected]>
> > Acked-by: Grant Likely <[email protected]>
> > ---
> > Changes since v8:
> > - Changed device tree compatible string to "mmio-sram"
> > ---
> > arch/arm/boot/dts/imx53.dtsi | 5 +++++
> > arch/arm/boot/dts/imx6q.dtsi | 6 ++++++
>
> This doesn't apply, due to stuff which is pending in linux-next. I had
> a go at fixing it but ran out of confidence - those files have changed
> significantly.
>
> Redo and resend, please?

We will merge this patch via IMX tree.

Shawn

2013-03-28 02:54:44

[permalink] [raw]

Subject: Re: [PATCH v9 RESEND 4/4] ARM: dts: add sram for imx53 and imx6q

On Wed, Mar 20, 2013 at 11:52:47AM +0100, Philipp Zabel wrote:
> Signed-off-by: Philipp Zabel <[email protected]>
> Reviewed-by: Shawn Guo <[email protected]>
> Acked-by: Grant Likely <[email protected]>
> ---
> Changes since v8:
> - Changed device tree compatible string to "mmio-sram"
> ---
> arch/arm/boot/dts/imx53.dtsi | 5 +++++
> arch/arm/boot/dts/imx6q.dtsi | 6 ++++++
> 2 files changed, 11 insertions(+)
>
> diff --git a/arch/arm/boot/dts/imx53.dtsi b/arch/arm/boot/dts/imx53.dtsi
> index edc3f1e..69d0680 100644
> --- a/arch/arm/boot/dts/imx53.dtsi
> +++ b/arch/arm/boot/dts/imx53.dtsi
> @@ -664,5 +664,10 @@
> status = "disabled";
> };
> };
> +
> + ocram: ocram@f8000000 {
> + compatible = "fsl,imx-ocram", "mmio-sram";

We should probably just drop "fsl,imx-ocram".

Shawn

> + reg = <0xf8000000 0x20000>;
> + };
> };
> };
> diff --git a/arch/arm/boot/dts/imx6q.dtsi b/arch/arm/boot/dts/imx6q.dtsi
> index ff1205e..73302db 100644
> --- a/arch/arm/boot/dts/imx6q.dtsi
> +++ b/arch/arm/boot/dts/imx6q.dtsi
> @@ -124,6 +124,12 @@
> status = "disabled";
> };
>
> + ocram: ocram@00900000 {
> + compatible = "fsl,imx-ocram", "mmio-sram";
> + reg = <0x00900000 0x3f000>;
> + clocks = <&clks 142>;
> + };
> +
> timer@00a00600 {
> compatible = "arm,cortex-a9-twd-timer";
> reg = <0x00a00600 0x20>;
> --
> 1.7.10.4
>

2013-03-28 07:42:54

[permalink] [raw]

Subject: Re: [PATCH v9 RESEND 2/4] misc: Generic on-chip SRAM allocation driver

Hi Andrew,

2013/3/27 Andrew Morton <[email protected]>:
> On Wed, 20 Mar 2013 11:52:45 +0100 Philipp Zabel <[email protected]> wrote:
>
>> This driver requests and remaps a memory region as configured in the
>> device tree. It serves memory from this region via the genalloc API.
>> It optionally enables the SRAM clock.
>>
>> Other drivers can retrieve the genalloc pool from a phandle pointing
>> to this drivers' device node in the device tree.
>>
>> The allocation granularity is hard-coded to 32 bytes for now,
>> to make the SRAM driver useful for the 6502 remoteproc driver.
>> There is overhead for bigger SRAMs, where only a much coarser
>> allocation granularity is needed: At 32 bytes minimum allocation
>> size, a 256 KiB SRAM needs a 1 KiB bitmap to track allocations.
>>
>> Documentation/devicetree/bindings/misc/sram.txt | 16 +++
>> drivers/misc/Kconfig | 9 ++
>> drivers/misc/Makefile | 1 +
>> drivers/misc/sram.c | 121 +++++++++++++++++++++++
>
> drivers/misc/sram.c is a pretty generic-sounding thing. Is it really
> Linux's One True SRAM driver? How many different sorts of sram devices
> do we expect this can be used with? If I don't use DT?

I want to use it for xilinx microblaze BRAM and zynq OCM and BRAMS connected
to buses on Microblaze, PPC and ARM zynq.

I have there just one small problem with OCM because we have a suspend code
which is copied to it and execute from it and current implementation
has no handling for it because all memory you get is not executable.

I am not sure how to handle this properly. Currently I am calling
gen_pool_alloc which returns virtual address, then gen_pool_virt_to_phys to
get physical address and then __arm_ioremap with MT_DEVICE .
This works but it looks ugly. And also this is not generic solution
because it doesn't work on Microblaze.

Is there any other nice way how to ask for executable memory?

Thanks,
Michal

--
Michal Simek, Ing. (M.Eng)
w: http://www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/
Maintainer of Linux kernel - Xilinx Zynq ARM architecture
Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform

2013-03-28 09:05:44

[permalink] [raw]

Subject: Re: [PATCH v9 RESEND 4/4] ARM: dts: add sram for imx53 and imx6q

Am Donnerstag, den 28.03.2013, 10:54 +0800 schrieb Shawn Guo:
> On Wed, Mar 20, 2013 at 11:52:47AM +0100, Philipp Zabel wrote:
> > Signed-off-by: Philipp Zabel <[email protected]>
> > Reviewed-by: Shawn Guo <[email protected]>
> > Acked-by: Grant Likely <[email protected]>
> > ---
> > Changes since v8:
> > - Changed device tree compatible string to "mmio-sram"
> > ---
> > arch/arm/boot/dts/imx53.dtsi | 5 +++++
> > arch/arm/boot/dts/imx6q.dtsi | 6 ++++++
> > 2 files changed, 11 insertions(+)
> >
> > diff --git a/arch/arm/boot/dts/imx53.dtsi b/arch/arm/boot/dts/imx53.dtsi
> > index edc3f1e..69d0680 100644
> > --- a/arch/arm/boot/dts/imx53.dtsi
> > +++ b/arch/arm/boot/dts/imx53.dtsi
> > @@ -664,5 +664,10 @@
> > status = "disabled";
> > };
> > };
> > +
> > + ocram: ocram@f8000000 {
> > + compatible = "fsl,imx-ocram", "mmio-sram";
>
> We should probably just drop "fsl,imx-ocram".
>
> Shawn

I thought that in the future somebody might want to implement some i.MX
OCRAM specifics in a different driver, like the L2 cache / OCRAM mode
switching, the timing settings, or the TrustZone bits that are
configurable in the IOMUXC GPR registers.

regards
Philipp

2013-03-28 10:52:55

[permalink] [raw]

Subject: Re: [PATCH v9 RESEND 2/4] misc: Generic on-chip SRAM allocation driver

Hi Andrew,

thanks for taking care of these patches.

Am Mittwoch, den 27.03.2013, 15:27 -0700 schrieb Andrew Morton:
> On Wed, 20 Mar 2013 11:52:45 +0100 Philipp Zabel <[email protected]> wrote:
>
> > This driver requests and remaps a memory region as configured in the
> > device tree. It serves memory from this region via the genalloc API.
> > It optionally enables the SRAM clock.
> >
> > Other drivers can retrieve the genalloc pool from a phandle pointing
> > to this drivers' device node in the device tree.
> >
> > The allocation granularity is hard-coded to 32 bytes for now,
> > to make the SRAM driver useful for the 6502 remoteproc driver.
> > There is overhead for bigger SRAMs, where only a much coarser
> > allocation granularity is needed: At 32 bytes minimum allocation
> > size, a 256 KiB SRAM needs a 1 KiB bitmap to track allocations.
> >
> > Documentation/devicetree/bindings/misc/sram.txt | 16 +++
> > drivers/misc/Kconfig | 9 ++
> > drivers/misc/Makefile | 1 +
> > drivers/misc/sram.c | 121 +++++++++++++++++++++++
>
> drivers/misc/sram.c is a pretty generic-sounding thing. Is it really
> Linux's One True SRAM driver? How many different sorts of sram devices
> do we expect this can be used with? If I don't use DT?

It's just a driver for MMIO-accessible SRAM ranges that are provided to
other drivers (or suspend or dvfs platform code) via a genalloc pool.
This is for system use, as opposed to an SRAM mtd device exporting SRAM
to userspace. It's intended to unify all current SRAM genalloc pools in
use. DT support is completely optional, the SRAM driver just uses
platform resources.

One limitation is that currently the allocation granularity is not
configurable, that should be added to the driver.

> In other words, perhaps this should have a more specific and accurate
> name?

Grant already made me change the compatible string to "mmio-sram". We
could change the driver name to mmio_sram, too.

> > --- a/drivers/misc/Kconfig
> > +++ b/drivers/misc/Kconfig
> > @@ -510,6 +510,15 @@ config LATTICE_ECP3_CONFIG
> >
> > If unsure, say N.
> >
> > ...
> >
> > +static int 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;
>
> EADDRNOTAVAIL is a networking error. If your users see this error pop
> up on their console they'll start wiggling ethernet cables, wondering
> why that didn't fix it.

I should probably switch to devm_ioremap_resource() instead, which
returns -EBUSY or -ENOMEM, depending on whether the request_region or
ioremap fails.

> > + sram = devm_kzalloc(&pdev->dev, sizeof(*sram), GFP_KERNEL);
> > + if (!sram)
> > + return -ENOMEM;
> > +
> > + sram->clk = devm_clk_get(&pdev->dev, NULL);
> > + if (IS_ERR(sram->clk))
> > + sram->clk = NULL;
> > + else
> > + clk_prepare_enable(sram->clk);
> > +
> > + sram->pool = devm_gen_pool_create(&pdev->dev, ilog2(SRAM_GRANULARITY), -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;
> > +}
> >
> > ...
> >
> > +int __init sram_init(void)
> > +{
> > + return platform_driver_register(&sram_driver);
> > +}
> > +
> > +postcore_initcall(sram_init);
>
> Why is it postcore_initcall()?

Good question. A few architectures initialize their SRAM in a
core_initcall (davinci, sh, mmp), a few from init_machine (omap2)
postcore_initcall is when Freescale initialized their IRAM for i.MX.

I have not tried yet to use SRAM to execute code for bus frequency
changes during wfi or suspend, I'm not sure how early the sram pool is
really needed by everyone.

regards
Philipp

> Fixlets:
>
> From: Andrew Morton <[email protected]>
> Subject: misc-generic-on-chip-sram-allocation-driver-fix
>
> fix Kconfig text, make sram_init static
>
> Cc: Dong Aisheng <[email protected]>
> Cc: Fabio Estevam <[email protected]>
> Cc: Grant Likely <[email protected]>
> Cc: Greg Kroah-Hartman <[email protected]>
> Cc: Huang Shijie <[email protected]>
> Cc: Javier Martin <[email protected]>
> Cc: Matt Porter <[email protected]>
> Cc: Michal Simek <[email protected]>
> Cc: Paul Gortmaker <[email protected]>
> Cc: Philipp Zabel <[email protected]>
> Cc: Rob Herring <[email protected]>
> Cc: Shawn Guo <[email protected]>
> Signed-off-by: Andrew Morton <[email protected]>
> ---
>
> drivers/misc/Kconfig | 6 +++---
> drivers/misc/sram.c | 2 +-
> 2 files changed, 4 insertions(+), 4 deletions(-)
>
> diff -puN Documentation/devicetree/bindings/misc/sram.txt~misc-generic-on-chip-sram-allocation-driver-fix Documentation/devicetree/bindings/misc/sram.txt
> diff -puN drivers/misc/Kconfig~misc-generic-on-chip-sram-allocation-driver-fix drivers/misc/Kconfig
> --- a/drivers/misc/Kconfig~misc-generic-on-chip-sram-allocation-driver-fix
> +++ a/drivers/misc/Kconfig
> @@ -523,9 +523,9 @@ config SRAM
> 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 SoCs.
> + This driver allows you 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 SoCs.
>
> source "drivers/misc/c2port/Kconfig"
> source "drivers/misc/eeprom/Kconfig"
> diff -puN drivers/misc/Makefile~misc-generic-on-chip-sram-allocation-driver-fix drivers/misc/Makefile
> diff -puN drivers/misc/sram.c~misc-generic-on-chip-sram-allocation-driver-fix drivers/misc/sram.c
> --- a/drivers/misc/sram.c~misc-generic-on-chip-sram-allocation-driver-fix
> +++ a/drivers/misc/sram.c
> @@ -113,7 +113,7 @@ static struct platform_driver sram_drive
> .remove = sram_remove,
> };
>
> -int __init sram_init(void)
> +static int __init sram_init(void)
> {
> return platform_driver_register(&sram_driver);
> }
> _
>
>

2013-03-28 14:22:21

[permalink] [raw]

Subject: Re: [PATCH v9 RESEND 4/4] ARM: dts: add sram for imx53 and imx6q

On Thu, Mar 28, 2013 at 10:05:18AM +0100, Philipp Zabel wrote:
> > > +
> > > + ocram: ocram@f8000000 {
> > > + compatible = "fsl,imx-ocram", "mmio-sram";
> >
> > We should probably just drop "fsl,imx-ocram".
> >
> > Shawn
>
> I thought that in the future somebody might want to implement some i.MX
> OCRAM specifics in a different driver, like the L2 cache / OCRAM mode
> switching, the timing settings, or the TrustZone bits that are
> configurable in the IOMUXC GPR registers.

We can add i.MX OCRAM specific compatible when we see that real need.
Also, generally we should use a specific chip/osc name rather than "imx"
for compatible to specify the particular hardware version.

Shawn

2013-03-28 15:23:29