Re: [Linaro-mm-sig] [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 <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> @@ -37,6 +40,7 @@ struct cma { unsigned long base_pfn; unsigned long count; unsigned long *bitmap;

• char full_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; i<cma_map_count; i++) {

• if (cma_maps[i].node == dev->of_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; i<cma_area_count; i++)
  

• 	if (strstr(child->full_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@1000000 region@10000000

• 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 = cma;
  

• 	cma_maps[cma_map_count].node = node;
  

• 	cma_map_count++;
  

Bounds check cma_map_count against MAX_CMA_AREAS?

• }
  

• }

+} +#endif

• static int __init cma_init_reserved_areas(void) { int i;

@@ -275,6 +395,10 @@ static int __init cma_init_reserved_areas(void) return ret; }

+#ifdef CONFIG_OF

• scan_cma_nodes();
• bus_register_notifier(&platform_bus_type, &cma_dev_init_nb);

+#endif return 0; } core_initcall(cma_init_reserved_areas);

Thanks, Laura