From mboxrd@z Thu Jan 1 00:00:00 1970 From: Przemyslaw Marczak Date: Thu, 07 Jan 2016 12:45:51 +0100 Subject: [U-Boot] [PATCH 0/3] dm: add dev_get_reg() for getting device node's reg In-Reply-To: <568BF8A2.5000208@wwwdotorg.org> 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> <568BF8A2.5000208@wwwdotorg.org> Message-ID: <568E4FEF.1080606@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, On 01/05/2016 06:08 PM, Stephen Warren wrote: > 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. > > Agree, then I think my new patch should be suitable for the both cases: https://patchwork.ozlabs.org/patch/564246/ Best regards, -- Przemyslaw Marczak Samsung R&D Institute Poland Samsung Electronics p.marczak at samsung.com