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.
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.
For this purpose this patch set introduces new core function:
fdt_addr_t dev_get_reg(struct udevice *dev)
which returns the 'reg' value in the same way as previously
dev_get_addr().
This fixes s5p gpio driver and booting issue on few Exynos based boards:
- Trats2
- Odroid U3/X2
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:
---- case 1 -----
parent {
#address-cells = <1>;
#size-cells = <0>;
child {
reg = <0x9>;
};
};
This is ok!
---- 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)
---- 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)
Now I don't have a time for checking it in dtc code, however we can
check in U-Boot, that the dtb is not coherent?
So I can compile my default dts - no dtc warnings, and next in U-Boot I
can see:
------------ check i2c node --------------------
Trats2 # fdt list /i2c@138d0000
i2c@138d0000 {
#address-cells = <0x00000001>;
#size-cells = <0x00000000>;
compatible = "samsung,s3c2440-i2c";
reg = <0x138d0000 0x00000100>;
interrupts = <0x00000007 0x0000003f 0x00000000>;
samsung,i2c-sda-delay = <0x00000064>;
samsung,i2c-slave-addr = <0x00000010>;
samsung,i2c-max-bus-freq = <0x000186a0>;
status = "okay";
max77686_pmic@09 {
};
};
------------ check pmic node --------------------
Trats2 # fdt list /i2c@138d0000/max77686_pmic@09
max77686_pmic@09 {
compatible = "maxim,max77686";
interrupts = <0x00000007 0x00000000>;
reg = <0x00000009 0x00000000 0x00000000>;
#clock-cells = <0x00000001>;
voltage-regulators {
};
};
So, the parent defines the summarized cells count as 1, and the child
node can exceed it, because it provides 3 cells.
However it looks that we are still safe, since the code doesn't try
exceed the cells count, defined by the parent.
What do you think about this?
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
