diff mbox

[v2,5/5] of: document bindings for reserved-memory nodes

Message ID 1391515773-6112-6-git-send-email-m.szyprowski@samsung.com (mailing list archive)
State New, archived
Headers show

Commit Message

Marek Szyprowski Feb. 4, 2014, 12:09 p.m. UTC
From: Grant Likely <grant.likely@linaro.org>

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

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

Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Laura Abbott <lauraa@codeaurora.org>
Signed-off-by: Josh Cartwright <joshc@codeaurora.org>
Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
 .../bindings/reserved-memory/reserved-memory.txt   |  138 ++++++++++++++++++++
 1 file changed, 138 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt

Comments

Grant Likely Feb. 5, 2014, 10:07 a.m. UTC | #1
On Tue, 04 Feb 2014 13:09:33 +0100, Marek Szyprowski <m.szyprowski@samsung.com> wrote:
> From: Grant Likely <grant.likely@linaro.org>
> 
> Reserved memory nodes allow for the reservation of static (fixed
> address) regions, or dynamically allocated regions for a specific
> purpose.
> 
> [joshc: Based on binding document proposed (in non-patch form) here:
>  http://lkml.kernel.org/g/20131030134702.19B57C402A0@trevor.secretlab.ca
>  adapted to support #memory-region-cells]
> 
> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Cc: Laura Abbott <lauraa@codeaurora.org>
> Signed-off-by: Josh Cartwright <joshc@codeaurora.org>
> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
> ---
>  .../bindings/reserved-memory/reserved-memory.txt   |  138 ++++++++++++++++++++
>  1 file changed, 138 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt
> 
> diff --git a/Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt b/Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt
> new file mode 100644
> index 000000000000..a606ce90c9c4
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt
> @@ -0,0 +1,138 @@
> +*** Reserved memory regions ***
> +
> +Reserved memory is specified as a node under the /reserved-memory node.
> +The operating system shall exclude reserved memory from normal usage
> +one can create child nodes describing particular reserved (excluded from
> +normal use) memory regions. Such memory regions are usually designed for
> +the special usage by various device drivers.
> +
> +Parameters for each memory region can be encoded into the device tree
> +with the following nodes:
> +
> +/reserved-memory node
> +---------------------
> +#address-cells, #size-cells (required) - standard definition
> +    - Should use the same values as the root node
> +#memory-region-cells (required) - dictates number of cells used in the child
> +                                  nodes memory-region specifier

I don't think this isn't defined well enough. These reserved regions may
not have a driver attached to them, so there is no central agent to
decide what the specifier means. That leaves the interpretation of
the memory region in the hands of the client drivers. How do you see the
specifier getting parsed and used?

Otherwise the binding looks good to me. Also, please add my s-o-b line
to the patch (instead of a-b because I authored part of the text)

Signed-off-by: Grant Likely <grant.likely@linaro.org>

g.

> +ranges (required) - standard definition
> +    - Should be empty
> +
> +/reserved-memory/ child nodes
> +-----------------------------
> +Each child of the reserved-memory node specifies one or more regions of
> +reserved memory. Each child node may either use a 'reg' property to
> +specify a specific range of reserved memory, or a 'size' property with
> +optional constraints to request a dynamically allocated block of memory.
> +
> +Following the generic-names recommended practice, node names should
> +reflect the purpose of the node (ie. "framebuffer" or "dma-pool"). Unit
> +address (@<address>) should be appended to the name if the node is a
> +static allocation.
> +
> +Properties:
> +Requires either a) or b) below.
> +a) static allocation
> +   reg (required) - standard definition
> +b) dynamic allocation
> +   size (required) - length based on parent's #size-cells
> +                   - Size in bytes of memory to reserve.
> +   alignment (optional) - length based on parent's #size-cells
> +                        - Address boundary for alignment of allocation.
> +   alloc-ranges (optional) - prop-encoded-array (address, length pairs).
> +                           - Specifies regions of memory that are
> +                             acceptable to allocate from.
> +
> +If both reg and size are present, then the reg property takes precedence
> +and size is ignored.
> +
> +Additional properties:
> +compatible (optional) - standard definition
> +    - may contain the following strings:
> +        - shared-dma-pool: This indicates a region of memory meant to be
> +          used as a shared pool of DMA buffers for a set of devices. It can
> +          be used by an operating system to instanciate the necessary pool
> +          management subsystem if necessary.
> +        - vendor specific string in the form <vendor>,[<device>-]<usage>
> +no-map (optional) - empty property
> +    - Indicates the operating system must not create a virtual mapping
> +      of the region as part of its standard mapping of system memory,
> +      nor permit speculative access to it under any circumstances other
> +      than under the control of the device driver using the region.
> +reusable (optional) - empty property
> +    - The operating system can use the memory in this region with the
> +      limitation that the device driver(s) owning the region need to be
> +      able to reclaim it back. Typically that means that the operating
> +      system can use that region to store volatile or cached data that
> +      can be otherwise regenerated or migrated elsewhere.
> +
> +Linux implementation note:
> +- If a "linux,cma-default" property is present, then Linux will use the
> +  region for the default pool of the contiguous memory allocator.
> +
> +Device node references to reserved memory
> +-----------------------------------------
> +Regions in the /reserved-memory node may be referenced by other device
> +nodes by adding a memory-region property to the device node.
> +
> +memory-region (optional) - phandle, specifier pairs to children of /reserved-memory
> +
> +Example
> +-------
> +This example defines 3 contiguous regions are defined for Linux kernel:
> +one default of all device drivers (named linux,cma@72000000 and 64MiB in size),
> +one dedicated to the framebuffer device (named framebuffer@78000000, 8MiB), and
> +one for multimedia processing (named multimedia-memory@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@78000000 {
> +			#memory-region-cells = <0>;
> +			reg = <0x78000000 0x800000>;
> +		};
> +
> +		multimedia_reserved: multimedia@77000000 {
> +			compatible = "acme,multimedia-memory";
> +			#memory-region-cells = <1>;
> +			reg = <0x77000000 0x4000000>;
> +		};
> +	};
> +
> +	/* ... */
> +
> +	fb0: video@12300000 {
> +		memory-region = <&display_reserved>;
> +		/* ... */
> +	};
> +
> +	scaler: scaler@12500000 {
> +		memory-region = <&multimedia_reserved 0xdeadbeef>;
> +		/* ... */
> +	};
> +
> +	codec: codec@12600000 {
> +		memory-region = <&multimedia_reserved 0xfeebdaed>;
> +		/* ... */
> +	};
> +};
> -- 
> 1.7.9.5
>
Josh Cartwright Feb. 5, 2014, 7:25 p.m. UTC | #2
On Wed, Feb 05, 2014 at 10:07:50AM +0000, Grant Likely wrote:
> On Tue, 04 Feb 2014 13:09:33 +0100, Marek Szyprowski <m.szyprowski@samsung.com> wrote:
> > From: Grant Likely <grant.likely@linaro.org>
> > +/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 don't think this isn't defined well enough. These reserved regions may
> not have a driver attached to them, so there is no central agent to
> decide what the specifier means. That leaves the interpretation of
> the memory region in the hands of the client drivers. How do you see the
> specifier getting parsed and used?

I had a specific usecase in mind when I added it, although admittedly I
haven't yet worked through all of the details.

On MSM chips, there is a region of memory accessible from the various
processors on the SoC.  This region is used for (among other things)
inter-processor communication.  Inside this region, a heap allocation
protocol is implemented by software on all interested processors.

Consumers of this shared memory heap specify a 32-bit identifier and a
size, and are either given a matching preexisting chunk (for example,
another processor has already allocated a chunk with the corresponding
identifier), or are allocated a new chunk for that identifier out of the
region.

Given it's shared nature, this region has some specific requirements
about how it may be accessed by the kernel (specifically regarding
cacheability/how it's mapped), which means it at least needs _some_
representation in a reserved-memory node.

I had envisioned expressing the shared memory/consumer relationship in
the device tree:

	reserved-memory {
		smem : smem {
			compatible = "qcom,shared-memory";
			reg = <...>;
			#memory-region-cells = <2>;
			no-map;
		};
	};

	consumer {
		/* ... */;
		memory-region = <&smem 0xDEADBEEF 0x1000>;
	};

That is, the heap protocol implementation exists as a "driver" for the
smem reserved-memory node, and the consumer's 2-cell specifier is a
32-bit identifier and a size.

If your concern is for the case where a "qcom,shared-memory" node is
specified in a device tree, but the "driver" hasn't been built into the
kernel, then the appropriate behavior would be the same as the DMA/CMA
case: fallback to a default case of memblock_reserve/memblock_remove'ing
the region.

Would using reserved-memory in this way be outside the scope of what was
originally intended?

Thanks,
  Josh
diff mbox

Patch

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