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...
> >
> > 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.
> > >
> > > v2: Add the hunk lost in an earlier patch to this one (Thierry).
> > >
> > > Cc: Thierry Reding <thierry.reding@xxxxxxxxx>
> > > Signed-off-by: Daniel Vetter <daniel.vetter@xxxxxxxx>
> > > ---
> > > drivers/gpu/drm/drm_irq.c | 18 +++++++++++-------
> > > include/drm/drmP.h | 2 ++
> > > 2 files changed, 13 insertions(+), 7 deletions(-)
> > >
> > > 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_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);
> > > +
> > > if (!drm_core_check_feature(dev, DRIVER_HAVE_IRQ))
> > > return -EINVAL;
> > >
> > > - if (drm_dev_to_irq(dev) == 0)
> > > + if (irq == 0)
> >
> > Isn't 0 a valid IRQ number ? Shouldn't you check for irq < 0 instead ? At
> > least platform_get_irq() returns a negative error value on failure.
>
> 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.
>
> 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.
>
> 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
Attachment:
pgpLiLNljLDyi.pgp
Description: PGP signature
_______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/dri-devel
