All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/2] Device Tree support for CMA (Contiguous Memory Allocator)
@ 2013-02-14 12:45 ` Marek Szyprowski
  0 siblings, 0 replies; 36+ messages in thread
From: Marek Szyprowski @ 2013-02-14 12:45 UTC (permalink / raw)
  To: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linaro-mm-sig-cunTk1MwBs8s++Sfvej+rw,
	devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ
  Cc: Michal Nazarewicz, Kyungmin Park, Marek Szyprowski

Hello,

Here is my initial proposal for device tree integration for Contiguous
Memory Allocator. The code is quite straightforward, however I expect
that the memory bindings require some discussion.

The proposed bindings allows to define contiguous memory regions of
specified base address and size. Then, the defined regions can be
assigned to the given device(s) by adding a property with a phanle to
the defined contiguous memory region. From the device tree perspective
that's all. Once the bindings are added, all the memory allocations from
dma-mapping subsystem will be served from the defined contiguous memory
regions.

Contiguous Memory Allocator is a framework, which lets to provide a
large contiguous memory buffers for (usually a multimedia) devices. The
contiguous memory is reserved during early boot and then shared with
kernel, which is allowed to allocate it for movable pages. Then, when
device driver requests a contigouous buffer, the framework migrates
movable pages out of contiguous region and gives it to the driver. When
device driver frees the buffer, it is added to kernel memory pool again.
For more information, please refer to commit c64be2bb1c6eb43c838b2c6d57
("drivers: add Contiguous Memory Allocator") and d484864dd96e1830e76895
(CMA merge commit).

Best regards
Marek Szyprowski
Samsung Poland R&D Center


Patch summary:

Marek Szyprowski (2):
  drivers: dma-contiguous: clean source code and prepare for device
    tree
  drivers: dma-contiguous: add initialization from device tree

 Documentation/devicetree/bindings/memory.txt |  101 ++++++++++
 arch/arm/boot/dts/skeleton.dtsi              |    7 +-
 drivers/base/dma-contiguous.c                |  278 +++++++++++++++++++-------
 include/asm-generic/dma-contiguous.h         |    4 +-
 include/linux/dma-contiguous.h               |   32 ++-
 5 files changed, 338 insertions(+), 84 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/memory.txt

-- 
1.7.9.5

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

* [PATCH 0/2] Device Tree support for CMA (Contiguous Memory Allocator)
@ 2013-02-14 12:45 ` Marek Szyprowski
  0 siblings, 0 replies; 36+ messages in thread
From: Marek Szyprowski @ 2013-02-14 12:45 UTC (permalink / raw)
  To: linux-arm-kernel

Hello,

Here is my initial proposal for device tree integration for Contiguous
Memory Allocator. The code is quite straightforward, however I expect
that the memory bindings require some discussion.

The proposed bindings allows to define contiguous memory regions of
specified base address and size. Then, the defined regions can be
assigned to the given device(s) by adding a property with a phanle to
the defined contiguous memory region. From the device tree perspective
that's all. Once the bindings are added, all the memory allocations from
dma-mapping subsystem will be served from the defined contiguous memory
regions.

Contiguous Memory Allocator is a framework, which lets to provide a
large contiguous memory buffers for (usually a multimedia) devices. The
contiguous memory is reserved during early boot and then shared with
kernel, which is allowed to allocate it for movable pages. Then, when
device driver requests a contigouous buffer, the framework migrates
movable pages out of contiguous region and gives it to the driver. When
device driver frees the buffer, it is added to kernel memory pool again.
For more information, please refer to commit c64be2bb1c6eb43c838b2c6d57
("drivers: add Contiguous Memory Allocator") and d484864dd96e1830e76895
(CMA merge commit).

Best regards
Marek Szyprowski
Samsung Poland R&D Center


Patch summary:

Marek Szyprowski (2):
  drivers: dma-contiguous: clean source code and prepare for device
    tree
  drivers: dma-contiguous: add initialization from device tree

 Documentation/devicetree/bindings/memory.txt |  101 ++++++++++
 arch/arm/boot/dts/skeleton.dtsi              |    7 +-
 drivers/base/dma-contiguous.c                |  278 +++++++++++++++++++-------
 include/asm-generic/dma-contiguous.h         |    4 +-
 include/linux/dma-contiguous.h               |   32 ++-
 5 files changed, 338 insertions(+), 84 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/memory.txt

-- 
1.7.9.5

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

* [PATCH 1/2] drivers: dma-contiguous: clean source code and prepare for device tree
  2013-02-14 12:45 ` Marek Szyprowski
@ 2013-02-14 12:45     ` Marek Szyprowski
  -1 siblings, 0 replies; 36+ messages in thread
From: Marek Szyprowski @ 2013-02-14 12:45 UTC (permalink / raw)
  To: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linaro-mm-sig-cunTk1MwBs8s++Sfvej+rw,
	devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ
  Cc: Michal Nazarewicz, Kyungmin Park, Marek Szyprowski

This patch cleans the initialization of dma contiguous framework. The
all-in-one dma_declare_contiguous() function is now separated into
dma_contiguous_reserve_area() which only steals the the memory from
memblock allocator and dma_contiguous_add_device() function, which
assigns given device to the specified reserved memory area. This improves
the flexibility in defining contiguous memory areas and assigning device
to them, because now it is possible to assign more than one device to
the given contiguous memory area. This split in initialization is also
required for upcoming device tree support.

Signed-off-by: Marek Szyprowski <m.szyprowski-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
Acked-by: Kyungmin Park <kyungmin.park-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
---
 drivers/base/dma-contiguous.c        |  210 +++++++++++++++++++++-------------
 include/asm-generic/dma-contiguous.h |    4 +-
 include/linux/dma-contiguous.h       |   32 +++++-
 3 files changed, 161 insertions(+), 85 deletions(-)

diff --git a/drivers/base/dma-contiguous.c b/drivers/base/dma-contiguous.c
index 0ca5442..085389c 100644
--- a/drivers/base/dma-contiguous.c
+++ b/drivers/base/dma-contiguous.c
@@ -39,7 +39,33 @@ struct cma {
 	unsigned long	*bitmap;
 };
 
-struct cma *dma_contiguous_default_area;
+static DEFINE_MUTEX(cma_mutex);
+
+struct cma *dma_contiguous_def_area;
+phys_addr_t dma_contiguous_def_base;
+
+static struct cma_area {
+	phys_addr_t base;
+	unsigned long size;
+	struct cma *cma;
+} cma_areas[MAX_CMA_AREAS] __initdata;
+static unsigned cma_area_count __initdata;
+
+
+static struct cma_map {
+	phys_addr_t base;
+	struct device *dev;
+} cma_maps[MAX_CMA_AREAS] __initdata;
+static unsigned cma_map_count __initdata;
+
+static struct cma *cma_get_area(phys_addr_t base)
+{
+	int i;
+	for (i = 0; i < cma_area_count; i++)
+		if (cma_areas[i].base == base)
+			return cma_areas[i].cma;
+	return NULL;
+}
 
 #ifdef CONFIG_CMA_SIZE_MBYTES
 #define CMA_SIZE_MBYTES CONFIG_CMA_SIZE_MBYTES
@@ -95,45 +121,6 @@ static inline __maybe_unused phys_addr_t cma_early_percent_memory(void)
 
 #endif
 
-/**
- * dma_contiguous_reserve() - reserve area for contiguous memory handling
- * @limit: End address of the reserved memory (optional, 0 for any).
- *
- * This function reserves memory from early allocator. It should be
- * called by arch specific code once the early allocator (memblock or bootmem)
- * has been activated and all other subsystems have already allocated/reserved
- * memory.
- */
-void __init dma_contiguous_reserve(phys_addr_t limit)
-{
-	phys_addr_t selected_size = 0;
-
-	pr_debug("%s(limit %08lx)\n", __func__, (unsigned long)limit);
-
-	if (size_cmdline != -1) {
-		selected_size = size_cmdline;
-	} else {
-#ifdef CONFIG_CMA_SIZE_SEL_MBYTES
-		selected_size = size_bytes;
-#elif defined(CONFIG_CMA_SIZE_SEL_PERCENTAGE)
-		selected_size = cma_early_percent_memory();
-#elif defined(CONFIG_CMA_SIZE_SEL_MIN)
-		selected_size = min(size_bytes, cma_early_percent_memory());
-#elif defined(CONFIG_CMA_SIZE_SEL_MAX)
-		selected_size = max(size_bytes, cma_early_percent_memory());
-#endif
-	}
-
-	if (selected_size) {
-		pr_debug("%s: reserving %ld MiB for global area\n", __func__,
-			 (unsigned long)selected_size / SZ_1M);
-
-		dma_declare_contiguous(NULL, selected_size, 0, limit);
-	}
-};
-
-static DEFINE_MUTEX(cma_mutex);
-
 static __init int cma_activate_area(unsigned long base_pfn, unsigned long count)
 {
 	unsigned long pfn = base_pfn;
@@ -190,55 +177,73 @@ no_mem:
 	return ERR_PTR(ret);
 }
 
-static struct cma_reserved {
-	phys_addr_t start;
-	unsigned long size;
-	struct device *dev;
-} cma_reserved[MAX_CMA_AREAS] __initdata;
-static unsigned cma_reserved_count __initdata;
-
-static int __init cma_init_reserved_areas(void)
+/**
+ * dma_contiguous_reserve() - reserve area for contiguous memory handling
+ * @limit: End address of the reserved memory (optional, 0 for any).
+ *
+ * This function reserves memory from early allocator. It should be
+ * called by arch specific code once the early allocator (memblock or bootmem)
+ * has been activated and all other subsystems have already allocated/reserved
+ * memory. It reserves contiguous areas for global, device independent
+ * allocations and (optionally) all areas defined in device tree structures.
+ */
+void __init dma_contiguous_reserve(phys_addr_t limit)
 {
-	struct cma_reserved *r = cma_reserved;
-	unsigned i = cma_reserved_count;
+	phys_addr_t sel_size = 0;
 
-	pr_debug("%s()\n", __func__);
+	pr_debug("%s(limit %08lx)\n", __func__, (unsigned long)limit);
 
-	for (; i; --i, ++r) {
-		struct cma *cma;
-		cma = cma_create_area(PFN_DOWN(r->start),
-				      r->size >> PAGE_SHIFT);
-		if (!IS_ERR(cma))
-			dev_set_cma_area(r->dev, cma);
+	if (size_cmdline != -1) {
+		sel_size = size_cmdline;
+	} else {
+#ifdef CONFIG_CMA_SIZE_SEL_MBYTES
+		sel_size = size_bytes;
+#elif defined(CONFIG_CMA_SIZE_SEL_PERCENTAGE)
+		sel_size = cma_early_percent_memory();
+#elif defined(CONFIG_CMA_SIZE_SEL_MIN)
+		sel_size = min(size_bytes, cma_early_percent_memory());
+#elif defined(CONFIG_CMA_SIZE_SEL_MAX)
+		sel_size = max(size_bytes, cma_early_percent_memory());
+#endif
 	}
-	return 0;
-}
-core_initcall(cma_init_reserved_areas);
+
+	if (sel_size) {
+		phys_addr_t base = 0;
+		pr_debug("%s: reserving %ld MiB for global area\n", __func__,
+			 (unsigned long)sel_size / SZ_1M);
+
+		if (dma_contiguous_reserve_area(sel_size, &base, limit) == 0)
+			dma_contiguous_def_base = base;
+	}
+};
 
 /**
- * dma_declare_contiguous() - reserve area for contiguous memory handling
- *			      for particular device
- * @dev:   Pointer to device structure.
- * @size:  Size of the reserved memory.
- * @base:  Start address of the reserved memory (optional, 0 for any).
+ * dma_contiguous_reserve_area() - reserve custom contiguous area
+ * @size: Size of the reserved area (in bytes),
+ * @base: Pointer to the base address of the reserved area, also used to return
+ * 	  base address of the actually reserved area, optional, use pointer to
+ *	  0 for any
  * @limit: End address of the reserved memory (optional, 0 for any).
  *
- * This function reserves memory for specified device. It should be
- * called by board specific code when early allocator (memblock or bootmem)
- * is still activate.
+ * This function reserves memory from early allocator. It should be
+ * called by arch specific code once the early allocator (memblock or bootmem)
+ * has been activated and all other subsystems have already allocated/reserved
+ * memory. This function allows to create custom reserved areas for specific
+ * devices.
  */
-int __init dma_declare_contiguous(struct device *dev, phys_addr_t size,
-				  phys_addr_t base, phys_addr_t limit)
+int __init dma_contiguous_reserve_area(phys_addr_t size, phys_addr_t *res_base,
+				       phys_addr_t limit)
 {
-	struct cma_reserved *r = &cma_reserved[cma_reserved_count];
+	phys_addr_t base = *res_base;
 	phys_addr_t alignment;
+	int ret = 0;
 
 	pr_debug("%s(size %lx, base %08lx, limit %08lx)\n", __func__,
 		 (unsigned long)size, (unsigned long)base,
 		 (unsigned long)limit);
 
 	/* Sanity checks */
-	if (cma_reserved_count == ARRAY_SIZE(cma_reserved)) {
+	if (cma_area_count == ARRAY_SIZE(cma_areas)) {
 		pr_err("Not enough slots for CMA reserved regions!\n");
 		return -ENOSPC;
 	}
@@ -256,7 +261,7 @@ int __init dma_declare_contiguous(struct device *dev, phys_addr_t size,
 	if (base) {
 		if (memblock_is_region_reserved(base, size) ||
 		    memblock_reserve(base, size) < 0) {
-			base = -EBUSY;
+			ret = -EBUSY;
 			goto err;
 		}
 	} else {
@@ -266,7 +271,7 @@ int __init dma_declare_contiguous(struct device *dev, phys_addr_t size,
 		 */
 		phys_addr_t addr = __memblock_alloc_base(size, alignment, limit);
 		if (!addr) {
-			base = -ENOMEM;
+			ret = -ENOMEM;
 			goto err;
 		} else {
 			base = addr;
@@ -277,10 +282,11 @@ int __init dma_declare_contiguous(struct device *dev, phys_addr_t size,
 	 * Each reserved area must be initialised later, when more kernel
 	 * subsystems (like slab allocator) are available.
 	 */
-	r->start = base;
-	r->size = size;
-	r->dev = dev;
-	cma_reserved_count++;
+	cma_areas[cma_area_count].base = base;
+	cma_areas[cma_area_count].size = size;
+	cma_area_count++;
+	*res_base = base;
+
 	pr_info("CMA: reserved %ld MiB at %08lx\n", (unsigned long)size / SZ_1M,
 		(unsigned long)base);
 
@@ -289,9 +295,55 @@ int __init dma_declare_contiguous(struct device *dev, phys_addr_t size,
 	return 0;
 err:
 	pr_err("CMA: failed to reserve %ld MiB\n", (unsigned long)size / SZ_1M);
-	return base;
+	return ret;
+}
+
+/**
+ * dma_contiguous_add_device() - add device to custom contiguous reserved area
+ * @dev:   Pointer to device structure.
+ * @base: Pointer to the base address of the reserved area returned by
+ *        dma_contiguous_reserve_area() function, also used to return
+ *
+ * This function assigns the given device to the contiguous memory area
+ * reserved earlier by dma_contiguous_reserve_area() function.
+ */
+int __init dma_contiguous_add_device(struct device *dev, phys_addr_t base)
+{
+	if (cma_map_count == ARRAY_SIZE(cma_maps)) {
+		pr_err("Not enough slots for CMA reserved regions!\n");
+		return -ENOSPC;
+	}
+	cma_maps[cma_map_count].dev = dev;
+	cma_maps[cma_map_count].base = base;
+	cma_map_count++;
+	return 0;
 }
 
+static int __init cma_init_reserved_areas(void)
+{
+	struct cma *cma;
+	int i;
+
+	for (i = 0; i < cma_area_count; i++) {
+		phys_addr_t base = PFN_DOWN(cma_areas[i].base);
+		unsigned int count = cma_areas[i].size >> PAGE_SHIFT;
+
+		cma = cma_create_area(base, count);
+		if (!IS_ERR(cma))
+			cma_areas[i].cma = cma;
+	}
+
+	dma_contiguous_def_area = cma_get_area(dma_contiguous_def_base);
+
+	for (i = 0; i < cma_map_count; i++) {
+		cma = cma_get_area(cma_maps[i].base);
+		dev_set_cma_area(cma_maps[i].dev, cma);
+	}
+
+	return 0;
+}
+core_initcall(cma_init_reserved_areas);
+
 /**
  * dma_alloc_from_contiguous() - allocate pages from contiguous area
  * @dev:   Pointer to device for which the allocation is performed.
diff --git a/include/asm-generic/dma-contiguous.h b/include/asm-generic/dma-contiguous.h
index 294b1e7..9071ef1 100644
--- a/include/asm-generic/dma-contiguous.h
+++ b/include/asm-generic/dma-contiguous.h
@@ -11,15 +11,13 @@ static inline struct cma *dev_get_cma_area(struct device *dev)
 {
 	if (dev && dev->cma_area)
 		return dev->cma_area;
-	return dma_contiguous_default_area;
+	return dma_contiguous_def_area;
 }
 
 static inline void dev_set_cma_area(struct device *dev, struct cma *cma)
 {
 	if (dev)
 		dev->cma_area = cma;
-	if (!dev && !dma_contiguous_default_area)
-		dma_contiguous_default_area = cma;
 }
 
 #endif
diff --git a/include/linux/dma-contiguous.h b/include/linux/dma-contiguous.h
index 01b5c84..285b593 100644
--- a/include/linux/dma-contiguous.h
+++ b/include/linux/dma-contiguous.h
@@ -65,11 +65,37 @@ struct device;
  */
 #define MAX_CMA_AREAS	(1 + CONFIG_CMA_AREAS)
 
-extern struct cma *dma_contiguous_default_area;
+extern struct cma *dma_contiguous_def_area;
 
 void dma_contiguous_reserve(phys_addr_t addr_limit);
-int dma_declare_contiguous(struct device *dev, phys_addr_t size,
-			   phys_addr_t base, phys_addr_t limit);
+
+int dma_contiguous_reserve_area(phys_addr_t size, phys_addr_t *res_base,
+				  phys_addr_t limit);
+
+int dma_contiguous_add_device(struct device *dev, phys_addr_t base);
+
+/**
+ * dma_declare_contiguous() - reserve area for contiguous memory handling
+ *			      for particular device
+ * @dev:   Pointer to device structure.
+ * @size:  Size of the reserved memory.
+ * @base:  Start address of the reserved memory (optional, 0 for any).
+ * @limit: End address of the reserved memory (optional, 0 for any).
+ *
+ * This function reserves memory for specified device. It should be
+ * called by board specific code when early allocator (memblock or bootmem)
+ * is still activate.
+ */
+
+static inline int dma_declare_contiguous(struct device *dev, phys_addr_t size,
+					 phys_addr_t base, phys_addr_t limit)
+{
+	int ret;
+	ret = dma_contiguous_reserve_area(size, &base, limit);
+	if (ret == 0)
+		ret = dma_contiguous_add_device(dev, base);
+	return ret;
+}
 
 struct page *dma_alloc_from_contiguous(struct device *dev, int count,
 				       unsigned int order);
-- 
1.7.9.5

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

* [PATCH 1/2] drivers: dma-contiguous: clean source code and prepare for device tree
@ 2013-02-14 12:45     ` Marek Szyprowski
  0 siblings, 0 replies; 36+ messages in thread
From: Marek Szyprowski @ 2013-02-14 12:45 UTC (permalink / raw)
  To: linux-arm-kernel

This patch cleans the initialization of dma contiguous framework. The
all-in-one dma_declare_contiguous() function is now separated into
dma_contiguous_reserve_area() which only steals the the memory from
memblock allocator and dma_contiguous_add_device() function, which
assigns given device to the specified reserved memory area. This improves
the flexibility in defining contiguous memory areas and assigning device
to them, because now it is possible to assign more than one device to
the given contiguous memory area. This split in initialization is also
required for upcoming device tree support.

Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
Acked-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 drivers/base/dma-contiguous.c        |  210 +++++++++++++++++++++-------------
 include/asm-generic/dma-contiguous.h |    4 +-
 include/linux/dma-contiguous.h       |   32 +++++-
 3 files changed, 161 insertions(+), 85 deletions(-)

diff --git a/drivers/base/dma-contiguous.c b/drivers/base/dma-contiguous.c
index 0ca5442..085389c 100644
--- a/drivers/base/dma-contiguous.c
+++ b/drivers/base/dma-contiguous.c
@@ -39,7 +39,33 @@ struct cma {
 	unsigned long	*bitmap;
 };
 
-struct cma *dma_contiguous_default_area;
+static DEFINE_MUTEX(cma_mutex);
+
+struct cma *dma_contiguous_def_area;
+phys_addr_t dma_contiguous_def_base;
+
+static struct cma_area {
+	phys_addr_t base;
+	unsigned long size;
+	struct cma *cma;
+} cma_areas[MAX_CMA_AREAS] __initdata;
+static unsigned cma_area_count __initdata;
+
+
+static struct cma_map {
+	phys_addr_t base;
+	struct device *dev;
+} cma_maps[MAX_CMA_AREAS] __initdata;
+static unsigned cma_map_count __initdata;
+
+static struct cma *cma_get_area(phys_addr_t base)
+{
+	int i;
+	for (i = 0; i < cma_area_count; i++)
+		if (cma_areas[i].base == base)
+			return cma_areas[i].cma;
+	return NULL;
+}
 
 #ifdef CONFIG_CMA_SIZE_MBYTES
 #define CMA_SIZE_MBYTES CONFIG_CMA_SIZE_MBYTES
@@ -95,45 +121,6 @@ static inline __maybe_unused phys_addr_t cma_early_percent_memory(void)
 
 #endif
 
-/**
- * dma_contiguous_reserve() - reserve area for contiguous memory handling
- * @limit: End address of the reserved memory (optional, 0 for any).
- *
- * This function reserves memory from early allocator. It should be
- * called by arch specific code once the early allocator (memblock or bootmem)
- * has been activated and all other subsystems have already allocated/reserved
- * memory.
- */
-void __init dma_contiguous_reserve(phys_addr_t limit)
-{
-	phys_addr_t selected_size = 0;
-
-	pr_debug("%s(limit %08lx)\n", __func__, (unsigned long)limit);
-
-	if (size_cmdline != -1) {
-		selected_size = size_cmdline;
-	} else {
-#ifdef CONFIG_CMA_SIZE_SEL_MBYTES
-		selected_size = size_bytes;
-#elif defined(CONFIG_CMA_SIZE_SEL_PERCENTAGE)
-		selected_size = cma_early_percent_memory();
-#elif defined(CONFIG_CMA_SIZE_SEL_MIN)
-		selected_size = min(size_bytes, cma_early_percent_memory());
-#elif defined(CONFIG_CMA_SIZE_SEL_MAX)
-		selected_size = max(size_bytes, cma_early_percent_memory());
-#endif
-	}
-
-	if (selected_size) {
-		pr_debug("%s: reserving %ld MiB for global area\n", __func__,
-			 (unsigned long)selected_size / SZ_1M);
-
-		dma_declare_contiguous(NULL, selected_size, 0, limit);
-	}
-};
-
-static DEFINE_MUTEX(cma_mutex);
-
 static __init int cma_activate_area(unsigned long base_pfn, unsigned long count)
 {
 	unsigned long pfn = base_pfn;
@@ -190,55 +177,73 @@ no_mem:
 	return ERR_PTR(ret);
 }
 
-static struct cma_reserved {
-	phys_addr_t start;
-	unsigned long size;
-	struct device *dev;
-} cma_reserved[MAX_CMA_AREAS] __initdata;
-static unsigned cma_reserved_count __initdata;
-
-static int __init cma_init_reserved_areas(void)
+/**
+ * dma_contiguous_reserve() - reserve area for contiguous memory handling
+ * @limit: End address of the reserved memory (optional, 0 for any).
+ *
+ * This function reserves memory from early allocator. It should be
+ * called by arch specific code once the early allocator (memblock or bootmem)
+ * has been activated and all other subsystems have already allocated/reserved
+ * memory. It reserves contiguous areas for global, device independent
+ * allocations and (optionally) all areas defined in device tree structures.
+ */
+void __init dma_contiguous_reserve(phys_addr_t limit)
 {
-	struct cma_reserved *r = cma_reserved;
-	unsigned i = cma_reserved_count;
+	phys_addr_t sel_size = 0;
 
-	pr_debug("%s()\n", __func__);
+	pr_debug("%s(limit %08lx)\n", __func__, (unsigned long)limit);
 
-	for (; i; --i, ++r) {
-		struct cma *cma;
-		cma = cma_create_area(PFN_DOWN(r->start),
-				      r->size >> PAGE_SHIFT);
-		if (!IS_ERR(cma))
-			dev_set_cma_area(r->dev, cma);
+	if (size_cmdline != -1) {
+		sel_size = size_cmdline;
+	} else {
+#ifdef CONFIG_CMA_SIZE_SEL_MBYTES
+		sel_size = size_bytes;
+#elif defined(CONFIG_CMA_SIZE_SEL_PERCENTAGE)
+		sel_size = cma_early_percent_memory();
+#elif defined(CONFIG_CMA_SIZE_SEL_MIN)
+		sel_size = min(size_bytes, cma_early_percent_memory());
+#elif defined(CONFIG_CMA_SIZE_SEL_MAX)
+		sel_size = max(size_bytes, cma_early_percent_memory());
+#endif
 	}
-	return 0;
-}
-core_initcall(cma_init_reserved_areas);
+
+	if (sel_size) {
+		phys_addr_t base = 0;
+		pr_debug("%s: reserving %ld MiB for global area\n", __func__,
+			 (unsigned long)sel_size / SZ_1M);
+
+		if (dma_contiguous_reserve_area(sel_size, &base, limit) == 0)
+			dma_contiguous_def_base = base;
+	}
+};
 
 /**
- * dma_declare_contiguous() - reserve area for contiguous memory handling
- *			      for particular device
- * @dev:   Pointer to device structure.
- * @size:  Size of the reserved memory.
- * @base:  Start address of the reserved memory (optional, 0 for any).
+ * dma_contiguous_reserve_area() - reserve custom contiguous area
+ * @size: Size of the reserved area (in bytes),
+ * @base: Pointer to the base address of the reserved area, also used to return
+ * 	  base address of the actually reserved area, optional, use pointer to
+ *	  0 for any
  * @limit: End address of the reserved memory (optional, 0 for any).
  *
- * This function reserves memory for specified device. It should be
- * called by board specific code when early allocator (memblock or bootmem)
- * is still activate.
+ * This function reserves memory from early allocator. It should be
+ * called by arch specific code once the early allocator (memblock or bootmem)
+ * has been activated and all other subsystems have already allocated/reserved
+ * memory. This function allows to create custom reserved areas for specific
+ * devices.
  */
-int __init dma_declare_contiguous(struct device *dev, phys_addr_t size,
-				  phys_addr_t base, phys_addr_t limit)
+int __init dma_contiguous_reserve_area(phys_addr_t size, phys_addr_t *res_base,
+				       phys_addr_t limit)
 {
-	struct cma_reserved *r = &cma_reserved[cma_reserved_count];
+	phys_addr_t base = *res_base;
 	phys_addr_t alignment;
+	int ret = 0;
 
 	pr_debug("%s(size %lx, base %08lx, limit %08lx)\n", __func__,
 		 (unsigned long)size, (unsigned long)base,
 		 (unsigned long)limit);
 
 	/* Sanity checks */
-	if (cma_reserved_count == ARRAY_SIZE(cma_reserved)) {
+	if (cma_area_count == ARRAY_SIZE(cma_areas)) {
 		pr_err("Not enough slots for CMA reserved regions!\n");
 		return -ENOSPC;
 	}
@@ -256,7 +261,7 @@ int __init dma_declare_contiguous(struct device *dev, phys_addr_t size,
 	if (base) {
 		if (memblock_is_region_reserved(base, size) ||
 		    memblock_reserve(base, size) < 0) {
-			base = -EBUSY;
+			ret = -EBUSY;
 			goto err;
 		}
 	} else {
@@ -266,7 +271,7 @@ int __init dma_declare_contiguous(struct device *dev, phys_addr_t size,
 		 */
 		phys_addr_t addr = __memblock_alloc_base(size, alignment, limit);
 		if (!addr) {
-			base = -ENOMEM;
+			ret = -ENOMEM;
 			goto err;
 		} else {
 			base = addr;
@@ -277,10 +282,11 @@ int __init dma_declare_contiguous(struct device *dev, phys_addr_t size,
 	 * Each reserved area must be initialised later, when more kernel
 	 * subsystems (like slab allocator) are available.
 	 */
-	r->start = base;
-	r->size = size;
-	r->dev = dev;
-	cma_reserved_count++;
+	cma_areas[cma_area_count].base = base;
+	cma_areas[cma_area_count].size = size;
+	cma_area_count++;
+	*res_base = base;
+
 	pr_info("CMA: reserved %ld MiB at %08lx\n", (unsigned long)size / SZ_1M,
 		(unsigned long)base);
 
@@ -289,9 +295,55 @@ int __init dma_declare_contiguous(struct device *dev, phys_addr_t size,
 	return 0;
 err:
 	pr_err("CMA: failed to reserve %ld MiB\n", (unsigned long)size / SZ_1M);
-	return base;
+	return ret;
+}
+
+/**
+ * dma_contiguous_add_device() - add device to custom contiguous reserved area
+ * @dev:   Pointer to device structure.
+ * @base: Pointer to the base address of the reserved area returned by
+ *        dma_contiguous_reserve_area() function, also used to return
+ *
+ * This function assigns the given device to the contiguous memory area
+ * reserved earlier by dma_contiguous_reserve_area() function.
+ */
+int __init dma_contiguous_add_device(struct device *dev, phys_addr_t base)
+{
+	if (cma_map_count == ARRAY_SIZE(cma_maps)) {
+		pr_err("Not enough slots for CMA reserved regions!\n");
+		return -ENOSPC;
+	}
+	cma_maps[cma_map_count].dev = dev;
+	cma_maps[cma_map_count].base = base;
+	cma_map_count++;
+	return 0;
 }
 
+static int __init cma_init_reserved_areas(void)
+{
+	struct cma *cma;
+	int i;
+
+	for (i = 0; i < cma_area_count; i++) {
+		phys_addr_t base = PFN_DOWN(cma_areas[i].base);
+		unsigned int count = cma_areas[i].size >> PAGE_SHIFT;
+
+		cma = cma_create_area(base, count);
+		if (!IS_ERR(cma))
+			cma_areas[i].cma = cma;
+	}
+
+	dma_contiguous_def_area = cma_get_area(dma_contiguous_def_base);
+
+	for (i = 0; i < cma_map_count; i++) {
+		cma = cma_get_area(cma_maps[i].base);
+		dev_set_cma_area(cma_maps[i].dev, cma);
+	}
+
+	return 0;
+}
+core_initcall(cma_init_reserved_areas);
+
 /**
  * dma_alloc_from_contiguous() - allocate pages from contiguous area
  * @dev:   Pointer to device for which the allocation is performed.
diff --git a/include/asm-generic/dma-contiguous.h b/include/asm-generic/dma-contiguous.h
index 294b1e7..9071ef1 100644
--- a/include/asm-generic/dma-contiguous.h
+++ b/include/asm-generic/dma-contiguous.h
@@ -11,15 +11,13 @@ static inline struct cma *dev_get_cma_area(struct device *dev)
 {
 	if (dev && dev->cma_area)
 		return dev->cma_area;
-	return dma_contiguous_default_area;
+	return dma_contiguous_def_area;
 }
 
 static inline void dev_set_cma_area(struct device *dev, struct cma *cma)
 {
 	if (dev)
 		dev->cma_area = cma;
-	if (!dev && !dma_contiguous_default_area)
-		dma_contiguous_default_area = cma;
 }
 
 #endif
diff --git a/include/linux/dma-contiguous.h b/include/linux/dma-contiguous.h
index 01b5c84..285b593 100644
--- a/include/linux/dma-contiguous.h
+++ b/include/linux/dma-contiguous.h
@@ -65,11 +65,37 @@ struct device;
  */
 #define MAX_CMA_AREAS	(1 + CONFIG_CMA_AREAS)
 
-extern struct cma *dma_contiguous_default_area;
+extern struct cma *dma_contiguous_def_area;
 
 void dma_contiguous_reserve(phys_addr_t addr_limit);
-int dma_declare_contiguous(struct device *dev, phys_addr_t size,
-			   phys_addr_t base, phys_addr_t limit);
+
+int dma_contiguous_reserve_area(phys_addr_t size, phys_addr_t *res_base,
+				  phys_addr_t limit);
+
+int dma_contiguous_add_device(struct device *dev, phys_addr_t base);
+
+/**
+ * dma_declare_contiguous() - reserve area for contiguous memory handling
+ *			      for particular device
+ * @dev:   Pointer to device structure.
+ * @size:  Size of the reserved memory.
+ * @base:  Start address of the reserved memory (optional, 0 for any).
+ * @limit: End address of the reserved memory (optional, 0 for any).
+ *
+ * This function reserves memory for specified device. It should be
+ * called by board specific code when early allocator (memblock or bootmem)
+ * is still activate.
+ */
+
+static inline int dma_declare_contiguous(struct device *dev, phys_addr_t size,
+					 phys_addr_t base, phys_addr_t limit)
+{
+	int ret;
+	ret = dma_contiguous_reserve_area(size, &base, limit);
+	if (ret == 0)
+		ret = dma_contiguous_add_device(dev, base);
+	return ret;
+}
 
 struct page *dma_alloc_from_contiguous(struct device *dev, int count,
 				       unsigned int order);
-- 
1.7.9.5

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

* [PATCH 2/2] drivers: dma-contiguous: add initialization from device tree
  2013-02-14 12:45 ` Marek Szyprowski
@ 2013-02-14 12:45     ` Marek Szyprowski
  -1 siblings, 0 replies; 36+ messages in thread
From: Marek Szyprowski @ 2013-02-14 12:45 UTC (permalink / raw)
  To: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linaro-mm-sig-cunTk1MwBs8s++Sfvej+rw,
	devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ
  Cc: Michal Nazarewicz, Kyungmin Park, Marek Szyprowski

Add device tree support for contiguous memory regions defined in device
tree. Initialization is done in 2 steps. First, the contiguous memory is
reserved, what happens very early, when only flattened device tree is
available. Then on device initialization the corresponding cma regions are
assigned to device structures.

Signed-off-by: Marek Szyprowski <m.szyprowski-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
Acked-by: Kyungmin Park <kyungmin.park-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
---
 Documentation/devicetree/bindings/memory.txt |  101 ++++++++++++++++++++++++++
 arch/arm/boot/dts/skeleton.dtsi              |    7 +-
 drivers/base/dma-contiguous.c                |   72 ++++++++++++++++++
 3 files changed, 179 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/devicetree/bindings/memory.txt

diff --git a/Documentation/devicetree/bindings/memory.txt b/Documentation/devicetree/bindings/memory.txt
new file mode 100644
index 0000000..74e0476
--- /dev/null
+++ b/Documentation/devicetree/bindings/memory.txt
@@ -0,0 +1,101 @@
+* Memory binding
+
+The /memory node provides basic information about the address and size
+of the physical memory. This node is usually filled or updated by the
+bootloader, depending on the actual memory configuration of the given
+hardware.
+
+The memory layout is described by the folllowing node:
+
+memory {
+	reg =  <(baseaddr1) (size1)
+		(baseaddr2) (size2)
+		...
+		(baseaddrN) (sizeN)>;
+};
+
+baseaddrX:	the base address of the defined memory bank
+sizeX:		the size of the defined memory bank
+
+More than one memory bank can be defined.
+
+
+* Memory regions
+
+In /memory node one can create additional nodes describing particular
+memory regions, usually for the special usage by various device drivers.
+A good example are contiguous memory allocations or memory sharing with
+other operating system on the same hardware board. Those special memory
+regions might depend on the board configuration and devices used on the
+target system.
+
+Parameters for each memory region can be encoded into the device tree
+wit the following convention:
+
+(name): region@(base-address) {
+	reg = <(baseaddr) (size)>;
+	(linux,contiguous-region);
+	(linux,default-contiguous-region);
+};
+
+name:		an name given to the defined region.
+base-address:	the base address of the defined region.
+size:		the size of the memory region.
+linux,contiguous-region: property indicating that the defined memory
+		region is used for contiguous memory allocations,
+		Linux specific (optional)
+linux,default-contiguous-region: property indicating that the region
+		is the default region for all contiguous memory
+		allocations, Linux specific (optional)
+
+
+* Device nodes
+
+Once the regions in the /memory node are defined, they can be assigned
+to device some device nodes for their special use. The following
+properties are defined:
+
+linux,contiguous-region = <&phandle>;
+	This property indicates that the device driver should use the
+	memory region pointed by the given phandle.
+
+
+* Example:
+
+This example defines a memory consisting of 4 memory banks. 2 contiguous
+regions are defined for Linux kernel, one default of all device drivers
+(named contig_mem, placed at 0x72000000, 64MiB) and one dedicated to the
+framebuffer device (named display_mem, placed at 0x78000000, 16MiB). The
+display_mem region is then assigned to fb@12300000 device for contiguous
+memory allocation with Linux kernel drivers.
+
+The reason for creating a separate region for framebuffer device is to
+match the framebuffer address of from configuration done by bootloader,
+so once Linux kernel drivers starts, no glitches on the displayed boot
+logo appears.
+
+/ {
+	/* ... */
+	memory {
+		reg =  <0x40000000 0x10000000
+			0x50000000 0x10000000
+			0x60000000 0x10000000
+			0x70000000 0x10000000>;
+
+		contig_mem: region@72000000 {
+			linux,contiguous-region;
+			linux,default-contiguous-region;
+			reg = <0x72000000 0x4000000>;
+		};
+
+		display_mem: region@78000000 {
+			linux,contiguous-region;
+			reg = <0x78000000 0x1000000>;
+		};
+	};
+
+	fb@12300000 {
+		linux,contiguous-region = <&display_mem>;
+		status = "okay";
+	};
+};
diff --git a/arch/arm/boot/dts/skeleton.dtsi b/arch/arm/boot/dts/skeleton.dtsi
index b41d241..f9988cd 100644
--- a/arch/arm/boot/dts/skeleton.dtsi
+++ b/arch/arm/boot/dts/skeleton.dtsi
@@ -9,5 +9,10 @@
 	#size-cells = <1>;
 	chosen { };
 	aliases { };
-	memory { device_type = "memory"; reg = <0 0>; };
+	memory {
+		#address-cells = <1>;
+		#size-cells = <1>;
+		device_type = "memory";
+		reg = <0 0>;
+	};
 };
diff --git a/drivers/base/dma-contiguous.c b/drivers/base/dma-contiguous.c
index 085389c..5761f73 100644
--- a/drivers/base/dma-contiguous.c
+++ b/drivers/base/dma-contiguous.c
@@ -24,6 +24,9 @@
 
 #include <linux/memblock.h>
 #include <linux/err.h>
+#include <linux/of.h>
+#include <linux/of_fdt.h>
+#include <linux/of_platform.h>
 #include <linux/mm.h>
 #include <linux/mutex.h>
 #include <linux/page-isolation.h>
@@ -177,6 +180,35 @@ no_mem:
 	return ERR_PTR(ret);
 }
 
+/*****************************************************************************/
+
+#ifdef CONFIG_OF
+int __init cma_fdt_scan(unsigned long node, const char *uname,
+				int depth, void *data)
+{
+	phys_addr_t base, size;
+	unsigned long len;
+	__be32 *prop;
+
+	if (strncmp(uname, "region@", 7) != 0 || depth != 2 ||
+	    !of_get_flat_dt_prop(node, "contiguous-region", NULL))
+		return 0;
+
+	prop = of_get_flat_dt_prop(node, "reg", &len);
+	if (!prop || (len != 2 * sizeof(unsigned long)))
+		return 0;
+
+	base = be32_to_cpu(prop[0]);
+	size = be32_to_cpu(prop[1]);
+
+	pr_info("Found %s, memory base %lx, size %ld MiB\n", uname,
+		(unsigned long)base, (unsigned long)size / SZ_1M);
+	dma_contiguous_reserve_area(size, &base, 0);
+
+	return 0;
+}
+#endif
+
 /**
  * dma_contiguous_reserve() - reserve area for contiguous memory handling
  * @limit: End address of the reserved memory (optional, 0 for any).
@@ -215,6 +247,9 @@ void __init dma_contiguous_reserve(phys_addr_t limit)
 		if (dma_contiguous_reserve_area(sel_size, &base, limit) == 0)
 			dma_contiguous_def_base = base;
 	}
+#ifdef CONFIG_OF
+	of_scan_flat_dt(cma_fdt_scan, NULL);
+#endif
 };
 
 /**
@@ -319,6 +354,40 @@ int __init dma_contiguous_add_device(struct device *dev, phys_addr_t base)
 	return 0;
 }
 
+#ifdef CONFIG_OF
+static void cma_assign_device_from_dt(struct device *dev)
+{
+	struct device_node *node;
+	struct cma *cma;
+	u32 value;
+
+	node = of_parse_phandle(dev->of_node, "linux,contiguous-region", 0);
+	if (!node)
+		return;
+	if (of_property_read_u32(node, "reg", &value) && !value)
+		return;
+	cma = cma_get_area(value);
+	if (!cma)
+		return;
+
+	dev_set_cma_area(dev, cma);
+	pr_info("Assigned CMA region at %lx to %s device\n", (unsigned long)value, dev_name(dev));
+}
+
+static int cma_device_init_notifier_call(struct notifier_block *nb,
+					 unsigned long event, void *data)
+{
+	struct device *dev = data;
+	if (event == BUS_NOTIFY_ADD_DEVICE && dev->of_node)
+		cma_assign_device_from_dt(dev);
+	return NOTIFY_DONE;
+}
+
+static struct notifier_block cma_dev_init_nb = {
+	.notifier_call = cma_device_init_notifier_call,
+};
+#endif
+
 static int __init cma_init_reserved_areas(void)
 {
 	struct cma *cma;
@@ -340,6 +409,9 @@ static int __init cma_init_reserved_areas(void)
 		dev_set_cma_area(cma_maps[i].dev, cma);
 	}
 
+#ifdef CONFIG_OF
+	bus_register_notifier(&platform_bus_type, &cma_dev_init_nb);
+#endif
 	return 0;
 }
 core_initcall(cma_init_reserved_areas);
-- 
1.7.9.5

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

* [PATCH 2/2] drivers: dma-contiguous: add initialization from device tree
@ 2013-02-14 12:45     ` Marek Szyprowski
  0 siblings, 0 replies; 36+ messages in thread
From: Marek Szyprowski @ 2013-02-14 12:45 UTC (permalink / raw)
  To: linux-arm-kernel

Add device tree support for contiguous memory regions defined in device
tree. Initialization is done in 2 steps. First, the contiguous memory is
reserved, what happens very early, when only flattened device tree is
available. Then on device initialization the corresponding cma regions are
assigned to device structures.

Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
Acked-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 Documentation/devicetree/bindings/memory.txt |  101 ++++++++++++++++++++++++++
 arch/arm/boot/dts/skeleton.dtsi              |    7 +-
 drivers/base/dma-contiguous.c                |   72 ++++++++++++++++++
 3 files changed, 179 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/devicetree/bindings/memory.txt

diff --git a/Documentation/devicetree/bindings/memory.txt b/Documentation/devicetree/bindings/memory.txt
new file mode 100644
index 0000000..74e0476
--- /dev/null
+++ b/Documentation/devicetree/bindings/memory.txt
@@ -0,0 +1,101 @@
+* Memory binding
+
+The /memory node provides basic information about the address and size
+of the physical memory. This node is usually filled or updated by the
+bootloader, depending on the actual memory configuration of the given
+hardware.
+
+The memory layout is described by the folllowing node:
+
+memory {
+	reg =  <(baseaddr1) (size1)
+		(baseaddr2) (size2)
+		...
+		(baseaddrN) (sizeN)>;
+};
+
+baseaddrX:	the base address of the defined memory bank
+sizeX:		the size of the defined memory bank
+
+More than one memory bank can be defined.
+
+
+* Memory regions
+
+In /memory node one can create additional nodes describing particular
+memory regions, usually for the special usage by various device drivers.
+A good example are contiguous memory allocations or memory sharing with
+other operating system on the same hardware board. Those special memory
+regions might depend on the board configuration and devices used on the
+target system.
+
+Parameters for each memory region can be encoded into the device tree
+wit the following convention:
+
+(name): region@(base-address) {
+	reg = <(baseaddr) (size)>;
+	(linux,contiguous-region);
+	(linux,default-contiguous-region);
+};
+
+name:		an name given to the defined region.
+base-address:	the base address of the defined region.
+size:		the size of the memory region.
+linux,contiguous-region: property indicating that the defined memory
+		region is used for contiguous memory allocations,
+		Linux specific (optional)
+linux,default-contiguous-region: property indicating that the region
+		is the default region for all contiguous memory
+		allocations, Linux specific (optional)
+
+
+* Device nodes
+
+Once the regions in the /memory node are defined, they can be assigned
+to device some device nodes for their special use. The following
+properties are defined:
+
+linux,contiguous-region = <&phandle>;
+	This property indicates that the device driver should use the
+	memory region pointed by the given phandle.
+
+
+* Example:
+
+This example defines a memory consisting of 4 memory banks. 2 contiguous
+regions are defined for Linux kernel, one default of all device drivers
+(named contig_mem, placed at 0x72000000, 64MiB) and one dedicated to the
+framebuffer device (named display_mem, placed at 0x78000000, 16MiB). The
+display_mem region is then assigned to fb at 12300000 device for contiguous
+memory allocation with Linux kernel drivers.
+
+The reason for creating a separate region for framebuffer device is to
+match the framebuffer address of from configuration done by bootloader,
+so once Linux kernel drivers starts, no glitches on the displayed boot
+logo appears.
+
+/ {
+	/* ... */
+	memory {
+		reg =  <0x40000000 0x10000000
+			0x50000000 0x10000000
+			0x60000000 0x10000000
+			0x70000000 0x10000000>;
+
+		contig_mem: region at 72000000 {
+			linux,contiguous-region;
+			linux,default-contiguous-region;
+			reg = <0x72000000 0x4000000>;
+		};
+
+		display_mem: region at 78000000 {
+			linux,contiguous-region;
+			reg = <0x78000000 0x1000000>;
+		};
+	};
+
+	fb at 12300000 {
+		linux,contiguous-region = <&display_mem>;
+		status = "okay";
+	};
+};
diff --git a/arch/arm/boot/dts/skeleton.dtsi b/arch/arm/boot/dts/skeleton.dtsi
index b41d241..f9988cd 100644
--- a/arch/arm/boot/dts/skeleton.dtsi
+++ b/arch/arm/boot/dts/skeleton.dtsi
@@ -9,5 +9,10 @@
 	#size-cells = <1>;
 	chosen { };
 	aliases { };
-	memory { device_type = "memory"; reg = <0 0>; };
+	memory {
+		#address-cells = <1>;
+		#size-cells = <1>;
+		device_type = "memory";
+		reg = <0 0>;
+	};
 };
diff --git a/drivers/base/dma-contiguous.c b/drivers/base/dma-contiguous.c
index 085389c..5761f73 100644
--- a/drivers/base/dma-contiguous.c
+++ b/drivers/base/dma-contiguous.c
@@ -24,6 +24,9 @@
 
 #include <linux/memblock.h>
 #include <linux/err.h>
+#include <linux/of.h>
+#include <linux/of_fdt.h>
+#include <linux/of_platform.h>
 #include <linux/mm.h>
 #include <linux/mutex.h>
 #include <linux/page-isolation.h>
@@ -177,6 +180,35 @@ no_mem:
 	return ERR_PTR(ret);
 }
 
+/*****************************************************************************/
+
+#ifdef CONFIG_OF
+int __init cma_fdt_scan(unsigned long node, const char *uname,
+				int depth, void *data)
+{
+	phys_addr_t base, size;
+	unsigned long len;
+	__be32 *prop;
+
+	if (strncmp(uname, "region@", 7) != 0 || depth != 2 ||
+	    !of_get_flat_dt_prop(node, "contiguous-region", NULL))
+		return 0;
+
+	prop = of_get_flat_dt_prop(node, "reg", &len);
+	if (!prop || (len != 2 * sizeof(unsigned long)))
+		return 0;
+
+	base = be32_to_cpu(prop[0]);
+	size = be32_to_cpu(prop[1]);
+
+	pr_info("Found %s, memory base %lx, size %ld MiB\n", uname,
+		(unsigned long)base, (unsigned long)size / SZ_1M);
+	dma_contiguous_reserve_area(size, &base, 0);
+
+	return 0;
+}
+#endif
+
 /**
  * dma_contiguous_reserve() - reserve area for contiguous memory handling
  * @limit: End address of the reserved memory (optional, 0 for any).
@@ -215,6 +247,9 @@ void __init dma_contiguous_reserve(phys_addr_t limit)
 		if (dma_contiguous_reserve_area(sel_size, &base, limit) == 0)
 			dma_contiguous_def_base = base;
 	}
+#ifdef CONFIG_OF
+	of_scan_flat_dt(cma_fdt_scan, NULL);
+#endif
 };
 
 /**
@@ -319,6 +354,40 @@ int __init dma_contiguous_add_device(struct device *dev, phys_addr_t base)
 	return 0;
 }
 
+#ifdef CONFIG_OF
+static void cma_assign_device_from_dt(struct device *dev)
+{
+	struct device_node *node;
+	struct cma *cma;
+	u32 value;
+
+	node = of_parse_phandle(dev->of_node, "linux,contiguous-region", 0);
+	if (!node)
+		return;
+	if (of_property_read_u32(node, "reg", &value) && !value)
+		return;
+	cma = cma_get_area(value);
+	if (!cma)
+		return;
+
+	dev_set_cma_area(dev, cma);
+	pr_info("Assigned CMA region at %lx to %s device\n", (unsigned long)value, dev_name(dev));
+}
+
+static int cma_device_init_notifier_call(struct notifier_block *nb,
+					 unsigned long event, void *data)
+{
+	struct device *dev = data;
+	if (event == BUS_NOTIFY_ADD_DEVICE && dev->of_node)
+		cma_assign_device_from_dt(dev);
+	return NOTIFY_DONE;
+}
+
+static struct notifier_block cma_dev_init_nb = {
+	.notifier_call = cma_device_init_notifier_call,
+};
+#endif
+
 static int __init cma_init_reserved_areas(void)
 {
 	struct cma *cma;
@@ -340,6 +409,9 @@ static int __init cma_init_reserved_areas(void)
 		dev_set_cma_area(cma_maps[i].dev, cma);
 	}
 
+#ifdef CONFIG_OF
+	bus_register_notifier(&platform_bus_type, &cma_dev_init_nb);
+#endif
 	return 0;
 }
 core_initcall(cma_init_reserved_areas);
-- 
1.7.9.5

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

* Re: [PATCH 0/2] Device Tree support for CMA (Contiguous Memory Allocator)
  2013-02-14 12:45 ` Marek Szyprowski
@ 2013-02-14 21:30     ` Sascha Hauer
  -1 siblings, 0 replies; 36+ messages in thread
From: Sascha Hauer @ 2013-02-14 21:30 UTC (permalink / raw)
  To: Marek Szyprowski
  Cc: linaro-mm-sig-cunTk1MwBs8s++Sfvej+rw, Kyungmin Park,
	devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ, Michal Nazarewicz,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r

Hi Marek,

On Thu, Feb 14, 2013 at 01:45:26PM +0100, Marek Szyprowski wrote:
> Hello,
> 
> Here is my initial proposal for device tree integration for Contiguous
> Memory Allocator. The code is quite straightforward, however I expect
> that the memory bindings require some discussion.
> 
> The proposed bindings allows to define contiguous memory regions of
> specified base address and size. Then, the defined regions can be
> assigned to the given device(s) by adding a property with a phanle to
> the defined contiguous memory region. From the device tree perspective
> that's all. Once the bindings are added, all the memory allocations from
> dma-mapping subsystem will be served from the defined contiguous memory
> regions.
> 

I think CMA regions should not be described in the devicetre at all. The
devicetree is about hardware description and it should be OS agnostic,
but CMA is only a Linux specific implementation detail. It's not even
specific to a particular board, it's specific to a particular usecase of
a board.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

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

* [PATCH 0/2] Device Tree support for CMA (Contiguous Memory Allocator)
@ 2013-02-14 21:30     ` Sascha Hauer
  0 siblings, 0 replies; 36+ messages in thread
From: Sascha Hauer @ 2013-02-14 21:30 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Marek,

On Thu, Feb 14, 2013 at 01:45:26PM +0100, Marek Szyprowski wrote:
> Hello,
> 
> Here is my initial proposal for device tree integration for Contiguous
> Memory Allocator. The code is quite straightforward, however I expect
> that the memory bindings require some discussion.
> 
> The proposed bindings allows to define contiguous memory regions of
> specified base address and size. Then, the defined regions can be
> assigned to the given device(s) by adding a property with a phanle to
> the defined contiguous memory region. From the device tree perspective
> that's all. Once the bindings are added, all the memory allocations from
> dma-mapping subsystem will be served from the defined contiguous memory
> regions.
> 

I think CMA regions should not be described in the devicetre at all. The
devicetree is about hardware description and it should be OS agnostic,
but CMA is only a Linux specific implementation detail. It's not even
specific to a particular board, it's specific to a particular usecase of
a board.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

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

* Re: [Linaro-mm-sig] [PATCH 2/2] drivers: dma-contiguous: add initialization from device tree
  2013-02-14 12:45     ` Marek Szyprowski
@ 2013-02-14 21:34       ` Laura Abbott
  -1 siblings, 0 replies; 36+ messages in thread
From: Laura Abbott @ 2013-02-14 21:34 UTC (permalink / raw)
  To: Marek Szyprowski
  Cc: Grant Likely, Arnd Bergmann, Tomasz Figa, Michal Nazarewicz,
	linaro-mm-sig, Kyungmin Park, devicetree-discuss,
	linux-arm-kernel

Hi,


On 2/14/2013 4:45 AM, Marek Szyprowski wrote:
<snip>
> +name:		an name given to the defined region.
> +base-address:	the base address of the defined region.
> +size:		the size of the memory region.
> +linux,contiguous-region: property indicating that the defined memory
> +		region is used for contiguous memory allocations,
> +		Linux specific (optional)
> +linux,default-contiguous-region: property indicating that the region
> +		is the default region for all contiguous memory
> +		allocations, Linux specific (optional)
> +
> +

I don't see any code actually implementing the default-contiguous-region 
binding. Currently on ARM systems we will still setup the default region 
based on the Kconfig. Is this intentional?


> diff --git a/drivers/base/dma-contiguous.c b/drivers/base/dma-contiguous.c
> index 085389c..5761f73 100644
> --- a/drivers/base/dma-contiguous.c
> +++ b/drivers/base/dma-contiguous.c
> @@ -24,6 +24,9 @@
>
>   #include <linux/memblock.h>
>   #include <linux/err.h>
> +#include <linux/of.h>
> +#include <linux/of_fdt.h>
> +#include <linux/of_platform.h>
>   #include <linux/mm.h>
>   #include <linux/mutex.h>
>   #include <linux/page-isolation.h>
> @@ -177,6 +180,35 @@ no_mem:
>   	return ERR_PTR(ret);
>   }
>
> +/*****************************************************************************/
> +
> +#ifdef CONFIG_OF
> +int __init cma_fdt_scan(unsigned long node, const char *uname,
> +				int depth, void *data)
> +{
> +	phys_addr_t base, size;
> +	unsigned long len;
> +	__be32 *prop;
> +
> +	if (strncmp(uname, "region@", 7) != 0 || depth != 2 ||
> +	    !of_get_flat_dt_prop(node, "contiguous-region", NULL))

The documentation says "linux,contiguous-region"


> +#ifdef CONFIG_OF
> +static void cma_assign_device_from_dt(struct device *dev)
> +{
> +	struct device_node *node;
> +	struct cma *cma;
> +	u32 value;
> +
> +	node = of_parse_phandle(dev->of_node, "linux,contiguous-region", 0);
> +	if (!node)
> +		return;
> +	if (of_property_read_u32(node, "reg", &value) && !value)
> +		return;
> +	cma = cma_get_area(value);
> +	if (!cma)
> +		return;
> +
> +	dev_set_cma_area(dev, cma);
> +	pr_info("Assigned CMA region at %lx to %s device\n", (unsigned long)value, dev_name(dev));
> +}
> +

This scheme of associating devices with CMA regions by base does not 
work if you want to let CMA figure out where to place the region (base = 
0). Can we use the name to associate the device with the region? I had 
been working on something similar internally and that was the only 
solution I had come up with to associate arbitrary CMA nodes with devices.

Thanks,
Laura

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation

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

* [Linaro-mm-sig] [PATCH 2/2] drivers: dma-contiguous: add initialization from device tree
@ 2013-02-14 21:34       ` Laura Abbott
  0 siblings, 0 replies; 36+ messages in thread
From: Laura Abbott @ 2013-02-14 21:34 UTC (permalink / raw)
  To: linux-arm-kernel

Hi,


On 2/14/2013 4:45 AM, Marek Szyprowski wrote:
<snip>
> +name:		an name given to the defined region.
> +base-address:	the base address of the defined region.
> +size:		the size of the memory region.
> +linux,contiguous-region: property indicating that the defined memory
> +		region is used for contiguous memory allocations,
> +		Linux specific (optional)
> +linux,default-contiguous-region: property indicating that the region
> +		is the default region for all contiguous memory
> +		allocations, Linux specific (optional)
> +
> +

I don't see any code actually implementing the default-contiguous-region 
binding. Currently on ARM systems we will still setup the default region 
based on the Kconfig. Is this intentional?


> diff --git a/drivers/base/dma-contiguous.c b/drivers/base/dma-contiguous.c
> index 085389c..5761f73 100644
> --- a/drivers/base/dma-contiguous.c
> +++ b/drivers/base/dma-contiguous.c
> @@ -24,6 +24,9 @@
>
>   #include <linux/memblock.h>
>   #include <linux/err.h>
> +#include <linux/of.h>
> +#include <linux/of_fdt.h>
> +#include <linux/of_platform.h>
>   #include <linux/mm.h>
>   #include <linux/mutex.h>
>   #include <linux/page-isolation.h>
> @@ -177,6 +180,35 @@ no_mem:
>   	return ERR_PTR(ret);
>   }
>
> +/*****************************************************************************/
> +
> +#ifdef CONFIG_OF
> +int __init cma_fdt_scan(unsigned long node, const char *uname,
> +				int depth, void *data)
> +{
> +	phys_addr_t base, size;
> +	unsigned long len;
> +	__be32 *prop;
> +
> +	if (strncmp(uname, "region@", 7) != 0 || depth != 2 ||
> +	    !of_get_flat_dt_prop(node, "contiguous-region", NULL))

The documentation says "linux,contiguous-region"


> +#ifdef CONFIG_OF
> +static void cma_assign_device_from_dt(struct device *dev)
> +{
> +	struct device_node *node;
> +	struct cma *cma;
> +	u32 value;
> +
> +	node = of_parse_phandle(dev->of_node, "linux,contiguous-region", 0);
> +	if (!node)
> +		return;
> +	if (of_property_read_u32(node, "reg", &value) && !value)
> +		return;
> +	cma = cma_get_area(value);
> +	if (!cma)
> +		return;
> +
> +	dev_set_cma_area(dev, cma);
> +	pr_info("Assigned CMA region at %lx to %s device\n", (unsigned long)value, dev_name(dev));
> +}
> +

This scheme of associating devices with CMA regions by base does not 
work if you want to let CMA figure out where to place the region (base = 
0). Can we use the name to associate the device with the region? I had 
been working on something similar internally and that was the only 
solution I had come up with to associate arbitrary CMA nodes with devices.

Thanks,
Laura

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation

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

* Re: [PATCH 1/2] drivers: dma-contiguous: clean source code and prepare for device tree
  2013-02-14 12:45     ` Marek Szyprowski
@ 2013-02-14 21:37       ` Laura Abbott
  -1 siblings, 0 replies; 36+ messages in thread
From: Laura Abbott @ 2013-02-14 21:37 UTC (permalink / raw)
  To: Marek Szyprowski
  Cc: linaro-mm-sig, Kyungmin Park, devicetree-discuss,
	Michal Nazarewicz, linux-arm-kernel

Hi,

On 2/14/2013 4:45 AM, Marek Szyprowski wrote:
> This patch cleans the initialization of dma contiguous framework. The
> all-in-one dma_declare_contiguous() function is now separated into
> dma_contiguous_reserve_area() which only steals the the memory from
> memblock allocator and dma_contiguous_add_device() function, which
> assigns given device to the specified reserved memory area. This improves
> the flexibility in defining contiguous memory areas and assigning device
> to them, because now it is possible to assign more than one device to
> the given contiguous memory area. This split in initialization is also
> required for upcoming device tree support.
>
> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
> Acked-by: Kyungmin Park <kyungmin.park@samsung.com>
> ---
>   drivers/base/dma-contiguous.c        |  210 +++++++++++++++++++++-------------
>   include/asm-generic/dma-contiguous.h |    4 +-
>   include/linux/dma-contiguous.h       |   32 +++++-
>   3 files changed, 161 insertions(+), 85 deletions(-)
>
> diff --git a/drivers/base/dma-contiguous.c b/drivers/base/dma-contiguous.c
> index 0ca5442..085389c 100644
> --- a/drivers/base/dma-contiguous.c
> +++ b/drivers/base/dma-contiguous.c
> @@ -39,7 +39,33 @@ struct cma {
>   	unsigned long	*bitmap;
>   };
>
> -struct cma *dma_contiguous_default_area;
> +static DEFINE_MUTEX(cma_mutex);
> +
> +struct cma *dma_contiguous_def_area;
> +phys_addr_t dma_contiguous_def_base;
> +
> +static struct cma_area {
> +	phys_addr_t base;
> +	unsigned long size;
> +	struct cma *cma;
> +} cma_areas[MAX_CMA_AREAS] __initdata;
> +static unsigned cma_area_count __initdata;
> +

cma_areas and cma_area_count are accessed from cma_get_area which gets 
called from cma_assign_device_from_dt. You need to drop the __initdata 
since the notifier can be called at anytime.

Thanks,
Laura

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation

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

* [PATCH 1/2] drivers: dma-contiguous: clean source code and prepare for device tree
@ 2013-02-14 21:37       ` Laura Abbott
  0 siblings, 0 replies; 36+ messages in thread
From: Laura Abbott @ 2013-02-14 21:37 UTC (permalink / raw)
  To: linux-arm-kernel

Hi,

On 2/14/2013 4:45 AM, Marek Szyprowski wrote:
> This patch cleans the initialization of dma contiguous framework. The
> all-in-one dma_declare_contiguous() function is now separated into
> dma_contiguous_reserve_area() which only steals the the memory from
> memblock allocator and dma_contiguous_add_device() function, which
> assigns given device to the specified reserved memory area. This improves
> the flexibility in defining contiguous memory areas and assigning device
> to them, because now it is possible to assign more than one device to
> the given contiguous memory area. This split in initialization is also
> required for upcoming device tree support.
>
> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
> Acked-by: Kyungmin Park <kyungmin.park@samsung.com>
> ---
>   drivers/base/dma-contiguous.c        |  210 +++++++++++++++++++++-------------
>   include/asm-generic/dma-contiguous.h |    4 +-
>   include/linux/dma-contiguous.h       |   32 +++++-
>   3 files changed, 161 insertions(+), 85 deletions(-)
>
> diff --git a/drivers/base/dma-contiguous.c b/drivers/base/dma-contiguous.c
> index 0ca5442..085389c 100644
> --- a/drivers/base/dma-contiguous.c
> +++ b/drivers/base/dma-contiguous.c
> @@ -39,7 +39,33 @@ struct cma {
>   	unsigned long	*bitmap;
>   };
>
> -struct cma *dma_contiguous_default_area;
> +static DEFINE_MUTEX(cma_mutex);
> +
> +struct cma *dma_contiguous_def_area;
> +phys_addr_t dma_contiguous_def_base;
> +
> +static struct cma_area {
> +	phys_addr_t base;
> +	unsigned long size;
> +	struct cma *cma;
> +} cma_areas[MAX_CMA_AREAS] __initdata;
> +static unsigned cma_area_count __initdata;
> +

cma_areas and cma_area_count are accessed from cma_get_area which gets 
called from cma_assign_device_from_dt. You need to drop the __initdata 
since the notifier can be called at anytime.

Thanks,
Laura

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation

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

* Re: [PATCH 0/2] Device Tree support for CMA (Contiguous Memory Allocator)
  2013-02-14 21:30     ` Sascha Hauer
@ 2013-02-14 22:08         ` Sylwester Nawrocki
  -1 siblings, 0 replies; 36+ messages in thread
From: Sylwester Nawrocki @ 2013-02-14 22:08 UTC (permalink / raw)
  To: Sascha Hauer
  Cc: devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ, Michal Nazarewicz,
	linaro-mm-sig-cunTk1MwBs8s++Sfvej+rw, Kyungmin Park,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	Marek Szyprowski

Hi,

On 02/14/2013 10:30 PM, Sascha Hauer wrote:
> On Thu, Feb 14, 2013 at 01:45:26PM +0100, Marek Szyprowski wrote:
...
>> Here is my initial proposal for device tree integration for Contiguous
>> Memory Allocator. The code is quite straightforward, however I expect
>> that the memory bindings require some discussion.
>>
>> The proposed bindings allows to define contiguous memory regions of
>> specified base address and size. Then, the defined regions can be
>> assigned to the given device(s) by adding a property with a phanle to
>> the defined contiguous memory region. From the device tree perspective
>> that's all. Once the bindings are added, all the memory allocations from
>> dma-mapping subsystem will be served from the defined contiguous memory
>> regions.
>>
>
> I think CMA regions should not be described in the devicetre at all. The
> devicetree is about hardware description and it should be OS agnostic,
> but CMA is only a Linux specific implementation detail. It's not even
> specific to a particular board, it's specific to a particular usecase of
> a board.

I disagree. For example, in a multiprocessor system describing the memory
regions this way allows to assign memory to each subsystem, e.g. shared
memory, so that the memory region constraints are satisfied.

CMA just happens to be an implementation of a method of assigning memory
to each device in Linux. The constraints on the memory are real hardware
constraints, resulting from a particular subsystem architecture.

---

Thanks,
Sylwester

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

* [PATCH 0/2] Device Tree support for CMA (Contiguous Memory Allocator)
@ 2013-02-14 22:08         ` Sylwester Nawrocki
  0 siblings, 0 replies; 36+ messages in thread
From: Sylwester Nawrocki @ 2013-02-14 22:08 UTC (permalink / raw)
  To: linux-arm-kernel

Hi,

On 02/14/2013 10:30 PM, Sascha Hauer wrote:
> On Thu, Feb 14, 2013 at 01:45:26PM +0100, Marek Szyprowski wrote:
...
>> Here is my initial proposal for device tree integration for Contiguous
>> Memory Allocator. The code is quite straightforward, however I expect
>> that the memory bindings require some discussion.
>>
>> The proposed bindings allows to define contiguous memory regions of
>> specified base address and size. Then, the defined regions can be
>> assigned to the given device(s) by adding a property with a phanle to
>> the defined contiguous memory region. From the device tree perspective
>> that's all. Once the bindings are added, all the memory allocations from
>> dma-mapping subsystem will be served from the defined contiguous memory
>> regions.
>>
>
> I think CMA regions should not be described in the devicetre at all. The
> devicetree is about hardware description and it should be OS agnostic,
> but CMA is only a Linux specific implementation detail. It's not even
> specific to a particular board, it's specific to a particular usecase of
> a board.

I disagree. For example, in a multiprocessor system describing the memory
regions this way allows to assign memory to each subsystem, e.g. shared
memory, so that the memory region constraints are satisfied.

CMA just happens to be an implementation of a method of assigning memory
to each device in Linux. The constraints on the memory are real hardware
constraints, resulting from a particular subsystem architecture.

---

Thanks,
Sylwester

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

* Re: [PATCH 0/2] Device Tree support for CMA (Contiguous Memory Allocator)
  2013-02-14 22:08         ` Sylwester Nawrocki
@ 2013-02-15  8:33             ` Sascha Hauer
  -1 siblings, 0 replies; 36+ messages in thread
From: Sascha Hauer @ 2013-02-15  8:33 UTC (permalink / raw)
  To: Sylwester Nawrocki
  Cc: devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ, Michal Nazarewicz,
	linaro-mm-sig-cunTk1MwBs8s++Sfvej+rw, Kyungmin Park,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	Marek Szyprowski

On Thu, Feb 14, 2013 at 11:08:54PM +0100, Sylwester Nawrocki wrote:
> Hi,
> 
> On 02/14/2013 10:30 PM, Sascha Hauer wrote:
> >On Thu, Feb 14, 2013 at 01:45:26PM +0100, Marek Szyprowski wrote:
> ...
> >>Here is my initial proposal for device tree integration for Contiguous
> >>Memory Allocator. The code is quite straightforward, however I expect
> >>that the memory bindings require some discussion.
> >>
> >>The proposed bindings allows to define contiguous memory regions of
> >>specified base address and size. Then, the defined regions can be
> >>assigned to the given device(s) by adding a property with a phanle to
> >>the defined contiguous memory region. From the device tree perspective
> >>that's all. Once the bindings are added, all the memory allocations from
> >>dma-mapping subsystem will be served from the defined contiguous memory
> >>regions.
> >>
> >
> >I think CMA regions should not be described in the devicetre at all. The
> >devicetree is about hardware description and it should be OS agnostic,
> >but CMA is only a Linux specific implementation detail. It's not even
> >specific to a particular board, it's specific to a particular usecase of
> >a board.
> 
> I disagree. For example, in a multiprocessor system describing the memory
> regions this way allows to assign memory to each subsystem, e.g. shared
> memory, so that the memory region constraints are satisfied.
> 
> CMA just happens to be an implementation of a method of assigning memory
> to each device in Linux. The constraints on the memory are real hardware
> constraints, resulting from a particular subsystem architecture.

If you are talking about DMA controllers which can only access a certain
memory area, then yes, that's a hardware constraint, I'm not sure though
if describing this as CMA in the devicetree is the way to go.

If you are talking about 'on this board I want to have 128MiB for this
device because I'm doing 1080p while on another board 64MiB are enough
because I'm doing 720p', then this is not a hardware constraint.

There may be valid scenarios for putting CMA into the devicetrees, but
doing this also opens the door for abuse of this binding. I for once
don't want to find areas being allocated for CMA in the devicetree for
devices I don't care about. I know I can always exchange a devicetree,
but I think the devicetree should be seen as firmware to a certain
degree.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

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

* [PATCH 0/2] Device Tree support for CMA (Contiguous Memory Allocator)
@ 2013-02-15  8:33             ` Sascha Hauer
  0 siblings, 0 replies; 36+ messages in thread
From: Sascha Hauer @ 2013-02-15  8:33 UTC (permalink / raw)
  To: linux-arm-kernel

On Thu, Feb 14, 2013 at 11:08:54PM +0100, Sylwester Nawrocki wrote:
> Hi,
> 
> On 02/14/2013 10:30 PM, Sascha Hauer wrote:
> >On Thu, Feb 14, 2013 at 01:45:26PM +0100, Marek Szyprowski wrote:
> ...
> >>Here is my initial proposal for device tree integration for Contiguous
> >>Memory Allocator. The code is quite straightforward, however I expect
> >>that the memory bindings require some discussion.
> >>
> >>The proposed bindings allows to define contiguous memory regions of
> >>specified base address and size. Then, the defined regions can be
> >>assigned to the given device(s) by adding a property with a phanle to
> >>the defined contiguous memory region. From the device tree perspective
> >>that's all. Once the bindings are added, all the memory allocations from
> >>dma-mapping subsystem will be served from the defined contiguous memory
> >>regions.
> >>
> >
> >I think CMA regions should not be described in the devicetre at all. The
> >devicetree is about hardware description and it should be OS agnostic,
> >but CMA is only a Linux specific implementation detail. It's not even
> >specific to a particular board, it's specific to a particular usecase of
> >a board.
> 
> I disagree. For example, in a multiprocessor system describing the memory
> regions this way allows to assign memory to each subsystem, e.g. shared
> memory, so that the memory region constraints are satisfied.
> 
> CMA just happens to be an implementation of a method of assigning memory
> to each device in Linux. The constraints on the memory are real hardware
> constraints, resulting from a particular subsystem architecture.

If you are talking about DMA controllers which can only access a certain
memory area, then yes, that's a hardware constraint, I'm not sure though
if describing this as CMA in the devicetree is the way to go.

If you are talking about 'on this board I want to have 128MiB for this
device because I'm doing 1080p while on another board 64MiB are enough
because I'm doing 720p', then this is not a hardware constraint.

There may be valid scenarios for putting CMA into the devicetrees, but
doing this also opens the door for abuse of this binding. I for once
don't want to find areas being allocated for CMA in the devicetree for
devices I don't care about. I know I can always exchange a devicetree,
but I think the devicetree should be seen as firmware to a certain
degree.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

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

* Re: [Linaro-mm-sig] [PATCH 2/2] drivers: dma-contiguous: add initialization from device tree
  2013-02-14 21:34       ` Laura Abbott
@ 2013-02-15 16:12         ` Nishanth Peethambaran
  -1 siblings, 0 replies; 36+ messages in thread
From: Nishanth Peethambaran @ 2013-02-15 16:12 UTC (permalink / raw)
  To: Laura Abbott
  Cc: linaro-mm-sig, Arnd Bergmann, Tomasz Figa, Michal Nazarewicz,
	Grant Likely, Kyungmin Park, devicetree-discuss,
	linux-arm-kernel, Marek Szyprowski

On Fri, Feb 15, 2013 at 3:04 AM, Laura Abbott <lauraa@codeaurora.org> wrote:
> Hi,
>
>
> On 2/14/2013 4:45 AM, Marek Szyprowski wrote:
> <snip>
>
>> +name:          an name given to the defined region.
>> +base-address:  the base address of the defined region.
>> +size:          the size of the memory region.
>> +linux,contiguous-region: property indicating that the defined memory
>> +               region is used for contiguous memory allocations,
>> +               Linux specific (optional)
>> +linux,default-contiguous-region: property indicating that the region
>> +               is the default region for all contiguous memory
>> +               allocations, Linux specific (optional)
>> +
>> +
>
>
> I don't see any code actually implementing the default-contiguous-region
> binding. Currently on ARM systems we will still setup the default region
> based on the Kconfig. Is this intentional?
>
>
>
>> diff --git a/drivers/base/dma-contiguous.c b/drivers/base/dma-contiguous.c
>> index 085389c..5761f73 100644
>> --- a/drivers/base/dma-contiguous.c
>> +++ b/drivers/base/dma-contiguous.c
>> @@ -24,6 +24,9 @@
>>
>>   #include <linux/memblock.h>
>>   #include <linux/err.h>
>> +#include <linux/of.h>
>> +#include <linux/of_fdt.h>
>> +#include <linux/of_platform.h>
>>   #include <linux/mm.h>
>>   #include <linux/mutex.h>
>>   #include <linux/page-isolation.h>
>> @@ -177,6 +180,35 @@ no_mem:
>>         return ERR_PTR(ret);
>>   }
>>
>>
>> +/*****************************************************************************/
>> +
>> +#ifdef CONFIG_OF
>> +int __init cma_fdt_scan(unsigned long node, const char *uname,
>> +                               int depth, void *data)
>> +{
>> +       phys_addr_t base, size;
>> +       unsigned long len;
>> +       __be32 *prop;
>> +
>> +       if (strncmp(uname, "region@", 7) != 0 || depth != 2 ||
>> +           !of_get_flat_dt_prop(node, "contiguous-region", NULL))
>
>
> The documentation says "linux,contiguous-region"
>
>
>
>> +#ifdef CONFIG_OF
>> +static void cma_assign_device_from_dt(struct device *dev)
>> +{
>> +       struct device_node *node;
>> +       struct cma *cma;
>> +       u32 value;
>> +
>> +       node = of_parse_phandle(dev->of_node, "linux,contiguous-region",
>> 0);
>> +       if (!node)
>> +               return;
>> +       if (of_property_read_u32(node, "reg", &value) && !value)
>> +               return;
>> +       cma = cma_get_area(value);
>> +       if (!cma)
>> +               return;
>> +
>> +       dev_set_cma_area(dev, cma);
>> +       pr_info("Assigned CMA region at %lx to %s device\n", (unsigned
>> long)value, dev_name(dev));
>> +}
>> +
>
>
> This scheme of associating devices with CMA regions by base does not work if
> you want to let CMA figure out where to place the region (base = 0). Can we
> use the name to associate the device with the region? I had been working on
> something similar internally and that was the only solution I had come up
> with to associate arbitrary CMA nodes with devices.
>

The phandle for the region can also be used as a unique identifier.
cma_fdt_scan() can get the property(own phandle) and pass as an extra
parameter to dma_contiguous_reserve_area().
This could be stored as part of cma_area (extra field).
The devices get the cma area by passing the phandle to cma_get_area()
which should be matching criteria.

For non-DT cases, board file will have to assign a unique id while calling the
dma_declare_contiguous()

> Thanks,
> Laura
>
> --
> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
> hosted by The Linux Foundation
>
>
> _______________________________________________
> Linaro-mm-sig mailing list
> Linaro-mm-sig@lists.linaro.org
> http://lists.linaro.org/mailman/listinfo/linaro-mm-sig



- Nishanth Peethambaran
  +91-9448074166

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

* [Linaro-mm-sig] [PATCH 2/2] drivers: dma-contiguous: add initialization from device tree
@ 2013-02-15 16:12         ` Nishanth Peethambaran
  0 siblings, 0 replies; 36+ messages in thread
From: Nishanth Peethambaran @ 2013-02-15 16:12 UTC (permalink / raw)
  To: linux-arm-kernel

On Fri, Feb 15, 2013 at 3:04 AM, Laura Abbott <lauraa@codeaurora.org> wrote:
> Hi,
>
>
> On 2/14/2013 4:45 AM, Marek Szyprowski wrote:
> <snip>
>
>> +name:          an name given to the defined region.
>> +base-address:  the base address of the defined region.
>> +size:          the size of the memory region.
>> +linux,contiguous-region: property indicating that the defined memory
>> +               region is used for contiguous memory allocations,
>> +               Linux specific (optional)
>> +linux,default-contiguous-region: property indicating that the region
>> +               is the default region for all contiguous memory
>> +               allocations, Linux specific (optional)
>> +
>> +
>
>
> I don't see any code actually implementing the default-contiguous-region
> binding. Currently on ARM systems we will still setup the default region
> based on the Kconfig. Is this intentional?
>
>
>
>> diff --git a/drivers/base/dma-contiguous.c b/drivers/base/dma-contiguous.c
>> index 085389c..5761f73 100644
>> --- a/drivers/base/dma-contiguous.c
>> +++ b/drivers/base/dma-contiguous.c
>> @@ -24,6 +24,9 @@
>>
>>   #include <linux/memblock.h>
>>   #include <linux/err.h>
>> +#include <linux/of.h>
>> +#include <linux/of_fdt.h>
>> +#include <linux/of_platform.h>
>>   #include <linux/mm.h>
>>   #include <linux/mutex.h>
>>   #include <linux/page-isolation.h>
>> @@ -177,6 +180,35 @@ no_mem:
>>         return ERR_PTR(ret);
>>   }
>>
>>
>> +/*****************************************************************************/
>> +
>> +#ifdef CONFIG_OF
>> +int __init cma_fdt_scan(unsigned long node, const char *uname,
>> +                               int depth, void *data)
>> +{
>> +       phys_addr_t base, size;
>> +       unsigned long len;
>> +       __be32 *prop;
>> +
>> +       if (strncmp(uname, "region@", 7) != 0 || depth != 2 ||
>> +           !of_get_flat_dt_prop(node, "contiguous-region", NULL))
>
>
> The documentation says "linux,contiguous-region"
>
>
>
>> +#ifdef CONFIG_OF
>> +static void cma_assign_device_from_dt(struct device *dev)
>> +{
>> +       struct device_node *node;
>> +       struct cma *cma;
>> +       u32 value;
>> +
>> +       node = of_parse_phandle(dev->of_node, "linux,contiguous-region",
>> 0);
>> +       if (!node)
>> +               return;
>> +       if (of_property_read_u32(node, "reg", &value) && !value)
>> +               return;
>> +       cma = cma_get_area(value);
>> +       if (!cma)
>> +               return;
>> +
>> +       dev_set_cma_area(dev, cma);
>> +       pr_info("Assigned CMA region at %lx to %s device\n", (unsigned
>> long)value, dev_name(dev));
>> +}
>> +
>
>
> This scheme of associating devices with CMA regions by base does not work if
> you want to let CMA figure out where to place the region (base = 0). Can we
> use the name to associate the device with the region? I had been working on
> something similar internally and that was the only solution I had come up
> with to associate arbitrary CMA nodes with devices.
>

The phandle for the region can also be used as a unique identifier.
cma_fdt_scan() can get the property(own phandle) and pass as an extra
parameter to dma_contiguous_reserve_area().
This could be stored as part of cma_area (extra field).
The devices get the cma area by passing the phandle to cma_get_area()
which should be matching criteria.

For non-DT cases, board file will have to assign a unique id while calling the
dma_declare_contiguous()

> Thanks,
> Laura
>
> --
> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
> hosted by The Linux Foundation
>
>
> _______________________________________________
> Linaro-mm-sig mailing list
> Linaro-mm-sig at lists.linaro.org
> http://lists.linaro.org/mailman/listinfo/linaro-mm-sig



- Nishanth Peethambaran
  +91-9448074166

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

* Re: [PATCH 0/2] Device Tree support for CMA (Contiguous Memory Allocator)
  2013-02-15  8:33             ` Sascha Hauer
@ 2013-02-15 16:24                 ` Rob Herring
  -1 siblings, 0 replies; 36+ messages in thread
From: Rob Herring @ 2013-02-15 16:24 UTC (permalink / raw)
  To: Sascha Hauer
  Cc: devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ, Michal Nazarewicz,
	linaro-mm-sig-cunTk1MwBs8s++Sfvej+rw, Kyungmin Park,
	Sylwester Nawrocki,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	Marek Szyprowski

On 02/15/2013 02:33 AM, Sascha Hauer wrote:
> On Thu, Feb 14, 2013 at 11:08:54PM +0100, Sylwester Nawrocki wrote:
>> Hi,
>>
>> On 02/14/2013 10:30 PM, Sascha Hauer wrote:
>>> On Thu, Feb 14, 2013 at 01:45:26PM +0100, Marek Szyprowski wrote:
>> ...
>>>> Here is my initial proposal for device tree integration for Contiguous
>>>> Memory Allocator. The code is quite straightforward, however I expect
>>>> that the memory bindings require some discussion.
>>>>
>>>> The proposed bindings allows to define contiguous memory regions of
>>>> specified base address and size. Then, the defined regions can be
>>>> assigned to the given device(s) by adding a property with a phanle to
>>>> the defined contiguous memory region. From the device tree perspective
>>>> that's all. Once the bindings are added, all the memory allocations from
>>>> dma-mapping subsystem will be served from the defined contiguous memory
>>>> regions.
>>>>
>>>
>>> I think CMA regions should not be described in the devicetre at all. The
>>> devicetree is about hardware description and it should be OS agnostic,
>>> but CMA is only a Linux specific implementation detail. It's not even
>>> specific to a particular board, it's specific to a particular usecase of
>>> a board.
>>
>> I disagree. For example, in a multiprocessor system describing the memory
>> regions this way allows to assign memory to each subsystem, e.g. shared
>> memory, so that the memory region constraints are satisfied.
>>
>> CMA just happens to be an implementation of a method of assigning memory
>> to each device in Linux. The constraints on the memory are real hardware
>> constraints, resulting from a particular subsystem architecture.
> 
> If you are talking about DMA controllers which can only access a certain
> memory area, then yes, that's a hardware constraint, I'm not sure though
> if describing this as CMA in the devicetree is the way to go.
> 
> If you are talking about 'on this board I want to have 128MiB for this
> device because I'm doing 1080p while on another board 64MiB are enough
> because I'm doing 720p', then this is not a hardware constraint.
> 
> There may be valid scenarios for putting CMA into the devicetrees, but
> doing this also opens the door for abuse of this binding. I for once
> don't want to find areas being allocated for CMA in the devicetree for
> devices I don't care about. I know I can always exchange a devicetree,
> but I think the devicetree should be seen as firmware to a certain
> degree.

I agree this does not belong in DT. As a kernel developer, the DT comes
from firmware. Can the firmware author decide how much CMA memory is
needed? I don't think so.

I would suggest a kernel command line parameter instead if that does not
already exist.

Rob

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

* [PATCH 0/2] Device Tree support for CMA (Contiguous Memory Allocator)
@ 2013-02-15 16:24                 ` Rob Herring
  0 siblings, 0 replies; 36+ messages in thread
From: Rob Herring @ 2013-02-15 16:24 UTC (permalink / raw)
  To: linux-arm-kernel

On 02/15/2013 02:33 AM, Sascha Hauer wrote:
> On Thu, Feb 14, 2013 at 11:08:54PM +0100, Sylwester Nawrocki wrote:
>> Hi,
>>
>> On 02/14/2013 10:30 PM, Sascha Hauer wrote:
>>> On Thu, Feb 14, 2013 at 01:45:26PM +0100, Marek Szyprowski wrote:
>> ...
>>>> Here is my initial proposal for device tree integration for Contiguous
>>>> Memory Allocator. The code is quite straightforward, however I expect
>>>> that the memory bindings require some discussion.
>>>>
>>>> The proposed bindings allows to define contiguous memory regions of
>>>> specified base address and size. Then, the defined regions can be
>>>> assigned to the given device(s) by adding a property with a phanle to
>>>> the defined contiguous memory region. From the device tree perspective
>>>> that's all. Once the bindings are added, all the memory allocations from
>>>> dma-mapping subsystem will be served from the defined contiguous memory
>>>> regions.
>>>>
>>>
>>> I think CMA regions should not be described in the devicetre at all. The
>>> devicetree is about hardware description and it should be OS agnostic,
>>> but CMA is only a Linux specific implementation detail. It's not even
>>> specific to a particular board, it's specific to a particular usecase of
>>> a board.
>>
>> I disagree. For example, in a multiprocessor system describing the memory
>> regions this way allows to assign memory to each subsystem, e.g. shared
>> memory, so that the memory region constraints are satisfied.
>>
>> CMA just happens to be an implementation of a method of assigning memory
>> to each device in Linux. The constraints on the memory are real hardware
>> constraints, resulting from a particular subsystem architecture.
> 
> If you are talking about DMA controllers which can only access a certain
> memory area, then yes, that's a hardware constraint, I'm not sure though
> if describing this as CMA in the devicetree is the way to go.
> 
> If you are talking about 'on this board I want to have 128MiB for this
> device because I'm doing 1080p while on another board 64MiB are enough
> because I'm doing 720p', then this is not a hardware constraint.
> 
> There may be valid scenarios for putting CMA into the devicetrees, but
> doing this also opens the door for abuse of this binding. I for once
> don't want to find areas being allocated for CMA in the devicetree for
> devices I don't care about. I know I can always exchange a devicetree,
> but I think the devicetree should be seen as firmware to a certain
> degree.

I agree this does not belong in DT. As a kernel developer, the DT comes
from firmware. Can the firmware author decide how much CMA memory is
needed? I don't think so.

I would suggest a kernel command line parameter instead if that does not
already exist.

Rob

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

* Re: [Linaro-mm-sig] [PATCH 0/2] Device Tree support for CMA (Contiguous Memory Allocator)
  2013-02-15 16:24                 ` Rob Herring
@ 2013-02-17  5:18                   ` Nishanth Peethambaran
  -1 siblings, 0 replies; 36+ messages in thread
From: Nishanth Peethambaran @ 2013-02-17  5:18 UTC (permalink / raw)
  To: Rob Herring
  Cc: Sascha Hauer, Michal Nazarewicz, linaro-mm-sig, Kyungmin Park,
	Sylwester Nawrocki, devicetree-discuss, linux-arm-kernel

Limiting the scope of the product to a particular use-case (720p or
1080p video playback) will be part of firmware mostly OTP-ed in the
SoC. Firmware author could decide the amount of CMA memory needed and
put this as part of DT.
Number of CMA areas and linking them to devices could be done in DT
and allows using the same kernel for any of the combinations.

- Nishanth Peethambaran
  +91-9448074166



On Fri, Feb 15, 2013 at 9:54 PM, Rob Herring <robherring2@gmail.com> wrote:
> On 02/15/2013 02:33 AM, Sascha Hauer wrote:
>> On Thu, Feb 14, 2013 at 11:08:54PM +0100, Sylwester Nawrocki wrote:
>>> Hi,
>>>
>>> On 02/14/2013 10:30 PM, Sascha Hauer wrote:
>>>> On Thu, Feb 14, 2013 at 01:45:26PM +0100, Marek Szyprowski wrote:
>>> ...
>>>>> Here is my initial proposal for device tree integration for Contiguous
>>>>> Memory Allocator. The code is quite straightforward, however I expect
>>>>> that the memory bindings require some discussion.
>>>>>
>>>>> The proposed bindings allows to define contiguous memory regions of
>>>>> specified base address and size. Then, the defined regions can be
>>>>> assigned to the given device(s) by adding a property with a phanle to
>>>>> the defined contiguous memory region. From the device tree perspective
>>>>> that's all. Once the bindings are added, all the memory allocations from
>>>>> dma-mapping subsystem will be served from the defined contiguous memory
>>>>> regions.
>>>>>
>>>>
>>>> I think CMA regions should not be described in the devicetre at all. The
>>>> devicetree is about hardware description and it should be OS agnostic,
>>>> but CMA is only a Linux specific implementation detail. It's not even
>>>> specific to a particular board, it's specific to a particular usecase of
>>>> a board.
>>>
>>> I disagree. For example, in a multiprocessor system describing the memory
>>> regions this way allows to assign memory to each subsystem, e.g. shared
>>> memory, so that the memory region constraints are satisfied.
>>>
>>> CMA just happens to be an implementation of a method of assigning memory
>>> to each device in Linux. The constraints on the memory are real hardware
>>> constraints, resulting from a particular subsystem architecture.
>>
>> If you are talking about DMA controllers which can only access a certain
>> memory area, then yes, that's a hardware constraint, I'm not sure though
>> if describing this as CMA in the devicetree is the way to go.
>>
>> If you are talking about 'on this board I want to have 128MiB for this
>> device because I'm doing 1080p while on another board 64MiB are enough
>> because I'm doing 720p', then this is not a hardware constraint.
>>
>> There may be valid scenarios for putting CMA into the devicetrees, but
>> doing this also opens the door for abuse of this binding. I for once
>> don't want to find areas being allocated for CMA in the devicetree for
>> devices I don't care about. I know I can always exchange a devicetree,
>> but I think the devicetree should be seen as firmware to a certain
>> degree.
>
> I agree this does not belong in DT. As a kernel developer, the DT comes
> from firmware. Can the firmware author decide how much CMA memory is
> needed? I don't think so.
>
> I would suggest a kernel command line parameter instead if that does not
> already exist.
>
> Rob
>
> _______________________________________________
> Linaro-mm-sig mailing list
> Linaro-mm-sig@lists.linaro.org
> http://lists.linaro.org/mailman/listinfo/linaro-mm-sig

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

* [Linaro-mm-sig] [PATCH 0/2] Device Tree support for CMA (Contiguous Memory Allocator)
@ 2013-02-17  5:18                   ` Nishanth Peethambaran
  0 siblings, 0 replies; 36+ messages in thread
From: Nishanth Peethambaran @ 2013-02-17  5:18 UTC (permalink / raw)
  To: linux-arm-kernel

Limiting the scope of the product to a particular use-case (720p or
1080p video playback) will be part of firmware mostly OTP-ed in the
SoC. Firmware author could decide the amount of CMA memory needed and
put this as part of DT.
Number of CMA areas and linking them to devices could be done in DT
and allows using the same kernel for any of the combinations.

- Nishanth Peethambaran
  +91-9448074166



On Fri, Feb 15, 2013 at 9:54 PM, Rob Herring <robherring2@gmail.com> wrote:
> On 02/15/2013 02:33 AM, Sascha Hauer wrote:
>> On Thu, Feb 14, 2013 at 11:08:54PM +0100, Sylwester Nawrocki wrote:
>>> Hi,
>>>
>>> On 02/14/2013 10:30 PM, Sascha Hauer wrote:
>>>> On Thu, Feb 14, 2013 at 01:45:26PM +0100, Marek Szyprowski wrote:
>>> ...
>>>>> Here is my initial proposal for device tree integration for Contiguous
>>>>> Memory Allocator. The code is quite straightforward, however I expect
>>>>> that the memory bindings require some discussion.
>>>>>
>>>>> The proposed bindings allows to define contiguous memory regions of
>>>>> specified base address and size. Then, the defined regions can be
>>>>> assigned to the given device(s) by adding a property with a phanle to
>>>>> the defined contiguous memory region. From the device tree perspective
>>>>> that's all. Once the bindings are added, all the memory allocations from
>>>>> dma-mapping subsystem will be served from the defined contiguous memory
>>>>> regions.
>>>>>
>>>>
>>>> I think CMA regions should not be described in the devicetre at all. The
>>>> devicetree is about hardware description and it should be OS agnostic,
>>>> but CMA is only a Linux specific implementation detail. It's not even
>>>> specific to a particular board, it's specific to a particular usecase of
>>>> a board.
>>>
>>> I disagree. For example, in a multiprocessor system describing the memory
>>> regions this way allows to assign memory to each subsystem, e.g. shared
>>> memory, so that the memory region constraints are satisfied.
>>>
>>> CMA just happens to be an implementation of a method of assigning memory
>>> to each device in Linux. The constraints on the memory are real hardware
>>> constraints, resulting from a particular subsystem architecture.
>>
>> If you are talking about DMA controllers which can only access a certain
>> memory area, then yes, that's a hardware constraint, I'm not sure though
>> if describing this as CMA in the devicetree is the way to go.
>>
>> If you are talking about 'on this board I want to have 128MiB for this
>> device because I'm doing 1080p while on another board 64MiB are enough
>> because I'm doing 720p', then this is not a hardware constraint.
>>
>> There may be valid scenarios for putting CMA into the devicetrees, but
>> doing this also opens the door for abuse of this binding. I for once
>> don't want to find areas being allocated for CMA in the devicetree for
>> devices I don't care about. I know I can always exchange a devicetree,
>> but I think the devicetree should be seen as firmware to a certain
>> degree.
>
> I agree this does not belong in DT. As a kernel developer, the DT comes
> from firmware. Can the firmware author decide how much CMA memory is
> needed? I don't think so.
>
> I would suggest a kernel command line parameter instead if that does not
> already exist.
>
> Rob
>
> _______________________________________________
> Linaro-mm-sig mailing list
> Linaro-mm-sig at lists.linaro.org
> http://lists.linaro.org/mailman/listinfo/linaro-mm-sig

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

* Re: [Linaro-mm-sig] [PATCH 0/2] Device Tree support for CMA (Contiguous Memory Allocator)
  2013-02-17  5:18                   ` Nishanth Peethambaran
@ 2013-02-18 21:58                       ` Rob Herring
  -1 siblings, 0 replies; 36+ messages in thread
From: Rob Herring @ 2013-02-18 21:58 UTC (permalink / raw)
  To: Nishanth Peethambaran
  Cc: Sascha Hauer, Michal Nazarewicz,
	linaro-mm-sig-cunTk1MwBs8s++Sfvej+rw, Kyungmin Park,
	Sylwester Nawrocki, devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r

On 02/16/2013 11:18 PM, Nishanth Peethambaran wrote:
> Limiting the scope of the product to a particular use-case (720p or
> 1080p video playback) will be part of firmware mostly OTP-ed in the
> SoC. Firmware author could decide the amount of CMA memory needed and
> put this as part of DT.

What kernel developer would want a firmware author deciding how much
memory is needed? It is not a decision that can be made without
knowledge of the OS. While the frame size could be determined easily
enough based on resolution, the number of buffers needed cannot.

Rob

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

* [Linaro-mm-sig] [PATCH 0/2] Device Tree support for CMA (Contiguous Memory Allocator)
@ 2013-02-18 21:58                       ` Rob Herring
  0 siblings, 0 replies; 36+ messages in thread
From: Rob Herring @ 2013-02-18 21:58 UTC (permalink / raw)
  To: linux-arm-kernel

On 02/16/2013 11:18 PM, Nishanth Peethambaran wrote:
> Limiting the scope of the product to a particular use-case (720p or
> 1080p video playback) will be part of firmware mostly OTP-ed in the
> SoC. Firmware author could decide the amount of CMA memory needed and
> put this as part of DT.

What kernel developer would want a firmware author deciding how much
memory is needed? It is not a decision that can be made without
knowledge of the OS. While the frame size could be determined easily
enough based on resolution, the number of buffers needed cannot.

Rob

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

* Re: [PATCH 0/2] Device Tree support for CMA (Contiguous Memory Allocator)
  2013-02-15  8:33             ` Sascha Hauer
@ 2013-02-18 22:25                 ` Sylwester Nawrocki
  -1 siblings, 0 replies; 36+ messages in thread
From: Sylwester Nawrocki @ 2013-02-18 22:25 UTC (permalink / raw)
  To: Sascha Hauer
  Cc: devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ, Michal Nazarewicz,
	linaro-mm-sig-cunTk1MwBs8s++Sfvej+rw, Kyungmin Park,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	Marek Szyprowski

On 02/15/2013 09:33 AM, Sascha Hauer wrote:
> On Thu, Feb 14, 2013 at 11:08:54PM +0100, Sylwester Nawrocki wrote:
>> On 02/14/2013 10:30 PM, Sascha Hauer wrote:
>>> On Thu, Feb 14, 2013 at 01:45:26PM +0100, Marek Szyprowski wrote:
>> ...
>>>> Here is my initial proposal for device tree integration for Contiguous
>>>> Memory Allocator. The code is quite straightforward, however I expect
>>>> that the memory bindings require some discussion.
>>>>
>>>> The proposed bindings allows to define contiguous memory regions of
>>>> specified base address and size. Then, the defined regions can be
>>>> assigned to the given device(s) by adding a property with a phanle to
>>>> the defined contiguous memory region. From the device tree perspective
>>>> that's all. Once the bindings are added, all the memory allocations from
>>>> dma-mapping subsystem will be served from the defined contiguous memory
>>>> regions.
>>>>
>>>
>>> I think CMA regions should not be described in the devicetre at all. The
>>> devicetree is about hardware description and it should be OS agnostic,
>>> but CMA is only a Linux specific implementation detail. It's not even
>>> specific to a particular board, it's specific to a particular usecase of
>>> a board.
>>
>> I disagree. For example, in a multiprocessor system describing the memory
>> regions this way allows to assign memory to each subsystem, e.g. shared
>> memory, so that the memory region constraints are satisfied.
>>
>> CMA just happens to be an implementation of a method of assigning memory
>> to each device in Linux. The constraints on the memory are real hardware
>> constraints, resulting from a particular subsystem architecture.
>
> If you are talking about DMA controllers which can only access a certain
> memory area, then yes, that's a hardware constraint, I'm not sure though
> if describing this as CMA in the devicetree is the way to go.

My context was a multiprocessor SoC, where one of the processors is somehow
application specific (e.g. camera ISP) and can be considered as a slave
subsystem, running it's firmware and sharing system memory with the main
processor.

The slave subsystem can have insane constraints on the memory region where
its firmware is located and which it also uses as its generic purpose RAM.

While the region assigned to such a slave device would also likely include
a memory for its DMA buffers, the main concern here was an allocation of
a working memory for the slave processor.

Perhaps the device tree is not a perfect place for defining exact memory
regions. If we happen to decide against it, then I guess some set of
properties would be needed, that would allow CMA to assign appropriate
memory region(s) to a device.

> If you are talking about 'on this board I want to have 128MiB for this
> device because I'm doing 1080p while on another board 64MiB are enough
> because I'm doing 720p', then this is not a hardware constraint.

No, that's not something I was referring to.

> There may be valid scenarios for putting CMA into the devicetrees, but
> doing this also opens the door for abuse of this binding. I for once
> don't want to find areas being allocated for CMA in the devicetree for
> devices I don't care about. I know I can always exchange a devicetree,
> but I think the device tree should be seen as firmware to a certain
> degree.

I agree. You're likely aware of it, I just want to remind that the CMA
memory can be used by the OS in a normal way, until a device driver
actually allocates and uses it.

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

* [PATCH 0/2] Device Tree support for CMA (Contiguous Memory Allocator)
@ 2013-02-18 22:25                 ` Sylwester Nawrocki
  0 siblings, 0 replies; 36+ messages in thread
From: Sylwester Nawrocki @ 2013-02-18 22:25 UTC (permalink / raw)
  To: linux-arm-kernel

On 02/15/2013 09:33 AM, Sascha Hauer wrote:
> On Thu, Feb 14, 2013 at 11:08:54PM +0100, Sylwester Nawrocki wrote:
>> On 02/14/2013 10:30 PM, Sascha Hauer wrote:
>>> On Thu, Feb 14, 2013 at 01:45:26PM +0100, Marek Szyprowski wrote:
>> ...
>>>> Here is my initial proposal for device tree integration for Contiguous
>>>> Memory Allocator. The code is quite straightforward, however I expect
>>>> that the memory bindings require some discussion.
>>>>
>>>> The proposed bindings allows to define contiguous memory regions of
>>>> specified base address and size. Then, the defined regions can be
>>>> assigned to the given device(s) by adding a property with a phanle to
>>>> the defined contiguous memory region. From the device tree perspective
>>>> that's all. Once the bindings are added, all the memory allocations from
>>>> dma-mapping subsystem will be served from the defined contiguous memory
>>>> regions.
>>>>
>>>
>>> I think CMA regions should not be described in the devicetre at all. The
>>> devicetree is about hardware description and it should be OS agnostic,
>>> but CMA is only a Linux specific implementation detail. It's not even
>>> specific to a particular board, it's specific to a particular usecase of
>>> a board.
>>
>> I disagree. For example, in a multiprocessor system describing the memory
>> regions this way allows to assign memory to each subsystem, e.g. shared
>> memory, so that the memory region constraints are satisfied.
>>
>> CMA just happens to be an implementation of a method of assigning memory
>> to each device in Linux. The constraints on the memory are real hardware
>> constraints, resulting from a particular subsystem architecture.
>
> If you are talking about DMA controllers which can only access a certain
> memory area, then yes, that's a hardware constraint, I'm not sure though
> if describing this as CMA in the devicetree is the way to go.

My context was a multiprocessor SoC, where one of the processors is somehow
application specific (e.g. camera ISP) and can be considered as a slave
subsystem, running it's firmware and sharing system memory with the main
processor.

The slave subsystem can have insane constraints on the memory region where
its firmware is located and which it also uses as its generic purpose RAM.

While the region assigned to such a slave device would also likely include
a memory for its DMA buffers, the main concern here was an allocation of
a working memory for the slave processor.

Perhaps the device tree is not a perfect place for defining exact memory
regions. If we happen to decide against it, then I guess some set of
properties would be needed, that would allow CMA to assign appropriate
memory region(s) to a device.

> If you are talking about 'on this board I want to have 128MiB for this
> device because I'm doing 1080p while on another board 64MiB are enough
> because I'm doing 720p', then this is not a hardware constraint.

No, that's not something I was referring to.

> There may be valid scenarios for putting CMA into the devicetrees, but
> doing this also opens the door for abuse of this binding. I for once
> don't want to find areas being allocated for CMA in the devicetree for
> devices I don't care about. I know I can always exchange a devicetree,
> but I think the device tree should be seen as firmware to a certain
> degree.

I agree. You're likely aware of it, I just want to remind that the CMA
memory can be used by the OS in a normal way, until a device driver
actually allocates and uses it.

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

* Re: [PATCH 0/2] Device Tree support for CMA (Contiguous Memory Allocator)
  2013-02-18 22:25                 ` Sylwester Nawrocki
@ 2013-02-19  5:03                     ` Olof Johansson
  -1 siblings, 0 replies; 36+ messages in thread
From: Olof Johansson @ 2013-02-19  5:03 UTC (permalink / raw)
  To: Sylwester Nawrocki
  Cc: Sascha Hauer, Michal Nazarewicz,
	linaro-mm-sig-cunTk1MwBs8s++Sfvej+rw, Kyungmin Park,
	devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	Marek Szyprowski

On Mon, Feb 18, 2013 at 2:25 PM, Sylwester Nawrocki
<sylvester.nawrocki-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:

> My context was a multiprocessor SoC, where one of the processors is somehow
> application specific (e.g. camera ISP) and can be considered as a slave
> subsystem, running it's firmware and sharing system memory with the main
> processor.
>
> The slave subsystem can have insane constraints on the memory region where
> its firmware is located and which it also uses as its generic purpose RAM.
>
> While the region assigned to such a slave device would also likely include
> a memory for its DMA buffers, the main concern here was an allocation of
> a working memory for the slave processor.
>
> Perhaps the device tree is not a perfect place for defining exact memory
> regions. If we happen to decide against it, then I guess some set of
> properties would be needed, that would allow CMA to assign appropriate
> memory region(s) to a device.

The place to add such constraints are in the bindings for the camera ISP.

This isn't that different from how dma-window properties used on some
of the PowerPC platforms; it specifies (among other things) the
available address space as seen from the bus/device side.


-Olof

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

* [PATCH 0/2] Device Tree support for CMA (Contiguous Memory Allocator)
@ 2013-02-19  5:03                     ` Olof Johansson
  0 siblings, 0 replies; 36+ messages in thread
From: Olof Johansson @ 2013-02-19  5:03 UTC (permalink / raw)
  To: linux-arm-kernel

On Mon, Feb 18, 2013 at 2:25 PM, Sylwester Nawrocki
<sylvester.nawrocki@gmail.com> wrote:

> My context was a multiprocessor SoC, where one of the processors is somehow
> application specific (e.g. camera ISP) and can be considered as a slave
> subsystem, running it's firmware and sharing system memory with the main
> processor.
>
> The slave subsystem can have insane constraints on the memory region where
> its firmware is located and which it also uses as its generic purpose RAM.
>
> While the region assigned to such a slave device would also likely include
> a memory for its DMA buffers, the main concern here was an allocation of
> a working memory for the slave processor.
>
> Perhaps the device tree is not a perfect place for defining exact memory
> regions. If we happen to decide against it, then I guess some set of
> properties would be needed, that would allow CMA to assign appropriate
> memory region(s) to a device.

The place to add such constraints are in the bindings for the camera ISP.

This isn't that different from how dma-window properties used on some
of the PowerPC platforms; it specifies (among other things) the
available address space as seen from the bus/device side.


-Olof

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

* Re: [Linaro-mm-sig] [PATCH 0/2] Device Tree support for CMA (Contiguous Memory Allocator)
  2013-02-18 21:58                       ` Rob Herring
@ 2013-02-19  9:29                         ` Nishanth Peethambaran
  -1 siblings, 0 replies; 36+ messages in thread
From: Nishanth Peethambaran @ 2013-02-19  9:29 UTC (permalink / raw)
  To: Rob Herring
  Cc: Sascha Hauer, Michal Nazarewicz, linaro-mm-sig, Kyungmin Park,
	Sylwester Nawrocki, devicetree-discuss, linux-arm-kernel

On Tue, Feb 19, 2013 at 3:28 AM, Rob Herring <robherring2@gmail.com> wrote:
> On 02/16/2013 11:18 PM, Nishanth Peethambaran wrote:
>> Limiting the scope of the product to a particular use-case (720p or
>> 1080p video playback) will be part of firmware mostly OTP-ed in the
>> SoC. Firmware author could decide the amount of CMA memory needed and
>> put this as part of DT.
>
> What kernel developer would want a firmware author deciding how much
> memory is needed? It is not a decision that can be made without
> knowledge of the OS. While the frame size could be determined easily
> enough based on resolution, the number of buffers needed cannot.
>
> Rob
>

Got your point. Agree that firmware cannot decide on number of buffers.
Hard coding cma areas, and their relation to devices and CMA areas
inside kernel would bring back the concept of board files as drivers should
not have these details. Bootargs support defing cma size but not the relation
to devices.
Having a separate system configuration DT file would bring this configuration
out of firmware. OF format can be reused. System architect should
own the system configuration DT and not kernel developer.

- Nishanth Peethambaran

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

* [Linaro-mm-sig] [PATCH 0/2] Device Tree support for CMA (Contiguous Memory Allocator)
@ 2013-02-19  9:29                         ` Nishanth Peethambaran
  0 siblings, 0 replies; 36+ messages in thread
From: Nishanth Peethambaran @ 2013-02-19  9:29 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, Feb 19, 2013 at 3:28 AM, Rob Herring <robherring2@gmail.com> wrote:
> On 02/16/2013 11:18 PM, Nishanth Peethambaran wrote:
>> Limiting the scope of the product to a particular use-case (720p or
>> 1080p video playback) will be part of firmware mostly OTP-ed in the
>> SoC. Firmware author could decide the amount of CMA memory needed and
>> put this as part of DT.
>
> What kernel developer would want a firmware author deciding how much
> memory is needed? It is not a decision that can be made without
> knowledge of the OS. While the frame size could be determined easily
> enough based on resolution, the number of buffers needed cannot.
>
> Rob
>

Got your point. Agree that firmware cannot decide on number of buffers.
Hard coding cma areas, and their relation to devices and CMA areas
inside kernel would bring back the concept of board files as drivers should
not have these details. Bootargs support defing cma size but not the relation
to devices.
Having a separate system configuration DT file would bring this configuration
out of firmware. OF format can be reused. System architect should
own the system configuration DT and not kernel developer.

- Nishanth Peethambaran

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

* Re: [PATCH 0/2] Device Tree support for CMA (Contiguous Memory Allocator)
  2013-02-14 21:30     ` Sascha Hauer
@ 2013-03-15 15:05       ` Marek Szyprowski
  -1 siblings, 0 replies; 36+ messages in thread
From: Marek Szyprowski @ 2013-03-15 15:05 UTC (permalink / raw)
  To: Sascha Hauer
  Cc: Laura Abbott, Arnd Bergmann, devicetree-discuss,
	Nishanth Peethambaran, Michal Nazarewicz, Tomasz Figa,
	linaro-mm-sig, Kyungmin Park, Rob Herring, Sylwester Nawrocki,
	Olof Johansson, linux-arm-kernel

Hello,

It took me some time to get back to this topic due to my holidays and other
urgent issues. I hope that this I will be able to participate a bit more 
actively.

On 2/14/2013 10:30 PM, Sascha Hauer wrote:
> Hi Marek,
>
> On Thu, Feb 14, 2013 at 01:45:26PM +0100, Marek Szyprowski wrote:
> > Hello,
> >
> > Here is my initial proposal for device tree integration for Contiguous
> > Memory Allocator. The code is quite straightforward, however I expect
> > that the memory bindings require some discussion.
> >
> > The proposed bindings allows to define contiguous memory regions of
> > specified base address and size. Then, the defined regions can be
> > assigned to the given device(s) by adding a property with a phanle to
> > the defined contiguous memory region. From the device tree perspective
> > that's all. Once the bindings are added, all the memory allocations from
> > dma-mapping subsystem will be served from the defined contiguous memory
> > regions.
> >
>
> I think CMA regions should not be described in the devicetre at all. The
> devicetree is about hardware description and it should be OS agnostic,
> but CMA is only a Linux specific implementation detail. It's not even
> specific to a particular board, it's specific to a particular usecase of
> a board.

Well, I see your point. The main problem is the fact that board files 
consisted
of both hw related definitions AND particular kernel configuration for 
the device
drivers (like the dma_declare_contiguous() calls for example). Now hw 
related
definitions are being moved to device tree, but what should we do with the
kernel configuration data, which was previously in the board files? I 
know that
in the ideal scenario there should be no such data at all and all 
drivers should
be able to automatically configure all related parameters or give 
userspace a
method for changing them in runtime. The real world is not ideal 
however. There
is a set of kernel configuration data/parameters which can be set only once,
usually early in the boot process. How should we handle it?

There is kernel command line parameter (surprisingly stored also in 
device tree,
which should not have such data...), but it intended mainly for 
debug/development
purposes and simple, usually one value parameters.

There have been discussion about CMA configuration via kernel cmd line 
long time
ago (in early CMA development days), but such attempt has been rejected, 
because
it introduced too complicated kernel parameters, see
http://thread.gmane.org/gmane.linux.kernel.mm/50669/focus=50679

So we need another solution. I'm aware of the idea that DT should be 
limited to
hw description. This principle is good and it might let us to find some 
better,
more generic approach. However, we already have some of Linux kernel 
specific
attributes ("git grep 'linux,' -- Documentation/devicetree | wc -l" 
gives 71,
most of them are in keypad and buttons drivers).

Are there any KNOWN use cases for using exiting DTS from Linux kernel 
with other
operating systems? I believe that 99% of the DTS files will be both 
developed and
used only together with Linux kernel. I'm also seeing the DT as a generic
way of describing different variants of the same hardware (what is quite 
common
in the development process) and a method for standardizing the hw 
description
to ease the developers (for a good example please just compare some old 
OMAP and
Exynos board files - each used completely different way of describing 
the same
things).

The CMA DT patches are quite short and lets anyone to put the required 
kernel
configuration together with the respective HW definitions in a very 
convenient
way. Especially referencing regions and device entries by phandle 
pointers allows
us to avoid any text-based config entries and in-kernel parsers for them.

If the presented approach is definitely prohibited then what way should 
we go?

As an alternative I'm thinking about extending 'chosen' node for such cases.
Embedding the CMA configuration data into a few sub-nodes and attributes 
there
should not be a problem. It would also keep all the config related 
entries in
a single place. The another advantage I see is the possibility to update
'chosen' node from the extended dt-aware bootloaders, which might also 
let one
to edit them. What's your opinion?

Best regards
-- 
Marek Szyprowski
Samsung Poland R&D Center

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

* [PATCH 0/2] Device Tree support for CMA (Contiguous Memory Allocator)
@ 2013-03-15 15:05       ` Marek Szyprowski
  0 siblings, 0 replies; 36+ messages in thread
From: Marek Szyprowski @ 2013-03-15 15:05 UTC (permalink / raw)
  To: linux-arm-kernel

Hello,

It took me some time to get back to this topic due to my holidays and other
urgent issues. I hope that this I will be able to participate a bit more 
actively.

On 2/14/2013 10:30 PM, Sascha Hauer wrote:
> Hi Marek,
>
> On Thu, Feb 14, 2013 at 01:45:26PM +0100, Marek Szyprowski wrote:
> > Hello,
> >
> > Here is my initial proposal for device tree integration for Contiguous
> > Memory Allocator. The code is quite straightforward, however I expect
> > that the memory bindings require some discussion.
> >
> > The proposed bindings allows to define contiguous memory regions of
> > specified base address and size. Then, the defined regions can be
> > assigned to the given device(s) by adding a property with a phanle to
> > the defined contiguous memory region. From the device tree perspective
> > that's all. Once the bindings are added, all the memory allocations from
> > dma-mapping subsystem will be served from the defined contiguous memory
> > regions.
> >
>
> I think CMA regions should not be described in the devicetre at all. The
> devicetree is about hardware description and it should be OS agnostic,
> but CMA is only a Linux specific implementation detail. It's not even
> specific to a particular board, it's specific to a particular usecase of
> a board.

Well, I see your point. The main problem is the fact that board files 
consisted
of both hw related definitions AND particular kernel configuration for 
the device
drivers (like the dma_declare_contiguous() calls for example). Now hw 
related
definitions are being moved to device tree, but what should we do with the
kernel configuration data, which was previously in the board files? I 
know that
in the ideal scenario there should be no such data at all and all 
drivers should
be able to automatically configure all related parameters or give 
userspace a
method for changing them in runtime. The real world is not ideal 
however. There
is a set of kernel configuration data/parameters which can be set only once,
usually early in the boot process. How should we handle it?

There is kernel command line parameter (surprisingly stored also in 
device tree,
which should not have such data...), but it intended mainly for 
debug/development
purposes and simple, usually one value parameters.

There have been discussion about CMA configuration via kernel cmd line 
long time
ago (in early CMA development days), but such attempt has been rejected, 
because
it introduced too complicated kernel parameters, see
http://thread.gmane.org/gmane.linux.kernel.mm/50669/focus=50679

So we need another solution. I'm aware of the idea that DT should be 
limited to
hw description. This principle is good and it might let us to find some 
better,
more generic approach. However, we already have some of Linux kernel 
specific
attributes ("git grep 'linux,' -- Documentation/devicetree | wc -l" 
gives 71,
most of them are in keypad and buttons drivers).

Are there any KNOWN use cases for using exiting DTS from Linux kernel 
with other
operating systems? I believe that 99% of the DTS files will be both 
developed and
used only together with Linux kernel. I'm also seeing the DT as a generic
way of describing different variants of the same hardware (what is quite 
common
in the development process) and a method for standardizing the hw 
description
to ease the developers (for a good example please just compare some old 
OMAP and
Exynos board files - each used completely different way of describing 
the same
things).

The CMA DT patches are quite short and lets anyone to put the required 
kernel
configuration together with the respective HW definitions in a very 
convenient
way. Especially referencing regions and device entries by phandle 
pointers allows
us to avoid any text-based config entries and in-kernel parsers for them.

If the presented approach is definitely prohibited then what way should 
we go?

As an alternative I'm thinking about extending 'chosen' node for such cases.
Embedding the CMA configuration data into a few sub-nodes and attributes 
there
should not be a problem. It would also keep all the config related 
entries in
a single place. The another advantage I see is the possibility to update
'chosen' node from the extended dt-aware bootloaders, which might also 
let one
to edit them. What's your opinion?

Best regards
-- 
Marek Szyprowski
Samsung Poland R&D Center

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

* Re: [Linaro-mm-sig] [PATCH 2/2] drivers: dma-contiguous: add initialization from device tree
  2013-02-14 21:34       ` Laura Abbott
@ 2013-03-15 15:21           ` Marek Szyprowski
  -1 siblings, 0 replies; 36+ messages in thread
From: Marek Szyprowski @ 2013-03-15 15:21 UTC (permalink / raw)
  To: Laura Abbott
  Cc: Michal Nazarewicz, linaro-mm-sig-cunTk1MwBs8s++Sfvej+rw,
	Kyungmin Park, devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r

Hello,

On 2/14/2013 10:34 PM, Laura Abbott wrote:
>
> On 2/14/2013 4:45 AM, Marek Szyprowski wrote:
> <snip>
>> +name:        an name given to the defined region.
>> +base-address:    the base address of the defined region.
>> +size:        the size of the memory region.
>> +linux,contiguous-region: property indicating that the defined memory
>> +        region is used for contiguous memory allocations,
>> +        Linux specific (optional)
>> +linux,default-contiguous-region: property indicating that the region
>> +        is the default region for all contiguous memory
>> +        allocations, Linux specific (optional)
>> +
>> +
>
> I don't see any code actually implementing the 
> default-contiguous-region binding. Currently on ARM systems we will 
> still setup the default region based on the Kconfig. Is this intentional?

Nope, this was my fault. I've missed the fixup which added support for
default-contiguous-region (it was just a few lines more to cma_fdt_scan()
function).

>> diff --git a/drivers/base/dma-contiguous.c 
>> b/drivers/base/dma-contiguous.c
>> index 085389c..5761f73 100644
>> --- a/drivers/base/dma-contiguous.c
>> +++ b/drivers/base/dma-contiguous.c
>> @@ -24,6 +24,9 @@
>>
>>   #include <linux/memblock.h>
>>   #include <linux/err.h>
>> +#include <linux/of.h>
>> +#include <linux/of_fdt.h>
>> +#include <linux/of_platform.h>
>>   #include <linux/mm.h>
>>   #include <linux/mutex.h>
>>   #include <linux/page-isolation.h>
>> @@ -177,6 +180,35 @@ no_mem:
>>       return ERR_PTR(ret);
>>   }
>>
>> +/*****************************************************************************/ 
>>
>> +
>> +#ifdef CONFIG_OF
>> +int __init cma_fdt_scan(unsigned long node, const char *uname,
>> +                int depth, void *data)
>> +{
>> +    phys_addr_t base, size;
>> +    unsigned long len;
>> +    __be32 *prop;
>> +
>> +    if (strncmp(uname, "region@", 7) != 0 || depth != 2 ||
>> +        !of_get_flat_dt_prop(node, "contiguous-region", NULL))
>
> The documentation says "linux,contiguous-region"
>

Right, lack of another fixup. It looks that I posted an incomplete 
version, I'm sorry.
I hurried that time (it was my last day in the office before going for 
holidays).

>
>> +#ifdef CONFIG_OF
>> +static void cma_assign_device_from_dt(struct device *dev)
>> +{
>> +    struct device_node *node;
>> +    struct cma *cma;
>> +    u32 value;
>> +
>> +    node = of_parse_phandle(dev->of_node, "linux,contiguous-region", 
>> 0);
>> +    if (!node)
>> +        return;
>> +    if (of_property_read_u32(node, "reg", &value) && !value)
>> +        return;
>> +    cma = cma_get_area(value);
>> +    if (!cma)
>> +        return;
>> +
>> +    dev_set_cma_area(dev, cma);
>> +    pr_info("Assigned CMA region at %lx to %s device\n", (unsigned 
>> long)value, dev_name(dev));
>> +}
>> +
>
> This scheme of associating devices with CMA regions by base does not 
> work if you want to let CMA figure out where to place the region (base 
> = 0). Can we use the name to associate the device with the region? I 
> had been working on something similar internally and that was the only 
> solution I had come up with to associate arbitrary CMA nodes with devices.

Right, support for base = 0 requires different handling, but I thought 
that if
we use the device tree approach, the designer already knows the complete 
memory
configuration, so providing the correct base address is not that hard.

Best regards
-- 
Marek Szyprowski
Samsung Poland R&D Center

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

* [Linaro-mm-sig] [PATCH 2/2] drivers: dma-contiguous: add initialization from device tree
@ 2013-03-15 15:21           ` Marek Szyprowski
  0 siblings, 0 replies; 36+ messages in thread
From: Marek Szyprowski @ 2013-03-15 15:21 UTC (permalink / raw)
  To: linux-arm-kernel

Hello,

On 2/14/2013 10:34 PM, Laura Abbott wrote:
>
> On 2/14/2013 4:45 AM, Marek Szyprowski wrote:
> <snip>
>> +name:        an name given to the defined region.
>> +base-address:    the base address of the defined region.
>> +size:        the size of the memory region.
>> +linux,contiguous-region: property indicating that the defined memory
>> +        region is used for contiguous memory allocations,
>> +        Linux specific (optional)
>> +linux,default-contiguous-region: property indicating that the region
>> +        is the default region for all contiguous memory
>> +        allocations, Linux specific (optional)
>> +
>> +
>
> I don't see any code actually implementing the 
> default-contiguous-region binding. Currently on ARM systems we will 
> still setup the default region based on the Kconfig. Is this intentional?

Nope, this was my fault. I've missed the fixup which added support for
default-contiguous-region (it was just a few lines more to cma_fdt_scan()
function).

>> diff --git a/drivers/base/dma-contiguous.c 
>> b/drivers/base/dma-contiguous.c
>> index 085389c..5761f73 100644
>> --- a/drivers/base/dma-contiguous.c
>> +++ b/drivers/base/dma-contiguous.c
>> @@ -24,6 +24,9 @@
>>
>>   #include <linux/memblock.h>
>>   #include <linux/err.h>
>> +#include <linux/of.h>
>> +#include <linux/of_fdt.h>
>> +#include <linux/of_platform.h>
>>   #include <linux/mm.h>
>>   #include <linux/mutex.h>
>>   #include <linux/page-isolation.h>
>> @@ -177,6 +180,35 @@ no_mem:
>>       return ERR_PTR(ret);
>>   }
>>
>> +/*****************************************************************************/ 
>>
>> +
>> +#ifdef CONFIG_OF
>> +int __init cma_fdt_scan(unsigned long node, const char *uname,
>> +                int depth, void *data)
>> +{
>> +    phys_addr_t base, size;
>> +    unsigned long len;
>> +    __be32 *prop;
>> +
>> +    if (strncmp(uname, "region@", 7) != 0 || depth != 2 ||
>> +        !of_get_flat_dt_prop(node, "contiguous-region", NULL))
>
> The documentation says "linux,contiguous-region"
>

Right, lack of another fixup. It looks that I posted an incomplete 
version, I'm sorry.
I hurried that time (it was my last day in the office before going for 
holidays).

>
>> +#ifdef CONFIG_OF
>> +static void cma_assign_device_from_dt(struct device *dev)
>> +{
>> +    struct device_node *node;
>> +    struct cma *cma;
>> +    u32 value;
>> +
>> +    node = of_parse_phandle(dev->of_node, "linux,contiguous-region", 
>> 0);
>> +    if (!node)
>> +        return;
>> +    if (of_property_read_u32(node, "reg", &value) && !value)
>> +        return;
>> +    cma = cma_get_area(value);
>> +    if (!cma)
>> +        return;
>> +
>> +    dev_set_cma_area(dev, cma);
>> +    pr_info("Assigned CMA region at %lx to %s device\n", (unsigned 
>> long)value, dev_name(dev));
>> +}
>> +
>
> This scheme of associating devices with CMA regions by base does not 
> work if you want to let CMA figure out where to place the region (base 
> = 0). Can we use the name to associate the device with the region? I 
> had been working on something similar internally and that was the only 
> solution I had come up with to associate arbitrary CMA nodes with devices.

Right, support for base = 0 requires different handling, but I thought 
that if
we use the device tree approach, the designer already knows the complete 
memory
configuration, so providing the correct base address is not that hard.

Best regards
-- 
Marek Szyprowski
Samsung Poland R&D Center

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

* Re: [Linaro-mm-sig] [PATCH 2/2] drivers: dma-contiguous: add initialization from device tree
  2013-03-15 15:21           ` Marek Szyprowski
@ 2013-03-19 17:54               ` Laura Abbott
  -1 siblings, 0 replies; 36+ messages in thread
From: Laura Abbott @ 2013-03-19 17:54 UTC (permalink / raw)
  To: Marek Szyprowski
  Cc: Michal Nazarewicz, linaro-mm-sig-cunTk1MwBs8s++Sfvej+rw,
	Kyungmin Park, devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r

On 3/15/2013 8:21 AM, Marek Szyprowski wrote:
>>
>> This scheme of associating devices with CMA regions by base does not
>> work if you want to let CMA figure out where to place the region (base
>> = 0). Can we use the name to associate the device with the region? I
>> had been working on something similar internally and that was the only
>> solution I had come up with to associate arbitrary CMA nodes with
>> devices.
>
> Right, support for base = 0 requires different handling, but I thought
> that if
> we use the device tree approach, the designer already knows the complete
> memory
> configuration, so providing the correct base address is not that hard.

Not necessarily. The sizes of and number of regions may change depending 
on use cases. It's much easier to let Linux figure out where to place 
the regions vs. having to manually place everything each time.
(This also gets into the fact that some of the way we use CMA is a 
'grey' area that isn't actually hardware related)

Thanks,
Laura

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation

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

* [Linaro-mm-sig] [PATCH 2/2] drivers: dma-contiguous: add initialization from device tree
@ 2013-03-19 17:54               ` Laura Abbott
  0 siblings, 0 replies; 36+ messages in thread
From: Laura Abbott @ 2013-03-19 17:54 UTC (permalink / raw)
  To: linux-arm-kernel

On 3/15/2013 8:21 AM, Marek Szyprowski wrote:
>>
>> This scheme of associating devices with CMA regions by base does not
>> work if you want to let CMA figure out where to place the region (base
>> = 0). Can we use the name to associate the device with the region? I
>> had been working on something similar internally and that was the only
>> solution I had come up with to associate arbitrary CMA nodes with
>> devices.
>
> Right, support for base = 0 requires different handling, but I thought
> that if
> we use the device tree approach, the designer already knows the complete
> memory
> configuration, so providing the correct base address is not that hard.

Not necessarily. The sizes of and number of regions may change depending 
on use cases. It's much easier to let Linux figure out where to place 
the regions vs. having to manually place everything each time.
(This also gets into the fact that some of the way we use CMA is a 
'grey' area that isn't actually hardware related)

Thanks,
Laura

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation

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

end of thread, other threads:[~2013-03-19 17:54 UTC | newest]

Thread overview: 36+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-02-14 12:45 [PATCH 0/2] Device Tree support for CMA (Contiguous Memory Allocator) Marek Szyprowski
2013-02-14 12:45 ` Marek Szyprowski
     [not found] ` <1360845928-8107-1-git-send-email-m.szyprowski-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
2013-02-14 12:45   ` [PATCH 1/2] drivers: dma-contiguous: clean source code and prepare for device tree Marek Szyprowski
2013-02-14 12:45     ` Marek Szyprowski
2013-02-14 21:37     ` Laura Abbott
2013-02-14 21:37       ` Laura Abbott
2013-02-14 12:45   ` [PATCH 2/2] drivers: dma-contiguous: add initialization from " Marek Szyprowski
2013-02-14 12:45     ` Marek Szyprowski
2013-02-14 21:34     ` [Linaro-mm-sig] " Laura Abbott
2013-02-14 21:34       ` Laura Abbott
2013-02-15 16:12       ` Nishanth Peethambaran
2013-02-15 16:12         ` Nishanth Peethambaran
     [not found]       ` <511D586A.5060902-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
2013-03-15 15:21         ` Marek Szyprowski
2013-03-15 15:21           ` Marek Szyprowski
     [not found]           ` <51433C8B.20607-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
2013-03-19 17:54             ` Laura Abbott
2013-03-19 17:54               ` Laura Abbott
2013-02-14 21:30   ` [PATCH 0/2] Device Tree support for CMA (Contiguous Memory Allocator) Sascha Hauer
2013-02-14 21:30     ` Sascha Hauer
     [not found]     ` <20130214213013.GG1906-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
2013-02-14 22:08       ` Sylwester Nawrocki
2013-02-14 22:08         ` Sylwester Nawrocki
     [not found]         ` <511D6076.9090503-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2013-02-15  8:33           ` Sascha Hauer
2013-02-15  8:33             ` Sascha Hauer
     [not found]             ` <20130215083304.GK1906-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
2013-02-15 16:24               ` Rob Herring
2013-02-15 16:24                 ` Rob Herring
2013-02-17  5:18                 ` [Linaro-mm-sig] " Nishanth Peethambaran
2013-02-17  5:18                   ` Nishanth Peethambaran
     [not found]                   ` <CAMcxFTQAOjmzy77eB8nj3JDZ-6mwoMpm8yabtQj04tcLw-giLg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2013-02-18 21:58                     ` Rob Herring
2013-02-18 21:58                       ` Rob Herring
2013-02-19  9:29                       ` Nishanth Peethambaran
2013-02-19  9:29                         ` Nishanth Peethambaran
2013-02-18 22:25               ` Sylwester Nawrocki
2013-02-18 22:25                 ` Sylwester Nawrocki
     [not found]                 ` <5122AA3F.8030001-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2013-02-19  5:03                   ` Olof Johansson
2013-02-19  5:03                     ` Olof Johansson
2013-03-15 15:05     ` Marek Szyprowski
2013-03-15 15:05       ` Marek Szyprowski

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.