[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-id: <53105CC6.1090902@samsung.com>
Date: Fri, 28 Feb 2014 10:54:14 +0100
From: Marek Szyprowski <m.szyprowski@...sung.com>
To: Grant Likely <grant.likely@...aro.org>,
linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
linaro-mm-sig@...ts.linaro.org, devicetree@...r.kernel.org,
linux-doc@...r.kernel.org
Cc: Benjamin Herrenschmidt <benh@...nel.crashing.org>,
Arnd Bergmann <arnd@...db.de>,
Michal Nazarewicz <mina86@...a86.com>,
Tomasz Figa <t.figa@...sung.com>,
Sascha Hauer <s.hauer@...gutronix.de>,
Laura Abbott <lauraa@...eaurora.org>,
Rob Herring <robh+dt@...nel.org>,
Olof Johansson <olof@...om.net>,
Pawel Moll <pawel.moll@....com>,
Mark Rutland <mark.rutland@....com>,
Stephen Warren <swarren@...dotorg.org>,
Ian Campbell <ian.campbell@...rix.com>,
Tomasz Figa <tomasz.figa@...il.com>,
Kumar Gala <galak@...eaurora.org>,
Nishanth Peethambaran <nishanth.p@...il.com>,
Marc <marc.ceeeee@...il.com>,
Josh Cartwright <joshc@...eaurora.org>,
Catalin Marinas <catalin.marinas@....com>,
Will Deacon <will.deacon@....com>,
Paul Mackerras <paulus@...ba.org>
Subject: Re: [PATCH v5 01/11] of: document bindings for reserved-memory nodes
Hello,
On 2014-02-26 12:51, Grant Likely wrote:
> On Fri, 21 Feb 2014 13:25:17 +0100, Marek Szyprowski <m.szyprowski@...sung.com> wrote:
> > From: Grant Likely <grant.likely@...aro.org>
> >
> > 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 <grant.likely@...aro.org>
> > [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]
> > Signed-off-by: Josh Cartwright <joshc@...eaurora.org>
> > Signed-off-by: Marek Szyprowski <m.szyprowski@...sung.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 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?
>
> > +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>
>
> 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?
> > +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@...00000 and 64MiB in size),
> > +one dedicated to the framebuffer device (named framebuffer@...00000, 8MiB), and
> > +one for multimedia processing (named multimedia-memory@...00000, 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@...00000 {
> > + #memory-region-cells = <0>;
> > + reg = <0x78000000 0x800000>;
> > + };
> > +
> > + multimedia_reserved: multimedia@...00000 {
> > + compatible = "acme,multimedia-memory";
> > + #memory-region-cells = <1>;
> > + reg = <0x77000000 0x4000000>;
> > + };
> > + };
> > +
> > + /* ... */
> > +
> > + fb0: video@...00000 {
> > + memory-region = <&display_reserved>;
> > + /* ... */
> > + };
> > +
> > + scaler: scaler@...00000 {
> > + memory-region = <&multimedia_reserved 0xdeadbeef>;
> > + /* ... */
> > + };
> > +
> > + codec: codec@...00000 {
> > + memory-region = <&multimedia_reserved 0xfeebdaed>;
> > + /* ... */
> > + };
> > +};
> > --
> > 1.7.9.5
> >
>
>
Best regards
--
Marek Szyprowski, PhD
Samsung R&D Institute Poland
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists