Re: [PATCH v4 07/10] drivers: pinctrl: msm: setup GPIO irqchip hierarchy

Stephen Boyd
2019-03-15T16:28:31Z

Quoting Lina Iyer (2019-03-13 14:18:41)
> ---
> Changes in v4:
> - Remove irq_set_wake() on summary IRQ interrupt
> Changes in v3:
> - Use of_irq_domain_map() and pass PDC pin to parent irqdomain
> Changes in v2:
> - Call parent mask when masking GPIO interrupt
> Changes in v1:
> - Fix bug when unmasking PDC interrupt
[...]
> +}
> +
> +/*
> + * TODO: Get rid of this and push it into gpiochip_to_irq()
> + */

Any chance this TODO can be resolved?

> @@ -994,6 +1092,22 @@ static int msm_gpio_init(struct msm_pinctrl *pctrl)
> pctrl->irq_chip.irq_request_resources = msm_gpio_irq_reqres;
> pctrl->irq_chip.irq_release_resources = msm_gpio_irq_relres;
>
> + chip->irq.chip = &pctrl->irq_chip;
> + chip->irq.domain_ops = &msm_gpio_domain_ops;
> + chip->irq.handler = handle_edge_irq;
> + chip->irq.default_type = IRQ_TYPE_EDGE_RISING;

This also changed from v3. It used to be IRQ_TYPE_NONE. Specifying this
here seems to cause gpiolib to print a WARN.

/*
* Specifying a default trigger is a terrible idea if DT or ACPI is
* used to configure the interrupts, as you may end up with
* conflicting triggers. Tell the user, and reset to NONE.
*/
if (WARN(np && type != IRQ_TYPE_NONE,
"%s: Ignoring %u default trigger\n", np->full_name, type))
type = IRQ_TYPE_NONE;

So I guess this change should be dropped. Or at the least, it should be
split out to it's own patch and the motivations can be discussed in the
commit text.