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
