Hi Michael,

On Wed, 19 Feb 2020 at 11:43, Michael Trimarchi michael@amarulasolutions.com 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 michael@amarulasolutions.com

common/fdt_support.c | 40 ++++++++++++++++++++++++++++++++++++++++ include/fdt_support.h | 11 +++++++++++ 2 files changed, 51 insertions(+)

Reviewed-by: Simon Glass sjg@chromium.org

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", &address_cells, sizeof(address_cells));
  

•           fdt_setprop(blob, offs, "#size-cells", &size_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 start Begin 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

Hi Michael,

On Wed, 19 Feb 2020 at 11:43, Michael Trimarchi michael@amarulasolutions.com wrote:

Memory reserved for the simple framebuffer should not be used and part of memory linear mapping

Signed-off-by: Michael Trimarchi michael@amarulasolutions.com

drivers/video/meson/meson_vpu.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)

Is there a binding file for this? Please add it to a patch.

diff --git a/drivers/video/meson/meson_vpu.c b/drivers/video/meson/meson_vpu.c index 4eb66398d0..134e011b82 100644 --- a/drivers/video/meson/meson_vpu.c +++ b/drivers/video/meson/meson_vpu.c @@ -173,9 +173,9 @@ static void meson_vpu_setup_simplefb(void *fdt) * at the end of the RAM and we strip this portion from the kernel * allowed region */

•   mem_start = gd->bd->bi_dram[0].start;
  

•   mem_size = gd->bd->bi_dram[0].size - meson_fb.fb_size;
  

•   ret = fdt_fixup_memory_banks(fdt, &mem_start, &mem_size, 1);
  

•   mem_start = meson_fb.base;
  

•   mem_size = meson_fb.fb_size;
  

•   ret = fdt_fixup_reserved_memory(blob, "display_reserved", &mem_start, &mem_size);
  

Odd that this uses underscore...normally bindings use hyphen.

    if (ret) {
            eprintf("Cannot setup simplefb: Error reserving memory\n");
            return;

-- 2.17.1

Regards, Simon