Re: [PATCH 4/4] media: dw100: Split interrupt handler to fix timeout error

[Nicolas Dufresne]

Hi,

Le lundi 05 janvier 2026 à 12:35 +0100, Stefan Klug a écrit :
> In the previous commit, the interrupt handler was changed to threaded.
> This sometimes leads to DW100_INTERRUPT_STATUS_INT_ERR_TIME_OUT being
> set after changing the vertex map. This can be seen by repeated error
> outputs in dmesg:
> 
> dw100 32e30000.dwe: Interrupt error: 0x1
> 
> As there is no documentation available, it is unclear why that happens
> and if this condition can simply be ignored. By splitting the interrupt
> handling into two parts and only handling the dw100_job_finish() within
> the threaded part, the error does not occur anymore.
> 
> Signed-off-by: Stefan Klug <stefan.klug@xxxxxxxxxxxxxxxx>

Ok, but arguably, this could be squashed.

Nicolas

> 
> ---
> 
> As noted on the cover letter, this patch still is intended to start the
> discussion for a proper fix.
> 
> While writing this I noted that when
> DW100_INTERRUPT_STATUS_INT_FRAME_DONE is set, the job gets finished
> without error even when err_irqs != 0. Is that on purpose?
> ---
>  drivers/media/platform/nxp/dw100/dw100.c | 20 +++++++++++++++-----
>  1 file changed, 15 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/media/platform/nxp/dw100/dw100.c
> b/drivers/media/platform/nxp/dw100/dw100.c
> index
> 4f5ef70e5f4a052fb5f208e35f8785f9d30dc54e..67d941bdf768398edc611c94896cc42a70b8
> 8225 100644
> --- a/drivers/media/platform/nxp/dw100/dw100.c
> +++ b/drivers/media/platform/nxp/dw100/dw100.c
> @@ -10,6 +10,7 @@
>  #include <linux/clk.h>
>  #include <linux/debugfs.h>
>  #include <linux/interrupt.h>
> +#include <linux/irqreturn.h>
>  #include <linux/io.h>
>  #include <linux/minmax.h>
>  #include <linux/module.h>
> @@ -74,6 +75,7 @@ struct dw100_device {
>  	struct clk_bulk_data		*clks;
>  	int				num_clks;
>  	struct dentry			*debugfs_root;
> +	bool				frame_failed;
>  };
>  
>  struct dw100_q_data {
> @@ -1411,7 +1413,8 @@ static irqreturn_t dw100_irq_handler(int irq, void
> *dev_id)
>  {
>  	struct dw100_device *dw_dev = dev_id;
>  	u32 pending_irqs, err_irqs, frame_done_irq;
> -	bool with_error = true;
> +
> +	dw_dev->frame_failed = true;
>  
>  	pending_irqs = dw_hw_get_pending_irqs(dw_dev);
>  	frame_done_irq = pending_irqs &
> DW100_INTERRUPT_STATUS_INT_FRAME_DONE;
> @@ -1419,7 +1422,7 @@ static irqreturn_t dw100_irq_handler(int irq, void
> *dev_id)
>  
>  	if (frame_done_irq) {
>  		dev_dbg(&dw_dev->pdev->dev, "Frame done interrupt\n");
> -		with_error = false;
> +		dw_dev->frame_failed = false;
>  		err_irqs &= ~DW100_INTERRUPT_STATUS_INT_ERR_STATUS
>  			(DW100_INTERRUPT_STATUS_INT_ERR_FRAME_DONE);
>  	}
> @@ -1432,7 +1435,14 @@ static irqreturn_t dw100_irq_handler(int irq, void
> *dev_id)
>  	dw100_hw_clear_irq(dw_dev, pending_irqs |
>  			   DW100_INTERRUPT_STATUS_INT_ERR_TIME_OUT);
>  
> -	dw100_job_finish(dw_dev, with_error);
> +	return IRQ_WAKE_THREAD;
> +}
> +
> +static irqreturn_t dw100_irq_thread_fn(int irq, void *dev_id)
> +{
> +	struct dw100_device *dw_dev = dev_id;
> +
> +	dw100_job_finish(dw_dev, dw_dev->frame_failed);
>  
>  	return IRQ_HANDLED;
>  }
> @@ -1600,8 +1610,8 @@ static int dw100_probe(struct platform_device *pdev)
>  
>  	pm_runtime_put_sync(&pdev->dev);
>  
> -	ret = devm_request_threaded_irq(&pdev->dev, irq, NULL,
> -					dw100_irq_handler, IRQF_ONESHOT,
> +	ret = devm_request_threaded_irq(&pdev->dev, irq, dw100_irq_handler,
> +					dw100_irq_thread_fn, IRQF_ONESHOT,
>  					dev_name(&pdev->dev), dw_dev);
>  	if (ret < 0) {
>  		dev_err(&pdev->dev, "Failed to request irq: %d\n", ret);

Attachment: signature.asc
Description: This is a digitally signed message part