2014-02-20 10:53:00

by Marek Szyprowski

[permalink] [raw]
Subject: [PATCH v4 0/7] reserved-memory regions/CMA in devicetree, again

Hi all!

This is another quick update of the patches which add basic support for
dynamic allocation of memory reserved regions defined in device tree.

This time I hope I've really managed to address all the issues reported
by Grant Likely, see the change log for more details. As a bonus, I've
added support for ARM64 and PowerPC, as those architectures had quite
well defined place where early memory reservation can be done.

The initial code for this feature were posted here [1], merged as commit
9d8eab7af79cb4ce2de5de39f82c455b1f796963 ("drivers: of: add
initialization code for dma reserved memory") and later reverted by
commit 1931ee143b0ab72924944bc06e363d837ba05063. For more information,
see [2]. Finally a new bindings has been proposed [3] and Josh
Cartwright a few days ago prepared some code which implements those
bindings [4]. This finally pushed me again to find some time to finish
this task and review the code. Josh agreed to give me the ownership of
this series to continue preparing them for mainline inclusion.

For more information please refer to the changlelog below.

[1]: http://lkml.kernel.org/g/[email protected]
[2]: http://lkml.kernel.org/g/[email protected]
[3]: http://lkml.kernel.org/g/[email protected]
[4]: http://thread.gmane.org/gmane.linux.documentation/19579

Changelog:

v4:
- dynamic allocations are processed after all static reservations has been
done
- moved code for handling static reservations to drivers/of/fdt.c
- removed node matching by string comparison, now phandle values are used
directly
- moved code for DMA and CMA handling directly to
drivers/base/dma-{coherent,contiguous}.c
- added checks for proper #size-cells, #address-cells, ranges properties
in /reserved-memory node
- even more code cleanup
- added init code for ARM64 and PowerPC

v3: http://article.gmane.org/gmane.linux.documentation/20169/
- refactored memory reservation code, created common code to parse reg, size,
align, alloc-ranges properties
- added support for multiple tuples in 'reg' property
- memory is reserved regardless of presence of the driver for its compatible
- prepared arch specific hooks for memory reservation (defaults use memblock
calls)
- removed node matching by string during device initialization
- CMA init code: added checks for required region alignment
- more code cleanup here and there

v2: http://thread.gmane.org/gmane.linux.documentation/19870/
- removed copying of the node name
- split shared-dma-pool handling into separate files (one for CMA and one
for dma_declare_coherent based implementations) for making the code easier
to understand
- added support for AMBA devices, changed prototypes to use struct decice
instead of struct platform_device
- renamed some functions to better match other names used in drivers/of/
- restructured the rest of the code a bit for better readability
- added 'reusable' property to exmaple linux,cma node in documentation
- exclusive dma (dma_coherent) is used for only handling 'shared-dma-pool'
regions without 'reusable' property and CMA is used only for handling
'shared-dma-pool' regions with 'reusable' property.

v1: http://thread.gmane.org/gmane.linux.documentation/19579
- initial version prepared by Josh Cartwright

Summary:

Grant Likely (1):
of: document bindings for reserved-memory nodes

Marek Szyprowski (6):
drivers: of: add initialization code for reserved memory
drivers: dma-coherent: add initialization from device tree
drivers: dma-contiguous: add initialization from device tree
arm: add support for reserved memory defined by device tree
arm64: add support for reserved memory defined by device tree
powerpc: add support for reserved memory defined by device tree

.../bindings/reserved-memory/reserved-memory.txt | 138 +++++++++
arch/arm/Kconfig | 1 +
arch/arm/mm/init.c | 3 +
arch/arm64/Kconfig | 1 +
arch/arm64/mm/init.c | 2 +
arch/powerpc/Kconfig | 1 +
arch/powerpc/kernel/prom.c | 3 +
drivers/base/dma-coherent.c | 41 +++
drivers/base/dma-contiguous.c | 130 +++++++--
drivers/of/Kconfig | 6 +
drivers/of/Makefile | 1 +
drivers/of/fdt.c | 145 ++++++++++
drivers/of/of_reserved_mem.c | 296 ++++++++++++++++++++
drivers/of/platform.c | 7 +
include/asm-generic/vmlinux.lds.h | 11 +
include/linux/of_reserved_mem.h | 65 +++++
16 files changed, 829 insertions(+), 22 deletions(-)
create mode 100644 Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt
create mode 100644 drivers/of/of_reserved_mem.c
create mode 100644 include/linux/of_reserved_mem.h

--
1.7.9.5


2014-02-20 10:53:09

by Marek Szyprowski

[permalink] [raw]
Subject: [PATCH v4 7/7] powerpc: add support for reserved memory defined by device tree

Enable reserved memory initialization from device tree.

Signed-off-by: Marek Szyprowski <[email protected]>
---
arch/powerpc/Kconfig | 1 +
arch/powerpc/kernel/prom.c | 3 +++
2 files changed, 4 insertions(+)

diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index 957bf344c0f5..3b6617fed8fc 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -90,6 +90,7 @@ config PPC
select BINFMT_ELF
select OF
select OF_EARLY_FLATTREE
+ select OF_RESERVED_MEM
select HAVE_FTRACE_MCOUNT_RECORD
select HAVE_DYNAMIC_FTRACE
select HAVE_FUNCTION_TRACER
diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c
index f58c0d3aaeb4..b814262a2048 100644
--- a/arch/powerpc/kernel/prom.c
+++ b/arch/powerpc/kernel/prom.c
@@ -33,6 +33,7 @@
#include <linux/irq.h>
#include <linux/memblock.h>
#include <linux/of.h>
+#include <linux/of_reserved_mem.h>

#include <asm/prom.h>
#include <asm/rtas.h>
@@ -588,6 +589,8 @@ static void __init early_reserve_mem_dt(void)
memblock_reserve(base, size);
}
}
+
+ early_init_fdt_scan_reserved_mem();
}

static void __init early_reserve_mem(void)
--
1.7.9.5

2014-02-20 10:53:08

by Marek Szyprowski

[permalink] [raw]
Subject: [PATCH v4 3/7] drivers: dma-contiguous: add initialization from device tree

Refactor internal dma_contiguous_init_reserved_mem() function, which
creates CMA area from previously reserved memory region and add support
for handling 'shared-dma-pool' reserved-memory device tree nodes.

Based on previous code provided by Josh Cartwright <[email protected]>

Signed-off-by: Marek Szyprowski <[email protected]>
---
drivers/base/dma-contiguous.c | 130 ++++++++++++++++++++++++++++++++++-------
1 file changed, 108 insertions(+), 22 deletions(-)

diff --git a/drivers/base/dma-contiguous.c b/drivers/base/dma-contiguous.c
index 165c2c299e57..5dfcc3743d9b 100644
--- a/drivers/base/dma-contiguous.c
+++ b/drivers/base/dma-contiguous.c
@@ -182,6 +182,49 @@ static int __init cma_init_reserved_areas(void)
core_initcall(cma_init_reserved_areas);

/**
+ * dma_contiguous_init_reserved_mem() - reserve custom contiguous area
+ * @size: Size of the reserved area (in bytes),
+ * @base: Base address of the reserved area optional, use 0 for any
+ * @limit: End address of the reserved memory (optional, 0 for any).
+ * @res_cma: Pointer to store the created cma region.
+ *
+ * This function reserves memory from early allocator. It should be
+ * called by arch specific code once the early allocator (memblock or bootmem)
+ * has been activated and all other subsystems have already allocated/reserved
+ * memory. This function allows to create custom reserved areas for specific
+ * devices.
+ */
+static int __init dma_contiguous_init_reserved_mem(phys_addr_t size,
+ phys_addr_t base, struct cma **res_cma)
+{
+ struct cma *cma = &cma_areas[cma_area_count];
+ phys_addr_t alignment;
+
+ /* Sanity checks */
+ if (cma_area_count == ARRAY_SIZE(cma_areas)) {
+ pr_err("Not enough slots for CMA reserved regions!\n");
+ return -ENOSPC;
+ }
+
+ if (!size || !memblock_is_region_reserved(base, size))
+ return -EINVAL;
+
+ /* Sanitise input arguments */
+ alignment = PAGE_SIZE << max(MAX_ORDER - 1, pageblock_order);
+ if (ALIGN(base, alignment) != base || ALIGN(size, alignment) != size)
+ return -EINVAL;
+
+ cma->base_pfn = PFN_DOWN(base);
+ cma->count = size >> PAGE_SHIFT;
+ *res_cma = cma;
+ cma_area_count++;
+
+ /* Architecture specific contiguous memory fixup. */
+ dma_contiguous_early_fixup(base, size);
+ return 0;
+}
+
+/**
* dma_contiguous_reserve_area() - reserve custom contiguous area
* @size: Size of the reserved area (in bytes),
* @base: Base address of the reserved area optional, use 0 for any
@@ -197,7 +240,6 @@ core_initcall(cma_init_reserved_areas);
int __init dma_contiguous_reserve_area(phys_addr_t size, phys_addr_t base,
phys_addr_t limit, struct cma **res_cma)
{
- struct cma *cma = &cma_areas[cma_area_count];
phys_addr_t alignment;
int ret = 0;

@@ -205,12 +247,6 @@ int __init dma_contiguous_reserve_area(phys_addr_t size, phys_addr_t base,
(unsigned long)size, (unsigned long)base,
(unsigned long)limit);

- /* Sanity checks */
- if (cma_area_count == ARRAY_SIZE(cma_areas)) {
- pr_err("Not enough slots for CMA reserved regions!\n");
- return -ENOSPC;
- }
-
if (!size)
return -EINVAL;

@@ -241,21 +277,12 @@ int __init dma_contiguous_reserve_area(phys_addr_t size, phys_addr_t base,
}
}

- /*
- * Each reserved area must be initialised later, when more kernel
- * subsystems (like slab allocator) are available.
- */
- cma->base_pfn = PFN_DOWN(base);
- cma->count = size >> PAGE_SHIFT;
- *res_cma = cma;
- cma_area_count++;
-
- pr_info("CMA: reserved %ld MiB at %08lx\n", (unsigned long)size / SZ_1M,
- (unsigned long)base);
-
- /* Architecture specific contiguous memory fixup. */
- dma_contiguous_early_fixup(base, size);
- return 0;
+ ret = dma_contiguous_init_reserved_mem(size, base, res_cma);
+ if (ret == 0) {
+ pr_info("CMA: reserved %ld MiB at %08lx\n",
+ (unsigned long)size / SZ_1M, (unsigned long)base);
+ return 0;
+ }
err:
pr_err("CMA: failed to reserve %ld MiB\n", (unsigned long)size / SZ_1M);
return ret;
@@ -357,3 +384,62 @@ bool dma_release_from_contiguous(struct device *dev, struct page *pages,

return true;
}
+
+/*
+ * Support for reserved memory regions defined in device tree
+ */
+#ifdef CONFIG_OF_RESERVED_MEM
+#include <linux/of.h>
+#include <linux/of_fdt.h>
+#include <linux/of_reserved_mem.h>
+
+#undef pr_fmt
+#define pr_fmt(fmt) fmt
+
+static void rmem_cma_device_init(struct reserved_mem *rmem,
+ struct device *dev, struct of_phandle_args *args)
+{
+ struct cma *cma = rmem->priv;
+ dev_set_cma_area(dev, cma);
+}
+
+static const struct reserved_mem_ops rmem_cma_ops = {
+ .device_init = rmem_cma_device_init,
+};
+
+static int __init rmem_cma_setup(struct reserved_mem *rmem,
+ unsigned long node,
+ const char *uname)
+{
+ phys_addr_t align = PAGE_SIZE << max(MAX_ORDER - 1, pageblock_order);
+ phys_addr_t mask = align - 1;
+ struct cma *cma;
+ int err;
+
+ if (!of_get_flat_dt_prop(node, "reusable", NULL))
+ return -EINVAL;
+
+ if ((rmem->base & mask) || (rmem->size & mask)) {
+ pr_err("Reserved memory: incorrect alignment of CMA region\n");
+ return -EINVAL;
+ }
+
+ err = dma_contiguous_init_reserved_mem(rmem->size, rmem->base, &cma);
+ if (err) {
+ pr_err("Reserved memory: unable to setup CMA region\n");
+ return err;
+ }
+
+ if (of_get_flat_dt_prop(node, "linux,cma-default", NULL))
+ dma_contiguous_set_default(cma);
+
+ rmem->ops = &rmem_cma_ops;
+ rmem->priv = cma;
+
+ pr_info("Reserved memory: created CMA memory pool at %pa, size %ld MiB\n",
+ &rmem->base, (unsigned long)rmem->size / SZ_1M);
+
+ return 0;
+}
+RESERVEDMEM_OF_DECLARE(cma, "shared-dma-pool", rmem_cma_setup);
+#endif
--
1.7.9.5

2014-02-20 10:53:04

by Marek Szyprowski

[permalink] [raw]
Subject: [PATCH v4 2/7] drivers: dma-coherent: add initialization from device tree

Add support for handling 'shared-dma-pool' reserved-memory device tree
nodes.

Based on previous code provided by Josh Cartwright <[email protected]>

Signed-off-by: Marek Szyprowski <[email protected]>
---
drivers/base/dma-coherent.c | 41 +++++++++++++++++++++++++++++++++++++++++
1 file changed, 41 insertions(+)

diff --git a/drivers/base/dma-coherent.c b/drivers/base/dma-coherent.c
index bc256b641027..2343ba5d6d43 100644
--- a/drivers/base/dma-coherent.c
+++ b/drivers/base/dma-coherent.c
@@ -218,3 +218,44 @@ int dma_mmap_from_coherent(struct device *dev, struct vm_area_struct *vma,
return 0;
}
EXPORT_SYMBOL(dma_mmap_from_coherent);
+
+/*
+ * Support for reserved memory regions defined in device tree
+ */
+#ifdef CONFIG_OF_RESERVED_MEM
+#include <linux/of.h>
+#include <linux/of_fdt.h>
+#include <linux/of_reserved_mem.h>
+
+static void rmem_dma_device_init(struct reserved_mem *rmem,
+ struct device *dev, struct of_phandle_args *args)
+{
+ dma_declare_coherent_memory(dev, rmem->base, rmem->base,
+ rmem->size, DMA_MEMORY_MAP | DMA_MEMORY_EXCLUSIVE);
+}
+
+static void rmem_dma_device_release(struct reserved_mem *rmem,
+ struct device *dev)
+{
+ dma_release_declared_memory(dev);
+}
+
+static const struct reserved_mem_ops rmem_dma_ops = {
+ .device_init = rmem_dma_device_init,
+ .device_release = rmem_dma_device_release,
+};
+
+static int __init rmem_dma_setup(struct reserved_mem *rmem,
+ unsigned long node,
+ const char *uname)
+{
+ if (of_get_flat_dt_prop(node, "reusable", NULL))
+ return -EINVAL;
+
+ rmem->ops = &rmem_dma_ops;
+ pr_info("Reserved memory: created DMA memory pool at %pa, size %ld MiB\n",
+ &rmem->base, (unsigned long)rmem->size / SZ_1M);
+ return 0;
+}
+RESERVEDMEM_OF_DECLARE(dma, "shared-dma-pool", rmem_dma_setup);
+#endif
--
1.7.9.5

2014-02-20 10:53:55

by Marek Szyprowski

[permalink] [raw]
Subject: [PATCH v4 6/7] arm64: add support for reserved memory defined by device tree

Enable reserved memory initialization from device tree.

Signed-off-by: Marek Szyprowski <[email protected]>
---
arch/arm64/Kconfig | 1 +
arch/arm64/mm/init.c | 2 ++
2 files changed, 3 insertions(+)

diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index 27bbcfc7202a..6abf15407dca 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -43,6 +43,7 @@ config ARM64
select NO_BOOTMEM
select OF
select OF_EARLY_FLATTREE
+ select OF_RESERVED_MEM
select PERF_USE_VMALLOC
select POWER_RESET
select POWER_SUPPLY
diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c
index d0b4c2efda90..447550d6bd9b 100644
--- a/arch/arm64/mm/init.c
+++ b/arch/arm64/mm/init.c
@@ -30,6 +30,7 @@
#include <linux/memblock.h>
#include <linux/sort.h>
#include <linux/of_fdt.h>
+#include <linux/of_reserved_mem.h>
#include <linux/dma-contiguous.h>

#include <asm/sections.h>
@@ -160,6 +161,7 @@ void __init arm64_memblock_init(void)
memblock_reserve(base, size);
}

+ early_init_fdt_scan_reserved_mem();
dma_contiguous_reserve(0);

memblock_allow_resize();
--
1.7.9.5

2014-02-20 10:56:02

by Marek Szyprowski

[permalink] [raw]
Subject: [PATCH v4 4/7] of: document bindings for reserved-memory nodes

From: Grant Likely <[email protected]>

Reserved memory nodes allow for the reservation of static (fixed
address) regions, or dynamically allocated regions for a specific
purpose.

Signed-off-by: Grant Likely <[email protected]>
[joshc: Based on binding document proposed (in non-patch form) here:
http://lkml.kernel.org/g/[email protected]
adapted to support #memory-region-cells]
Signed-off-by: Josh Cartwright <[email protected]>
Signed-off-by: Marek Szyprowski <[email protected]>
---
.../bindings/reserved-memory/reserved-memory.txt | 138 ++++++++++++++++++++
1 file changed, 138 insertions(+)
create mode 100644 Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt

diff --git a/Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt b/Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt
new file mode 100644
index 000000000000..a606ce90c9c4
--- /dev/null
+++ b/Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt
@@ -0,0 +1,138 @@
+*** Reserved memory regions ***
+
+Reserved memory is specified as a node under the /reserved-memory node.
+The operating system shall exclude reserved memory from normal usage
+one can create child nodes describing particular reserved (excluded from
+normal use) memory regions. Such memory regions are usually designed for
+the special usage by various device drivers.
+
+Parameters for each memory region can be encoded into the device tree
+with the following nodes:
+
+/reserved-memory node
+---------------------
+#address-cells, #size-cells (required) - standard definition
+ - Should use the same values as the root node
+#memory-region-cells (required) - dictates number of cells used in the child
+ nodes memory-region specifier
+ranges (required) - standard definition
+ - Should be empty
+
+/reserved-memory/ child nodes
+-----------------------------
+Each child of the reserved-memory node specifies one or more regions of
+reserved memory. Each child node may either use a 'reg' property to
+specify a specific range of reserved memory, or a 'size' property with
+optional constraints to request a dynamically allocated block of memory.
+
+Following the generic-names recommended practice, node names should
+reflect the purpose of the node (ie. "framebuffer" or "dma-pool"). Unit
+address (@<address>) should be appended to the name if the node is a
+static allocation.
+
+Properties:
+Requires either a) or b) below.
+a) static allocation
+ reg (required) - standard definition
+b) dynamic allocation
+ size (required) - length based on parent's #size-cells
+ - Size in bytes of memory to reserve.
+ alignment (optional) - length based on parent's #size-cells
+ - Address boundary for alignment of allocation.
+ alloc-ranges (optional) - prop-encoded-array (address, length pairs).
+ - Specifies regions of memory that are
+ acceptable to allocate from.
+
+If both reg and size are present, then the reg property takes precedence
+and size is ignored.
+
+Additional properties:
+compatible (optional) - standard definition
+ - may contain the following strings:
+ - shared-dma-pool: This indicates a region of memory meant to be
+ used as a shared pool of DMA buffers for a set of devices. It can
+ be used by an operating system to instanciate the necessary pool
+ management subsystem if necessary.
+ - vendor specific string in the form <vendor>,[<device>-]<usage>
+no-map (optional) - empty property
+ - Indicates the operating system must not create a virtual mapping
+ of the region as part of its standard mapping of system memory,
+ nor permit speculative access to it under any circumstances other
+ than under the control of the device driver using the region.
+reusable (optional) - empty property
+ - The operating system can use the memory in this region with the
+ limitation that the device driver(s) owning the region need to be
+ able to reclaim it back. Typically that means that the operating
+ system can use that region to store volatile or cached data that
+ can be otherwise regenerated or migrated elsewhere.
+
+Linux implementation note:
+- If a "linux,cma-default" property is present, then Linux will use the
+ region for the default pool of the contiguous memory allocator.
+
+Device node references to reserved memory
+-----------------------------------------
+Regions in the /reserved-memory node may be referenced by other device
+nodes by adding a memory-region property to the device node.
+
+memory-region (optional) - phandle, specifier pairs to children of /reserved-memory
+
+Example
+-------
+This example defines 3 contiguous regions are defined for Linux kernel:
+one default of all device drivers (named linux,[email protected] and 64MiB in size),
+one dedicated to the framebuffer device (named [email protected], 8MiB), and
+one for multimedia processing (named [email protected], 64MiB).
+
+/ {
+ #address-cells = <1>;
+ #size-cells = <1>;
+
+ memory {
+ reg = <0x40000000 0x40000000>;
+ };
+
+ reserved-memory {
+ #address-cells = <1>;
+ #size-cells = <1>;
+ ranges;
+
+ /* global autoconfigured region for contiguous allocations */
+ linux,cma {
+ compatible = "shared-dma-pool";
+ reusable;
+ #memory-region-cells = <0>;
+ size = <0x4000000>;
+ alignment = <0x2000>;
+ linux,cma-default;
+ };
+
+ display_reserved: [email protected] {
+ #memory-region-cells = <0>;
+ reg = <0x78000000 0x800000>;
+ };
+
+ multimedia_reserved: [email protected] {
+ compatible = "acme,multimedia-memory";
+ #memory-region-cells = <1>;
+ reg = <0x77000000 0x4000000>;
+ };
+ };
+
+ /* ... */
+
+ fb0: [email protected] {
+ memory-region = <&display_reserved>;
+ /* ... */
+ };
+
+ scaler: [email protected] {
+ memory-region = <&multimedia_reserved 0xdeadbeef>;
+ /* ... */
+ };
+
+ codec: [email protected] {
+ memory-region = <&multimedia_reserved 0xfeebdaed>;
+ /* ... */
+ };
+};
--
1.7.9.5

2014-02-20 10:55:59

by Marek Szyprowski

[permalink] [raw]
Subject: [PATCH v4 5/7] arm: add support for reserved memory defined by device tree

Enable reserved memory initialization from device tree.

Signed-off-by: Marek Szyprowski <[email protected]>
---
arch/arm/Kconfig | 1 +
arch/arm/mm/init.c | 3 +++
2 files changed, 4 insertions(+)

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index e25419817791..d0262bea8020 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -1918,6 +1918,7 @@ config USE_OF
select IRQ_DOMAIN
select OF
select OF_EARLY_FLATTREE
+ select OF_RESERVED_MEM
help
Include support for flattened device tree machine descriptions.

diff --git a/arch/arm/mm/init.c b/arch/arm/mm/init.c
index 804d61566a53..eee55d75bbfa 100644
--- a/arch/arm/mm/init.c
+++ b/arch/arm/mm/init.c
@@ -17,6 +17,7 @@
#include <linux/nodemask.h>
#include <linux/initrd.h>
#include <linux/of_fdt.h>
+#include <linux/of_reserved_mem.h>
#include <linux/highmem.h>
#include <linux/gfp.h>
#include <linux/memblock.h>
@@ -323,6 +324,8 @@ void __init arm_memblock_init(struct meminfo *mi,
if (mdesc->reserve)
mdesc->reserve();

+ early_init_fdt_scan_reserved_mem();
+
/*
* reserve memory for DMA contigouos allocations,
* must come from DMA area inside low memory
--
1.7.9.5

2014-02-20 10:56:52

by Marek Szyprowski

[permalink] [raw]
Subject: [PATCH v4 1/7] drivers: of: add initialization code for reserved memory

This patch adds device tree support for contiguous and reserved memory
regions defined in device tree.

Large memory blocks can be reliably reserved only during early boot.
This must happen before the whole memory management subsystem is
initialized, because we need to ensure that the given contiguous blocks
are not yet allocated by kernel. Also it must happen before kernel
mappings for the whole low memory are created, to ensure that there will
be no mappings (for reserved blocks) or mapping with special properties
can be created (for CMA blocks). This all happens before device tree
structures are unflattened, so we need to get reserved memory layout
directly from fdt.

Later, those reserved memory regions are assigned to devices on each
device structure initialization.

Signed-off-by: Marek Szyprowski <[email protected]>
[joshc: rework to implement new DT binding, provide mechanism for
plugging in new reserved-memory node handlers via
RESERVEDMEM_OF_DECLARE]
Signed-off-by: Josh Cartwright <[email protected]>
[mszyprow: added generic memory reservation code, refactored code to
put it directly into fdt.c]
Signed-off-by: Marek Szyprowski <[email protected]>
---
drivers/of/Kconfig | 6 +
drivers/of/Makefile | 1 +
drivers/of/fdt.c | 145 ++++++++++++++++++
drivers/of/of_reserved_mem.c | 296 +++++++++++++++++++++++++++++++++++++
drivers/of/platform.c | 7 +
include/asm-generic/vmlinux.lds.h | 11 ++
include/linux/of_reserved_mem.h | 65 ++++++++
7 files changed, 531 insertions(+)
create mode 100644 drivers/of/of_reserved_mem.c
create mode 100644 include/linux/of_reserved_mem.h

diff --git a/drivers/of/Kconfig b/drivers/of/Kconfig
index c6973f101a3e..30a7d87a8077 100644
--- a/drivers/of/Kconfig
+++ b/drivers/of/Kconfig
@@ -75,4 +75,10 @@ config OF_MTD
depends on MTD
def_bool y

+config OF_RESERVED_MEM
+ depends on OF_EARLY_FLATTREE
+ bool
+ help
+ Helpers to allow for reservation of memory regions
+
endmenu # OF
diff --git a/drivers/of/Makefile b/drivers/of/Makefile
index efd05102c405..ed9660adad77 100644
--- a/drivers/of/Makefile
+++ b/drivers/of/Makefile
@@ -9,3 +9,4 @@ obj-$(CONFIG_OF_MDIO) += of_mdio.o
obj-$(CONFIG_OF_PCI) += of_pci.o
obj-$(CONFIG_OF_PCI_IRQ) += of_pci_irq.o
obj-$(CONFIG_OF_MTD) += of_mtd.o
+obj-$(CONFIG_OF_RESERVED_MEM) += of_reserved_mem.o
diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
index 758b4f8b30b7..04efe2ba736f 100644
--- a/drivers/of/fdt.c
+++ b/drivers/of/fdt.c
@@ -15,6 +15,7 @@
#include <linux/module.h>
#include <linux/of.h>
#include <linux/of_fdt.h>
+#include <linux/of_reserved_mem.h>
#include <linux/string.h>
#include <linux/errno.h>
#include <linux/slab.h>
@@ -438,6 +439,150 @@ int __initdata dt_root_size_cells;
struct boot_param_header *initial_boot_params;

#ifdef CONFIG_OF_EARLY_FLATTREE
+#if defined(CONFIG_HAVE_MEMBLOCK)
+int __init __weak
+early_init_dt_reserve_memory_arch(phys_addr_t base, phys_addr_t size,
+ bool nomap)
+{
+ if (memblock_is_region_reserved(base, size))
+ return -EBUSY;
+ if (nomap)
+ return memblock_remove(base, size);
+ return memblock_reserve(base, size);
+}
+#else
+int __init __weak
+early_init_dt_reserve_memory_arch(phys_addr_t base, phys_addr_t size,
+ bool nomap)
+{
+ pr_error("Reserved memory not supported, ignoring range 0x%llx - 0x%llx%s\n",
+ base, size, nomap ? " (nomap)" : "");
+ return -ENOSYS;
+}
+#endif
+
+/**
+ * res_mem_reserve_reg() - reserve all memory described in 'reg' property
+ */
+static int __init
+__reserved_mem_reserve_reg(unsigned long node, const char *uname,
+ phys_addr_t *res_base, phys_addr_t *res_size)
+{
+ int t_len = (dt_root_addr_cells + dt_root_size_cells) * sizeof(__be32);
+ phys_addr_t base, size;
+ unsigned long len;
+ __be32 *prop;
+ int nomap;
+
+ prop = of_get_flat_dt_prop(node, "reg", &len);
+ if (!prop)
+ return -ENOENT;
+
+ if (len && len % t_len != 0) {
+ pr_err("Reserved memory: invalid reg property in '%s', skipping node.\n",
+ uname);
+ return -EINVAL;
+ }
+
+ nomap = of_get_flat_dt_prop(node, "no-map", NULL) != NULL;
+
+ /* store base and size values from the first reg tuple */
+ *res_base = 0;
+ while (len > 0) {
+ base = dt_mem_next_cell(dt_root_addr_cells, &prop);
+ size = dt_mem_next_cell(dt_root_size_cells, &prop);
+
+ if (base && size &&
+ early_init_dt_reserve_memory_arch(base, size, nomap) == 0)
+ pr_debug("Reserved memory: reserved region for node '%s': base %pa, size %ld MiB\n",
+ uname, &base, (unsigned long)size / SZ_1M);
+ else
+ pr_info("Reserved memory: failed to reserve memory for node '%s': base %pa, size %ld MiB\n",
+ uname, &base, (unsigned long)size / SZ_1M);
+
+ len -= t_len;
+
+ if (!(*res_base)) {
+ *res_base = base;
+ *res_size = size;
+ }
+ }
+ return 0;
+}
+
+static int __reserved_mem_check_root(unsigned long node)
+{
+ __be32 *prop;
+
+ prop = of_get_flat_dt_prop(node, "#size-cells", NULL);
+ if (prop && be32_to_cpup(prop) != dt_root_size_cells)
+ return -EINVAL;
+
+ prop = of_get_flat_dt_prop(node, "#address-cells", NULL);
+ if (prop && be32_to_cpup(prop) != dt_root_addr_cells)
+ return -EINVAL;
+
+ prop = of_get_flat_dt_prop(node, "ranges", NULL);
+ if (!prop)
+ return -EINVAL;
+ return 0;
+}
+
+/**
+ * fdt_scan_reserved_mem() - scan a single FDT node for reserved memory
+ */
+static int __init
+__fdt_scan_reserved_mem(unsigned long node, const char *uname, int depth,
+ void *data)
+{
+ phys_addr_t base = 0, size = 0;
+ static int found;
+ const char *status;
+ int err;
+
+ if (!found && depth == 1 && strcmp(uname, "reserved-memory") == 0) {
+ if (__reserved_mem_check_root(node) != 0) {
+ pr_err("Reserved memory: unsupported node format, ignoring\n");
+ /* break scan */
+ return 1;
+ }
+ found = 1;
+ /* scan next node */
+ return 0;
+ } else if (!found) {
+ /* scan next node */
+ return 0;
+ } else if (found && depth < 2) {
+ /* scanning of /reserved-memory has been finished */
+ return 1;
+ }
+
+ status = of_get_flat_dt_prop(node, "status", NULL);
+ if (status && strcmp(status, "okay") != 0 && strcmp(status, "ok") != 0)
+ return 0;
+
+ err = __reserved_mem_reserve_reg(node, uname, &base, &size);
+ if (err == -ENOENT && of_get_flat_dt_prop(node, "size", NULL) == NULL)
+ goto end;
+
+ fdt_reserved_mem_save_node(node, uname, base, size);
+end:
+ /* scan next node */
+ return 0;
+}
+
+/**
+ * early_init_fdt_scan_reserved_mem() - create reserved memory regions
+ *
+ * This function grabs memory from early allocator for device exclusive use
+ * defined in device tree structures. It should be called by arch specific code
+ * once the early allocator (i.e. memblock) has been fully activated.
+ */
+void __init early_init_fdt_scan_reserved_mem(void)
+{
+ of_scan_flat_dt(__fdt_scan_reserved_mem, NULL);
+ fdt_init_reserved_mem();
+}

/**
* of_scan_flat_dt - scan flattened tree blob and call callback on each.
diff --git a/drivers/of/of_reserved_mem.c b/drivers/of/of_reserved_mem.c
new file mode 100644
index 000000000000..cacf04810b87
--- /dev/null
+++ b/drivers/of/of_reserved_mem.c
@@ -0,0 +1,296 @@
+/*
+ * Device tree based initialization code for reserved memory.
+ *
+ * Copyright (c) 2013, The Linux Foundation. All Rights Reserved.
+ * Copyright (c) 2013,2014 Samsung Electronics Co., Ltd.
+ * http://www.samsung.com
+ * Author: Marek Szyprowski <[email protected]>
+ * Author: Josh Cartwright <[email protected]>
+ *
+ * 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 optional) any later version of the license.
+ */
+
+#include <linux/err.h>
+#include <linux/of.h>
+#include <linux/of_fdt.h>
+#include <linux/of_platform.h>
+#include <linux/mm.h>
+#include <linux/sizes.h>
+#include <linux/of_reserved_mem.h>
+
+#define MAX_RESERVED_REGIONS 16
+static struct reserved_mem reserved_mem[MAX_RESERVED_REGIONS];
+static int reserved_mem_count;
+
+#if defined(CONFIG_HAVE_MEMBLOCK)
+#include <linux/memblock.h>
+int __init __weak
+early_init_dt_alloc_reserved_memory_arch(phys_addr_t size, phys_addr_t align,
+ phys_addr_t start, phys_addr_t end,
+ bool nomap, phys_addr_t *res_base)
+{
+ /*
+ * We use __memblock_alloc_base() since memblock_alloc_base() panic()s.
+ */
+ phys_addr_t base = __memblock_alloc_base(size, align, end);
+ if (!base)
+ return -ENOMEM;
+
+ if (base < start) {
+ memblock_free(base, size);
+ return -ENOMEM;
+ }
+
+ *res_base = base;
+ if (nomap)
+ return memblock_remove(base, size);
+ return 0;
+}
+#else
+int __init __weak
+early_init_dt_alloc_reserved_memory_arch(phys_addr_t size, phys_addr_t align,
+ phys_addr_t start, phys_addr_t end,
+ bool nomap, phys_addr_t *res_base)
+{
+ pr_error("Reserved memory not supported, ignoring region 0x%llx%s\n",
+ size, nomap ? " (nomap)" : "");
+ return -ENOSYS;
+}
+#endif
+
+/**
+ * res_mem_save_node() - save fdt node for second pass initialization
+ */
+int __init fdt_reserved_mem_save_node(unsigned long node, const char *uname,
+ phys_addr_t base, phys_addr_t size)
+{
+ struct reserved_mem *rmem = &reserved_mem[reserved_mem_count];
+
+ if (reserved_mem_count == ARRAY_SIZE(reserved_mem)) {
+ pr_err("Reserved memory: not enough space all defined regions.\n");
+ return -ENOSPC;
+ }
+
+ rmem->fdt_node = node;
+ rmem->name = uname;
+ rmem->base = base;
+ rmem->size = size;
+
+ reserved_mem_count++;
+ return 0;
+}
+
+/**
+ * res_mem_alloc_size() - allocate reserved memory described by 'size', 'align'
+ * and 'alloc-ranges' properties
+ */
+static int __init
+__reserved_mem_alloc_size(unsigned long node, const char *uname,
+ phys_addr_t *res_base, phys_addr_t *res_size)
+{
+ int t_len = (dt_root_addr_cells + dt_root_size_cells) * sizeof(__be32);
+ phys_addr_t start = 0, end = 0;
+ phys_addr_t base = 0, align = 0, size;
+ unsigned long len;
+ __be32 *prop;
+ int nomap;
+ int ret;
+
+ prop = of_get_flat_dt_prop(node, "size", &len);
+ if (!prop)
+ return -EINVAL;
+
+ if (len != dt_root_size_cells * sizeof(__be32)) {
+ pr_err("Reserved memory: invalid size property in '%s' node.\n",
+ uname);
+ return -EINVAL;
+ }
+ size = dt_mem_next_cell(dt_root_size_cells, &prop);
+
+ nomap = of_get_flat_dt_prop(node, "no-map", NULL) != NULL;
+
+ prop = of_get_flat_dt_prop(node, "align", &len);
+ if (prop) {
+ if (len != dt_root_addr_cells * sizeof(__be32)) {
+ pr_err("Reserved memory: invalid align property in '%s' node.\n",
+ uname);
+ return -EINVAL;
+ }
+ align = dt_mem_next_cell(dt_root_addr_cells, &prop);
+ }
+
+ prop = of_get_flat_dt_prop(node, "alloc-ranges", &len);
+ if (prop) {
+
+ if (len % t_len != 0) {
+ pr_err("Reserved memory: invalid alloc-ranges property in '%s', skipping node.\n",
+ uname);
+ return -EINVAL;
+ }
+
+ base = 0;
+
+ while (len > 0) {
+ start = dt_mem_next_cell(dt_root_addr_cells, &prop);
+ end = start + dt_mem_next_cell(dt_root_size_cells,
+ &prop);
+
+ ret = early_init_dt_alloc_reserved_memory_arch(size,
+ align, start, end, nomap, &base);
+ if (ret == 0) {
+ pr_debug("Reserved memory: allocated memory for '%s' node: base %pa, size %ld MiB\n",
+ uname, &base,
+ (unsigned long)size / SZ_1M);
+ break;
+ }
+ len -= t_len;
+ }
+
+ } else {
+ ret = early_init_dt_alloc_reserved_memory_arch(size, align,
+ 0, 0, nomap, &base);
+ if (ret == 0)
+ pr_debug("Reserved memory: allocated memory for '%s' node: base %pa, size %ld MiB\n",
+ uname, &base, (unsigned long)size / SZ_1M);
+ }
+
+ if (base == 0) {
+ pr_info("Reserved memory: failed to allocate memory for node '%s'\n",
+ uname);
+ return -ENOMEM;
+ }
+
+ *res_base = base;
+ *res_size = size;
+
+ return 0;
+}
+
+static const struct of_device_id __rmem_of_table_sentinel
+ __used __section(__reservedmem_of_table_end);
+
+/**
+ * res_mem_init_node() - call region specific reserved memory init code
+ */
+static int __init __reserved_mem_init_node(struct reserved_mem *rmem)
+{
+ extern const struct of_device_id __reservedmem_of_table[];
+ const struct of_device_id *i;
+
+ for (i = __reservedmem_of_table; i < &__rmem_of_table_sentinel; i++) {
+ reservedmem_of_init_fn initfn = i->data;
+ const char *compat = i->compatible;
+
+ if (!of_flat_dt_is_compatible(rmem->fdt_node, compat))
+ continue;
+
+ if (initfn(rmem, rmem->fdt_node, rmem->name) == 0) {
+ pr_info("Reserved memory: initialized node %s, compatible id %s\n",
+ rmem->name, compat);
+ return 0;
+ }
+ }
+ return -ENOENT;
+}
+
+/**
+ * fdt_init_reserved_mem - allocate and init all saved reserved memory regions
+ */
+void __init fdt_init_reserved_mem(void)
+{
+ int i;
+ for (i = 0; i < reserved_mem_count; i++) {
+ struct reserved_mem *rmem = &reserved_mem[i];
+ unsigned long node = rmem->fdt_node;
+ unsigned long len;
+ __be32 *prop;
+ int err = 0;
+
+ prop = of_get_flat_dt_prop(node, "phandle", &len);
+ if (!prop)
+ prop = of_get_flat_dt_prop(node, "linux,phandle", &len);
+ if (prop)
+ rmem->phandle = of_read_number(prop, len/4);
+
+ if (rmem->size == 0)
+ err = __reserved_mem_alloc_size(node, rmem->name,
+ &rmem->base, &rmem->size);
+ if (err == 0)
+ __reserved_mem_init_node(rmem);
+ }
+}
+
+static inline struct reserved_mem *__find_rmem(struct device_node *node)
+{
+ unsigned int len, phandle_val;
+ const __be32 *prop;
+ unsigned int i;
+
+ prop = of_get_property(node, "phandle", &len);
+ if (!prop)
+ prop = of_get_property(node, "linux,phandle", &len);
+ if (!prop || len < sizeof(__be32))
+ return NULL;
+
+ phandle_val = be32_to_cpup(prop);
+ for (i = 0; i < reserved_mem_count; i++)
+ if (reserved_mem[i].phandle == phandle_val)
+ return &reserved_mem[i];
+ return NULL;
+}
+
+/**
+ * of_reserved_mem_device_init() - assign reserved memory region to given device
+ *
+ * This function assign memory region pointed by "memory-region" device tree
+ * property to the given device.
+ */
+void of_reserved_mem_device_init(struct device *dev)
+{
+ struct device_node *np = dev->of_node;
+ struct reserved_mem *rmem;
+ struct of_phandle_args s;
+ unsigned int i;
+
+ for (i = 0; of_parse_phandle_with_args(np, "memory-region",
+ "#memory-region-cells", i, &s) == 0; i++) {
+
+ rmem = __find_rmem(s.np);
+ if (!rmem || !rmem->ops || !rmem->ops->device_init) {
+ of_node_put(s.np);
+ continue;
+ }
+
+ rmem->ops->device_init(rmem, dev, &s);
+ dev_info(dev, "assigned reserved memory node %s\n", rmem->name);
+ of_node_put(s.np);
+ break;
+ }
+}
+
+/**
+ * of_reserved_mem_device_release() - release reserved memory device structures
+ *
+ * This function releases structures allocated for memory region handling for
+ * the given device.
+ */
+void of_reserved_mem_device_release(struct device *dev)
+{
+ struct device_node *np = dev->of_node;
+ struct reserved_mem *rmem;
+ struct of_phandle_args s;
+ unsigned int i;
+
+ for (i = 0; of_parse_phandle_with_args(np, "memory-region",
+ "#memory-region-cells", i, &s) == 0; i++) {
+
+ rmem = __find_rmem(s.np);
+ if (rmem && rmem->ops && rmem->ops->device_release)
+ rmem->ops->device_release(rmem, dev);
+
+ of_node_put(s.np);
+ }
+}
diff --git a/drivers/of/platform.c b/drivers/of/platform.c
index 404d1daebefa..3df0b1826e8b 100644
--- a/drivers/of/platform.c
+++ b/drivers/of/platform.c
@@ -21,6 +21,7 @@
#include <linux/of_device.h>
#include <linux/of_irq.h>
#include <linux/of_platform.h>
+#include <linux/of_reserved_mem.h>
#include <linux/platform_device.h>

const struct of_device_id of_default_bus_match_table[] = {
@@ -220,6 +221,8 @@ static struct platform_device *of_platform_device_create_pdata(
dev->dev.bus = &platform_bus_type;
dev->dev.platform_data = platform_data;

+ of_reserved_mem_device_init(&dev->dev);
+
/* We do not fill the DMA ops for platform devices by default.
* This is currently the responsibility of the platform code
* to do such, possibly using a device notifier
@@ -227,6 +230,7 @@ static struct platform_device *of_platform_device_create_pdata(

if (of_device_add(dev) != 0) {
platform_device_put(dev);
+ of_reserved_mem_device_release(&dev->dev);
return NULL;
}

@@ -282,6 +286,8 @@ static struct amba_device *of_amba_device_create(struct device_node *node,
else
of_device_make_bus_id(&dev->dev);

+ of_reserved_mem_device_init(&dev->dev);
+
/* Allow the HW Peripheral ID to be overridden */
prop = of_get_property(node, "arm,primecell-periphid", NULL);
if (prop)
@@ -308,6 +314,7 @@ static struct amba_device *of_amba_device_create(struct device_node *node,
return dev;

err_free:
+ of_reserved_mem_device_release(&dev->dev);
amba_device_put(dev);
return NULL;
}
diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
index bc2121fa9132..f10f64fcc815 100644
--- a/include/asm-generic/vmlinux.lds.h
+++ b/include/asm-generic/vmlinux.lds.h
@@ -167,6 +167,16 @@
#define CLK_OF_TABLES()
#endif

+#ifdef CONFIG_OF_RESERVED_MEM
+#define RESERVEDMEM_OF_TABLES() \
+ . = ALIGN(8); \
+ VMLINUX_SYMBOL(__reservedmem_of_table) = .; \
+ *(__reservedmem_of_table) \
+ *(__reservedmem_of_table_end)
+#else
+#define RESERVEDMEM_OF_TABLES()
+#endif
+
#define KERNEL_DTB() \
STRUCT_ALIGN(); \
VMLINUX_SYMBOL(__dtb_start) = .; \
@@ -490,6 +500,7 @@
TRACE_SYSCALLS() \
MEM_DISCARD(init.rodata) \
CLK_OF_TABLES() \
+ RESERVEDMEM_OF_TABLES() \
CLKSRC_OF_TABLES() \
KERNEL_DTB() \
IRQCHIP_OF_MATCH_TABLE()
diff --git a/include/linux/of_reserved_mem.h b/include/linux/of_reserved_mem.h
new file mode 100644
index 000000000000..0bbec4bf23ce
--- /dev/null
+++ b/include/linux/of_reserved_mem.h
@@ -0,0 +1,65 @@
+#ifndef __OF_RESERVED_MEM_H
+#define __OF_RESERVED_MEM_H
+
+struct cma;
+struct device;
+struct of_phandle_args;
+struct reserved_mem_ops;
+
+struct reserved_mem {
+ const char *name;
+ unsigned long fdt_node;
+ unsigned long phandle;
+ const struct reserved_mem_ops *ops;
+ phys_addr_t base;
+ phys_addr_t size;
+ void *priv;
+};
+
+struct reserved_mem_ops {
+ void (*device_init)(struct reserved_mem *rmem,
+ struct device *dev,
+ struct of_phandle_args *args);
+ void (*device_release)(struct reserved_mem *rmem,
+ struct device *dev);
+};
+
+typedef int (*reservedmem_of_init_fn)(struct reserved_mem *rmem,
+ unsigned long node, const char *uname);
+
+#ifdef CONFIG_OF_RESERVED_MEM
+void of_reserved_mem_device_init(struct device *dev);
+void of_reserved_mem_device_release(struct device *dev);
+
+void fdt_init_reserved_mem(void);
+void early_init_fdt_scan_reserved_mem(void);
+int fdt_reserved_mem_save_node(unsigned long node, const char *uname,
+ phys_addr_t base, phys_addr_t size);
+
+#define RESERVEDMEM_OF_DECLARE(name, compat, init) \
+ static const struct of_device_id __reservedmem_of_table_##name \
+ __used __section(__reservedmem_of_table) \
+ = { .compatible = compat, \
+ .data = (init == (reservedmem_of_init_fn)NULL) ? \
+ init : init }
+
+#else
+static inline void of_reserved_mem_device_init(struct device *dev) { }
+static inline void of_reserved_mem_device_release(struct device *pdev) { }
+
+static inline void fdt_init_reserved_mem(void) { }
+static inline void early_init_fdt_scan_reserved_mem(void) { }
+static inline int
+fdt_reserved_mem_save_node(unsigned long node, const char *uname,
+ phys_addr_t base, phys_addr_t size) { }
+
+#define RESERVEDMEM_OF_DECLARE(name, compat, init) \
+ static const struct of_device_id __reservedmem_of_table_##name \
+ __attribute__((unused)) \
+ = { .compatible = compat, \
+ .data = (init == (reservedmem_of_init_fn)NULL) ? \
+ init : init }
+
+#endif
+
+#endif /* __OF_RESERVED_MEM_H */
--
1.7.9.5

2014-02-20 12:40:47

by Marek Szyprowski

[permalink] [raw]
Subject: [PATCH v4 1/7 fixup] drivers: of: add initialization code for reserved memory (fixup)

Rename 'align' property to 'alignment' to match documentation.

Signed-off-by: Marek Szyprowski <[email protected]>
---
drivers/of/of_reserved_mem.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/of/of_reserved_mem.c b/drivers/of/of_reserved_mem.c
index cacf04810b87..49cf430a9e2a 100644
--- a/drivers/of/of_reserved_mem.c
+++ b/drivers/of/of_reserved_mem.c
@@ -112,10 +112,10 @@ __reserved_mem_alloc_size(unsigned long node, const char *uname,

nomap = of_get_flat_dt_prop(node, "no-map", NULL) != NULL;

- prop = of_get_flat_dt_prop(node, "align", &len);
+ prop = of_get_flat_dt_prop(node, "alignment", &len);
if (prop) {
if (len != dt_root_addr_cells * sizeof(__be32)) {
- pr_err("Reserved memory: invalid align property in '%s' node.\n",
+ pr_err("Reserved memory: invalid alignment property in '%s' node.\n",
uname);
return -EINVAL;
}
--
1.7.9.5

2014-02-20 14:01:11

by Grant Likely

[permalink] [raw]
Subject: Re: [PATCH v4 1/7] drivers: of: add initialization code for reserved memory

On Thu, 20 Feb 2014 11:52:41 +0100, Marek Szyprowski <[email protected]> wrote:
> This patch adds device tree support for contiguous and reserved memory
> regions defined in device tree.
>
> Large memory blocks can be reliably reserved only during early boot.
> This must happen before the whole memory management subsystem is
> initialized, because we need to ensure that the given contiguous blocks
> are not yet allocated by kernel. Also it must happen before kernel
> mappings for the whole low memory are created, to ensure that there will
> be no mappings (for reserved blocks) or mapping with special properties
> can be created (for CMA blocks). This all happens before device tree
> structures are unflattened, so we need to get reserved memory layout
> directly from fdt.
>
> Later, those reserved memory regions are assigned to devices on each
> device structure initialization.
>
> Signed-off-by: Marek Szyprowski <[email protected]>
> [joshc: rework to implement new DT binding, provide mechanism for
> plugging in new reserved-memory node handlers via
> RESERVEDMEM_OF_DECLARE]
> Signed-off-by: Josh Cartwright <[email protected]>
> [mszyprow: added generic memory reservation code, refactored code to
> put it directly into fdt.c]
> Signed-off-by: Marek Szyprowski <[email protected]>
> ---
> drivers/of/Kconfig | 6 +
> drivers/of/Makefile | 1 +
> drivers/of/fdt.c | 145 ++++++++++++++++++
> drivers/of/of_reserved_mem.c | 296 +++++++++++++++++++++++++++++++++++++
> drivers/of/platform.c | 7 +
> include/asm-generic/vmlinux.lds.h | 11 ++
> include/linux/of_reserved_mem.h | 65 ++++++++
> 7 files changed, 531 insertions(+)
> create mode 100644 drivers/of/of_reserved_mem.c
> create mode 100644 include/linux/of_reserved_mem.h

Hi Marek,

There's a lot of moving parts in this patch. Can you split the patch up a bit please. There are parts that I'm not entierly comfortable with yet and it will help reviewing them if they are added separately. For instance, the attaching regions to devices is something that I want to have some discussion about, but the core reserving static ranges I think is pretty much ready to be merged. I can merge the later while still debating the former if they are split.

I would recommend splitting into four:
1) reservation of static regions without the support code for referencing them later
2) code to also do dynamic allocations of reserved regions - again without any driver references
3) add hooks to reference specific regions.
4) hooks into drivers/of/platform.c for wiring into the driver model.

Can you also make the binding doc the first patch?

>
> diff --git a/drivers/of/Kconfig b/drivers/of/Kconfig
> index c6973f101a3e..30a7d87a8077 100644
> --- a/drivers/of/Kconfig
> +++ b/drivers/of/Kconfig
> @@ -75,4 +75,10 @@ config OF_MTD
> depends on MTD
> def_bool y
>
> +config OF_RESERVED_MEM
> + depends on OF_EARLY_FLATTREE
> + bool
> + help
> + Helpers to allow for reservation of memory regions
> +
> endmenu # OF
> diff --git a/drivers/of/Makefile b/drivers/of/Makefile
> index efd05102c405..ed9660adad77 100644
> --- a/drivers/of/Makefile
> +++ b/drivers/of/Makefile
> @@ -9,3 +9,4 @@ obj-$(CONFIG_OF_MDIO) += of_mdio.o
> obj-$(CONFIG_OF_PCI) += of_pci.o
> obj-$(CONFIG_OF_PCI_IRQ) += of_pci_irq.o
> obj-$(CONFIG_OF_MTD) += of_mtd.o
> +obj-$(CONFIG_OF_RESERVED_MEM) += of_reserved_mem.o
> diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
> index 758b4f8b30b7..04efe2ba736f 100644
> --- a/drivers/of/fdt.c
> +++ b/drivers/of/fdt.c
> @@ -15,6 +15,7 @@
> #include <linux/module.h>
> #include <linux/of.h>
> #include <linux/of_fdt.h>
> +#include <linux/of_reserved_mem.h>
> #include <linux/string.h>
> #include <linux/errno.h>
> #include <linux/slab.h>
> @@ -438,6 +439,150 @@ int __initdata dt_root_size_cells;
> struct boot_param_header *initial_boot_params;
>
> #ifdef CONFIG_OF_EARLY_FLATTREE
> +#if defined(CONFIG_HAVE_MEMBLOCK)
> +int __init __weak
> +early_init_dt_reserve_memory_arch(phys_addr_t base, phys_addr_t size,
> + bool nomap)
> +{
> + if (memblock_is_region_reserved(base, size))
> + return -EBUSY;
> + if (nomap)
> + return memblock_remove(base, size);
> + return memblock_reserve(base, size);
> +}
> +#else
> +int __init __weak
> +early_init_dt_reserve_memory_arch(phys_addr_t base, phys_addr_t size,
> + bool nomap)
> +{
> + pr_error("Reserved memory not supported, ignoring range 0x%llx - 0x%llx%s\n",
> + base, size, nomap ? " (nomap)" : "");
> + return -ENOSYS;
> +}
> +#endif

Group the above with the early_init_dt_add_memory_arch() and
early_init_dt_alloc_memory_arch() hooks.

> +
> +/**
> + * res_mem_reserve_reg() - reserve all memory described in 'reg' property
> + */
> +static int __init
> +__reserved_mem_reserve_reg(unsigned long node, const char *uname,
> + phys_addr_t *res_base, phys_addr_t *res_size)

Nit: put the funciton name on the same line as "static int __init". It's
more grep friendly that way and is the style used by fdt.c

> +{
> + int t_len = (dt_root_addr_cells + dt_root_size_cells) * sizeof(__be32);
> + phys_addr_t base, size;
> + unsigned long len;
> + __be32 *prop;
> + int nomap;
> +
> + prop = of_get_flat_dt_prop(node, "reg", &len);
> + if (!prop)
> + return -ENOENT;
> +
> + if (len && len % t_len != 0) {
> + pr_err("Reserved memory: invalid reg property in '%s', skipping node.\n",
> + uname);
> + return -EINVAL;
> + }
> +
> + nomap = of_get_flat_dt_prop(node, "no-map", NULL) != NULL;
> +
> + /* store base and size values from the first reg tuple */
> + *res_base = 0;
> + while (len > 0) {
> + base = dt_mem_next_cell(dt_root_addr_cells, &prop);
> + size = dt_mem_next_cell(dt_root_size_cells, &prop);
> +
> + if (base && size &&
> + early_init_dt_reserve_memory_arch(base, size, nomap) == 0)
> + pr_debug("Reserved memory: reserved region for node '%s': base %pa, size %ld MiB\n",
> + uname, &base, (unsigned long)size / SZ_1M);
> + else
> + pr_info("Reserved memory: failed to reserve memory for node '%s': base %pa, size %ld MiB\n",
> + uname, &base, (unsigned long)size / SZ_1M);
> +
> + len -= t_len;
> +
> + if (!(*res_base)) {
> + *res_base = base;
> + *res_size = size;
> + }
> + }
> + return 0;
> +}
> +
> +static int __reserved_mem_check_root(unsigned long node)
> +{
> + __be32 *prop;
> +
> + prop = of_get_flat_dt_prop(node, "#size-cells", NULL);
> + if (prop && be32_to_cpup(prop) != dt_root_size_cells)
> + return -EINVAL;
> +
> + prop = of_get_flat_dt_prop(node, "#address-cells", NULL);
> + if (prop && be32_to_cpup(prop) != dt_root_addr_cells)
> + return -EINVAL;
> +
> + prop = of_get_flat_dt_prop(node, "ranges", NULL);
> + if (!prop)
> + return -EINVAL;
> + return 0;
> +}
> +
> +/**
> + * fdt_scan_reserved_mem() - scan a single FDT node for reserved memory
> + */
> +static int __init
> +__fdt_scan_reserved_mem(unsigned long node, const char *uname, int depth,
> + void *data)
> +{
> + phys_addr_t base = 0, size = 0;
> + static int found;
> + const char *status;
> + int err;
> +
> + if (!found && depth == 1 && strcmp(uname, "reserved-memory") == 0) {
> + if (__reserved_mem_check_root(node) != 0) {
> + pr_err("Reserved memory: unsupported node format, ignoring\n");
> + /* break scan */
> + return 1;
> + }
> + found = 1;
> + /* scan next node */
> + return 0;
> + } else if (!found) {
> + /* scan next node */
> + return 0;
> + } else if (found && depth < 2) {
> + /* scanning of /reserved-memory has been finished */
> + return 1;
> + }
> +
> + status = of_get_flat_dt_prop(node, "status", NULL);
> + if (status && strcmp(status, "okay") != 0 && strcmp(status, "ok") != 0)
> + return 0;
> +
> + err = __reserved_mem_reserve_reg(node, uname, &base, &size);
> + if (err == -ENOENT && of_get_flat_dt_prop(node, "size", NULL) == NULL)
> + goto end;
> +
> + fdt_reserved_mem_save_node(node, uname, base, size);

There is only one path here and the fdt_reserved_mem_save_node() call is
the only user of base,size. Why not move the hook directly into
__reserved_mem_reserve_reg() and drop the &base/&size arguments? I'm
finding the logic a little convoluted.

For that matter, why split it into a separate function at all?

Otherwise, all of the above code loks good to me. I like the way you've
done the early parsing. It will eventually hook neatly into
early_init_dt_scan().

(I am ignoring the question of whether child nodes should be processed.
that is a separate debate and the code can be extended later).

> +end:
> + /* scan next node */
> + return 0;
> +}
> +
> +/**
> + * early_init_fdt_scan_reserved_mem() - create reserved memory regions
> + *
> + * This function grabs memory from early allocator for device exclusive use
> + * defined in device tree structures. It should be called by arch specific code
> + * once the early allocator (i.e. memblock) has been fully activated.
> + */
> +void __init early_init_fdt_scan_reserved_mem(void)
> +{
> + of_scan_flat_dt(__fdt_scan_reserved_mem, NULL);
> + fdt_init_reserved_mem();
> +}
>
> /**
> * of_scan_flat_dt - scan flattened tree blob and call callback on each.
> diff --git a/drivers/of/of_reserved_mem.c b/drivers/of/of_reserved_mem.c
> new file mode 100644
> index 000000000000..cacf04810b87
> --- /dev/null
> +++ b/drivers/of/of_reserved_mem.c
> @@ -0,0 +1,296 @@
> +/*
> + * Device tree based initialization code for reserved memory.
> + *
> + * Copyright (c) 2013, The Linux Foundation. All Rights Reserved.
> + * Copyright (c) 2013,2014 Samsung Electronics Co., Ltd.
> + * http://www.samsung.com
> + * Author: Marek Szyprowski <[email protected]>
> + * Author: Josh Cartwright <[email protected]>
> + *
> + * 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 optional) any later version of the license.
> + */
> +
> +#include <linux/err.h>
> +#include <linux/of.h>
> +#include <linux/of_fdt.h>
> +#include <linux/of_platform.h>
> +#include <linux/mm.h>
> +#include <linux/sizes.h>
> +#include <linux/of_reserved_mem.h>
> +
> +#define MAX_RESERVED_REGIONS 16
> +static struct reserved_mem reserved_mem[MAX_RESERVED_REGIONS];
> +static int reserved_mem_count;
> +
> +#if defined(CONFIG_HAVE_MEMBLOCK)
> +#include <linux/memblock.h>
> +int __init __weak
> +early_init_dt_alloc_reserved_memory_arch(phys_addr_t size, phys_addr_t align,
> + phys_addr_t start, phys_addr_t end,
> + bool nomap, phys_addr_t *res_base)
> +{
> + /*
> + * We use __memblock_alloc_base() since memblock_alloc_base() panic()s.
> + */

Why does it panic?

> + phys_addr_t base = __memblock_alloc_base(size, align, end);
> + if (!base)
> + return -ENOMEM;
> +
> + if (base < start) {

The above test could use an explanitory comment.

> + memblock_free(base, size);
> + return -ENOMEM;
> + }
> +
> + *res_base = base;
> + if (nomap)
> + return memblock_remove(base, size);
> + return 0;
> +}
> +#else
> +int __init __weak
> +early_init_dt_alloc_reserved_memory_arch(phys_addr_t size, phys_addr_t align,
> + phys_addr_t start, phys_addr_t end,
> + bool nomap, phys_addr_t *res_base)
> +{
> + pr_error("Reserved memory not supported, ignoring region 0x%llx%s\n",
> + size, nomap ? " (nomap)" : "");
> + return -ENOSYS;
> +}
> +#endif
> +
> +/**
> + * res_mem_save_node() - save fdt node for second pass initialization
> + */
> +int __init fdt_reserved_mem_save_node(unsigned long node, const char *uname,
> + phys_addr_t base, phys_addr_t size)

The return code is never used. Return void instead.

> +{
> + struct reserved_mem *rmem = &reserved_mem[reserved_mem_count];
> +
> + if (reserved_mem_count == ARRAY_SIZE(reserved_mem)) {
> + pr_err("Reserved memory: not enough space all defined regions.\n");
> + return -ENOSPC;
> + }
> +
> + rmem->fdt_node = node;
> + rmem->name = uname;
> + rmem->base = base;
> + rmem->size = size;
> +
> + reserved_mem_count++;
> + return 0;
> +}
> +
> +/**
> + * res_mem_alloc_size() - allocate reserved memory described by 'size', 'align'
> + * and 'alloc-ranges' properties
> + */
> +static int __init
> +__reserved_mem_alloc_size(unsigned long node, const char *uname,
> + phys_addr_t *res_base, phys_addr_t *res_size)
> +{
> + int t_len = (dt_root_addr_cells + dt_root_size_cells) * sizeof(__be32);
> + phys_addr_t start = 0, end = 0;
> + phys_addr_t base = 0, align = 0, size;
> + unsigned long len;
> + __be32 *prop;
> + int nomap;
> + int ret;
> +
> + prop = of_get_flat_dt_prop(node, "size", &len);
> + if (!prop)
> + return -EINVAL;
> +
> + if (len != dt_root_size_cells * sizeof(__be32)) {
> + pr_err("Reserved memory: invalid size property in '%s' node.\n",
> + uname);
> + return -EINVAL;
> + }
> + size = dt_mem_next_cell(dt_root_size_cells, &prop);
> +
> + nomap = of_get_flat_dt_prop(node, "no-map", NULL) != NULL;
> +
> + prop = of_get_flat_dt_prop(node, "align", &len);
> + if (prop) {
> + if (len != dt_root_addr_cells * sizeof(__be32)) {
> + pr_err("Reserved memory: invalid align property in '%s' node.\n",
> + uname);
> + return -EINVAL;
> + }
> + align = dt_mem_next_cell(dt_root_addr_cells, &prop);
> + }
> +
> + prop = of_get_flat_dt_prop(node, "alloc-ranges", &len);
> + if (prop) {
> +
> + if (len % t_len != 0) {
> + pr_err("Reserved memory: invalid alloc-ranges property in '%s', skipping node.\n",
> + uname);
> + return -EINVAL;
> + }
> +
> + base = 0;
> +
> + while (len > 0) {
> + start = dt_mem_next_cell(dt_root_addr_cells, &prop);
> + end = start + dt_mem_next_cell(dt_root_size_cells,
> + &prop);
> +
> + ret = early_init_dt_alloc_reserved_memory_arch(size,
> + align, start, end, nomap, &base);
> + if (ret == 0) {
> + pr_debug("Reserved memory: allocated memory for '%s' node: base %pa, size %ld MiB\n",
> + uname, &base,
> + (unsigned long)size / SZ_1M);
> + break;
> + }
> + len -= t_len;
> + }
> +
> + } else {
> + ret = early_init_dt_alloc_reserved_memory_arch(size, align,
> + 0, 0, nomap, &base);
> + if (ret == 0)
> + pr_debug("Reserved memory: allocated memory for '%s' node: base %pa, size %ld MiB\n",
> + uname, &base, (unsigned long)size / SZ_1M);
> + }
> +
> + if (base == 0) {
> + pr_info("Reserved memory: failed to allocate memory for node '%s'\n",
> + uname);
> + return -ENOMEM;
> + }

<off topic> Wow, the flattree parsing code has to be really verbose. We really need better
flat tree parsing functions and helpers.

> +
> + *res_base = base;
> + *res_size = size;
> +
> + return 0;
> +}
> +
> +static const struct of_device_id __rmem_of_table_sentinel
> + __used __section(__reservedmem_of_table_end);
> +
> +/**
> + * res_mem_init_node() - call region specific reserved memory init code
> + */
> +static int __init __reserved_mem_init_node(struct reserved_mem *rmem)
> +{
> + extern const struct of_device_id __reservedmem_of_table[];
> + const struct of_device_id *i;
> +
> + for (i = __reservedmem_of_table; i < &__rmem_of_table_sentinel; i++) {
> + reservedmem_of_init_fn initfn = i->data;
> + const char *compat = i->compatible;
> +
> + if (!of_flat_dt_is_compatible(rmem->fdt_node, compat))
> + continue;

What if two entries both match the compatible list? Ideally score would
be taken into account. (I won't block on this issue, it can be a future
enhancement)

> +
> + if (initfn(rmem, rmem->fdt_node, rmem->name) == 0) {
> + pr_info("Reserved memory: initialized node %s, compatible id %s\n",
> + rmem->name, compat);
> + return 0;
> + }
> + }
> + return -ENOENT;
> +}
> +
> +/**
> + * fdt_init_reserved_mem - allocate and init all saved reserved memory regions
> + */
> +void __init fdt_init_reserved_mem(void)
> +{
> + int i;
> + for (i = 0; i < reserved_mem_count; i++) {
> + struct reserved_mem *rmem = &reserved_mem[i];
> + unsigned long node = rmem->fdt_node;
> + unsigned long len;
> + __be32 *prop;
> + int err = 0;
> +
> + prop = of_get_flat_dt_prop(node, "phandle", &len);
> + if (!prop)
> + prop = of_get_flat_dt_prop(node, "linux,phandle", &len);
> + if (prop)
> + rmem->phandle = of_read_number(prop, len/4);
> +
> + if (rmem->size == 0)
> + err = __reserved_mem_alloc_size(node, rmem->name,
> + &rmem->base, &rmem->size);
> + if (err == 0)
> + __reserved_mem_init_node(rmem);
> + }
> +}
> +
> +static inline struct reserved_mem *__find_rmem(struct device_node *node)
> +{
> + unsigned int len, phandle_val;
> + const __be32 *prop;
> + unsigned int i;
> +
> + prop = of_get_property(node, "phandle", &len);
> + if (!prop)
> + prop = of_get_property(node, "linux,phandle", &len);
> + if (!prop || len < sizeof(__be32))
> + return NULL;
> +
> + phandle_val = be32_to_cpup(prop);

The above gymnastics aren't needed. phandle is already stored in
node->phandle. You still need to check for a 0 phandle though.

> + for (i = 0; i < reserved_mem_count; i++)
> + if (reserved_mem[i].phandle == phandle_val)
> + return &reserved_mem[i];
> + return NULL;
> +}
> +
> +/**
> + * of_reserved_mem_device_init() - assign reserved memory region to given device
> + *
> + * This function assign memory region pointed by "memory-region" device tree
> + * property to the given device.
> + */
> +void of_reserved_mem_device_init(struct device *dev)
> +{
> + struct device_node *np = dev->of_node;
> + struct reserved_mem *rmem;
> + struct of_phandle_args s;
> + unsigned int i;
> +
> + for (i = 0; of_parse_phandle_with_args(np, "memory-region",
> + "#memory-region-cells", i, &s) == 0; i++) {
> +
> + rmem = __find_rmem(s.np);
> + if (!rmem || !rmem->ops || !rmem->ops->device_init) {
> + of_node_put(s.np);
> + continue;
> + }
> +
> + rmem->ops->device_init(rmem, dev, &s);
> + dev_info(dev, "assigned reserved memory node %s\n", rmem->name);
> + of_node_put(s.np);
> + break;
> + }
> +}
> +
> +/**
> + * of_reserved_mem_device_release() - release reserved memory device structures
> + *
> + * This function releases structures allocated for memory region handling for
> + * the given device.
> + */
> +void of_reserved_mem_device_release(struct device *dev)
> +{
> + struct device_node *np = dev->of_node;
> + struct reserved_mem *rmem;
> + struct of_phandle_args s;
> + unsigned int i;
> +
> + for (i = 0; of_parse_phandle_with_args(np, "memory-region",
> + "#memory-region-cells", i, &s) == 0; i++) {
> +
> + rmem = __find_rmem(s.np);
> + if (rmem && rmem->ops && rmem->ops->device_release)
> + rmem->ops->device_release(rmem, dev);
> +
> + of_node_put(s.np);
> + }
> +}
> diff --git a/drivers/of/platform.c b/drivers/of/platform.c
> index 404d1daebefa..3df0b1826e8b 100644
> --- a/drivers/of/platform.c
> +++ b/drivers/of/platform.c
> @@ -21,6 +21,7 @@
> #include <linux/of_device.h>
> #include <linux/of_irq.h>
> #include <linux/of_platform.h>
> +#include <linux/of_reserved_mem.h>
> #include <linux/platform_device.h>
>
> const struct of_device_id of_default_bus_match_table[] = {
> @@ -220,6 +221,8 @@ static struct platform_device *of_platform_device_create_pdata(
> dev->dev.bus = &platform_bus_type;
> dev->dev.platform_data = platform_data;
>
> + of_reserved_mem_device_init(&dev->dev);
> +
> /* We do not fill the DMA ops for platform devices by default.
> * This is currently the responsibility of the platform code
> * to do such, possibly using a device notifier
> @@ -227,6 +230,7 @@ static struct platform_device *of_platform_device_create_pdata(
>
> if (of_device_add(dev) != 0) {
> platform_device_put(dev);
> + of_reserved_mem_device_release(&dev->dev);
> return NULL;
> }
>
> @@ -282,6 +286,8 @@ static struct amba_device *of_amba_device_create(struct device_node *node,
> else
> of_device_make_bus_id(&dev->dev);
>
> + of_reserved_mem_device_init(&dev->dev);
> +
> /* Allow the HW Peripheral ID to be overridden */
> prop = of_get_property(node, "arm,primecell-periphid", NULL);
> if (prop)
> @@ -308,6 +314,7 @@ static struct amba_device *of_amba_device_create(struct device_node *node,
> return dev;
>
> err_free:
> + of_reserved_mem_device_release(&dev->dev);
> amba_device_put(dev);
> return NULL;
> }
> diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
> index bc2121fa9132..f10f64fcc815 100644
> --- a/include/asm-generic/vmlinux.lds.h
> +++ b/include/asm-generic/vmlinux.lds.h
> @@ -167,6 +167,16 @@
> #define CLK_OF_TABLES()
> #endif
>
> +#ifdef CONFIG_OF_RESERVED_MEM
> +#define RESERVEDMEM_OF_TABLES() \
> + . = ALIGN(8); \
> + VMLINUX_SYMBOL(__reservedmem_of_table) = .; \
> + *(__reservedmem_of_table) \
> + *(__reservedmem_of_table_end)
> +#else
> +#define RESERVEDMEM_OF_TABLES()
> +#endif
> +
> #define KERNEL_DTB() \
> STRUCT_ALIGN(); \
> VMLINUX_SYMBOL(__dtb_start) = .; \
> @@ -490,6 +500,7 @@
> TRACE_SYSCALLS() \
> MEM_DISCARD(init.rodata) \
> CLK_OF_TABLES() \
> + RESERVEDMEM_OF_TABLES() \
> CLKSRC_OF_TABLES() \
> KERNEL_DTB() \
> IRQCHIP_OF_MATCH_TABLE()
> diff --git a/include/linux/of_reserved_mem.h b/include/linux/of_reserved_mem.h
> new file mode 100644
> index 000000000000..0bbec4bf23ce
> --- /dev/null
> +++ b/include/linux/of_reserved_mem.h
> @@ -0,0 +1,65 @@
> +#ifndef __OF_RESERVED_MEM_H
> +#define __OF_RESERVED_MEM_H
> +
> +struct cma;
> +struct device;
> +struct of_phandle_args;
> +struct reserved_mem_ops;
> +
> +struct reserved_mem {
> + const char *name;
> + unsigned long fdt_node;
> + unsigned long phandle;
> + const struct reserved_mem_ops *ops;
> + phys_addr_t base;
> + phys_addr_t size;
> + void *priv;
> +};
> +
> +struct reserved_mem_ops {
> + void (*device_init)(struct reserved_mem *rmem,
> + struct device *dev,
> + struct of_phandle_args *args);
> + void (*device_release)(struct reserved_mem *rmem,
> + struct device *dev);
> +};
> +
> +typedef int (*reservedmem_of_init_fn)(struct reserved_mem *rmem,
> + unsigned long node, const char *uname);
> +
> +#ifdef CONFIG_OF_RESERVED_MEM
> +void of_reserved_mem_device_init(struct device *dev);
> +void of_reserved_mem_device_release(struct device *dev);
> +
> +void fdt_init_reserved_mem(void);
> +void early_init_fdt_scan_reserved_mem(void);

The early_init_fdt_scan_reserved_mem() stub should be in of_fdt.h

> +int fdt_reserved_mem_save_node(unsigned long node, const char *uname,
> + phys_addr_t base, phys_addr_t size);
> +
> +#define RESERVEDMEM_OF_DECLARE(name, compat, init) \
> + static const struct of_device_id __reservedmem_of_table_##name \
> + __used __section(__reservedmem_of_table) \
> + = { .compatible = compat, \
> + .data = (init == (reservedmem_of_init_fn)NULL) ? \
> + init : init }
> +
> +#else
> +static inline void of_reserved_mem_device_init(struct device *dev) { }
> +static inline void of_reserved_mem_device_release(struct device *pdev) { }
> +
> +static inline void fdt_init_reserved_mem(void) { }
> +static inline void early_init_fdt_scan_reserved_mem(void) { }

early_init_fdt_scan_reserved_mem() should not have an empty stub.

> +static inline int
> +fdt_reserved_mem_save_node(unsigned long node, const char *uname,
> + phys_addr_t base, phys_addr_t size) { }
> +
> +#define RESERVEDMEM_OF_DECLARE(name, compat, init) \
> + static const struct of_device_id __reservedmem_of_table_##name \
> + __attribute__((unused)) \
> + = { .compatible = compat, \
> + .data = (init == (reservedmem_of_init_fn)NULL) ? \
> + init : init }
> +
> +#endif
> +
> +#endif /* __OF_RESERVED_MEM_H */
> --
> 1.7.9.5
>

2014-02-21 11:00:51

by Marek Szyprowski

[permalink] [raw]
Subject: Re: [PATCH v4 1/7] drivers: of: add initialization code for reserved memory

Hello,

On 2014-02-20 15:01, Grant Likely wrote:
> On Thu, 20 Feb 2014 11:52:41 +0100, Marek Szyprowski <[email protected]> wrote:
> > This patch adds device tree support for contiguous and reserved memory
> > regions defined in device tree.
> >
> > Large memory blocks can be reliably reserved only during early boot.
> > This must happen before the whole memory management subsystem is
> > initialized, because we need to ensure that the given contiguous blocks
> > are not yet allocated by kernel. Also it must happen before kernel
> > mappings for the whole low memory are created, to ensure that there will
> > be no mappings (for reserved blocks) or mapping with special properties
> > can be created (for CMA blocks). This all happens before device tree
> > structures are unflattened, so we need to get reserved memory layout
> > directly from fdt.
> >
> > Later, those reserved memory regions are assigned to devices on each
> > device structure initialization.
> >
> > Signed-off-by: Marek Szyprowski <[email protected]>
> > [joshc: rework to implement new DT binding, provide mechanism for
> > plugging in new reserved-memory node handlers via
> > RESERVEDMEM_OF_DECLARE]
> > Signed-off-by: Josh Cartwright <[email protected]>
> > [mszyprow: added generic memory reservation code, refactored code to
> > put it directly into fdt.c]
> > Signed-off-by: Marek Szyprowski <[email protected]>
> > ---
> > drivers/of/Kconfig | 6 +
> > drivers/of/Makefile | 1 +
> > drivers/of/fdt.c | 145 ++++++++++++++++++
> > drivers/of/of_reserved_mem.c | 296 +++++++++++++++++++++++++++++++++++++
> > drivers/of/platform.c | 7 +
> > include/asm-generic/vmlinux.lds.h | 11 ++
> > include/linux/of_reserved_mem.h | 65 ++++++++
> > 7 files changed, 531 insertions(+)
> > create mode 100644 drivers/of/of_reserved_mem.c
> > create mode 100644 include/linux/of_reserved_mem.h
>
> Hi Marek,
>
> There's a lot of moving parts in this patch. Can you split the patch up a bit please. There are parts that I'm not entierly comfortable with yet and it will help reviewing them if they are added separately. For instance, the attaching regions to devices is something that I want to have some discussion about, but the core reserving static ranges I think is pretty much ready to be merged. I can merge the later while still debating the former if they are split.
>
> I would recommend splitting into four:
> 1) reservation of static regions without the support code for referencing them later
> 2) code to also do dynamic allocations of reserved regions - again without any driver references
> 3) add hooks to reference specific regions.
> 4) hooks into drivers/of/platform.c for wiring into the driver model.
>
> Can you also make the binding doc the first patch?

Ok, I will slice the patch into 4 pieces.

(snipped)

> > +/**
> > + * res_mem_alloc_size() - allocate reserved memory described by 'size', 'align'
> > + * and 'alloc-ranges' properties
> > + */
> > +static int __init
> > +__reserved_mem_alloc_size(unsigned long node, const char *uname,
> > + phys_addr_t *res_base, phys_addr_t *res_size)
> > +{
> > + int t_len = (dt_root_addr_cells + dt_root_size_cells) * sizeof(__be32);
> > + phys_addr_t start = 0, end = 0;
> > + phys_addr_t base = 0, align = 0, size;
> > + unsigned long len;
> > + __be32 *prop;
> > + int nomap;
> > + int ret;
> > +
> > + prop = of_get_flat_dt_prop(node, "size", &len);
> > + if (!prop)
> > + return -EINVAL;
> > +
> > + if (len != dt_root_size_cells * sizeof(__be32)) {
> > + pr_err("Reserved memory: invalid size property in '%s' node.\n",
> > + uname);
> > + return -EINVAL;
> > + }
> > + size = dt_mem_next_cell(dt_root_size_cells, &prop);
> > +
> > + nomap = of_get_flat_dt_prop(node, "no-map", NULL) != NULL;
> > +
> > + prop = of_get_flat_dt_prop(node, "align", &len);
> > + if (prop) {
> > + if (len != dt_root_addr_cells * sizeof(__be32)) {
> > + pr_err("Reserved memory: invalid align property in '%s' node.\n",
> > + uname);
> > + return -EINVAL;
> > + }
> > + align = dt_mem_next_cell(dt_root_addr_cells, &prop);
> > + }
> > +
> > + prop = of_get_flat_dt_prop(node, "alloc-ranges", &len);
> > + if (prop) {
> > +
> > + if (len % t_len != 0) {
> > + pr_err("Reserved memory: invalid alloc-ranges property in '%s', skipping node.\n",
> > + uname);
> > + return -EINVAL;
> > + }
> > +
> > + base = 0;
> > +
> > + while (len > 0) {
> > + start = dt_mem_next_cell(dt_root_addr_cells, &prop);
> > + end = start + dt_mem_next_cell(dt_root_size_cells,
> > + &prop);
> > +
> > + ret = early_init_dt_alloc_reserved_memory_arch(size,
> > + align, start, end, nomap, &base);
> > + if (ret == 0) {
> > + pr_debug("Reserved memory: allocated memory for '%s' node: base %pa, size %ld MiB\n",
> > + uname, &base,
> > + (unsigned long)size / SZ_1M);
> > + break;
> > + }
> > + len -= t_len;
> > + }
> > +
> > + } else {
> > + ret = early_init_dt_alloc_reserved_memory_arch(size, align,
> > + 0, 0, nomap, &base);
> > + if (ret == 0)
> > + pr_debug("Reserved memory: allocated memory for '%s' node: base %pa, size %ld MiB\n",
> > + uname, &base, (unsigned long)size / SZ_1M);
> > + }
> > +
> > + if (base == 0) {
> > + pr_info("Reserved memory: failed to allocate memory for node '%s'\n",
> > + uname);
> > + return -ENOMEM;
> > + }
>
> <off topic> Wow, the flattree parsing code has to be really verbose. We really need better
> flat tree parsing functions and helpers.

Yes, parsing fdt is a real pain, but please don't ask me to implement
all the
helpers to make it easier together with this patch. I (and probably other
developers) would really like to get this piece merged asap.

> > +
> > + *res_base = base;
> > + *res_size = size;
> > +
> > + return 0;
> > +}
> > +
> > +static const struct of_device_id __rmem_of_table_sentinel
> > + __used __section(__reservedmem_of_table_end);
> > +
> > +/**
> > + * res_mem_init_node() - call region specific reserved memory init code
> > + */
> > +static int __init __reserved_mem_init_node(struct reserved_mem *rmem)
> > +{
> > + extern const struct of_device_id __reservedmem_of_table[];
> > + const struct of_device_id *i;
> > +
> > + for (i = __reservedmem_of_table; i < &__rmem_of_table_sentinel; i++) {
> > + reservedmem_of_init_fn initfn = i->data;
> > + const char *compat = i->compatible;
> > +
> > + if (!of_flat_dt_is_compatible(rmem->fdt_node, compat))
> > + continue;
>
> What if two entries both match the compatible list? Ideally score would
> be taken into account. (I won't block on this issue, it can be a future
> enhancement)

If two entries have same compatible value they will be probed in the order
of presence in the kernel binary. The return value is checked and the next
one is being tried if init fails for the given function. The provided code
already makes use of this feature. Both DMA coherent and CMA use
"shared-dma-pool" compatible. DMA coherent init fails if 'reusable'
property has been found. On the other hand, CMA fails initialization if
'reusable' property is missing. Frankly I would like to change standard
DMA coherent compatible value to 'dma-pool' and keep 'shared-dma-pool'
only for CMA, but I've implemented it the way it has been described in
your binding documentation.

(snipped)

Thanks for your comments, I will send updated patches asap.

Best regards
--
Marek Szyprowski, PhD
Samsung R&D Institute Poland

2014-02-21 13:38:20

by Grant Likely

[permalink] [raw]
Subject: Re: [PATCH v4 1/7] drivers: of: add initialization code for reserved memory

On Fri, 21 Feb 2014 12:00:44 +0100, Marek Szyprowski <[email protected]> wrote:
> Hello,
>
> On 2014-02-20 15:01, Grant Likely wrote:
> > On Thu, 20 Feb 2014 11:52:41 +0100, Marek Szyprowski <[email protected]> wrote:
> > > This patch adds device tree support for contiguous and reserved memory
> > > regions defined in device tree.
> > >
> > > Large memory blocks can be reliably reserved only during early boot.
> > > This must happen before the whole memory management subsystem is
> > > initialized, because we need to ensure that the given contiguous blocks
> > > are not yet allocated by kernel. Also it must happen before kernel
> > > mappings for the whole low memory are created, to ensure that there will
> > > be no mappings (for reserved blocks) or mapping with special properties
> > > can be created (for CMA blocks). This all happens before device tree
> > > structures are unflattened, so we need to get reserved memory layout
> > > directly from fdt.
> > >
> > > Later, those reserved memory regions are assigned to devices on each
> > > device structure initialization.
> > >
> > > Signed-off-by: Marek Szyprowski <[email protected]>
> > > [joshc: rework to implement new DT binding, provide mechanism for
> > > plugging in new reserved-memory node handlers via
> > > RESERVEDMEM_OF_DECLARE]
> > > Signed-off-by: Josh Cartwright <[email protected]>
> > > [mszyprow: added generic memory reservation code, refactored code to
> > > put it directly into fdt.c]
> > > Signed-off-by: Marek Szyprowski <[email protected]>
> > > ---
> > > drivers/of/Kconfig | 6 +
> > > drivers/of/Makefile | 1 +
> > > drivers/of/fdt.c | 145 ++++++++++++++++++
> > > drivers/of/of_reserved_mem.c | 296 +++++++++++++++++++++++++++++++++++++
> > > drivers/of/platform.c | 7 +
> > > include/asm-generic/vmlinux.lds.h | 11 ++
> > > include/linux/of_reserved_mem.h | 65 ++++++++
> > > 7 files changed, 531 insertions(+)
> > > create mode 100644 drivers/of/of_reserved_mem.c
> > > create mode 100644 include/linux/of_reserved_mem.h
> >
> > Hi Marek,
> >
> > There's a lot of moving parts in this patch. Can you split the patch up a bit please. There are parts that I'm not entierly comfortable with yet and it will help reviewing them if they are added separately. For instance, the attaching regions to devices is something that I want to have some discussion about, but the core reserving static ranges I think is pretty much ready to be merged. I can merge the later while still debating the former if they are split.
> >
> > I would recommend splitting into four:
> > 1) reservation of static regions without the support code for referencing them later
> > 2) code to also do dynamic allocations of reserved regions - again without any driver references
> > 3) add hooks to reference specific regions.
> > 4) hooks into drivers/of/platform.c for wiring into the driver model.
> >
> > Can you also make the binding doc the first patch?
>
> Ok, I will slice the patch into 4 pieces.
> >
> > <off topic> Wow, the flattree parsing code has to be really verbose. We really need better
> > flat tree parsing functions and helpers.
>
> Yes, parsing fdt is a real pain, but please don't ask me to implement
> all the
> helpers to make it easier together with this patch. I (and probably other
> developers) would really like to get this piece merged asap.

I won't. Mostly I was thinking out loud.

> > > + for (i = __reservedmem_of_table; i < &__rmem_of_table_sentinel; i++) {
> > > + reservedmem_of_init_fn initfn = i->data;
> > > + const char *compat = i->compatible;
> > > +
> > > + if (!of_flat_dt_is_compatible(rmem->fdt_node, compat))
> > > + continue;
> >
> > What if two entries both match the compatible list? Ideally score would
> > be taken into account. (I won't block on this issue, it can be a future
> > enhancement)
>
> If two entries have same compatible value they will be probed in the order
> of presence in the kernel binary. The return value is checked and the next
> one is being tried if init fails for the given function. The provided code
> already makes use of this feature. Both DMA coherent and CMA use
> "shared-dma-pool" compatible. DMA coherent init fails if 'reusable'
> property has been found. On the other hand, CMA fails initialization if
> 'reusable' property is missing. Frankly I would like to change standard
> DMA coherent compatible value to 'dma-pool' and keep 'shared-dma-pool'
> only for CMA, but I've implemented it the way it has been described in
> your binding documentation.

My binding document isn't gospel and it hasn't been merged yet. Reply to
the binding patch and make your argument for the change.

g.