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@xxxxxxxxxxxxxxxx>
> ---
> 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@xxxxxxxxxxxxxxxx>
>
> #define CAL_HL_IRQ_OCPO_ERR_MASK BIT(6)
>
--
Regards,
Laurent Pinchart
