[U-Boot] [PATCH] fdt: __of_translate_address(): check parent's 'ranges' before translate

[Stephen Warren]

On 01/11/2016 04:21 AM, Przemyslaw Marczak wrote:
> Hello Stephen,
>
> On 01/07/2016 07:25 PM, Stephen Warren wrote:
>> On 01/07/2016 04:40 AM, Przemyslaw Marczak wrote:
>>> The present implementation of __of_translate_address() taken
>>> from the Linux, is designed for translate bus/child address
>>> mappings by using 'ranges' property - and it doesn't allow
>>> for checking an address for a device's node with zero size-cells.
>>>
>>> The 'size-cells > 0' is required for bus/child address mapping,
>>> but is not required for non-memory mapped address, e.g.: I2C chip.
>>> Then when we need only raw 'reg' property's value.
>>>
>>> Since the I2C device address goes to a single-cell reg property,
>>> support for that case is welcome, but currently calling dev_get_addr()
>>> for I2C device will return 'FDT_ADDR_T_NONE', and print the warning:
>>>
>>> warning:
>>> __of_translate_address: Bad cell count for 'some-dev'
>>
>> This patch takes the wrong approach.
>>
>> It simply doesn't make sense to /attempt/ to translate an I2C address
>> into an MMIO address space. It's a nonsensical operation; no such
>> translation is possible under any circumstances because I2C and MMIO
>> addresses mean completely different things and simply can't be
>> translated to each-other.
>>
>> Rather than making this nonsensical operation succeed in a way that
>> gives the desired no-op result, the nonsensical operation simply
>> shouldn't be performed in the first place.
>>
>>
>
> Okay, the example with I2C may be little confusing - I could use some
> general naming convention. However, this patch updates FDT-related code
> only.
>
> In one of your previous e-mails, you well argued that we shouldn't use
> dev_get_reg() for some buses, since they have a different 'reg' meaning.
>
> You are right, using dev_get_addr() as universal function may be
> nonsensical.
>
> Please note, that the present implementation of function:
> '__of_translate_address()' - allows for 1:1 translation, but only if
> '#size-cells' exists. So the below case is possible:
>
> ----------------------
> parent {
>      address-cells = <1>;
>      size-cells = <1>;
>      reg = <0x10000000 0x1000>;
>
>      child {
>          reg = <0xa00 0x100>;
>      };
> };
>
> dev_get_reg(child) - will return '0xa00'
> ----------------------
>
> If we don't need the address length, we can define:
> ----------------------
> parent {
>      address-cells = <1>;
>      size-cells = <0>;
>      reg = <0x10000000 0x1000>;
>
>      child {
>          reg = <0xa00>;
>      };
> };

This case won't ever appear in a correctly written DT where reg 
represents an MMIO address; MMIO addresses always have sizes, and hence 
can't have size-cells=0. Hence, translating through a DT structures like 
that is an error case, and shouldn't work.