[U-Boot] [PATCH v3 6/6] fdt: add decode helper library

[Simon Glass]

Hi Mike,

On Thu, Oct 13, 2011 at 1:33 PM, Mike Frysinger <vapier at gentoo.org> wrote:
> On Tuesday 11 October 2011 18:26:11 Simon Glass wrote:
>> --- /dev/null
>> +++ b/include/fdtdec.h
>>
>> +/*
>> + * A typedef for a physical address. Note that fdt data is always big
>> + * endian even on a litle endian machine.
>> + */
>> +#ifdef CONFIG_PHYS_64BIT
>> +typedef u64 addr_t;
>> +#define ADDR_T_NONE (-1ULL)
>> +#define addr_to_cpu(reg) be64_to_cpu(reg)
>> +#else
>> +typedef u32 addr_t;
>> +#define ADDR_T_NONE (-1U)
>> +#define addr_to_cpu(reg) be32_to_cpu(reg)
>> +#endif
>
> "addr" is fairly generic.  how about "fdt_addr" instead ?

OK

>
>> --- /dev/null
>> +++ b/lib/fdtdec.c
>>
>> +/* we need a generic GPIO interface here */
>> +#include <asm/arch/gpio.h>
>
> we have asm/gpio.h now, although i don't see this code using anything from the
> gpio header to need this include ...

Will remove it.

>
>> +static const char *compat_names[COMPAT_COUNT] = {
>
> static const char * const compat_names[COMPAT_COUNT] = {
>
>> +int fdtdec_next_alias(const void *blob, const char *name,
>> +             enum fdt_compat_id id, int *upto)
>> +{
>> +#define MAX_STR_LEN 20
>> +     char str[MAX_STR_LEN + 20];
>> +     int node, err;
>> +
>> +     sprintf(str, "%.*s%d", MAX_STR_LEN, name, *upto);
>
> where's that "20" coming from ?  just arbitrarily defined ?  might want to add
> an assert(strlen(name) <= MAX_STR_LEN).
> -mike
>

OK. Of course I would like to use snprintf()...

Regards,
Simon