From: m.szyprowski@samsung•com (Marek Szyprowski)
To: linux-arm-kernel@lists•infradead.org
Subject: [RFC/PATCH v2 2/2] drivers: dma-contiguous: add initialization from device tree
Date: Fri, 12 Apr 2013 14:28:59 +0200 [thread overview]
Message-ID: <5167FE0B.80605@samsung.com> (raw)
In-Reply-To: <5166F932.9000003@codeaurora.org>
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 <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 at 0). The devicetree will take the last
> region at 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; 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 at 1000000
> region at 10000000
>
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 = 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
>
Best regards
--
Marek Szyprowski
Samsung Poland R&D Center
next prev parent reply other threads:[~2013-04-12 12:28 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-04-11 11:22 [RFC/PATCH v2 0/2] Device Tree support for CMA (Contiguous Memory Allocator) Marek Szyprowski
2013-04-11 11:22 ` [RFC/PATCH v2 1/2] drivers: dma-contiguous: clean source code and prepare for device tree Marek Szyprowski
2013-04-11 11:22 ` [RFC/PATCH v2 2/2] drivers: dma-contiguous: add initialization from " Marek Szyprowski
2013-04-11 17:56 ` Laura Abbott
2013-04-12 12:28 ` Marek Szyprowski [this message]
2013-04-24 10:30 ` [Linaro-mm-sig] " Francesco Lavra
2013-04-29 21:21 ` Marc C
2013-04-30 8:53 ` Marek Szyprowski
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=5167FE0B.80605@samsung.com \
--to=m.szyprowski@samsung$(echo .)com \
--cc=linux-arm-kernel@lists$(echo .)infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox