Re: [PATCH/RFC] drivers/irqchip: add irq-inverter

From: Geert Uytterhoeven <geert@linux-m68k.org>
To: Nikita Yushchenko <nikita.yoush@cogentembedded.com>
Cc: Thomas Gleixner <tglx@linutronix.de>,
	Marc Zyngier <maz@kernel.org>,
	Magnus Damm <magnus.damm@gmail.com>,
	Linux-Renesas <linux-renesas-soc@vger.kernel.org>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	Eugeniu Rosca <erosca@de.adit-jv.com>
Subject: Re: [PATCH/RFC] drivers/irqchip: add irq-inverter
Date: Tue, 28 Dec 2021 20:04:19 +0100	[thread overview]
Message-ID: <CAMuHMdWbqi_ac2wVUooO_mukG++y=ZjOwSir87fu+NLXYEKUqg@mail.gmail.com> (raw)
In-Reply-To: <20211228165642.2514766-1-nikita.yoush@cogentembedded.com>

Hi Nikita,

On Tue, Dec 28, 2021 at 6:23 PM Nikita Yushchenko
<nikita.yoush@cogentembedded.com> wrote:
> Interrupt trigger type is typically used to configure interrupt
> controller to properly interpret interrupt signal sent from a device.
>
> However, some devices have configureable interrupt outputs, and drivers
> tend to use interrupt trigger type also to configure device interrupt
> output.
>
> This works well when device interrupt output is connected directly to
> interrupt controller input. However, this is not always the case.
> Sometimes the interrupt signal gets inverted between the device
> producing it and the controller consuming it. Combined with both sides
> using the same interrupt trigger type to configure the signal, this
> results into non-working setup regardless of what interrupt trigger type
> is configured.
>
> Irq-inverer is a solution for this case. It is a virtual irqchip that
> provides additional virq number that behaves exactly as existing one,
> but with inverted trigger type reported via irq_get_trigger_type() API.
>
> Usage example, for Kingfisher extension board for Renesas Gen-3 Soc,
> that has WiFi interrupt delivered over inverting level-shifter:
>
> / {
>         wlcore_interrupt: inverter {
>                 compatible = "linux,irq-inverter";
>                 interrupts-extended = <&gpio1 25 IRQ_TYPE_EDGE_RISING>;
>                 interrupt-controller;
>                 #interrupt-cells = <0>;
>         };
> };
>
> &wlcore {
>         interrupts-extended = <&wlcore_interrupt>;
> };
>
> Then, wl18xx driver gets IRQ_TYPE_EDGE_FALLING return from
> irq_get_trigger_type() call, and configures interrupt output for that.
> Then the signal is delivered inverted to the GPIO module, and handled
> correctly, because GPIO is configured for IRQ_TYPE_EDGE_RISING.
>
> Implementation notes:
>
> - why platform_driver and not IRQCHIP_DECLARE()?
> - because IRQCHIP_DECLARE() does not process EPROBE_DEFER properly
>
> - why not using hierarchial irq_domain?
> - because with hierarchial irq_domain, same interrupt gets the same virq
>   number at all levels, and trigger type is tied to virq number, so need
>   different virq numbers for reporting different trigger types
>
> - why using request_irq() for parent irq, instead of setting up chained
>   interrupt in irqchips?
> - because this way code is much simpler, and shall work for all cases
>   (such as normal/threaded parent irq, normal/threaded child irq,
>   different parent interrupt chips, etc)
>
> - why just not introducing separate API for consumer-side and
>   produced-side trigger type?
> - because with the chosen approach, no changes are needed to any cases
>   that don't suffer from inverted interrupt routing
>
> Signed-off-by: Nikita Yushchenko <nikita.yoush@cogentembedded.com>

Thanks for your patch!

FTR, here's a link to the previous discussion about this topic:
https://lore.kernel.org/all/20190607172958.20745-1-erosca@de.adit-jv.com/

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds