Hi Stephen, On Mon, Nov 28, 2011 at 10:41 AM, Stephen Warren <swar...@nvidia.com> wrote: > On 11/23/2011 08:54 PM, Simon Glass wrote: >> Add a function to lookup a property which is a phandle in a node, and >> another to read a fixed-length integer array from an fdt property. >> Also add a function to read boolean properties. >> >> Signed-off-by: Simon Glass <s...@chromium.org> > > Looking at the U-Boot custodians web page, you need to send the core DT > changes (well, probably anything DT related) to Jerry Van Baren.
Yes the tag was there but not picked up as I didn't have Mike's alias file in my tree. Will fix. > >> +/** >> + * Look up a property in a node and return its contents in an integer >> + * array of given length. The property must have at least enough data for >> + * the array (4*count bytes). It may have more, but this will be ignored. >> + * >> + * @param blob FDT blob >> + * @param node node to examine >> + * @param prop_name name of property to find >> + * @param array array to fill with data >> + * @param count number of array elements >> + * @return 0 if ok, or -FDT_ERR_NOTFOUND if the property is not found, >> + * or -FDT_ERR_BADLAYOUT if not enough data >> + */ >> +int fdtdec_get_int_array(const void *blob, int node, const char *prop_name, >> + int *array, int count); > > The kernel's equivalent of this function retrieves an array of U32s. Is > one version more correct than the other? I would prefer to have signed, but I will change it to use u32 *. > >> +/** >> + * Look up a boolean property in a node and return it. >> + * >> + * A boolean properly is true if present in the device tree and false if not >> + * present, or present with a 0 value. >> + * >> + * @param blob FDT blob >> + * @param node node to examine >> + * @param prop_name name of property to find >> + * @return 1 if the properly is present; 0 if it isn't present or is 0 >> + */ >> +int fdtdec_get_bool(const void *blob, int node, const char *prop_name); > > Does U-Boot allow use of the "bool" type here? Which bool type? It is returning an int. > > >> +/** >> + * Look up a property in a node and check that it has a minimum length. >> + * >> + * @param blob FDT blob >> + * @param node node to examine >> + * @param prop_name name of property to find >> + * @param min_len minimum property length in bytes >> + * @param err 0 if ok, or -FDT_ERR_NOTFOUND if the property >> is not >> + found, or -FDT_ERR_BADLAYOUT if not enough data >> + * @return pointer to cell, which is only valid if err == 0 >> + */ >> +static const void *get_prop_len(const void *blob, int node, >> + const char *prop_name, int min_len, int *err) > > Based on the function name, I'd expect it to return the length of the > property; perhaps get_prop_check_min_len? Changed, thanks. > >> +/** >> + * Look up a boolean property in a node and return it. >> + * >> + * A boolean properly is true if present in the device tree and false if not >> + * present, or present with a 0 value. >> + * >> + * @param blob FDT blob >> + * @param node node to examine >> + * @param prop_name name of property to find >> + * @return 1 if the properly is present; 0 if it isn't present or is 0 >> + */ >> +int fdtdec_get_bool(const void *blob, int node, const char *prop_name) >> +{ >> + const s32 *cell; >> + int len; >> + >> + debug("%s: %s\n", __func__, prop_name); >> + cell = fdt_getprop(blob, node, prop_name, &len); >> + if (!cell) >> + return 0; >> + if (len >= sizeof(u32) && *cell == 0) >> + return 0; >> + >> + return 1; >> +} > > In the kernel, I believe that property existence is all that's usually > checked. Is that wrong? Did the definition of a boolean property's value > in the function description above come from the specification? If a > property had a length of 0/1/2/3 with a zero value, it seems very odd to > treat that as true. It is useful to be able to set the value to 0 or 1 (with fdtget/put), rather than remove or add the property. A value with a length of less than one cell is considered illegal here. The basic idea is that the presence of the property means that it is 'true'. If it happens to have a value, then we allow that to specify 'false' if it is zero. Thoughts? Regards, Simon > > -- > nvpublic > _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot