2024-04-30 11:54:49

by Wolfram Sang

[permalink] [raw]
Subject: [PATCH 0/4] ASoC: use 'time_left' instead of 'timeout' with wait_for_*() functions

There is a confusing pattern in the kernel to use a variable named 'timeout' to
store the result of wait_for_*() 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
(some parts may still be WIP):

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 (4):
ASoC: codecs: wm8962: use 'time_left' variable with
wait_for_completion_timeout()
ASoC: codecs: wm8993: use 'time_left' variable with
wait_for_completion_timeout()
ASoC: codecs: wm8994: use 'time_left' variable with
wait_for_completion_timeout()
ASoC: codecs: wm8996: use 'time_left' variable with
wait_for_completion_timeout()

sound/soc/codecs/wm8962.c | 12 ++++++------
sound/soc/codecs/wm8993.c | 12 ++++++------
sound/soc/codecs/wm8994.c | 8 ++++----
sound/soc/codecs/wm8996.c | 14 +++++++-------
4 files changed, 23 insertions(+), 23 deletions(-)

--
2.43.0



2024-04-30 11:54:58

by Wolfram Sang

[permalink] [raw]
Subject: [PATCH 1/4] ASoC: codecs: wm8962: use 'time_left' variable with wait_for_completion_timeout()

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 <[email protected]>
---
sound/soc/codecs/wm8962.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/sound/soc/codecs/wm8962.c b/sound/soc/codecs/wm8962.c
index 7c6ed2983128..3856e7a8eeff 100644
--- a/sound/soc/codecs/wm8962.c
+++ b/sound/soc/codecs/wm8962.c
@@ -2886,7 +2886,7 @@ static int wm8962_set_fll(struct snd_soc_component *component, int fll_id, int s
{
struct wm8962_priv *wm8962 = snd_soc_component_get_drvdata(component);
struct _fll_div fll_div;
- unsigned long timeout;
+ unsigned long time_left;
int ret;
int fll1 = 0;

@@ -2974,14 +2974,14 @@ static int wm8962_set_fll(struct snd_soc_component *component, int fll_id, int s
* higher if we'll error out
*/
if (wm8962->irq)
- timeout = msecs_to_jiffies(5);
+ time_left = msecs_to_jiffies(5);
else
- timeout = msecs_to_jiffies(1);
+ time_left = msecs_to_jiffies(1);

- timeout = wait_for_completion_timeout(&wm8962->fll_lock,
- timeout);
+ time_left = wait_for_completion_timeout(&wm8962->fll_lock,
+ time_left);

- if (timeout == 0 && wm8962->irq) {
+ if (time_left == 0 && wm8962->irq) {
dev_err(component->dev, "FLL lock timed out");
snd_soc_component_update_bits(component, WM8962_FLL_CONTROL_1,
WM8962_FLL_ENA, 0);
--
2.43.0


2024-04-30 11:55:21

by Wolfram Sang

[permalink] [raw]
Subject: [PATCH 2/4] ASoC: codecs: wm8993: use 'time_left' variable with wait_for_completion_timeout()

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 <[email protected]>
---
sound/soc/codecs/wm8993.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/sound/soc/codecs/wm8993.c b/sound/soc/codecs/wm8993.c
index 5b788f35e5e4..dcc6301223bc 100644
--- a/sound/soc/codecs/wm8993.c
+++ b/sound/soc/codecs/wm8993.c
@@ -470,7 +470,7 @@ static int _wm8993_set_fll(struct snd_soc_component *component, int fll_id, int
struct i2c_client *i2c = to_i2c_client(component->dev);
u16 reg1, reg4, reg5;
struct _fll_div fll_div;
- unsigned int timeout;
+ unsigned long time_left;
int ret;

/* Any change? */
@@ -543,19 +543,19 @@ static int _wm8993_set_fll(struct snd_soc_component *component, int fll_id, int

/* If we've got an interrupt wired up make sure we get it */
if (i2c->irq)
- timeout = msecs_to_jiffies(20);
+ time_left = msecs_to_jiffies(20);
else if (Fref < 1000000)
- timeout = msecs_to_jiffies(3);
+ time_left = msecs_to_jiffies(3);
else
- timeout = msecs_to_jiffies(1);
+ time_left = msecs_to_jiffies(1);

try_wait_for_completion(&wm8993->fll_lock);

/* Enable the FLL */
snd_soc_component_write(component, WM8993_FLL_CONTROL_1, reg1 | WM8993_FLL_ENA);

- timeout = wait_for_completion_timeout(&wm8993->fll_lock, timeout);
- if (i2c->irq && !timeout)
+ time_left = wait_for_completion_timeout(&wm8993->fll_lock, time_left);
+ if (i2c->irq && !time_left)
dev_warn(component->dev, "Timed out waiting for FLL\n");

dev_dbg(component->dev, "FLL enabled at %dHz->%dHz\n", Fref, Fout);
--
2.43.0


2024-04-30 11:55:38

by Wolfram Sang

[permalink] [raw]
Subject: [PATCH 3/4] ASoC: codecs: wm8994: use 'time_left' variable with wait_for_completion_timeout()

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 <[email protected]>
---
sound/soc/codecs/wm8994.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/sound/soc/codecs/wm8994.c b/sound/soc/codecs/wm8994.c
index fc9894975a1d..a99908582a50 100644
--- a/sound/soc/codecs/wm8994.c
+++ b/sound/soc/codecs/wm8994.c
@@ -2210,7 +2210,7 @@ static int _wm8994_set_fll(struct snd_soc_component *component, int id, int src,
int reg_offset, ret;
struct fll_div fll;
u16 reg, clk1, aif_reg, aif_src;
- unsigned long timeout;
+ unsigned long time_left;
bool was_enabled;
struct clk *mclk;

@@ -2403,9 +2403,9 @@ static int _wm8994_set_fll(struct snd_soc_component *component, int id, int src,
WM8994_FLL1_FRAC, reg);

if (wm8994->fll_locked_irq) {
- timeout = wait_for_completion_timeout(&wm8994->fll_locked[id],
- msecs_to_jiffies(10));
- if (timeout == 0)
+ time_left = wait_for_completion_timeout(&wm8994->fll_locked[id],
+ msecs_to_jiffies(10));
+ if (time_left == 0)
dev_warn(component->dev,
"Timed out waiting for FLL lock\n");
} else {
--
2.43.0


2024-04-30 11:55:38

by Wolfram Sang

[permalink] [raw]
Subject: [PATCH 4/4] ASoC: codecs: wm8996: use 'time_left' variable with wait_for_completion_timeout()

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 <[email protected]>
---
sound/soc/codecs/wm8996.c | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/sound/soc/codecs/wm8996.c b/sound/soc/codecs/wm8996.c
index e738326e33ed..66096e09c953 100644
--- a/sound/soc/codecs/wm8996.c
+++ b/sound/soc/codecs/wm8996.c
@@ -655,28 +655,28 @@ static void wait_for_dc_servo(struct snd_soc_component *component, u16 mask)
struct i2c_client *i2c = to_i2c_client(component->dev);
struct wm8996_priv *wm8996 = snd_soc_component_get_drvdata(component);
int ret;
- unsigned long timeout = 200;
+ unsigned long time_left = 200;

snd_soc_component_write(component, WM8996_DC_SERVO_2, mask);

/* Use the interrupt if possible */
do {
if (i2c->irq) {
- timeout = wait_for_completion_timeout(&wm8996->dcs_done,
- msecs_to_jiffies(200));
- if (timeout == 0)
+ time_left = wait_for_completion_timeout(&wm8996->dcs_done,
+ msecs_to_jiffies(200));
+ if (time_left == 0)
dev_err(component->dev, "DC servo timed out\n");

} else {
msleep(1);
- timeout--;
+ time_left--;
}

ret = snd_soc_component_read(component, WM8996_DC_SERVO_2);
dev_dbg(component->dev, "DC servo state: %x\n", ret);
- } while (timeout && ret & mask);
+ } while (time_left && ret & mask);

- if (timeout == 0)
+ if (time_left == 0)
dev_err(component->dev, "DC servo timed out for %x\n", mask);
else
dev_dbg(component->dev, "DC servo complete for %x\n", mask);
--
2.43.0


2024-05-02 08:18:56

by Charles Keepax

[permalink] [raw]
Subject: Re: [PATCH 0/4] ASoC: use 'time_left' instead of 'timeout' with wait_for_*() functions

On Tue, Apr 30, 2024 at 01:54:33PM +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_*() 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
> (some parts may still be WIP):
>
> 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 (4):
> ASoC: codecs: wm8962: use 'time_left' variable with
> wait_for_completion_timeout()
> ASoC: codecs: wm8993: use 'time_left' variable with
> wait_for_completion_timeout()
> ASoC: codecs: wm8994: use 'time_left' variable with
> wait_for_completion_timeout()
> ASoC: codecs: wm8996: use 'time_left' variable with
> wait_for_completion_timeout()
>
> sound/soc/codecs/wm8962.c | 12 ++++++------
> sound/soc/codecs/wm8993.c | 12 ++++++------
> sound/soc/codecs/wm8994.c | 8 ++++----
> sound/soc/codecs/wm8996.c | 14 +++++++-------
> 4 files changed, 23 insertions(+), 23 deletions(-)
>

All look good to me.

Reviewed-by: Charles Keepax <[email protected]>

Thanks,
Charles

2024-05-07 14:35:30

by Mark Brown

[permalink] [raw]
Subject: Re: [PATCH 0/4] ASoC: use 'time_left' instead of 'timeout' with wait_for_*() functions

On Tue, 30 Apr 2024 13:54:33 +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_*() 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.
>
> [...]

Applied to

https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next

Thanks!

[1/4] ASoC: codecs: wm8962: use 'time_left' variable with wait_for_completion_timeout()
commit: cfcd957e63506273dc54f34b320172c8709244c7
[2/4] ASoC: codecs: wm8993: use 'time_left' variable with wait_for_completion_timeout()
commit: 0800660d8c59539b628f5a6646bb63091d58152f
[3/4] ASoC: codecs: wm8994: use 'time_left' variable with wait_for_completion_timeout()
commit: 19c70b4668306632d3cbbecdf5fea98b528e873e
[4/4] ASoC: codecs: wm8996: use 'time_left' variable with wait_for_completion_timeout()
commit: 4e1f953a4a447b5e001655b453505c4c15904c61

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark