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

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

[Wolfram Sang]

Changes since v1:
* fixed another occasion in the allegro driver (Thanks, Michael)
* added tags (Thanks Ismael and Thierry)
* rebased to 6.11-rc1

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. Also correct the type of the variable if
the original code got it wrong.

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 | 24 +++++++++----------
 drivers/media/platform/atmel/atmel-isi.c      |  8 +++----
 .../media/platform/nvidia/tegra-vde/h264.c    | 10 ++++----
 .../platform/samsung/exynos-gsc/gsc-core.c    | 10 ++++----
 .../platform/samsung/exynos4-is/fimc-core.c   | 10 ++++----
 .../media/platform/st/sti/bdisp/bdisp-v4l2.c  | 10 ++++----
 drivers/media/platform/ti/cal/cal.c           |  8 +++----
 8 files changed, 44 insertions(+), 44 deletions(-)

-- 
2.43.0

[PATCH v2 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, also for the code path using 'tmo' as a variable.

Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---

Change since v1:
* fixed the 'tmo' variable as well

 .../media/platform/allegro-dvt/allegro-core.c | 24 +++++++++----------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/drivers/media/platform/allegro-dvt/allegro-core.c b/drivers/media/platform/allegro-dvt/allegro-core.c
index da61f9beb6b4..0142ba175198 100644
--- a/drivers/media/platform/allegro-dvt/allegro-core.c
+++ b/drivers/media/platform/allegro-dvt/allegro-core.c
@@ -1415,11 +1415,11 @@ static int allegro_mcu_send_encode_frame(struct allegro_dev *dev,
 static int allegro_mcu_wait_for_init_timeout(struct allegro_dev *dev,
 					     unsigned long timeout_ms)
 {
-	unsigned long tmo;
+	unsigned long time_left;
 
-	tmo = wait_for_completion_timeout(&dev->init_complete,
-					  msecs_to_jiffies(timeout_ms));
-	if (tmo == 0)
+	time_left = wait_for_completion_timeout(&dev->init_complete,
+						msecs_to_jiffies(timeout_ms));
+	if (time_left == 0)
 		return -ETIMEDOUT;
 
 	reinit_completion(&dev->init_complete);
@@ -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 v2 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 v2 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 v2 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 v2 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 v2 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>
Acked-by: Ismael Luceno <ismael@iodev.co.uk>
---

Change since v1: added tag

 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 v2 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>
Acked-by: Thierry Reding <treding@nvidia.com>
---

Change since v1: added tag

 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 d8812fc06c67..0e56a4331b0d 100644
--- a/drivers/media/platform/nvidia/tegra-vde/h264.c
+++ b/drivers/media/platform/nvidia/tegra-vde/h264.c
@@ -623,14 +623,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 v2 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 v2 0/8] media: use 'time_left' instead of 'timeout' with wait_*() functions

[Hans Verkuil]

Hi Wolfram,

On 05/08/2024 23:51, Wolfram Sang wrote:
> Changes since v1:
> * fixed another occasion in the allegro driver (Thanks, Michael)
> * added tags (Thanks Ismael and Thierry)
> * rebased to 6.11-rc1

Can you resend this series? This patch series wasn't picked up by our patchwork,
probably due to a full filesystem.

Apologies for the inconvenience.

Regards,

	Hans

> 
> 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. Also correct the type of the variable if
> the original code got it wrong.
> 
> 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 | 24 +++++++++----------
>  drivers/media/platform/atmel/atmel-isi.c      |  8 +++----
>  .../media/platform/nvidia/tegra-vde/h264.c    | 10 ++++----
>  .../platform/samsung/exynos-gsc/gsc-core.c    | 10 ++++----
>  .../platform/samsung/exynos4-is/fimc-core.c   | 10 ++++----
>  .../media/platform/st/sti/bdisp/bdisp-v4l2.c  | 10 ++++----
>  drivers/media/platform/ti/cal/cal.c           |  8 +++----
>  8 files changed, 44 insertions(+), 44 deletions(-)
> 

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

[Hans Verkuil]

On 07/08/2024 15:08, Hans Verkuil wrote:
> Hi Wolfram,
> 
> On 05/08/2024 23:51, Wolfram Sang wrote:
>> Changes since v1:
>> * fixed another occasion in the allegro driver (Thanks, Michael)
>> * added tags (Thanks Ismael and Thierry)
>> * rebased to 6.11-rc1
> 
> Can you resend this series? This patch series wasn't picked up by our patchwork,
> probably due to a full filesystem.

Actually, it's better to wait a bit: I now see that patchwork hasn't accepted new
patches since August 5th, so until that is fixed, there is no point in resending...

I'll let you know when it is OK again.

> Apologies for the inconvenience.

Even more apologies,

	Hans

> 
> Regards,
> 
> 	Hans
> 
>>
>> 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. Also correct the type of the variable if
>> the original code got it wrong.
>>
>> 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 | 24 +++++++++----------
>>  drivers/media/platform/atmel/atmel-isi.c      |  8 +++----
>>  .../media/platform/nvidia/tegra-vde/h264.c    | 10 ++++----
>>  .../platform/samsung/exynos-gsc/gsc-core.c    | 10 ++++----
>>  .../platform/samsung/exynos4-is/fimc-core.c   | 10 ++++----
>>  .../media/platform/st/sti/bdisp/bdisp-v4l2.c  | 10 ++++----
>>  drivers/media/platform/ti/cal/cal.c           |  8 +++----
>>  8 files changed, 44 insertions(+), 44 deletions(-)
>>
> 
> 

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

[Wolfram Sang]

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

Hi Hans,

thanks for the fast reply!

> > Can you resend this series? This patch series wasn't picked up by our patchwork,
> > probably due to a full filesystem.

You use the kernel.org one, or? There was an update including a small
downtime but no mail got lost. patchwork only needs to catch up.

> I'll let you know when it is OK again.

Seems to be good now?

https://patchwork.kernel.org/project/linux-media/list/?series=876862

> > Apologies for the inconvenience.

No worries, things happen!

All the best,

   Wolfram


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

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

[Hans Verkuil]

On 07/08/2024 16:26, Wolfram Sang wrote:
> Hi Hans,
> 
> thanks for the fast reply!
> 
>>> Can you resend this series? This patch series wasn't picked up by our patchwork,
>>> probably due to a full filesystem.
> 
> You use the kernel.org one, or? There was an update including a small
> downtime but no mail got lost. patchwork only needs to catch up.

No, we use https://patchwork.linuxtv.org/project/linux-media/list/

The server was rebooted and now emails are trickling in again.

I'm optimistic that nothing was lost, but I'll let you know if your
series disappeared after all.

Regards,

	Hans

> 
>> I'll let you know when it is OK again.
> 
> Seems to be good now?
> 
> https://patchwork.kernel.org/project/linux-media/list/?series=876862
> 
>>> Apologies for the inconvenience.
> 
> No worries, things happen!
> 
> All the best,
> 
>    Wolfram
> 

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

[Michael Tretter]

On Mon, 05 Aug 2024 23:51:14 +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, also for the code path using 'tmo' as a variable.
> 
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>

Reviewed-by: Michael Tretter <m.tretter@pengutronix.de>

> ---
> 
> Change since v1:
> * fixed the 'tmo' variable as well
> 
>  .../media/platform/allegro-dvt/allegro-core.c | 24 +++++++++----------
>  1 file changed, 12 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/media/platform/allegro-dvt/allegro-core.c b/drivers/media/platform/allegro-dvt/allegro-core.c
> index da61f9beb6b4..0142ba175198 100644
> --- a/drivers/media/platform/allegro-dvt/allegro-core.c
> +++ b/drivers/media/platform/allegro-dvt/allegro-core.c
> @@ -1415,11 +1415,11 @@ static int allegro_mcu_send_encode_frame(struct allegro_dev *dev,
>  static int allegro_mcu_wait_for_init_timeout(struct allegro_dev *dev,
>  					     unsigned long timeout_ms)
>  {
> -	unsigned long tmo;
> +	unsigned long time_left;
>  
> -	tmo = wait_for_completion_timeout(&dev->init_complete,
> -					  msecs_to_jiffies(timeout_ms));
> -	if (tmo == 0)
> +	time_left = wait_for_completion_timeout(&dev->init_complete,
> +						msecs_to_jiffies(timeout_ms));
> +	if (time_left == 0)
>  		return -ETIMEDOUT;
>  
>  	reinit_completion(&dev->init_complete);
> @@ -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 v2 2/8] media: atmel-isi: use 'time_left' variable with wait_for_completion_timeout()

[Hari.PrasathGE]

On 8/6/24 3:21 AM, Wolfram Sang wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> 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.
> 

Looks good

Reviewed-by: Hari Prasath Gujulan Elango <hari.prasathge@microchip.com>

Regards,
Hari


> 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
> 
>