Re: [PATCH 2/7] drm/xe: Fix error handling in xe_irq_install()

[Rodrigo Vivi <rodrigo.vivi@intel.com>]

On Fri, Jan 31, 2025 at 02:31:35PM -0800, Lucas De Marchi wrote:
> When devm_add_action_or_reset() fails, it already calls the function
> passed as parameter and that function is already free'ing the irqs.
> Drop the goto and just return.
> 
> The caller, xe_device_probe(), should also do the same thing instead of
> wrongly doing `goto err` and calling the unrelated xe_display_fini()
> function.
> 
> Cc: Dani Liberman <dliberman@habana.ai>
> Cc: Ohad Sharabi <osharabi@habana.ai>
> Fixes: 14d25d8d684d ("drm/xe: change old msi irq api to a new one")
> Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>

Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

> ---
>  drivers/gpu/drm/xe/xe_device.c |  2 +-
>  drivers/gpu/drm/xe/xe_irq.c    | 14 +-------------
>  2 files changed, 2 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_device.c b/drivers/gpu/drm/xe/xe_device.c
> index 589bbe21c32b4..2e874906d4f31 100644
> --- a/drivers/gpu/drm/xe/xe_device.c
> +++ b/drivers/gpu/drm/xe/xe_device.c
> @@ -846,7 +846,7 @@ int xe_device_probe(struct xe_device *xe)
>  
>  	err = xe_irq_install(xe);
>  	if (err)
> -		goto err;
> +		return err;
>  
>  	for_each_gt(gt, xe, id) {
>  		last_gt = id;
> diff --git a/drivers/gpu/drm/xe/xe_irq.c b/drivers/gpu/drm/xe/xe_irq.c
> index 32f5a67a917b5..08552ee3fb94b 100644
> --- a/drivers/gpu/drm/xe/xe_irq.c
> +++ b/drivers/gpu/drm/xe/xe_irq.c
> @@ -757,19 +757,7 @@ int xe_irq_install(struct xe_device *xe)
>  
>  	xe_irq_postinstall(xe);
>  
> -	err = devm_add_action_or_reset(xe->drm.dev, irq_uninstall, xe);
> -	if (err)
> -		goto free_irq_handler;
> -
> -	return 0;
> -
> -free_irq_handler:
> -	if (xe_device_has_msix(xe))
> -		xe_irq_msix_free(xe);
> -	else
> -		xe_irq_msi_free(xe);
> -
> -	return err;
> +	return devm_add_action_or_reset(xe->drm.dev, irq_uninstall, xe);
>  }
>  
>  static void xe_irq_msi_synchronize_irq(struct xe_device *xe)
> -- 
> 2.48.0
>