Hello,
On 01/04/2016 09:02 PM, Stephen Warren wrote:
On 12/29/2015 01:47 AM, Przemyslaw Marczak wrote:
Hello Stephen,
On 12/16/2015 07:53 PM, Stephen Warren wrote:
On 12/15/2015 09:32 AM, Przemyslaw Marczak wrote:
commit: dm: core: Enable optional use of fdt_translate_address()
enables device's bus/child address translation method, depending
on bus 'ranges' property and including child 'reg' property.
This change makes impossible to decode the 'reg' for node with
'#size-cells' equal to 0.
Such case is possible by the specification and is also used in U-Boot,
e.g. by I2C uclass or S5P GPIO - the last one is broken at present.
Can you please explain the problem you're seeing in more detail? Without
any context, my initial reaction is that this is simply a bug somewhere.
That bug should be fixed, rather than introducing new APIs to hide the
problem.
Some time ago I send a patch with such fix:
[1] https://patchwork.ozlabs.org/patch/537372/
Sorry, I didn't add you to the 'CC' list.
However. I checked this in linux, and the code is the same, the
size-cells == 0 is not supported also in Linux.
The discussion there does indicate that removing the check on
#size-cells would be incorrect.
Ok, this probably would be good if we assume that dts is always well
written, so this is not acceptable.
So to prevent breaking some consistency in parsing fdt between U-boot
and Linux, I sent the patch which adds dev_get_reg(). And it seem to be
useful at least for I2C and Exynos GPIO driver.
OK; as I mentioned in my other reply, some form of new function or new
parameter does seem reasonable here.
...
At this point I can say, that the device-tree files and some compatible
drivers are using wrong assumptions.
I think, that adding the new function is not needed, and also that we
don't need any new parameter to the function dev_get_reg(),
because the right way is to fix the fdt.
For the Exynos GPIO issue, we can use two cases:
- define proper ranges
- move #size-cells=0 to #size-cells=1 and extend the reg property by
it's size (actually not too much to do)
This will fix the Exynos boot issue.
Looking at arch/arm/dts/exynos4412-trats2.dts, I see the following:
i2c@138d0000 {
samsung,i2c-sda-delay = <100>;
samsung,i2c-slave-addr = <0x10>;
samsung,i2c-max-bus-freq = <100000>;
status = "okay";
max77686_pmic@09 {
compatible = "maxim,max77686";
interrupts = <7 0>;
reg = <0x09 0 0>;
Is that the node you're having problems with? If so, I believe this may
simply be due to invalid DT content. In exynos4.dtsi, that i2c node is
defined as:
i2c@138d0000 {
#address-cells = <1>;
#size-cells = <0>;
Thus, any reg property in a child of that node must only contain a
single cell (the sum of #address-cells and #size-cells in the parent).
Does fixing the DT so it's valid solve your issue at all?
Nice hit above! However we don't use DM API yet for the above example,
so probably this is why it is still working - currently, the driver uses
fdtdec_get_int(), for getting this value.
But for test, after switching it to use of sequence: fdt_getprop() ->
fdt_translate_address(), then I can see the warning:
---- cut ----
_of_translate_address: Bad cell count for max77686_pmic@09
---- cut ----
And for the above issue - applying patch [1] - allows return the right
device address: 0x9 - without FDT modifying.
Now, I checked, why the above example compiles by dtc with no warning.
It looks, that dtc ignores some child's reg cells-count combination:
dtc doesn't check that the length of the reg property is *equal* to the
sum of #address-cells and #size-cells, but rather that the length is a
*multiple* of that value. This is because the reg property can contain
multiple addresses.
---- case 1 -----
parent {
#address-cells = <1>;
#size-cells = <0>;
child {
reg = <0x9>;
};
};
This is ok!
This is "1 * (1 + 0)".
---- case 2 -----
parent {
#address-cells = <1>;
#size-cells = <0>;
child {
reg = <0x9 0 0>;
};
};
This is ok: (the 2nd and 3rd child's cells are ignored by dtc)
The extra cells aren't ignored; the length is "3 * (1 + 0)".
---- case 3 -----
parent {
#address-cells = <1>;
#size-cells = <1>;
child {
reg = <0x9 0 0>;
};
};
This is wrong! dtc warning:
Warning (reg_format): "reg" property in /i2c@138d0000/max77686_pmic has
invalid length (12 bytes) (#address-cells == 1, #size-cells == 1)
Yes, this is "1.5 * (1 + 1)", yet the "1.5" isn't an integer, hence the
warning is triggered.
Also thank you for the explanation.
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
