* [PATCH] drm/irq: Modify the return value type of drm_irq_uninstall @ 2020-11-02 12:38 Tian Tao 2020-11-02 12:48 ` Thomas Zimmermann 0 siblings, 1 reply; 3+ messages in thread From: Tian Tao @ 2020-11-02 12:38 UTC (permalink / raw) To: maarten.lankhorst, mripard, tzimmermann, airlied, daniel, dri-devel, linux-kernel 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); 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 -- 2.7.4 ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] drm/irq: Modify the return value type of drm_irq_uninstall 2020-11-02 12:38 [PATCH] drm/irq: Modify the return value type of drm_irq_uninstall Tian Tao @ 2020-11-02 12:48 ` Thomas Zimmermann 2020-11-02 13:05 ` Daniel Vetter 0 siblings, 1 reply; 3+ messages in thread From: Thomas Zimmermann @ 2020-11-02 12:48 UTC (permalink / raw) To: Tian Tao, maarten.lankhorst, mripard, airlied, daniel, dri-devel, linux-kernel [-- Attachment #1.1.1: Type: text/plain, Size: 2817 bytes --] 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. 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 #1.1.2: OpenPGP_0x680DC11D530B7A23.asc --] [-- Type: application/pgp-keys, Size: 4259 bytes --] [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 495 bytes --] ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] drm/irq: Modify the return value type of drm_irq_uninstall 2020-11-02 12:48 ` Thomas Zimmermann @ 2020-11-02 13:05 ` Daniel Vetter 0 siblings, 0 replies; 3+ messages in thread From: Daniel Vetter @ 2020-11-02 13:05 UTC (permalink / raw) To: Thomas Zimmermann Cc: Tian Tao, Maarten Lankhorst, Maxime Ripard, Dave Airlie, dri-devel, Linux Kernel Mailing List 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 -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2020-11-02 13:06 UTC | newest] Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-11-02 12:38 [PATCH] drm/irq: Modify the return value type of drm_irq_uninstall Tian Tao 2020-11-02 12:48 ` Thomas Zimmermann 2020-11-02 13:05 ` Daniel Vetter
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®