From mboxrd@z Thu Jan  1 00:00:00 1970
From: Simon Glass <sjg@chromium.org>
Date: Mon, 18 Aug 2014 12:06:26 -0600
Subject: [U-Boot] [PATCH 03/23] fdt: Add resource parsing functions
In-Reply-To: <1408346196-30419-4-git-send-email-thierry.reding@gmail.com>
References: <1408346196-30419-1-git-send-email-thierry.reding@gmail.com>
	<1408346196-30419-4-git-send-email-thierry.reding@gmail.com>
Message-ID: <CAPnjgZ2X6q29ZwjpsohXoNVvu71YOg_PEPpciHLg87jK58s3nQ@mail.gmail.com>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

Hi Thierry,

On 18 August 2014 01:16, Thierry Reding <thierry.reding@gmail.com> wrote:
> From: Thierry Reding <treding@nvidia.com>
>
> Add the fdt_get_resource() and fdt_get_named_resource() functions which
> can be used to parse resources (memory regions) from an FDT. A helper to
> compute the size of a region is also provided.
>
> Signed-off-by: Thierry Reding <treding@nvidia.com>
> ---
>  include/fdtdec.h | 48 ++++++++++++++++++++++++++++++++++++++++++++++++
>  lib/fdtdec.c     | 43 +++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 91 insertions(+)
>
> diff --git a/include/fdtdec.h b/include/fdtdec.h
> index 856e6cf766de..e9091eee6bae 100644
> --- a/include/fdtdec.h
> +++ b/include/fdtdec.h
> @@ -40,6 +40,23 @@ struct fdt_memory {
>         fdt_addr_t end;
>  };
>
> +/* information about a resource */

Please add comments, e.g. that end is inclusive.

> +struct fdt_resource {
> +       fdt_addr_t start;
> +       fdt_addr_t end;
> +};
> +
> +/**
> + * Compute the size of a resource.
> + *
> + * @param res  the resource to operate on
> + * @return the size of the resource
> + */
> +static inline fdt_size_t fdt_resource_size(const struct fdt_resource *res)
> +{
> +       return res->end - res->start + 1;
> +}
> +
>  /**
>   * Compat types that we know about and for which we might have drivers.
>   * Each is named COMPAT_<dir>_<filename> where <dir> is the directory
> @@ -583,4 +600,35 @@ struct fmap_entry {
>   */
>  int fdtdec_read_fmap_entry(const void *blob, int node, const char *name,
>                            struct fmap_entry *entry);
> +
> +/**
> + * Obtain an indexed resource from a device property.
> + *
> + * @param fdt          FDT blob
> + * @param node         node to examine
> + * @param property     name of the property to parse
> + * @param index                index of the resource to retrieve
> + * @param res          returns the resource
> + * @return 0 if ok, negative on error
> + */
> +int fdt_get_resource(const void *fdt, int node, const char *property,
> +                    unsigned int index, struct fdt_resource *res);
> +
> +/**
> + * Obtain a named resource from a device property.
> + *
> + * Look up the index of the name in a list of strings and return the resource
> + * at that index.
> + *
> + * @param fdt          FDT blob
> + * @param node         node to examine
> + * @param property     name of the property to parse
> + * @param names                name of the property to obtain the match the name to
> + * @param name         the name of the entry to look up

These two parameters are confusing. Perhaps rename names to something better?

> + * @param res          returns the resource
> + */
> +int fdt_get_named_resource(const void *fdt, int node, const char *property,
> +                          const char *names, const char *name,
> +                          struct fdt_resource *res);
> +
>  #endif
> diff --git a/lib/fdtdec.c b/lib/fdtdec.c
> index eb5aa20526fd..fbfae4a7cbaf 100644
> --- a/lib/fdtdec.c
> +++ b/lib/fdtdec.c
> @@ -691,4 +691,47 @@ int fdtdec_read_fmap_entry(const void *blob, int node, const char *name,
>
>         return 0;
>  }
> +
> +int fdt_get_resource(const void *fdt, int node, const char *property,
> +                    unsigned int index, struct fdt_resource *res)

s/index/find_index/

> +{
> +       const fdt32_t *ptr, *end;
> +       unsigned int i = 0;
> +       int na, ns, len;
> +
> +       na = fdt_n_addr_cells(fdt, node);
> +       ns = fdt_n_size_cells(fdt, node);
> +
> +       ptr = fdt_getprop(fdt, node, property, &len);
> +       if (!ptr)
> +               return len;
> +
> +       end = ptr + len / 4;

sizeof(*ptr) might be better than 4.

> +
> +       while (ptr + na + ns <= end) {
> +               if (i == index) {
> +                       res->start = fdt_addr_to_cpu(*ptr);

This doesn't deal with 64-bit addresses. There is a half-hearted
attempt with fdt_addr_t, but I wonder if we need a helper function
like fdt_get_addr(ptr, num_cells)?

> +                       res->end = res->start + fdt_size_to_cpu(ptr[na]) - 1;
> +                       return 0;
> +               }
> +
> +               ptr += na + ns;
> +               i++;
> +       }
> +
> +       return -FDT_ERR_NOTFOUND;
> +}
> +
> +int fdt_get_named_resource(const void *fdt, int node, const char *property,
> +                          const char *names, const char *name,
> +                          struct fdt_resource *res)
> +{
> +       int index;
> +
> +       index = fdt_get_string_index(fdt, node, names, name);
> +       if (index < 0)
> +               return index;
> +
> +       return fdt_get_resource(fdt, node, property, index, res);
> +}
>  #endif
> --
> 2.0.4
>

Regards,
Simon