On Wed, Apr 23, 2014 at 09:27:58AM +0200, Thierry Reding wrote:
> On Tue, Apr 22, 2014 at 10:44:20PM +0200, Daniel Vetter wrote:
> [...]
> > diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c
> > index 589e865832cd..7cf407bbfed5 100644
> > --- a/drivers/gpu/drm/drm_irq.c
> > +++ b/drivers/gpu/drm/drm_irq.c
> > @@ -249,14 +249,16 @@ static inline int drm_dev_to_irq(struct drm_device *dev)
> > */
> > int drm_irq_install(struct drm_device *dev)
> > {
> > - int ret;
> > + int ret, irq;
> > unsigned long sh_flags = 0;
> > char *irqname;
> >
> > + irq = drm_dev_to_irq(dev);
>
> I think the assignment could have happened either when the variable is
> declared, or...
>
> > +
> > if (!drm_core_check_feature(dev, DRIVER_HAVE_IRQ))
> > return -EINVAL;
> >
> > - if (drm_dev_to_irq(dev) == 0)
> > + if (irq == 0)
>
> ... right above this, since it is where it is first used (it may not be
> necessary to query it before here at all if the driver doesn't set
> DRIVER_HAVE_IRQ).
>
> But I realize that that's pure bike-shedding, so either way:
Follow-on patches will move this assignement into drivers and make int irq
an function parameter, so I think I'll leave this ;-)
>
> Reviewed-by: Thierry Reding <treding@xxxxxxxxxx>
Thanks for the review, I'll send the pull request to Dave now.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/dri-devel
