Re: [RFC/PATCH v2 2/2] drivers: dma-contiguous: add initialization from device tree
Hello,
On 4/29/2013 11:21 PM, Marc C wrote:
> /**
> * dma_contiguous_reserve() - reserve area(s) for contiguous memory handling
> * @limit: End address of the reserved memory (optional, 0 for any).
> @@ -149,6 +200,10 @@ void __init dma_contiguous_reserve(phys_addr_t limit)
>
> pr_debug("%s(limit %08lx)\n", __func__, (unsigned long)limit);
>
> +#ifdef CONFIG_OF
> + of_scan_flat_dt(cma_fdt_scan, NULL);
> +#endif
> +
What is your expectation with the contention between default region
setup via the kernel config (CONFIG_CMA_SIZE_SEL_*) and via the DT?
Could the call to 'of_scan_flat_dt()' be done before the setup of the
kernel config early regions, followed by a return code check
'fail-over' scheme, or were you intending for the default region
setups to be mutually-exclusive?
In the proposed patch the default/global cma region setup from device tree
had higher priority than kernel command line parameter and .config saved
values,
but now as I think of this, it looks that it would make more sense to
have the
following priority for setting up the default cma region:
1. kernel cmd line - if not available, then use:
2. device tree - if not available, then use:
3. kernel compiled .config
I will update this in the next version of the CMA DT patches.
Best regards
--
Marek Szyprowski
Samsung Poland R&D Center
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [RFC/PATCH v2 2/2] drivers: dma-contiguous: add initialization from device tree
Hi Laura,
Thanks for your thorough review! I will fix all the pointed issues once the
main point of this patch set (using /chosen/contiguous-memory for CMA DT
bindings) will be agreed and accepted.
On 4/11/2013 7:56 PM, Laura Abbott wrote:
Hi,
On 4/11/2013 4:22 AM, Marek Szyprowski wrote:
...
+
diff --git a/drivers/base/dma-contiguous.c
b/drivers/base/dma-contiguous.c
index 01fe743..6a8abab 100644
--- a/drivers/base/dma-contiguous.c
+++ b/drivers/base/dma-contiguous.c
@@ -24,6 +24,9 @@
#include
#include
+#include
+#include
+#include
#include
#include
#include
@@ -37,6 +40,7 @@ struct cma {
unsigned longbase_pfn;
unsigned longcount;
unsigned long*bitmap;
+charfull_name[32];
};
static DEFINE_MUTEX(cma_mutex);
@@ -133,6 +137,53 @@ static __init int cma_activate_area(struct cma
*cma)
return 0;
}
+/*/
+
+#ifdef CONFIG_OF
+int __init cma_fdt_scan(unsigned long node, const char *uname,
+int depth, void *data)
+{
+static int level;
+phys_addr_t base, size;
+unsigned long len;
+struct cma *cma;
+__be32 *prop;
+
+if (depth == 1 && strcmp(uname, "chosen") == 0) {
+level = depth;
+return 0;
+}
+
+if (depth == 2 && strcmp(uname, "contiguous-memory") == 0) {
+level = depth;
+return 0;
+}
+
+if (level != 2 || depth != 3 || strncmp(uname, "region@", 7) != 0)
+return 0;
+
Requiring the region@ label does not work if you want two dynamically
placed regions (i.e. two region@0). The devicetree will take the last
region@0 entry and ignore all the other ones
Hmm. You are right, I need different solution here to get it working
with autoconfigured base address.
+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, &cma);
+
Need to check the return of dma_contiguous_reserve_area, else there is
an abort when trying to access cma->name on an error
+strcpy(cma->full_name, uname);
+
+if (of_get_flat_dt_prop(node, "linux,default-contiguous-region",
NULL)) {
+
+dma_contiguous_default_area = cma;
+}
+return 0;
+}
+#endif
+
/**
* dma_contiguous_reserve() - reserve area(s) for contiguous memory
handling
* @limit: End address of the reserved memory (optional, 0 for any).
if the contiguous region isn't set via devicetree,
default_area->full_name needs to be set in dma_contiguous_reserve,
else we get wrong associations in scan_cma_node.
@@ -149,6 +200,10 @@ void __init dma_contiguous_reserve(phys_addr_t
limit)
pr_debug("%s(limit %08lx)\n", __func__, (unsigned long)limit);
+#ifdef CONFIG_OF
+of_scan_flat_dt(cma_fdt_scan, NULL);
+#endif
+
if (size_cmdline != -1) {
sel_size = size_cmdline;
} else {
@@ -265,6 +320,71 @@ int __init dma_contiguous_add_device(struct
device *dev, struct cma *cma)
return 0;
}
+#ifdef CONFIG_OF
+static struct cma_map {
+struct cma *cma;
+struct device_node *node;
+} cma_maps[MAX_CMA_AREAS];
+static unsigned cma_map_count;
+
+static void cma_assign_device_from_dt(struct device *dev)
+{
+int i;
+for (i=0; iof_node) {
+dev_set_cma_area(dev, cma_maps[i].cma);
+pr_info("Assigned CMA %s to %s device\n",
+cma_maps[i].cma->full_name,
+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,
+};
+
+void scan_cma_nodes(void)
+{
+struct device_node *parent =
of_find_node_by_path("/chosen/contiguous-memory");
+struct device_node *child;
+
+if (!parent)
+return;
+
+for_each_child_of_node(parent, child) {
+struct cma *cma = NULL;
+int i;
+
+for (i=0; ifull_name, cma_areas[i].full_name))
+cma = &cma_areas[i];
break out of the loop once the area is found?
Also, how will the code deal with region names that are substrings of
each other e.g.
region@100
region@1000
Thanks for pointing this.
+if (!cma)
+continue;
+
+for (i=0;; i++) {
+struct device_node *node;
+node = of_parse_phandle(child, "device", i);
+if (!node)
+break;
+
+cma_maps[cma_map_count].cma =
Re: [RFC/PATCH v2 2/2] drivers: dma-contiguous: add initialization from device tree
Hi,
On 4/11/2013 4:22 AM, Marek Szyprowski wrote:
...
+
diff --git a/drivers/base/dma-contiguous.c b/drivers/base/dma-contiguous.c
index 01fe743..6a8abab 100644
--- a/drivers/base/dma-contiguous.c
+++ b/drivers/base/dma-contiguous.c
@@ -24,6 +24,9 @@
#include
#include
+#include
+#include
+#include
#include
#include
#include
@@ -37,6 +40,7 @@ struct cma {
unsigned long base_pfn;
unsigned long count;
unsigned long *bitmap;
+ charfull_name[32];
};
static DEFINE_MUTEX(cma_mutex);
@@ -133,6 +137,53 @@ static __init int cma_activate_area(struct cma *cma)
return 0;
}
+/*/
+
+#ifdef CONFIG_OF
+int __init cma_fdt_scan(unsigned long node, const char *uname,
+ int depth, void *data)
+{
+ static int level;
+ phys_addr_t base, size;
+ unsigned long len;
+ struct cma *cma;
+ __be32 *prop;
+
+ if (depth == 1 && strcmp(uname, "chosen") == 0) {
+ level = depth;
+ return 0;
+ }
+
+ if (depth == 2 && strcmp(uname, "contiguous-memory") == 0) {
+ level = depth;
+ return 0;
+ }
+
+ if (level != 2 || depth != 3 || strncmp(uname, "region@", 7) != 0)
+ return 0;
+
Requiring the region@ label does not work if you want two dynamically
placed regions (i.e. two region@0). The devicetree will take the last
region@0 entry and ignore all the other ones
+ 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, &cma);
+
Need to check the return of dma_contiguous_reserve_area, else there is
an abort when trying to access cma->name on an error
+ strcpy(cma->full_name, uname);
+
+ if (of_get_flat_dt_prop(node, "linux,default-contiguous-region", NULL))
{
+
+ dma_contiguous_default_area = cma;
+ }
+ return 0;
+}
+#endif
+
/**
* dma_contiguous_reserve() - reserve area(s) for contiguous memory handling
* @limit: End address of the reserved memory (optional, 0 for any).
if the contiguous region isn't set via devicetree,
default_area->full_name needs to be set in dma_contiguous_reserve, else
we get wrong associations in scan_cma_node.
@@ -149,6 +200,10 @@ void __init dma_contiguous_reserve(phys_addr_t limit)
pr_debug("%s(limit %08lx)\n", __func__, (unsigned long)limit);
+#ifdef CONFIG_OF
+ of_scan_flat_dt(cma_fdt_scan, NULL);
+#endif
+
if (size_cmdline != -1) {
sel_size = size_cmdline;
} else {
@@ -265,6 +320,71 @@ int __init dma_contiguous_add_device(struct device *dev,
struct cma *cma)
return 0;
}
+#ifdef CONFIG_OF
+static struct cma_map {
+ struct cma *cma;
+ struct device_node *node;
+} cma_maps[MAX_CMA_AREAS];
+static unsigned cma_map_count;
+
+static void cma_assign_device_from_dt(struct device *dev)
+{
+ int i;
+ for (i=0; iof_node) {
+ dev_set_cma_area(dev, cma_maps[i].cma);
+ pr_info("Assigned CMA %s to %s device\n",
+ cma_maps[i].cma->full_name,
+ 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,
+};
+
+void scan_cma_nodes(void)
+{
+ struct device_node *parent =
of_find_node_by_path("/chosen/contiguous-memory");
+ struct device_node *child;
+
+ if (!parent)
+ return;
+
+ for_each_child_of_node(parent, child) {
+ struct cma *cma = NULL;
+ int i;
+
+ for (i=0; ifull_name, cma_areas[i].full_name))
+ cma = &cma_areas[i];
break out of the loop once the area is found?
Also, how will the code deal with region names that are substrings of
each other e.g.
region@100
region@1000
+ if (!cma)
+ continue;
+
+ for (i=0;; i++) {
+ struct device_node *node;
+ node = of_parse_phandle(child, "device", i);
+ if (!node)
+ break;
+
+ cma_maps[cm
[RFC/PATCH v2 2/2] drivers: dma-contiguous: add initialization from device tree
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
Acked-by: Kyungmin Park
---
.../devicetree/bindings/contiguous-memory.txt | 69 +++
arch/arm/boot/dts/skeleton.dtsi|8 +-
drivers/base/dma-contiguous.c | 124
3 files changed, 200 insertions(+), 1 deletion(-)
create mode 100644 Documentation/devicetree/bindings/contiguous-memory.txt
diff --git a/Documentation/devicetree/bindings/contiguous-memory.txt
b/Documentation/devicetree/bindings/contiguous-memory.txt
new file mode 100644
index 000..fa598bf
--- /dev/null
+++ b/Documentation/devicetree/bindings/contiguous-memory.txt
@@ -0,0 +1,69 @@
+* Contiguous memory binding
+
+The /chosen/contiguous-memory node provides runtime configuration of
+contiguous memory regions for Linux kernel. One can create such regions
+for 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:
+
+contiguous-memory {
+
+ (name): region@(base-address) {
+ reg = <(baseaddr) (size)>;
+ (linux,default-contiguous-region);
+ device = <&device_0 &device_1 ...>
+ };
+};
+
+name: an name given to the defined region.
+base-address: the base address of the defined region.
+size: the size of the memory region (bytes).
+linux,default-contiguous-region: property indicating that the region
+ is the default region for all contiguous memory
+ allocations, Linux specific (optional)
+device:array of phandles to the client device nodes, which use
+ the given contiguous region
+
+* Example:
+
+This example defines a memory configuration containing 2 contiguous
+regions for Linux kernel, one default of all device drivers (named
+contig_mem, placed at 0x7200, 64MiB) and one dedicated to the
+framebuffer device (named display_mem, placed at 0x7800, 16MiB). The
+display_mem region is then assigned to fb@1230 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.
+
+/ {
+ /* ... */
+
+ chosen {
+ bootargs = /* ... */
+
+ contiguous-memory {
+ contig_mem: region@7200 {
+ reg = <0x7200 0x400>;
+ linux,default-contiguous-region;
+ };
+
+ display_mem: region@7800 {
+ reg = <0x7800 0x100>;
+ device = <&fb0>;
+ };
+ };
+ };
+
+ /* ... */
+
+ fb0: fb@1230 {
+ status = "okay";
+ };
+};
diff --git a/arch/arm/boot/dts/skeleton.dtsi b/arch/arm/boot/dts/skeleton.dtsi
index b41d241..538a868 100644
--- a/arch/arm/boot/dts/skeleton.dtsi
+++ b/arch/arm/boot/dts/skeleton.dtsi
@@ -7,7 +7,13 @@
/ {
#address-cells = <1>;
#size-cells = <1>;
- chosen { };
+ chosen {
+ contiguous-memory {
+ #address-cells = <1>;
+ #size-cells = <1>;
+ };
+ };
aliases { };
memory { device_type = "memory"; reg = <0 0>; };
};
+
diff --git a/drivers/base/dma-contiguous.c b/drivers/base/dma-contiguous.c
index 01fe743..6a8abab 100644
--- a/drivers/base/dma-contiguous.c
+++ b/drivers/base/dma-contiguous.c
@@ -24,6 +24,9 @@
#include
#include
+#include
+#include
+#include
#include
#include
#include
@@ -37,6 +40,7 @@ struct cma {
unsigned long base_pfn;
unsigned long count;
unsigned long *bitmap;
+ charfull_name[32];
};
static DEFINE_MUTEX(cma_mutex);
@@ -133,6 +137,53 @@ static __init int cma_activate_area(struct cma *cma)
return 0;
}
+/*/
+
+#ifdef CONFIG_OF
+int __init cma_fdt_scan(unsigned long node, const char *uname,
+ int depth, void *data)
+{
+ static int level;
+ phys_addr_t base, size;
+ unsigned long len;
+ struc
