Hi Stephen,
On 4 January 2016 at 13:02, Stephen Warren <swar...@wwwdotorg.org> 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, but since we have a breakage and a release in a week, I'm planning
on picking up Przemyslaw's patch. We can revert it immediately
afterwards.
Who is going to work out a proper solution (post-release)?
>
>> 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.
>
> ...
>
>>> Looking at arch/arm/dts/exynos4412-trats2.dts, I see the following:
>>>
>>>
>>> i2c@138d0000 {
>>> samsung,i2c-sda-delay = <100>;
>>> samsung,i2c-slave-addr = <0x10>;
>>> samsung,i2c-max-bus-freq = <100000>;
>>> status = "okay";
>>>
>>> max77686_pmic@09 {
>>> compatible = "maxim,max77686";
>>> interrupts = <7 0>;
>>> reg = <0x09 0 0>;
>>>
>>> Is that the node you're having problems with? If so, I believe this may
>>> simply be due to invalid DT content. In exynos4.dtsi, that i2c node is
>>> defined as:
>>>
>>> i2c@138d0000 {
>>> #address-cells = <1>;
>>> #size-cells = <0>;
>>>
>>> Thus, any reg property in a child of that node must only contain a
>>> single cell (the sum of #address-cells and #size-cells in the parent).
>>> Does fixing the DT so it's valid solve your issue at all?
>>
>>
>> Nice hit above! However we don't use DM API yet for the above example,
>> so probably this is why it is still working - currently, the driver uses
>> fdtdec_get_int(), for getting this value.
>>
>> But for test, after switching it to use of sequence: fdt_getprop() ->
>> fdt_translate_address(), then I can see the warning:
>>
>> ---- cut ----
>> _of_translate_address: Bad cell count for max77686_pmic@09
>> ---- cut ----
>>
>> And for the above issue - applying patch [1] - allows return the right
>> device address: 0x9 - without FDT modifying.
>>
>> Now, I checked, why the above example compiles by dtc with no warning.
>> It looks, that dtc ignores some child's reg cells-count combination:
>
>
> dtc doesn't check that the length of the reg property is *equal* to the sum
> of #address-cells and #size-cells, but rather that the length is a *multiple*
> of that value. This is because the reg property can contain multiple
> addresses.
>
>> ---- case 1 -----
>> parent {
>> #address-cells = <1>;
>> #size-cells = <0>;
>> child {
>> reg = <0x9>;
>> };
>> };
>> This is ok!
>
>
> This is "1 * (1 + 0)".
>
>> ---- case 2 -----
>> parent {
>> #address-cells = <1>;
>> #size-cells = <0>;
>> child {
>> reg = <0x9 0 0>;
>> };
>> };
>> This is ok: (the 2nd and 3rd child's cells are ignored by dtc)
>
>
> The extra cells aren't ignored; the length is "3 * (1 + 0)".
>>
>> ---- case 3 -----
>> parent {
>> #address-cells = <1>;
>> #size-cells = <1>;
>> child {
>> reg = <0x9 0 0>;
>> };
>> };
>>
>> This is wrong! dtc warning:
>> Warning (reg_format): "reg" property in /i2c@138d0000/max77686_pmic has
>> invalid length (12 bytes) (#address-cells == 1, #size-cells == 1)
>
>
> Yes, this is "1.5 * (1 + 1)", yet the "1.5" isn't an integer, hence the
> warning is triggered.
Great that you can explain this, thanks.
Regards,
Simon
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
