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

[Roy Franz <roy.franz@xxxxxxxxxx>]

On Wed, Sep 24, 2014 at 9:11 AM, Julien Grall <julien.grall@xxxxxxxxxx> wrote:
> 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.

I think this will work - I'll take a closer look to confirm.  This
would clean this code
up a fair bit.

>
>> 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.

I'll fix this.

Thanks,
Roy

>
> Regards,
>
> --
> Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel