[U-Boot] [PATCH 03/23] fdt: Add resource parsing functions

[Simon Glass]

Hi Thierry,

On 19 August 2014 07:12, Thierry Reding <thierry.reding at gmail.com> wrote:
> On Tue, Aug 19, 2014 at 06:55:18AM -0600, Simon Glass wrote:
>> On 19 August 2014 05:35, Thierry Reding <thierry.reding at gmail.com> wrote:
> [...]
>> > The Linux kernel handles this by wrapping it in an of_read_number()
>> > helper and always returning u64, like this:
>> >
>> >         static inline u64 of_read_number(const __be32 *cell, int size)
>> >         {
>> >                 u64 r = 0;
>> >
>> >                 while (size--)
>> >                         r = (r << 32) | be32_to_cpu(*(cell++));
>> >
>> >                 return r;
>> >         }
>> >
>> > It obviously only works for size in { 0, 1, 2 }, but I can add a helper
>> > like that if you want.
>>
>>
>> That looks good. It works for the cases we need and it's obvious later
>> where the logic is if we want to extend it.
>
> This is what I have for U-Boot currently.
>
> static fdt_addr_t fdtdec_get_address(const fdt32_t *ptr, unsigned int cells)
> {
>         fdt_addr_t addr = 0;
>
>         while (cells--)
>                 /* do the shift in two steps to avoid warning on 32-bit */
>                 addr = (addr << 16) << 16 | fdt32_to_cpu(*ptr++);

Odd warning! Does 32UL help?

>
>         return addr;
> }
>
> I use the same function to parse the size field of reg entries, even
> though the types aren't technically the same. But making the types
> consistent would duplicate the above exactly, so I'm not sure it's worth
> it.

Seems fine.

>
> Alternatively perhaps something like this could be done:
>
>         static u64 fdtdec_get_number(const fdt32_t *ptr, unsigned int cells)
>         {
>                 /* the above */
>         }
>
>         static fdt_addr_t fdtdec_get_address(const fdt32_t *ptr, unsigned int cells)
>         {
>                 return fdtdec_get_number(ptr, cells);
>         }
>
>         static fdt_size_t fdtdec_get_size(const fdt32_t *ptr, unsigned int cells)
>         {
>                 return fdtdec_get_number(ptr, cells);
>         }
>
> Again, I'm not sure it's really worth the trouble since fdt_addr_t and
> fdt_size_t are both always either u32 or u64.

Yes, although I suppose ultimately these might be 64-bit always,
Perhaps we should merge the types?

Regards,
Simon