Re: [PATCH 12/28] media: ti-vpe: cal: rename CAL_HL_IRQ_MASK

[Laurent Pinchart <laurent.pinchart@ideasonboard.com>]

Hi Tomi,

Thank you for the patch.

On Mon, Apr 12, 2021 at 02:34:41PM +0300, Tomi Valkeinen wrote:
> CAL_HL_IRQ_MASK macro is used for both WDMA start and end status bits.
> For clarity, rename CAL_HL_IRQ_MASK macro to CAL_HL_IRQ_WDMA_END_MASK
> and CAL_HL_IRQ_WDMA_START_MASK.
> 
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
> ---
>  drivers/media/platform/ti-vpe/cal.c      | 12 ++++++------
>  drivers/media/platform/ti-vpe/cal_regs.h |  3 ++-
>  2 files changed, 8 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/media/platform/ti-vpe/cal.c b/drivers/media/platform/ti-vpe/cal.c
> index daa0c1ab94e7..0abcc83841c6 100644
> --- a/drivers/media/platform/ti-vpe/cal.c
> +++ b/drivers/media/platform/ti-vpe/cal.c
> @@ -453,9 +453,9 @@ void cal_ctx_start(struct cal_ctx *ctx)
>  
>  	/* Enable IRQ_WDMA_END and IRQ_WDMA_START. */
>  	cal_write(ctx->cal, CAL_HL_IRQENABLE_SET(1),
> -		  CAL_HL_IRQ_MASK(ctx->dma_ctx));
> +		  CAL_HL_IRQ_WDMA_END_MASK(ctx->dma_ctx));
>  	cal_write(ctx->cal, CAL_HL_IRQENABLE_SET(2),
> -		  CAL_HL_IRQ_MASK(ctx->dma_ctx));
> +		  CAL_HL_IRQ_WDMA_START_MASK(ctx->dma_ctx));
>  }
>  
>  void cal_ctx_stop(struct cal_ctx *ctx)
> @@ -479,9 +479,9 @@ void cal_ctx_stop(struct cal_ctx *ctx)
>  
>  	/* Disable IRQ_WDMA_END and IRQ_WDMA_START. */
>  	cal_write(ctx->cal, CAL_HL_IRQENABLE_CLR(1),
> -		  CAL_HL_IRQ_MASK(ctx->dma_ctx));
> +		  CAL_HL_IRQ_WDMA_END_MASK(ctx->dma_ctx));
>  	cal_write(ctx->cal, CAL_HL_IRQENABLE_CLR(2),
> -		  CAL_HL_IRQ_MASK(ctx->dma_ctx));
> +		  CAL_HL_IRQ_WDMA_START_MASK(ctx->dma_ctx));
>  
>  	ctx->dma.state = CAL_DMA_STOPPED;
>  }
> @@ -589,7 +589,7 @@ static irqreturn_t cal_irq(int irq_cal, void *data)
>  		cal_write(cal, CAL_HL_IRQSTATUS(1), status);
>  
>  		for (i = 0; i < ARRAY_SIZE(cal->ctx); ++i) {
> -			if (status & CAL_HL_IRQ_MASK(i))
> +			if (status & CAL_HL_IRQ_WDMA_END_MASK(i))
>  				cal_irq_wdma_end(cal->ctx[i]);
>  		}
>  	}
> @@ -603,7 +603,7 @@ static irqreturn_t cal_irq(int irq_cal, void *data)
>  		cal_write(cal, CAL_HL_IRQSTATUS(2), status);
>  
>  		for (i = 0; i < ARRAY_SIZE(cal->ctx); ++i) {
> -			if (status & CAL_HL_IRQ_MASK(i))
> +			if (status & CAL_HL_IRQ_WDMA_START_MASK(i))
>  				cal_irq_wdma_start(cal->ctx[i]);
>  		}
>  	}
> diff --git a/drivers/media/platform/ti-vpe/cal_regs.h b/drivers/media/platform/ti-vpe/cal_regs.h
> index 5c4f9e642185..93d9bf1f3c00 100644
> --- a/drivers/media/platform/ti-vpe/cal_regs.h
> +++ b/drivers/media/platform/ti-vpe/cal_regs.h
> @@ -125,7 +125,8 @@
>  #define CAL_HL_IRQ_EOI_LINE_NUMBER_READ0		0
>  #define CAL_HL_IRQ_EOI_LINE_NUMBER_EOI0			0
>  
> -#define CAL_HL_IRQ_MASK(m)			BIT(m)
> +#define CAL_HL_IRQ_WDMA_END_MASK(m)		BIT(m)
> +#define CAL_HL_IRQ_WDMA_START_MASK(m)		BIT(m)

I foresee a risk of mixing the two macros, using
CAL_HL_IRQ_WDMA_START_MASK with IRQ(1) or CAL_HL_IRQ_WDMA_END_MASK with
IRQ(2). Not sure how we could prevent that though, and it won't make any
functional difference (but could lead to other bugs), so

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

>  
>  #define CAL_HL_IRQ_OCPO_ERR_MASK		BIT(6)
>  

-- 
Regards,

Laurent Pinchart