[U-Boot] Regression: gpio: pca953x_gpio: Make live-tree compatible

[Mario Six]

Hi Martin,

On Thu, Mar 1, 2018 at 11:07 AM, Martin Fuzzey <mfuzzey at parkeon.com> wrote:
> Hi Mario,
>
> thank you for your very quick reply.
>
>
> On 01/03/18 09:01, Mario Six wrote:
>>
>>
>> Looks like that's it (from drivers/core/of_addr.c):
>>
>>   * Note: We consider that crossing any level with #size-cells == 0 to
>> mean
>>   * that translation is impossible (that is we are not dealing with a
>> value
>>   * that can be mapped to a cpu physical address). This is not really
>> specified
>>   * that way, but this is traditionally the way IBM at least do things
>>
>> So, of_translate_address fails by design if #size-cells == 0.
>>
>> I propose the following patch, which would prevent translation in this
>> case in
>> ofnode_get_addr_index:
>>
>> ---- >8 ----
>> diff --git a/drivers/core/ofnode.c b/drivers/core/ofnode.c
>> index 98f4b539ea..b2f6ffe385 100644
>> --- a/drivers/core/ofnode.c
>> +++ b/drivers/core/ofnode.c
>> @@ -200,13 +200,16 @@ fdt_addr_t ofnode_get_addr_index(ofnode node, int
>> index)
>>                  uint flags;
>>                  u64 size;
>>                  int na;
>> +               int ns;
>>
>>                  prop_val = of_get_address(ofnode_to_np(node), index,
>> &size,
>>                                            &flags);
>>                  if (!prop_val)
>>                          return FDT_ADDR_T_NONE;
>>
>> -               if (IS_ENABLED(CONFIG_OF_TRANSLATE)) {
>> +               ns = of_n_size_cells(ofnode_to_np(node));
>> +
>> +               if (IS_ENABLED(CONFIG_OF_TRANSLATE) && ns > 0) {
>>                          return
>> of_translate_address(ofnode_to_np(node), prop_val);
>>                  } else {
>>                          na = of_n_addr_cells(ofnode_to_np(node));
>> ---- >8 ----
>>
>> In other words:
>> #size-cells == 0 -> No translation, return the plain reg value.
>> #size-cells != 0 -> Do proper translation.
>>
>> This fixes the issue on our board.
>
>
> That isn't enough for the non live-tree case.
>
> I've added this too:
>
> diff --git a/drivers/core/fdtaddr.c b/drivers/core/fdtaddr.c
> index 3847dd8..9a3b4c3 100644
> --- a/drivers/core/fdtaddr.c
> +++ b/drivers/core/fdtaddr.c
> @@ -49,12 +49,17 @@ fdt_addr_t devfdt_get_addr_index(struct udevice *dev,
> int index)
>
>                 reg += index * (na + ns);
>
> -               /*
> -                * Use the full-fledged translate function for complex
> -                * bus setups.
> -                */
> -               addr = fdt_translate_address((void *)gd->fdt_blob,
> -                                            dev_of_offset(dev), reg);
> +               if (ns) {
> +                       /*
> +                        * Use the full-fledged translate function for
> complex
> +                        * bus setups.
> +                        */
> +                       addr = fdt_translate_address((void *)gd->fdt_blob,
> + dev_of_offset(dev), reg);
> +               } else {
> +                       /* Non translatable if #size-cells == 0 */
> +                       addr = fdt_read_number(reg, na);
> +               }
>         } else {
>                 /*
>                  * Use the "simple" translate function for less complex
>

Ah, yes, indeed, thanks for catching that. The enhanced patch still works in
the live-tree case, so, can I go ahead and add your Signed-off-by to the patch
and send it?

Thanks for testing, and sorry for the inconvenience.

>> I have a patch I use on our boards that looks for a "label" string
>> property in
>>>
>>> the DT, which is then used for the bank name if it's there, and falls
>>> back to
>>> the "gpio@%x_" in the case it's not there. If you think it's useful, I
>>> could
>>> send the patch to the mailing list?
>
>
> Yes please.
>

OK, I'll send it in a few minutes.

>
> Regards,
>
> Martin
>

Best regards,

Mario