Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Content-Type: text/plain; charset="utf-8"
MIME-Version: 1.0
Content-Transfer-Encoding: quoted-printable
In-Reply-To: <20190313211844.29416-8-ilina@codeaurora.org>
References: <20190313211844.29416-1-ilina@codeaurora.org> <20190313211844.29416-8-ilina@codeaurora.org>
From:   Stephen Boyd <swboyd@chromium.org>
Subject: Re: [PATCH v4 07/10] drivers: pinctrl: msm: setup GPIO irqchip hierarchy
Cc:     linux-kernel@vger.kernel.org, rplsssn@codeaurora.org,
        linux-arm-msm@vger.kernel.org, thierry.reding@gmail.com,
        bjorn.andersson@linaro.org, dianders@chromium.org,
        linus.walleij@linaro.org, Lina Iyer <ilina@codeaurora.org>
To:     Lina Iyer <ilina@codeaurora.org>, evgreen@chromium.org,
        marc.zyngier@arm.com
Message-ID: <155266731117.20095.4543997300651173812@swboyd.mtv.corp.google.com>
User-Agent: alot/0.8
Date:   Fri, 15 Mar 2019 09:28:31 -0700
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

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 =3D msm_gpio_irq_reqres;
>         pctrl->irq_chip.irq_release_resources =3D msm_gpio_irq_relres;
> =20
> +       chip->irq.chip =3D &pctrl->irq_chip;
> +       chip->irq.domain_ops =3D &msm_gpio_domain_ops;
> +       chip->irq.handler =3D handle_edge_irq;
> +       chip->irq.default_type =3D 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 !=3D IRQ_TYPE_NONE,
                 "%s: Ignoring %u default trigger\n", np->full_name, type))
                type =3D 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.