From mboxrd@z Thu Jan 1 00:00:00 1970 From: Thierry Reding Date: Mon, 11 Mar 2019 10:27:21 +0100 Subject: [U-Boot] [PATCH 2/6] fdtdec: Implement fdtdec_get_max_phandle() In-Reply-To: References: <20190308201140.2383-1-thierry.reding@gmail.com> <20190308201140.2383-2-thierry.reding@gmail.com> Message-ID: <20190311092721.GA11545@ulmo> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de On Sun, Mar 10, 2019 at 03:51:31PM -0600, Simon Glass wrote: > Hi Thierry, > > On Fri, 8 Mar 2019 at 13:11, Thierry Reding wrote: > > > > From: Thierry Reding > > > > This function allows looking up the highest phandle value stored in a > > device tree, which is useful to determine the next best phandle value > > for new nodes. > > > > Signed-off-by: Thierry Reding > > --- > > include/fdtdec.h | 12 ++++++++++++ > > lib/fdtdec.c | 28 ++++++++++++++++++++++++++++ > > 2 files changed, 40 insertions(+) > > Can we use fdt_get_max_phandle() instead? If not, could you please add > a bit more detail to the commit message as we might consider changing > the upstream function. fdt_get_max_phandle() has a slightly awkward signature. It returns the phandle via the return value, which means that it can distinguish between error conditions and also uses 0 and -1 to signal no phandle found and error conditions, respectively. Using the function requires weird looking code like this: uint32_t phandle; phandle = fdt_get_max_phandle(fdt); if (phandle == 0 || phandle == (uint32_t)-1) /* process error */; So the goal here was to add an alternative that would provide a more standard interface where a regular error could be returned via the return value and the phandle would be stored in an output parameter on success. I specifically didn't want to update the upstream function because it was introduced about 2.5 years ago and will probably be used by some number of users. I'm not sure if there's a stable API policy for libfdt, but I would expect a lot of users to be annoyed if we just changed the signature of fdt_get_max_phandle(). That said, perhaps another alternative would be to implement this as a different function. If you look at the subsequent patches, the goal is to generate a new phandle for newly added nodes, so perhaps something like this could work: int fdtdec_generate_phandle(const void *fdt, uint32_t *phandle); That would be slightly more in line with the higher level of fdtdec compared to libfdt. Or perhaps you'd prefer fdt_generate_phandle() in libfdt? Thierry -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 833 bytes Desc: not available URL: