[PATCH v2 1/5] drivers: of: add initialization code for reserved memory
Tomasz Figa
t.figa at samsung.com
Tue Feb 11 09:29:54 EST 2014
Hi,
On 11.02.2014 13:13, Grant Likely wrote:
> On Tue, 11 Feb 2014 12:45:50 +0100, Marek Szyprowski <m.szyprowski at samsung.com> wrote:
>> Hello,
>>
>> On 2014-02-05 12:05, Grant Likely wrote:
>>> On Tue, 04 Feb 2014 13:09:29 +0100, Marek Szyprowski <m.szyprowski at samsung.com> wrote:
>>>> This patch adds device tree support for contiguous and reserved memory
>>>> regions defined in device tree.
>>>>
>>>> Large memory blocks can be reliably reserved only during early boot.
>>>> This must happen before the whole memory management subsystem is
>>>> initialized, because we need to ensure that the given contiguous blocks
>>>> are not yet allocated by kernel. Also it must happen before kernel
>>>> mappings for the whole low memory are created, to ensure that there will
>>>> be no mappings (for reserved blocks) or mapping with special properties
>>>> can be created (for CMA blocks). This all happens before device tree
>>>> structures are unflattened, so we need to get reserved memory layout
>>>> directly from fdt.
>>>>
>>>> Later, those reserved memory regions are assigned to devices on each
>>>> device structure initialization.
>>>>
>>>> Cc: Benjamin Herrenschmidt <benh at kernel.crashing.org>
>>>> Cc: Laura Abbott <lauraa at codeaurora.org>
>>>> Signed-off-by: Marek Szyprowski <m.szyprowski at samsung.com>
>>>> [joshc: rework to implement new DT binding, provide mechanism for
>>>> plugging in new reserved-memory node handlers via
>>>> RESERVEDMEM_OF_DECLARE]
>>>> Signed-off-by: Josh Cartwright <joshc at codeaurora.org>
>>>> [mszyprow: little code cleanup]
>>>> Signed-off-by: Marek Szyprowski <m.szyprowski at samsung.com>
>>>> ---
>>>> drivers/of/Kconfig | 6 +
>>>> drivers/of/Makefile | 1 +
>>>> drivers/of/of_reserved_mem.c | 219 +++++++++++++++++++++++++++++++++++++
>>>> drivers/of/platform.c | 7 ++
>>>> include/asm-generic/vmlinux.lds.h | 11 ++
>>>> include/linux/of_reserved_mem.h | 62 +++++++++++
>>>> 6 files changed, 306 insertions(+)
>>>> create mode 100644 drivers/of/of_reserved_mem.c
>>>> create mode 100644 include/linux/of_reserved_mem.h
>>>>
>>>> diff --git a/drivers/of/Kconfig b/drivers/of/Kconfig
>>>> index c6973f101a3e..aba13df56f3a 100644
>>>> --- a/drivers/of/Kconfig
>>>> +++ b/drivers/of/Kconfig
>>>> @@ -75,4 +75,10 @@ config OF_MTD
>>>> depends on MTD
>>>> def_bool y
>>>>
>>>> +config OF_RESERVED_MEM
>>>> + depends on HAVE_MEMBLOCK
>>>> + def_bool y
>>>> + help
>>>> + Helpers to allow for reservation of memory regions
>>>> +
>>>> endmenu # OF
>>>> diff --git a/drivers/of/Makefile b/drivers/of/Makefile
>>>> index efd05102c405..ed9660adad77 100644
>>>> --- a/drivers/of/Makefile
>>>> +++ b/drivers/of/Makefile
>>>> @@ -9,3 +9,4 @@ obj-$(CONFIG_OF_MDIO) += of_mdio.o
>>>> obj-$(CONFIG_OF_PCI) += of_pci.o
>>>> obj-$(CONFIG_OF_PCI_IRQ) += of_pci_irq.o
>>>> obj-$(CONFIG_OF_MTD) += of_mtd.o
>>>> +obj-$(CONFIG_OF_RESERVED_MEM) += of_reserved_mem.o
>>>> diff --git a/drivers/of/of_reserved_mem.c b/drivers/of/of_reserved_mem.c
>>>> new file mode 100644
>>>> index 000000000000..f17cd56e68d9
>>>> --- /dev/null
>>>> +++ b/drivers/of/of_reserved_mem.c
>>>> @@ -0,0 +1,219 @@
>>>> +/*
>>>> + * Device tree based initialization code for reserved memory.
>>>> + *
>>>> + * Copyright (c) 2013, The Linux Foundation. All Rights Reserved.
>>>> + * Copyright (c) 2013 Samsung Electronics Co., Ltd.
>>>> + * http://www.samsung.com
>>>> + * Author: Marek Szyprowski <m.szyprowski at samsung.com>
>>>> + * Author: Josh Cartwright <joshc at codeaurora.org>
>>>> + *
>>>> + * This program is free software; you can redistribute it and/or
>>>> + * modify it under the terms of the GNU General Public License as
>>>> + * published by the Free Software Foundation; either version 2 of the
>>>> + * License or (at your optional) any later version of the license.
>>>> + */
>>>> +#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/sizes.h>
>>>> +#include <linux/of_reserved_mem.h>
>>>> +
>>>> +#define MAX_RESERVED_REGIONS 16
>>>> +static struct reserved_mem reserved_mem[MAX_RESERVED_REGIONS];
>>>> +static int reserved_mem_count;
>>>> +
>>>> +int __init of_parse_flat_dt_reg(unsigned long node, const char *uname,
>>>> + phys_addr_t *base, phys_addr_t *size)
>>>
>>> Useful utility function; move to drivers/of/fdt.c
>>>
>>>> +{
>>>> + unsigned long len;
>>>> + __be32 *prop;
>>>> +
>>>> + prop = of_get_flat_dt_prop(node, "reg", &len);
>>>> + if (!prop)
>>>> + return -EINVAL;
>>>> +
>>>> + if (len < (dt_root_addr_cells + dt_root_size_cells) * sizeof(__be32)) {
>>>> + pr_err("Reserved memory: invalid reg property in '%s' node.\n",
>>>> + uname);
>>>> + return -EINVAL;
>>>> + }
>>>
>>> This is /okay/ for an initial implementation, but it is naive. While I
>>> suggested making #address-cells and #size-cells equal the root node
>>> values for the purpose of simplicity, it should still be perfectly valid
>>> to have different values if the ranges property is correctly formed.
>>>
>>>> +
>>>> + *base = dt_mem_next_cell(dt_root_addr_cells, &prop);
>>>> + *size = dt_mem_next_cell(dt_root_size_cells, &prop);
>>>
>>> Future enhancement; allow for parsing more than just the first reg
>>> tuple.
>>
>> One more question. Does it really makes any sense to support more than
>> one tuple for reg property? For consistency we should also allow more
>> than one entry in size, align and alloc-ranges property, but I don't
>> see any benefits for defining more than one range for a single region.
>> Same can be achieved by defining more regions instead if one really
>> needs such configuration.
>
> Yes, if only because it is an define usage of the reg property. If a
> devtree has multiple tuples in reg, then all of those tuples should be
> treated as reserved, even if the kernel doesn't know how to use them.
>
> I would not do the same for size/align/alloc-ranges unless there is a
> very specific use case that you can define. These ones are different
> from the static regions because they aren't ever used to protect
> something that already exists in the memory.
Is there a reason why multiple regions could not be used for this
purpose, instead of adding extra complexity of having multiple reg
entries per region?
I.e. I don't see a difference between
reg1: region at 00000000 {
reg = <0x00000000 0x1000>;
};
reg2: region at 10000000 {
reg = <0x10000000 0x1000>;
};
user {
regions = <®1>, <®2>;
};
and
reg: region at 00000000 {
reg = <0x00000000 0x1000>, <0x10000000 0x1000>;
};
user {
regions = <®>;
};
except that the former IMHO better suits the definition of memory
region, which I see as a single contiguous range of memory and can be
simplified to have a single reg entry per region.
Best regards,
Tomasz
More information about the linux-arm-kernel
mailing list