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>;
> + };
> };
>
