Re: [PATCH 1/2] libfdt: Introduce fdt_create_with_flags()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]



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





[Index of Archives]     [Device Tree]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]

  Powered by Linux