| Message ID | 1606898835-40775-3-git-send-email-tiantao6@hisilicon.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Add the new api to install irq | expand |
Hi Tian. On Wed, Dec 02, 2020 at 04:47:14PM +0800, Tian Tao wrote: > Add new api devm_drm_irq_install() to register interrupts, > no need to call drm_irq_uninstall() when the drm module is removed. > > Signed-off-by: Tian Tao <tiantao6@hisilicon.com> Just a few details to fix. Sam > --- > drivers/gpu/drm/drm_irq.c | 35 +++++++++++++++++++++++++++++++++++ > include/drm/drm_irq.h | 2 +- > 2 files changed, 36 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c > index 09d6e9e..b363dec 100644 > --- a/drivers/gpu/drm/drm_irq.c > +++ b/drivers/gpu/drm/drm_irq.c > @@ -214,6 +214,41 @@ int drm_irq_uninstall(struct drm_device *dev) > } > EXPORT_SYMBOL(drm_irq_uninstall); > > +static void devm_drm_irq_uninstall(void *data) > +{ > + drm_irq_uninstall(data); > +} > + > +/** > + * devm_drm_irq_install - install IRQ handler > + * @dev: DRM device > + * @irq: IRQ number to install the handler for > + * > + * devm_drm_irq_install is a help function of drm_irq_install. Drop the extra space after "is a" > + * > + * if the driver uses devm_drm_irq_install, there is no need Start with capital "I" in If > + * to call drm_irq_uninstall when the drm module get unloaded, > + * as this will done automagically. > + * > + * Returns: > + * Zero on success or a negative error code on failure. > + */ > +int devm_drm_irq_install(struct drm_device *dev, int irq) > +{ > + int ret; > + > + ret = drm_irq_install(dev, irq); > + if (ret) > + return ret; > + > + ret = devm_add_action_or_reset(dev->dev, devm_drm_irq_uninstall, dev); > + if (ret) > + devm_drm_irq_uninstall(dev); devm_add_action_or_reset() will call devm_drm_irq_uninstall() if ret is != 0. See include/device.h. I guess that is the "_or_reset" part of the name that can tell us that. So you can drop the if condition as it just will cause the code to call drm_irq_uninstall() twice. > + > + return ret; > +} > +EXPORT_SYMBOL(devm_drm_irq_install); > + > #if IS_ENABLED(CONFIG_DRM_LEGACY) > int drm_legacy_irq_control(struct drm_device *dev, void *data, > struct drm_file *file_priv) > diff --git a/include/drm/drm_irq.h b/include/drm/drm_irq.h > index d77f6e6..631b22f 100644 > --- a/include/drm/drm_irq.h > +++ b/include/drm/drm_irq.h > @@ -28,5 +28,5 @@ struct drm_device; > > int drm_irq_install(struct drm_device *dev, int irq); > int drm_irq_uninstall(struct drm_device *dev); > - > +int devm_drm_irq_install(struct drm_device *dev, int irq); > #endif > -- > 2.7.4 > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel
Hi Tian, > > + ret = devm_add_action_or_reset(dev->dev, devm_drm_irq_uninstall, dev); > > + if (ret) > > + devm_drm_irq_uninstall(dev); > devm_add_action_or_reset() will call devm_drm_irq_uninstall() if ret is > != 0. See include/device.h. > > I guess that is the "_or_reset" part of the name that can tell us that. > So you can drop the if condition as it just will cause the code to call > drm_irq_uninstall() twice. Noticed this was fixed in v2 - so all is fine here. Sam
diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c index 09d6e9e..b363dec 100644 --- a/drivers/gpu/drm/drm_irq.c +++ b/drivers/gpu/drm/drm_irq.c @@ -214,6 +214,41 @@ int drm_irq_uninstall(struct drm_device *dev) } EXPORT_SYMBOL(drm_irq_uninstall); +static void devm_drm_irq_uninstall(void *data) +{ + drm_irq_uninstall(data); +} + +/** + * devm_drm_irq_install - install IRQ handler + * @dev: DRM device + * @irq: IRQ number to install the handler for + * + * devm_drm_irq_install is a help function of drm_irq_install. + * + * if the driver uses devm_drm_irq_install, there is no need + * to call drm_irq_uninstall when the drm module get unloaded, + * as this will done automagically. + * + * Returns: + * Zero on success or a negative error code on failure. + */ +int devm_drm_irq_install(struct drm_device *dev, int irq) +{ + int ret; + + ret = drm_irq_install(dev, irq); + if (ret) + return ret; + + ret = devm_add_action_or_reset(dev->dev, devm_drm_irq_uninstall, dev); + if (ret) + devm_drm_irq_uninstall(dev); + + return ret; +} +EXPORT_SYMBOL(devm_drm_irq_install); + #if IS_ENABLED(CONFIG_DRM_LEGACY) int drm_legacy_irq_control(struct drm_device *dev, void *data, struct drm_file *file_priv) diff --git a/include/drm/drm_irq.h b/include/drm/drm_irq.h index d77f6e6..631b22f 100644 --- a/include/drm/drm_irq.h +++ b/include/drm/drm_irq.h @@ -28,5 +28,5 @@ struct drm_device; int drm_irq_install(struct drm_device *dev, int irq); int drm_irq_uninstall(struct drm_device *dev); - +int devm_drm_irq_install(struct drm_device *dev, int irq); #endif
Add new api devm_drm_irq_install() to register interrupts, no need to call drm_irq_uninstall() when the drm module is removed. Signed-off-by: Tian Tao <tiantao6@hisilicon.com> --- drivers/gpu/drm/drm_irq.c | 35 +++++++++++++++++++++++++++++++++++ include/drm/drm_irq.h | 2 +- 2 files changed, 36 insertions(+), 1 deletion(-)