linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [RESEND][PATCHv2 0/3] Devicetree bindings for Ion
@ 2016-01-11 21:39 Laura Abbott
  2016-01-11 21:39 ` [RESEND][PATCHv2 1/3] ion: " Laura Abbott
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Laura Abbott @ 2016-01-11 21:39 UTC (permalink / raw)
  To: Rob Herring, Frank Rowand, Sumit Semwal, Andrew Andrianov, arve,
	Riley Andrews
  Cc: Laura Abbott, John Stultz, Grant Likely, devicetree,
	linux-kernel, Tom Gall, Colin Cross, devel, Greg Kroah-Hartman,
	romlem, mitchelh, linux-arm-kernel, Feng Tang, Marek Szyprowski,
	Mark Rutland

Hi,

There was some recent discussion
(http://thread.gmane.org/gmane.linux.ports.arm.kernel/468051/)
about Ion DT bindings and a request that I resend my proposal for common DT
bindings. This is a straight resend, no bux fixed that were pointed out.
Previous thread: http://article.gmane.org/gmane.linux.drivers.driver-project.devel/80475

>From before:

This is another attempt at devicetree bindings for Ion. The big complaint from
v1 was that too much unnecessary data was being pushed into devicetree.
v2 takes a different approach of using just compatbile strings for the heaps.
This gets closer to the devicetree philosophy of describing just the hardware.
Each heap ultimately represents some kind of memory that exists in the system.
The compatible string indicates the match for how to handle the type of memory
on this system. The details like heap-id, name etc. are now pushed out to C
files. This makes Ion heaps look closer to something like a quirks framework.
(I'd argue this isn't a complete mischaracterization given the type of setup
Ion gets used for...) The one downside here is that this leads to more new
bindings for each board that gets added.

This version also includes a sample C file which shows what the structure
might look like. As always, your comments and reviews are appreciated.


Thanks,
Laura

-- 
2.5.0

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [RESEND][PATCHv2 1/3] ion: Devicetree bindings for Ion
  2016-01-11 21:39 [RESEND][PATCHv2 0/3] Devicetree bindings for Ion Laura Abbott
@ 2016-01-11 21:39 ` Laura Abbott
  2016-01-11 23:28   ` Rob Herring
  2016-01-11 21:39 ` [RESEND][PATCHv2 2/3] staging: ion: Add files for parsing the devicetree Laura Abbott
  2016-01-11 21:39 ` [RESEND][PATCHv2 3/3] NOMERGE: Sample driver Laura Abbott
  2 siblings, 1 reply; 7+ messages in thread
From: Laura Abbott @ 2016-01-11 21:39 UTC (permalink / raw)
  To: Rob Herring, Frank Rowand, Sumit Semwal, Andrew Andrianov, arve,
	Riley Andrews
  Cc: Laura Abbott, John Stultz, Grant Likely, devicetree,
	linux-kernel, Tom Gall, Colin Cross, devel, Greg Kroah-Hartman,
	romlem, mitchelh, linux-arm-kernel, Feng Tang, Marek Szyprowski,
	Mark Rutland


This adds a base set of devicetree bindings for the Ion memory
manager. This supports setting up the generic set of heaps and
their properties.

Signed-off-by: Laura Abbott <laura@labbott.name>
---
 drivers/staging/android/ion/devicetree.txt | 50 ++++++++++++++++++++++++++++++
 1 file changed, 50 insertions(+)
 create mode 100644 drivers/staging/android/ion/devicetree.txt

diff --git a/drivers/staging/android/ion/devicetree.txt b/drivers/staging/android/ion/devicetree.txt
new file mode 100644
index 0000000..e1ea537
--- /dev/null
+++ b/drivers/staging/android/ion/devicetree.txt
@@ -0,0 +1,50 @@
+Ion Memory Manager
+
+Ion is a memory manager that allows for sharing of buffers via dma-buf.
+Ion allows for different types of allocation via an abstraction called
+a 'heap'. A heap represents a specific type of memory. Each heap has
+a different type. There can be multiple instances of the same heap
+type.
+
+Required properties for Ion
+
+- compatible: "linux,ion" PLUS a compatible property for the device
+
+All child nodes of a linux,ion node are interpreted as heaps
+
+required properties for heaps
+
+- compatible: compatible string for a heap type PLUS a compatible property
+for the specific instance of the heap. Current heap types
+-- linux,ion-heap-system
+-- linux,ion-heap-system-contig
+-- linux,ion-heap-carveout
+-- linux,ion-heap-chunk
+-- linux,ion-heap-dma
+-- linux,ion-heap-custom
+
+Optional properties
+- memory-region: A phandle to a memory region. Required for DMA heap type
+(see reserved-memory.txt for details on the reservation)
+
+Example:
+
+	ion {
+		compatbile = "qcom,msm8916-ion", "linux,ion";
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		ion-system-heap {
+			compatbile = "qcom,system-heap", "linux,ion-heap-system"
+		};
+
+		ion-camera-region {
+			compatible = "qcom,camera-heap", "linux,ion-heap-dma"
+			memory-region = <&camera_region>;
+		};
+
+		ion-fb-region {
+			compatbile = "qcom,fb-heap", "linux,ion-heap-dma"
+			memory-region = <&fb_region>;
+		};
+	}
-- 
2.5.0

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [RESEND][PATCHv2 2/3] staging: ion: Add files for parsing the devicetree
  2016-01-11 21:39 [RESEND][PATCHv2 0/3] Devicetree bindings for Ion Laura Abbott
  2016-01-11 21:39 ` [RESEND][PATCHv2 1/3] ion: " Laura Abbott
@ 2016-01-11 21:39 ` Laura Abbott
  2016-01-11 23:24   ` kbuild test robot
  2016-01-11 21:39 ` [RESEND][PATCHv2 3/3] NOMERGE: Sample driver Laura Abbott
  2 siblings, 1 reply; 7+ messages in thread
From: Laura Abbott @ 2016-01-11 21:39 UTC (permalink / raw)
  To: Rob Herring, Frank Rowand, Sumit Semwal, Andrew Andrianov, arve,
	Riley Andrews
  Cc: Laura Abbott, John Stultz, Grant Likely, devicetree,
	linux-kernel, Tom Gall, Colin Cross, devel, Greg Kroah-Hartman,
	romlem, mitchelh, linux-arm-kernel, Feng Tang, Marek Szyprowski,
	Mark Rutland



Devicetree is the preferred mechanism for platform definition
these days. Add a set of files for supporting Ion with devicetree.
This is a set of functions for drivers to call to parse the Ion
devicetree along with definitons for defining heaps.

Signed-off-by: Laura Abbott <laura@labbott.name>
---
 drivers/staging/android/ion/Kconfig  |  10 +++
 drivers/staging/android/ion/Makefile |   1 +
 drivers/staging/android/ion/ion_of.c | 168 +++++++++++++++++++++++++++++++++++
 drivers/staging/android/ion/ion_of.h |  35 ++++++++
 4 files changed, 214 insertions(+)
 create mode 100644 drivers/staging/android/ion/ion_of.c
 create mode 100644 drivers/staging/android/ion/ion_of.h

diff --git a/drivers/staging/android/ion/Kconfig b/drivers/staging/android/ion/Kconfig
index 3452346..9b6d65d 100644
--- a/drivers/staging/android/ion/Kconfig
+++ b/drivers/staging/android/ion/Kconfig
@@ -33,3 +33,13 @@ config ION_TEGRA
 	help
 	  Choose this option if you wish to use ion on an nVidia Tegra.
 
+config ION_OF
+	bool "Devicetree support for Ion"
+	depends on ION && OF
+	help
+	  Provides base support for defining Ion heaps in devicetree
+	  and setting them up. Also includes functions for platforms
+	  to parse the devicetree and expand for their own custom
+	  extensions
+
+	  If using Ion and devicetree, you should say Y here
diff --git a/drivers/staging/android/ion/Makefile b/drivers/staging/android/ion/Makefile
index b56fd2b..a77417b 100644
--- a/drivers/staging/android/ion/Makefile
+++ b/drivers/staging/android/ion/Makefile
@@ -7,4 +7,5 @@ endif
 
 obj-$(CONFIG_ION_DUMMY) += ion_dummy_driver.o
 obj-$(CONFIG_ION_TEGRA) += tegra/
+obj-$(CONFIG_ION_OF) += ion_of.o ion_physmem.o
 
diff --git a/drivers/staging/android/ion/ion_of.c b/drivers/staging/android/ion/ion_of.c
new file mode 100644
index 0000000..9a56194
--- /dev/null
+++ b/drivers/staging/android/ion/ion_of.c
@@ -0,0 +1,168 @@
+/*
+ * Based on work from:
+ *   Andrew Andrianov <andrew@ncrmnt.org>
+ *   Google
+ *   The Linux Foundation
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/init.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
+#include <linux/of.h>
+#include <linux/of_platform.h>
+#include <linux/of_address.h>
+#include <linux/clk.h>
+#include <linux/dma-mapping.h>
+#include <linux/cma.h>
+#include <linux/dma-contiguous.h>
+#include <linux/io.h>
+#include <linux/of_reserved_mem.h>
+#include "ion.h"
+#include "ion_priv.h"
+#include "ion_of.h"
+
+int ion_parse_dt_heap_common(struct device_node *heap_node,
+			struct ion_platform_heap *heap,
+			struct ion_of_heap *compatible)
+{
+	int i;
+
+	for (i = 0; compatible[i].name != NULL; i++) {
+		if (of_device_is_compatible(heap_node, compatible[i].compat))
+			break;
+	}
+
+	if (compatible[i].name == NULL)
+		return -ENODEV;
+
+	heap->id = compatible[i].heap_id;
+	heap->type = compatible[i].type;
+	heap->name = compatible[i].name;
+	heap->align = compatible[i].align;
+
+	/* Some kind of callback function pointer? */
+
+	pr_info("%s: id %d type %d name %s align %lx\n", __func__,
+			heap->id, heap->type, heap->name, heap->align);
+	return 0;
+}
+
+int ion_setup_heap_common(struct platform_device *parent,
+			struct device_node *heap_node,
+			struct ion_platform_heap *heap)
+{
+	int ret = 0;
+
+	switch (heap->type) {
+		case ION_HEAP_TYPE_CARVEOUT:
+		case ION_HEAP_TYPE_CHUNK:
+			if (heap->base && heap->size)
+				return 0;
+
+			ret = of_reserved_mem_device_init(heap->priv);
+			break;
+		default:
+			break;
+	}
+
+	return ret;
+}
+
+struct ion_platform_data *ion_parse_dt(struct platform_device *pdev,
+					struct ion_of_heap *compatible)
+{
+	int num_heaps, ret;
+	const struct device_node *dt_node = pdev->dev.of_node;
+	struct device_node *node;
+	struct ion_platform_heap *heaps;
+	struct ion_platform_data *data;
+	int i = 0;
+
+	num_heaps = of_get_available_child_count(dt_node);
+
+	if (!num_heaps)
+		return ERR_PTR(-EINVAL);
+
+	heaps = devm_kzalloc(&pdev->dev,
+				sizeof(struct ion_platform_heap)*num_heaps,
+				GFP_KERNEL);
+	if (!heaps)
+		return ERR_PTR(-ENOMEM);
+
+	data = devm_kzalloc(&pdev->dev, sizeof(struct ion_platform_data),
+				GFP_KERNEL);
+	if (!data)
+		return ERR_PTR(-ENOMEM);
+
+	for_each_available_child_of_node(dt_node, node) {
+		struct platform_device *heap_pdev;
+
+		ret = ion_parse_dt_heap_common(node, &heaps[i], compatible);
+		if (ret)
+			return ERR_PTR(ret);
+
+		heap_pdev = of_platform_device_create(node, heaps[i].name,
+							&pdev->dev);
+		if (!pdev)
+			return ERR_PTR(-ENOMEM);
+		heap_pdev->dev.platform_data = &heaps[i];
+
+		heaps[i].priv = &heap_pdev->dev;
+
+		ret = ion_setup_heap_common(pdev, node, &heaps[i]);
+		if (ret)
+			return ERR_PTR(ret);
+		i++;
+	}
+
+
+	data->heaps = heaps;
+	data->nr = num_heaps;
+	return data;
+}
+
+#ifdef CONFIG_OF_RESERVED_MEM
+#include <linux/of.h>
+#include <linux/of_fdt.h>
+#include <linux/of_reserved_mem.h>
+
+static int rmem_ion_device_init(struct reserved_mem *rmem, struct device *dev)
+{
+	struct platform_device *pdev = to_platform_device(dev);
+	struct ion_platform_heap *heap = pdev->dev.platform_data;
+
+	heap->base = rmem->base;
+	heap->base = rmem->size;
+	pr_debug("%s: heap %s base %pa size %pa dev %p\n", __func__,
+			heap->name, &rmem->base, &rmem->size, dev);
+	return 0;
+}
+
+static void rmem_ion_device_release(struct reserved_mem *rmem,
+					struct device *dev)
+{
+	return;
+}
+
+static const struct reserved_mem_ops rmem_dma_ops = {
+	.device_init	= rmem_ion_device_init,
+	.device_release	= rmem_ion_device_release,
+};
+
+static int __init rmem_ion_setup(struct reserved_mem *rmem)
+{
+	phys_addr_t size = rmem->size;
+
+	size = size / 1024;
+
+	pr_info("Ion memory setup at %pa size %pa MiB\n",
+		&rmem->base, &size);
+	rmem->ops = &rmem_dma_ops;
+	return 0;
+}
+RESERVEDMEM_OF_DECLARE(ion, "ion-region", rmem_ion_setup);
+#endif
diff --git a/drivers/staging/android/ion/ion_of.h b/drivers/staging/android/ion/ion_of.h
new file mode 100644
index 0000000..6b767e7
--- /dev/null
+++ b/drivers/staging/android/ion/ion_of.h
@@ -0,0 +1,35 @@
+/*
+ * Based on work from:
+ *   Andrew Andrianov <andrew@ncrmnt.org>
+ *   Google
+ *   The Linux Foundation
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#ifndef _ION_OF_H
+#define _ION_OF_H
+
+struct ion_of_heap {
+	const char *compat;
+	int heap_id;
+	int type;
+	const char *name;
+	int align;
+};
+
+#define PLATFORM_HEAP(_compat, _id, _type, _name) \
+{ \
+	.compat = _compat, \
+	.heap_id = _id, \
+	.type = _type, \
+	.name = _name, \
+	.align = PAGE_SIZE, \
+}
+
+struct ion_platform_data *ion_parse_dt(struct platform_device *pdev,
+					struct ion_of_heap *compatible);
+
+#endif
-- 
2.5.0

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [RESEND][PATCHv2 3/3] NOMERGE: Sample driver
  2016-01-11 21:39 [RESEND][PATCHv2 0/3] Devicetree bindings for Ion Laura Abbott
  2016-01-11 21:39 ` [RESEND][PATCHv2 1/3] ion: " Laura Abbott
  2016-01-11 21:39 ` [RESEND][PATCHv2 2/3] staging: ion: Add files for parsing the devicetree Laura Abbott
@ 2016-01-11 21:39 ` Laura Abbott
  2 siblings, 0 replies; 7+ messages in thread
From: Laura Abbott @ 2016-01-11 21:39 UTC (permalink / raw)
  To: Rob Herring, Frank Rowand, Sumit Semwal, Andrew Andrianov, arve,
	Riley Andrews
  Cc: Laura Abbott, John Stultz, Grant Likely, devicetree,
	linux-kernel, Tom Gall, Colin Cross, devel, Greg Kroah-Hartman,
	romlem, mitchelh, linux-arm-kernel, Feng Tang, Marek Szyprowski,
	Mark Rutland



Small sample driver to show what setup would look like with the dt
bindings

Signed-off-by: Laura Abbott <laura@labbott.name>
---
 drivers/staging/android/ion/Kconfig         |  6 ++
 drivers/staging/android/ion/Makefile        |  1 +
 drivers/staging/android/ion/ion_of_sample.c | 96 +++++++++++++++++++++++++++++
 3 files changed, 103 insertions(+)
 create mode 100644 drivers/staging/android/ion/ion_of_sample.c

diff --git a/drivers/staging/android/ion/Kconfig b/drivers/staging/android/ion/Kconfig
index 9b6d65d..c2afb35 100644
--- a/drivers/staging/android/ion/Kconfig
+++ b/drivers/staging/android/ion/Kconfig
@@ -43,3 +43,9 @@ config ION_OF
 	  extensions
 
 	  If using Ion and devicetree, you should say Y here
+
+config ION_OF_SAMPLE
+	bool "Sample driver"
+	depends on ION_OF
+	help
+	  Small sample driver showing the Ion OF interface
diff --git a/drivers/staging/android/ion/Makefile b/drivers/staging/android/ion/Makefile
index a77417b..1309b91 100644
--- a/drivers/staging/android/ion/Makefile
+++ b/drivers/staging/android/ion/Makefile
@@ -8,4 +8,5 @@ endif
 obj-$(CONFIG_ION_DUMMY) += ion_dummy_driver.o
 obj-$(CONFIG_ION_TEGRA) += tegra/
 obj-$(CONFIG_ION_OF) += ion_of.o ion_physmem.o
+obj-$(CONFIG_ION_OF_SAMPLE) += ion_of_sample.o
 
diff --git a/drivers/staging/android/ion/ion_of_sample.c b/drivers/staging/android/ion/ion_of_sample.c
new file mode 100644
index 0000000..bbcdf4d
--- /dev/null
+++ b/drivers/staging/android/ion/ion_of_sample.c
@@ -0,0 +1,96 @@
+/*
+ * Copyright (C) 2015 RC Module
+ * Andrew Andrianov <andrew@ncrmnt.org>
+ * Also based on work from Google, The Linux Foundation
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ */
+
+#include <linux/init.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
+#include <linux/of.h>
+#include <linux/of_platform.h>
+#include <linux/of_address.h>
+#include <linux/clk.h>
+#include <linux/dma-mapping.h>
+#include <linux/io.h>
+#include "ion.h"
+#include "ion_priv.h"
+#include "ion_of.h"
+
+struct sample_ion_dev {
+        struct ion_heap         **heaps;
+        struct ion_device       *idev;
+};
+
+static struct ion_of_heap heaps[] = {
+	PLATFORM_HEAP("sample-system", 0, ION_HEAP_TYPE_SYSTEM, "system"),
+	PLATFORM_HEAP("sample-camera", 1, ION_HEAP_TYPE_DMA, "camera"),
+	PLATFORM_HEAP("sample-fb", 2, ION_HEAP_TYPE_DMA, "fb"),
+	{}
+};
+
+static int ion_sample_probe(struct platform_device *pdev)
+{
+	struct ion_platform_data *data;
+	struct sample_ion_dev *ipdev;
+	int i;
+
+	ipdev = devm_kzalloc(&pdev->dev, sizeof(*ipdev), GFP_KERNEL);
+	if (!ipdev)
+		return -ENOMEM;
+
+	platform_set_drvdata(pdev, ipdev);
+
+	ipdev->idev = ion_device_create(NULL);
+	if (!ipdev->idev)
+		return -ENOMEM;
+
+	data = ion_parse_dt(pdev, heaps);
+	if (IS_ERR(data))
+		return PTR_ERR(data);
+
+	ipdev->heaps = devm_kzalloc(&pdev->dev,
+				sizeof(struct ion_heap)*data->nr, GFP_KERNEL);
+
+	if (!ipdev->heaps)
+		return -ENOMEM;
+
+	for (i = 0; i < data->nr; i++) {
+		ipdev->heaps[i] = ion_heap_create(&data->heaps[i]);
+		if (!ipdev->heaps[i])
+			return -ENOMEM;
+		ion_device_add_heap(ipdev->idev, ipdev->heaps[i]);
+	}
+	return 0;
+}
+
+static int ion_sample_remove(struct platform_device *pdev)
+{
+	/* Everything should be devm */
+	return 0;
+}
+
+static const struct of_device_id of_match_table[] = {
+	{ .compatible = "sample-ion", },
+	{ /* end of list */ }
+};
+
+static struct platform_driver ion_sample_driver = {
+	.probe		= ion_sample_probe,
+	.remove		= ion_sample_remove,
+	.driver		= {
+		.name	= "ion-of",
+		.of_match_table = of_match_ptr(of_match_table),
+	},
+};
+
+static int __init ion_sample_init(void)
+{
+	return platform_driver_register(&ion_sample_driver);
+}
+device_initcall(ion_sample_init);
-- 
2.5.0

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [RESEND][PATCHv2 2/3] staging: ion: Add files for parsing the devicetree
  2016-01-11 21:39 ` [RESEND][PATCHv2 2/3] staging: ion: Add files for parsing the devicetree Laura Abbott
@ 2016-01-11 23:24   ` kbuild test robot
  0 siblings, 0 replies; 7+ messages in thread
From: kbuild test robot @ 2016-01-11 23:24 UTC (permalink / raw)
  To: Laura Abbott
  Cc: kbuild-all, Rob Herring, Frank Rowand, Sumit Semwal,
	Andrew Andrianov, arve, Riley Andrews, Laura Abbott, John Stultz,
	Grant Likely, devicetree, linux-kernel, Tom Gall, Colin Cross,
	devel, Greg Kroah-Hartman, romlem, mitchelh, linux-arm-kernel,
	Feng Tang, Marek Szyprowski, Mark Rutland

[-- Attachment #1: Type: text/plain, Size: 850 bytes --]

Hi Laura,

[auto build test ERROR on v4.4-rc8]
[cannot apply to staging/staging-testing next-20160111]
[if your patch is applied to the wrong git tree, please drop us a note to help improving the system]

url:    https://github.com/0day-ci/linux/commits/Laura-Abbott/Devicetree-bindings-for-Ion/20160112-054709
config: x86_64-allmodconfig (attached as .config)
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All errors (new ones prefixed by >>):

>> make[5]: *** No rule to make target 'drivers/staging/android/ion/ion_physmem.o', needed by 'drivers/staging/android/ion/built-in.o'.
   make[5]: Target '__build' not remade because of errors.

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/octet-stream, Size: 51055 bytes --]

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [RESEND][PATCHv2 1/3] ion: Devicetree bindings for Ion
  2016-01-11 21:39 ` [RESEND][PATCHv2 1/3] ion: " Laura Abbott
@ 2016-01-11 23:28   ` Rob Herring
  2016-01-12  0:26     ` Laura Abbott
  0 siblings, 1 reply; 7+ messages in thread
From: Rob Herring @ 2016-01-11 23:28 UTC (permalink / raw)
  To: Laura Abbott
  Cc: Frank Rowand, Sumit Semwal, Andrew Andrianov, arve,
	Riley Andrews, John Stultz, Grant Likely, devicetree,
	linux-kernel, Tom Gall, Colin Cross, devel, Greg Kroah-Hartman,
	Rom Lemarchand, Mitchel Humpherys, linux-arm-kernel, Feng Tang,
	Marek Szyprowski, Mark Rutland

On Mon, Jan 11, 2016 at 3:39 PM, Laura Abbott <laura@labbott.name> wrote:
>
> This adds a base set of devicetree bindings for the Ion memory
> manager. This supports setting up the generic set of heaps and
> their properties.
>
> Signed-off-by: Laura Abbott <laura@labbott.name>
> ---
>  drivers/staging/android/ion/devicetree.txt | 50 ++++++++++++++++++++++++++++++
>  1 file changed, 50 insertions(+)
>  create mode 100644 drivers/staging/android/ion/devicetree.txt
>
> diff --git a/drivers/staging/android/ion/devicetree.txt b/drivers/staging/android/ion/devicetree.txt
> new file mode 100644
> index 0000000..e1ea537
> --- /dev/null
> +++ b/drivers/staging/android/ion/devicetree.txt
> @@ -0,0 +1,50 @@
> +Ion Memory Manager
> +
> +Ion is a memory manager that allows for sharing of buffers via dma-buf.
> +Ion allows for different types of allocation via an abstraction called
> +a 'heap'. A heap represents a specific type of memory. Each heap has
> +a different type. There can be multiple instances of the same heap
> +type.
> +
> +Required properties for Ion
> +
> +- compatible: "linux,ion" PLUS a compatible property for the device
> +
> +All child nodes of a linux,ion node are interpreted as heaps
> +
> +required properties for heaps
> +
> +- compatible: compatible string for a heap type PLUS a compatible property
> +for the specific instance of the heap. Current heap types
> +-- linux,ion-heap-system
> +-- linux,ion-heap-system-contig
> +-- linux,ion-heap-carveout
> +-- linux,ion-heap-chunk
> +-- linux,ion-heap-dma
> +-- linux,ion-heap-custom
> +
> +Optional properties
> +- memory-region: A phandle to a memory region. Required for DMA heap type
> +(see reserved-memory.txt for details on the reservation)

Why would all of them not require a memory region other than system heap?

> +
> +Example:
> +
> +       ion {
> +               compatbile = "qcom,msm8916-ion", "linux,ion";
> +               #address-cells = <1>;
> +               #size-cells = <0>;
> +
> +               ion-system-heap {
> +                       compatbile = "qcom,system-heap", "linux,ion-heap-system"
> +               };

What is the purpose of this in DT? Don't we always want/need a system
heap? How does it vary by platform?

> +               ion-camera-region {
> +                       compatible = "qcom,camera-heap", "linux,ion-heap-dma"
> +                       memory-region = <&camera_region>;

Why not just add a property (or properties) to the camera_region node
that ION drivers can search for?

Rob

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [RESEND][PATCHv2 1/3] ion: Devicetree bindings for Ion
  2016-01-11 23:28   ` Rob Herring
@ 2016-01-12  0:26     ` Laura Abbott
  0 siblings, 0 replies; 7+ messages in thread
From: Laura Abbott @ 2016-01-12  0:26 UTC (permalink / raw)
  To: Rob Herring
  Cc: Frank Rowand, Sumit Semwal, Andrew Andrianov, arve,
	Riley Andrews, John Stultz, Grant Likely, devicetree,
	linux-kernel, Tom Gall, Colin Cross, devel, Greg Kroah-Hartman,
	Rom Lemarchand, Mitchel Humpherys, linux-arm-kernel, Feng Tang,
	Marek Szyprowski, Mark Rutland

On 1/11/16 3:28 PM, Rob Herring wrote:
> On Mon, Jan 11, 2016 at 3:39 PM, Laura Abbott <laura@labbott.name> wrote:
>>
>> This adds a base set of devicetree bindings for the Ion memory
>> manager. This supports setting up the generic set of heaps and
>> their properties.
>>
>> Signed-off-by: Laura Abbott <laura@labbott.name>
>> ---
>>   drivers/staging/android/ion/devicetree.txt | 50 ++++++++++++++++++++++++++++++
>>   1 file changed, 50 insertions(+)
>>   create mode 100644 drivers/staging/android/ion/devicetree.txt
>>
>> diff --git a/drivers/staging/android/ion/devicetree.txt b/drivers/staging/android/ion/devicetree.txt
>> new file mode 100644
>> index 0000000..e1ea537
>> --- /dev/null
>> +++ b/drivers/staging/android/ion/devicetree.txt
>> @@ -0,0 +1,50 @@
>> +Ion Memory Manager
>> +
>> +Ion is a memory manager that allows for sharing of buffers via dma-buf.
>> +Ion allows for different types of allocation via an abstraction called
>> +a 'heap'. A heap represents a specific type of memory. Each heap has
>> +a different type. There can be multiple instances of the same heap
>> +type.
>> +
>> +Required properties for Ion
>> +
>> +- compatible: "linux,ion" PLUS a compatible property for the device
>> +
>> +All child nodes of a linux,ion node are interpreted as heaps
>> +
>> +required properties for heaps
>> +
>> +- compatible: compatible string for a heap type PLUS a compatible property
>> +for the specific instance of the heap. Current heap types
>> +-- linux,ion-heap-system
>> +-- linux,ion-heap-system-contig
>> +-- linux,ion-heap-carveout
>> +-- linux,ion-heap-chunk
>> +-- linux,ion-heap-dma
>> +-- linux,ion-heap-custom
>> +
>> +Optional properties
>> +- memory-region: A phandle to a memory region. Required for DMA heap type
>> +(see reserved-memory.txt for details on the reservation)
>
> Why would all of them not require a memory region other than system heap?
>
>> +
>> +Example:
>> +
>> +       ion {
>> +               compatbile = "qcom,msm8916-ion", "linux,ion";
>> +               #address-cells = <1>;
>> +               #size-cells = <0>;
>> +
>> +               ion-system-heap {
>> +                       compatbile = "qcom,system-heap", "linux,ion-heap-system"
>> +               };
>
> What is the purpose of this in DT? Don't we always want/need a system
> heap? How does it vary by platform?
>
>> +               ion-camera-region {
>> +                       compatible = "qcom,camera-heap", "linux,ion-heap-dma"
>> +                       memory-region = <&camera_region>;
>
> Why not just add a property (or properties) to the camera_region node
> that ION drivers can search for?
>
> Rob
>

Thinking about all of this put together with the general comments, would the
following be more acceptable:

- Keep the approach of this patch series with having the heaps specified
in an array in a file
- For each array of heaps, have a list of machine compatible strings that
work with the heaps.
- Call of_machine_is_compatible on all the heaps until one is found
- When setting up the heaps, check for a property like Rob suggested
to get the appropriate memory node
- Add appropriate documentation in the devicetree directory explaining
the entire approach

The only Ion data in the DT with this approach would be the property in the
appropriate memory node. There is no ABI except the memory property so as
Ion changes there would be no breakage. This approach does go a little
bit out outside of the traditional way devicetree works. A more
traditional approach would be to just have an ion node with a compatible
string and then just find the memory node via a property (although this
does taint the DT more with Ion even if the ABI may be relatively stable)

Thanks,
Laura

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2016-01-12  0:26 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-01-11 21:39 [RESEND][PATCHv2 0/3] Devicetree bindings for Ion Laura Abbott
2016-01-11 21:39 ` [RESEND][PATCHv2 1/3] ion: " Laura Abbott
2016-01-11 23:28   ` Rob Herring
2016-01-12  0:26     ` Laura Abbott
2016-01-11 21:39 ` [RESEND][PATCHv2 2/3] staging: ion: Add files for parsing the devicetree Laura Abbott
2016-01-11 23:24   ` kbuild test robot
2016-01-11 21:39 ` [RESEND][PATCHv2 3/3] NOMERGE: Sample driver Laura Abbott

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).