Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <CAFr9PX=dz1kt_BphpWr250asLiHFQNeBgNPhuPje==hzkcHWhg@mail.gmail.com>
 <20210311031242.24232-1-mark-pk.tsai@mediatek.com>
In-Reply-To: <20210311031242.24232-1-mark-pk.tsai@mediatek.com>
From:   Daniel Palmer <daniel@0x0f.com>
Date:   Thu, 11 Mar 2021 21:33:35 +0900
Message-ID: <CAFr9PXmHhB=PT1kRoYYowf2dw2fqa1-LGCD5S52hW9jJtq5Q4w@mail.gmail.com>
Subject: Re: [PATCH v2] irqchip/irq-mst: Support polarity configuration
To:     Mark-PK Tsai <mark-pk.tsai@mediatek.com>
Cc:     Daniel Palmer <daniel@thingy.jp>,
        linux-arm-kernel <linux-arm-kernel@lists.infradead.org>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
        linux-mediatek@lists.infradead.org,
        Matthias Brugger <matthias.bgg@gmail.com>,
        Marc Zyngier <maz@kernel.org>,
        Thomas Gleixner <tglx@linutronix.de>,
        =?UTF-8?B?WUogQ2hpYW5nICjmsZ/oi7HmnbAp?= <yj.chiang@mediatek.com>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

Hi Mark-PK.

On Thu, 11 Mar 2021 at 12:12, Mark-PK Tsai <mark-pk.tsai@mediatek.com> wrote:
> For a fiq controller, the input edge signal will be convert to level and
> keep the interrupt status until we do EOI operation.
> That means if a rising edge input if trigger the ouput line will keep high
> until we clear the interrupt status.

I think maybe the fiq is always edge triggered?
It seems like it latches on an edge and holds it's output to the GIC
high until it is reset by eoi and then only triggers again on another
edge.
I can experiment to confirm that's what it actually does for the chips I have.

Then it seems like the irq version is almost just a configurable
inverter that passes either the input signal or the inverted input
signal to the GIC.

So maybe fiq should only accept edge type interrupts and irq could
accept either?

>  static int mst_irq_chip_set_type(struct irq_data *data, unsigned int type)
>  {
> -       if (type != IRQ_TYPE_LEVEL_LOW && type != IRQ_TYPE_LEVEL_HIGH)
> -               return -EINVAL;
> -
> -       if (type == IRQ_TYPE_LEVEL_LOW) {
> +       if (type == IRQ_TYPE_EDGE_FALLING) {
> +               mst_set_irq(data, INTC_REV_POLARITY);
> +               type = IRQ_TYPE_EDGE_RISING;
> +       } else if (type == IRQ_TYPE_LEVEL_LOW) {
>                 mst_set_irq(data, INTC_REV_POLARITY);
>                 type = IRQ_TYPE_LEVEL_HIGH;
>         }

I think this still needs the logic to check that type is something we
can handle (not IRQ_TYPE_EDGE_BOTH) and maybe if the fiq controller
can only do edge interrupts level types should return -EINVAL?

Thanks,

Daniel