Hi Stephen,
On 4 January 2016 at 13:02, Stephen Warren swarren@wwwdotorg.org 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, but since we have a breakage and a release in a week, I'm planning on picking up Przemyslaw's patch. We can revert it immediately afterwards.
Who is going to work out a proper solution (post-release)?
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.
...
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.
Great that you can explain this, thanks.
Regards, Simon