Re: [PATCH v3] libfdt: Add phandle generation helper

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



On 3/20/19 8:10 AM, Thierry Reding wrote:
> From: Thierry Reding <treding@xxxxxxxxxx>
> 
> The new fdt_generate_phandle() function can be used to generate a new,
> unused phandle given a specific device tree blob. The implementation is
> somewhat naive in that it simply walks the entire device tree to find
> the highest phandle value and then returns a phandle value one higher
> than that. A more clever implementation might try to find holes in the
> current set of phandle values and fill them. But this implementation is
> relatively simple and works reliably.
> 
> Also add a test that validates that phandles generated by this new API
> are indeed unique.

I don't see any issues.

I am curious as to what the envisioned use is for fdt_generate_phandle().

-Frank


> Signed-off-by: Thierry Reding <treding@xxxxxxxxxx>
> ---
> Changes in v3:
> - add fdt_generate_phandle() to version.lds
> 
> Changes in v2:
> - Handle the case where the number of phandles has been exhausted. Also
>   add more testing to exercise this corner case. Note that this is tied
>   to the current, naive implementation of fdt_generate_phandle(). If it
>   is ever changed to be more clevel, the tests will have to be updated.
> ---
>  libfdt/fdt_ro.c            | 31 +++++++++++++++++++++++++++++++
>  libfdt/libfdt.h            | 19 +++++++++++++++++++
>  libfdt/libfdt_env.h        |  1 +
>  libfdt/version.lds         |  1 +
>  tests/get_phandle.c        | 31 ++++++++++++++++++++++++++++++-
>  tests/multilabel.dts       |  5 +++++
>  tests/multilabel_merge.dts |  5 +++++
>  tests/references.c         | 19 +++++++++++++++++--
>  tests/references.dts       |  5 +++++
>  9 files changed, 114 insertions(+), 3 deletions(-)
> 
> diff --git a/libfdt/fdt_ro.c b/libfdt/fdt_ro.c
> index eafc14282892..97cdcda0dbec 100644
> --- a/libfdt/fdt_ro.c
> +++ b/libfdt/fdt_ro.c
> @@ -170,6 +170,37 @@ uint32_t fdt_get_max_phandle(const void *fdt)
>  	return 0;
>  }
>  
> +int fdt_generate_phandle(const void *fdt, uint32_t *phandle)
> +{
> +	uint32_t max = 0;
> +	int offset = -1;
> +
> +	while (true) {
> +		uint32_t value;
> +
> +		offset = fdt_next_node(fdt, offset, NULL);
> +		if (offset < 0) {
> +			if (offset == -FDT_ERR_NOTFOUND)
> +				break;
> +
> +			return offset;
> +		}
> +
> +		value = fdt_get_phandle(fdt, offset);
> +
> +		if (value > max)
> +			max = value;
> +	}
> +
> +	if (max == FDT_MAX_PHANDLE)
> +		return -FDT_ERR_NOPHANDLES;
> +
> +	if (phandle)
> +		*phandle = max + 1;
> +
> +	return 0;
> +}
> +
>  static const struct fdt_reserve_entry *fdt_mem_rsv(const void *fdt, int n)
>  {
>  	int offset = n * sizeof(struct fdt_reserve_entry);
> diff --git a/libfdt/libfdt.h b/libfdt/libfdt.h
> index a470d1df6d2a..7e102d188e86 100644
> --- a/libfdt/libfdt.h
> +++ b/libfdt/libfdt.h
> @@ -140,6 +140,10 @@
>  
>  #define FDT_ERR_MAX		17
>  
> +/* constants */
> +#define FDT_MAX_PHANDLE 0xfffffffe
> +	/* Valid values for phandles range from 1 to 2^32-2. */
> +
>  /**********************************************************************/
>  /* Low-level functions (you probably don't need these)                */
>  /**********************************************************************/
> @@ -376,6 +380,21 @@ const char *fdt_string(const void *fdt, int stroffset);
>   */
>  uint32_t fdt_get_max_phandle(const void *fdt);
>  
> +/**
> + * fdt_generate_phandle - return a new, unused phandle for a device tree blob
> + * @fdt: pointer to the device tree blob
> + * @phandle: return location for the new phandle
> + *
> + * Walks the device tree blob and looks for the highest phandle value. On
> + * success, the new, unused phandle value (one higher than the previously
> + * highest phandle value in the device tree blob) will be returned in the
> + * @phandle parameter.
> + *
> + * Returns:
> + *   0 on success or a negative error-code on failure
> + */
> +int fdt_generate_phandle(const void *fdt, uint32_t *phandle);
> +
>  /**
>   * fdt_num_mem_rsv - retrieve the number of memory reserve map entries
>   * @fdt: pointer to the device tree blob
> diff --git a/libfdt/libfdt_env.h b/libfdt/libfdt_env.h
> index eb2053845c9c..4d1cdfa58547 100644
> --- a/libfdt/libfdt_env.h
> +++ b/libfdt/libfdt_env.h
> @@ -52,6 +52,7 @@
>   *     EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
>   */
>  
> +#include <stdbool.h>
>  #include <stddef.h>
>  #include <stdint.h>
>  #include <stdlib.h>
> diff --git a/libfdt/version.lds b/libfdt/version.lds
> index 9f5d70880f24..5897f1e288a3 100644
> --- a/libfdt/version.lds
> +++ b/libfdt/version.lds
> @@ -66,6 +66,7 @@ LIBFDT_1.2 {
>  		fdt_resize;
>  		fdt_overlay_apply;
>  		fdt_get_string;
> +		fdt_generate_phandle;
>  	local:
>  		*;
>  };
> diff --git a/tests/get_phandle.c b/tests/get_phandle.c
> index 22bd7b81b3f0..d87aee50b065 100644
> --- a/tests/get_phandle.c
> +++ b/tests/get_phandle.c
> @@ -17,6 +17,7 @@
>   * License along with this library; if not, write to the Free Software
>   * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
>   */
> +#include <stdbool.h>
>  #include <stdlib.h>
>  #include <stdio.h>
>  #include <string.h>
> @@ -42,10 +43,32 @@ static void check_phandle(void *fdt, const char *path, uint32_t checkhandle)
>  		     path, phandle, checkhandle);
>  }
>  
> +static void check_phandle_unique(const void *fdt, uint32_t checkhandle)
> +{
> +	uint32_t phandle;
> +	int offset = -1;
> +
> +	while (true) {
> +		offset = fdt_next_node(fdt, offset, NULL);
> +		if (offset < 0) {
> +			if (offset == -FDT_ERR_NOTFOUND)
> +				break;
> +
> +			FAIL("error looking for phandle %#x", checkhandle);
> +		}
> +
> +		phandle = fdt_get_phandle(fdt, offset);
> +
> +		if (phandle == checkhandle)
> +			FAIL("generated phandle already exists");
> +	}
> +}
> +
>  int main(int argc, char *argv[])
>  {
> -	uint32_t max;
> +	uint32_t max, phandle;
>  	void *fdt;
> +	int err;
>  
>  	test_init(argc, argv);
>  	fdt = load_blob_arg(argc, argv);
> @@ -59,5 +82,11 @@ int main(int argc, char *argv[])
>  		FAIL("fdt_get_max_phandle returned 0x%x instead of 0x%x\n",
>  		     max, PHANDLE_2);
>  
> +	err = fdt_generate_phandle(fdt, &phandle);
> +	if (err < 0)
> +		FAIL("failed to generate phandle: %d", err);
> +
> +	check_phandle_unique(fdt, phandle);
> +
>  	PASS();
>  }
> diff --git a/tests/multilabel.dts b/tests/multilabel.dts
> index 77da06cc4174..d80ebe1fb082 100644
> --- a/tests/multilabel.dts
> +++ b/tests/multilabel.dts
> @@ -41,4 +41,9 @@ m1: mq: /memreserve/ 0 0x1000;
>  		n2 = &n2;
>  		n3 = &n3;
>  	};
> +
> +	node6 {
> +		linux,phandle = <0xfffffffe>;
> +		phandle = <0xfffffffe>;
> +	};
>  };
> diff --git a/tests/multilabel_merge.dts b/tests/multilabel_merge.dts
> index 3e8029897405..a27d856f80dd 100644
> --- a/tests/multilabel_merge.dts
> +++ b/tests/multilabel_merge.dts
> @@ -39,6 +39,11 @@ m1: mq: /memreserve/ 0 0x1000;
>  		n2 = &n2;
>  		n3 = &n3;
>  	};
> +
> +	node6 {
> +		linux,phandle = <0xfffffffe>;
> +		phandle = <0xfffffffe>;
> +	};
>  };
>  
>  / {
> diff --git a/tests/references.c b/tests/references.c
> index cab70f6579af..5b233a4bda20 100644
> --- a/tests/references.c
> +++ b/tests/references.c
> @@ -76,8 +76,8 @@ static void check_rref(const void *fdt)
>  int main(int argc, char *argv[])
>  {
>  	void *fdt;
> -	int n1, n2, n3, n4, n5;
> -	uint32_t h1, h2, h4, h5;
> +	int n1, n2, n3, n4, n5, n6, err;
> +	uint32_t h1, h2, h4, h5, h6, hn;
>  
>  	test_init(argc, argv);
>  	fdt = load_blob_arg(argc, argv);
> @@ -97,11 +97,15 @@ int main(int argc, char *argv[])
>  	n5 = fdt_path_offset(fdt, "/node5");
>  	if (n5 < 0)
>  		FAIL("fdt_path_offset(/node5): %s", fdt_strerror(n5));
> +	n6 = fdt_path_offset(fdt, "/node6");
> +	if (n6 < 0)
> +		FAIL("fdt_path_offset(/node6): %s", fdt_strerror(n6));
>  
>  	h1 = fdt_get_phandle(fdt, n1);
>  	h2 = fdt_get_phandle(fdt, n2);
>  	h4 = fdt_get_phandle(fdt, n4);
>  	h5 = fdt_get_phandle(fdt, n5);
> +	h6 = fdt_get_phandle(fdt, n6);
>  
>  	if (h1 != 0x2000)
>  		FAIL("/node1 has wrong phandle, 0x%x instead of 0x%x",
> @@ -109,6 +113,9 @@ int main(int argc, char *argv[])
>  	if (h2 != 0x1)
>  		FAIL("/node2 has wrong phandle, 0x%x instead of 0x%x",
>  		     h2, 0x1);
> +	if (h6 != FDT_MAX_PHANDLE)
> +		FAIL("/node6 has wrong phandle, 0x%x instead of 0x%x",
> +		     h6, FDT_MAX_PHANDLE);
>  	if ((h4 == 0x2000) || (h4 == 0x1) || (h4 == 0))
>  		FAIL("/node4 has bad phandle, 0x%x", h4);
>  
> @@ -117,6 +124,14 @@ int main(int argc, char *argv[])
>  	if ((h5 == h4) || (h5 == h2) || (h5 == h1))
>  		FAIL("/node5 has duplicate phandle, 0x%x", h5);
>  
> +	/*
> +	 * /node6 has phandle FDT_MAX_PHANDLE, so fdt_generate_phandle() is
> +	 * expected to fail.
> +	 */
> +	err = fdt_generate_phandle(fdt, &hn);
> +	if (err != -FDT_ERR_NOPHANDLES)
> +		FAIL("generated invalid phandle 0x%x\n", hn);
> +
>  	check_ref(fdt, n1, h2);
>  	check_ref(fdt, n2, h1);
>  	check_ref(fdt, n3, h4);
> diff --git a/tests/references.dts b/tests/references.dts
> index f783e8bc8643..b39063999189 100644
> --- a/tests/references.dts
> +++ b/tests/references.dts
> @@ -33,4 +33,9 @@
>  		linux,phandle = <&n5>;
>  		phandle = <&n5>;
>  	};
> +
> +	node6 {
> +		linux,phandle = <0xfffffffe>;
> +		phandle = <0xfffffffe>;
> +	};
>  };
> 




[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