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

[Stephen Boyd]

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.