[U-Boot] [PATCH] cmd/fdt: Make fdt get value endian-safe for single-cell properties

[Simon Glass]

Hi Andreas,

On 15 November 2016 at 17:18, Simon Glass <sjg at chromium.org> wrote:
> Hi Andreas,
>
> On 15 November 2016 at 10:07, Andreas Färber <afaerber at suse.de> wrote:
>> Hi Simon,
>>
>> Am 28.10.2016 um 03:52 schrieb Simon Glass:
>>> On 27 October 2016 at 05:44, Andreas Färber <afaerber at suse.de> wrote:
>>>> Am 26.10.2016 um 21:19 schrieb Simon Glass:
>>>>> On 26 October 2016 at 09:02, Andreas Färber <afaerber at suse.de> wrote:
>>>>>> On a Raspberry Pi 2 disagreements on cell endianness can be observed:
>>>>>>
>>>>>>   U-Boot> fdt print /soc/gpio at 7e200000 phandle
>>>>>>   phandle = <0x0000000d>
>>>>>>   U-Boot> fdt get value myvar /soc/gpio at 7e200000 phandle; printenv myvar
>>>>>>   myvar=0x0D000000
>>>>>>
>>>>>> Fix this by always treating the pointer as __be32 and converting it in
>>>>>> fdt_value_setenv(), like its counterpart fdt_parse_prop() already does.
>>>>>>
>>>>>> Fixes: bc80295 ("fdt: Add get commands to fdt")
>>>>>> Cc: Joe Hershberger <joe.hershberger at ni.com>
>>>>>> Cc: Gerald Van Baren <gvb at unssw.com>
>>>>>> Signed-off-by: Andreas Färber <afaerber at suse.de>
>>>>>> ---
>>>>>>  cmd/fdt.c | 2 +-
>>>>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>>>
>>>>> The patch looks good, but please can you add a function comment to
>>>>> fdt_value_setenv() ?
>>>>
>>>> Where and what should it say? There is a general comment above the
>>>> function already, and by choosing __be32* instead of uint32_t* I thought
>>>> my code was fairly self-documenting.
>>>
>>> It should explain what the function does and, importantly, what the
>>> arguments and return value are.
>>
>> Can you please do general documentation updates in a separate patch on
>> top of this bug fix, so that we can get this merged soonish?
>
> Actually that is what I was asking you to do. A separate patch is fine :-)

Are you able to respin this or send another patch? If we are going to
get this applied for he next release, now is the time...

Also I think it is better to use fdt32_to_cpu() instead __be32_to_cpu().

Regards,
Simon