On Tue, Mar 19, 2019 at 12:20:09PM +0100, 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.
>
> Signed-off-by: Thierry Reding <treding@xxxxxxxxxx>
Looks pretty good.
> ---
> libfdt/fdt_ro.c | 28 ++++++++++++++++++++++++++++
> libfdt/libfdt.h | 15 +++++++++++++++
> libfdt/libfdt_env.h | 1 +
> tests/get_phandle.c | 31 ++++++++++++++++++++++++++++++-
> 4 files changed, 74 insertions(+), 1 deletion(-)
>
> diff --git a/libfdt/fdt_ro.c b/libfdt/fdt_ro.c
> index eafc14282892..cfb8499ecbd0 100644
> --- a/libfdt/fdt_ro.c
> +++ b/libfdt/fdt_ro.c
> @@ -170,6 +170,34 @@ 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 (phandle)
> + *phandle = max + 1;
Tiny nit: if the tree already has the maximum permitted phandle in it
(0xfffffffe), this will set *phandle to -1 (0xfffffff) and return
success. -1 is not a valid phandle, so you should return an error in
that case.
> + 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..cfe7e5fe3efe 100644
> --- a/libfdt/libfdt.h
> +++ b/libfdt/libfdt.h
> @@ -376,6 +376,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/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();
> }
--
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
