Hi On Wed, Feb 26, 2020 at 4:33 PM Simon Glass <s...@chromium.org> wrote: > > Hi Michael, > > On Mon, 24 Feb 2020 at 22:10, Michael Trimarchi > <mich...@amarulasolutions.com> wrote: > > > > The intent is to reserve memory _and_ prevent it from being included > > in the kernel's linear map. For thos reason it is also necessary to include > > the > > 'no-map' property for this reserved-mem node. > > > > From Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt: > > > > no-map (optional) - empty property > > - Indicates the operating system must not create a virtual mapping > > of the region as part of its standard mapping of system memory, > > nor permit speculative access to it under any circumstances other > > than under the control of the device driver using the region. > > > > Signed-off-by: Michael Trimarchi <mich...@amarulasolutions.com> > > --- > > Changes: RFC->v1 > > - Add a better commit message > > --- > > 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); > > Error check >
Ok > > + 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", &address_cells, > > sizeof(address_cells)); > > + fdt_setprop(blob, offs, "#size-cells", &size_cells, > > sizeof(size_cells)); > > + fdt_setprop(blob, offs, "ranges", NULL, 0); > > Need error-checking on these three > ok > > + } > > + > > + offs = fdt_add_subnode(blob, offs, area ? : "private"); > > Is this in a binding file somewhere? > The reserved memory is needed to avoid the linux kernel to use it and to get a persistent framebuffer. On some SoC I have implemented the reuse on the memory on their graphics driver. I need to check how this is documented in Linux. The name of the reserved memory it's not mandatory. > > + if (offs < 0) { > > + printf("Could not create %s%s node.\n", path, subpath); > > + return -1; > > return offs > ok > > + } > > + > > + fdt_setprop(blob, offs, "no-map", NULL, 0); > > and this? > ok > Also needs error check > > > + len = fdt_pack_reg(blob, temp, start, size, 1); > > + fdt_setprop(blob, offs, "reg", temp, len); > > blank line before return > done > > + 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 > > Really we should return an FDT_ERR always. -1 is not a good idea and > in fact is an FDT_ERR > Now with the change FDT_ERR only is returned. > > + */ > > +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 I will post a new one Michael