From mboxrd@z Thu Jan 1 00:00:00 1970 From: Stephen Warren Date: Tue, 5 Jan 2016 10:08:50 -0700 Subject: [U-Boot] [PATCH 0/3] dm: add dev_get_reg() for getting device node's reg In-Reply-To: <568BE344.9010003@samsung.com> References: <1450197123-1822-1-git-send-email-p.marczak@samsung.com> <5671B33E.5070701@wwwdotorg.org> <5682489B.2050408@samsung.com> <568ACFDE.5080306@wwwdotorg.org> <568BE344.9010003@samsung.com> Message-ID: <568BF8A2.5000208@wwwdotorg.org> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de On 01/05/2016 08:37 AM, Przemyslaw Marczak wrote: > 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. Why not? Certainly it's reasonable to expect code not to blow up in crashy ways in the face of bad DTs. However, I don't think it's reasonable to require that code perform exactly the desired function in the face of arbitrary bad input data. Besides, the problem here isn't a bad DT. It's perfectly legal for a DT to contain #size-cells=<0>. The problem is bad code, which attempts to translate an address beyond the root of the address space that the address is within. This is a bug in the code, pure and simple. >>> 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. That may fix/hide the issue, but semantically seems entirely incorrect. I2C addresses cannot logically be mapped into the CPU's MMIO address space, so adding a ranges property that attempts to do that doesn't make sense. I2C addresses don't have a size, so adding one to the DT doesn't make sense.