[U-Boot] [PATCH] Revert "fdt: Fix fdtdec_get_addr_size() for 64-bit"

[Stephen Warren]

On 08/06/2015 01:09 AM, Michal Suchanek wrote:
> Hello,
>
> On 6 August 2015 at 01:45, Simon Glass <sjg at chromium.org> wrote:
>> Hi Stephen,
>>
>> On 5 August 2015 at 12:22, Stephen Warren <swarren at wwwdotorg.org> wrote:
>>> On 08/04/2015 10:08 PM, Simon Glass wrote:
>>>>
>>>> Hi Stephen,
>>>>
>>>> On 3 August 2015 at 12:20, Stephen Warren <swarren at wwwdotorg.org> wrote:
>>>>>
>>>>> On 08/03/2015 09:52 AM, Simon Glass wrote:
>>>>>>
>>>>>>
>>>>>> Hi Stephen,
>>>>>>
>>>>>> On 3 August 2015 at 09:12, Stephen Warren <swarren at wwwdotorg.org> wrote:
>>>>>>>
>>>>>>>
>>>>>>> On 08/02/2015 06:13 PM, Simon Glass wrote:
>>>>>>>>
>>>>>>>>
>
>>>> This is a boot loader so we should be willing to make some
>>>> simplifications.
>>>
>>>
>>> When dealing with internal bootloader details, sure assumptions,
>>> simplifications, etc. can be made.
>>>
>>> However, DT is an externally defined standard. The content of DT must be
>>> identical across all OSs (SW stacks, bootloader) and not influenced by
>>> requirements/... of any specific individual OS's (SW stack, bootloader)
>>> quirks. We can't just pick and choose which parts of it we care about. Well,
>>> perhaps if we stop calling it DT we could.
>>
>> So I think in summary:
>>
>> - 64-bit machines should have CONFIG_PHYS_64BIT set correctly
>> - then fdtdec_get_addr_size() would work as expected
>> - I want to make this cases as efficient as possible since it will be
>> called in SPL
>> - You are concerned that making assumptions like this violates the DT spec
>>
>> One option is to split the functions into two - one that works in SPL
>> and makes assumptions, and one that does not and does things the hard
>> way. I suppose we could also add checks/warnings that the DT is
>> 'well-formed' and that the address size matches the machine it is
>> running on.
>
> This depends on the way the binding is defined. If the binding
> description says that the value has as much data as defined in
> #address-cells and #size-cells then you have to parse those values to
> interpret the binding correctly. When the binding description says the
> value is always 32bit or always 64bit or whatever you must always read
> a value with that many bits.
>
> Prime example of this are MTD partitions defined in dt.
>
> If your flash is small you can get away with 32bit partitions and you
> would define #address-cells and #size-cells as 1 even on 64bit system.
> When your flash is large you need 64bits to store the partition size
> and offset and you will define #address-cells and #size-cells as 2
> even on 32bit system because it won't work otherwise.

I agree entirely.

FWIW, my part of this discussion was mainly targeted at the "reg" 
property, where #address-cells/#size-cells are defined as applying as 
part of the base DT definition. Indeed other bindings can be define this 
way to (or not) at the whim of the binding author. The bug with Intel 
flash was triggered by using the same piece of code to parse a property 
where #address-cells/#size-cells don't apply.