From mboxrd@z Thu Jan  1 00:00:00 1970
From: Daniel Vetter <daniel@ffwll.ch>
Subject: Re: [PATCH] drm/irq: track the irq installed in drm_irq_install in
 dev->irq
Date: Wed, 23 Apr 2014 10:31:58 +0200
Message-ID: <20140423083158.GX10722@phenom.ffwll.local>
References: <1397252175-14227-11-git-send-email-daniel.vetter@ffwll.ch>
 <1398199460-21508-1-git-send-email-daniel.vetter@ffwll.ch>
 <20140423072756.GC31226@ulmo>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Return-path: <dri-devel-bounces@lists.freedesktop.org>
Received: from mail-ee0-f46.google.com (mail-ee0-f46.google.com [74.125.83.46])
 by gabe.freedesktop.org (Postfix) with ESMTP id 3DA1D6EA1B
 for <dri-devel@lists.freedesktop.org>; Wed, 23 Apr 2014 01:32:04 -0700 (PDT)
Received: by mail-ee0-f46.google.com with SMTP id t10so468159eei.5
 for <dri-devel@lists.freedesktop.org>; Wed, 23 Apr 2014 01:32:03 -0700 (PDT)
Content-Disposition: inline
In-Reply-To: <20140423072756.GC31226@ulmo>
List-Unsubscribe: <http://lists.freedesktop.org/mailman/options/dri-devel>,
 <mailto:dri-devel-request@lists.freedesktop.org?subject=unsubscribe>
List-Archive: <http://lists.freedesktop.org/archives/dri-devel>
List-Post: <mailto:dri-devel@lists.freedesktop.org>
List-Help: <mailto:dri-devel-request@lists.freedesktop.org?subject=help>
List-Subscribe: <http://lists.freedesktop.org/mailman/listinfo/dri-devel>,
 <mailto:dri-devel-request@lists.freedesktop.org?subject=subscribe>
Errors-To: dri-devel-bounces@lists.freedesktop.org
Sender: "dri-devel" <dri-devel-bounces@lists.freedesktop.org>
To: Thierry Reding <thierry.reding@gmail.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>, Laurent Pinchart <laurent.pinchart@ideasonboard.com>, DRI Development <dri-devel@lists.freedesktop.org>
List-Id: dri-devel@lists.freedesktop.org

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

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