Linux Tegra architecture development
 help / color / mirror / Atom feed
* [PATCH v2 0/8] media: use 'time_left' instead of 'timeout' with wait_*() functions
@ 2024-08-05 21:51 Wolfram Sang
  2024-08-05 21:51 ` [PATCH v2 7/8] media: tegra-vde: use 'time_left' variable with wait_for_completion_interruptible_timeout() Wolfram Sang
  2024-08-07 13:08 ` [PATCH v2 0/8] media: use 'time_left' instead of 'timeout' with wait_*() functions Hans Verkuil
  0 siblings, 2 replies; 6+ messages in thread
From: Wolfram Sang @ 2024-08-05 21:51 UTC (permalink / raw)
  To: linux-media
  Cc: Wolfram Sang, Alexandre Belloni, Andrey Utkin, Benoit Parrot,
	Bluecherry Maintainers, Claudiu Beznea, Dmitry Osipenko,
	Eugen Hristev, Fabien Dessenne, Ismael Luceno, Jonathan Hunter,
	Krzysztof Kozlowski, linux-arm-kernel, linux-samsung-soc,
	linux-tegra, Mauro Carvalho Chehab, Michael Tretter,
	Nicolas Ferre, Sylwester Nawrocki, Thierry Reding

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


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH v2 7/8] media: tegra-vde: use 'time_left' variable with wait_for_completion_interruptible_timeout()
  2024-08-05 21:51 [PATCH v2 0/8] media: use 'time_left' instead of 'timeout' with wait_*() functions Wolfram Sang
@ 2024-08-05 21:51 ` Wolfram Sang
  2024-08-07 13:08 ` [PATCH v2 0/8] media: use 'time_left' instead of 'timeout' with wait_*() functions Hans Verkuil
  1 sibling, 0 replies; 6+ messages in thread
From: Wolfram Sang @ 2024-08-05 21:51 UTC (permalink / raw)
  To: linux-media
  Cc: Wolfram Sang, Thierry Reding, Dmitry Osipenko,
	Mauro Carvalho Chehab, Thierry Reding, Jonathan Hunter,
	linux-tegra

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


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH v2 0/8] media: use 'time_left' instead of 'timeout' with wait_*() functions
  2024-08-05 21:51 [PATCH v2 0/8] media: use 'time_left' instead of 'timeout' with wait_*() functions Wolfram Sang
  2024-08-05 21:51 ` [PATCH v2 7/8] media: tegra-vde: use 'time_left' variable with wait_for_completion_interruptible_timeout() Wolfram Sang
@ 2024-08-07 13:08 ` Hans Verkuil
  2024-08-07 13:16   ` Hans Verkuil
  1 sibling, 1 reply; 6+ messages in thread
From: Hans Verkuil @ 2024-08-07 13:08 UTC (permalink / raw)
  To: Wolfram Sang, linux-media
  Cc: Alexandre Belloni, Andrey Utkin, Benoit Parrot,
	Bluecherry Maintainers, Claudiu Beznea, Dmitry Osipenko,
	Eugen Hristev, Fabien Dessenne, Ismael Luceno, Jonathan Hunter,
	Krzysztof Kozlowski, linux-arm-kernel, linux-samsung-soc,
	linux-tegra, Mauro Carvalho Chehab, Michael Tretter,
	Nicolas Ferre, Sylwester Nawrocki, Thierry Reding

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


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2 0/8] media: use 'time_left' instead of 'timeout' with wait_*() functions
  2024-08-07 13:08 ` [PATCH v2 0/8] media: use 'time_left' instead of 'timeout' with wait_*() functions Hans Verkuil
@ 2024-08-07 13:16   ` Hans Verkuil
  2024-08-07 14:26     ` Wolfram Sang
  0 siblings, 1 reply; 6+ messages in thread
From: Hans Verkuil @ 2024-08-07 13:16 UTC (permalink / raw)
  To: Wolfram Sang, linux-media
  Cc: Alexandre Belloni, Andrey Utkin, Benoit Parrot,
	Bluecherry Maintainers, Claudiu Beznea, Dmitry Osipenko,
	Eugen Hristev, Fabien Dessenne, Ismael Luceno, Jonathan Hunter,
	Krzysztof Kozlowski, linux-arm-kernel, linux-samsung-soc,
	linux-tegra, Mauro Carvalho Chehab, Michael Tretter,
	Nicolas Ferre, Sylwester Nawrocki, Thierry Reding

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


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2 0/8] media: use 'time_left' instead of 'timeout' with wait_*() functions
  2024-08-07 13:16   ` Hans Verkuil
@ 2024-08-07 14:26     ` Wolfram Sang
  2024-08-07 14:29       ` Hans Verkuil
  0 siblings, 1 reply; 6+ messages in thread
From: Wolfram Sang @ 2024-08-07 14:26 UTC (permalink / raw)
  To: Hans Verkuil
  Cc: linux-media, Alexandre Belloni, Andrey Utkin, Benoit Parrot,
	Bluecherry Maintainers, Claudiu Beznea, Dmitry Osipenko,
	Eugen Hristev, Fabien Dessenne, Ismael Luceno, Jonathan Hunter,
	Krzysztof Kozlowski, linux-arm-kernel, linux-samsung-soc,
	linux-tegra, Mauro Carvalho Chehab, Michael Tretter,
	Nicolas Ferre, Sylwester Nawrocki, Thierry Reding

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2 0/8] media: use 'time_left' instead of 'timeout' with wait_*() functions
  2024-08-07 14:26     ` Wolfram Sang
@ 2024-08-07 14:29       ` Hans Verkuil
  0 siblings, 0 replies; 6+ messages in thread
From: Hans Verkuil @ 2024-08-07 14:29 UTC (permalink / raw)
  To: Wolfram Sang, linux-media, Alexandre Belloni, Andrey Utkin,
	Benoit Parrot, Bluecherry Maintainers, Claudiu Beznea,
	Dmitry Osipenko, Eugen Hristev, Fabien Dessenne, Ismael Luceno,
	Jonathan Hunter, Krzysztof Kozlowski, linux-arm-kernel,
	linux-samsung-soc, linux-tegra, Mauro Carvalho Chehab,
	Michael Tretter, Nicolas Ferre, Sylwester Nawrocki,
	Thierry Reding

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
> 


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2024-08-07 14:29 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-08-05 21:51 [PATCH v2 0/8] media: use 'time_left' instead of 'timeout' with wait_*() functions Wolfram Sang
2024-08-05 21:51 ` [PATCH v2 7/8] media: tegra-vde: use 'time_left' variable with wait_for_completion_interruptible_timeout() Wolfram Sang
2024-08-07 13:08 ` [PATCH v2 0/8] media: use 'time_left' instead of 'timeout' with wait_*() functions Hans Verkuil
2024-08-07 13:16   ` Hans Verkuil
2024-08-07 14:26     ` Wolfram Sang
2024-08-07 14:29       ` Hans Verkuil

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox