reserved-memory regions/CMA in devicetree, again

From: Grant Likely <[email protected]>

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

Signed-off-by Grant Likely <[email protected]>

[joshc: Based on binding document proposed (in non-patch form) here:
http://lkml.kernel.org/g/20131030134702.19B57C402A0 at trevor.secretlab.ca adapted to support #memory-region-cells]

Signed-off-by Josh Cartwright <[email protected]>

Signed-off-by Marek Szyprowski <[email protected]>

---
 .../bindings/reserved-memory/reserved-memory.txt   |  138 ++++++++++++++++++++
 1 file changed, 138 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt

Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt new file +7 -0

new file mode 100644
index 000000000000..a606ce90c9c4
@@ -0,0 +1,138 @@
+*** Reserved memory regions ***
+
+Reserved memory is specified as a node under the /reserved-memory node.
+The operating system shall exclude reserved memory from normal usage
+one can create child nodes describing particular reserved (excluded from
+normal use) memory regions. Such memory regions are usually designed for
+the special usage by various device drivers.

Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt new file +29 -0

+ranges (required) - standard definition
+    - Should be empty
+
+/reserved-memory/ child nodes
+-----------------------------
+Each child of the reserved-memory node specifies one or more regions of
+reserved memory. Each child node may either use a 'reg' property to
+specify a specific range of reserved memory, or a 'size' property with
+optional constraints to request a dynamically allocated block of memory.
+
+Following the generic-names recommended practice, node names should
+reflect the purpose of the node (ie. "framebuffer" or "dma-pool"). Unit
+address (@<address>) should be appended to the name if the node is a
+static allocation.
+
+Properties:
+Requires either a) or b) below.
+a) static allocation
+   reg (required) - standard definition
+b) dynamic allocation
+   size (required) - length based on parent's #size-cells
+                   - Size in bytes of memory to reserve.
+   alignment (optional) - length based on parent's #size-cells
+                        - Address boundary for alignment of allocation.
+   alloc-ranges (optional) - prop-encoded-array (address, length pairs).
+                           - Specifies regions of memory that are
+                             acceptable to allocate from.
+
+If both reg and size are present, then the reg property takes precedence

Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt new file +82 -0

+no-map (optional) - empty property
+    - Indicates the operating system must not create a virtual mapping
+      of the region as part of its standard mapping of system memory,
+      nor permit speculative access to it under any circumstances other
+      than under the control of the device driver using the region.
+reusable (optional) - empty property
+    - The operating system can use the memory in this region with the
+      limitation that the device driver(s) owning the region need to be
+      able to reclaim it back. Typically that means that the operating
+      system can use that region to store volatile or cached data that
+      can be otherwise regenerated or migrated elsewhere.
+
+Linux implementation note:
+- If a "linux,cma-default" property is present, then Linux will use the
+  region for the default pool of the contiguous memory allocator.
+
+Device node references to reserved memory
+-----------------------------------------
+Regions in the /reserved-memory node may be referenced by other device
+nodes by adding a memory-region property to the device node.
+
+memory-region (optional) - phandle, specifier pairs to children of /reserved-memory
+
+Example
+-------
+This example defines 3 contiguous regions are defined for Linux kernel:
+one default of all device drivers (named linux,cma at 72000000 and 64MiB in size),
+one dedicated to the framebuffer device (named framebuffer at 78000000, 8MiB), and
+one for multimedia processing (named multimedia-memory at 77000000, 64MiB).
+
+/ {
+	#address-cells = <1>;
+	#size-cells = <1>;
+
+	memory {
+		reg = <0x40000000 0x40000000>;
+	};
+
+	reserved-memory {
+		#address-cells = <1>;
+		#size-cells = <1>;
+		ranges;
+
+		/* global autoconfigured region for contiguous allocations */
+		linux,cma {
+			compatible = "shared-dma-pool";
+			reusable;
+			#memory-region-cells = <0>;
+			size = <0x4000000>;
+			alignment = <0x2000>;
+			linux,cma-default;
+		};
+
+		display_reserved: framebuffer at 78000000 {
+			#memory-region-cells = <0>;
+			reg = <0x78000000 0x800000>;
+		};
+
+		multimedia_reserved: multimedia at 77000000 {
+			compatible = "acme,multimedia-memory";
+			#memory-region-cells = <1>;
+			reg = <0x77000000 0x4000000>;
+		};
+	};
+
+	/* ... */
+
+	fb0: video at 12300000 {
+		memory-region = <&display_reserved>;
+		/* ... */
+	};
+
+	scaler: scaler at 12500000 {
+		memory-region = <&multimedia_reserved 0xdeadbeef>;
+		/* ... */
+	};
+
+	codec: codec at 12600000 {
+		memory-region = <&multimedia_reserved 0xfeebdaed>;
+		/* ... */
+	};
+};

Signature · 1 line

1.7.9.5

Quoted 30 lines · Marek Szyprowski

From: Grant Likely <[email protected]>

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

Signed-off-by Grant Likely <[email protected]>

[joshc: Based on binding document proposed (in non-patch form) here:
http://lkml.kernel.org/g/20131030134702.19B57C402A0 at trevor.secretlab.ca adapted to support #memory-region-cells]

Signed-off-by Josh Cartwright <[email protected]>

Signed-off-by Marek Szyprowski <[email protected]>

---
 .../bindings/reserved-memory/reserved-memory.txt   |  138 ++++++++++++++++++++
 1 file changed, 138 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt

Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt new file +7 -0

new file mode 100644
index 000000000000..a606ce90c9c4
@@ -0,0 +1,138 @@
+*** Reserved memory regions ***
+
+Reserved memory is specified as a node under the /reserved-memory node.
+The operating system shall exclude reserved memory from normal usage
+one can create child nodes describing particular reserved (excluded from
+normal use) memory regions. Such memory regions are usually designed for
+the special usage by various device drivers.

Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt line 17

Quoted 10 lines

Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt new file +10 -0

+
+Parameters for each memory region can be encoded into the device tree
+with the following nodes:
+
+/reserved-memory node
+---------------------
+#address-cells, #size-cells (required) - standard definition
+    - Should use the same values as the root node
+#memory-region-cells (required) - dictates number of cells used in the child
+                                  nodes memory-region specifier

I still don't like this portion of the binding. I'm not convinced that it is necessary in the majority of cases and it is going to be very driver specific. I would rather drop it entirely from the common binding. If a specific driver needs to do something like the above then it can have a driver specific binding. Otherwise I think the default should be a simple phandle with no arguments to a single reserved memory node.

Ben, can you weigh in on the current state of this document. I'm mostly happy with it aside from my comment above. Do you think this is ready to be merged?

Quoted 35 lines

Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt new file +35 -0

+ranges (required) - standard definition
+    - Should be empty
+
+/reserved-memory/ child nodes
+-----------------------------
+Each child of the reserved-memory node specifies one or more regions of
+reserved memory. Each child node may either use a 'reg' property to
+specify a specific range of reserved memory, or a 'size' property with
+optional constraints to request a dynamically allocated block of memory.
+
+Following the generic-names recommended practice, node names should
+reflect the purpose of the node (ie. "framebuffer" or "dma-pool"). Unit
+address (@<address>) should be appended to the name if the node is a
+static allocation.
+
+Properties:
+Requires either a) or b) below.
+a) static allocation
+   reg (required) - standard definition
+b) dynamic allocation
+   size (required) - length based on parent's #size-cells
+                   - Size in bytes of memory to reserve.
+   alignment (optional) - length based on parent's #size-cells
+                        - Address boundary for alignment of allocation.
+   alloc-ranges (optional) - prop-encoded-array (address, length pairs).
+                           - Specifies regions of memory that are
+                             acceptable to allocate from.
+
+If both reg and size are present, then the reg property takes precedence
+and size is ignored.
+
+Additional properties:
+compatible (optional) - standard definition
+    - may contain the following strings:
+        - shared-dma-pool: This indicates a region of memory meant to be

Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt line 56

Quoted 4 lines

Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt new file +4 -0

+          used as a shared pool of DMA buffers for a set of devices. It can
+          be used by an operating system to instanciate the necessary pool
+          management subsystem if necessary.
+        - vendor specific string in the form <vendor>,[<device>-]<usage>

Add "Use vendor strings to identify regions dedicates for a specific vendor device. For example: 'acme,framebuffer'. Platform code can use vendor strings to identify device specific regions"

Quoted 77 lines

Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt new file +77 -0

+no-map (optional) - empty property
+    - Indicates the operating system must not create a virtual mapping
+      of the region as part of its standard mapping of system memory,
+      nor permit speculative access to it under any circumstances other
+      than under the control of the device driver using the region.
+reusable (optional) - empty property
+    - The operating system can use the memory in this region with the
+      limitation that the device driver(s) owning the region need to be
+      able to reclaim it back. Typically that means that the operating
+      system can use that region to store volatile or cached data that
+      can be otherwise regenerated or migrated elsewhere.
+
+Linux implementation note:
+- If a "linux,cma-default" property is present, then Linux will use the
+  region for the default pool of the contiguous memory allocator.
+
+Device node references to reserved memory
+-----------------------------------------
+Regions in the /reserved-memory node may be referenced by other device
+nodes by adding a memory-region property to the device node.
+
+memory-region (optional) - phandle, specifier pairs to children of /reserved-memory
+
+Example
+-------
+This example defines 3 contiguous regions are defined for Linux kernel:
+one default of all device drivers (named linux,cma at 72000000 and 64MiB in size),
+one dedicated to the framebuffer device (named framebuffer at 78000000, 8MiB), and
+one for multimedia processing (named multimedia-memory at 77000000, 64MiB).
+
+/ {
+	#address-cells = <1>;
+	#size-cells = <1>;
+
+	memory {
+		reg = <0x40000000 0x40000000>;
+	};
+
+	reserved-memory {
+		#address-cells = <1>;
+		#size-cells = <1>;
+		ranges;
+
+		/* global autoconfigured region for contiguous allocations */
+		linux,cma {
+			compatible = "shared-dma-pool";
+			reusable;
+			#memory-region-cells = <0>;
+			size = <0x4000000>;
+			alignment = <0x2000>;
+			linux,cma-default;
+		};
+
+		display_reserved: framebuffer at 78000000 {
+			#memory-region-cells = <0>;
+			reg = <0x78000000 0x800000>;
+		};
+
+		multimedia_reserved: multimedia at 77000000 {
+			compatible = "acme,multimedia-memory";
+			#memory-region-cells = <1>;
+			reg = <0x77000000 0x4000000>;
+		};
+	};
+
+	/* ... */
+
+	fb0: video at 12300000 {
+		memory-region = <&display_reserved>;
+		/* ... */
+	};
+
+	scaler: scaler at 12500000 {
+		memory-region = <&multimedia_reserved 0xdeadbeef>;
+		/* ... */
+	};
+

Quoted 8 lines

Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt new file +5 -1

+	codec: codec at 12600000 {
+		memory-region = <&multimedia_reserved 0xfeebdaed>;
+		/* ... */
+	};
+};
--

1.7.9.5

Hello,

Quoted 115 lines · Grant Likely

Quoted 9 lines · Marek Szyprowski

From: Grant Likely <[email protected]>

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

Signed-off-by Grant Likely <[email protected]>

[joshc: Based on binding document proposed (in non-patch form) here:

http://lkml.kernel.org/g/20131030134702.19B57C402A0 at trevor.secretlab.ca

Quoted 5 lines

adapted to support #memory-region-cells]

Signed-off-by Josh Cartwright <[email protected]>

Signed-off-by Marek Szyprowski <[email protected]>

---
 .../bindings/reserved-memory/reserved-memory.txt   |  138

++++++++++++++++++++

Quoted 2 lines

 1 file changed, 138 insertions(+)
 create mode 100644

Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt

Quoted 2 lines

diff --git

a/Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt b/Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt

Quoted 4 lines

new file mode 100644
index 000000000000..a606ce90c9c4
--- /dev/null
+++

b/Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt

Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt line 3

Quoted 4 lines

Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt new file +3 -0

@@ -0,0 +1,138 @@
+*** Reserved memory regions ***
+
+Reserved memory is specified as a node under the /reserved-memory

node.

Review line 5

Quoted 2 lines

(quoted diff) +2 -0

+The operating system shall exclude reserved memory from normal usage
+one can create child nodes describing particular reserved (excluded

from

Review line 6

Quoted 1 line

(quoted diff) +1 -0

+normal use) memory regions. Such memory regions are usually

designed for

Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt

Quoted 10 lines

Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt new file +10 -0

+the special usage by various device drivers.
+
+Parameters for each memory region can be encoded into the device tree
+with the following nodes:
+
+/reserved-memory node
+---------------------
+#address-cells, #size-cells (required) - standard definition
+    - Should use the same values as the root node
+#memory-region-cells (required) - dictates number of cells used in

the child

Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt line 17

Quoted 1 line

Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt new file +1 -0

+                                  nodes memory-region specifier

I still don't like this portion of the binding. I'm not convinced that it is necessary in the majority of cases and it is going to be very driver specific. I would rather drop it entirely from the common binding. If a specific driver needs to do something like the above then it can have a driver specific binding. Otherwise I think the default should be a simple phandle with no arguments to a single reserved memory node.

Ben, can you weigh in on the current state of this document. I'm mostly happy with it aside from my comment above. Do you think this is ready to be merged?

Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt

Quoted 6 lines

Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt new file +6 -0

+ranges (required) - standard definition
+    - Should be empty
+
+/reserved-memory/ child nodes
+-----------------------------
+Each child of the reserved-memory node specifies one or more

regions of

Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt

Quoted 3 lines

Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt new file +3 -0

+reserved memory. Each child node may either use a 'reg' property to
+specify a specific range of reserved memory, or a 'size' property with
+optional constraints to request a dynamically allocated block of

memory.

Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt

Quoted 3 lines

Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt new file +3 -0

+
+Following the generic-names recommended practice, node names should
+reflect the purpose of the node (ie. "framebuffer" or "dma-pool").

Unit

Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt

Quoted 12 lines

Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt new file +12 -0

+address (@<address>) should be appended to the name if the node is a
+static allocation.
+
+Properties:
+Requires either a) or b) below.
+a) static allocation
+   reg (required) - standard definition
+b) dynamic allocation
+   size (required) - length based on parent's #size-cells
+                   - Size in bytes of memory to reserve.
+   alignment (optional) - length based on parent's #size-cells
+                        - Address boundary for alignment of

allocation.

Review line 42

Quoted 1 line

(quoted diff) +1 -0

+   alloc-ranges (optional) - prop-encoded-array (address, length

pairs).

Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt

Quoted 4 lines

Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt new file +4 -0

+                           - Specifies regions of memory that are
+                             acceptable to allocate from.
+
+If both reg and size are present, then the reg property takes

precedence

Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt

Quoted 6 lines

Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt new file +6 -0

+and size is ignored.
+
+Additional properties:
+compatible (optional) - standard definition
+    - may contain the following strings:
+        - shared-dma-pool: This indicates a region of memory meant

to be

Review line 53

Quoted 1 line

(quoted diff) +1 -0

+          used as a shared pool of DMA buffers for a set of

devices. It can

Review line 54

Quoted 1 line

(quoted diff) +1 -0

+          be used by an operating system to instanciate the

necessary pool

Review line 56

Quoted 2 lines

(quoted diff) +2 -0

+          management subsystem if necessary.
+        - vendor specific string in the form

<vendor>,[<device>-]<usage>

Quoted 4 lines

Add "Use vendor strings to identify regions dedicates for a specific vendor device. For example: 'acme,framebuffer'. Platform code can use vendor
strings to identify device specific regions"

So do you want to completely drop phandle based links between device nodes and
memory regions?

Hello,

Quoted 89 lines · Grant Likely

Quoted 30 lines · Marek Szyprowski

From: Grant Likely <[email protected]>

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

Signed-off-by Grant Likely <[email protected]>

[joshc: Based on binding document proposed (in non-patch form) here:
http://lkml.kernel.org/g/20131030134702.19B57C402A0 at trevor.secretlab.ca adapted to support #memory-region-cells]

Signed-off-by Josh Cartwright <[email protected]>

Signed-off-by Marek Szyprowski <[email protected]>

---
 .../bindings/reserved-memory/reserved-memory.txt   |  138 ++++++++++++++++++++
 1 file changed, 138 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt

Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt new file +7 -0

new file mode 100644
index 000000000000..a606ce90c9c4
@@ -0,0 +1,138 @@
+*** Reserved memory regions ***
+
+Reserved memory is specified as a node under the /reserved-memory node.
+The operating system shall exclude reserved memory from normal usage
+one can create child nodes describing particular reserved (excluded from
+normal use) memory regions. Such memory regions are usually designed for
+the special usage by various device drivers.

Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt line 17

Quoted 10 lines

Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt new file +10 -0

+
+Parameters for each memory region can be encoded into the device tree
+with the following nodes:
+
+/reserved-memory node
+---------------------
+#address-cells, #size-cells (required) - standard definition
+    - Should use the same values as the root node
+#memory-region-cells (required) - dictates number of cells used in the child
+                                  nodes memory-region specifier

I still don't like this portion of the binding. I'm not convinced that it is necessary in the majority of cases and it is going to be very driver specific. I would rather drop it entirely from the common binding. If a specific driver needs to do something like the above then it can have a driver specific binding. Otherwise I think the default should be a simple phandle with no arguments to a single reserved memory node.

Ben, can you weigh in on the current state of this document. I'm mostly happy with it aside from my comment above. Do you think this is ready to be merged?

Quoted 35 lines

Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt new file +35 -0

+ranges (required) - standard definition
+    - Should be empty
+
+/reserved-memory/ child nodes
+-----------------------------
+Each child of the reserved-memory node specifies one or more regions of
+reserved memory. Each child node may either use a 'reg' property to
+specify a specific range of reserved memory, or a 'size' property with
+optional constraints to request a dynamically allocated block of memory.
+
+Following the generic-names recommended practice, node names should
+reflect the purpose of the node (ie. "framebuffer" or "dma-pool"). Unit
+address (@<address>) should be appended to the name if the node is a
+static allocation.
+
+Properties:
+Requires either a) or b) below.
+a) static allocation
+   reg (required) - standard definition
+b) dynamic allocation
+   size (required) - length based on parent's #size-cells
+                   - Size in bytes of memory to reserve.
+   alignment (optional) - length based on parent's #size-cells
+                        - Address boundary for alignment of allocation.
+   alloc-ranges (optional) - prop-encoded-array (address, length pairs).
+                           - Specifies regions of memory that are
+                             acceptable to allocate from.
+
+If both reg and size are present, then the reg property takes precedence
+and size is ignored.
+
+Additional properties:
+compatible (optional) - standard definition
+    - may contain the following strings:
+        - shared-dma-pool: This indicates a region of memory meant to be

Quoted 8 lines

Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt line 56

Quoted 4 lines

Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt new file +4 -0

+          used as a shared pool of DMA buffers for a set of devices. It can
+          be used by an operating system to instanciate the necessary pool
+          management subsystem if necessary.
+        - vendor specific string in the form <vendor>,[<device>-]<usage>

Add "Use vendor strings to identify regions dedicates for a specific vendor device. For example: 'acme,framebuffer'. Platform code can use vendor strings to identify device specific regions"

So do you want to completely drop phandle based links between device nodes and memory regions?

This patch adds support for static (defined by 'reg' property) reserved memory regions declared in device tree.

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). Typically, all this happens before device tree structures are unflattened, so we need to get reserved memory layout directly from fdt.

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

Signed-off-by Marek Szyprowski <[email protected]>

---
 drivers/of/fdt.c       |  125 ++++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/of_fdt.h |    3 ++
 2 files changed, 128 insertions(+)

drivers/of/fdt.c +14 -0

index 758b4f8b30b7..12809e20ef71 100644
@@ -440,6 +440,113 @@ struct boot_param_header *initial_boot_params;
 #ifdef CONFIG_OF_EARLY_FLATTREE
 
 /**
+ * res_mem_reserve_reg() - reserve all memory described in 'reg' property
+ */
+static int __init __reserved_mem_reserve_reg(unsigned long node,
+					     const char *uname)
+{
+	int t_len = (dt_root_addr_cells + dt_root_size_cells) * sizeof(__be32);
+	phys_addr_t base, size;
+	unsigned long len;
+	__be32 *prop;
+	int nomap;
+
+	prop = of_get_flat_dt_prop(node, "reg", &len);
+	if (!prop)
+		return -ENOENT;

drivers/of/fdt.c +7 -0

+		base = dt_mem_next_cell(dt_root_addr_cells, &prop);
+		size = dt_mem_next_cell(dt_root_size_cells, &prop);
+
+		if (base && size &&
+		    early_init_dt_reserve_memory_arch(base, size, nomap) == 0)
+			pr_debug("Reserved memory: reserved region for node '%s': base %pa, size %ld MiB\n",
+				uname, &base, (unsigned long)size / SZ_1M);

drivers/of/fdt.c +54 -0

+{
+	__be32 *prop;
+
+	prop = of_get_flat_dt_prop(node, "#size-cells", NULL);
+	if (prop && be32_to_cpup(prop) != dt_root_size_cells)
+		return -EINVAL;
+
+	prop = of_get_flat_dt_prop(node, "#address-cells", NULL);
+	if (prop && be32_to_cpup(prop) != dt_root_addr_cells)
+		return -EINVAL;
+
+	prop = of_get_flat_dt_prop(node, "ranges", NULL);
+	if (!prop)
+		return -EINVAL;
+	return 0;
+}
+
+/**
+ * fdt_scan_reserved_mem() - scan a single FDT node for reserved memory
+ */
+static int __init __fdt_scan_reserved_mem(unsigned long node, const char *uname,
+					  int depth, void *data)
+{
+	static int found;
+	const char *status;
+
+	if (!found && depth == 1 && strcmp(uname, "reserved-memory") == 0) {
+		if (__reserved_mem_check_root(node) != 0) {
+			pr_err("Reserved memory: unsupported node format, ignoring\n");
+			/* break scan */
+			return 1;
+		}
+		found = 1;
+		/* scan next node */
+		return 0;
+	} else if (!found) {
+		/* scan next node */
+		return 0;
+	} else if (found && depth < 2) {
+		/* scanning of /reserved-memory has been finished */
+		return 1;
+	}
+
+	status = of_get_flat_dt_prop(node, "status", NULL);
+	if (status && strcmp(status, "okay") != 0 && strcmp(status, "ok") != 0)
+		return 0;
+
+	__reserved_mem_reserve_reg(node, uname);
+
+	/* scan next node */
+	return 0;
+}
+
+/**

drivers/of/fdt.c +20 -0

+
+/**
  * of_scan_flat_dt - scan flattened tree blob and call callback on each.
  * @it: callback function
  * @data: context data pointer

@@ -856,6 +963,16 @@ void __init __weak early_init_dt_add_memory_arch(u64 base, u64 size)
 	memblock_add(base, size);
 }
 
+int __init __weak early_init_dt_reserve_memory_arch(phys_addr_t base,
+					phys_addr_t size, bool nomap)
+{
+	if (memblock_is_region_reserved(base, size))
+		return -EBUSY;
+	if (nomap)
+		return memblock_remove(base, size);
+	return memblock_reserve(base, size);
+}
+
 /*
  * called from unflatten_device_tree() to bootstrap devicetree itself
  * Architectures can override this definition if memblock isn't used

@@ -864,6 +981,14 @@ void * __init __weak early_init_dt_alloc_memory_arch(u64 size, u64 align)
 {
 	return __va(memblock_alloc(size, align));
 }
+#else
+int __init __weak early_init_dt_reserve_memory_arch(phys_addr_t base,
+					phys_addr_t size, bool nomap)
+{
+	pr_error("Reserved memory not supported, ignoring range 0x%llx - 0x%llx%s\n",
+		  base, size, nomap ? " (nomap)" : "");
+	return -ENOSYS;
+}
 #endif
 
 bool __init early_init_dt_scan(void *params)

include/linux/of_fdt.h +0 -0

index 2b77058a7335..8610ad8d77d2 100644

Signature · 2 lines

-- 
1.7.9.5

This patch adds support for dynamically allocated reserved memory regions declared in device tree. Such regions are defined by 'size', 'alignment' and 'alloc-ranges' properties.

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

Signed-off-by Marek Szyprowski <[email protected]>

---
 drivers/of/Kconfig              |    6 ++
 drivers/of/Makefile             |    1 +
 drivers/of/fdt.c                |   13 ++-
 drivers/of/of_reserved_mem.c    |  188 +++++++++++++++++++++++++++++++++++++++
 include/linux/of_reserved_mem.h |   21 +++++
 5 files changed, 227 insertions(+), 2 deletions(-)
 create mode 100644 drivers/of/of_reserved_mem.c
 create mode 100644 include/linux/of_reserved_mem.h

drivers/of/Kconfig +6 -0

index c6973f101a3e..30a7d87a8077 100644
@@ -75,4 +75,10 @@ config OF_MTD
 	depends on MTD
 	def_bool y
 
+config OF_RESERVED_MEM
+	depends on OF_EARLY_FLATTREE
+	bool
+	help
+	  Helpers to allow for reservation of memory regions
+
 endmenu # OF

drivers/of/Makefile +1 -0

index efd05102c405..ed9660adad77 100644
@@ -9,3 +9,4 @@ obj-$(CONFIG_OF_MDIO)	+= of_mdio.o
 obj-$(CONFIG_OF_PCI)	+= of_pci.o
 obj-$(CONFIG_OF_PCI_IRQ)  += of_pci_irq.o
 obj-$(CONFIG_OF_MTD)	+= of_mtd.o
+obj-$(CONFIG_OF_RESERVED_MEM) += of_reserved_mem.o

drivers/of/fdt.c +11 -2

index 12809e20ef71..eafe5805257a 100644
@@ -15,6 +15,7 @@
 #include <linux/module.h>
 #include <linux/of.h>
 #include <linux/of_fdt.h>
+#include <linux/of_reserved_mem.h>
 #include <linux/string.h>
 #include <linux/errno.h>
 #include <linux/slab.h>

@@ -449,7 +450,7 @@ static int __init __reserved_mem_reserve_reg(unsigned long node,
 	phys_addr_t base, size;
 	unsigned long len;
 	__be32 *prop;
-	int nomap;
+	int nomap, first = 1;
 
 	prop = of_get_flat_dt_prop(node, "reg", &len);
 	if (!prop)

@@ -476,6 +477,10 @@ static int __init __reserved_mem_reserve_reg(unsigned long node,
 				uname, &base, (unsigned long)size / SZ_1M);
 
 		len -= t_len;
+		if (first) {
+			fdt_reserved_mem_save_node(node, uname, base, size);
+			first = 0;
+		}
 	}
 	return 0;
 }

@@ -506,6 +511,7 @@ static int __init __fdt_scan_reserved_mem(unsigned long node, const char *uname,
 {
 	static int found;
 	const char *status;
+	int err;
 
 	if (!found && depth == 1 && strcmp(uname, "reserved-memory") == 0) {
 		if (__reserved_mem_check_root(node) != 0) {

@@ -528,7 +534,9 @@ static int __init __fdt_scan_reserved_mem(unsigned long node, const char *uname,
 	if (status && strcmp(status, "okay") != 0 && strcmp(status, "ok") != 0)
 		return 0;
 
-	__reserved_mem_reserve_reg(node, uname);
+	err = __reserved_mem_reserve_reg(node, uname);
+	if (err == -ENOENT && of_get_flat_dt_prop(node, "size", NULL))
+		fdt_reserved_mem_save_node(node, uname, 0, 0);
 
 	/* scan next node */
 	return 0;

@@ -544,6 +552,7 @@ static int __init __fdt_scan_reserved_mem(unsigned long node, const char *uname,
 void __init early_init_fdt_scan_reserved_mem(void)
 {
 	of_scan_flat_dt(__fdt_scan_reserved_mem, NULL);
+	fdt_init_reserved_mem();
 }
 
 /**

drivers/of/of_reserved_mem.c new file +188 -0

new file mode 100644
index 000000000000..c7ca6a4a42d1
@@ -0,0 +1,188 @@
+/*
+ * Device tree based initialization code for reserved memory.
+ *
+ * Copyright (c) 2013, The Linux Foundation. All Rights Reserved.
+ * Copyright (c) 2013,2014 Samsung Electronics Co., Ltd.
+ *		http://www.samsung.com
+ * Author: Marek Szyprowski <[email protected]>
+ * Author: Josh Cartwright <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of the
+ * License or (at your optional) any later version of the license.
+ */
+
+#include <linux/err.h>
+#include <linux/of.h>
+#include <linux/of_fdt.h>
+#include <linux/of_platform.h>
+#include <linux/mm.h>
+#include <linux/sizes.h>
+#include <linux/of_reserved_mem.h>
+
+#define MAX_RESERVED_REGIONS	16
+static struct reserved_mem reserved_mem[MAX_RESERVED_REGIONS];
+static int reserved_mem_count;
+
+#if defined(CONFIG_HAVE_MEMBLOCK)
+#include <linux/memblock.h>
+int __init __weak early_init_dt_alloc_reserved_memory_arch(phys_addr_t size,
+	phys_addr_t align, phys_addr_t start, phys_addr_t end, bool nomap,
+	phys_addr_t *res_base)
+{
+	/*
+	 * We use __memblock_alloc_base() because memblock_alloc_base()
+	 * panic()s on allocation failure.
+	 */
+	phys_addr_t base = __memblock_alloc_base(size, align, end);
+	if (!base)
+		return -ENOMEM;
+
+	/*
+	 * Check if the allocated region fits in to start..end window
+	 */
+	if (base < start) {
+		memblock_free(base, size);
+		return -ENOMEM;
+	}
+
+	*res_base = base;
+	if (nomap)
+		return memblock_remove(base, size);
+	return 0;
+}
+#else
+int __init __weak early_init_dt_alloc_reserved_memory_arch(phys_addr_t size,
+	phys_addr_t align, phys_addr_t start, phys_addr_t end, bool nomap,
+	phys_addr_t *res_base)
+{
+	pr_error("Reserved memory not supported, ignoring region 0x%llx%s\n",
+		  size, nomap ? " (nomap)" : "");
+	return -ENOSYS;
+}
+#endif
+
+/**
+ * res_mem_save_node() - save fdt node for second pass initialization
+ */
+void __init fdt_reserved_mem_save_node(unsigned long node, const char *uname,
+				      phys_addr_t base, phys_addr_t size)
+{
+	struct reserved_mem *rmem = &reserved_mem[reserved_mem_count];
+
+	if (reserved_mem_count == ARRAY_SIZE(reserved_mem)) {
+		pr_err("Reserved memory: not enough space all defined regions.\n");
+		return;
+	}
+
+	rmem->fdt_node = node;
+	rmem->name = uname;
+	rmem->base = base;
+	rmem->size = size;
+
+	reserved_mem_count++;
+	return;
+}
+
+/**
+ * res_mem_alloc_size() - allocate reserved memory described by 'size', 'align'
+ *			  and 'alloc-ranges' properties
+ */
+static int __init __reserved_mem_alloc_size(unsigned long node,
+	const char *uname, phys_addr_t *res_base, phys_addr_t *res_size)
+{
+	int t_len = (dt_root_addr_cells + dt_root_size_cells) * sizeof(__be32);
+	phys_addr_t start = 0, end = 0;
+	phys_addr_t base = 0, align = 0, size;
+	unsigned long len;
+	__be32 *prop;
+	int nomap;
+	int ret;
+
+	prop = of_get_flat_dt_prop(node, "size", &len);
+	if (!prop)
+		return -EINVAL;
+
+	if (len != dt_root_size_cells * sizeof(__be32)) {
+		pr_err("Reserved memory: invalid size property in '%s' node.\n",
+				uname);
+		return -EINVAL;
+	}
+	size = dt_mem_next_cell(dt_root_size_cells, &prop);
+
+	nomap = of_get_flat_dt_prop(node, "no-map", NULL) != NULL;
+
+	prop = of_get_flat_dt_prop(node, "alignment", &len);
+	if (prop) {
+		if (len != dt_root_addr_cells * sizeof(__be32)) {
+			pr_err("Reserved memory: invalid alignment property in '%s' node.\n",
+				uname);
+			return -EINVAL;
+		}
+		align = dt_mem_next_cell(dt_root_addr_cells, &prop);
+	}
+
+	prop = of_get_flat_dt_prop(node, "alloc-ranges", &len);
+	if (prop) {
+
+		if (len % t_len != 0) {
+			pr_err("Reserved memory: invalid alloc-ranges property in '%s', skipping node.\n",
+			       uname);
+			return -EINVAL;
+		}
+
+		base = 0;
+
+		while (len > 0) {
+			start = dt_mem_next_cell(dt_root_addr_cells, &prop);
+			end = start + dt_mem_next_cell(dt_root_size_cells,
+						       &prop);
+
+			ret = early_init_dt_alloc_reserved_memory_arch(size,
+					align, start, end, nomap, &base);
+			if (ret == 0) {
+				pr_debug("Reserved memory: allocated memory for '%s' node: base %pa, size %ld MiB\n",
+					uname, &base,
+					(unsigned long)size / SZ_1M);
+				break;
+			}
+			len -= t_len;
+		}
+
+	} else {
+		ret = early_init_dt_alloc_reserved_memory_arch(size, align,
+							0, 0, nomap, &base);
+		if (ret == 0)
+			pr_debug("Reserved memory: allocated memory for '%s' node: base %pa, size %ld MiB\n",
+				uname, &base, (unsigned long)size / SZ_1M);
+	}
+
+	if (base == 0) {
+		pr_info("Reserved memory: failed to allocate memory for node '%s'\n",
+			uname);
+		return -ENOMEM;
+	}
+
+	*res_base = base;
+	*res_size = size;
+
+	return 0;
+}
+
+/**
+ * fdt_init_reserved_mem - allocate and init all saved reserved memory regions
+ */
+void __init fdt_init_reserved_mem(void)
+{
+	int i;
+	for (i = 0; i < reserved_mem_count; i++) {
+		struct reserved_mem *rmem = &reserved_mem[i];
+		unsigned long node = rmem->fdt_node;
+		int err = 0;
+
+		if (rmem->size == 0)
+			err = __reserved_mem_alloc_size(node, rmem->name,
+						 &rmem->base, &rmem->size);
+	}
+}

include/linux/of_reserved_mem.h new file +21 -0

new file mode 100644
index 000000000000..89226ed7d954
@@ -0,0 +1,21 @@
+#ifndef __OF_RESERVED_MEM_H
+#define __OF_RESERVED_MEM_H
+
+struct reserved_mem {
+	const char			*name;
+	unsigned long			fdt_node;
+	phys_addr_t			base;
+	phys_addr_t			size;
+};
+
+#ifdef CONFIG_OF_RESERVED_MEM
+void fdt_init_reserved_mem(void);
+void fdt_reserved_mem_save_node(unsigned long node, const char *uname,
+			       phys_addr_t base, phys_addr_t size);
+#else
+static inline void fdt_init_reserved_mem(void) { }
+static inline void fdt_reserved_mem_save_node(unsigned long node,
+		const char *uname, phys_addr_t base, phys_addr_t size) { }
+#endif
+
+#endif /* __OF_RESERVED_MEM_H */

Signature · 1 line

1.7.9.5

Use recently introduced of_reserved_mem_device_init() function to automatically assign respective reserved memory region to the newly created platform and amba device.

Signed-off-by Marek Szyprowski <[email protected]>

---
 drivers/of/platform.c |    7 +++++++
 1 file changed, 7 insertions(+)

drivers/of/platform.c +7 -0

index 404d1daebefa..3df0b1826e8b 100644
@@ -21,6 +21,7 @@
 #include <linux/of_device.h>
 #include <linux/of_irq.h>
 #include <linux/of_platform.h>
+#include <linux/of_reserved_mem.h>
 #include <linux/platform_device.h>
 
 const struct of_device_id of_default_bus_match_table[] = {

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

@@ -227,6 +230,7 @@ static struct platform_device *of_platform_device_create_pdata(
 
 	if (of_device_add(dev) != 0) {
 		platform_device_put(dev);
+		of_reserved_mem_device_release(&dev->dev);
 		return NULL;
 	}
 

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

@@ -308,6 +314,7 @@ static struct amba_device *of_amba_device_create(struct device_node *node,
 	return dev;
 
 err_free:
+	of_reserved_mem_device_release(&dev->dev);
 	amba_device_put(dev);
 	return NULL;
 }

Signature · 1 line

1.7.9.5

Quoted 5 lines · Marek Szyprowski

Use recently introduced of_reserved_mem_device_init() function to automatically assign respective reserved memory region to the newly created platform and amba device.

Signed-off-by Marek Szyprowski <[email protected]>

I'm wary on this patch. It hides the assignment of regions into the core code and I worry that it is the wrong level of abstraction. I would think that drivers should know that they need a reserved memory region and should be calling the API to obtain the region directly rather than doing it for them. The reason being is that there may be some situations where the common code isn't quite right and the driver needs to override the behaviour. If it is called automatically then the driver cannot do that.

Is it really a big burden to have the driver call the reserved memory init function?

Hello,

Quoted 16 lines · Grant Likely

Quoted 5 lines · Marek Szyprowski

Use recently introduced of_reserved_mem_device_init() function to automatically assign respective reserved memory region to the newly created platform and amba device.

Signed-off-by Marek Szyprowski <[email protected]>

I'm wary on this patch. It hides the assignment of regions into the core code and I worry that it is the wrong level of abstraction. I would think that drivers should know that they need a reserved memory region and should be calling the API to obtain the region directly rather than doing it for them. The reason being is that there may be some situations where the common code isn't quite right and the driver needs to override the behaviour. If it is called automatically then the driver cannot do that.

Quoted 2 lines

Is it really a big burden to have the driver call the reserved memory init function?

If a device requires very special handling of the reserved memory region, it may simply provide its own reserved memory region driver which will do the required early initialization.

If we assume that driver needs to initialize reserved region manually, then why do we ever bother with adding support for custom reserved memory drivers and assigning regions to a device node? We can simply stick with just a set of reserved regions and tell drivers to use them.

In my opinion for most typical use cases a board designer will assign 'dma-shared-pool' driver to the given set of devices, which in turn ensures that all memory allocations for dma purposes for those device will be served from that region. It is really not a driver role to initialize it in such case. Driver should focus on controlling hw operations, regardless the way the hardware module has been integrated to the system.

I can perfectly imagine a generic driver which operates the same way in any of the following cases (depends mainly on the hw version): 1) restricted dma window, 2) iommu for all dma for the given device, 3) fully featured memory master for dma for the given device (no restrictions), if the respective kernel subsystems did the correct initialization and provide their own methods for managing DMA operation. I already have a working platform glue code for the above cases tested with Samsung multimedia drivers. No changes to the drivers were required.

Enable reserved memory initialization from device tree.

Signed-off-by Marek Szyprowski <[email protected]>

 arch/arm/Kconfig   |    1 +
 arch/arm/mm/init.c |    2 ++
 2 files changed, 3 insertions(+)

arch/arm/Kconfig +1 -0

index e25419817791..d0262bea8020 100644
@@ -1918,6 +1918,7 @@ config USE_OF
        select IRQ_DOMAIN
        select OF
        select OF_EARLY_FLATTREE
+       select OF_RESERVED_MEM
        help
          Include support for flattened device tree machine descriptions.
 

arch/arm/mm/init.c +2 -1

index 804d61566a53..2a77ba8796ae 100644
@@ -323,6 +323,8 @@ void __init arm_memblock_init(struct meminfo *mi,
        if (mdesc->reserve)
                mdesc->reserve();
 
+       early_init_fdt_scan_reserved_mem();
+
        /*
         * reserve memory for DMA contigouos allocations,
         * must come from DMA area inside low memory
--

1.7.9.5

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

Quoted 3 lines · Marek Szyprowski

Enable reserved memory initialization from device tree.

Signed-off-by Marek Szyprowski <[email protected]>

This patch has hit -next and several legacy (non-DT) boot failures were detected and bisected down to this patch. A quick scan looks like there needs to be some sanity checking whether a DT is even present.

Quoted 10 lines · Kevin Hilman

Quoted 3 lines · Marek Szyprowski

Enable reserved memory initialization from device tree.

Signed-off-by Marek Szyprowski <[email protected]>

This patch has hit -next and several legacy (non-DT) boot failures were detected and bisected down to this patch. A quick scan looks like there needs to be some sanity checking whether a DT is even present.

Hmm. Yes, the code unconditionally calls of_flat_dt_scan(), which will gladly touch initial_boot_params, even though it may be uninitialized. The below patch should allow these boards to boot...

However, I'm wondering if there is a good reason why we don't parse the /reserved-memory nodes at the right after we parse the /memory nodes as part of early_init_dt_scan()...

Thanks,
Josh

--8<--
Subject: [PATCH] drivers: of: only scan for reserved mem when fdt present

Reported-by Kevin Hilman <[email protected]>

Signed-off-by Josh Cartwright <[email protected]>

---
 drivers/of/fdt.c | 3 +++
 1 file changed, 3 insertions(+)

drivers/of/fdt.c +3 -0

index 510c0d8..501bc83 100644
@@ -557,6 +557,9 @@ static int __init __fdt_scan_reserved_mem(unsigned long node, const char *uname,
  */
 void __init early_init_fdt_scan_reserved_mem(void)
 {
+	if (!initial_boot_params)
+		return;
+
 	of_scan_flat_dt(__fdt_scan_reserved_mem, NULL);
 	fdt_init_reserved_mem();
 }

Quoted 26 lines · Josh Cartwright

Quoted 10 lines · Kevin Hilman

Quoted 3 lines · Marek Szyprowski

Enable reserved memory initialization from device tree.

Signed-off-by Marek Szyprowski <[email protected]>

This patch has hit -next and several legacy (non-DT) boot failures were detected and bisected down to this patch. A quick scan looks like there needs to be some sanity checking whether a DT is even present.

Hmm. Yes, the code unconditionally calls of_flat_dt_scan(), which will gladly touch initial_boot_params, even though it may be uninitialized. The below patch should allow these boards to boot...

However, I'm wondering if there is a good reason why we don't parse the /reserved-memory nodes at the right after we parse the /memory nodes as part of early_init_dt_scan()...

Thanks,
Josh

--8<--
Subject: [PATCH] drivers: of: only scan for reserved mem when fdt present

Quoted 2 lines

Reported-by Kevin Hilman <[email protected]>

Signed-off-by Josh Cartwright <[email protected]>

This gets legacy boot working again. Thanks.

Tested-by Kevin Hilman <[email protected]>