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

[Mario Six <mario.six@gdsys.cc>]

(Adding Simon, because of DM)

Hi Martin,

On Wed, Feb 28, 2018 at 7:27 PM, Martin Fuzzey <mfuzzey@parkeon.com> wrote:
> Hi,
>
> since this commit:
>
> commit f62ca2cd2ab8171f603960da9203ceb6ee8a1efd
> Author: Mario Six <mario.six@gdsys.cc>
> Date:   Mon Jan 15 11:07:45 2018 +0100
>
>     gpio: pca953x_gpio: Make live-tree compatible
>
>
> I am getting the error message "__of_translate_address: Bad cell count for
> pcal6524 at 1"
>
> I don't think the patch
>
> -       addr = fdtdec_get_int(gd->fdt_blob, dev_of_offset(dev), "reg", 0);
> +       addr = dev_read_addr(dev);
>
> is correct since it replaces a simple lookup of the value "reg" property
> with an attempted translation to a CPU address (which fails for I2C)
>

I see the issue on one of our boards as well, but the weird thing is that the
translation should not happen: the I2C bus has no ranges property, which
should, according to the DeviceTree specification, cause the translation
routine to not attempt a translation (because a missing ranges property implies
that the device's address space is not translatable). I'll take a look at the
code; maybe it's a Linux-specific exception that was imported with the code
that I was not aware of.

The dev_read_addr() is the go-to function for drivers to get device addresses
(and have all issues like translation and the like taken care of
automatically), so it would be nice to keep it in my opinion. Hence, fixing
the underlying issue is probably preferable.

> This does not cause the driver probe to fail since dev_read_addr() returns
> FDT_ADDR_T_NONE (== -1)  and is followed by
>
>     if (addr == 0)
>         return -ENODEV;
>
>     info->addr = addr;
>
> Although addr is stored in the driver data the only thing it is used for is
> to build the bank name
>
>     snprintf(name, sizeof(name), "gpio@%x_", info->addr);
>
> I'm not even sure that using the value of the "reg" property as before is
> really a good idea - what happens if we have multiple chips with same
> address on seperate busses?
>

We have that exact case on some of our boards. It works, but it's pretty
annoying if you want to use the gpio command: When setting values of GPIOs, it
only ever affects the first device the command comes across.

I have a potential solution for that, see below.

> Maybe it would be better to use the DT node name? But would that break
> existing configurations by renaming the device?

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?

>
>
> Regards,
>
> Martin
>

Best regards,

Mario