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