Re: [RFC/RFT PATCH] gpiolib: reverse-assign the fwnode to struct gpio_chip

[Linus Walleij]

On Fri, Oct 6, 2023 at 1:51 PM Bartosz Golaszewski <brgl@xxxxxxxx> wrote:

> struct gpio_chip is not only used to carry the information needed to
> set-up a GPIO device but is also used in all GPIOLIB callbacks and is
> passed to the matching functions of lookup helpers.
>
> In that last case, it is currently impossible to match a GPIO device by
> fwnode unless it was explicitly assigned to the chip in the provider
> code. If the fwnode is taken from the parent device, the pointer in
> struct gpio_chip will remain NULL.
>
> If we have a parent device but gc->fwnode was not assigned by the
> provider, let's assign it ourselves so that lookup by fwnode can work in
> all cases.
>
> Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@xxxxxxxxxx>

Acked-by: Linus Walleij <linus.walleij@xxxxxxxxxx>

because we want the code to work (rough consensus and running code)

> -       if (gc->fwnode)
> +       if (gc->fwnode) {
>                 device_set_node(&gdev->dev, gc->fwnode);
> -       else if (gc->parent)
> -               device_set_node(&gdev->dev, dev_fwnode(gc->parent));
> +       } else if (gc->parent) {
> +               parent_fwnode = dev_fwnode(gc->parent);
> +               device_set_node(&gdev->dev, parent_fwnode);
> +               gc->fwnode = parent_fwnode;

The core of the crux is that we have
information duplication with a reference to the fwnode in two
places. One in gdev->dev and one in gc->fwnode.

gc->of_node was the same duplicated before.

A gdev is created for each gpio_chip so in my naive brain we could
get rid of gc->fwnode and only have the one inside gdev->dev?
+/- some helpful getters/setters if need be.

Or what am I thinking wrong here?

Yours,
Linus Walleij