Re: [PATCH v2 dev-5.0 4/4] media: aspeed: refine interrupt handling logic

[Andrew Jeffery]

On Thu, 2 May 2019, at 07:12, Jae Hyun Yoo wrote:
> There are cases that interrupt bits are cleared by a 500ms delayed
> work which causes unnecessary irq calls. Also, the current
> interrupt handler returns IRQ_HANDLED always but it should return
> IRQ_NONE if there is any unhandled interrupt. So this commit
> refines the interrupt handling logic to fix these issues.
> 
> Signed-off-by: Jae Hyun Yoo <jae.hyun.yoo at linux.intel.com>

Reviewed-by: Andrew Jeffery <andrew at aj.id.au>

> ---
> Changes since v1:
>  * Moved bits clearing point to aspeed_video_off().
>  * Made irq handler return IRQ_NONE when there is any remaining bit.
> 
>  drivers/media/platform/aspeed-video.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/media/platform/aspeed-video.c 
> b/drivers/media/platform/aspeed-video.c
> index 77c209a472ca..1bb863b32836 100644
> --- a/drivers/media/platform/aspeed-video.c
> +++ b/drivers/media/platform/aspeed-video.c
> @@ -488,6 +488,7 @@ static void aspeed_video_off(struct aspeed_video 
> *video)
>  
>  	/* Disable interrupts */
>  	aspeed_video_write(video, VE_INTERRUPT_CTRL, 0);
> +	aspeed_video_write(video, VE_INTERRUPT_STATUS, 0xffffffff);
>  
>  	/* Turn off the relevant clocks */
>  	clk_disable(video->vclk);
> @@ -556,7 +557,7 @@ static irqreturn_t aspeed_video_irq(int irq, void *arg)
>  					    VE_INTERRUPT_MODE_DETECT, 0);
>  			aspeed_video_write(video, VE_INTERRUPT_STATUS,
>  					   VE_INTERRUPT_MODE_DETECT);
> -
> +			sts &= ~VE_INTERRUPT_MODE_DETECT;
>  			set_bit(VIDEO_MODE_DETECT_DONE, &video->flags);
>  			wake_up_interruptible_all(&video->wait);
>  		} else {
> @@ -601,12 +602,12 @@ static irqreturn_t aspeed_video_irq(int irq, void *arg)
>  				    VE_INTERRUPT_COMP_COMPLETE, 0);
>  		aspeed_video_write(video, VE_INTERRUPT_STATUS,
>  				   VE_INTERRUPT_COMP_COMPLETE);
> -
> +		sts &= ~VE_INTERRUPT_COMP_COMPLETE;
>  		if (test_bit(VIDEO_STREAMING, &video->flags) && buf)
>  			aspeed_video_start_frame(video);
>  	}
>  
> -	return IRQ_HANDLED;
> +	return sts ? IRQ_NONE : IRQ_HANDLED;
>  }
>  
>  static void aspeed_video_check_and_set_polarity(struct aspeed_video *video)
> -- 
> 2.21.0
> 
>