RE: [PATCH v3 2/4] irqchip/imx-irqsteer: Dispose of IRQ mappings in remove()

NXP Confidential
> -----Original Message-----
> From: Frank Li (OSS) <[email protected]>
> Sent: 2026年8月8日 3:11
> To: Zhipeng Wang (OSS) <[email protected]>
> Cc: Thomas Gleixner <[email protected]>; Marc Zyngier <[email protected]>;
> Frank Li <[email protected]>; Sascha Hauer <[email protected]>;
> Pengutronix Kernel Team <[email protected]>; Fabio Estevam
> <[email protected]>; Jindong Yue <[email protected]>; Xuegang Liu
> <[email protected]>; [email protected]; [email protected];
> [email protected]
> Subject: Re: [PATCH v3 2/4] irqchip/imx-irqsteer: Dispose of IRQ mappings in
> remove()
>
> On Fri, Aug 07, 2026 at 04:23:44PM +0900, [email protected]
> wrote:
> > From: Zhipeng Wang <[email protected]>
> >
> > remove() tears down the chained handlers and the IRQ domain but never
> > disposes of the IRQ mappings it created. The parent mappings from
> > irq_of_parse_and_map() and the child mappings handed out by the domain
> > are leaked, and the child irq_descs are left pointing at the driver's
> > irq_chip past irq_domain_remove().
> >
> > Dispose of the parent mappings alongside the chained handler teardown,
> > and dispose of the child mappings before removing the domain.
> >
> > Fixes: 0136afa08967 ("irqchip: Add driver for imx-irqsteer
> > controller")
> > Signed-off-by: Zhipeng Wang <[email protected]>
> > ---
> > Changes in v3:
> >  - Split out of the single v2 patch. In v2 this was folded into the
> >    module-conversion patch; no functional change.
> >
> >  drivers/irqchip/irq-imx-irqsteer.c | 7 ++++++-
> >  1 file changed, 6 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/irqchip/irq-imx-irqsteer.c
> > b/drivers/irqchip/irq-imx-irqsteer.c
> > index a2f0629b22a3..4a2fe8ba97f5 100644
> > --- a/drivers/irqchip/irq-imx-irqsteer.c
> > +++ b/drivers/irqchip/irq-imx-irqsteer.c
> > @@ -277,7 +277,7 @@ static int imx_irqsteer_probe(struct
> > platform_device *pdev)  static void imx_irqsteer_remove(struct
> > platform_device *pdev)  {
> >     struct irqsteer_data *irqsteer_data = platform_get_drvdata(pdev);
> > -   int i;
> > +   int hwirq, i;
> >
> >     for (i = 0; i < irqsteer_data->irq_count; i++) {
> >             if (!irqsteer_data->irq[i])
> > @@ -285,8 +285,13 @@ static void imx_irqsteer_remove(struct
> > platform_device *pdev)
> >
> >             irq_set_chained_handler_and_data(irqsteer_data->irq[i],
> >                                              NULL, NULL);
> > +           irq_dispose_mapping(irqsteer_data->irq[i]);
> >     }
> >
> > +   for (hwirq = 0; hwirq < irqsteer_data->reg_num * 32; hwirq++)
> > +           irq_dispose_mapping(irq_find_mapping(irqsteer_data->domain,
> > +                                                hwirq));
> > +
>
> only call once irq_of_parse_and_map(), why need irq_dispose_mapping()
> twice?
>
> Frank
The two loops disposed of two different sets of mappings, not the same
ones twice:

 - the first loop disposed of the parent output mappings the driver
   created itself with irq_of_parse_and_map() (irq_count of them);
 - the second loop disposed of the child input mappings the domain
   handed out to downstream consumers (reg_num * 32 hwirqs).

That said, the child mappings are owned and freed by their consumers,
and with the domain now managed by devres (patch 2/5) they are torn down
automatically. So the child loop was both unnecessary and not the
driver's job. In v4 (patch 3/5) remove() only disposes of the parent
mappings it created, matching the sibling irq-imx-intmux.c.

BRs,
Zhipeng

>
> >     irq_domain_remove(irqsteer_data->domain);
> >
> >     clk_disable_unprepare(irqsteer_data->ipg_clk);
> > --
> > 2.34.1
> >
> >