From mboxrd@z Thu Jan 1 00:00:00 1970 From: Przemyslaw Marczak Date: Tue, 29 Dec 2015 09:47:23 +0100 Subject: [U-Boot] [PATCH 0/3] dm: add dev_get_reg() for getting device node's reg In-Reply-To: <5671B33E.5070701@wwwdotorg.org> References: <1450197123-1822-1-git-send-email-p.marczak@samsung.com> <5671B33E.5070701@wwwdotorg.org> Message-ID: <5682489B.2050408@samsung.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de 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 at 138d0000 { > samsung,i2c-sda-delay = <100>; > samsung,i2c-slave-addr = <0x10>; > samsung,i2c-max-bus-freq = <100000>; > status = "okay"; > > max77686_pmic at 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 at 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 at 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 at 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 at 138d0000 i2c at 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 at 09 { }; }; ------------ check pmic node -------------------- Trats2 # fdt list /i2c at 138d0000/max77686_pmic at 09 max77686_pmic at 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.marczak at samsung.com