Re: [PATCH] sh: maple: use the matching IRQ cookie

[Florian Fuchs]

On 09 Oct 12:02, Runyu Xiao wrote:
> The Maple DMA and VBLANK handlers are registered with
> &maple_unsupported_device as their dev_id. Passing NULL to free_irq()
> cannot find either action, so the handlers remain registered and the
> IRQ core warns.
> 
> Use the same cookie when freeing both handlers on the error paths.
> 
> Fixes: 17be2d2b1c33 ("sh: Add maple bus support for the SEGA Dreamcast.")
> Cc: stable@xxxxxxxxxxxxxxx
> Assisted-by: LLM
> Signed-off-by: Runyu Xiao <runyu.xiao@xxxxxxxxxx>
> ---
>  drivers/sh/maple/maple.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/sh/maple/maple.c b/drivers/sh/maple/maple.c
> index 5585f220e..7df88e00c 100644
> --- a/drivers/sh/maple/maple.c
> +++ b/drivers/sh/maple/maple.c
> @@ -863,10 +863,10 @@ static int __init maple_bus_init(void)
>  	kmem_cache_destroy(maple_queue_cache);
>  
>  cleanup_bothirqs:
> -	free_irq(HW_EVENT_VSYNC, 0);
> +	free_irq(HW_EVENT_VSYNC, &maple_unsupported_device);
>  
>  cleanup_irq:
> -	free_irq(HW_EVENT_MAPLE_DMA, 0);
> +	free_irq(HW_EVENT_MAPLE_DMA, &maple_unsupported_device);
>  
>  cleanup_dma:
>  	free_pages((unsigned long) maple_sendbuf, MAPLE_DMA_PAGES);
> -- 
> 2.34.1

The patch looks correct to me and works like intended, that it frees the
corresponding IRQ handler in the error path.

Reviewed-By: Florian Fuchs <fuchsfl@xxxxxxxxx>
Tested-by: Florian Fuchs <fuchsfl@xxxxxxxxx>
Fixes: b3c69e248176 ("maple: more robust device detection.")

Just a little addition, it looks like commit b3c69e248176
("maple: more robust device detection.") introduced the issue, where the
static irqaction with IRQF_SHARED, was replaced by a request_irq() with
a cookie, but the free_irq() wasn't updated.

I tested the patch with a forced failure of maple_bus_init(), which
gives the WARNs and keeps the handlers registered in /proc/interrupts:
Trying to free already-free IRQ 69
WARNING: kernel/irq/manage.c:1880 at 0x8c0522c4, CPU#0: swapper/0/1

After the patch, the IRQ handlers are unregistered in /proc/interrupts
and no kernel WARN is shown.

And tested if keyboard, mouse still work with the patch, which they do.

Thank you!
Regards
Florian