Hello Stephen,

On 01/11/2016 05:47 PM, Stephen Warren wrote:
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.




As we found out, the 'reg' property can represent not only MMIO, but may have other meaning, so the above case is possible. The 'reg' for the parent bus can represent MMIO (depends on what its parent defines) and the child is non-MMIO.

You won't allow to use dev_get_addr() for other than MMIO addresses.
Ok, I have no more arguments and no more time.

My issue can be also fixed by removing dev_get_addr() call from Exynos GPIO driver - so I will do this and within this change, will also revert the commit:
"fdt: fix address cell count checking in fdt_translate_address()"

Best regards,
--
Przemyslaw Marczak
Samsung R&D Institute Poland
Samsung Electronics
p.marc...@samsung.com
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Reply via email to