diff mbox series

[5/6] media: ti: cal: combine wdma irq handling

Message ID 20220421143449.552312-6-tomi.valkeinen@ideasonboard.com
State Superseded
Headers show
Series media: ti: cal: misc fixes | expand

Commit Message

Tomi Valkeinen April 21, 2022, 2:34 p.m. UTC
Instead of handling the WDMA START and END interrupts separately, we
need to handle both at the same time to better manage the inherent race
conditions related to CAL interrupts.

Change the code so that we have a single function,
cal_irq_handle_wdma(), which gets two booleans, start and end, as
parameters, which allows us to manage the race conditions in the
following patch.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
---
 drivers/media/platform/ti/cal/cal.c | 59 ++++++++++++-----------------
 1 file changed, 25 insertions(+), 34 deletions(-)

Comments

Laurent Pinchart April 21, 2022, 11:54 p.m. UTC | #1
Hi Tomi,

Thank you for the patch.

On Thu, Apr 21, 2022 at 05:34:48PM +0300, Tomi Valkeinen wrote:
> Instead of handling the WDMA START and END interrupts separately, we
> need to handle both at the same time to better manage the inherent race
> conditions related to CAL interrupts.
> 
> Change the code so that we have a single function,
> cal_irq_handle_wdma(), which gets two booleans, start and end, as
> parameters, which allows us to manage the race conditions in the
> following patch.
> 
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>

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

> ---
>  drivers/media/platform/ti/cal/cal.c | 59 ++++++++++++-----------------
>  1 file changed, 25 insertions(+), 34 deletions(-)
> 
> diff --git a/drivers/media/platform/ti/cal/cal.c b/drivers/media/platform/ti/cal/cal.c
> index 783ce5a8cd79..e4355f266c58 100644
> --- a/drivers/media/platform/ti/cal/cal.c
> +++ b/drivers/media/platform/ti/cal/cal.c
> @@ -668,22 +668,33 @@ static inline void cal_irq_wdma_end(struct cal_ctx *ctx)
>  	}
>  }
>  
> +static void cal_irq_handle_wdma(struct cal_ctx *ctx, bool start, bool end)
> +{
> +	if (end)
> +		cal_irq_wdma_end(ctx);
> +
> +	if (start)
> +		cal_irq_wdma_start(ctx);
> +}
> +
>  static irqreturn_t cal_irq(int irq_cal, void *data)
>  {
>  	struct cal_dev *cal = data;
> -	u32 status;
> -
> -	status = cal_read(cal, CAL_HL_IRQSTATUS(0));
> -	if (status) {
> -		unsigned int i;
> +	u32 status[3];
> +	unsigned int i;
>  
> -		cal_write(cal, CAL_HL_IRQSTATUS(0), status);
> +	for (i = 0; i < 3; ++i) {
> +		status[i] = cal_read(cal, CAL_HL_IRQSTATUS(i));
> +		if (status[i])
> +			cal_write(cal, CAL_HL_IRQSTATUS(i), status[i]);
> +	}
>  
> -		if (status & CAL_HL_IRQ_OCPO_ERR_MASK)
> +	if (status[0]) {
> +		if (status[0] & CAL_HL_IRQ_OCPO_ERR_MASK)
>  			dev_err_ratelimited(cal->dev, "OCPO ERROR\n");
>  
>  		for (i = 0; i < cal->data->num_csi2_phy; ++i) {
> -			if (status & CAL_HL_IRQ_CIO_MASK(i)) {
> +			if (status[0] & CAL_HL_IRQ_CIO_MASK(i)) {
>  				u32 cio_stat = cal_read(cal,
>  							CAL_CSI2_COMPLEXIO_IRQSTATUS(i));
>  
> @@ -694,7 +705,7 @@ static irqreturn_t cal_irq(int irq_cal, void *data)
>  					  cio_stat);
>  			}
>  
> -			if (status & CAL_HL_IRQ_VC_MASK(i)) {
> +			if (status[0] & CAL_HL_IRQ_VC_MASK(i)) {
>  				u32 vc_stat = cal_read(cal, CAL_CSI2_VC_IRQSTATUS(i));
>  
>  				dev_err_ratelimited(cal->dev,
> @@ -706,32 +717,12 @@ static irqreturn_t cal_irq(int irq_cal, void *data)
>  		}
>  	}
>  
> -	/* Check which DMA just finished */
> -	status = cal_read(cal, CAL_HL_IRQSTATUS(1));
> -	if (status) {
> -		unsigned int i;
> -
> -		/* Clear Interrupt status */
> -		cal_write(cal, CAL_HL_IRQSTATUS(1), status);
> -
> -		for (i = 0; i < cal->num_contexts; ++i) {
> -			if (status & CAL_HL_IRQ_WDMA_END_MASK(i))
> -				cal_irq_wdma_end(cal->ctx[i]);
> -		}
> -	}
> -
> -	/* Check which DMA just started */
> -	status = cal_read(cal, CAL_HL_IRQSTATUS(2));
> -	if (status) {
> -		unsigned int i;
> -
> -		/* Clear Interrupt status */
> -		cal_write(cal, CAL_HL_IRQSTATUS(2), status);
> +	for (i = 0; i < cal->num_contexts; ++i) {
> +		bool end = !!(status[1] & CAL_HL_IRQ_WDMA_END_MASK(i));
> +		bool start = !!(status[2] & CAL_HL_IRQ_WDMA_START_MASK(i));
>  
> -		for (i = 0; i < cal->num_contexts; ++i) {
> -			if (status & CAL_HL_IRQ_WDMA_START_MASK(i))
> -				cal_irq_wdma_start(cal->ctx[i]);
> -		}
> +		if (start || end)
> +			cal_irq_handle_wdma(cal->ctx[i], start, end);
>  	}
>  
>  	return IRQ_HANDLED;
diff mbox series

Patch

diff --git a/drivers/media/platform/ti/cal/cal.c b/drivers/media/platform/ti/cal/cal.c
index 783ce5a8cd79..e4355f266c58 100644
--- a/drivers/media/platform/ti/cal/cal.c
+++ b/drivers/media/platform/ti/cal/cal.c
@@ -668,22 +668,33 @@  static inline void cal_irq_wdma_end(struct cal_ctx *ctx)
 	}
 }
 
+static void cal_irq_handle_wdma(struct cal_ctx *ctx, bool start, bool end)
+{
+	if (end)
+		cal_irq_wdma_end(ctx);
+
+	if (start)
+		cal_irq_wdma_start(ctx);
+}
+
 static irqreturn_t cal_irq(int irq_cal, void *data)
 {
 	struct cal_dev *cal = data;
-	u32 status;
-
-	status = cal_read(cal, CAL_HL_IRQSTATUS(0));
-	if (status) {
-		unsigned int i;
+	u32 status[3];
+	unsigned int i;
 
-		cal_write(cal, CAL_HL_IRQSTATUS(0), status);
+	for (i = 0; i < 3; ++i) {
+		status[i] = cal_read(cal, CAL_HL_IRQSTATUS(i));
+		if (status[i])
+			cal_write(cal, CAL_HL_IRQSTATUS(i), status[i]);
+	}
 
-		if (status & CAL_HL_IRQ_OCPO_ERR_MASK)
+	if (status[0]) {
+		if (status[0] & CAL_HL_IRQ_OCPO_ERR_MASK)
 			dev_err_ratelimited(cal->dev, "OCPO ERROR\n");
 
 		for (i = 0; i < cal->data->num_csi2_phy; ++i) {
-			if (status & CAL_HL_IRQ_CIO_MASK(i)) {
+			if (status[0] & CAL_HL_IRQ_CIO_MASK(i)) {
 				u32 cio_stat = cal_read(cal,
 							CAL_CSI2_COMPLEXIO_IRQSTATUS(i));
 
@@ -694,7 +705,7 @@  static irqreturn_t cal_irq(int irq_cal, void *data)
 					  cio_stat);
 			}
 
-			if (status & CAL_HL_IRQ_VC_MASK(i)) {
+			if (status[0] & CAL_HL_IRQ_VC_MASK(i)) {
 				u32 vc_stat = cal_read(cal, CAL_CSI2_VC_IRQSTATUS(i));
 
 				dev_err_ratelimited(cal->dev,
@@ -706,32 +717,12 @@  static irqreturn_t cal_irq(int irq_cal, void *data)
 		}
 	}
 
-	/* Check which DMA just finished */
-	status = cal_read(cal, CAL_HL_IRQSTATUS(1));
-	if (status) {
-		unsigned int i;
-
-		/* Clear Interrupt status */
-		cal_write(cal, CAL_HL_IRQSTATUS(1), status);
-
-		for (i = 0; i < cal->num_contexts; ++i) {
-			if (status & CAL_HL_IRQ_WDMA_END_MASK(i))
-				cal_irq_wdma_end(cal->ctx[i]);
-		}
-	}
-
-	/* Check which DMA just started */
-	status = cal_read(cal, CAL_HL_IRQSTATUS(2));
-	if (status) {
-		unsigned int i;
-
-		/* Clear Interrupt status */
-		cal_write(cal, CAL_HL_IRQSTATUS(2), status);
+	for (i = 0; i < cal->num_contexts; ++i) {
+		bool end = !!(status[1] & CAL_HL_IRQ_WDMA_END_MASK(i));
+		bool start = !!(status[2] & CAL_HL_IRQ_WDMA_START_MASK(i));
 
-		for (i = 0; i < cal->num_contexts; ++i) {
-			if (status & CAL_HL_IRQ_WDMA_START_MASK(i))
-				cal_irq_wdma_start(cal->ctx[i]);
-		}
+		if (start || end)
+			cal_irq_handle_wdma(cal->ctx[i], start, end);
 	}
 
 	return IRQ_HANDLED;