From mboxrd@z Thu Jan 1 00:00:00 1970 From: Przemyslaw Marczak Date: Tue, 05 Jan 2016 16:47:41 +0100 Subject: [U-Boot] [PATCH] fdt: Allow fdt_translate_address() to work with buses In-Reply-To: References: <1451862280-15245-1-git-send-email-sjg@chromium.org> <568AD2F9.2050903@wwwdotorg.org> Message-ID: <568BE59D.5040806@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 02:00 AM, Simon Glass wrote: > Hi Stephen, > > On 4 January 2016 at 13:15, Stephen Warren wrote: >> On 01/03/2016 04:04 PM, Simon Glass wrote: >>> >>> It is common for I2C and SPI buses to have a single-cell address and a >>> size >>> of 0. These produce a warning at present. For example on snow: >>> >>> __of_translate_address: Bad cell count for gpc4 >>> __of_translate_address: Bad cell count for gpx0 >>> __of_translate_address: Bad cell count for gpv2 >>> __of_translate_address: Bad cell count for gpv4 >>> >>> One of the nodes in question looks like this in part: >>> >>> pinctrl_2: pinctrl at 10d10000 { >>> #address-cells = <1>; >>> #size-cells = <0>; >>> gpv2: gpv2 { >>> reg = <0x060>; >>> }; >>> gpv4: gpv4 { >>> reg = <0xc0>; >>> }; >>> }; >>> >>> This is clearly valid so it looks like the conversion to use >>> fdt_translate_address() in dev_get_addr() is not currently a good move. >> >> >> To disable that, why not simply turn off CONFIG_OF_TRANSLATE on the affected >> platforms? That's precisely why that config option was introduced when the >> call to fdt_translate_address() was added to dev_get_addr()? >> >> That would prevent this patch from affecting platforms that don't trigger >> this issue, this leaving the valid check in place. > > But since this breaks normal behaviour we don't know what platforms > are affected. We have made CONFIG_OF_TRANSLATE the default. So this > approach doesn't seem (in effect) any better than Przemyslaw's newer > series, below. > >> >>> Przemyslaw Marczak sent three patches to resolve this for exynos boards: >>> >>> https://patchwork.ozlabs.org/patch/557008/ >>> https://patchwork.ozlabs.org/patch/557010/ >>> https://patchwork.ozlabs.org/patch/557009/ >>> >>> But this involves creating a new function, and everyone will need to know >>> when to use which one. Also the problem may affect other boards. >> >> >> I suggest adding an extra parameter to dev_get_addr() (or whatever calls it) >> that indicates the root of the address space. The check on #size-cells >> should be skipped for that one node (or level of translation) but enabled >> for all other levels. This way, there would be no need for anyone to choose >> between functions; there'd only be one. Most cases (i.e. translation of MMIO >> addresses) would simply pass 0 as the extra parameter (for the root node), >> but in special cases where it's known translation is not expected to reach >> the root MMIO space (e.g. I2C, SPI controllers), the controller node would >> be passed in. > > How would the caller know this root? It sounds plausible, but I do > want to avoid complex rules. I think you are saying that buses that > use their own address mechanism (i.e. not MMIO) must do something > special. The current dev_get_addr() is really simple. > > Regards, > Simon > > Simon, Stephen As a summary of the issue, please tell me your opinion about this: Since the __of_translate_address() is called always with the "ranges" as an argument, it looks reasonable to check it at the function beginning, that the "ranges" property exists in the parent node. If not exists - then don't check the size-cells count and this should fix the problem with additional argument. This is simple and correct from specification point of view - which says ranges and #size-cells property's - are not required [ePAPR v1.1]. Best regards, -- Przemyslaw Marczak Samsung R&D Institute Poland Samsung Electronics p.marczak at samsung.com