On 01/05/2016 08:37 AM, Przemyslaw Marczak wrote:
Hello,

On 01/04/2016 09:02 PM, Stephen Warren wrote:
On 12/29/2015 01:47 AM, Przemyslaw Marczak wrote:
Hello Stephen,

On 12/16/2015 07:53 PM, Stephen Warren wrote:
On 12/15/2015 09:32 AM, Przemyslaw Marczak wrote:
commit: dm: core: Enable optional use of fdt_translate_address()

enables device's bus/child address translation method, depending
on bus 'ranges' property and including child 'reg' property.
This change makes impossible to decode the 'reg' for node with
'#size-cells' equal to 0.

Such case is possible by the specification and is also used in U-Boot,
e.g. by I2C uclass or S5P GPIO - the last one is broken at present.

Can you please explain the problem you're seeing in more detail?
Without
any context, my initial reaction is that this is simply a bug
somewhere.
That bug should be fixed, rather than introducing new APIs to hide the
problem.


Some time ago I send a patch with such fix:

[1] https://patchwork.ozlabs.org/patch/537372/

Sorry, I didn't add you to the 'CC' list.

However. I checked this in linux, and the code is the same, the
size-cells == 0 is not supported also in Linux.

The discussion there does indicate that removing the check on
#size-cells would be incorrect.

Ok, this probably would be good if we assume that dts is always well
written, so this is not acceptable.

Why not?

Certainly it's reasonable to expect code not to blow up in crashy ways in the face of bad DTs. However, I don't think it's reasonable to require that code perform exactly the desired function in the face of arbitrary bad input data.

Besides, the problem here isn't a bad DT. It's perfectly legal for a DT to contain #size-cells=<0>. The problem is bad code, which attempts to translate an address beyond the root of the address space that the address is within. This is a bug in the code, pure and simple.

So to prevent breaking some consistency in parsing fdt between U-boot
and Linux, I sent the patch which adds dev_get_reg(). And it seem to be
useful at least for I2C and Exynos GPIO driver.

OK; as I mentioned in my other reply, some form of new function or new
parameter does seem reasonable here.

...

At this point I can say, that the device-tree files and some compatible
drivers are using wrong assumptions.

I think, that adding the new function is not needed, and also that we
don't need any new parameter to the function dev_get_reg(),
because the right way is to fix the fdt.

For the Exynos GPIO issue, we can use two cases:
- define proper ranges
- move  #size-cells=0 to #size-cells=1 and extend the reg property by
it's size (actually not too much to do)

This will fix the Exynos boot issue.

That may fix/hide the issue, but semantically seems entirely incorrect. I2C addresses cannot logically be mapped into the CPU's MMIO address space, so adding a ranges property that attempts to do that doesn't make sense. I2C addresses don't have a size, so adding one to the DT doesn't make sense.
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Reply via email to