Re: [PATCH] drm/irq: Modify the return value type of drm_irq_uninstall

[Thomas Zimmermann]

Hi

Am 02.11.20 um 13:38 schrieb Tian Tao:
> There is no driver to use the return value of drm_irq_uninstal,
> so modify the return value type of drm_irq_uninstal to void.
> 
> Signed-off-by: Tian Tao <tiantao6@xxxxxxxxxxxxx>
> ---
>  drivers/gpu/drm/drm_irq.c | 13 ++++++-------
>  include/drm/drm_irq.h     |  2 +-
>  2 files changed, 7 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c
> index 7537a3d..45e6471 100644
> --- a/drivers/gpu/drm/drm_irq.c
> +++ b/drivers/gpu/drm/drm_irq.c
> @@ -166,14 +166,14 @@ EXPORT_SYMBOL(drm_irq_install);
>   * Returns:
>   * Zero on success or a negative error code on failure.
>   */
> -int drm_irq_uninstall(struct drm_device *dev)
> +void drm_irq_uninstall(struct drm_device *dev)
>  {
>  	unsigned long irqflags;
>  	bool irq_enabled;
>  	int i;
>  
>  	if (!dev->irq_enabled || !dev)
> -		return 0;
> +		return;
>  
>  	irq_enabled = dev->irq_enabled;
>  	dev->irq_enabled = false;
> @@ -200,8 +200,8 @@ int drm_irq_uninstall(struct drm_device *dev)
>  		spin_unlock_irqrestore(&dev->vbl_lock, irqflags);
>  	}
>  
> -	if (!irq_enabled)
> -		return -EINVAL;
> +	if (!drm_WARN_ON(dev, !irq_enabled))
> +		return;
>  
>  	DRM_DEBUG("irq=%d\n", dev->irq);
>  
> @@ -213,7 +213,6 @@ int drm_irq_uninstall(struct drm_device *dev)
>  
>  	free_irq(dev->irq, dev);
>  
> -	return 0;
>  }
>  EXPORT_SYMBOL(drm_irq_uninstall);
>  
> @@ -250,10 +249,10 @@ int drm_legacy_irq_control(struct drm_device *dev, void *data,
>  		return ret;
>  	case DRM_UNINST_HANDLER:
>  		mutex_lock(&dev->struct_mutex);
> -		ret = drm_irq_uninstall(dev);
> +		drm_irq_uninstall(dev);

Oh, there actually is a user of this result! I grep'ed for this but
didn't see it. I'm sorry for misleading you here.

This is ioctl code and who which program depends on it.So we cannot
actually drop the result code.

I'll just ack your original patch, or you could add the managed
interface that I described and convert hibmc to it. Your choice, let me
know.

Best regards
Thomas

>  		mutex_unlock(&dev->struct_mutex);
>  
> -		return ret;
> +		return 0;
>  	default:
>  		return -EINVAL;
>  	}
> diff --git a/include/drm/drm_irq.h b/include/drm/drm_irq.h
> index d77f6e6..d9f6ec0 100644
> --- a/include/drm/drm_irq.h
> +++ b/include/drm/drm_irq.h
> @@ -27,6 +27,6 @@
>  struct drm_device;
>  
>  int drm_irq_install(struct drm_device *dev, int irq);
> -int drm_irq_uninstall(struct drm_device *dev);
> +void drm_irq_uninstall(struct drm_device *dev);
>  
>  #endif
> 

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer

Attachment: OpenPGP_0x680DC11D530B7A23.asc
Description: application/pgp-keys

Attachment: OpenPGP_signature
Description: OpenPGP digital signature