[PATCH 0/8] media: use 'time_left' instead of 'timeout' with wait_*() functions

[PATCH 0/8] media: use 'time_left' instead of 'timeout' with wait_*() functions

[Wolfram Sang]

There is a confusing pattern in the kernel to use a variable named 'timeout' to
store the result of wait_*() functions causing patterns like:

        timeout = wait_for_completion_timeout(...)
        if (!timeout) return -ETIMEDOUT;

with all kinds of permutations. Use 'time_left' as a variable to make the code
obvious and self explaining.

This is part of a tree-wide series. The rest of the patches can be found here:

git://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux.git i2c/time_left

Because these patches are generated, I audit them before sending. This is why I
will send series step by step. Build bot is happy with these patches, though.
No functional changes intended.

Wolfram Sang (8):
  media: allegro: use 'time_left' variable with
    wait_for_completion_timeout()
  media: atmel-isi: use 'time_left' variable with
    wait_for_completion_timeout()
  media: bdisp: use 'time_left' variable with wait_event_timeout()
  media: fimc-is: use 'time_left' variable with wait_event_timeout()
  media: platform: exynos-gsc: use 'time_left' variable with
    wait_event_timeout()
  media: solo6x10: use 'time_left' variable with
    wait_for_completion_timeout()
  media: tegra-vde: use 'time_left' variable with
    wait_for_completion_interruptible_timeout()
  media: ti: cal: use 'time_left' variable with wait_event_timeout()

 drivers/media/pci/solo6x10/solo6x10-p2m.c        |  8 ++++----
 .../media/platform/allegro-dvt/allegro-core.c    | 16 ++++++++--------
 drivers/media/platform/atmel/atmel-isi.c         |  8 ++++----
 drivers/media/platform/nvidia/tegra-vde/h264.c   | 10 +++++-----
 .../media/platform/samsung/exynos-gsc/gsc-core.c | 10 +++++-----
 .../platform/samsung/exynos4-is/fimc-core.c      | 10 +++++-----
 drivers/media/platform/st/sti/bdisp/bdisp-v4l2.c | 10 +++++-----
 drivers/media/platform/ti/cal/cal.c              |  8 ++++----
 8 files changed, 40 insertions(+), 40 deletions(-)

-- 
2.43.0

[PATCH 1/8] media: allegro: use 'time_left' variable with wait_for_completion_timeout()

[Wolfram Sang]

There is a confusing pattern in the kernel to use a variable named 'timeout' to
store the result of wait_for_completion_timeout() causing patterns like:

	timeout = wait_for_completion_timeout(...)
	if (!timeout) return -ETIMEDOUT;

with all kinds of permutations. Use 'time_left' as a variable to make the code
self explaining.

Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
 .../media/platform/allegro-dvt/allegro-core.c    | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/media/platform/allegro-dvt/allegro-core.c b/drivers/media/platform/allegro-dvt/allegro-core.c
index da61f9beb6b4..4016aef5fdb9 100644
--- a/drivers/media/platform/allegro-dvt/allegro-core.c
+++ b/drivers/media/platform/allegro-dvt/allegro-core.c
@@ -2481,14 +2481,14 @@ static void allegro_mcu_interrupt(struct allegro_dev *dev)
 static void allegro_destroy_channel(struct allegro_channel *channel)
 {
 	struct allegro_dev *dev = channel->dev;
-	unsigned long timeout;
+	unsigned long time_left;
 
 	if (channel_exists(channel)) {
 		reinit_completion(&channel->completion);
 		allegro_mcu_send_destroy_channel(dev, channel);
-		timeout = wait_for_completion_timeout(&channel->completion,
-						      msecs_to_jiffies(5000));
-		if (timeout == 0)
+		time_left = wait_for_completion_timeout(&channel->completion,
+							msecs_to_jiffies(5000));
+		if (time_left == 0)
 			v4l2_warn(&dev->v4l2_dev,
 				  "channel %d: timeout while destroying\n",
 				  channel->mcu_channel_id);
@@ -2544,7 +2544,7 @@ static void allegro_destroy_channel(struct allegro_channel *channel)
 static int allegro_create_channel(struct allegro_channel *channel)
 {
 	struct allegro_dev *dev = channel->dev;
-	unsigned long timeout;
+	unsigned long time_left;
 
 	if (channel_exists(channel)) {
 		v4l2_warn(&dev->v4l2_dev,
@@ -2595,9 +2595,9 @@ static int allegro_create_channel(struct allegro_channel *channel)
 
 	reinit_completion(&channel->completion);
 	allegro_mcu_send_create_channel(dev, channel);
-	timeout = wait_for_completion_timeout(&channel->completion,
-					      msecs_to_jiffies(5000));
-	if (timeout == 0)
+	time_left = wait_for_completion_timeout(&channel->completion,
+						msecs_to_jiffies(5000));
+	if (time_left == 0)
 		channel->error = -ETIMEDOUT;
 	if (channel->error)
 		goto err;
-- 
2.43.0

[PATCH 2/8] media: atmel-isi: use 'time_left' variable with wait_for_completion_timeout()

[Wolfram Sang]

There is a confusing pattern in the kernel to use a variable named 'timeout' to
store the result of wait_for_completion_timeout() causing patterns like:

	timeout = wait_for_completion_timeout(...)
	if (!timeout) return -ETIMEDOUT;

with all kinds of permutations. Use 'time_left' as a variable to make the code
self explaining.

Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
 drivers/media/platform/atmel/atmel-isi.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/media/platform/atmel/atmel-isi.c b/drivers/media/platform/atmel/atmel-isi.c
index c1108df72dd5..5c823d3f9cc0 100644
--- a/drivers/media/platform/atmel/atmel-isi.c
+++ b/drivers/media/platform/atmel/atmel-isi.c
@@ -242,7 +242,7 @@ static irqreturn_t isi_interrupt(int irq, void *dev_id)
 #define	WAIT_ISI_DISABLE	0
 static int atmel_isi_wait_status(struct atmel_isi *isi, int wait_reset)
 {
-	unsigned long timeout;
+	unsigned long time_left;
 	/*
 	 * The reset or disable will only succeed if we have a
 	 * pixel clock from the camera.
@@ -257,9 +257,9 @@ static int atmel_isi_wait_status(struct atmel_isi *isi, int wait_reset)
 		isi_writel(isi, ISI_CTRL, ISI_CTRL_DIS);
 	}
 
-	timeout = wait_for_completion_timeout(&isi->complete,
-			msecs_to_jiffies(500));
-	if (timeout == 0)
+	time_left = wait_for_completion_timeout(&isi->complete,
+						msecs_to_jiffies(500));
+	if (time_left == 0)
 		return -ETIMEDOUT;
 
 	return 0;
-- 
2.43.0

[PATCH 3/8] media: bdisp: use 'time_left' variable with wait_event_timeout()

[Wolfram Sang]

There is a confusing pattern in the kernel to use a variable named 'timeout' to
store the result of wait_event_timeout() causing patterns like:

	timeout = wait_event_timeout(...)
	if (!timeout) return -ETIMEDOUT;

with all kinds of permutations. Use 'time_left' as a variable to make the code
self explaining.

Fix to the proper variable type 'long' while here.

Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
 drivers/media/platform/st/sti/bdisp/bdisp-v4l2.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/media/platform/st/sti/bdisp/bdisp-v4l2.c b/drivers/media/platform/st/sti/bdisp/bdisp-v4l2.c
index 1328b4eb6b9f..c7ee6e1a4451 100644
--- a/drivers/media/platform/st/sti/bdisp/bdisp-v4l2.c
+++ b/drivers/media/platform/st/sti/bdisp/bdisp-v4l2.c
@@ -1160,7 +1160,7 @@ static void bdisp_irq_timeout(struct work_struct *ptr)
 static int bdisp_m2m_suspend(struct bdisp_dev *bdisp)
 {
 	unsigned long flags;
-	int timeout;
+	long time_left;
 
 	spin_lock_irqsave(&bdisp->slock, flags);
 	if (!test_bit(ST_M2M_RUNNING, &bdisp->state)) {
@@ -1171,13 +1171,13 @@ static int bdisp_m2m_suspend(struct bdisp_dev *bdisp)
 	set_bit(ST_M2M_SUSPENDING, &bdisp->state);
 	spin_unlock_irqrestore(&bdisp->slock, flags);
 
-	timeout = wait_event_timeout(bdisp->irq_queue,
-				     test_bit(ST_M2M_SUSPENDED, &bdisp->state),
-				     BDISP_WORK_TIMEOUT);
+	time_left = wait_event_timeout(bdisp->irq_queue,
+				       test_bit(ST_M2M_SUSPENDED, &bdisp->state),
+				       BDISP_WORK_TIMEOUT);
 
 	clear_bit(ST_M2M_SUSPENDING, &bdisp->state);
 
-	if (!timeout) {
+	if (!time_left) {
 		dev_err(bdisp->dev, "%s IRQ timeout\n", __func__);
 		return -EAGAIN;
 	}
-- 
2.43.0

[PATCH 4/8] media: fimc-is: use 'time_left' variable with wait_event_timeout()

[Wolfram Sang]

There is a confusing pattern in the kernel to use a variable named 'timeout' to
store the result of wait_event_timeout() causing patterns like:

	timeout = wait_event_timeout(...)
	if (!timeout) return -ETIMEDOUT;

with all kinds of permutations. Use 'time_left' as a variable to make the code
self explaining.

Fix to the proper variable type 'long' while here.

Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
 drivers/media/platform/samsung/exynos4-is/fimc-core.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/media/platform/samsung/exynos4-is/fimc-core.c b/drivers/media/platform/samsung/exynos4-is/fimc-core.c
index aae74b501a42..adfc2d73d04b 100644
--- a/drivers/media/platform/samsung/exynos4-is/fimc-core.c
+++ b/drivers/media/platform/samsung/exynos4-is/fimc-core.c
@@ -822,7 +822,7 @@ static int fimc_clk_get(struct fimc_dev *fimc)
 static int fimc_m2m_suspend(struct fimc_dev *fimc)
 {
 	unsigned long flags;
-	int timeout;
+	long time_left;
 
 	spin_lock_irqsave(&fimc->slock, flags);
 	if (!fimc_m2m_pending(fimc)) {
@@ -833,12 +833,12 @@ static int fimc_m2m_suspend(struct fimc_dev *fimc)
 	set_bit(ST_M2M_SUSPENDING, &fimc->state);
 	spin_unlock_irqrestore(&fimc->slock, flags);
 
-	timeout = wait_event_timeout(fimc->irq_queue,
-			     test_bit(ST_M2M_SUSPENDED, &fimc->state),
-			     FIMC_SHUTDOWN_TIMEOUT);
+	time_left = wait_event_timeout(fimc->irq_queue,
+				       test_bit(ST_M2M_SUSPENDED, &fimc->state),
+				       FIMC_SHUTDOWN_TIMEOUT);
 
 	clear_bit(ST_M2M_SUSPENDING, &fimc->state);
-	return timeout == 0 ? -EAGAIN : 0;
+	return time_left == 0 ? -EAGAIN : 0;
 }
 
 static int fimc_m2m_resume(struct fimc_dev *fimc)
-- 
2.43.0

[PATCH 5/8] media: platform: exynos-gsc: use 'time_left' variable with wait_event_timeout()

[Wolfram Sang]

There is a confusing pattern in the kernel to use a variable named 'timeout' to
store the result of wait_event_timeout() causing patterns like:

	timeout = wait_event_timeout(...)
	if (!timeout) return -ETIMEDOUT;

with all kinds of permutations. Use 'time_left' as a variable to make the code
self explaining.

Fix to the proper variable type 'long' while here.

Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
 drivers/media/platform/samsung/exynos-gsc/gsc-core.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/media/platform/samsung/exynos-gsc/gsc-core.c b/drivers/media/platform/samsung/exynos-gsc/gsc-core.c
index 618ae55fe396..f45f5c8612a6 100644
--- a/drivers/media/platform/samsung/exynos-gsc/gsc-core.c
+++ b/drivers/media/platform/samsung/exynos-gsc/gsc-core.c
@@ -1225,7 +1225,7 @@ static void gsc_remove(struct platform_device *pdev)
 static int gsc_m2m_suspend(struct gsc_dev *gsc)
 {
 	unsigned long flags;
-	int timeout;
+	long time_left;
 
 	spin_lock_irqsave(&gsc->slock, flags);
 	if (!gsc_m2m_pending(gsc)) {
@@ -1236,12 +1236,12 @@ static int gsc_m2m_suspend(struct gsc_dev *gsc)
 	set_bit(ST_M2M_SUSPENDING, &gsc->state);
 	spin_unlock_irqrestore(&gsc->slock, flags);
 
-	timeout = wait_event_timeout(gsc->irq_queue,
-			     test_bit(ST_M2M_SUSPENDED, &gsc->state),
-			     GSC_SHUTDOWN_TIMEOUT);
+	time_left = wait_event_timeout(gsc->irq_queue,
+				       test_bit(ST_M2M_SUSPENDED, &gsc->state),
+				       GSC_SHUTDOWN_TIMEOUT);
 
 	clear_bit(ST_M2M_SUSPENDING, &gsc->state);
-	return timeout == 0 ? -EAGAIN : 0;
+	return time_left == 0 ? -EAGAIN : 0;
 }
 
 static void gsc_m2m_resume(struct gsc_dev *gsc)
-- 
2.43.0

[PATCH 6/8] media: solo6x10: use 'time_left' variable with wait_for_completion_timeout()

[Wolfram Sang]

There is a confusing pattern in the kernel to use a variable named 'timeout' to
store the result of wait_for_completion_timeout() causing patterns like:

	timeout = wait_for_completion_timeout(...)
	if (!timeout) return -ETIMEDOUT;

with all kinds of permutations. Use 'time_left' as a variable to make the code
self explaining.

Fix to the proper variable type 'unsigned long' while here.

Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
 drivers/media/pci/solo6x10/solo6x10-p2m.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/media/pci/solo6x10/solo6x10-p2m.c b/drivers/media/pci/solo6x10/solo6x10-p2m.c
index ca70a864a3ef..5f100e5e03d9 100644
--- a/drivers/media/pci/solo6x10/solo6x10-p2m.c
+++ b/drivers/media/pci/solo6x10/solo6x10-p2m.c
@@ -57,7 +57,7 @@ int solo_p2m_dma_desc(struct solo_dev *solo_dev,
 		      int desc_cnt)
 {
 	struct solo_p2m_dev *p2m_dev;
-	unsigned int timeout;
+	unsigned long time_left;
 	unsigned int config = 0;
 	int ret = 0;
 	unsigned int p2m_id = 0;
@@ -99,12 +99,12 @@ int solo_p2m_dma_desc(struct solo_dev *solo_dev,
 			       desc[1].ctrl);
 	}
 
-	timeout = wait_for_completion_timeout(&p2m_dev->completion,
-					      solo_dev->p2m_jiffies);
+	time_left = wait_for_completion_timeout(&p2m_dev->completion,
+						solo_dev->p2m_jiffies);
 
 	if (WARN_ON_ONCE(p2m_dev->error))
 		ret = -EIO;
-	else if (timeout == 0) {
+	else if (time_left == 0) {
 		solo_dev->p2m_timeouts++;
 		ret = -EAGAIN;
 	}
-- 
2.43.0

[PATCH 7/8] media: tegra-vde: use 'time_left' variable with wait_for_completion_interruptible_timeout()

[Wolfram Sang]

There is a confusing pattern in the kernel to use a variable named 'timeout' to
store the result of wait_for_completion_interruptible_timeout() causing patterns like:

	timeout = wait_for_completion_interruptible_timeout(...)
	if (!timeout) return -ETIMEDOUT;

with all kinds of permutations. Use 'time_left' as a variable to make the code
self explaining.

Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
 drivers/media/platform/nvidia/tegra-vde/h264.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/media/platform/nvidia/tegra-vde/h264.c b/drivers/media/platform/nvidia/tegra-vde/h264.c
index cfea5572a1b8..d20b7b9fb79c 100644
--- a/drivers/media/platform/nvidia/tegra-vde/h264.c
+++ b/drivers/media/platform/nvidia/tegra-vde/h264.c
@@ -628,14 +628,14 @@ static int tegra_vde_decode_end(struct tegra_vde *vde)
 	unsigned int read_bytes, macroblocks_nb;
 	struct device *dev = vde->dev;
 	dma_addr_t bsev_ptr;
-	long timeout;
+	long time_left;
 	int ret;
 
-	timeout = wait_for_completion_interruptible_timeout(
+	time_left = wait_for_completion_interruptible_timeout(
 			&vde->decode_completion, msecs_to_jiffies(1000));
-	if (timeout < 0) {
-		ret = timeout;
-	} else if (timeout == 0) {
+	if (time_left < 0) {
+		ret = time_left;
+	} else if (time_left == 0) {
 		bsev_ptr = tegra_vde_readl(vde, vde->bsev, 0x10);
 		macroblocks_nb = tegra_vde_readl(vde, vde->sxe, 0xC8) & 0x1FFF;
 		read_bytes = bsev_ptr ? bsev_ptr - vde->bitstream_data_addr : 0;
-- 
2.43.0

[PATCH 8/8] media: ti: cal: use 'time_left' variable with wait_event_timeout()

[Wolfram Sang]

There is a confusing pattern in the kernel to use a variable named 'timeout' to
store the result of wait_event_timeout() causing patterns like:

	timeout = wait_event_timeout(...)
	if (!timeout) return -ETIMEDOUT;

with all kinds of permutations. Use 'time_left' as a variable to make the code
self explaining.

Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
 drivers/media/platform/ti/cal/cal.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/media/platform/ti/cal/cal.c b/drivers/media/platform/ti/cal/cal.c
index 528909ae4bd6..5c2c04142aee 100644
--- a/drivers/media/platform/ti/cal/cal.c
+++ b/drivers/media/platform/ti/cal/cal.c
@@ -549,7 +549,7 @@ void cal_ctx_start(struct cal_ctx *ctx)
 void cal_ctx_stop(struct cal_ctx *ctx)
 {
 	struct cal_camerarx *phy = ctx->phy;
-	long timeout;
+	long time_left;
 
 	WARN_ON(phy->vc_enable_count[ctx->vc] == 0);
 
@@ -565,9 +565,9 @@ void cal_ctx_stop(struct cal_ctx *ctx)
 	ctx->dma.state = CAL_DMA_STOP_REQUESTED;
 	spin_unlock_irq(&ctx->dma.lock);
 
-	timeout = wait_event_timeout(ctx->dma.wait, cal_ctx_wr_dma_stopped(ctx),
-				     msecs_to_jiffies(500));
-	if (!timeout) {
+	time_left = wait_event_timeout(ctx->dma.wait, cal_ctx_wr_dma_stopped(ctx),
+				       msecs_to_jiffies(500));
+	if (!time_left) {
 		ctx_err(ctx, "failed to disable dma cleanly\n");
 		cal_ctx_wr_dma_disable(ctx);
 	}
-- 
2.43.0

Re: [PATCH 6/8] media: solo6x10: use 'time_left' variable with wait_for_completion_timeout()

[Ismael Luceno]

On 03/Jun/2024 11:28, Wolfram Sang wrote:
> There is a confusing pattern in the kernel to use a variable named 'timeout' to
> store the result of wait_for_completion_timeout() causing patterns like:
> 
> 	timeout = wait_for_completion_timeout(...)
> 	if (!timeout) return -ETIMEDOUT;
> 
> with all kinds of permutations. Use 'time_left' as a variable to make the code
> self explaining.
> 
> Fix to the proper variable type 'unsigned long' while here.
> 
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
> ---
>  drivers/media/pci/solo6x10/solo6x10-p2m.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/media/pci/solo6x10/solo6x10-p2m.c b/drivers/media/pci/solo6x10/solo6x10-p2m.c
> index ca70a864a3ef..5f100e5e03d9 100644
> --- a/drivers/media/pci/solo6x10/solo6x10-p2m.c
> +++ b/drivers/media/pci/solo6x10/solo6x10-p2m.c
> @@ -57,7 +57,7 @@ int solo_p2m_dma_desc(struct solo_dev *solo_dev,
>  		      int desc_cnt)
>  {
>  	struct solo_p2m_dev *p2m_dev;
> -	unsigned int timeout;
> +	unsigned long time_left;
>  	unsigned int config = 0;
>  	int ret = 0;
>  	unsigned int p2m_id = 0;
> @@ -99,12 +99,12 @@ int solo_p2m_dma_desc(struct solo_dev *solo_dev,
>  			       desc[1].ctrl);
>  	}
>  
> -	timeout = wait_for_completion_timeout(&p2m_dev->completion,
> -					      solo_dev->p2m_jiffies);
> +	time_left = wait_for_completion_timeout(&p2m_dev->completion,
> +						solo_dev->p2m_jiffies);
>  
>  	if (WARN_ON_ONCE(p2m_dev->error))
>  		ret = -EIO;
> -	else if (timeout == 0) {
> +	else if (time_left == 0) {
>  		solo_dev->p2m_timeouts++;
>  		ret = -EAGAIN;
>  	}

Signed-off-by: Ismael Luceno <ismael@iodev.co.uk>

Re: [PATCH 6/8] media: solo6x10: use 'time_left' variable with wait_for_completion_timeout()

[Ismael Luceno]

On 03/Jun/2024 18:25, Ismael Luceno wrote:
> On 03/Jun/2024 11:28, Wolfram Sang wrote:
> > There is a confusing pattern in the kernel to use a variable named 'timeout' to
> > store the result of wait_for_completion_timeout() causing patterns like:
> > 
> > 	timeout = wait_for_completion_timeout(...)
> > 	if (!timeout) return -ETIMEDOUT;
> > 
> > with all kinds of permutations. Use 'time_left' as a variable to make the code
> > self explaining.
> > 
> > Fix to the proper variable type 'unsigned long' while here.
> > 
> > Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
> > ---
> >  drivers/media/pci/solo6x10/solo6x10-p2m.c | 8 ++++----
> >  1 file changed, 4 insertions(+), 4 deletions(-)
> > 
> > diff --git a/drivers/media/pci/solo6x10/solo6x10-p2m.c b/drivers/media/pci/solo6x10/solo6x10-p2m.c
> > index ca70a864a3ef..5f100e5e03d9 100644
> > --- a/drivers/media/pci/solo6x10/solo6x10-p2m.c
> > +++ b/drivers/media/pci/solo6x10/solo6x10-p2m.c
> > @@ -57,7 +57,7 @@ int solo_p2m_dma_desc(struct solo_dev *solo_dev,
> >  		      int desc_cnt)
> >  {
> >  	struct solo_p2m_dev *p2m_dev;
> > -	unsigned int timeout;
> > +	unsigned long time_left;
> >  	unsigned int config = 0;
> >  	int ret = 0;
> >  	unsigned int p2m_id = 0;
> > @@ -99,12 +99,12 @@ int solo_p2m_dma_desc(struct solo_dev *solo_dev,
> >  			       desc[1].ctrl);
> >  	}
> >  
> > -	timeout = wait_for_completion_timeout(&p2m_dev->completion,
> > -					      solo_dev->p2m_jiffies);
> > +	time_left = wait_for_completion_timeout(&p2m_dev->completion,
> > +						solo_dev->p2m_jiffies);
> >  
> >  	if (WARN_ON_ONCE(p2m_dev->error))
> >  		ret = -EIO;
> > -	else if (timeout == 0) {
> > +	else if (time_left == 0) {
> >  		solo_dev->p2m_timeouts++;
> >  		ret = -EAGAIN;
> >  	}
> 
> Signed-off-by: Ismael Luceno <ismael@iodev.co.uk>

Sorry, I meant Acked-by.

Re: [PATCH 1/8] media: allegro: use 'time_left' variable with wait_for_completion_timeout()

[Michael Tretter]

Hi Wolfram,

On Mon, 03 Jun 2024 11:28:32 +0200, Wolfram Sang wrote:
> There is a confusing pattern in the kernel to use a variable named 'timeout' to
> store the result of wait_for_completion_timeout() causing patterns like:
> 
> 	timeout = wait_for_completion_timeout(...)
> 	if (!timeout) return -ETIMEDOUT;
> 
> with all kinds of permutations. Use 'time_left' as a variable to make the code
> self explaining.

Thanks for the patch.

There is another instance of wait_for_completion_timeout() in the
driver, which uses tmo instead of timeout. Maybe this patch should
change that, too.

Michael

> 
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
> ---
>  .../media/platform/allegro-dvt/allegro-core.c    | 16 ++++++++--------
>  1 file changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/media/platform/allegro-dvt/allegro-core.c b/drivers/media/platform/allegro-dvt/allegro-core.c
> index da61f9beb6b4..4016aef5fdb9 100644
> --- a/drivers/media/platform/allegro-dvt/allegro-core.c
> +++ b/drivers/media/platform/allegro-dvt/allegro-core.c
> @@ -2481,14 +2481,14 @@ static void allegro_mcu_interrupt(struct allegro_dev *dev)
>  static void allegro_destroy_channel(struct allegro_channel *channel)
>  {
>  	struct allegro_dev *dev = channel->dev;
> -	unsigned long timeout;
> +	unsigned long time_left;
>  
>  	if (channel_exists(channel)) {
>  		reinit_completion(&channel->completion);
>  		allegro_mcu_send_destroy_channel(dev, channel);
> -		timeout = wait_for_completion_timeout(&channel->completion,
> -						      msecs_to_jiffies(5000));
> -		if (timeout == 0)
> +		time_left = wait_for_completion_timeout(&channel->completion,
> +							msecs_to_jiffies(5000));
> +		if (time_left == 0)
>  			v4l2_warn(&dev->v4l2_dev,
>  				  "channel %d: timeout while destroying\n",
>  				  channel->mcu_channel_id);
> @@ -2544,7 +2544,7 @@ static void allegro_destroy_channel(struct allegro_channel *channel)
>  static int allegro_create_channel(struct allegro_channel *channel)
>  {
>  	struct allegro_dev *dev = channel->dev;
> -	unsigned long timeout;
> +	unsigned long time_left;
>  
>  	if (channel_exists(channel)) {
>  		v4l2_warn(&dev->v4l2_dev,
> @@ -2595,9 +2595,9 @@ static int allegro_create_channel(struct allegro_channel *channel)
>  
>  	reinit_completion(&channel->completion);
>  	allegro_mcu_send_create_channel(dev, channel);
> -	timeout = wait_for_completion_timeout(&channel->completion,
> -					      msecs_to_jiffies(5000));
> -	if (timeout == 0)
> +	time_left = wait_for_completion_timeout(&channel->completion,
> +						msecs_to_jiffies(5000));
> +	if (time_left == 0)
>  		channel->error = -ETIMEDOUT;
>  	if (channel->error)
>  		goto err;
> -- 
> 2.43.0
> 
> 

Re: [PATCH 1/8] media: allegro: use 'time_left' variable with wait_for_completion_timeout()

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 276 bytes --]

Hi Michael,

> There is another instance of wait_for_completion_timeout() in the
> driver, which uses tmo instead of timeout. Maybe this patch should
> change that, too.

Good point. I will update the patch (after waiting some more for review
comments).

Thanks,

   Wolfram


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 7/8] media: tegra-vde: use 'time_left' variable with wait_for_completion_interruptible_timeout()

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 687 bytes --]

On Mon, Jun 03, 2024 at 11:28:38AM GMT, Wolfram Sang wrote:
> There is a confusing pattern in the kernel to use a variable named 'timeout' to
> store the result of wait_for_completion_interruptible_timeout() causing patterns like:
> 
> 	timeout = wait_for_completion_interruptible_timeout(...)
> 	if (!timeout) return -ETIMEDOUT;
> 
> with all kinds of permutations. Use 'time_left' as a variable to make the code
> self explaining.
> 
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
> ---
>  drivers/media/platform/nvidia/tegra-vde/h264.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)

Acked-by: Thierry Reding <treding@nvidia.com>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]