linux-kernel - Re: [PATCH 2/2] drm: mali-dp: Set the drm->irq

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20160802133058.GM6232@phenom.ffwll.local>
Date:	Tue, 2 Aug 2016 15:30:58 +0200
From:	Daniel Vetter <daniel@...ll.ch>
To:	Liviu Dudau <Liviu.Dudau@....com>
Cc:	David Airlie <airlied@...ux.ie>,
	Mali DP Maintainers <malidp@...s.arm.com>,
	Daniel Vetter <daniel.vetter@...ll.ch>,
	DRI devel <dri-devel@...ts.freedesktop.org>,
	LKML <linux-kernel@...r.kernel.org>,
	Brian Starkey <Brian.Starkey@....com>
Subject: Re: [PATCH 2/2] drm: mali-dp: Set the drm->irq_enabled flag to match
 driver's state.

On Fri, Jul 29, 2016 at 03:44:16PM +0100, Liviu Dudau wrote:
> Mali DP driver does not use drm_irq_{un,}install() function so the
> drm->irq_enabled flag does not get managed automatically. Among other
> DRM framework functions, drm_wait_vblank() checks the value of the
> flag and return -EINVAL if not set.
> 
> Cc: Brian Starkey <Brian.Starkey@....com>
> Signed-off-by: Liviu Dudau <Liviu.Dudau@....com>
> ---
>  drivers/gpu/drm/arm/malidp_drv.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/gpu/drm/arm/malidp_drv.c b/drivers/gpu/drm/arm/malidp_drv.c
> index 87c9249..a1c01e4 100644
> --- a/drivers/gpu/drm/arm/malidp_drv.c
> +++ b/drivers/gpu/drm/arm/malidp_drv.c
> @@ -377,6 +377,8 @@ static int malidp_bind(struct device *dev)
>  	if (ret < 0)
>  		goto irq_init_fail;
>  
> +	drm->irq_enabled = true;

I wonder whether we should have a cocci script or something with catching
some of these issues that every new driver seems to hit. Another one is
filling out legacy helpers for all the legacy entry points using atomic
functions. Or still using ->load/unload. Documentation is great, but it
seems like most folks don't read it until they hit a snag ;-)

Just idle musings ...
-Daniel

> +
>  	ret = drm_vblank_init(drm, drm->mode_config.num_crtc);
>  	if (ret < 0) {
>  		DRM_ERROR("failed to initialise vblank\n");
> @@ -402,6 +404,7 @@ fbdev_fail:
>  vblank_fail:
>  	malidp_se_irq_fini(drm);
>  	malidp_de_irq_fini(drm);
> +	drm->irq_enabled = false;
>  irq_init_fail:
>  	component_unbind_all(dev, drm);
>  bind_fail:
> @@ -439,6 +442,7 @@ static void malidp_unbind(struct device *dev)
>  	drm_kms_helper_poll_fini(drm);
>  	malidp_se_irq_fini(drm);
>  	malidp_de_irq_fini(drm);
> +	drm->irq_enabled = false;
>  	drm_vblank_cleanup(drm);
>  	component_unbind_all(dev, drm);
>  	of_node_put(malidp->crtc.port);
> -- 
> 2.9.0
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch