Re: [PATCH for-4.5 V6 14/14] Add ARM EFI boot support

[Julien Grall <julien.grall@linaro.org>]

Hi Roy,

On 09/24/2014 06:03 AM, Roy Franz wrote:
> +static int __init fdt_set_reg(void *fdt, int node, int addr_cells,
> +                              int size_cells, uint64_t addr, uint64_t len)
> +{
> +    uint8_t data[16]; /* at most 2 64 bit words */
> +    void *p = data;
> +
> +    /* Make sure that the values provided can be represented in
> +     * the reg property.
> +     */
> +    if ( addr_cells == 1 && (addr >> 32) )
> +        return -1;
> +    if ( size_cells == 1 && (len >> 32) )
> +        return -1;
> +
> +    if ( addr_cells == 1 )
> +    {
> +        *(uint32_t *)p = cpu_to_fdt32(addr);
> +        p += sizeof(uint32_t);
> +    }
> +    else if ( addr_cells == 2 )
> +    {
> +        *(uint64_t *)p = cpu_to_fdt64(addr);
> +        p += sizeof(uint64_t);
> +    }
> +    else
> +        return -1;
> +
> +    if ( size_cells == 1 )
> +    {
> +        *(uint32_t *)p = cpu_to_fdt32(len);
> +        p += sizeof(uint32_t);
> +    }
> +    else if ( size_cells == 2 )
> +    {
> +        *(uint64_t *)p = cpu_to_fdt64(len);
> +        p += sizeof(uint64_t);
> +    }
> +    else
> +        return -1;

You could give a look to dt_set_cell (xen/common/device_tree.c). I think
it will fit to your usage and avoid open coding size in this function.

> diff --git a/xen/common/efi/runtime.c b/xen/common/efi/runtime.c
> index 166852d..ac9deae 100644
> --- a/xen/common/efi/runtime.c
> +++ b/xen/common/efi/runtime.c
> @@ -4,17 +4,21 @@
>  #include <xen/guest_access.h>
>  #include <xen/irq.h>
>  #include <xen/time.h>
> -#include <asm/mc146818rtc.h>

NIT: Missing a blank line here.

Regards,

-- 
Julien Grall