Hello,
On 01/05/2016 06:26 PM, Stephen Warren wrote:
On 01/05/2016 08:47 AM, Przemyslaw Marczak wrote:
Hello,
On 01/05/2016 02:00 AM, 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.
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.
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].
It /might/ indeed be reasonable to skip the check on #size-cells if
there is no ranges property; a missing ranges property is already
treated as a 1:1 translation as described by the comment at the top of
of_translate_one(). Still, there are likely some edge-cases w.r.t. the
"length" fields in the ranges property that would need to be thought
through in more detail before I'd say for certain that this change does
absolutely make sense.
Either way though, making that change wouldn't solve the problem at all.
It's simply not possible to translate an I2C device address beyond the
root of the I2C address space; there is no equivalent of an I2C device's
address in MMIO address space. The root of the problem is that the code
is attempting to perform an I2C->MMIO address translation. If we prevent
that (by capping any such translation at the root of the I2C address
space for example), the problem is solved at that point. Once that's
solved, the issue of translating across a boundary with #size-cells=0
will never appear, and hence does not need to be solved.
The problem with I2C and I2C->MMIO address translation is solved by my
new patch. The specification is clear for such case, if reg can't be
mapped to it's parent address space, then don't use 'ranges' and this
can be easy solved by additional check.
Please review my new patch:
https://patchwork.ozlabs.org/patch/564246/
Best regards,
--
Przemyslaw Marczak
Samsung R&D Institute Poland
Samsung Electronics
p.marc...@samsung.com
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
