On Mon, Oct 08, 2012 at 02:59:18PM +0100, Jon Medhurst (Tixy) wrote:
> From: Jon Medhurst <tixy@xxxxxxxxxx>
>
> A subsequent patch will also need to obtain address-cells and
> size-cells, so lets factor out this code into a handy function.
The code looks ok. There are a couple of stylistic things that could be
tweaked, but it's not a big issue.
Cheers
---Dave
>
> Signed-off-by: Jon Medhurst <tixy@xxxxxxxxxx>
> ---
> semi_loader.c | 56 ++++++++++++++++++++++++++++++++++----------------------
> 1 file changed, 34 insertions(+), 22 deletions(-)
>
> diff --git a/semi_loader.c b/semi_loader.c
> index cbe911c..c9750be 100644
> --- a/semi_loader.c
> +++ b/semi_loader.c
> @@ -97,6 +97,39 @@ static int _fdt_make_node(void *fdt, int parentoffset, const char *name)
> return fdt_add_subnode(fdt, parentoffset, name);
> }
>
> +static void _fdt_address_and_size_cells(void *fdt, int* addrcells, int* sizecells)
Minor quibble: the rest of the code uses the type *decl spacing, not
type* decl.
> +{
> + int e;
> + uint32_t const *p;
> +
> + if(!(p = fdt_getprop(fdt, 0, "#address-cells", &e)))
> + goto libfdt_error;
> + else if(e != 4)
> + goto size_error;
Another minor quibble: this could be simplified slightly by dropping any
else which directly follows a goto.
(The superfluous elses were in the original code too, though.)
> + *addrcells = fdt32_to_cpu(*p);
> + if(!(p = fdt_getprop(fdt, 0, "#size-cells", &e)))
> + goto libfdt_error;
> + else if(e != 4)
> + goto size_error;
> + *sizecells = fdt32_to_cpu(*p);
> +
> + /*
> + * Sanity-check address sizes, since addresses and sizes which do
> + * not take up exactly 4 or 8 bytes are not supported.
> + */
> + if ((*addrcells != 1 && *addrcells != 2) ||
> + (*sizecells != 1 && *sizecells != 2))
> + goto size_error;
> +
> + return;
> +
> +libfdt_error:
> + fatal("libfdt: ", fdt_strerror(e), ", while looking for #address-cells/#size-cells\n");
> +
> +size_error:
> + fatal("Unexpected/invalid #address-cells/#size-cells in device tree\n");
> +}
> +
> static void update_fdt(void **dest, struct loader_info *info)
> {
> int e;
> @@ -112,25 +145,7 @@ static void update_fdt(void **dest, struct loader_info *info)
> if((e = fdt_open_into((void *)info->fdt_start, fdt, FDT_SIZE_MAX)) < 0)
> goto libfdt_error;
>
> - /*
> - * Sanity-check address sizes, since addresses and sizes which do
> - * not take up exactly 4 or 8 bytes are not supported.
> - */
> - {
> - if(!(p = fdt_getprop(fdt, 0, "#address-cells", &e)))
> - goto libfdt_error;
> - else if(e != 4)
> - goto size_error;
> - addrcells = fdt32_to_cpu(*p);
> - if(!(p = fdt_getprop(fdt, 0, "#size-cells", &e)))
> - goto libfdt_error;
> - else if(e != 4)
> - goto size_error;
> - sizecells = fdt32_to_cpu(*p);
> - if ((addrcells != 1 && addrcells != 2) ||
> - (sizecells != 1 && sizecells != 2))
> - goto size_error;
> - }
> + _fdt_address_and_size_cells(fdt, &addrcells, &sizecells);
>
> /*
> * Add a memory node, but only if there isn't one already. If
> @@ -225,9 +240,6 @@ no_add_memory:
>
> libfdt_error:
> fatal("libfdt: ", fdt_strerror(e), ", while updating device tree\n");
> -
> -size_error:
> - fatal("Unexpected/invalid #address-cells/#size-cells in device tree\n");
> }
>
> static int is_space(char c)
> --
> 1.7.10.4
>
_______________________________________________
kvmarm mailing list
kvmarm@xxxxxxxxxxxxxxxxxxxxx
https://lists.cs.columbia.edu/cucslists/listinfo/kvmarm
