Re: [RFC PATCH 1/2] common: fdt: Add a function for reserving memory without kernel linear mapping

2020-02-19 Thread Simon Glass
Hi Michael,

On Wed, 19 Feb 2020 at 11:43, Michael Trimarchi
 wrote:
>
> We need to reserve memory and not put in kernel linear mapping.
> This avoid problem during memory remapping from the simple
> frame buffer in linux kernel
>
> Signed-off-by: Michael Trimarchi 
> ---
>  common/fdt_support.c  | 40 
>  include/fdt_support.h | 11 +++
>  2 files changed, 51 insertions(+)

Reviewed-by: Simon Glass 

nits below

>
> diff --git a/common/fdt_support.c b/common/fdt_support.c
> index 02cf5c6241..a3662f4358 100644
> --- a/common/fdt_support.c
> +++ b/common/fdt_support.c
> @@ -410,6 +410,46 @@ static int fdt_pack_reg(const void *fdt, void *buf, u64 
> *address, u64 *size,
> return p - (char *)buf;
>  }
>
> +int fdt_fixup_reserved_memory(void *blob, const char *area, u64 start[], u64 
> size[])
> +{
> +   int offs, len;
> +   const char *subpath;
> +   const char *path = "/reserved-memory";
> +   fdt32_t address_cells = cpu_to_fdt32(fdt_address_cells(blob, 0));
> +   fdt32_t size_cells = cpu_to_fdt32(fdt_size_cells(blob, 0));
> +   u8 temp[16]; /* Up to 64-bit address + 64-bit size */
> +
> +   offs = fdt_path_offset(blob, path);
> +   if (offs < 0) {
> +   debug("Node %s not found\n", path);
> +   path = "/";
> +   subpath = "reserved-memory";
> +   offs = fdt_path_offset(blob, path);

This is always 0 by definition.

> +   offs = fdt_add_subnode(blob, offs, subpath);
> +   if (offs < 0) {
> +   printf("Could not create %s%s node.\n", path, 
> subpath);
> +   return -1;

This is -EPERM. I think it would be better to return ofs.

> +   }
> +   path = "/reserved-memory";
> +   offs = fdt_path_offset(blob, path);
> +
> +   fdt_setprop(blob, offs, "#address-cells", _cells, 
> sizeof(address_cells));
> +   fdt_setprop(blob, offs, "#size-cells", _cells, 
> sizeof(size_cells));
> +   fdt_setprop(blob, offs, "ranges", NULL, 0);
> +   }
> +
> +   offs = fdt_add_subnode(blob, offs, area ? : "private");
> +   if (offs < 0) {
> +   printf("Could not create %s%s node.\n", path, subpath);
> +   return -1;

and here

> +   }
> +
> +   fdt_setprop(blob, offs, "no-map", NULL, 0);
> +   len = fdt_pack_reg(blob, temp, start, size, 1);
> +   fdt_setprop(blob, offs, "reg", temp, len);

blank line before return

> +   return 0;
> +}
> +
>  #if CONFIG_NR_DRAM_BANKS > 4
>  #define MEMORY_BANKS_MAX CONFIG_NR_DRAM_BANKS
>  #else
> diff --git a/include/fdt_support.h b/include/fdt_support.h
> index ba14acd7f6..7c8a280f53 100644
> --- a/include/fdt_support.h
> +++ b/include/fdt_support.h
> @@ -93,6 +93,17 @@ void do_fixup_by_compat_u32(void *fdt, const char *compat,
>   */
>  int fdt_fixup_memory(void *blob, u64 start, u64 size);
>
> +/**
> + * Setup the memory reserved node in the DT. Creates one if none was 
> existing before.
> + *
> + * @param blob FDT blob to update
> + * @param area Reserved area name
> + * @param startBegin of DRAM mapping in physical memory
> + * @param size Size of the single memory bank
> + * @return 0 if ok, or -1 or -FDT_ERR_... on error

check this

> + */
> +int fdt_fixup_reserved_memory(void *blob, const char *area, u64 start[], u64 
> size[]);
> +
>  /**
>   * Fill the DT memory node with multiple memory banks.
>   * Creates the node if none was existing before.
> --
> 2.17.1
>

Regards,
Simon


[RFC PATCH 1/2] common: fdt: Add a function for reserving memory without kernel linear mapping

2020-02-19 Thread Michael Trimarchi
We need to reserve memory and not put in kernel linear mapping.
This avoid problem during memory remapping from the simple
frame buffer in linux kernel

Signed-off-by: Michael Trimarchi 
---
 common/fdt_support.c  | 40 
 include/fdt_support.h | 11 +++
 2 files changed, 51 insertions(+)

diff --git a/common/fdt_support.c b/common/fdt_support.c
index 02cf5c6241..a3662f4358 100644
--- a/common/fdt_support.c
+++ b/common/fdt_support.c
@@ -410,6 +410,46 @@ static int fdt_pack_reg(const void *fdt, void *buf, u64 
*address, u64 *size,
return p - (char *)buf;
 }
 
+int fdt_fixup_reserved_memory(void *blob, const char *area, u64 start[], u64 
size[])
+{
+   int offs, len;
+   const char *subpath;
+   const char *path = "/reserved-memory";
+   fdt32_t address_cells = cpu_to_fdt32(fdt_address_cells(blob, 0));
+   fdt32_t size_cells = cpu_to_fdt32(fdt_size_cells(blob, 0));
+   u8 temp[16]; /* Up to 64-bit address + 64-bit size */
+
+   offs = fdt_path_offset(blob, path);
+   if (offs < 0) {
+   debug("Node %s not found\n", path);
+   path = "/";
+   subpath = "reserved-memory";
+   offs = fdt_path_offset(blob, path);
+   offs = fdt_add_subnode(blob, offs, subpath);
+   if (offs < 0) {
+   printf("Could not create %s%s node.\n", path, subpath);
+   return -1;
+   }
+   path = "/reserved-memory";
+   offs = fdt_path_offset(blob, path);
+
+   fdt_setprop(blob, offs, "#address-cells", _cells, 
sizeof(address_cells));
+   fdt_setprop(blob, offs, "#size-cells", _cells, 
sizeof(size_cells));
+   fdt_setprop(blob, offs, "ranges", NULL, 0);
+   }
+
+   offs = fdt_add_subnode(blob, offs, area ? : "private");
+   if (offs < 0) {
+   printf("Could not create %s%s node.\n", path, subpath);
+   return -1;
+   }
+
+   fdt_setprop(blob, offs, "no-map", NULL, 0);
+   len = fdt_pack_reg(blob, temp, start, size, 1);
+   fdt_setprop(blob, offs, "reg", temp, len);
+   return 0;
+}
+
 #if CONFIG_NR_DRAM_BANKS > 4
 #define MEMORY_BANKS_MAX CONFIG_NR_DRAM_BANKS
 #else
diff --git a/include/fdt_support.h b/include/fdt_support.h
index ba14acd7f6..7c8a280f53 100644
--- a/include/fdt_support.h
+++ b/include/fdt_support.h
@@ -93,6 +93,17 @@ void do_fixup_by_compat_u32(void *fdt, const char *compat,
  */
 int fdt_fixup_memory(void *blob, u64 start, u64 size);
 
+/**
+ * Setup the memory reserved node in the DT. Creates one if none was existing 
before.
+ *
+ * @param blob FDT blob to update
+ * @param area Reserved area name
+ * @param startBegin of DRAM mapping in physical memory
+ * @param size Size of the single memory bank
+ * @return 0 if ok, or -1 or -FDT_ERR_... on error
+ */
+int fdt_fixup_reserved_memory(void *blob, const char *area, u64 start[], u64 
size[]);
+
 /**
  * Fill the DT memory node with multiple memory banks.
  * Creates the node if none was existing before.
-- 
2.17.1