Re: [PATCH 08/12] drivers: pinctrl: msm: setup GPIO chip in hierarchy

[Lina Iyer]

On Fri, Nov 15 2019 at 15:09 -0700, Stephen Boyd wrote:
> Quoting Lina Iyer (2019-11-15 13:57:37)
> > On Fri, Nov 15 2019 at 13:55 -0700, Lina Iyer wrote:
> > >>Quoting Lina Iyer (2019-11-14 10:35:17)
> >
> > >>>+static int msm_gpio_wakeirq(struct gpio_chip *gc,
> > >>>+                           unsigned int child,
> > >>>+                           unsigned int child_type,
> > >>>+                           unsigned int *parent,
> > >>>+                           unsigned int *parent_type)
> > >>>+{
> > >>>+       struct msm_pinctrl *pctrl = gpiochip_get_data(gc);
> > >>>+       const struct msm_gpio_wakeirq_map *map;
> > >>>+       int i;
> > >>>+
> > >>>+       *parent = GPIO_NO_WAKE_IRQ;
> > >>>+       *parent_type = IRQ_TYPE_EDGE_RISING;
> > >>>+
> > >>>+       for (i = 0; i < pctrl->soc->nwakeirq_map; i++) {
> > >>>+               map = &pctrl->soc->wakeirq_map[i];
> > >>>+               if (map->gpio == child) {
> > >>>+                       *parent = map->wakeirq;
> > >>>+                       break;
> > >>>+               }
> > >>>+       }
> > >>>+
> > >>>+       return 0;
> > >>
> > >>Shouldn't we return -EINVAL if we can't translate the gpio irq to the
> > >>parent domain? I would expect to see return -EINVAL here and the above
> > >>if condition to return 0 instead of break.
> > >>
> > >Good catch. Sure.
> > >>>+}
> > >>>+
> > Looking into this, we have been setting GPIO in hierarchy with PDC and
> > the return 0 is appropriate as it would set the GPIO_NO_WAKE_IRQ as the
> > parent and setup the IRQ correctly. We fail to setup GPIOs otherwise.
>
> Ah ok so by default we will set the parent irq to ~0 and that means
> bypass pdc and go directly to GIC?
>
> Where do we fail to setup a GPIO otherwise? It sounds like we can always
> translate to either something in the map or to ~0.
We do not, may be other architectures can use this check better.

--lina