From mboxrd@z Thu Jan 1 00:00:00 1970 From: Thierry Reding Subject: Re: [PATCH 10/18] drm/irq: track the irq installed in drm_irq_install in dev->irq Date: Tue, 22 Apr 2014 12:49:15 +0200 Message-ID: <20140422104913.GC22426@ulmo> References: <1397252175-14227-1-git-send-email-daniel.vetter@ffwll.ch> <1397252175-14227-11-git-send-email-daniel.vetter@ffwll.ch> <1978319.Njcif9FTVM@avalon> <20140422094645.GN10722@phenom.ffwll.local> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============0806465148==" Return-path: Received: from mail-ee0-f47.google.com (mail-ee0-f47.google.com [74.125.83.47]) by gabe.freedesktop.org (Postfix) with ESMTP id 41DD56E856 for ; Tue, 22 Apr 2014 03:50:33 -0700 (PDT) Received: by mail-ee0-f47.google.com with SMTP id b15so4455648eek.20 for ; Tue, 22 Apr 2014 03:50:32 -0700 (PDT) In-Reply-To: <20140422094645.GN10722@phenom.ffwll.local> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Daniel Vetter Cc: Daniel Vetter , Laurent Pinchart , dri-devel@lists.freedesktop.org List-Id: dri-devel@lists.freedesktop.org --===============0806465148== Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="i7F3eY7HS/tUJxUd" Content-Disposition: inline --i7F3eY7HS/tUJxUd Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Tue, Apr 22, 2014 at 11:46:45AM +0200, Daniel Vetter wrote: > On Thu, Apr 17, 2014 at 12:02:07AM +0200, Laurent Pinchart wrote: > > And another comment... > >=20 > > On Friday 11 April 2014 23:36:07 Daniel Vetter wrote: > > > To get rid of the dev->bus->get_irq callback we need to pass in the > > > desired irq explicitly into drm_irq_install. To avoid having to do the > > > same for drm_irq_unistall just track it internally. That leaves > > > drivers with less room to botch things up. > > >=20 > > > v2: Add the hunk lost in an earlier patch to this one (Thierry). > > >=20 > > > Cc: Thierry Reding > > > Signed-off-by: Daniel Vetter > > > --- > > > drivers/gpu/drm/drm_irq.c | 18 +++++++++++------- > > > include/drm/drmP.h | 2 ++ > > > 2 files changed, 13 insertions(+), 7 deletions(-) > > >=20 > > > diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c > > > index 330e85b19115..1c3b6229363d 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_dev= ice > > > *dev) */ > > > int drm_irq_install(struct drm_device *dev) > > > { > > > - int ret; > > > + int ret, irq; > > > unsigned long sh_flags =3D 0; > > > char *irqname; > > >=20 > > > + irq =3D drm_dev_to_irq(dev); > > > + > > > if (!drm_core_check_feature(dev, DRIVER_HAVE_IRQ)) > > > return -EINVAL; > > >=20 > > > - if (drm_dev_to_irq(dev) =3D=3D 0) > > > + if (irq =3D=3D 0) > >=20 > > Isn't 0 a valid IRQ number ? Shouldn't you check for irq < 0 instead ? = At=20 > > least platform_get_irq() returns a negative error value on failure. >=20 > tbh I'm not really clear on how this works. If we want to change this then > I think that should be a separate patch. Also it might be better to > extract this check into drm_control, which is the only function that > really needs it since it is called by userspace. >=20 > For all other places it is a simple driver bug and I'm not sure how much > we should care - request_irq should catch any abuse already. >=20 > In any case this should be a separate patch. I agree. This patch mostly mechanically replaces things and this bug did already exist previously. So let's fix it separately (if at all). Thierry --i7F3eY7HS/tUJxUd Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.22 (GNU/Linux) iQIcBAEBAgAGBQJTVkkpAAoJEN0jrNd/PrOh9LQQAKmRQQ2+AKKZ0gGoqeNU9AKY BUVAiIfKLk8Ws2nM8QEFl4MrobQd2Jve+Crgt9o0HWYSuGVX5VkwcAtY5OXFMYgx 1kSMutuH+RMKaA50sOMHrhG5H4ayRpbbYpOxYVmLIhhn7p6LPiK4D5MQ0T/wALIg oE2+6YvGxhAj5YfO9h5gX5MH1eCpHP9m0R/TSCxWCpG/hr9KWm5Hfi/HPkzwCFjJ X5jkjkuH6Kmo/4k7AX3KitLietRLncOTZxg1K9+UVBjz7DLzbp2qCeXdXdapo7zD nGj74wmJ99DXVOnp18PayfXCxopCwxBMm3EimGI9ruBoQhUu+KDTkZu3SI9Wvb9b ZDQzPbKjrxz7UXZ/LjFEjHYnPMarNuh+8kRHlNrr9ba2luB9LRKErqX76YijvyrY 42k7lJk5ntiNMjRWZMXejxwzLXCp4GckCCHeDvLjQIyzSzX/vlf6A6HDGfuzZP/v CV/s6WvDnrmX+bs/vzqa0DGSj92/CcIqGoWpAYjsi25gZbPadvdsyX7+dycFkUG6 ktvOQkOpiG0sSI4pAIXyg7DNxadRMbqKgHs3I3mP/j+Glv0TdppFQbRk3hN6DZTA lJVjMHOHpCh14dW26s8YhymokAvYijfApu56pYQcRRTtDdfj2uzkOSjLuH1kyI6i fSVhG/ArpHbZsOHEpS1c =wjVs -----END PGP SIGNATURE----- --i7F3eY7HS/tUJxUd-- --===============0806465148== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel --===============0806465148==--