Re: [Minios-devel] [UNIKRAFT PATCHv2 2/8] lib/fdt: Introduce a helper of fdt_get_cells

[Julien Grall <julien.grall@xxxxxxx>]

Hi,

On 11/03/2019 09:14, Jia He wrote:
> From: Wei Chen <wei.chen@xxxxxxx>
> In device tree, we always use fdt_address_cells, fdt_size_cells
> and fdt_interrupt_cells to get cells' size. fdt_get_cells will
> reduce the redundant code for this *_cells APIs.
> If fdt_get_cells could not find the property in current node,
> in previous implement, it will return 2 arbitrarily. In this patch
> it will try to find the property in parent node.
This patch is actually introducing fdt_get_cells. So which previous 
implementation are you talking about?
Also, 2 is not arbitrary, this is based on the spec (see 2.3.5 in [1]). 
Although, technically for #size-cells, you should return 1 here.
> Jira: ENTOS-860
What is it for?

> Signed-off-by: Wei Chen <wei.chen@xxxxxxx>
> Signed-off-by: Jianyong Wu <jianyong.wu@xxxxxxx>
> Signed-off-by: Jia He <justin.he@xxxxxxx>
> ---
>   lib/fdt/fdt_ro.c         | 22 ++++++++++++++++++++++
>   lib/fdt/include/libfdt.h | 20 ++++++++++++++++++++
>   2 files changed, 42 insertions(+)
>
> diff --git a/lib/fdt/fdt_ro.c b/lib/fdt/fdt_ro.c
> index 9077cff..9afede3 100644
> --- a/lib/fdt/fdt_ro.c
> +++ b/lib/fdt/fdt_ro.c
> @@ -456,6 +456,28 @@ int fdt_getprop_u32_by_offset(const void *fdt, int offset,
>         return -FDT_ERR_NOTFOUND;
>   }
>   
> +int fdt_get_cells(const void *fdt, const char *prop, int nodeoffset)
> +{
> +       int val;
> +
> +       do {
> +               /* Find whether the property exists in this node */
> +               if (!fdt_getprop_u32_by_offset(fdt, nodeoffset,
> +                               prop, (uint32_t *)&val))
> +                       break;
> +               /* If not, try to find in parent node */
> +               nodeoffset = fdt_parent_offset(fdt, nodeoffset);
> +       } while (nodeoffset >= 0);
> +
> +       if (nodeoffset < 0)
> +               return nodeoffset;
> +
> +       if ((val <= 0) || (val > FDT_MAX_NCELLS))
> +               return -FDT_ERR_BADNCELLS;
> +
> +       return val;
> +}
> +
>   int fdt_supernode_atdepth_offset(const void *fdt, int nodeoffset,
>                                  int supernodedepth, int *nodedepth)
>   {
> diff --git a/lib/fdt/include/libfdt.h b/lib/fdt/include/libfdt.h
> index 1383f42..bda6456 100644
> --- a/lib/fdt/include/libfdt.h
> +++ b/lib/fdt/include/libfdt.h
> @@ -1082,6 +1082,26 @@ const char *fdt_stringlist_get(const void *fdt, int 
> nodeoffset,
>    */
>   #define FDT_MAX_NCELLS                4
>   
> +/**
> + * fdt_get_cells - retrieve cell size for a bus represented in the tree
> + * @fdt: pointer to the device tree blob
> + * @prop: cell name of the property containing the string list
> + * @nodeoffset: offset of the node to find the address size for
> + *
> + * When the node has a valid #address-cells property, returns its value.
> + *
> + * returns:
> + *     0 <= n < FDT_MAX_NCELLS, on success
> + *      -FDT_ERR_BADNCELLS, if the node has a badly formatted or invalid
> + *             #address-cells property
> + *     -FDT_ERR_BADMAGIC,
> + *     -FDT_ERR_BADVERSION,
> + *     -FDT_ERR_BADSTATE,
> + *     -FDT_ERR_BADSTRUCTURE,
> + *     -FDT_ERR_TRUNCATED, standard meanings
> + */
> +int fdt_get_cells(const void *fdt, const char *prop, int nodeoffset);
> +
>   /**
>    * fdt_address_cells - retrieve address size for a bus represented in the tree
>    * @fdt: pointer to the device tree blob
Cheers,

[1] https://github.com/devicetree-org/devicetree-specification/releases/tag/v0.2


--
Julien Grall

_______________________________________________
Minios-devel mailing list
Minios-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/minios-devel