David Gibson's on May 2, 2019 2:16 pm: > On Tue, Apr 30, 2019 at 06:15:34PM +1000, Nicholas Piggin wrote: >> There is a need to be able to specify some options when building an FDT >> with the SW interface. This can be accomplished with minimal changes by >> storing intermediate data in the fdt header itself, in fields that are >> not otherwise needed during the creation process and can be set by >> fdt_finish(). >> >> The fdt.magic field is already used exactly this way, as a state to >> check with callers that the FDT has been created but not yet finished. >> >> fdt.version and fdt.last_comp_version are used to make room for more >> intermediate state. These are adjacent and unused during the building >> process. last_comp_version is not yet used for intermediate state, but >> it is zeroed and treated as used, so as to allow future growth easily. >> >> A new interface, fdt_create_with_flags() is added, which takes 32-bit >> flag value to control creation. >> >> Signed-off-by: Nicholas Piggin <npiggin@xxxxxxxxx> >> --- >> >> This is a different approach to the problem than the previous [PATCH] >> libfdt: Add fdt_property_nocompress variants that trade size for speed. >> Hopefully a nicer interface that's more extensible. >> >> Note, I was not able to get the python tests working on my distro, but >> I ran the program by hand a few times and tried to add some sane tests, >> so apologies in advance for submitting the un-tested test code. >> >> libfdt/fdt_sw.c | 27 +++++++++++++++++++++++++-- >> libfdt/libfdt.h | 1 + >> libfdt/version.lds | 1 + >> 3 files changed, 27 insertions(+), 2 deletions(-) >> >> diff --git a/libfdt/fdt_sw.c b/libfdt/fdt_sw.c >> index 9fa4a94..f162579 100644 >> --- a/libfdt/fdt_sw.c >> +++ b/libfdt/fdt_sw.c >> @@ -121,6 +121,12 @@ static int fdt_sw_probe_struct_(void *fdt) >> return err; \ >> } >> >> +static inline uint32_t fdt_sw_create_flags(void *fdt) > > I'd prefer just "sw_flags"; it's alocal so it doesn't have to be > namespaced. Sure. > >> +{ >> + /* assert: (fdt_magic(fdt) == FDT_SW_MAGIC) */ >> + return fdt_last_comp_version(fdt); >> +} >> + >> /* 'complete' state: Enter this state after fdt_finish() >> * >> * Allowed functions: none >> @@ -141,7 +147,7 @@ static void *fdt_grab_space_(void *fdt, size_t len) >> return fdt_offset_ptr_w_(fdt, offset); >> } >> >> -int fdt_create(void *buf, int bufsize) >> +int fdt_create_with_flags(void *buf, int bufsize, uint32_t flags) >> { >> const size_t hdrsize = FDT_ALIGN(sizeof(struct fdt_header), >> sizeof(struct fdt_reserve_entry)); >> @@ -152,9 +158,17 @@ int fdt_create(void *buf, int bufsize) >> >> memset(buf, 0, bufsize); > > Hm. Rule 1 of flags fields - always validate them when you introduce > them, which means rejecting anything except 0 at this point. > Otherwise later we'll have to live with broken callers that didn't > zero the field. Good point, will add that. Thanks, Nick