Re: [Minios-devel] [UNIKRAFT PATCHv4 7/9] plat/common: Introduce fdt_node_offset_by_compatible_list helper

[Sharan Santhanam <sharan.santhanam@xxxxxxxxx>]

Hello Jia he,

Please find the comments inline:

Thanks & Regards
Sharan

On 3/27/19 3:34 AM, Jia He wrote:
> From: Jianyong Wu <jianyong.wu@xxxxxxx>
> For most of devices that will use the device tree will have one
> or more compatible strings. This helper will avoid implement a
> function to match compatible list everywhere.
> The valid compatible strings array should be ended with NULL.
> E.g.
> static const char *gic_device_list[] = {
>         "arm,cortex-a15-gic",
>         "arm,cortex-a7-gic",
>         "arm,cortex-a9-gic",
>         NULL}
>
> Signed-off-by: Wei Chen <wei.chen@xxxxxxx>
> Signed-off-by: Jianyong Wu <jianyong.wu@xxxxxxx>
> Signed-off-by: Jia He <justin.he@xxxxxxx>
> ---
>   plat/common/fdt.c         | 15 +++++++++++++++
>   plat/common/include/fdt.h | 26 ++++++++++++++++++++++++++
>   2 files changed, 41 insertions(+)
>
> diff --git a/plat/common/fdt.c b/plat/common/fdt.c
> index 6cbedec..1707ef2 100644
> --- a/plat/common/fdt.c
> +++ b/plat/common/fdt.c
> @@ -327,3 +327,18 @@ int fdt_get_address(const void *fdt, int nodeoffset, 
> uint32_t index,
>   
>   	return 0;
>   }
> +
> +int fdt_node_offset_by_compatible_list(const void *fdt, int startoffset,
> +                                 const char *compatibles[])
> +{
> +       int idx, offset;
> +
> +       for (idx = 0; compatibles[idx] != NULL; idx++) {
> +               offset = fdt_node_offset_by_compatible(fdt, startoffset,
> +                                 compatibles[idx]);
> +               if (offset >= 0)
> +                       return offset;
> +       }
> +
Shouldn't this be -FDT_ERR_NOTFOUND as positive value are valid offset?

> +       return FDT_ERR_NOTFOUND;
> +}
> diff --git a/plat/common/include/fdt.h b/plat/common/include/fdt.h
> index 27f4f2c..be6b265 100644
> --- a/plat/common/include/fdt.h
> +++ b/plat/common/include/fdt.h
> @@ -128,4 +128,30 @@ int fdt_size_cells_or_parent(const void *fdt, int 
> nodeoffset);
>   int fdt_get_address(const void *fdt, int nodeoffset, int index,
>                         uint64_t *addr, uint64_t *size);
>   
> +/**
> + * fdt_node_offset_by_compatible_list - find nodes with a given
> + *                                     'compatible' list value
> + * @fdt: pointer to the device tree blob
> + * @startoffset: only find nodes after this offset
> + * @compatibles: a list of 'compatible' string to match, should be ended
> + * with NULL string.
> + * fdt_node_offset_by_compatible_list() returns the offset of the
> + * first matched node after startoffset, which has a 'compatible'
> + * property which lists the given compatible string; or if
> + * startoffset is -1, the very first such node in the tree.
> + *
> + * returns:
> + *     structure block offset of the located node (>= 0, >startoffset),
> + *              on success
> + *     -FDT_ERR_NOTFOUND, no node matching the criterion exists in the
> + *             tree after startoffset
> + *     -FDT_ERR_BADOFFSET, nodeoffset does not refer to a BEGIN_NODE tag
> + *     -FDT_ERR_BADMAGIC,
> + *     -FDT_ERR_BADVERSION,
> + *     -FDT_ERR_BADSTATE,
> + *     -FDT_ERR_BADSTRUCTURE, standard meanings
> + */
> +int fdt_node_offset_by_compatible_list(const void *fdt, int startoffset,
> +                                       const char *compatibles[]);
> +
>   #endif
_______________________________________________
Minios-devel mailing list
Minios-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/minios-devel