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

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



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.

> +{
> +	/* 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.

>  
> +	/*
> +	 * magic and last_comp_version keep intermediate state during the fdt
> +	 * creation process, which is replaced with the proper FDT format by
> +	 * fdt_finish().
> +	 *
> +	 * flags should be accessed with fdt_sw_create_flags().
> +	 */
>  	fdt_set_magic(fdt, FDT_SW_MAGIC);
>  	fdt_set_version(fdt, FDT_LAST_SUPPORTED_VERSION);
> -	fdt_set_last_comp_version(fdt, FDT_FIRST_SUPPORTED_VERSION);
> +	fdt_set_last_comp_version(fdt, flags);
> +
>  	fdt_set_totalsize(fdt,  bufsize);
>  
>  	fdt_set_off_mem_rsvmap(fdt, hdrsize);
> @@ -164,6 +178,11 @@ int fdt_create(void *buf, int bufsize)
>  	return 0;
>  }
>  
> +int fdt_create(void *buf, int bufsize)
> +{
> +	return fdt_create_with_flags(buf, bufsize, 0);
> +}
> +
>  int fdt_resize(void *fdt, void *buf, int bufsize)
>  {
>  	size_t headsize, tailsize;
> @@ -360,6 +379,10 @@ int fdt_finish(void *fdt)
>  
>  	/* Finally, adjust the header */
>  	fdt_set_totalsize(fdt, newstroffset + fdt_size_dt_strings(fdt));
> +
> +	/* And fix up fields that were keeping intermediate state. */
> +	fdt_set_last_comp_version(fdt, FDT_FIRST_SUPPORTED_VERSION);
>  	fdt_set_magic(fdt, FDT_MAGIC);
> +
>  	return 0;
>  }
> diff --git a/libfdt/libfdt.h b/libfdt/libfdt.h
> index 1f44177..574904d 100644
> --- a/libfdt/libfdt.h
> +++ b/libfdt/libfdt.h
> @@ -1430,6 +1430,7 @@ int fdt_nop_node(void *fdt, int nodeoffset);
>  /**********************************************************************/
>  
>  int fdt_create(void *buf, int bufsize);
> +int fdt_create_with_flags(void *buf, int bufsize, uint32_t flags);
>  int fdt_resize(void *fdt, void *buf, int bufsize);
>  int fdt_add_reservemap_entry(void *fdt, uint64_t addr, uint64_t size);
>  int fdt_finish_reservemap(void *fdt);
> diff --git a/libfdt/version.lds b/libfdt/version.lds
> index 9a92652..0d52217 100644
> --- a/libfdt/version.lds
> +++ b/libfdt/version.lds
> @@ -74,6 +74,7 @@ LIBFDT_1.2 {
>  		fdt_header_size_;
>  		fdt_appendprop_addrrange;
>  		fdt_setprop_inplace_namelen_partial;
> +		fdt_create_with_flags;
>  	local:
>  		*;
>  };

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

Attachment: signature.asc
Description: PGP signature


[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