From: "Christophe Leroy (CS GROUP)" <chleroy@kernel.org>
To: Paul Louvel <paul.louvel@bootlin.com>,
Qiang Zhao <qiang.zhao@nxp.com>,
Thomas Gleixner <tglx@kernel.org>, Rob Herring <robh@kernel.org>,
Krzysztof Kozlowski <krzk+dt@kernel.org>,
Conor Dooley <conor+dt@kernel.org>,
Linus Walleij <linusw@kernel.org>,
Bartosz Golaszewski <brgl@kernel.org>,
Madhavan Srinivasan <maddy@linux.ibm.com>,
Michael Ellerman <mpe@ellerman.id.au>,
Nicholas Piggin <npiggin@gmail.com>
Cc: linuxppc-dev@lists.ozlabs.org,
linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org, devicetree@vger.kernel.org,
linux-gpio@vger.kernel.org,
Thomas Petazzoni <thomas.petazzoni@bootlin.com>
Subject: Re: [PATCH 08/12] soc: fsl: qe: Convert to generic IRQ chip
Date: Mon, 6 Jul 2026 09:29:26 +0200 [thread overview]
Message-ID: <34946670-2a76-47fa-af82-3d70cdc9d8f0@kernel.org> (raw)
In-Reply-To: <20260703-qe-pic-gpios-v1-8-6c3e706e27dc@bootlin.com>
Hi Paul,
Le 03/07/2026 à 15:30, Paul Louvel a écrit :
> The generic IRQ chip framework is available to handle IRQ chips. Using
> this framework for the QE interrupt controller allows to simplify the
> driver. Indeed, the framework internally handles operations coded
> directly in the driver.
>
> Add a select dependency to GENERIC_IRQ_CHIP in the PPC platform Kconfig.
>
> Signed-off-by: Paul Louvel <paul.louvel@bootlin.com>
> ---
> arch/powerpc/platforms/Kconfig | 1 +
> drivers/soc/fsl/qe/qe_ports_ic.c | 103 ++++++++++++++++++++++++++-------------
> 2 files changed, 70 insertions(+), 34 deletions(-)
>
> diff --git a/arch/powerpc/platforms/Kconfig b/arch/powerpc/platforms/Kconfig
> index c4e61843d9d9..b0b3a80f8cde 100644
> --- a/arch/powerpc/platforms/Kconfig
> +++ b/arch/powerpc/platforms/Kconfig
> @@ -232,6 +232,7 @@ config QE_GPIO
> bool "QE GPIO support"
> depends on QUICC_ENGINE
> select GPIOLIB
> + select GENERIC_IRQ_CHIP
> help
> Say Y here if you're going to use hardware that connects to the
> QE GPIOs.
> diff --git a/drivers/soc/fsl/qe/qe_ports_ic.c b/drivers/soc/fsl/qe/qe_ports_ic.c
> index c8b73b0aa233..d022aa224f6d 100644
> --- a/drivers/soc/fsl/qe/qe_ports_ic.c
> +++ b/drivers/soc/fsl/qe/qe_ports_ic.c
> @@ -20,63 +20,65 @@ struct qepic_data {
> void __iomem *reg;
> struct irq_domain *host;
> int irq;
> + struct irq_chip_generic *gc;
> };
>
> static void qepic_mask(struct irq_data *d)
> {
> - struct qepic_data *data = irq_data_get_irq_chip_data(d);
> + struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
> + struct irq_chip_type *ct = irq_data_get_chip_type(d);
>
> - clrbits32(data->reg + CEPIMR, 1 << (31 - irqd_to_hwirq(d)));
> + clrbits32(gc->reg_base + ct->regs.mask, d->mask);
Is there a real added value with this change ?
Previously we had:
00000000 <qepic_mask>:
0: 81 03 00 18 lwz r8,24(r3)
4: 81 28 00 00 lwz r9,0(r8)
8: 7c 00 04 ac hwsync
c: 81 29 00 10 lwz r9,16(r9)
10: 0c 09 00 00 twi 0,r9,0
14: 4c 00 01 2c isync
18: 80 e3 00 08 lwz r7,8(r3)
1c: 3d 40 80 00 lis r10,-32768
20: 81 08 00 00 lwz r8,0(r8)
24: 7d 4a 3c 30 srw r10,r10,r7
28: 7d 29 50 78 andc r9,r9,r10
2c: 7c 00 04 ac hwsync
30: 91 28 00 10 stw r9,16(r8)
34: 4e 80 00 20 blr
Now we have:
00000000 <qepic_mask>:
0: 80 e3 00 18 lwz r7,24(r3)
4: 81 03 00 10 lwz r8,16(r3)
8: 81 27 00 00 lwz r9,0(r7)
c: 81 48 00 94 lwz r10,148(r8)
10: 7d 29 52 14 add r9,r9,r10
14: 7c 00 04 ac hwsync
18: 81 49 00 00 lwz r10,0(r9)
1c: 0c 0a 00 00 twi 0,r10,0
20: 4c 00 01 2c isync
24: 80 c3 00 00 lwz r6,0(r3)
28: 81 27 00 00 lwz r9,0(r7)
2c: 81 08 00 94 lwz r8,148(r8)
30: 7d 4a 30 78 andc r10,r10,r6
34: 7d 29 42 14 add r9,r9,r8
38: 7c 00 04 ac hwsync
3c: 91 49 00 00 stw r10,0(r9)
40: 4e 80 00 20 blr
We now have three more indirect loads (8x lwz instead of 5x), for
loading some value which is already known at compile time.
> }
>
> static void qepic_unmask(struct irq_data *d)
> {
> - struct qepic_data *data = irq_data_get_irq_chip_data(d);
> + struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
> + struct irq_chip_type *ct = irq_data_get_chip_type(d);
>
> - setbits32(data->reg + CEPIMR, 1 << (31 - irqd_to_hwirq(d)));
> + setbits32(gc->reg_base + ct->regs.mask, d->mask);
> }
>
> static void qepic_end(struct irq_data *d)
> {
> - struct qepic_data *data = irq_data_get_irq_chip_data(d);
> + struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
> + struct irq_chip_type *ct = irq_data_get_chip_type(d);
>
> - out_be32(data->reg + CEPIER, 1 << (31 - irqd_to_hwirq(d)));
> + out_be32(gc->reg_base + ct->regs.eoi, d->mask);
> +}
> +
> +static void qepic_calc_mask(struct irq_data *d)
> +{
> + d->mask = 1 << (31 - irqd_to_hwirq(d));
> }
>
> static int qepic_set_type(struct irq_data *d, unsigned int flow_type)
> {
> - struct qepic_data *data = irq_data_get_irq_chip_data(d);
> - unsigned int vec = (unsigned int)irqd_to_hwirq(d);
> + struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
> + struct irq_chip_type *ct = irq_data_get_chip_type(d);
>
> switch (flow_type & IRQ_TYPE_SENSE_MASK) {
> case IRQ_TYPE_EDGE_FALLING:
> - setbits32(data->reg + CEPICR, 1 << (31 - vec));
> + setbits32(gc->reg_base + ct->regs.type, d->mask);
> return 0;
> case IRQ_TYPE_EDGE_BOTH:
> case IRQ_TYPE_NONE:
> - clrbits32(data->reg + CEPICR, 1 << (31 - vec));
> + clrbits32(gc->reg_base + ct->regs.type, d->mask);
> return 0;
> }
> return -EINVAL;
> }
>
> -static struct irq_chip qepic = {
> - .name = "QEPIC",
> - .irq_mask = qepic_mask,
> - .irq_unmask = qepic_unmask,
> - .irq_eoi = qepic_end,
> - .irq_set_type = qepic_set_type,
> -};
> -
> static void qepic_cascade(struct irq_desc *desc)
> {
> struct qepic_data *data = irq_desc_get_handler_data(desc);
> + struct irq_chip_type *ct = data->gc->chip_types;
> struct irq_chip *chip = irq_desc_get_chip(desc);
> unsigned long event, bit;
>
> chained_irq_enter(chip, desc);
>
> - event = in_be32(data->reg + CEPIER);
> + event = in_be32(data->gc->reg_base + ct->regs.eoi);
> if (!event) {
> handle_bad_irq(desc);
> goto out;
> @@ -89,33 +91,64 @@ static void qepic_cascade(struct irq_desc *desc)
> chained_irq_exit(chip, desc);
> }
>
> -static int qepic_host_map(struct irq_domain *h, unsigned int virq, irq_hw_number_t hw)
> +static int qepic_chip_init(struct irq_chip_generic *gc)
> {
> - irq_set_chip_data(virq, h->host_data);
> - irq_set_chip_and_handler(virq, &qepic, handle_fasteoi_irq);
> + struct irq_chip_type *ct = gc->chip_types;
> +
> + ct->regs.mask = CEPIMR;
> + ct->chip.irq_mask = qepic_mask;
> + ct->chip.irq_unmask = qepic_unmask;
> + ct->regs.eoi = CEPIER;
> + ct->chip.irq_eoi = qepic_end;
> + ct->regs.type = CEPICR;
> + ct->chip.irq_set_type = qepic_set_type;
> + ct->chip.irq_calc_mask = qepic_calc_mask;
Are ct->regs.mask, ct->regs.eoi and ct->regs.type used anywhere else
than locally in qepic_{mask/unmask/end/set_type} ?
Christophe
> +
> return 0;
> }
>
> -static const struct irq_domain_ops qepic_host_ops = {
> - .map = qepic_host_map,
> -};
> +static int qepic_domain_init(struct irq_domain *d)
> +{
> + struct qepic_data *data = d->host_data;
>
> -static void qepic_remove(void *res)
> + irq_set_chained_handler_and_data(data->irq, qepic_cascade, data);
> +
> + return 0;
> +}
> +
> +static void qepic_domain_exit(struct irq_domain *d)
> {
> - struct qepic_data *data = res;
> + struct qepic_data *data = d->host_data;
>
> irq_set_chained_handler_and_data(data->irq, NULL, NULL);
> - irq_domain_remove(data->host);
> }
>
> static int qepic_probe(struct platform_device *pdev)
> {
> + struct irq_domain_chip_generic_info dgc_info = {
> + .name = "QEPIC",
> + .handler = handle_fasteoi_irq,
> + .irqs_per_chip = 32,
> + .num_ct = 1,
> + .init = qepic_chip_init,
> + };
> + struct irq_domain_info d_info = {
> + .fwnode = of_fwnode_handle(pdev->dev.of_node),
> + .domain_flags = IRQ_DOMAIN_FLAG_DESTROY_GC,
> + .size = 32,
> + .hwirq_max = 32,
> + .ops = &irq_generic_chip_ops,
> + .dgc_info = &dgc_info,
> + .init = qepic_domain_init,
> + .exit = qepic_domain_exit,
> + };
> struct device *dev = &pdev->dev;
> struct qepic_data *data;
>
> data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
> if (!data)
> return -ENOMEM;
> + d_info.host_data = data;
>
> data->reg = devm_platform_ioremap_resource(pdev, 0);
> if (IS_ERR(data->reg))
> @@ -125,14 +158,16 @@ static int qepic_probe(struct platform_device *pdev)
> if (data->irq < 0)
> return data->irq;
>
> - data->host = irq_domain_create_linear(dev_fwnode(dev), 32, &qepic_host_ops, data);
> - if (!data->host)
> - return -ENODEV;
> + data->host = devm_irq_domain_instantiate(dev, &d_info);
> + if (IS_ERR(data->host))
> + return PTR_ERR(data->host);
>
> - irq_set_chained_handler_and_data(data->irq, qepic_cascade, data);
> -
> - return devm_add_action_or_reset(dev, qepic_remove, data);
> + data->gc = irq_get_domain_generic_chip(data->host, 0);
> + if (!data->gc)
> + return -ENODEV;
> + data->gc->reg_base = data->reg;
>
> + return 0;
> }
>
> static const struct of_device_id qepic_match[] = {
>
next prev parent reply other threads:[~2026-07-06 7:29 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-07-03 13:30 [PATCH 00/12] soc: fsl: qe: QE PIC improvement and add support of IRQs to QUICC ENGINE GPIOs Paul Louvel
2026-07-03 13:30 ` [PATCH 01/12] soc: fsl: qe: Add chained_irq_{enter,exit}() calls in cascade handler Paul Louvel
2026-07-03 13:30 ` [PATCH 02/12] dt-bindings: soc: fsl: qe: Set #interrupt-cells to 2 to support interrupt type encoding Paul Louvel
2026-07-03 13:30 ` [PATCH 03/12] dt-bindings: soc: fsl: qe: Convert QE GPIO to DT schema Paul Louvel
2026-07-06 6:48 ` Krzysztof Kozlowski
2026-07-06 9:03 ` Paul Louvel
2026-07-03 13:30 ` [PATCH 04/12] dt-bindings: soc: fsl: qe: Add support of IRQ in QE GPIO Paul Louvel
2026-07-06 6:52 ` Krzysztof Kozlowski
2026-07-06 8:48 ` Paul Louvel
2026-07-03 13:30 ` [PATCH 05/12] soc: fsl: qe: Use generic_handle_domain_irq() Paul Louvel
2026-07-03 13:30 ` [PATCH 06/12] soc: fsl: qe: Iterate over all pending interrupts in cascade handler Paul Louvel
2026-07-03 13:30 ` [PATCH 07/12] soc: fsl: qe: Handle spurious interrupts Paul Louvel
2026-07-03 13:30 ` [PATCH 08/12] soc: fsl: qe: Convert to generic IRQ chip Paul Louvel
2026-07-06 7:29 ` Christophe Leroy (CS GROUP) [this message]
2026-07-06 8:56 ` Paul Louvel
2026-07-03 13:30 ` [PATCH 09/12] soc: fsl: qe: Rename irq variable to parent_irq Paul Louvel
2026-07-03 13:30 ` [PATCH 10/12] soc: fsl: qe: Rename host member to domain in struct qepic_data Paul Louvel
2026-07-03 13:30 ` [PATCH 11/12] soc: fsl: qe: Remove useless struct member Paul Louvel
2026-07-03 13:30 ` [PATCH 12/12] soc: fsl: qe: Add support of IRQs in QE GPIO Paul Louvel
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=34946670-2a76-47fa-af82-3d70cdc9d8f0@kernel.org \
--to=chleroy@kernel.org \
--cc=brgl@kernel.org \
--cc=conor+dt@kernel.org \
--cc=devicetree@vger.kernel.org \
--cc=krzk+dt@kernel.org \
--cc=linusw@kernel.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-gpio@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linuxppc-dev@lists.ozlabs.org \
--cc=maddy@linux.ibm.com \
--cc=mpe@ellerman.id.au \
--cc=npiggin@gmail.com \
--cc=paul.louvel@bootlin.com \
--cc=qiang.zhao@nxp.com \
--cc=robh@kernel.org \
--cc=tglx@kernel.org \
--cc=thomas.petazzoni@bootlin.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox