[U-Boot] [PATCH] fdt: Allow fdt_translate_address() to work with buses

[Simon Glass]

Hi Stephen,

On 4 January 2016 at 13:15, Stephen Warren <swarren at wwwdotorg.org> 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