On 01/04/2016 06:00 PM, Simon Glass wrote:
Hi Stephen,
On 4 January 2016 at 13:15, Stephen Warren <swar...@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@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.
It'd be better since it'd isolate the disabling of the feature only to
those platforms that need it to work around bugs.
Still, if you're absolutely certain that this change will be reverted as
soon as the release is made, that is probably OK.
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?
I appear to have answered this already in response to one of
Przemyslaw's emails.
> 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.
dev_get_addr() is simple yes, but also incorrect (to use in certain
contexts, as currently implemented).
DT has special cases. Code that parses DT has to deal with them. To
paraphrase some famous quote: Things should be as simple as possible,
but no simpler.
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
