Re: [PATCH v4 1/7] drivers: of: add initialization code for reserved memory

2014-02-21 Thread Grant Likely
On Fri, 21 Feb 2014 12:00:44 +0100, Marek Szyprowski  
wrote:
> Hello,
> 
> On 2014-02-20 15:01, Grant Likely wrote:
> > On Thu, 20 Feb 2014 11:52:41 +0100, Marek Szyprowski 
> >  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.
> > >
> > > Signed-off-by: Marek Szyprowski 
> > > [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 
> > > [mszyprow: added generic memory reservation code, refactored code to
> > >  put it directly into fdt.c]
> > > Signed-off-by: Marek Szyprowski 
> > > ---
> > >  drivers/of/Kconfig|6 +
> > >  drivers/of/Makefile   |1 +
> > >  drivers/of/fdt.c  |  145 ++
> > >  drivers/of/of_reserved_mem.c  |  296 
> > > +
> > >  drivers/of/platform.c |7 +
> > >  include/asm-generic/vmlinux.lds.h |   11 ++
> > >  include/linux/of_reserved_mem.h   |   65 
> > >  7 files changed, 531 insertions(+)
> > >  create mode 100644 drivers/of/of_reserved_mem.c
> > >  create mode 100644 include/linux/of_reserved_mem.h
> >
> > Hi Marek,
> >
> > There's a lot of moving parts in this patch. Can you split the patch up a 
> > bit please. There are parts that I'm not entierly comfortable with yet and 
> > it will help reviewing them if they are added separately. For instance, the 
> > attaching regions to devices is something that I want to have some 
> > discussion about, but the core reserving static ranges I think is pretty 
> > much ready to be merged. I can merge the later while still debating the 
> > former if they are split.
> >
> > I would recommend splitting into four:
> > 1) reservation of static regions without the support code for referencing 
> > them later
> > 2) code to also do dynamic allocations of reserved regions - again without 
> > any driver references
> > 3) add hooks to reference specific regions.
> > 4) hooks into drivers/of/platform.c for wiring into the driver model.
> >
> > Can you also make the binding doc the first patch?
> 
> Ok, I will slice the patch into 4 pieces.
> >
> >  Wow, the flattree parsing code has to be really verbose. We 
> > really need better
> > flat tree parsing functions and helpers.
> 
> Yes, parsing fdt is a real pain, but please don't ask me to implement 
> all the
> helpers to make it easier together with this patch. I (and probably other
> developers) would really like to get this piece merged asap.

I won't. Mostly I was thinking out loud.

> > > + for (i = __reservedmem_of_table; i < &__rmem_of_table_sentinel; i++) {
> > > + reservedmem_of_init_fn initfn = i->data;
> > > + const char *compat = i->compatible;
> > > +
> > > + if (!of_flat_dt_is_compatible(rmem->fdt_node, compat))
> > > + continue;
> >
> > What if two entries both match the compatible list? Ideally score would
> > be taken into account. (I won't block on this issue, it can be a future
> > enhancement)
> 
> If two entries have same compatible value they will be probed in the order
> of presence in the kernel binary. The return value is checked and the next
> one is being tried if init fails for the given function. The provided code
> already makes use of this feature. Both DMA coherent and CMA use
> "shared-dma-pool" compatible. DMA coherent init fails if 'reusable'
> property has been found. On the other hand, CMA fails initialization if
> 'reusable' property is missing. Frankly I would like to change standard
> DMA coherent compatible value to 'dma-pool' and keep 'shared-dma-pool'
> only for CMA, but I've implemented it the way it has been described in
> your binding documentation.

My binding document isn't gospel and it hasn't been merged yet. Reply to
the binding patch and make your argument for the change.

g.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v4 1/7] drivers: of: add initialization code for reserved memory

2014-02-21 Thread Marek Szyprowski

Hello,

On 2014-02-20 15:01, Grant Likely wrote:

On Thu, 20 Feb 2014 11:52:41 +0100, Marek Szyprowski  
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.
>
> Signed-off-by: Marek Szyprowski 
> [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 
> [mszyprow: added generic memory reservation code, refactored code to
>  put it directly into fdt.c]
> Signed-off-by: Marek Szyprowski 
> ---
>  drivers/of/Kconfig|6 +
>  drivers/of/Makefile   |1 +
>  drivers/of/fdt.c  |  145 ++
>  drivers/of/of_reserved_mem.c  |  296 
+
>  drivers/of/platform.c |7 +
>  include/asm-generic/vmlinux.lds.h |   11 ++
>  include/linux/of_reserved_mem.h   |   65 
>  7 files changed, 531 insertions(+)
>  create mode 100644 drivers/of/of_reserved_mem.c
>  create mode 100644 include/linux/of_reserved_mem.h

Hi Marek,

There's a lot of moving parts in this patch. Can you split the patch up a bit 
please. There are parts that I'm not entierly comfortable with yet and it will 
help reviewing them if they are added separately. For instance, the attaching 
regions to devices is something that I want to have some discussion about, but 
the core reserving static ranges I think is pretty much ready to be merged. I 
can merge the later while still debating the former if they are split.

I would recommend splitting into four:
1) reservation of static regions without the support code for referencing them 
later
2) code to also do dynamic allocations of reserved regions - again without any 
driver references
3) add hooks to reference specific regions.
4) hooks into drivers/of/platform.c for wiring into the driver model.

Can you also make the binding doc the first patch?


Ok, I will slice the patch into 4 pieces.

(snipped)


> +/**
> + * res_mem_alloc_size() - allocate reserved memory described by 'size', 
'align'
> + *  and 'alloc-ranges' properties
> + */
> +static int __init
> +__reserved_mem_alloc_size(unsigned long node, const char *uname,
> +phys_addr_t *res_base, phys_addr_t *res_size)
> +{
> +  int t_len = (dt_root_addr_cells + dt_root_size_cells) * sizeof(__be32);
> +  phys_addr_t start = 0, end = 0;
> +  phys_addr_t base = 0, align = 0, size;
> +  unsigned long len;
> +  __be32 *prop;
> +  int nomap;
> +  int ret;
> +
> +  prop = of_get_flat_dt_prop(node, "size", );
> +  if (!prop)
> +  return -EINVAL;
> +
> +  if (len != dt_root_size_cells * sizeof(__be32)) {
> +  pr_err("Reserved memory: invalid size property in '%s' node.\n",
> +  uname);
> +  return -EINVAL;
> +  }
> +  size = dt_mem_next_cell(dt_root_size_cells, );
> +
> +  nomap = of_get_flat_dt_prop(node, "no-map", NULL) != NULL;
> +
> +  prop = of_get_flat_dt_prop(node, "align", );
> +  if (prop) {
> +  if (len != dt_root_addr_cells * sizeof(__be32)) {
> +  pr_err("Reserved memory: invalid align property in '%s' 
node.\n",
> +  uname);
> +  return -EINVAL;
> +  }
> +  align = dt_mem_next_cell(dt_root_addr_cells, );
> +  }
> +
> +  prop = of_get_flat_dt_prop(node, "alloc-ranges", );
> +  if (prop) {
> +
> +  if (len % t_len != 0) {
> +  pr_err("Reserved memory: invalid alloc-ranges property in '%s', 
skipping node.\n",
> + uname);
> +  return -EINVAL;
> +  }
> +
> +  base = 0;
> +
> +  while (len > 0) {
> +  start = dt_mem_next_cell(dt_root_addr_cells, );
> +  end = start + dt_mem_next_cell(dt_root_size_cells,
> + );
> +
> +  ret = early_init_dt_alloc_reserved_memory_arch(size,
> +  align, start, end, nomap, );
> +  if (ret == 0) {
> +  pr_debug("Reserved memory: allocated memory for '%s' 
node: base %pa, size %ld MiB\n",
> +  

Re: [PATCH v4 1/7] drivers: of: add initialization code for reserved memory

2014-02-21 Thread Marek Szyprowski

Hello,

On 2014-02-20 15:01, Grant Likely wrote:

On Thu, 20 Feb 2014 11:52:41 +0100, Marek Szyprowski m.szyprow...@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.

 Signed-off-by: Marek Szyprowski m.szyprow...@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 jo...@codeaurora.org
 [mszyprow: added generic memory reservation code, refactored code to
  put it directly into fdt.c]
 Signed-off-by: Marek Szyprowski m.szyprow...@samsung.com
 ---
  drivers/of/Kconfig|6 +
  drivers/of/Makefile   |1 +
  drivers/of/fdt.c  |  145 ++
  drivers/of/of_reserved_mem.c  |  296 
+
  drivers/of/platform.c |7 +
  include/asm-generic/vmlinux.lds.h |   11 ++
  include/linux/of_reserved_mem.h   |   65 
  7 files changed, 531 insertions(+)
  create mode 100644 drivers/of/of_reserved_mem.c
  create mode 100644 include/linux/of_reserved_mem.h

Hi Marek,

There's a lot of moving parts in this patch. Can you split the patch up a bit 
please. There are parts that I'm not entierly comfortable with yet and it will 
help reviewing them if they are added separately. For instance, the attaching 
regions to devices is something that I want to have some discussion about, but 
the core reserving static ranges I think is pretty much ready to be merged. I 
can merge the later while still debating the former if they are split.

I would recommend splitting into four:
1) reservation of static regions without the support code for referencing them 
later
2) code to also do dynamic allocations of reserved regions - again without any 
driver references
3) add hooks to reference specific regions.
4) hooks into drivers/of/platform.c for wiring into the driver model.

Can you also make the binding doc the first patch?


Ok, I will slice the patch into 4 pieces.

(snipped)


 +/**
 + * res_mem_alloc_size() - allocate reserved memory described by 'size', 
'align'
 + *  and 'alloc-ranges' properties
 + */
 +static int __init
 +__reserved_mem_alloc_size(unsigned long node, const char *uname,
 +phys_addr_t *res_base, phys_addr_t *res_size)
 +{
 +  int t_len = (dt_root_addr_cells + dt_root_size_cells) * sizeof(__be32);
 +  phys_addr_t start = 0, end = 0;
 +  phys_addr_t base = 0, align = 0, size;
 +  unsigned long len;
 +  __be32 *prop;
 +  int nomap;
 +  int ret;
 +
 +  prop = of_get_flat_dt_prop(node, size, len);
 +  if (!prop)
 +  return -EINVAL;
 +
 +  if (len != dt_root_size_cells * sizeof(__be32)) {
 +  pr_err(Reserved memory: invalid size property in '%s' node.\n,
 +  uname);
 +  return -EINVAL;
 +  }
 +  size = dt_mem_next_cell(dt_root_size_cells, prop);
 +
 +  nomap = of_get_flat_dt_prop(node, no-map, NULL) != NULL;
 +
 +  prop = of_get_flat_dt_prop(node, align, len);
 +  if (prop) {
 +  if (len != dt_root_addr_cells * sizeof(__be32)) {
 +  pr_err(Reserved memory: invalid align property in '%s' 
node.\n,
 +  uname);
 +  return -EINVAL;
 +  }
 +  align = dt_mem_next_cell(dt_root_addr_cells, prop);
 +  }
 +
 +  prop = of_get_flat_dt_prop(node, alloc-ranges, len);
 +  if (prop) {
 +
 +  if (len % t_len != 0) {
 +  pr_err(Reserved memory: invalid alloc-ranges property in '%s', 
skipping node.\n,
 + uname);
 +  return -EINVAL;
 +  }
 +
 +  base = 0;
 +
 +  while (len  0) {
 +  start = dt_mem_next_cell(dt_root_addr_cells, prop);
 +  end = start + dt_mem_next_cell(dt_root_size_cells,
 + prop);
 +
 +  ret = early_init_dt_alloc_reserved_memory_arch(size,
 +  align, start, end, nomap, base);
 +  if (ret == 0) {
 +  pr_debug(Reserved memory: allocated memory for '%s' 
node: base %pa, size %ld MiB\n,
 + 

Re: [PATCH v4 1/7] drivers: of: add initialization code for reserved memory

2014-02-21 Thread Grant Likely
On Fri, 21 Feb 2014 12:00:44 +0100, Marek Szyprowski m.szyprow...@samsung.com 
wrote:
 Hello,
 
 On 2014-02-20 15:01, Grant Likely wrote:
  On Thu, 20 Feb 2014 11:52:41 +0100, Marek Szyprowski 
  m.szyprow...@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.
  
   Signed-off-by: Marek Szyprowski m.szyprow...@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 jo...@codeaurora.org
   [mszyprow: added generic memory reservation code, refactored code to
put it directly into fdt.c]
   Signed-off-by: Marek Szyprowski m.szyprow...@samsung.com
   ---
drivers/of/Kconfig|6 +
drivers/of/Makefile   |1 +
drivers/of/fdt.c  |  145 ++
drivers/of/of_reserved_mem.c  |  296 
   +
drivers/of/platform.c |7 +
include/asm-generic/vmlinux.lds.h |   11 ++
include/linux/of_reserved_mem.h   |   65 
7 files changed, 531 insertions(+)
create mode 100644 drivers/of/of_reserved_mem.c
create mode 100644 include/linux/of_reserved_mem.h
 
  Hi Marek,
 
  There's a lot of moving parts in this patch. Can you split the patch up a 
  bit please. There are parts that I'm not entierly comfortable with yet and 
  it will help reviewing them if they are added separately. For instance, the 
  attaching regions to devices is something that I want to have some 
  discussion about, but the core reserving static ranges I think is pretty 
  much ready to be merged. I can merge the later while still debating the 
  former if they are split.
 
  I would recommend splitting into four:
  1) reservation of static regions without the support code for referencing 
  them later
  2) code to also do dynamic allocations of reserved regions - again without 
  any driver references
  3) add hooks to reference specific regions.
  4) hooks into drivers/of/platform.c for wiring into the driver model.
 
  Can you also make the binding doc the first patch?
 
 Ok, I will slice the patch into 4 pieces.
 
  off topic Wow, the flattree parsing code has to be really verbose. We 
  really need better
  flat tree parsing functions and helpers.
 
 Yes, parsing fdt is a real pain, but please don't ask me to implement 
 all the
 helpers to make it easier together with this patch. I (and probably other
 developers) would really like to get this piece merged asap.

I won't. Mostly I was thinking out loud.

   + for (i = __reservedmem_of_table; i  __rmem_of_table_sentinel; i++) {
   + reservedmem_of_init_fn initfn = i-data;
   + const char *compat = i-compatible;
   +
   + if (!of_flat_dt_is_compatible(rmem-fdt_node, compat))
   + continue;
 
  What if two entries both match the compatible list? Ideally score would
  be taken into account. (I won't block on this issue, it can be a future
  enhancement)
 
 If two entries have same compatible value they will be probed in the order
 of presence in the kernel binary. The return value is checked and the next
 one is being tried if init fails for the given function. The provided code
 already makes use of this feature. Both DMA coherent and CMA use
 shared-dma-pool compatible. DMA coherent init fails if 'reusable'
 property has been found. On the other hand, CMA fails initialization if
 'reusable' property is missing. Frankly I would like to change standard
 DMA coherent compatible value to 'dma-pool' and keep 'shared-dma-pool'
 only for CMA, but I've implemented it the way it has been described in
 your binding documentation.

My binding document isn't gospel and it hasn't been merged yet. Reply to
the binding patch and make your argument for the change.

g.

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v4 1/7] drivers: of: add initialization code for reserved memory

2014-02-20 Thread Grant Likely
On Thu, 20 Feb 2014 11:52:41 +0100, Marek Szyprowski  
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.
> 
> Signed-off-by: Marek Szyprowski 
> [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 
> [mszyprow: added generic memory reservation code, refactored code to
>  put it directly into fdt.c]
> Signed-off-by: Marek Szyprowski 
> ---
>  drivers/of/Kconfig|6 +
>  drivers/of/Makefile   |1 +
>  drivers/of/fdt.c  |  145 ++
>  drivers/of/of_reserved_mem.c  |  296 
> +
>  drivers/of/platform.c |7 +
>  include/asm-generic/vmlinux.lds.h |   11 ++
>  include/linux/of_reserved_mem.h   |   65 
>  7 files changed, 531 insertions(+)
>  create mode 100644 drivers/of/of_reserved_mem.c
>  create mode 100644 include/linux/of_reserved_mem.h

Hi Marek,

There's a lot of moving parts in this patch. Can you split the patch up a bit 
please. There are parts that I'm not entierly comfortable with yet and it will 
help reviewing them if they are added separately. For instance, the attaching 
regions to devices is something that I want to have some discussion about, but 
the core reserving static ranges I think is pretty much ready to be merged. I 
can merge the later while still debating the former if they are split.

I would recommend splitting into four:
1) reservation of static regions without the support code for referencing them 
later
2) code to also do dynamic allocations of reserved regions - again without any 
driver references
3) add hooks to reference specific regions.
4) hooks into drivers/of/platform.c for wiring into the driver model.

Can you also make the binding doc the first patch?

> 
> diff --git a/drivers/of/Kconfig b/drivers/of/Kconfig
> index c6973f101a3e..30a7d87a8077 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 OF_EARLY_FLATTREE
> + bool
> + 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/fdt.c b/drivers/of/fdt.c
> index 758b4f8b30b7..04efe2ba736f 100644
> --- a/drivers/of/fdt.c
> +++ b/drivers/of/fdt.c
> @@ -15,6 +15,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -438,6 +439,150 @@ int __initdata dt_root_size_cells;
>  struct boot_param_header *initial_boot_params;
>  
>  #ifdef CONFIG_OF_EARLY_FLATTREE
> +#if defined(CONFIG_HAVE_MEMBLOCK)
> +int __init __weak
> +early_init_dt_reserve_memory_arch(phys_addr_t base, phys_addr_t size,
> +   bool nomap)
> +{
> + if (memblock_is_region_reserved(base, size))
> + return -EBUSY;
> + if (nomap)
> + return memblock_remove(base, size);
> + return memblock_reserve(base, size);
> +}
> +#else
> +int __init __weak
> +early_init_dt_reserve_memory_arch(phys_addr_t base, phys_addr_t size,
> +   bool nomap)
> +{
> + pr_error("Reserved memory not supported, ignoring range 0x%llx - 
> 0x%llx%s\n",
> +   base, size, nomap ? " (nomap)" : "");
> + return -ENOSYS;
> +}
> +#endif

Group the above with the early_init_dt_add_memory_arch() and
early_init_dt_alloc_memory_arch() hooks.

> +
> +/**
> + * res_mem_reserve_reg() - reserve all memory described in 'reg' property
> + */
> +static int __init
> +__reserved_mem_reserve_reg(unsigned long node, const char *uname,
> + phys_addr_t *res_base, phys_addr_t *res_size)

Nit: put the funciton name on the same line as "static int __init". It's
more grep friendly 

[PATCH v4 1/7] drivers: of: add initialization code for reserved memory

2014-02-20 Thread Marek Szyprowski
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.

Signed-off-by: Marek Szyprowski 
[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 
[mszyprow: added generic memory reservation code, refactored code to
 put it directly into fdt.c]
Signed-off-by: Marek Szyprowski 
---
 drivers/of/Kconfig|6 +
 drivers/of/Makefile   |1 +
 drivers/of/fdt.c  |  145 ++
 drivers/of/of_reserved_mem.c  |  296 +
 drivers/of/platform.c |7 +
 include/asm-generic/vmlinux.lds.h |   11 ++
 include/linux/of_reserved_mem.h   |   65 
 7 files changed, 531 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..30a7d87a8077 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 OF_EARLY_FLATTREE
+   bool
+   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/fdt.c b/drivers/of/fdt.c
index 758b4f8b30b7..04efe2ba736f 100644
--- a/drivers/of/fdt.c
+++ b/drivers/of/fdt.c
@@ -15,6 +15,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -438,6 +439,150 @@ int __initdata dt_root_size_cells;
 struct boot_param_header *initial_boot_params;
 
 #ifdef CONFIG_OF_EARLY_FLATTREE
+#if defined(CONFIG_HAVE_MEMBLOCK)
+int __init __weak
+early_init_dt_reserve_memory_arch(phys_addr_t base, phys_addr_t size,
+ bool nomap)
+{
+   if (memblock_is_region_reserved(base, size))
+   return -EBUSY;
+   if (nomap)
+   return memblock_remove(base, size);
+   return memblock_reserve(base, size);
+}
+#else
+int __init __weak
+early_init_dt_reserve_memory_arch(phys_addr_t base, phys_addr_t size,
+ bool nomap)
+{
+   pr_error("Reserved memory not supported, ignoring range 0x%llx - 
0x%llx%s\n",
+ base, size, nomap ? " (nomap)" : "");
+   return -ENOSYS;
+}
+#endif
+
+/**
+ * res_mem_reserve_reg() - reserve all memory described in 'reg' property
+ */
+static int __init
+__reserved_mem_reserve_reg(unsigned long node, const char *uname,
+   phys_addr_t *res_base, phys_addr_t *res_size)
+{
+   int t_len = (dt_root_addr_cells + dt_root_size_cells) * sizeof(__be32);
+   phys_addr_t base, size;
+   unsigned long len;
+   __be32 *prop;
+   int nomap;
+
+   prop = of_get_flat_dt_prop(node, "reg", );
+   if (!prop)
+   return -ENOENT;
+
+   if (len && len % t_len != 0) {
+   pr_err("Reserved memory: invalid reg property in '%s', skipping 
node.\n",
+  uname);
+   return -EINVAL;
+   }
+
+   nomap = of_get_flat_dt_prop(node, "no-map", NULL) != NULL;
+
+   /* store base and size values from the first reg tuple */
+   *res_base = 0;
+   while (len > 0) {
+   base = dt_mem_next_cell(dt_root_addr_cells, );
+   size = dt_mem_next_cell(dt_root_size_cells, );
+
+   if (base && size &&
+   early_init_dt_reserve_memory_arch(base, size, nomap) == 0)
+   pr_debug("Reserved memory: reserved region for node 
'%s': base %pa, size %ld MiB\n",
+   uname, , (unsigned long)size / SZ_1M);
+   else
+   pr_info("Reserved memory: failed to reserve memory for 
node '%s': base %pa, size %ld MiB\n",
+   uname, , (unsigned long)size / 

[PATCH v4 1/7] drivers: of: add initialization code for reserved memory

2014-02-20 Thread Marek Szyprowski
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.

Signed-off-by: Marek Szyprowski m.szyprow...@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 jo...@codeaurora.org
[mszyprow: added generic memory reservation code, refactored code to
 put it directly into fdt.c]
Signed-off-by: Marek Szyprowski m.szyprow...@samsung.com
---
 drivers/of/Kconfig|6 +
 drivers/of/Makefile   |1 +
 drivers/of/fdt.c  |  145 ++
 drivers/of/of_reserved_mem.c  |  296 +
 drivers/of/platform.c |7 +
 include/asm-generic/vmlinux.lds.h |   11 ++
 include/linux/of_reserved_mem.h   |   65 
 7 files changed, 531 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..30a7d87a8077 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 OF_EARLY_FLATTREE
+   bool
+   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/fdt.c b/drivers/of/fdt.c
index 758b4f8b30b7..04efe2ba736f 100644
--- a/drivers/of/fdt.c
+++ b/drivers/of/fdt.c
@@ -15,6 +15,7 @@
 #include linux/module.h
 #include linux/of.h
 #include linux/of_fdt.h
+#include linux/of_reserved_mem.h
 #include linux/string.h
 #include linux/errno.h
 #include linux/slab.h
@@ -438,6 +439,150 @@ int __initdata dt_root_size_cells;
 struct boot_param_header *initial_boot_params;
 
 #ifdef CONFIG_OF_EARLY_FLATTREE
+#if defined(CONFIG_HAVE_MEMBLOCK)
+int __init __weak
+early_init_dt_reserve_memory_arch(phys_addr_t base, phys_addr_t size,
+ bool nomap)
+{
+   if (memblock_is_region_reserved(base, size))
+   return -EBUSY;
+   if (nomap)
+   return memblock_remove(base, size);
+   return memblock_reserve(base, size);
+}
+#else
+int __init __weak
+early_init_dt_reserve_memory_arch(phys_addr_t base, phys_addr_t size,
+ bool nomap)
+{
+   pr_error(Reserved memory not supported, ignoring range 0x%llx - 
0x%llx%s\n,
+ base, size, nomap ?  (nomap) : );
+   return -ENOSYS;
+}
+#endif
+
+/**
+ * res_mem_reserve_reg() - reserve all memory described in 'reg' property
+ */
+static int __init
+__reserved_mem_reserve_reg(unsigned long node, const char *uname,
+   phys_addr_t *res_base, phys_addr_t *res_size)
+{
+   int t_len = (dt_root_addr_cells + dt_root_size_cells) * sizeof(__be32);
+   phys_addr_t base, size;
+   unsigned long len;
+   __be32 *prop;
+   int nomap;
+
+   prop = of_get_flat_dt_prop(node, reg, len);
+   if (!prop)
+   return -ENOENT;
+
+   if (len  len % t_len != 0) {
+   pr_err(Reserved memory: invalid reg property in '%s', skipping 
node.\n,
+  uname);
+   return -EINVAL;
+   }
+
+   nomap = of_get_flat_dt_prop(node, no-map, NULL) != NULL;
+
+   /* store base and size values from the first reg tuple */
+   *res_base = 0;
+   while (len  0) {
+   base = dt_mem_next_cell(dt_root_addr_cells, prop);
+   size = dt_mem_next_cell(dt_root_size_cells, prop);
+
+   if (base  size 
+   early_init_dt_reserve_memory_arch(base, size, nomap) == 0)
+   pr_debug(Reserved memory: reserved region for node 
'%s': base %pa, size %ld MiB\n,
+   uname, base, (unsigned long)size / SZ_1M);
+   else
+

Re: [PATCH v4 1/7] drivers: of: add initialization code for reserved memory

2014-02-20 Thread Grant Likely
On Thu, 20 Feb 2014 11:52:41 +0100, Marek Szyprowski m.szyprow...@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.
 
 Signed-off-by: Marek Szyprowski m.szyprow...@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 jo...@codeaurora.org
 [mszyprow: added generic memory reservation code, refactored code to
  put it directly into fdt.c]
 Signed-off-by: Marek Szyprowski m.szyprow...@samsung.com
 ---
  drivers/of/Kconfig|6 +
  drivers/of/Makefile   |1 +
  drivers/of/fdt.c  |  145 ++
  drivers/of/of_reserved_mem.c  |  296 
 +
  drivers/of/platform.c |7 +
  include/asm-generic/vmlinux.lds.h |   11 ++
  include/linux/of_reserved_mem.h   |   65 
  7 files changed, 531 insertions(+)
  create mode 100644 drivers/of/of_reserved_mem.c
  create mode 100644 include/linux/of_reserved_mem.h

Hi Marek,

There's a lot of moving parts in this patch. Can you split the patch up a bit 
please. There are parts that I'm not entierly comfortable with yet and it will 
help reviewing them if they are added separately. For instance, the attaching 
regions to devices is something that I want to have some discussion about, but 
the core reserving static ranges I think is pretty much ready to be merged. I 
can merge the later while still debating the former if they are split.

I would recommend splitting into four:
1) reservation of static regions without the support code for referencing them 
later
2) code to also do dynamic allocations of reserved regions - again without any 
driver references
3) add hooks to reference specific regions.
4) hooks into drivers/of/platform.c for wiring into the driver model.

Can you also make the binding doc the first patch?

 
 diff --git a/drivers/of/Kconfig b/drivers/of/Kconfig
 index c6973f101a3e..30a7d87a8077 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 OF_EARLY_FLATTREE
 + bool
 + 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/fdt.c b/drivers/of/fdt.c
 index 758b4f8b30b7..04efe2ba736f 100644
 --- a/drivers/of/fdt.c
 +++ b/drivers/of/fdt.c
 @@ -15,6 +15,7 @@
  #include linux/module.h
  #include linux/of.h
  #include linux/of_fdt.h
 +#include linux/of_reserved_mem.h
  #include linux/string.h
  #include linux/errno.h
  #include linux/slab.h
 @@ -438,6 +439,150 @@ int __initdata dt_root_size_cells;
  struct boot_param_header *initial_boot_params;
  
  #ifdef CONFIG_OF_EARLY_FLATTREE
 +#if defined(CONFIG_HAVE_MEMBLOCK)
 +int __init __weak
 +early_init_dt_reserve_memory_arch(phys_addr_t base, phys_addr_t size,
 +   bool nomap)
 +{
 + if (memblock_is_region_reserved(base, size))
 + return -EBUSY;
 + if (nomap)
 + return memblock_remove(base, size);
 + return memblock_reserve(base, size);
 +}
 +#else
 +int __init __weak
 +early_init_dt_reserve_memory_arch(phys_addr_t base, phys_addr_t size,
 +   bool nomap)
 +{
 + pr_error(Reserved memory not supported, ignoring range 0x%llx - 
 0x%llx%s\n,
 +   base, size, nomap ?  (nomap) : );
 + return -ENOSYS;
 +}
 +#endif

Group the above with the early_init_dt_add_memory_arch() and
early_init_dt_alloc_memory_arch() hooks.

 +
 +/**
 + * res_mem_reserve_reg() - reserve all memory described in 'reg' property
 + */
 +static int __init
 +__reserved_mem_reserve_reg(unsigned long node, const char *uname,
 + phys_addr_t *res_base, phys_addr_t *res_size)

Nit: put the