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@gmail.com Signed-off-by: Daniel Vetter daniel.vetter@ffwll.ch
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