On Mon, Nov 2, 2020 at 1:48 PM Thomas Zimmermann tzimmermann@suse.de wrote:

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@hisilicon.com

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.

It's old UMS gunk, no one cares :-)

If you're paranoid, make an internal __drm_irq_uninstall function which keeps the return value. But what we probably want here is to just split this up into 2 functions, drm_legacy_irq_uninstall, which does the validation and additional check, and then calls drm_irq_uninstall, which is for kms drivers, and which just has a WARN_ON(!dev->irq_installed) or so to catch driver bugs. -Daniel

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