* [PATCH v5 0/3] Device Tree support for CMA (Contiguous Memory Allocator)
@ 2013-08-09 11:51 Marek Szyprowski
2013-08-09 11:51 ` [PATCH v5 1/3] drivers: dma-contiguous: clean source code and prepare for device tree Marek Szyprowski
` (3 more replies)
0 siblings, 4 replies; 14+ messages in thread
From: Marek Szyprowski @ 2013-08-09 11:51 UTC (permalink / raw)
To: linux-arm-kernel
Hello,
This is a fifth version of my proposal for device tree integration for
reserved memory and Contiguous Memory Allocator. I've fixed issues
pointed by Michal Nazarewicz, Rob Herring and Kumar Gala. For more
information, please check the change log at the end of the mail.
In fourth version of this patch set, after the comments from Grant
Likely I moved back memory region definitions back to /memory node (as
it was in the first version of this proposal). I've also extended the
code and made it more generic, added support for so called reserved dma
memory (special dma memory regions created by dma_alloc_coherent()
function, for exclusive usage for dma allocation for the given device).
Just a few words for those who see this code for the first time:
The proposed bindings allows to define contiguous memory regions of
specified base address and size. Then, the defined regions can be
assigned to the given device(s) by adding a property with a phanle to
the defined contiguous memory region. From the device tree perspective
that's all. Once the bindings are added, all the memory allocations from
dma-mapping subsystem will be served from the defined contiguous memory
regions.
Contiguous Memory Allocator is a framework, which lets to provide a
large contiguous memory buffers for (usually a multimedia) devices. The
contiguous memory is reserved during early boot and then shared with
kernel, which is allowed to allocate it for movable pages. Then, when
device driver requests a contigouous buffer, the framework migrates
movable pages out of contiguous region and gives it to the driver. When
device driver frees the buffer, it is added to kernel memory pool again.
For more information, please refer to commit c64be2bb1c6eb43c838b2c6d57
("drivers: add Contiguous Memory Allocator") and d484864dd96e1830e76895
(CMA merge commit).
Why we need device tree bindings for CMA at all?
Older ARM kernels used so called board-based initialization. Those board
files contained a definition of all hardware blocks available on the
target system and particular kernel and driver software configuration
selected by the board maintainer.
In the new approach the board files will be removed completely and
Device Tree approach is used to describe all hardware blocks available
on the target system. By definition, the bindings should be software
independent, so at least in theory it should be possible to use those
bindings with other operating systems than Linux kernel.
Reserved memory configuration belongs to the grey area. It might depend
on hardware restriction of the board or modules and low-level
configuration done by bootloader. Putting reserved and contiguous memory
regions to /memory node and having phandles to those regions in the
device nodes however matches well with the device-tree typical style of
linking devices with other resources like clocks, interrupts,
regulators, power domains, etc. This is the main reason to use such
approach instead of putting everything to /chosen node as it has been
proposed in v2 and v3.
Best regards
Marek Szyprowski
Samsung R&D Institute Poland
Changelog:
v5:
- renamed "contiguous-memory-region" compatibility string to
"linux,contiguous-memory-region" (this one is really specific to Linux
kernel implementation)
- renamed "dma-memory-region" property to "memory-region" (suggested by
Kumar Gala)
- added support for #address-cells, #size-cells for memory regions
(thanks to Rob Herring for suggestion)
- removed generic code to scan specific path in flat device tree (cannot
be used to fdt one-pass scan based initialization of memory regions with
#address-cells and #size-cells parsing)
- replaced dma_contiguous_set_default_area() and dma_contiguous_add_device()
functions with dev_set_cma_area() call
v4: http://thread.gmane.org/gmane.linux.ports.arm.kernel/256491
- corrected Devcie Tree mailing list address (resend)
- moved back contiguous-memory bindings from /chosen/contiguous-memory
to /memory nodes as suggested by Grant (see
http://article.gmane.org/gmane.linux.drivers.devicetree/41030
for more details)
- added support for DMA reserved memory with dma_declare_coherent()
- moved code to drivers/of/of_reserved_mem.c
- added generic code to scan specific path in flat device tree
v3: http://thread.gmane.org/gmane.linux.drivers.devicetree/40013/
- fixed issues pointed by Laura and updated documentation
v2: http://thread.gmane.org/gmane.linux.drivers.devicetree/34075
- moved contiguous-memory bindings from /memory to /chosen/contiguous-memory/
node to avoid spreading Linux specific parameters over the whole device
tree definitions
- added support for autoconfigured regions (use zero base)
- fixes minor bugs
v1: http://thread.gmane.org/gmane.linux.drivers.devicetree/30111/
- initial proposal
Patch summary:
Marek Szyprowski (3):
drivers: dma-contiguous: clean source code and prepare for device
tree
drivers: of: add initialization code for dma reserved memory
ARM: init: add support for reserved memory defined by device tree
Documentation/devicetree/bindings/memory.txt | 152 ++++++++++++++++++++
arch/arm/include/asm/dma-contiguous.h | 1 -
arch/arm/mm/init.c | 3 +
arch/x86/include/asm/dma-contiguous.h | 1 -
drivers/base/dma-contiguous.c | 119 ++++++----------
drivers/of/Kconfig | 6 +
drivers/of/Makefile | 1 +
drivers/of/of_reserved_mem.c | 197 ++++++++++++++++++++++++++
drivers/of/platform.c | 5 +
include/asm-generic/dma-contiguous.h | 28 ----
include/linux/dma-contiguous.h | 55 ++++++-
include/linux/of_reserved_mem.h | 14 ++
12 files changed, 475 insertions(+), 107 deletions(-)
create mode 100644 Documentation/devicetree/bindings/memory.txt
create mode 100644 drivers/of/of_reserved_mem.c
delete mode 100644 include/asm-generic/dma-contiguous.h
create mode 100644 include/linux/of_reserved_mem.h
--
1.7.9.5
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v5 1/3] drivers: dma-contiguous: clean source code and prepare for device tree
2013-08-09 11:51 [PATCH v5 0/3] Device Tree support for CMA (Contiguous Memory Allocator) Marek Szyprowski
@ 2013-08-09 11:51 ` Marek Szyprowski
2013-08-09 11:51 ` [PATCH v5 2/3] drivers: of: add initialization code for dma reserved memory Marek Szyprowski
` (2 subsequent siblings)
3 siblings, 0 replies; 14+ messages in thread
From: Marek Szyprowski @ 2013-08-09 11:51 UTC (permalink / raw)
To: linux-arm-kernel
This patch cleans the initialization of dma contiguous framework. The
all-in-one dma_declare_contiguous() function is now separated into
dma_contiguous_reserve_area() which only steals the the memory from
memblock allocator and dma_contiguous_add_device() function, which
assigns given device to the specified reserved memory area. This improves
the flexibility in defining contiguous memory areas and assigning device
to them, because now it is possible to assign more than one device to
the given contiguous memory area. Such split in initialization procedure
is also required for upcoming device tree support.
Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
Acked-by: Kyungmin Park <kyungmin.park@samsung.com>
Acked-by: Michal Nazarewicz <mina86@mina86.com>
---
arch/arm/include/asm/dma-contiguous.h | 1 -
arch/x86/include/asm/dma-contiguous.h | 1 -
drivers/base/dma-contiguous.c | 119 ++++++++++++---------------------
include/asm-generic/dma-contiguous.h | 28 --------
include/linux/dma-contiguous.h | 55 ++++++++++++++-
5 files changed, 97 insertions(+), 107 deletions(-)
delete mode 100644 include/asm-generic/dma-contiguous.h
diff --git a/arch/arm/include/asm/dma-contiguous.h b/arch/arm/include/asm/dma-contiguous.h
index 3ed37b4..63277bc 100644
--- a/arch/arm/include/asm/dma-contiguous.h
+++ b/arch/arm/include/asm/dma-contiguous.h
@@ -5,7 +5,6 @@
#ifdef CONFIG_CMA
#include <linux/types.h>
-#include <asm-generic/dma-contiguous.h>
void dma_contiguous_early_fixup(phys_addr_t base, unsigned long size);
diff --git a/arch/x86/include/asm/dma-contiguous.h b/arch/x86/include/asm/dma-contiguous.h
index c092416..b4b38ba 100644
--- a/arch/x86/include/asm/dma-contiguous.h
+++ b/arch/x86/include/asm/dma-contiguous.h
@@ -4,7 +4,6 @@
#ifdef __KERNEL__
#include <linux/types.h>
-#include <asm-generic/dma-contiguous.h>
static inline void
dma_contiguous_early_fixup(phys_addr_t base, unsigned long size) { }
diff --git a/drivers/base/dma-contiguous.c b/drivers/base/dma-contiguous.c
index 0ca5442..4b94c91 100644
--- a/drivers/base/dma-contiguous.c
+++ b/drivers/base/dma-contiguous.c
@@ -96,7 +96,7 @@ static inline __maybe_unused phys_addr_t cma_early_percent_memory(void)
#endif
/**
- * dma_contiguous_reserve() - reserve area for contiguous memory handling
+ * dma_contiguous_reserve() - reserve area(s) for contiguous memory handling
* @limit: End address of the reserved memory (optional, 0 for any).
*
* This function reserves memory from early allocator. It should be
@@ -124,22 +124,29 @@ void __init dma_contiguous_reserve(phys_addr_t limit)
#endif
}
- if (selected_size) {
+ if (selected_size && !dma_contiguous_default_area) {
pr_debug("%s: reserving %ld MiB for global area\n", __func__,
(unsigned long)selected_size / SZ_1M);
- dma_declare_contiguous(NULL, selected_size, 0, limit);
+ dma_contiguous_reserve_area(selected_size, 0, limit,
+ &dma_contiguous_default_area);
}
};
static DEFINE_MUTEX(cma_mutex);
-static __init int cma_activate_area(unsigned long base_pfn, unsigned long count)
+static __init int cma_activate_area(struct cma *cma)
{
- unsigned long pfn = base_pfn;
- unsigned i = count >> pageblock_order;
+ int bitmap_size = BITS_TO_LONGS(cma->count) * sizeof(long);
+ unsigned long base_pfn = cma->base_pfn, pfn = base_pfn;
+ unsigned i = cma->count >> pageblock_order;
struct zone *zone;
+ cma->bitmap = kzalloc(bitmap_size, GFP_KERNEL);
+
+ if (!cma->bitmap)
+ return -ENOMEM;
+
WARN_ON_ONCE(!pfn_valid(pfn));
zone = page_zone(pfn_to_page(pfn));
@@ -153,92 +160,53 @@ static __init int cma_activate_area(unsigned long base_pfn, unsigned long count)
}
init_cma_reserved_pageblock(pfn_to_page(base_pfn));
} while (--i);
- return 0;
-}
-
-static __init struct cma *cma_create_area(unsigned long base_pfn,
- unsigned long count)
-{
- int bitmap_size = BITS_TO_LONGS(count) * sizeof(long);
- struct cma *cma;
- int ret = -ENOMEM;
-
- pr_debug("%s(base %08lx, count %lx)\n", __func__, base_pfn, count);
-
- cma = kmalloc(sizeof *cma, GFP_KERNEL);
- if (!cma)
- return ERR_PTR(-ENOMEM);
-
- cma->base_pfn = base_pfn;
- cma->count = count;
- cma->bitmap = kzalloc(bitmap_size, GFP_KERNEL);
- if (!cma->bitmap)
- goto no_mem;
-
- ret = cma_activate_area(base_pfn, count);
- if (ret)
- goto error;
-
- pr_debug("%s: returned %p\n", __func__, (void *)cma);
- return cma;
-
-error:
- kfree(cma->bitmap);
-no_mem:
- kfree(cma);
- return ERR_PTR(ret);
+ return 0;
}
-static struct cma_reserved {
- phys_addr_t start;
- unsigned long size;
- struct device *dev;
-} cma_reserved[MAX_CMA_AREAS] __initdata;
-static unsigned cma_reserved_count __initdata;
+static struct cma cma_areas[MAX_CMA_AREAS];
+static unsigned cma_area_count;
static int __init cma_init_reserved_areas(void)
{
- struct cma_reserved *r = cma_reserved;
- unsigned i = cma_reserved_count;
-
- pr_debug("%s()\n", __func__);
+ int i;
- for (; i; --i, ++r) {
- struct cma *cma;
- cma = cma_create_area(PFN_DOWN(r->start),
- r->size >> PAGE_SHIFT);
- if (!IS_ERR(cma))
- dev_set_cma_area(r->dev, cma);
+ for (i = 0; i < cma_area_count; i++) {
+ int ret = cma_activate_area(&cma_areas[i]);
+ if (ret)
+ return ret;
}
+
return 0;
}
core_initcall(cma_init_reserved_areas);
/**
- * dma_declare_contiguous() - reserve area for contiguous memory handling
- * for particular device
- * @dev: Pointer to device structure.
- * @size: Size of the reserved memory.
- * @base: Start address of the reserved memory (optional, 0 for any).
+ * 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
* @limit: End address of the reserved memory (optional, 0 for any).
+ * @res_cma: Pointer to store the created cma region.
*
- * This function reserves memory for specified device. It should be
- * called by board specific code when early allocator (memblock or bootmem)
- * is still activate.
+ * 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.
*/
-int __init dma_declare_contiguous(struct device *dev, phys_addr_t size,
- phys_addr_t base, phys_addr_t limit)
+int __init dma_contiguous_reserve_area(phys_addr_t size, phys_addr_t base,
+ phys_addr_t limit, struct cma **res_cma)
{
- struct cma_reserved *r = &cma_reserved[cma_reserved_count];
+ struct cma *cma = &cma_areas[cma_area_count];
phys_addr_t alignment;
+ int ret = 0;
pr_debug("%s(size %lx, base %08lx, limit %08lx)\n", __func__,
(unsigned long)size, (unsigned long)base,
(unsigned long)limit);
/* Sanity checks */
- if (cma_reserved_count == ARRAY_SIZE(cma_reserved)) {
+ if (cma_area_count == ARRAY_SIZE(cma_areas)) {
pr_err("Not enough slots for CMA reserved regions!\n");
return -ENOSPC;
}
@@ -256,7 +224,7 @@ int __init dma_declare_contiguous(struct device *dev, phys_addr_t size,
if (base) {
if (memblock_is_region_reserved(base, size) ||
memblock_reserve(base, size) < 0) {
- base = -EBUSY;
+ ret = -EBUSY;
goto err;
}
} else {
@@ -266,7 +234,7 @@ int __init dma_declare_contiguous(struct device *dev, phys_addr_t size,
*/
phys_addr_t addr = __memblock_alloc_base(size, alignment, limit);
if (!addr) {
- base = -ENOMEM;
+ ret = -ENOMEM;
goto err;
} else {
base = addr;
@@ -277,10 +245,11 @@ int __init dma_declare_contiguous(struct device *dev, phys_addr_t size,
* Each reserved area must be initialised later, when more kernel
* subsystems (like slab allocator) are available.
*/
- r->start = base;
- r->size = size;
- r->dev = dev;
- cma_reserved_count++;
+ 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);
@@ -289,7 +258,7 @@ int __init dma_declare_contiguous(struct device *dev, phys_addr_t size,
return 0;
err:
pr_err("CMA: failed to reserve %ld MiB\n", (unsigned long)size / SZ_1M);
- return base;
+ return ret;
}
/**
diff --git a/include/asm-generic/dma-contiguous.h b/include/asm-generic/dma-contiguous.h
deleted file mode 100644
index 294b1e7..0000000
--- a/include/asm-generic/dma-contiguous.h
+++ /dev/null
@@ -1,28 +0,0 @@
-#ifndef ASM_DMA_CONTIGUOUS_H
-#define ASM_DMA_CONTIGUOUS_H
-
-#ifdef __KERNEL__
-#ifdef CONFIG_CMA
-
-#include <linux/device.h>
-#include <linux/dma-contiguous.h>
-
-static inline struct cma *dev_get_cma_area(struct device *dev)
-{
- if (dev && dev->cma_area)
- return dev->cma_area;
- return dma_contiguous_default_area;
-}
-
-static inline void dev_set_cma_area(struct device *dev, struct cma *cma)
-{
- if (dev)
- dev->cma_area = cma;
- if (!dev && !dma_contiguous_default_area)
- dma_contiguous_default_area = cma;
-}
-
-#endif
-#endif
-
-#endif
diff --git a/include/linux/dma-contiguous.h b/include/linux/dma-contiguous.h
index 01b5c84..9439659 100644
--- a/include/linux/dma-contiguous.h
+++ b/include/linux/dma-contiguous.h
@@ -67,9 +67,48 @@ struct device;
extern struct cma *dma_contiguous_default_area;
+static inline struct cma *dev_get_cma_area(struct device *dev)
+{
+ if (dev && dev->cma_area)
+ return dev->cma_area;
+ return dma_contiguous_default_area;
+}
+
+static inline void dev_set_cma_area(struct device *dev, struct cma *cma)
+{
+ if (dev)
+ dev->cma_area = cma;
+}
+
void dma_contiguous_reserve(phys_addr_t addr_limit);
-int dma_declare_contiguous(struct device *dev, phys_addr_t size,
- phys_addr_t base, phys_addr_t limit);
+
+int __init dma_contiguous_reserve_area(phys_addr_t size, phys_addr_t base,
+ phys_addr_t limit, struct cma **res_cma);
+
+/**
+ * dma_declare_contiguous() - reserve area for contiguous memory handling
+ * for particular device
+ * @dev: Pointer to device structure.
+ * @size: Size of the reserved memory.
+ * @base: Start address of the reserved memory (optional, 0 for any).
+ * @limit: End address of the reserved memory (optional, 0 for any).
+ *
+ * This function reserves memory for specified device. It should be
+ * called by board specific code when early allocator (memblock or bootmem)
+ * is still activate.
+ */
+
+static inline int dma_declare_contiguous(struct device *dev, phys_addr_t size,
+ phys_addr_t base, phys_addr_t limit)
+{
+ struct cma *cma;
+ int ret;
+ ret = dma_contiguous_reserve_area(size, base, limit, &cma);
+ if (ret == 0)
+ dev_set_cma_area(dev, cma);
+
+ return ret;
+}
struct page *dma_alloc_from_contiguous(struct device *dev, int count,
unsigned int order);
@@ -80,8 +119,20 @@ bool dma_release_from_contiguous(struct device *dev, struct page *pages,
#define MAX_CMA_AREAS (0)
+static inline struct cma *dev_get_cma_area(struct device *dev)
+{
+ return NULL;
+}
+
+static inline void dev_set_cma_area(struct device *dev, struct cma *cma) { }
+
static inline void dma_contiguous_reserve(phys_addr_t limit) { }
+static inline int dma_contiguous_reserve_area(phys_addr_t size, phys_addr_t base,
+ phys_addr_t limit, struct cma **res_cma) {
+ return -ENOSYS;
+}
+
static inline
int dma_declare_contiguous(struct device *dev, phys_addr_t size,
phys_addr_t base, phys_addr_t limit)
--
1.7.9.5
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v5 2/3] drivers: of: add initialization code for dma reserved memory
2013-08-09 11:51 [PATCH v5 0/3] Device Tree support for CMA (Contiguous Memory Allocator) Marek Szyprowski
2013-08-09 11:51 ` [PATCH v5 1/3] drivers: dma-contiguous: clean source code and prepare for device tree Marek Szyprowski
@ 2013-08-09 11:51 ` Marek Szyprowski
2013-08-10 17:33 ` Rob Herring
` (2 more replies)
2013-08-09 11:51 ` [PATCH v5 3/3] ARM: init: add support for reserved memory defined by device tree Marek Szyprowski
2013-08-09 13:19 ` [PATCH v5 0/3] Device Tree support for CMA (Contiguous Memory Allocator) Tomasz Figa
3 siblings, 3 replies; 14+ messages in thread
From: Marek Szyprowski @ 2013-08-09 11:51 UTC (permalink / raw)
To: linux-arm-kernel
Add device tree support for contiguous and reserved memory regions
defined in device tree. Initialization is done in 2 steps. First, the
memory is reserved, what happens very early when only flattened device
tree is available. Then on device initialization the corresponding cma
and reserved regions are assigned to each device structure.
Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
Acked-by: Kyungmin Park <kyungmin.park@samsung.com>
Acked-by: Michal Nazarewicz <mina86@mina86.com>
---
Documentation/devicetree/bindings/memory.txt | 152 ++++++++++++++++++++
drivers/of/Kconfig | 6 +
drivers/of/Makefile | 1 +
drivers/of/of_reserved_mem.c | 197 ++++++++++++++++++++++++++
drivers/of/platform.c | 5 +
include/linux/of_reserved_mem.h | 14 ++
6 files changed, 375 insertions(+)
create mode 100644 Documentation/devicetree/bindings/memory.txt
create mode 100644 drivers/of/of_reserved_mem.c
create mode 100644 include/linux/of_reserved_mem.h
diff --git a/Documentation/devicetree/bindings/memory.txt b/Documentation/devicetree/bindings/memory.txt
new file mode 100644
index 0000000..167d013
--- /dev/null
+++ b/Documentation/devicetree/bindings/memory.txt
@@ -0,0 +1,152 @@
+*** Memory binding ***
+
+The /memory node provides basic information about the address and size
+of the physical memory. This node is usually filled or updated by the
+bootloader, depending on the actual memory configuration of the given
+hardware.
+
+The memory layout is described by the folllowing node:
+
+memory {
+ device_type = "memory";
+ reg = <(baseaddr1) (size1)
+ (baseaddr2) (size2)
+ ...
+ (baseaddrN) (sizeN)>;
+};
+
+baseaddrX: the base address of the defined memory bank
+sizeX: the size of the defined memory bank
+
+More than one memory bank can be defined.
+
+
+*** Reserved memory regions ***
+
+In /memory/reserved-memory node one can create additional nodes
+describing particular reserved (excluded from normal use) memory
+regions. Such memory regions are usually designed for the special usage
+by various device drivers. A good example are contiguous memory
+allocations or memory sharing with other operating system on the same
+hardware board. Those special memory regions might depend on the board
+configuration and devices used on the target system.
+
+Parameters for each memory region can be encoded into the device tree
+wit the following convention:
+
+[(label):] (name)@(address) {
+ compatible = "contiguous-memory-region", "reserved-memory-region";
+ reg = <(address) (size)>;
+ (linux,default-contiguous-region);
+};
+
+label: label given to the defined region (optional)
+name: an name given to the defined region
+address: the base address of the defined region
+size: the size of the memory region
+
+compatible: "contiguous-memory-region" - enables binding of this
+ region to Contiguous Memory Allocator (special region for
+ contiguous memory allocations, shared with movable system
+ memory, Linux kernel-specific), alternatively if
+ "reserved-memory-region" - compatibility is defined, given
+ region is assigned for exclusive usage for DMA transfers
+
+linux,default-contiguous-region: property indicating that the region
+ is the default region for all contiguous memory
+ allocations, Linux specific (optional)
+
+Each defined region must use unique name. It is optional to specify the
+base address, so if one wants to use autoconfiguration of the base
+address, he must specify the '0' as base address in the 'reg' property
+and assign ann unique name to such regions.
+
+
+*** Device node's properties ***
+
+Once the regions in the /memory/reserved-memory node are defined, they
+can be assigned to device nodes to enable drivers for their special use.
+The following properties are defined:
+
+memory-region = <&phandle_to_defined_region>;
+
+This property indicates that the device driver should use the
+memory region pointed by the given phandle.
+
+
+*** Example ***
+
+This example defines a memory consisting of 4 memory banks. 3 contiguous
+regions are defined for Linux kernel, one default of all device drivers
+(named contig_mem, placed at 0x72000000, 64MiB), one dedicated to the
+framebuffer device (labelled display_mem, placed at 0x78000000, 8MiB)
+and one for multimedia processing (labelled multimedia_mem, placed at
+0x77000000, 64MiB). 'display_mem' region is then assigned to fb at 12300000
+device for DMA memory allocations (Linux kernel drivers will use CMA is
+available or dma-exclusive usage otherwise). 'multimedia_mem' is
+assigned to scaler at 12500000 and codec at 12600000 devices for contiguous
+memory allocations when CMA driver is enabled.
+
+The reason for creating a separate region for framebuffer device is to
+match the framebuffer base address to the one configured by bootloader,
+so once Linux kernel drivers starts no glitches on the displayed boot
+logo appears. Scaller and codec drivers should share the memory
+allocations.
+
+/ {
+ /* ... */
+ memory {
+ reg = <0x40000000 0x10000000
+ 0x50000000 0x10000000
+ 0x60000000 0x10000000
+ 0x70000000 0x10000000>;
+
+ reserved-memory {
+ #address-cells = <1>;
+ #size-cells = <1>;
+
+ /*
+ * global autoconfigured region for contiguous allocations
+ * (used only with Contiguous Memory Allocator)
+ */
+ contig_region at 0 {
+ compatible = "contiguous-memory-region";
+ reg = <0x0 0x4000000>;
+ linux,default-contiguous-region;
+ };
+
+ /*
+ * special region for framebuffer
+ */
+ display_mem: region at 78000000 {
+ compatible = "contiguous-memory-region", "reserved-memory-region";
+ reg = <0x78000000 0x800000>;
+ };
+
+ /*
+ * special region for multimedia processing devices
+ */
+ multimedia_mem: region at 77000000 {
+ compatible = "contiguous-memory-region";
+ reg = <0x77000000 0x4000000>;
+ };
+ };
+ };
+
+ /* ... */
+
+ fb0: fb at 12300000 {
+ status = "okay";
+ memory-region = <&display_mem>;
+ };
+
+ scaler: scaler at 12500000 {
+ status = "okay";
+ memory-region = <&multimedia_mem>;
+ };
+
+ codec: codec at 12600000 {
+ status = "okay";
+ memory-region = <&multimedia_mem>;
+ };
+};
diff --git a/drivers/of/Kconfig b/drivers/of/Kconfig
index 80e5c13..a83ab43 100644
--- a/drivers/of/Kconfig
+++ b/drivers/of/Kconfig
@@ -80,4 +80,10 @@ config OF_MTD
depends on MTD
def_bool y
+config OF_RESERVED_MEM
+ depends on CMA || (HAVE_GENERIC_DMA_COHERENT && HAVE_MEMBLOCK)
+ def_bool y
+ help
+ Initialization code for DMA reserved memory
+
endmenu # OF
diff --git a/drivers/of/Makefile b/drivers/of/Makefile
index 1f9c0c4..e7e3322 100644
--- a/drivers/of/Makefile
+++ b/drivers/of/Makefile
@@ -10,3 +10,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/of_reserved_mem.c b/drivers/of/of_reserved_mem.c
new file mode 100644
index 0000000..b256b41
--- /dev/null
+++ b/drivers/of/of_reserved_mem.c
@@ -0,0 +1,197 @@
+/*
+ * Device tree based initialization code for reserved memory.
+ *
+ * Copyright (c) 2013 Samsung Electronics Co., Ltd.
+ * http://www.samsung.com
+ * Author: Marek Szyprowski <m.szyprowski@samsung.com>
+ *
+ * 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 <asm/dma-contiguous.h>
+
+#include <linux/memblock.h>
+#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/mm_types.h>
+#include <linux/dma-contiguous.h>
+#include <linux/dma-mapping.h>
+#include <linux/of_reserved_mem.h>
+
+#define MAX_RESERVED_REGIONS 16
+struct reserved_mem {
+ phys_addr_t base;
+ unsigned long size;
+ struct cma *cma;
+ char name[32];
+};
+static struct reserved_mem reserved_mem[MAX_RESERVED_REGIONS];
+static int reserved_mem_count;
+
+static int __init reserved_mem_fdt_scan(unsigned long node, const char *uname,
+ int depth, void *data)
+{
+ static int size_cells = OF_ROOT_NODE_SIZE_CELLS_DEFAULT;
+ static int addr_cells = OF_ROOT_NODE_ADDR_CELLS_DEFAULT;
+ static int my_depth;
+ phys_addr_t base, size;
+ int is_cma, is_reserved;
+ unsigned long len;
+ __be32 *prop;
+
+ if (depth == 1 && my_depth == 0 && strcmp(uname, "memory") == 0) {
+ my_depth = depth;
+ /* scan next node */
+ return 0;
+ } else if (depth == 2 && my_depth == 1 &&
+ strcmp(uname, "reserved-memory") == 0) {
+ prop = of_get_flat_dt_prop(node, "#size-cells", NULL);
+ if (prop)
+ size_cells = be32_to_cpup(prop);
+
+ prop = of_get_flat_dt_prop(node, "#address-cells", NULL);
+ if (prop)
+ addr_cells = be32_to_cpup(prop);
+
+ my_depth = depth;
+ /* scan next node */
+ return 0;
+ } else if (depth != 3 && my_depth != 2) {
+ /* scan next node */
+ return 0;
+ } else if (depth < my_depth) {
+ /* break scan now */
+ return 1;
+ }
+
+ /* now we are scanning a /memory/reserved-memory child */
+
+ is_cma = of_flat_dt_is_compatible(node, "linux,contiguous-memory-region");
+ is_reserved = of_flat_dt_is_compatible(node, "reserved-memory-region");
+
+ if (!is_reserved && !(is_cma && IS_ENABLED(CONFIG_CMA))) {
+ /* ignore node and scan next one */
+ return 0;
+ }
+
+ prop = of_get_flat_dt_prop(node, "reg", &len);
+ if (!prop || (len != (size_cells + addr_cells) * sizeof(__be32))) {
+ pr_err("Reserved mem: node %s, incorrect \"reg\" property\n",
+ uname);
+ /* ignore node and scan next one */
+ return 0;
+ }
+ base = dt_mem_next_cell(addr_cells, &prop);
+ size = dt_mem_next_cell(size_cells, &prop);
+
+ if (!size) {
+ /* ignore node and scan next one */
+ return 0;
+ }
+
+ pr_info("Reserved mem: found %s, memory base %lx, size %ld MiB\n",
+ uname, (unsigned long)base, (unsigned long)size / SZ_1M);
+
+ if (reserved_mem_count == ARRAY_SIZE(reserved_mem))
+ return -ENOSPC;
+
+ reserved_mem[reserved_mem_count].base = base;
+ reserved_mem[reserved_mem_count].size = size;
+ strlcpy(reserved_mem[reserved_mem_count].name, uname,
+ sizeof(reserved_mem[reserved_mem_count].name));
+
+ if (IS_ENABLED(CONFIG_CMA) && is_cma) {
+ struct cma *cma;
+ if (dma_contiguous_reserve_area(size, base, 0, &cma) == 0) {
+ reserved_mem[reserved_mem_count].cma = cma;
+ reserved_mem_count++;
+
+ if (of_get_flat_dt_prop(node,
+ "linux,default-contiguous-region",
+ NULL))
+ dma_contiguous_default_area = cma;
+ }
+ } else if (is_reserved) {
+ if (memblock_remove(base, size) == 0)
+ reserved_mem_count++;
+ else
+ pr_err("Failed to reserve memory for %s\n", uname);
+ }
+
+ return 0;
+}
+
+static struct reserved_mem *get_dma_memory_region(struct device *dev)
+{
+ struct device_node *node;
+ const char *name;
+ int i;
+
+ node = of_parse_phandle(dev->of_node, "memory-region", 0);
+ if (!node)
+ return NULL;
+
+ name = kbasename(node->full_name);
+ for (i = 0; i < reserved_mem_count; i++)
+ if (strcmp(name, reserved_mem[i].name) == 0)
+ 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 "dma-memory-region" device tree
+ * property to the given device.
+ */
+void of_reserved_mem_device_init(struct device *dev)
+{
+ struct reserved_mem *region = get_dma_memory_region(dev);
+ if (!region)
+ return;
+
+ if (region->cma) {
+ dev_set_cma_area(dev, region->cma);
+ pr_info("Assigned CMA %s to %s device\n", region->name,
+ dev_name(dev));
+ } else {
+ if (dma_declare_coherent_memory(dev, region->base, region->base,
+ region->size, DMA_MEMORY_MAP | DMA_MEMORY_EXCLUSIVE) != 0)
+ pr_info("Declared reserved memory %s to %s device\n",
+ region->name, dev_name(dev));
+ }
+}
+
+/**
+ * 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 reserved_mem *region = get_dma_memory_region(dev);
+ if (!region && !region->cma)
+ dma_release_declared_memory(dev);
+}
+
+/**
+ * dma_reserved_mem_reserve() - grab memory reserved for device exclusive use
+ *
+ * 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 (memblock) has been activated and all other
+ * subsystems have already allocated/reserved memory.
+ */
+void __init dma_reserved_mem_of_reserve(void)
+{
+ if (initial_boot_params)
+ of_scan_flat_dt(reserved_mem_fdt_scan, NULL);
+}
diff --git a/drivers/of/platform.c b/drivers/of/platform.c
index e0a6514..1e4e91d 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[] = {
@@ -196,6 +197,7 @@ EXPORT_SYMBOL(of_device_alloc);
* Returns pointer to created platform device, or NULL if a device was not
* registered. Unavailable devices will not get registered.
*/
+
struct platform_device *of_platform_device_create_pdata(
struct device_node *np,
const char *bus_id,
@@ -218,6 +220,8 @@ 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
@@ -225,6 +229,7 @@ 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;
}
diff --git a/include/linux/of_reserved_mem.h b/include/linux/of_reserved_mem.h
new file mode 100644
index 0000000..1274946
--- /dev/null
+++ b/include/linux/of_reserved_mem.h
@@ -0,0 +1,14 @@
+#ifndef __OF_RESERVED_MEM_H
+#define __OF_RESERVED_MEM_H
+
+#ifdef CONFIG_OF_RESERVED_MEM
+void of_reserved_mem_device_init(struct device *dev);
+void of_reserved_mem_device_release(struct device *dev);
+void __init dma_reserved_mem_of_reserve(void);
+#else
+#define of_reserved_mem_device_init(dev) (void)0
+#define of_reserved_mem_device_release(dev) (void)0
+#define dma_reserved_mem_of_reserve() (void)0
+#endif
+
+#endif /* __OF_RESERVED_MEM_H */
--
1.7.9.5
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v5 3/3] ARM: init: add support for reserved memory defined by device tree
2013-08-09 11:51 [PATCH v5 0/3] Device Tree support for CMA (Contiguous Memory Allocator) Marek Szyprowski
2013-08-09 11:51 ` [PATCH v5 1/3] drivers: dma-contiguous: clean source code and prepare for device tree Marek Szyprowski
2013-08-09 11:51 ` [PATCH v5 2/3] drivers: of: add initialization code for dma reserved memory Marek Szyprowski
@ 2013-08-09 11:51 ` Marek Szyprowski
2013-08-09 13:19 ` [PATCH v5 0/3] Device Tree support for CMA (Contiguous Memory Allocator) Tomasz Figa
3 siblings, 0 replies; 14+ messages in thread
From: Marek Szyprowski @ 2013-08-09 11:51 UTC (permalink / raw)
To: linux-arm-kernel
Enable reserved memory initialization from device tree.
Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
Acked-by: Kyungmin Park <kyungmin.park@samsung.com>
Acked-by: Michal Nazarewicz <mina86@mina86.com>
---
arch/arm/mm/init.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/arch/arm/mm/init.c b/arch/arm/mm/init.c
index 15225d8..7c1c4b2 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>
@@ -377,6 +378,8 @@ void __init arm_memblock_init(struct meminfo *mi, struct machine_desc *mdesc)
if (mdesc->reserve)
mdesc->reserve();
+ dma_reserved_mem_of_reserve();
+
/*
* reserve memory for DMA contigouos allocations,
* must come from DMA area inside low memory
--
1.7.9.5
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v5 0/3] Device Tree support for CMA (Contiguous Memory Allocator)
2013-08-09 11:51 [PATCH v5 0/3] Device Tree support for CMA (Contiguous Memory Allocator) Marek Szyprowski
` (2 preceding siblings ...)
2013-08-09 11:51 ` [PATCH v5 3/3] ARM: init: add support for reserved memory defined by device tree Marek Szyprowski
@ 2013-08-09 13:19 ` Tomasz Figa
3 siblings, 0 replies; 14+ messages in thread
From: Tomasz Figa @ 2013-08-09 13:19 UTC (permalink / raw)
To: linux-arm-kernel
Hi Marek,
On Friday 09 of August 2013 13:51:56 Marek Szyprowski wrote:
> Hello,
>
> This is a fifth version of my proposal for device tree integration for
> reserved memory and Contiguous Memory Allocator. I've fixed issues
> pointed by Michal Nazarewicz, Rob Herring and Kumar Gala. For more
> information, please check the change log at the end of the mail.
>
> In fourth version of this patch set, after the comments from Grant
> Likely I moved back memory region definitions back to /memory node (as
> it was in the first version of this proposal). I've also extended the
> code and made it more generic, added support for so called reserved dma
> memory (special dma memory regions created by dma_alloc_coherent()
> function, for exclusive usage for dma allocation for the given device).
>
> Just a few words for those who see this code for the first time:
>
> The proposed bindings allows to define contiguous memory regions of
> specified base address and size. Then, the defined regions can be
> assigned to the given device(s) by adding a property with a phanle to
> the defined contiguous memory region. From the device tree perspective
> that's all. Once the bindings are added, all the memory allocations from
> dma-mapping subsystem will be served from the defined contiguous memory
> regions.
>
> Contiguous Memory Allocator is a framework, which lets to provide a
> large contiguous memory buffers for (usually a multimedia) devices. The
> contiguous memory is reserved during early boot and then shared with
> kernel, which is allowed to allocate it for movable pages. Then, when
> device driver requests a contigouous buffer, the framework migrates
> movable pages out of contiguous region and gives it to the driver. When
> device driver frees the buffer, it is added to kernel memory pool again.
> For more information, please refer to commit c64be2bb1c6eb43c838b2c6d57
> ("drivers: add Contiguous Memory Allocator") and d484864dd96e1830e76895
> (CMA merge commit).
>
> Why we need device tree bindings for CMA at all?
>
> Older ARM kernels used so called board-based initialization. Those board
> files contained a definition of all hardware blocks available on the
> target system and particular kernel and driver software configuration
> selected by the board maintainer.
>
> In the new approach the board files will be removed completely and
> Device Tree approach is used to describe all hardware blocks available
> on the target system. By definition, the bindings should be software
> independent, so at least in theory it should be possible to use those
> bindings with other operating systems than Linux kernel.
>
> Reserved memory configuration belongs to the grey area. It might depend
> on hardware restriction of the board or modules and low-level
> configuration done by bootloader. Putting reserved and contiguous memory
> regions to /memory node and having phandles to those regions in the
> device nodes however matches well with the device-tree typical style of
> linking devices with other resources like clocks, interrupts,
> regulators, power domains, etc. This is the main reason to use such
> approach instead of putting everything to /chosen node as it has been
> proposed in v2 and v3.
>
> Best regards
> Marek Szyprowski
> Samsung R&D Institute Poland
>
> Changelog:
>
> v5:
> - renamed "contiguous-memory-region" compatibility string to
> "linux,contiguous-memory-region" (this one is really specific to Linux
> kernel implementation)
> - renamed "dma-memory-region" property to "memory-region" (suggested by
> Kumar Gala)
> - added support for #address-cells, #size-cells for memory regions
> (thanks to Rob Herring for suggestion)
> - removed generic code to scan specific path in flat device tree (cannot
> be used to fdt one-pass scan based initialization of memory regions
> with #address-cells and #size-cells parsing)
> - replaced dma_contiguous_set_default_area() and
> dma_contiguous_add_device() functions with dev_set_cma_area() call
>
> v4: http://thread.gmane.org/gmane.linux.ports.arm.kernel/256491
> - corrected Devcie Tree mailing list address (resend)
> - moved back contiguous-memory bindings from /chosen/contiguous-memory
> to /memory nodes as suggested by Grant (see
> http://article.gmane.org/gmane.linux.drivers.devicetree/41030
> for more details)
> - added support for DMA reserved memory with dma_declare_coherent()
> - moved code to drivers/of/of_reserved_mem.c
> - added generic code to scan specific path in flat device tree
>
> v3: http://thread.gmane.org/gmane.linux.drivers.devicetree/40013/
> - fixed issues pointed by Laura and updated documentation
>
> v2: http://thread.gmane.org/gmane.linux.drivers.devicetree/34075
> - moved contiguous-memory bindings from /memory to
> /chosen/contiguous-memory/ node to avoid spreading Linux specific
> parameters over the whole device tree definitions
> - added support for autoconfigured regions (use zero base)
> - fixes minor bugs
>
> v1: http://thread.gmane.org/gmane.linux.drivers.devicetree/30111/
> - initial proposal
>
> Patch summary:
>
> Marek Szyprowski (3):
> drivers: dma-contiguous: clean source code and prepare for device
> tree
> drivers: of: add initialization code for dma reserved memory
> ARM: init: add support for reserved memory defined by device tree
>
> Documentation/devicetree/bindings/memory.txt | 152 ++++++++++++++++++++
> arch/arm/include/asm/dma-contiguous.h | 1 -
> arch/arm/mm/init.c | 3 +
> arch/x86/include/asm/dma-contiguous.h | 1 -
> drivers/base/dma-contiguous.c | 119 ++++++----------
> drivers/of/Kconfig | 6 +
> drivers/of/Makefile | 1 +
> drivers/of/of_reserved_mem.c | 197
> ++++++++++++++++++++++++++ drivers/of/platform.c
> | 5 +
> include/asm-generic/dma-contiguous.h | 28 ----
> include/linux/dma-contiguous.h | 55 ++++++-
> include/linux/of_reserved_mem.h | 14 ++
> 12 files changed, 475 insertions(+), 107 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/memory.txt
> create mode 100644 drivers/of/of_reserved_mem.c
> delete mode 100644 include/asm-generic/dma-contiguous.h
> create mode 100644 include/linux/of_reserved_mem.h
We definitely need this and current version looks pretty good to me.
Acked-by: Tomasz Figa <t.figa@samsung.com>
Best regards,
Tomasz
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v5 2/3] drivers: of: add initialization code for dma reserved memory
2013-08-09 11:51 ` [PATCH v5 2/3] drivers: of: add initialization code for dma reserved memory Marek Szyprowski
@ 2013-08-10 17:33 ` Rob Herring
2013-08-12 8:34 ` Marek Szyprowski
2013-08-13 20:08 ` Stephen Warren
2013-08-16 5:32 ` Olof Johansson
2 siblings, 1 reply; 14+ messages in thread
From: Rob Herring @ 2013-08-10 17:33 UTC (permalink / raw)
To: linux-arm-kernel
On Fri, Aug 9, 2013 at 6:51 AM, Marek Szyprowski
<m.szyprowski@samsung.com> wrote:
> Add device tree support for contiguous and reserved memory regions
> defined in device tree. Initialization is done in 2 steps. First, the
> memory is reserved, what happens very early when only flattened device
s/what/which/
> tree is available. Then on device initialization the corresponding cma
> and reserved regions are assigned to each device structure.
What this commit message does not tell me is why does the reservation
have to happen before the fdt is unflattened? It would greatly
simplify the code if it didn't.
> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
> Acked-by: Kyungmin Park <kyungmin.park@samsung.com>
> Acked-by: Michal Nazarewicz <mina86@mina86.com>
> ---
> Documentation/devicetree/bindings/memory.txt | 152 ++++++++++++++++++++
> drivers/of/Kconfig | 6 +
> drivers/of/Makefile | 1 +
> drivers/of/of_reserved_mem.c | 197 ++++++++++++++++++++++++++
> drivers/of/platform.c | 5 +
> include/linux/of_reserved_mem.h | 14 ++
> 6 files changed, 375 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/memory.txt
> create mode 100644 drivers/of/of_reserved_mem.c
> create mode 100644 include/linux/of_reserved_mem.h
>
> diff --git a/Documentation/devicetree/bindings/memory.txt b/Documentation/devicetree/bindings/memory.txt
> new file mode 100644
> index 0000000..167d013
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/memory.txt
> @@ -0,0 +1,152 @@
> +*** Memory binding ***
> +
> +The /memory node provides basic information about the address and size
> +of the physical memory. This node is usually filled or updated by the
> +bootloader, depending on the actual memory configuration of the given
> +hardware.
> +
> +The memory layout is described by the folllowing node:
typo.
> +
> +memory {
> + device_type = "memory";
> + reg = <(baseaddr1) (size1)
> + (baseaddr2) (size2)
> + ...
> + (baseaddrN) (sizeN)>;
> +};
> +
> +baseaddrX: the base address of the defined memory bank
> +sizeX: the size of the defined memory bank
> +
> +More than one memory bank can be defined.
> +
> +
> +*** Reserved memory regions ***
> +
> +In /memory/reserved-memory node one can create additional nodes
> +describing particular reserved (excluded from normal use) memory
> +regions. Such memory regions are usually designed for the special usage
> +by various device drivers. A good example are contiguous memory
> +allocations or memory sharing with other operating system on the same
> +hardware board. Those special memory regions might depend on the board
> +configuration and devices used on the target system.
> +
> +Parameters for each memory region can be encoded into the device tree
> +wit the following convention:
> +
> +[(label):] (name)@(address) {
> + compatible = "contiguous-memory-region", "reserved-memory-region";
> + reg = <(address) (size)>;
> + (linux,default-contiguous-region);
> +};
> +
> +label: label given to the defined region (optional)
> +name: an name given to the defined region
> +address: the base address of the defined region
> +size: the size of the memory region
> +
> +compatible: "contiguous-memory-region" - enables binding of this
> + region to Contiguous Memory Allocator (special region for
> + contiguous memory allocations, shared with movable system
> + memory, Linux kernel-specific), alternatively if
> + "reserved-memory-region" - compatibility is defined, given
> + region is assigned for exclusive usage for DMA transfers
> +
> +linux,default-contiguous-region: property indicating that the region
> + is the default region for all contiguous memory
> + allocations, Linux specific (optional)
> +
> +Each defined region must use unique name. It is optional to specify the
> +base address, so if one wants to use autoconfiguration of the base
> +address, he must specify the '0' as base address in the 'reg' property
> +and assign ann unique name to such regions.
> +
> +
> +*** Device node's properties ***
> +
> +Once the regions in the /memory/reserved-memory node are defined, they
> +can be assigned to device nodes to enable drivers for their special use.
> +The following properties are defined:
> +
> +memory-region = <&phandle_to_defined_region>;
> +
> +This property indicates that the device driver should use the
> +memory region pointed by the given phandle.
> +
> +
> +*** Example ***
> +
> +This example defines a memory consisting of 4 memory banks. 3 contiguous
> +regions are defined for Linux kernel, one default of all device drivers
> +(named contig_mem, placed at 0x72000000, 64MiB), one dedicated to the
> +framebuffer device (labelled display_mem, placed at 0x78000000, 8MiB)
> +and one for multimedia processing (labelled multimedia_mem, placed at
> +0x77000000, 64MiB). 'display_mem' region is then assigned to fb at 12300000
> +device for DMA memory allocations (Linux kernel drivers will use CMA is
> +available or dma-exclusive usage otherwise). 'multimedia_mem' is
> +assigned to scaler at 12500000 and codec at 12600000 devices for contiguous
> +memory allocations when CMA driver is enabled.
> +
> +The reason for creating a separate region for framebuffer device is to
> +match the framebuffer base address to the one configured by bootloader,
> +so once Linux kernel drivers starts no glitches on the displayed boot
> +logo appears. Scaller and codec drivers should share the memory
> +allocations.
> +
> +/ {
> + /* ... */
> + memory {
> + reg = <0x40000000 0x10000000
> + 0x50000000 0x10000000
> + 0x60000000 0x10000000
> + 0x70000000 0x10000000>;
> +
> + reserved-memory {
> + #address-cells = <1>;
> + #size-cells = <1>;
> +
> + /*
> + * global autoconfigured region for contiguous allocations
> + * (used only with Contiguous Memory Allocator)
> + */
> + contig_region at 0 {
> + compatible = "contiguous-memory-region";
> + reg = <0x0 0x4000000>;
> + linux,default-contiguous-region;
> + };
> +
> + /*
> + * special region for framebuffer
> + */
> + display_mem: region at 78000000 {
> + compatible = "contiguous-memory-region", "reserved-memory-region";
> + reg = <0x78000000 0x800000>;
> + };
> +
> + /*
> + * special region for multimedia processing devices
> + */
> + multimedia_mem: region at 77000000 {
> + compatible = "contiguous-memory-region";
> + reg = <0x77000000 0x4000000>;
> + };
> + };
> + };
> +
> + /* ... */
> +
> + fb0: fb at 12300000 {
> + status = "okay";
> + memory-region = <&display_mem>;
> + };
> +
> + scaler: scaler at 12500000 {
> + status = "okay";
> + memory-region = <&multimedia_mem>;
> + };
> +
> + codec: codec at 12600000 {
> + status = "okay";
> + memory-region = <&multimedia_mem>;
> + };
> +};
> diff --git a/drivers/of/Kconfig b/drivers/of/Kconfig
> index 80e5c13..a83ab43 100644
> --- a/drivers/of/Kconfig
> +++ b/drivers/of/Kconfig
> @@ -80,4 +80,10 @@ config OF_MTD
> depends on MTD
> def_bool y
>
> +config OF_RESERVED_MEM
> + depends on CMA || (HAVE_GENERIC_DMA_COHERENT && HAVE_MEMBLOCK)
> + def_bool y
> + help
> + Initialization code for DMA reserved memory
> +
> endmenu # OF
> diff --git a/drivers/of/Makefile b/drivers/of/Makefile
> index 1f9c0c4..e7e3322 100644
> --- a/drivers/of/Makefile
> +++ b/drivers/of/Makefile
> @@ -10,3 +10,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/of_reserved_mem.c b/drivers/of/of_reserved_mem.c
> new file mode 100644
> index 0000000..b256b41
> --- /dev/null
> +++ b/drivers/of/of_reserved_mem.c
> @@ -0,0 +1,197 @@
> +/*
> + * Device tree based initialization code for reserved memory.
> + *
> + * Copyright (c) 2013 Samsung Electronics Co., Ltd.
> + * http://www.samsung.com
> + * Author: Marek Szyprowski <m.szyprowski@samsung.com>
> + *
> + * 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 <asm/dma-contiguous.h>
> +
> +#include <linux/memblock.h>
> +#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/mm_types.h>
> +#include <linux/dma-contiguous.h>
> +#include <linux/dma-mapping.h>
> +#include <linux/of_reserved_mem.h>
> +
> +#define MAX_RESERVED_REGIONS 16
> +struct reserved_mem {
> + phys_addr_t base;
> + unsigned long size;
> + struct cma *cma;
> + char name[32];
> +};
> +static struct reserved_mem reserved_mem[MAX_RESERVED_REGIONS];
> +static int reserved_mem_count;
> +
> +static int __init reserved_mem_fdt_scan(unsigned long node, const char *uname,
> + int depth, void *data)
early_init_dt_scan_reserved_mem would be a more consistent name.
> +{
> + static int size_cells = OF_ROOT_NODE_SIZE_CELLS_DEFAULT;
> + static int addr_cells = OF_ROOT_NODE_ADDR_CELLS_DEFAULT;
> + static int my_depth;
> + phys_addr_t base, size;
> + int is_cma, is_reserved;
> + unsigned long len;
> + __be32 *prop;
> +
> + if (depth == 1 && my_depth == 0 && strcmp(uname, "memory") == 0) {
> + my_depth = depth;
> + /* scan next node */
> + return 0;
> + } else if (depth == 2 && my_depth == 1 &&
> + strcmp(uname, "reserved-memory") == 0) {
> + prop = of_get_flat_dt_prop(node, "#size-cells", NULL);
> + if (prop)
> + size_cells = be32_to_cpup(prop);
> +
> + prop = of_get_flat_dt_prop(node, "#address-cells", NULL);
> + if (prop)
> + addr_cells = be32_to_cpup(prop);
I think we should just require these be the same size as the memory
node which would be dt_root_*_cells.
I'm fine with moving this into drivers/of/fdt.c if that simplifies things.
> +
> + my_depth = depth;
> + /* scan next node */
> + return 0;
> + } else if (depth != 3 && my_depth != 2) {
> + /* scan next node */
> + return 0;
> + } else if (depth < my_depth) {
> + /* break scan now */
> + return 1;
> + }
This code bothers me and is hard to follow. I don't think trying to
use of_scan_flat_dt is the right approach here. What you really want
here is check for reserved-memory node under the memory node and then
scan each child node. This could all be done from
early_init_dt_scan_memory. u-boot has more rich flat DT functions than
the kernel does since the kernel does very little with the flat DT.
Look at fdt_next_node and it's callers in u-boot for some examples.
But still my first question remains. Can we avoid the flat DT
altogether?
> + /* now we are scanning a /memory/reserved-memory child */
> +
> + is_cma = of_flat_dt_is_compatible(node, "linux,contiguous-memory-region");
> + is_reserved = of_flat_dt_is_compatible(node, "reserved-memory-region");
> +
> + if (!is_reserved && !(is_cma && IS_ENABLED(CONFIG_CMA))) {
> + /* ignore node and scan next one */
> + return 0;
> + }
> +
> + prop = of_get_flat_dt_prop(node, "reg", &len);
> + if (!prop || (len != (size_cells + addr_cells) * sizeof(__be32))) {
> + pr_err("Reserved mem: node %s, incorrect \"reg\" property\n",
> + uname);
> + /* ignore node and scan next one */
> + return 0;
> + }
> + base = dt_mem_next_cell(addr_cells, &prop);
> + size = dt_mem_next_cell(size_cells, &prop);
> +
> + if (!size) {
> + /* ignore node and scan next one */
> + return 0;
> + }
> +
> + pr_info("Reserved mem: found %s, memory base %lx, size %ld MiB\n",
> + uname, (unsigned long)base, (unsigned long)size / SZ_1M);
> +
> + if (reserved_mem_count == ARRAY_SIZE(reserved_mem))
> + return -ENOSPC;
> +
> + reserved_mem[reserved_mem_count].base = base;
> + reserved_mem[reserved_mem_count].size = size;
> + strlcpy(reserved_mem[reserved_mem_count].name, uname,
> + sizeof(reserved_mem[reserved_mem_count].name));
> +
> + if (IS_ENABLED(CONFIG_CMA) && is_cma) {
> + struct cma *cma;
> + if (dma_contiguous_reserve_area(size, base, 0, &cma) == 0) {
> + reserved_mem[reserved_mem_count].cma = cma;
> + reserved_mem_count++;
> +
> + if (of_get_flat_dt_prop(node,
> + "linux,default-contiguous-region",
> + NULL))
> + dma_contiguous_default_area = cma;
> + }
> + } else if (is_reserved) {
> + if (memblock_remove(base, size) == 0)
> + reserved_mem_count++;
> + else
> + pr_err("Failed to reserve memory for %s\n", uname);
> + }
> +
> + return 0;
> +}
> +
> +static struct reserved_mem *get_dma_memory_region(struct device *dev)
> +{
> + struct device_node *node;
> + const char *name;
> + int i;
> +
> + node = of_parse_phandle(dev->of_node, "memory-region", 0);
> + if (!node)
> + return NULL;
> +
> + name = kbasename(node->full_name);
> + for (i = 0; i < reserved_mem_count; i++)
> + if (strcmp(name, reserved_mem[i].name) == 0)
> + return &reserved_mem[i];
> + return NULL;
Matching against a struct device_node pointer would be more common way
to match. So it would be good to update reserved_mem with a
device_node ptr when we unflatten the DT.
> +}
> +
> +/**
> + * of_reserved_mem_device_init() - assign reserved memory region to given device
> + *
> + * This function assign memory region pointed by "dma-memory-region" device tree
> + * property to the given device.
> + */
> +void of_reserved_mem_device_init(struct device *dev)
> +{
> + struct reserved_mem *region = get_dma_memory_region(dev);
> + if (!region)
> + return;
> +
> + if (region->cma) {
> + dev_set_cma_area(dev, region->cma);
> + pr_info("Assigned CMA %s to %s device\n", region->name,
> + dev_name(dev));
> + } else {
> + if (dma_declare_coherent_memory(dev, region->base, region->base,
> + region->size, DMA_MEMORY_MAP | DMA_MEMORY_EXCLUSIVE) != 0)
> + pr_info("Declared reserved memory %s to %s device\n",
> + region->name, dev_name(dev));
> + }
> +}
> +
> +/**
> + * 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 reserved_mem *region = get_dma_memory_region(dev);
> + if (!region && !region->cma)
> + dma_release_declared_memory(dev);
> +}
> +
> +/**
> + * dma_reserved_mem_reserve() - grab memory reserved for device exclusive use
> + *
> + * 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 (memblock) has been activated and all other
> + * subsystems have already allocated/reserved memory.
> + */
> +void __init dma_reserved_mem_of_reserve(void)
> +{
> + if (initial_boot_params)
> + of_scan_flat_dt(reserved_mem_fdt_scan, NULL);
> +}
> diff --git a/drivers/of/platform.c b/drivers/of/platform.c
> index e0a6514..1e4e91d 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[] = {
> @@ -196,6 +197,7 @@ EXPORT_SYMBOL(of_device_alloc);
> * Returns pointer to created platform device, or NULL if a device was not
> * registered. Unavailable devices will not get registered.
> */
> +
stray ws change. Please remove.
> struct platform_device *of_platform_device_create_pdata(
> struct device_node *np,
> const char *bus_id,
> @@ -218,6 +220,8 @@ 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
> @@ -225,6 +229,7 @@ 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;
> }
>
> diff --git a/include/linux/of_reserved_mem.h b/include/linux/of_reserved_mem.h
> new file mode 100644
> index 0000000..1274946
> --- /dev/null
> +++ b/include/linux/of_reserved_mem.h
> @@ -0,0 +1,14 @@
> +#ifndef __OF_RESERVED_MEM_H
> +#define __OF_RESERVED_MEM_H
> +
> +#ifdef CONFIG_OF_RESERVED_MEM
> +void of_reserved_mem_device_init(struct device *dev);
> +void of_reserved_mem_device_release(struct device *dev);
> +void __init dma_reserved_mem_of_reserve(void);
Declarations don't need __init annotation.
> +#else
> +#define of_reserved_mem_device_init(dev) (void)0
> +#define of_reserved_mem_device_release(dev) (void)0
> +#define dma_reserved_mem_of_reserve() (void)0
static inline is preferred over defines.
Rob
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v5 2/3] drivers: of: add initialization code for dma reserved memory
2013-08-10 17:33 ` Rob Herring
@ 2013-08-12 8:34 ` Marek Szyprowski
2013-08-13 13:00 ` Rob Herring
0 siblings, 1 reply; 14+ messages in thread
From: Marek Szyprowski @ 2013-08-12 8:34 UTC (permalink / raw)
To: linux-arm-kernel
Hello,
On 8/10/2013 7:33 PM, Rob Herring wrote:
> On Fri, Aug 9, 2013 at 6:51 AM, Marek Szyprowski
> <m.szyprowski@samsung.com> wrote:
> > Add device tree support for contiguous and reserved memory regions
> > defined in device tree. Initialization is done in 2 steps. First, the
> > memory is reserved, what happens very early when only flattened device
>
> s/what/which/
>
> > tree is available. Then on device initialization the corresponding cma
> > and reserved regions are assigned to each device structure.
>
> What this commit message does not tell me is why does the reservation
> have to happen before the fdt is unflattened? It would greatly
> simplify the code if it didn't.
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.
> > Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
> > Acked-by: Kyungmin Park <kyungmin.park@samsung.com>
> > Acked-by: Michal Nazarewicz <mina86@mina86.com>
> > ---
> > Documentation/devicetree/bindings/memory.txt | 152 ++++++++++++++++++++
> > drivers/of/Kconfig | 6 +
> > drivers/of/Makefile | 1 +
> > drivers/of/of_reserved_mem.c | 197 ++++++++++++++++++++++++++
> > drivers/of/platform.c | 5 +
> > include/linux/of_reserved_mem.h | 14 ++
> > 6 files changed, 375 insertions(+)
> > create mode 100644 Documentation/devicetree/bindings/memory.txt
> > create mode 100644 drivers/of/of_reserved_mem.c
> > create mode 100644 include/linux/of_reserved_mem.h
> >
> > diff --git a/Documentation/devicetree/bindings/memory.txt b/Documentation/devicetree/bindings/memory.txt
> > new file mode 100644
> > index 0000000..167d013
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/memory.txt
> > @@ -0,0 +1,152 @@
> > +*** Memory binding ***
> > +
> > +The /memory node provides basic information about the address and size
> > +of the physical memory. This node is usually filled or updated by the
> > +bootloader, depending on the actual memory configuration of the given
> > +hardware.
> > +
> > +The memory layout is described by the folllowing node:
>
> typo.
>
> > +
> > +memory {
> > + device_type = "memory";
> > + reg = <(baseaddr1) (size1)
> > + (baseaddr2) (size2)
> > + ...
> > + (baseaddrN) (sizeN)>;
> > +};
> > +
> > +baseaddrX: the base address of the defined memory bank
> > +sizeX: the size of the defined memory bank
> > +
> > +More than one memory bank can be defined.
> > +
> > +
> > +*** Reserved memory regions ***
> > +
> > +In /memory/reserved-memory node one can create additional nodes
> > +describing particular reserved (excluded from normal use) memory
> > +regions. Such memory regions are usually designed for the special usage
> > +by various device drivers. A good example are contiguous memory
> > +allocations or memory sharing with other operating system on the same
> > +hardware board. Those special memory regions might depend on the board
> > +configuration and devices used on the target system.
> > +
> > +Parameters for each memory region can be encoded into the device tree
> > +wit the following convention:
> > +
> > +[(label):] (name)@(address) {
> > + compatible = "contiguous-memory-region", "reserved-memory-region";
> > + reg = <(address) (size)>;
> > + (linux,default-contiguous-region);
> > +};
> > +
> > +label: label given to the defined region (optional)
> > +name: an name given to the defined region
> > +address: the base address of the defined region
> > +size: the size of the memory region
> > +
> > +compatible: "contiguous-memory-region" - enables binding of this
> > + region to Contiguous Memory Allocator (special region for
> > + contiguous memory allocations, shared with movable system
> > + memory, Linux kernel-specific), alternatively if
> > + "reserved-memory-region" - compatibility is defined, given
> > + region is assigned for exclusive usage for DMA transfers
> > +
> > +linux,default-contiguous-region: property indicating that the region
> > + is the default region for all contiguous memory
> > + allocations, Linux specific (optional)
> > +
> > +Each defined region must use unique name. It is optional to specify the
> > +base address, so if one wants to use autoconfiguration of the base
> > +address, he must specify the '0' as base address in the 'reg' property
> > +and assign ann unique name to such regions.
> > +
> > +
> > +*** Device node's properties ***
> > +
> > +Once the regions in the /memory/reserved-memory node are defined, they
> > +can be assigned to device nodes to enable drivers for their special use.
> > +The following properties are defined:
> > +
> > +memory-region = <&phandle_to_defined_region>;
> > +
> > +This property indicates that the device driver should use the
> > +memory region pointed by the given phandle.
> > +
> > +
> > +*** Example ***
> > +
> > +This example defines a memory consisting of 4 memory banks. 3 contiguous
> > +regions are defined for Linux kernel, one default of all device drivers
> > +(named contig_mem, placed at 0x72000000, 64MiB), one dedicated to the
> > +framebuffer device (labelled display_mem, placed at 0x78000000, 8MiB)
> > +and one for multimedia processing (labelled multimedia_mem, placed at
> > +0x77000000, 64MiB). 'display_mem' region is then assigned to fb at 12300000
> > +device for DMA memory allocations (Linux kernel drivers will use CMA is
> > +available or dma-exclusive usage otherwise). 'multimedia_mem' is
> > +assigned to scaler at 12500000 and codec at 12600000 devices for contiguous
> > +memory allocations when CMA driver is enabled.
> > +
> > +The reason for creating a separate region for framebuffer device is to
> > +match the framebuffer base address to the one configured by bootloader,
> > +so once Linux kernel drivers starts no glitches on the displayed boot
> > +logo appears. Scaller and codec drivers should share the memory
> > +allocations.
> > +
> > +/ {
> > + /* ... */
> > + memory {
> > + reg = <0x40000000 0x10000000
> > + 0x50000000 0x10000000
> > + 0x60000000 0x10000000
> > + 0x70000000 0x10000000>;
> > +
> > + reserved-memory {
> > + #address-cells = <1>;
> > + #size-cells = <1>;
> > +
> > + /*
> > + * global autoconfigured region for contiguous allocations
> > + * (used only with Contiguous Memory Allocator)
> > + */
> > + contig_region at 0 {
> > + compatible = "contiguous-memory-region";
> > + reg = <0x0 0x4000000>;
> > + linux,default-contiguous-region;
> > + };
> > +
> > + /*
> > + * special region for framebuffer
> > + */
> > + display_mem: region at 78000000 {
> > + compatible = "contiguous-memory-region", "reserved-memory-region";
> > + reg = <0x78000000 0x800000>;
> > + };
> > +
> > + /*
> > + * special region for multimedia processing devices
> > + */
> > + multimedia_mem: region at 77000000 {
> > + compatible = "contiguous-memory-region";
> > + reg = <0x77000000 0x4000000>;
> > + };
> > + };
> > + };
> > +
> > + /* ... */
> > +
> > + fb0: fb at 12300000 {
> > + status = "okay";
> > + memory-region = <&display_mem>;
> > + };
> > +
> > + scaler: scaler at 12500000 {
> > + status = "okay";
> > + memory-region = <&multimedia_mem>;
> > + };
> > +
> > + codec: codec at 12600000 {
> > + status = "okay";
> > + memory-region = <&multimedia_mem>;
> > + };
> > +};
> > diff --git a/drivers/of/Kconfig b/drivers/of/Kconfig
> > index 80e5c13..a83ab43 100644
> > --- a/drivers/of/Kconfig
> > +++ b/drivers/of/Kconfig
> > @@ -80,4 +80,10 @@ config OF_MTD
> > depends on MTD
> > def_bool y
> >
> > +config OF_RESERVED_MEM
> > + depends on CMA || (HAVE_GENERIC_DMA_COHERENT && HAVE_MEMBLOCK)
> > + def_bool y
> > + help
> > + Initialization code for DMA reserved memory
> > +
> > endmenu # OF
> > diff --git a/drivers/of/Makefile b/drivers/of/Makefile
> > index 1f9c0c4..e7e3322 100644
> > --- a/drivers/of/Makefile
> > +++ b/drivers/of/Makefile
> > @@ -10,3 +10,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/of_reserved_mem.c b/drivers/of/of_reserved_mem.c
> > new file mode 100644
> > index 0000000..b256b41
> > --- /dev/null
> > +++ b/drivers/of/of_reserved_mem.c
> > @@ -0,0 +1,197 @@
> > +/*
> > + * Device tree based initialization code for reserved memory.
> > + *
> > + * Copyright (c) 2013 Samsung Electronics Co., Ltd.
> > + * http://www.samsung.com
> > + * Author: Marek Szyprowski <m.szyprowski@samsung.com>
> > + *
> > + * 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 <asm/dma-contiguous.h>
> > +
> > +#include <linux/memblock.h>
> > +#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/mm_types.h>
> > +#include <linux/dma-contiguous.h>
> > +#include <linux/dma-mapping.h>
> > +#include <linux/of_reserved_mem.h>
> > +
> > +#define MAX_RESERVED_REGIONS 16
> > +struct reserved_mem {
> > + phys_addr_t base;
> > + unsigned long size;
> > + struct cma *cma;
> > + char name[32];
> > +};
> > +static struct reserved_mem reserved_mem[MAX_RESERVED_REGIONS];
> > +static int reserved_mem_count;
> > +
> > +static int __init reserved_mem_fdt_scan(unsigned long node, const char *uname,
> > + int depth, void *data)
>
> early_init_dt_scan_reserved_mem would be a more consistent name.
>
> > +{
> > + static int size_cells = OF_ROOT_NODE_SIZE_CELLS_DEFAULT;
> > + static int addr_cells = OF_ROOT_NODE_ADDR_CELLS_DEFAULT;
> > + static int my_depth;
> > + phys_addr_t base, size;
> > + int is_cma, is_reserved;
> > + unsigned long len;
> > + __be32 *prop;
> > +
> > + if (depth == 1 && my_depth == 0 && strcmp(uname, "memory") == 0) {
> > + my_depth = depth;
> > + /* scan next node */
> > + return 0;
> > + } else if (depth == 2 && my_depth == 1 &&
> > + strcmp(uname, "reserved-memory") == 0) {
> > + prop = of_get_flat_dt_prop(node, "#size-cells", NULL);
> > + if (prop)
> > + size_cells = be32_to_cpup(prop);
> > +
> > + prop = of_get_flat_dt_prop(node, "#address-cells", NULL);
> > + if (prop)
> > + addr_cells = be32_to_cpup(prop);
>
> I think we should just require these be the same size as the memory
> node which would be dt_root_*_cells.
>
> I'm fine with moving this into drivers/of/fdt.c if that simplifies things.
dt_root_*_cells are global variables, so its not a problem to get access to
them. However I wonder how can we ensure that user/device tree creator will
set #size-cells/#address-cells to the same values as for root memory node?
Would it be enough to state that in binding documentation? If so then the
reserved memory code can skip parsing them and use dt_root_*_cells directly,
what will simplify the code.
> > +
> > + my_depth = depth;
> > + /* scan next node */
> > + return 0;
> > + } else if (depth != 3 && my_depth != 2) {
> > + /* scan next node */
> > + return 0;
> > + } else if (depth < my_depth) {
> > + /* break scan now */
> > + return 1;
> > + }
>
> This code bothers me and is hard to follow. I don't think trying to
> use of_scan_flat_dt is the right approach here. What you really want
> here is check for reserved-memory node under the memory node and then
> scan each child node. This could all be done from
> early_init_dt_scan_memory.
early_init_dt_scan_memory() is also called from of_scan_flat_dt() and
it also implements similar state machine to parse fdt. The only
difference is the fact that "memory" is a direct child of root node,
so the state machine is much simpler (there is no need to parse
/memory/reserved-memory path).
> u-boot has more rich flat DT functions than
> the kernel does since the kernel does very little with the flat DT.
> Look at fdt_next_node and it's callers in u-boot for some examples.
> But still my first question remains. Can we avoid the flat DT
> altogether?
I see no such possibility. FDT is unflattened too late to do reliably
all operation required for memory reservation.
> > + /* now we are scanning a /memory/reserved-memory child */
> > +
> > + is_cma = of_flat_dt_is_compatible(node, "linux,contiguous-memory-region");
> > + is_reserved = of_flat_dt_is_compatible(node, "reserved-memory-region");
> > +
> > + if (!is_reserved && !(is_cma && IS_ENABLED(CONFIG_CMA))) {
> > + /* ignore node and scan next one */
> > + return 0;
> > + }
> > +
> > + prop = of_get_flat_dt_prop(node, "reg", &len);
> > + if (!prop || (len != (size_cells + addr_cells) * sizeof(__be32))) {
> > + pr_err("Reserved mem: node %s, incorrect \"reg\" property\n",
> > + uname);
> > + /* ignore node and scan next one */
> > + return 0;
> > + }
> > + base = dt_mem_next_cell(addr_cells, &prop);
> > + size = dt_mem_next_cell(size_cells, &prop);
> > +
> > + if (!size) {
> > + /* ignore node and scan next one */
> > + return 0;
> > + }
> > +
> > + pr_info("Reserved mem: found %s, memory base %lx, size %ld MiB\n",
> > + uname, (unsigned long)base, (unsigned long)size / SZ_1M);
> > +
> > + if (reserved_mem_count == ARRAY_SIZE(reserved_mem))
> > + return -ENOSPC;
> > +
> > + reserved_mem[reserved_mem_count].base = base;
> > + reserved_mem[reserved_mem_count].size = size;
> > + strlcpy(reserved_mem[reserved_mem_count].name, uname,
> > + sizeof(reserved_mem[reserved_mem_count].name));
> > +
> > + if (IS_ENABLED(CONFIG_CMA) && is_cma) {
> > + struct cma *cma;
> > + if (dma_contiguous_reserve_area(size, base, 0, &cma) == 0) {
> > + reserved_mem[reserved_mem_count].cma = cma;
> > + reserved_mem_count++;
> > +
> > + if (of_get_flat_dt_prop(node,
> > + "linux,default-contiguous-region",
> > + NULL))
> > + dma_contiguous_default_area = cma;
> > + }
> > + } else if (is_reserved) {
> > + if (memblock_remove(base, size) == 0)
> > + reserved_mem_count++;
> > + else
> > + pr_err("Failed to reserve memory for %s\n", uname);
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +static struct reserved_mem *get_dma_memory_region(struct device *dev)
> > +{
> > + struct device_node *node;
> > + const char *name;
> > + int i;
> > +
> > + node = of_parse_phandle(dev->of_node, "memory-region", 0);
> > + if (!node)
> > + return NULL;
> > +
> > + name = kbasename(node->full_name);
> > + for (i = 0; i < reserved_mem_count; i++)
> > + if (strcmp(name, reserved_mem[i].name) == 0)
> > + return &reserved_mem[i];
> > + return NULL;
>
> Matching against a struct device_node pointer would be more common way
> to match. So it would be good to update reserved_mem with a
> device_node ptr when we unflatten the DT.
I wonder if it really makes sense. To get device_node ptr I will need to
scan /memody/reserved-memory node and match all its children BY NAME
with the structures parsed from FDT (stored in reserved_mem array). Then
I will need to iterate again for each device node with memory-region
property to find the needed entry. Names are unique, IMHO they can serve
as a key for matching structures between FDT and regular, unflattened DT.
> > +}
> > +
> > +/**
> > + * of_reserved_mem_device_init() - assign reserved memory region to given device
> > + *
> > + * This function assign memory region pointed by "dma-memory-region" device tree
> > + * property to the given device.
> > + */
> > +void of_reserved_mem_device_init(struct device *dev)
> > +{
> > + struct reserved_mem *region = get_dma_memory_region(dev);
> > + if (!region)
> > + return;
> > +
> > + if (region->cma) {
> > + dev_set_cma_area(dev, region->cma);
> > + pr_info("Assigned CMA %s to %s device\n", region->name,
> > + dev_name(dev));
> > + } else {
> > + if (dma_declare_coherent_memory(dev, region->base, region->base,
> > + region->size, DMA_MEMORY_MAP | DMA_MEMORY_EXCLUSIVE) != 0)
> > + pr_info("Declared reserved memory %s to %s device\n",
> > + region->name, dev_name(dev));
> > + }
> > +}
> > +
> > +/**
> > + * 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 reserved_mem *region = get_dma_memory_region(dev);
> > + if (!region && !region->cma)
> > + dma_release_declared_memory(dev);
> > +}
> > +
> > +/**
> > + * dma_reserved_mem_reserve() - grab memory reserved for device exclusive use
> > + *
> > + * 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 (memblock) has been activated and all other
> > + * subsystems have already allocated/reserved memory.
> > + */
> > +void __init dma_reserved_mem_of_reserve(void)
> > +{
> > + if (initial_boot_params)
> > + of_scan_flat_dt(reserved_mem_fdt_scan, NULL);
> > +}
> > diff --git a/drivers/of/platform.c b/drivers/of/platform.c
> > index e0a6514..1e4e91d 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[] = {
> > @@ -196,6 +197,7 @@ EXPORT_SYMBOL(of_device_alloc);
> > * Returns pointer to created platform device, or NULL if a device was not
> > * registered. Unavailable devices will not get registered.
> > */
> > +
>
> stray ws change. Please remove.
>
> > struct platform_device *of_platform_device_create_pdata(
> > struct device_node *np,
> > const char *bus_id,
> > @@ -218,6 +220,8 @@ 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
> > @@ -225,6 +229,7 @@ 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;
> > }
> >
> > diff --git a/include/linux/of_reserved_mem.h b/include/linux/of_reserved_mem.h
> > new file mode 100644
> > index 0000000..1274946
> > --- /dev/null
> > +++ b/include/linux/of_reserved_mem.h
> > @@ -0,0 +1,14 @@
> > +#ifndef __OF_RESERVED_MEM_H
> > +#define __OF_RESERVED_MEM_H
> > +
> > +#ifdef CONFIG_OF_RESERVED_MEM
> > +void of_reserved_mem_device_init(struct device *dev);
> > +void of_reserved_mem_device_release(struct device *dev);
> > +void __init dma_reserved_mem_of_reserve(void);
>
> Declarations don't need __init annotation.
>
> > +#else
> > +#define of_reserved_mem_device_init(dev) (void)0
> > +#define of_reserved_mem_device_release(dev) (void)0
> > +#define dma_reserved_mem_of_reserve() (void)0
>
> static inline is preferred over defines.
>
> Rob
>
Best regards
--
Marek Szyprowski
Samsung R&D Institute Poland
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v5 2/3] drivers: of: add initialization code for dma reserved memory
2013-08-12 8:34 ` Marek Szyprowski
@ 2013-08-13 13:00 ` Rob Herring
2013-08-19 14:47 ` Marek Szyprowski
0 siblings, 1 reply; 14+ messages in thread
From: Rob Herring @ 2013-08-13 13:00 UTC (permalink / raw)
To: linux-arm-kernel
On Mon, Aug 12, 2013 at 3:34 AM, Marek Szyprowski
<m.szyprowski@samsung.com> wrote:
> Hello,
>
>
> On 8/10/2013 7:33 PM, Rob Herring wrote:
>>
>> On Fri, Aug 9, 2013 at 6:51 AM, Marek Szyprowski
>> <m.szyprowski@samsung.com> wrote:
>> > Add device tree support for contiguous and reserved memory regions
>> > defined in device tree. Initialization is done in 2 steps. First, the
>> > memory is reserved, what happens very early when only flattened device
>>
>> s/what/which/
>>
>> > tree is available. Then on device initialization the corresponding cma
>> > and reserved regions are assigned to each device structure.
>>
>> What this commit message does not tell me is why does the reservation
>> have to happen before the fdt is unflattened? It would greatly
>> simplify the code if it didn't.
>
>
> 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.
>
Okay. Just making sure.
>> > + } else if (depth == 2 && my_depth == 1 &&
>> > + strcmp(uname, "reserved-memory") == 0) {
>> > + prop = of_get_flat_dt_prop(node, "#size-cells", NULL);
>> > + if (prop)
>> > + size_cells = be32_to_cpup(prop);
>> > +
>> > + prop = of_get_flat_dt_prop(node, "#address-cells",
>> > NULL);
>> > + if (prop)
>> > + addr_cells = be32_to_cpup(prop);
>>
>> I think we should just require these be the same size as the memory
>> node which would be dt_root_*_cells.
>>
>> I'm fine with moving this into drivers/of/fdt.c if that simplifies things.
>
>
> dt_root_*_cells are global variables, so its not a problem to get access to
> them. However I wonder how can we ensure that user/device tree creator will
> set #size-cells/#address-cells to the same values as for root memory node?
> Would it be enough to state that in binding documentation? If so then the
> reserved memory code can skip parsing them and use dt_root_*_cells directly,
> what will simplify the code.
Yes, just add a note to the binding that the cell sizes are the same
as the memory node.
>> > +
>> > + my_depth = depth;
>> > + /* scan next node */
>> > + return 0;
>> > + } else if (depth != 3 && my_depth != 2) {
>> > + /* scan next node */
>> > + return 0;
>> > + } else if (depth < my_depth) {
>> > + /* break scan now */
>> > + return 1;
>> > + }
>>
>> This code bothers me and is hard to follow. I don't think trying to
>> use of_scan_flat_dt is the right approach here. What you really want
>> here is check for reserved-memory node under the memory node and then
>> scan each child node. This could all be done from
>> early_init_dt_scan_memory.
>
>
> early_init_dt_scan_memory() is also called from of_scan_flat_dt() and
> it also implements similar state machine to parse fdt. The only
> difference is the fact that "memory" is a direct child of root node,
> so the state machine is much simpler (there is no need to parse
> /memory/reserved-memory path).
>
If you have already found the memory node, then why find it again? I
don't think the existing scan functions handle anything but top-level
nodes very well. So doing something like this from
early_init_dt_scan_memory() is what I was thinking. It is a very rough
sketch:
// find the reserved-memory node
for (ndepth = 0, off = fdt_next_node(blob, parent_offset, &ndepth);
(off >= 0) && (ndepth > 0);
off = fdt_next_node(blob, off, &ndepth)) {
if (ndepth == 1) {
name = fdt_get_name(blob, off, 0), off);
if (strcmp(name, "reserved-memory") == 0) {
parent_offset = off;
break; // found
}
}
// find the child nodes
for (ndepth = 0, off = fdt_next_node(blob, parent_offset, &ndepth);
(off >= 0) && (ndepth == 1);
off = fdt_next_node(blob, off, &ndepth)) {
// now handle each child
}
These could probably be further refined with some loop iterator macros.
>> > + name = kbasename(node->full_name);
>> > + for (i = 0; i < reserved_mem_count; i++)
>> > + if (strcmp(name, reserved_mem[i].name) == 0)
>> > + return &reserved_mem[i];
>> > + return NULL;
>>
>> Matching against a struct device_node pointer would be more common way
>> to match. So it would be good to update reserved_mem with a
>> device_node ptr when we unflatten the DT.
>
>
> I wonder if it really makes sense. To get device_node ptr I will need to
> scan /memody/reserved-memory node and match all its children BY NAME
> with the structures parsed from FDT (stored in reserved_mem array). Then
> I will need to iterate again for each device node with memory-region
> property to find the needed entry. Names are unique, IMHO they can serve
> as a key for matching structures between FDT and regular, unflattened DT.
You are iterating multiple times using a string match versus iterating
once more and then doing a pointer match. However, it is not really
performance critical and is fine for now.
Rob
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v5 2/3] drivers: of: add initialization code for dma reserved memory
2013-08-09 11:51 ` [PATCH v5 2/3] drivers: of: add initialization code for dma reserved memory Marek Szyprowski
2013-08-10 17:33 ` Rob Herring
@ 2013-08-13 20:08 ` Stephen Warren
2013-08-16 5:25 ` Olof Johansson
2013-08-16 5:32 ` Olof Johansson
2 siblings, 1 reply; 14+ messages in thread
From: Stephen Warren @ 2013-08-13 20:08 UTC (permalink / raw)
To: linux-arm-kernel
On 08/09/2013 05:51 AM, Marek Szyprowski wrote:
> Add device tree support for contiguous and reserved memory regions
> defined in device tree. Initialization is done in 2 steps. First, the
> memory is reserved, what happens very early when only flattened device
> tree is available. Then on device initialization the corresponding cma
> and reserved regions are assigned to each device structure.
Hmmm. This seems an awful lot like putting SW configuration/policy
information into DT rather than HW description. This feels like a
slippery slope... Isn't this kind of thing better handled by a kernel
command-line option to set up the CMA size?
> diff --git a/Documentation/devicetree/bindings/memory.txt b/Documentation/devicetree/bindings/memory.txt
> +*** Memory binding ***
> +
> +The /memory node provides basic information about the address and size
> +of the physical memory. This node is usually filled or updated by the
> +bootloader, depending on the actual memory configuration of the given
> +hardware.
> +
> +The memory layout is described by the folllowing node:
> +
> +memory {
> + device_type = "memory";
> + reg = <(baseaddr1) (size1)
> + (baseaddr2) (size2)
> + ...
> + (baseaddrN) (sizeN)>;
> +};
> +
> +baseaddrX: the base address of the defined memory bank
> +sizeX: the size of the defined memory bank
You probably want to mention that baseaddrX is #address-cells long and
sizeX is #size-cells long. Same for the reserved regions below.
> +*** Reserved memory regions ***
...
> +Parameters for each memory region can be encoded into the device tree
> +wit the following convention:
> +
> +[(label):] (name)@(address) {
That line is DT syntax nothing to do with this binding. I would re-write
this in the more typical DT binding style where the documentation only
specifies the content of the node, not the node itself.
In particular, there's no requirement for a node name to include the
unit address (@address) if it's already unique.
> + compatible = "contiguous-memory-region", "reserved-memory-region";
> + reg = <(address) (size)>;
> + (linux,default-contiguous-region);
(...) isn't a syntax typically used in DT bindings. You'd usually put
that in a a list of "Optional Properties:".
> +Each defined region must use unique name.
Well, DT nodes are supposed to be names based on the type of object they
represent, not by the name/identity of the object they represent.
> +*** Device node's properties ***
> +
> +Once the regions in the /memory/reserved-memory node are defined, they
> +can be assigned to device nodes to enable drivers for their special use.
> +The following properties are defined:
> +
> +memory-region = <&phandle_to_defined_region>;
> +
> +This property indicates that the device driver should use the
> +memory region pointed by the given phandle.
That's quite scary. This is essentially forcing a memory-region property
into every single binding that ever exists. I guess that's not too much
worse than e.g. interrupts/clocks/..., but I think it's worth somehow
requiring bindings to "opt-in" to allowing this property to be part of
their binding rather than just definining the property globally.
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v5 2/3] drivers: of: add initialization code for dma reserved memory
2013-08-13 20:08 ` Stephen Warren
@ 2013-08-16 5:25 ` Olof Johansson
2013-08-16 16:06 ` Stephen Warren
0 siblings, 1 reply; 14+ messages in thread
From: Olof Johansson @ 2013-08-16 5:25 UTC (permalink / raw)
To: linux-arm-kernel
On Tue, Aug 13, 2013 at 02:08:44PM -0600, Stephen Warren wrote:
> On 08/09/2013 05:51 AM, Marek Szyprowski wrote:
> > Add device tree support for contiguous and reserved memory regions
> > defined in device tree. Initialization is done in 2 steps. First, the
> > memory is reserved, what happens very early when only flattened device
> > tree is available. Then on device initialization the corresponding cma
> > and reserved regions are assigned to each device structure.
>
> Hmmm. This seems an awful lot like putting SW configuration/policy
> information into DT rather than HW description. This feels like a
> slippery slope... Isn't this kind of thing better handled by a kernel
> command-line option to set up the CMA size?
Sorry, you were not part of the in-person discussion since it happened
at Linaro Connect in Dublin. The concern is that we really need a way
to describe some of these _system_ properties. They're not necessarily
hardware properties, but they are well-known and likely properties of
the system that is running.
I.e. say you have a device with a known screen resolution running some
reasonably well-known software stacks/applications that you expect
will use a certain amount of graphics memory. Or you know how large
your camera sensor is, so you know how much carveout the ISP will need
for captures. So defining how large the default carveouts should be
is very much a property of this system. We would previously have put
these settings in a board file and be done, but now we need a way to
describe it.
/chosen was proposed, but that's mostly used for things that the
sysadmin would adjust (traditionally such as firmware stdin/stdout, etc).
/options isn't a good place either.
Having them on the command line doesn't really work well, over time
we'll start to accumulate cruft there. It's bad enough for some of the
devices out there that do it, I'd say.
So, for the specific case of memory carveouts, this was suggested as the best
way forward.
-Olof
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v5 2/3] drivers: of: add initialization code for dma reserved memory
2013-08-09 11:51 ` [PATCH v5 2/3] drivers: of: add initialization code for dma reserved memory Marek Szyprowski
2013-08-10 17:33 ` Rob Herring
2013-08-13 20:08 ` Stephen Warren
@ 2013-08-16 5:32 ` Olof Johansson
2 siblings, 0 replies; 14+ messages in thread
From: Olof Johansson @ 2013-08-16 5:32 UTC (permalink / raw)
To: linux-arm-kernel
Hi,
On Fri, Aug 09, 2013 at 01:51:58PM +0200, Marek Szyprowski wrote:
> Add device tree support for contiguous and reserved memory regions
> defined in device tree. Initialization is done in 2 steps. First, the
> memory is reserved, what happens very early when only flattened device
> tree is available. Then on device initialization the corresponding cma
> and reserved regions are assigned to each device structure.
Bikeshedding on some wording, mostly in case you have to respin for other
reasons.
> diff --git a/Documentation/devicetree/bindings/memory.txt b/Documentation/devicetree/bindings/memory.txt
> new file mode 100644
> index 0000000..167d013
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/memory.txt
> @@ -0,0 +1,152 @@
> +*** Memory binding ***
> +
> +The /memory node provides basic information about the address and size
> +of the physical memory. This node is usually filled or updated by the
> +bootloader, depending on the actual memory configuration of the given
> +hardware.
> +
> +The memory layout is described by the folllowing node:
> +
> +memory {
> + device_type = "memory";
> + reg = <(baseaddr1) (size1)
> + (baseaddr2) (size2)
> + ...
> + (baseaddrN) (sizeN)>;
> +};
> +
> +baseaddrX: the base address of the defined memory bank
> +sizeX: the size of the defined memory bank
> +
> +More than one memory bank can be defined.
> +
> +
> +*** Reserved memory regions ***
> +
> +In /memory/reserved-memory node one can create additional nodes
There's really no requirement on naming here. We should instead search all of
/memory for the compatible properties.
> +describing particular reserved (excluded from normal use) memory
> +regions. Such memory regions are usually designed for the special usage
> +by various device drivers. A good example are contiguous memory
> +allocations or memory sharing with other operating system on the same
> +hardware board. Those special memory regions might depend on the board
> +configuration and devices used on the target system.
> +
> +Parameters for each memory region can be encoded into the device tree
> +wit the following convention:
> +
> +[(label):] (name)@(address) {
@(unit-address)
> + compatible = "contiguous-memory-region", "reserved-memory-region";
> + reg = <(address) (size)>;
> + (linux,default-contiguous-region);
> +};
> +
> +label: label given to the defined region (optional)
> +name: an name given to the defined region
> +address: the base address of the defined region
unit-address. It's optional unless there are conflicting node names.
> +size: the size of the memory region
> +
> +compatible: "contiguous-memory-region" - enables binding of this
> + region to Contiguous Memory Allocator (special region for
> + contiguous memory allocations, shared with movable system
> + memory, Linux kernel-specific), alternatively if
> + "reserved-memory-region" - compatibility is defined, given
> + region is assigned for exclusive usage for DMA transfers
"reserved-memory-region" isn't necessarily just for DMA transfers, it can be
used pretty much for anything. pstore comes to mind.
> +linux,default-contiguous-region: property indicating that the region
> + is the default region for all contiguous memory
> + allocations, Linux specific (optional)
Hmm. We handle some of these defaults such as device naming (i2c busses, etc)
through /alias nodes. Maybe it would be more consistent to do that here too,
by providing a phandle there instead.
> +Each defined region must use unique name.
Hmm, this is quite different from how memory nodes work, and it'd
be nice to keep similar semantics. A "name" property would make more
sense for areas that need to be named, but even then, why do names here
matter? References should be by phandle, not by name. So I think it
should likely just be scrapped.
> It is optional to specify the
> +base address, so if one wants to use autoconfiguration of the base
> +address, he must specify the '0' as base address in the 'reg' property
> +and assign ann unique name to such regions.
"an unique"
But I don't think this is a good idea, since it violates basic use of the reg
properties, where you usually don't have overlaps, and no real requirements on
names (as mentioned above).
-Olof
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v5 2/3] drivers: of: add initialization code for dma reserved memory
2013-08-16 5:25 ` Olof Johansson
@ 2013-08-16 16:06 ` Stephen Warren
0 siblings, 0 replies; 14+ messages in thread
From: Stephen Warren @ 2013-08-16 16:06 UTC (permalink / raw)
To: linux-arm-kernel
On 08/15/2013 11:25 PM, Olof Johansson wrote:
> On Tue, Aug 13, 2013 at 02:08:44PM -0600, Stephen Warren wrote:
>> On 08/09/2013 05:51 AM, Marek Szyprowski wrote:
>>> Add device tree support for contiguous and reserved memory regions
>>> defined in device tree. Initialization is done in 2 steps. First, the
>>> memory is reserved, what happens very early when only flattened device
>>> tree is available. Then on device initialization the corresponding cma
>>> and reserved regions are assigned to each device structure.
>>
>> Hmmm. This seems an awful lot like putting SW configuration/policy
>> information into DT rather than HW description. This feels like a
>> slippery slope... Isn't this kind of thing better handled by a kernel
>> command-line option to set up the CMA size?
>
> Sorry, you were not part of the in-person discussion since it happened
> at Linaro Connect in Dublin. The concern is that we really need a way
> to describe some of these _system_ properties. They're not necessarily
> hardware properties, but they are well-known and likely properties of
> the system that is running.
OK, that seems reasonable enough. It's just the first I heard of this.
I rather suspect that similar arguments will be applied to a bunch of
other data people want to put into DT. I guess we'll just have to wait
and see what gets proposed, and what really is "system data" rather than
policy:-)
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v5 2/3] drivers: of: add initialization code for dma reserved memory
2013-08-13 13:00 ` Rob Herring
@ 2013-08-19 14:47 ` Marek Szyprowski
2013-08-19 19:36 ` Rob Herring
0 siblings, 1 reply; 14+ messages in thread
From: Marek Szyprowski @ 2013-08-19 14:47 UTC (permalink / raw)
To: linux-arm-kernel
Hello,
On 8/13/2013 3:00 PM, Rob Herring wrote:
> On Mon, Aug 12, 2013 at 3:34 AM, Marek Szyprowski
> <m.szyprowski@samsung.com> wrote:
> > Hello,
> >
> >
> > On 8/10/2013 7:33 PM, Rob Herring wrote:
> >>
> >> On Fri, Aug 9, 2013 at 6:51 AM, Marek Szyprowski
> >> <m.szyprowski@samsung.com> wrote:
> >> > Add device tree support for contiguous and reserved memory regions
> >> > defined in device tree. Initialization is done in 2 steps. First, the
> >> > memory is reserved, what happens very early when only flattened device
> >>
> >> s/what/which/
> >>
> >> > tree is available. Then on device initialization the corresponding cma
> >> > and reserved regions are assigned to each device structure.
> >>
> >> What this commit message does not tell me is why does the reservation
> >> have to happen before the fdt is unflattened? It would greatly
> >> simplify the code if it didn't.
> >
> >
> > 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.
> >
>
> Okay. Just making sure.
>
>
> >> > + } else if (depth == 2 && my_depth == 1 &&
> >> > + strcmp(uname, "reserved-memory") == 0) {
> >> > + prop = of_get_flat_dt_prop(node, "#size-cells", NULL);
> >> > + if (prop)
> >> > + size_cells = be32_to_cpup(prop);
> >> > +
> >> > + prop = of_get_flat_dt_prop(node, "#address-cells",
> >> > NULL);
> >> > + if (prop)
> >> > + addr_cells = be32_to_cpup(prop);
> >>
> >> I think we should just require these be the same size as the memory
> >> node which would be dt_root_*_cells.
> >>
> >> I'm fine with moving this into drivers/of/fdt.c if that simplifies things.
> >
> >
> > dt_root_*_cells are global variables, so its not a problem to get access to
> > them. However I wonder how can we ensure that user/device tree creator will
> > set #size-cells/#address-cells to the same values as for root memory node?
> > Would it be enough to state that in binding documentation? If so then the
> > reserved memory code can skip parsing them and use dt_root_*_cells directly,
> > what will simplify the code.
>
> Yes, just add a note to the binding that the cell sizes are the same
> as the memory node.
>
> >> > +
> >> > + my_depth = depth;
> >> > + /* scan next node */
> >> > + return 0;
> >> > + } else if (depth != 3 && my_depth != 2) {
> >> > + /* scan next node */
> >> > + return 0;
> >> > + } else if (depth < my_depth) {
> >> > + /* break scan now */
> >> > + return 1;
> >> > + }
> >>
> >> This code bothers me and is hard to follow. I don't think trying to
> >> use of_scan_flat_dt is the right approach here. What you really want
> >> here is check for reserved-memory node under the memory node and then
> >> scan each child node. This could all be done from
> >> early_init_dt_scan_memory.
> >
> >
> > early_init_dt_scan_memory() is also called from of_scan_flat_dt() and
> > it also implements similar state machine to parse fdt. The only
> > difference is the fact that "memory" is a direct child of root node,
> > so the state machine is much simpler (there is no need to parse
> > /memory/reserved-memory path).
> >
>
> If you have already found the memory node, then why find it again? I
> don't think the existing scan functions handle anything but top-level
> nodes very well. So doing something like this from
> early_init_dt_scan_memory() is what I was thinking. It is a very rough
> sketch:
>
> // find the reserved-memory node
> for (ndepth = 0, off = fdt_next_node(blob, parent_offset, &ndepth);
> (off >= 0) && (ndepth > 0);
> off = fdt_next_node(blob, off, &ndepth)) {
> if (ndepth == 1) {
> name = fdt_get_name(blob, off, 0), off);
> if (strcmp(name, "reserved-memory") == 0) {
> parent_offset = off;
> break; // found
> }
> }
>
> // find the child nodes
> for (ndepth = 0, off = fdt_next_node(blob, parent_offset, &ndepth);
> (off >= 0) && (ndepth == 1);
> off = fdt_next_node(blob, off, &ndepth)) {
> // now handle each child
> }
>
> These could probably be further refined with some loop iterator macros.
The above code looks pretty nice, but there are some problems with it:
1. Although it would look nice to call it from early_init_dt_scan_memory()
it won't be possible, because that time it is too early. memblock structures
are initialized after a call to early_init_dt_scan_memory() and until that
no changes to memory layout are easily possible.
2. Currently there are no fdt parsing functions in the kernel. I've tried
to split of_scan_flat_dt() into fdt_next_node() + fdt_get_name() and use
them both in of_scan_flat_dt() and above reserved memory parsing functions.
In the end I got quite a lot of code, which is still quite hard to follow.
Because of the above I decided to resurrect of_scan_flat_dt_by_path()
function from the previous version and use #size-cells/#address-cells
attributes from root node what in the end simplified reserved memory
parsing function. I hope that it can be accepted after such changes
without introducing a burden caused by the whole infrastructure for
manual parsing fdt.
> >> > + name = kbasename(node->full_name);
> >> > + for (i = 0; i < reserved_mem_count; i++)
> >> > + if (strcmp(name, reserved_mem[i].name) == 0)
> >> > + return &reserved_mem[i];
> >> > + return NULL;
> >>
> >> Matching against a struct device_node pointer would be more common way
> >> to match. So it would be good to update reserved_mem with a
> >> device_node ptr when we unflatten the DT.
> >
> >
> > I wonder if it really makes sense. To get device_node ptr I will need to
> > scan /memody/reserved-memory node and match all its children BY NAME
> > with the structures parsed from FDT (stored in reserved_mem array). Then
> > I will need to iterate again for each device node with memory-region
> > property to find the needed entry. Names are unique, IMHO they can serve
> > as a key for matching structures between FDT and regular, unflattened DT.
>
> You are iterating multiple times using a string match versus iterating
> once more and then doing a pointer match. However, it is not really
> performance critical and is fine for now.
Thanks!
Best regards
--
Marek Szyprowski
Samsung R&D Institute Poland
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v5 2/3] drivers: of: add initialization code for dma reserved memory
2013-08-19 14:47 ` Marek Szyprowski
@ 2013-08-19 19:36 ` Rob Herring
0 siblings, 0 replies; 14+ messages in thread
From: Rob Herring @ 2013-08-19 19:36 UTC (permalink / raw)
To: linux-arm-kernel
On Mon, Aug 19, 2013 at 9:47 AM, Marek Szyprowski
<m.szyprowski@samsung.com> wrote:
> Hello,
>
>
> On 8/13/2013 3:00 PM, Rob Herring wrote:
>>
>> On Mon, Aug 12, 2013 at 3:34 AM, Marek Szyprowski
>> <m.szyprowski@samsung.com> wrote:
>> > Hello,
>> >
>> >
>> > On 8/10/2013 7:33 PM, Rob Herring wrote:
[snip]
>> > early_init_dt_scan_memory() is also called from of_scan_flat_dt() and
>> > it also implements similar state machine to parse fdt. The only
>> > difference is the fact that "memory" is a direct child of root node,
>> > so the state machine is much simpler (there is no need to parse
>> > /memory/reserved-memory path).
>> >
>>
>> If you have already found the memory node, then why find it again? I
>> don't think the existing scan functions handle anything but top-level
>> nodes very well. So doing something like this from
>> early_init_dt_scan_memory() is what I was thinking. It is a very rough
>> sketch:
>>
>> // find the reserved-memory node
>> for (ndepth = 0, off = fdt_next_node(blob, parent_offset, &ndepth);
>> (off >= 0) && (ndepth > 0);
>> off = fdt_next_node(blob, off, &ndepth)) {
>> if (ndepth == 1) {
>> name = fdt_get_name(blob, off, 0), off);
>> if (strcmp(name, "reserved-memory") == 0) {
>> parent_offset = off;
>> break; // found
>> }
>> }
>>
>> // find the child nodes
>> for (ndepth = 0, off = fdt_next_node(blob, parent_offset, &ndepth);
>> (off >= 0) && (ndepth == 1);
>> off = fdt_next_node(blob, off, &ndepth)) {
>> // now handle each child
>> }
>>
>> These could probably be further refined with some loop iterator macros.
>
>
> The above code looks pretty nice, but there are some problems with it:
>
> 1. Although it would look nice to call it from early_init_dt_scan_memory()
> it won't be possible, because that time it is too early. memblock structures
> are initialized after a call to early_init_dt_scan_memory() and until that
> no changes to memory layout are easily possible.
Okay. So the reserved areas have to be setup after memblock_add is
called. It seems that what exactly early_init_dt_add_memory_arch does
varies by arch. For example, arm64 directly does a memblock_add call
and would probably work as I suggested. The arm code just puts the
information in an intermediate arch specific struct until later. There
is a lot of room for clean-up in the early DT code to reduce the
amount of arch code.
> 2. Currently there are no fdt parsing functions in the kernel. I've tried
> to split of_scan_flat_dt() into fdt_next_node() + fdt_get_name() and use
> them both in of_scan_flat_dt() and above reserved memory parsing functions.
> In the end I got quite a lot of code, which is still quite hard to follow.
>
> Because of the above I decided to resurrect of_scan_flat_dt_by_path()
> function from the previous version and use #size-cells/#address-cells
> attributes from root node what in the end simplified reserved memory
> parsing function. I hope that it can be accepted after such changes
> without introducing a burden caused by the whole infrastructure for
> manual parsing fdt.
This all looks fine at first glance.
Rob
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2013-08-19 19:36 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-08-09 11:51 [PATCH v5 0/3] Device Tree support for CMA (Contiguous Memory Allocator) Marek Szyprowski
2013-08-09 11:51 ` [PATCH v5 1/3] drivers: dma-contiguous: clean source code and prepare for device tree Marek Szyprowski
2013-08-09 11:51 ` [PATCH v5 2/3] drivers: of: add initialization code for dma reserved memory Marek Szyprowski
2013-08-10 17:33 ` Rob Herring
2013-08-12 8:34 ` Marek Szyprowski
2013-08-13 13:00 ` Rob Herring
2013-08-19 14:47 ` Marek Szyprowski
2013-08-19 19:36 ` Rob Herring
2013-08-13 20:08 ` Stephen Warren
2013-08-16 5:25 ` Olof Johansson
2013-08-16 16:06 ` Stephen Warren
2013-08-16 5:32 ` Olof Johansson
2013-08-09 11:51 ` [PATCH v5 3/3] ARM: init: add support for reserved memory defined by device tree Marek Szyprowski
2013-08-09 13:19 ` [PATCH v5 0/3] Device Tree support for CMA (Contiguous Memory Allocator) Tomasz Figa
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.