From mboxrd@z Thu Jan 1 00:00:00 1970 From: Stephen Warren Date: Mon, 11 Jan 2016 09:47:27 -0700 Subject: [U-Boot] [PATCH] fdt: __of_translate_address(): check parent's 'ranges' before translate In-Reply-To: <56939045.5070806@samsung.com> References: <1452166849-24461-1-git-send-email-p.marczak@samsung.com> <568EAD8B.6090909@wwwdotorg.org> <56939045.5070806@samsung.com> Message-ID: <5693DC9F.2090100@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/11/2016 04:21 AM, Przemyslaw Marczak wrote: > Hello Stephen, > > On 01/07/2016 07:25 PM, Stephen Warren wrote: >> On 01/07/2016 04:40 AM, Przemyslaw Marczak wrote: >>> The present implementation of __of_translate_address() taken >>> from the Linux, is designed for translate bus/child address >>> mappings by using 'ranges' property - and it doesn't allow >>> for checking an address for a device's node with zero size-cells. >>> >>> The 'size-cells > 0' is required for bus/child address mapping, >>> but is not required for non-memory mapped address, e.g.: I2C chip. >>> Then when we need only raw 'reg' property's value. >>> >>> Since the I2C device address goes to a single-cell reg property, >>> support for that case is welcome, but currently calling dev_get_addr() >>> for I2C device will return 'FDT_ADDR_T_NONE', and print the warning: >>> >>> warning: >>> __of_translate_address: Bad cell count for 'some-dev' >> >> This patch takes the wrong approach. >> >> It simply doesn't make sense to /attempt/ to translate an I2C address >> into an MMIO address space. It's a nonsensical operation; no such >> translation is possible under any circumstances because I2C and MMIO >> addresses mean completely different things and simply can't be >> translated to each-other. >> >> Rather than making this nonsensical operation succeed in a way that >> gives the desired no-op result, the nonsensical operation simply >> shouldn't be performed in the first place. >> >> > > Okay, the example with I2C may be little confusing - I could use some > general naming convention. However, this patch updates FDT-related code > only. > > In one of your previous e-mails, you well argued that we shouldn't use > dev_get_reg() for some buses, since they have a different 'reg' meaning. > > You are right, using dev_get_addr() as universal function may be > nonsensical. > > Please note, that the present implementation of function: > '__of_translate_address()' - allows for 1:1 translation, but only if > '#size-cells' exists. So the below case is possible: > > ---------------------- > parent { > address-cells = <1>; > size-cells = <1>; > reg = <0x10000000 0x1000>; > > child { > reg = <0xa00 0x100>; > }; > }; > > dev_get_reg(child) - will return '0xa00' > ---------------------- > > If we don't need the address length, we can define: > ---------------------- > parent { > address-cells = <1>; > size-cells = <0>; > reg = <0x10000000 0x1000>; > > child { > reg = <0xa00>; > }; > }; This case won't ever appear in a correctly written DT where reg represents an MMIO address; MMIO addresses always have sizes, and hence can't have size-cells=0. Hence, translating through a DT structures like that is an error case, and shouldn't work.