Re: [PATCH 06/16] irqchip/loongson-liointc: unmap per-core iomaps on error

[Radu Rendec]

On Tue, 2026-07-14 at 21:24 +0800, Haofeng Li wrote:
> From: Haofeng Li <lihaofeng@xxxxxxxxxx>
> 
> For revision > 1, liointc_init() maps per-core ISR regions with
> of_iomap(). out_iounmap only unmapped the main base, leaking successful
> per-core mappings on later failures.
> 
> On the error path, iounmap any core_isr entry that is not an offset into
> the main base mapping.
> 
> Fixes: 0858ed035a85 ("irqchip/loongson-liointc: Add ACPI init support")
> Signed-off-by: Haofeng Li <lihaofeng@xxxxxxxxxx>
> ---
>  drivers/irqchip/irq-loongson-liointc.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/drivers/irqchip/irq-loongson-liointc.c b/drivers/irqchip/irq-loongson-liointc.c
> index cf44a333b9c8..6451d8b466ce 100644
> --- a/drivers/irqchip/irq-loongson-liointc.c
> +++ b/drivers/irqchip/irq-loongson-liointc.c
> @@ -312,6 +312,14 @@ static int liointc_init(phys_addr_t addr, unsigned long size, int revision,
>  out_free_domain:
>  	irq_domain_remove(domain);
>  out_iounmap:
> +	if (revision > 1) {
> +		for (i = 0; i < LIOINTC_NUM_CORES; i++) {
> +			void __iomem *core_base = base + LIOINTC_REG_INTC_STATUS(i);
> +
> +			if (priv->core_isr[i] && priv->core_isr[i] != core_base)

The first condition here is unnecessary. It's safe to call iounmap()
with a NULL pointer (like most deallocation functions in Linux).

Can you please explain why the second condition is needed? Even if the
physical ranges happen to overlap, they are mapped independently via
the of_iomap() call earlier in the function. That allocates a separate
vm area (and a PTE) that still needs to be freed.

FWIW, in the two relevant device trees I could find, the physical
ranges do *not* overlap:
arch/loongarch/boot/dts/loongson-2k0500.dtsi
arch/loongarch/boot/dts/loongson-2k1000.dtsi

> +				iounmap(priv->core_isr[i]);
> +		}
> +	}
>  	iounmap(base);
>  out_free_priv:
>  	kfree(priv);