Rob,
On Mon, Dec 17, 2018 at 11:56:28AM -0600, Rob Herring wrote:
> On Thu, Dec 13, 2018 at 12:37 AM AKASHI Takahiro
> <takahiro.akashi@xxxxxxxxxx> wrote:
> >
> > This function will add a memory region property based on parent node's
> > "#address-cells" and "#size-cells" properties.
> >
> > It will be used in implementing kdump with kexec_file_load system call
> > at linux kernel for arm64 once it is merged into kernel tree.
> >
> > Signed-off-by: AKASHI Takahiro <takahiro.akashi@xxxxxxxxxx>
> > ---
> > libfdt/fdt_addresses.c | 49 ++++++++++++++++++++++++++++++++++++++++++
> > libfdt/libfdt.h | 30 ++++++++++++++++++++++++++
> > 2 files changed, 79 insertions(+)
> >
> > diff --git a/libfdt/fdt_addresses.c b/libfdt/fdt_addresses.c
> > index f13a87dfa068..08615c9f669a 100644
> > --- a/libfdt/fdt_addresses.c
> > +++ b/libfdt/fdt_addresses.c
> > @@ -95,3 +95,52 @@ int fdt_size_cells(const void *fdt, int nodeoffset)
> > return 1;
> > return val;
> > }
> > +
> > +static void cpu64_to_fdt_cells(void *prop, uint64_t val, int cells)
> > +{
> > + fdt32_t val32;
> > +
> > + while (cells) {
> > + val32 = cpu_to_fdt32(val >> (32 * (--cells)));
> > + memcpy(prop, &val32, sizeof(val32));
> > + prop = (uint8_t *)prop + sizeof(val32);
> > + }
> > +}
> > +
> > +/* This function assumes that [address|size]_cells is 1 or 2 */
> > +int fdt_setprop_reg(void *fdt, int nodeoffset, const char *name,
>
> Do you have any use for name besides 'reg'? There are some I think
> (assigned-addresses?),
Yes. For kdump, we will use "linux,usable-memory-range" and
"linux,elfcorehdr." So we need a "name" argument.
> but I can't think of any widely used. So
> perhaps drop 'name'.
>
> > + uint64_t addr, uint64_t size)
>
> Once someone wants more than 1 entry, this function doesn't work.
> IIRC, u-boot would need that to use this. It would be better to take
> an array of values here. Probably up to some fixed max of 4 or 8
> entries would be fine.
So, something like this?
int fdt_setprop_reg(void *fdt, int nodeoffset, const char *name,
int num_reg, uint64_t (*reg)[2]);
Alternatively,
int fdt_setprop_reg(void *fdt, int nodeoffset, const char *name,
int num_reg, fdt_reserve_entry *reg);
In this case, converting values to fdt64_t is a user's job.
> And maybe do a wrapper for the single entry case.
Sure.
-Takahiro Akashi
> > +{
> > + int parent, addr_cells, size_cells, buf_size;
> > + char buf[sizeof(fdt32_t) * 2 * 2];
> > + void *prop;
