There is a confusing pattern in the kernel to use a variable named 'timeout' to
store the result of wait_for_*() 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.
This is the I2C part of a tree-wide series. The rest of the patches can
be found here (slightly WIP):
git://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux.git i2c/time_left
Because these patches are generated, they need manual audit. So, I will
send them step by step. This is part 1 and also a call for opinions if
this is a desirable change. But at least in the I2C realm, I really want
to have it proper.
Build bot is happy with these patches and I also compile tested them
(except two). No functional changes intended.
Wolfram Sang (15):
i2c: amd-mp2-plat: use 'time_left' variable with
wait_for_completion_timeout()
i2c: digicolor: use 'time_left' variable with
wait_for_completion_timeout()
i2c: exynos5: use 'time_left' variable with
wait_for_completion_timeout()
i2c: hix5hd2: use 'time_left' variable with
wait_for_completion_timeout()
i2c: imx-lpi2c: use 'time_left' variable with
wait_for_completion_timeout()
i2c: omap: use 'time_left' variable with wait_for_completion_timeout()
i2c: st: use 'time_left' variable with wait_for_completion_timeout()
i2c: stm32f4: use 'time_left' variable with
wait_for_completion_timeout()
i2c: stm32f7: use 'time_left' variable with
wait_for_completion_timeout()
i2c: synquacer: use 'time_left' variable with
wait_for_completion_timeout()
i2c: jz4780: use 'time_left' variable with
wait_for_completion_timeout()
i2c: qcom-geni: use 'time_left' variable with
wait_for_completion_timeout()
i2c: rk3x: use 'time_left' variable with wait_event_timeout()
i2c: s3c2410: use 'time_left' variable with wait_event_timeout()
i2c: pxa: use 'time_left' variable with wait_event_timeout()
--
2.43.0
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]>
---
drivers/i2c/busses/i2c-hix5hd2.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/i2c/busses/i2c-hix5hd2.c b/drivers/i2c/busses/i2c-hix5hd2.c
index 8e75515c3ca4..a47b9939fa2c 100644
--- a/drivers/i2c/busses/i2c-hix5hd2.c
+++ b/drivers/i2c/busses/i2c-hix5hd2.c
@@ -314,7 +314,7 @@ static void hix5hd2_i2c_message_start(struct hix5hd2_i2c_priv *priv, int stop)
static int hix5hd2_i2c_xfer_msg(struct hix5hd2_i2c_priv *priv,
struct i2c_msg *msgs, int stop)
{
- unsigned long timeout;
+ unsigned long time_left;
int ret;
priv->msg = msgs;
@@ -327,9 +327,9 @@ static int hix5hd2_i2c_xfer_msg(struct hix5hd2_i2c_priv *priv,
reinit_completion(&priv->msg_complete);
hix5hd2_i2c_message_start(priv, stop);
- timeout = wait_for_completion_timeout(&priv->msg_complete,
- priv->adap.timeout);
- if (timeout == 0) {
+ time_left = wait_for_completion_timeout(&priv->msg_complete,
+ priv->adap.timeout);
+ if (time_left == 0) {
priv->state = HIX5I2C_STAT_RW_ERR;
priv->err = -ETIMEDOUT;
dev_warn(priv->dev, "%s timeout=%d\n",
--
2.43.0
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]>
---
drivers/i2c/busses/i2c-exynos5.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/i2c/busses/i2c-exynos5.c b/drivers/i2c/busses/i2c-exynos5.c
index 385ef9d9e4d4..d8baca9b610c 100644
--- a/drivers/i2c/busses/i2c-exynos5.c
+++ b/drivers/i2c/busses/i2c-exynos5.c
@@ -763,7 +763,7 @@ static bool exynos5_i2c_poll_irqs_timeout(struct exynos5_i2c *i2c,
static int exynos5_i2c_xfer_msg(struct exynos5_i2c *i2c,
struct i2c_msg *msgs, int stop)
{
- unsigned long timeout;
+ unsigned long time_left;
int ret;
i2c->msg = msgs;
@@ -775,13 +775,13 @@ static int exynos5_i2c_xfer_msg(struct exynos5_i2c *i2c,
exynos5_i2c_message_start(i2c, stop);
if (!i2c->atomic)
- timeout = wait_for_completion_timeout(&i2c->msg_complete,
- EXYNOS5_I2C_TIMEOUT);
- else
- timeout = exynos5_i2c_poll_irqs_timeout(i2c,
+ time_left = wait_for_completion_timeout(&i2c->msg_complete,
EXYNOS5_I2C_TIMEOUT);
+ else
+ time_left = exynos5_i2c_poll_irqs_timeout(i2c,
+ EXYNOS5_I2C_TIMEOUT);
- if (timeout == 0)
+ if (time_left == 0)
ret = -ETIMEDOUT;
else
ret = i2c->state;
--
2.43.0
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]>
---
drivers/i2c/busses/i2c-omap.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c
index 36bebef36740..30a5ea282a8b 100644
--- a/drivers/i2c/busses/i2c-omap.c
+++ b/drivers/i2c/busses/i2c-omap.c
@@ -660,7 +660,7 @@ static int omap_i2c_xfer_msg(struct i2c_adapter *adap,
struct i2c_msg *msg, int stop, bool polling)
{
struct omap_i2c_dev *omap = i2c_get_adapdata(adap);
- unsigned long timeout;
+ unsigned long time_left;
u16 w;
int ret;
@@ -740,18 +740,18 @@ static int omap_i2c_xfer_msg(struct i2c_adapter *adap,
* into arbitration and we're currently unable to recover from it.
*/
if (!polling) {
- timeout = wait_for_completion_timeout(&omap->cmd_complete,
- OMAP_I2C_TIMEOUT);
+ time_left = wait_for_completion_timeout(&omap->cmd_complete,
+ OMAP_I2C_TIMEOUT);
} else {
do {
omap_i2c_wait(omap);
ret = omap_i2c_xfer_data(omap);
} while (ret == -EAGAIN);
- timeout = !ret;
+ time_left = !ret;
}
- if (timeout == 0) {
+ if (time_left == 0) {
omap_i2c_reset(omap);
__omap_i2c_init(omap);
return -ETIMEDOUT;
--
2.43.0
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]>
---
drivers/i2c/busses/i2c-st.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/i2c/busses/i2c-st.c b/drivers/i2c/busses/i2c-st.c
index dbb93394ff94..5e01fe3dbb63 100644
--- a/drivers/i2c/busses/i2c-st.c
+++ b/drivers/i2c/busses/i2c-st.c
@@ -647,7 +647,7 @@ static int st_i2c_xfer_msg(struct st_i2c_dev *i2c_dev, struct i2c_msg *msg,
{
struct st_i2c_client *c = &i2c_dev->client;
u32 ctl, i2c, it;
- unsigned long timeout;
+ unsigned long time_left;
int ret;
c->addr = i2c_8bit_addr_from_msg(msg);
@@ -685,11 +685,11 @@ static int st_i2c_xfer_msg(struct st_i2c_dev *i2c_dev, struct i2c_msg *msg,
st_i2c_set_bits(i2c_dev->base + SSC_I2C, SSC_I2C_STRTG);
}
- timeout = wait_for_completion_timeout(&i2c_dev->complete,
- i2c_dev->adap.timeout);
+ time_left = wait_for_completion_timeout(&i2c_dev->complete,
+ i2c_dev->adap.timeout);
ret = c->result;
- if (!timeout)
+ if (!time_left)
ret = -ETIMEDOUT;
i2c = SSC_I2C_STOPG | SSC_I2C_REPSTRTG;
--
2.43.0
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]>
---
drivers/i2c/busses/i2c-stm32f7.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/i2c/busses/i2c-stm32f7.c b/drivers/i2c/busses/i2c-stm32f7.c
index 01210452216b..cfee2d9c09de 100644
--- a/drivers/i2c/busses/i2c-stm32f7.c
+++ b/drivers/i2c/busses/i2c-stm32f7.c
@@ -1789,7 +1789,7 @@ static int stm32f7_i2c_smbus_xfer(struct i2c_adapter *adapter, u16 addr,
struct stm32f7_i2c_msg *f7_msg = &i2c_dev->f7_msg;
struct stm32_i2c_dma *dma = i2c_dev->dma;
struct device *dev = i2c_dev->dev;
- unsigned long timeout;
+ unsigned long time_left;
int i, ret;
f7_msg->addr = addr;
@@ -1809,8 +1809,8 @@ static int stm32f7_i2c_smbus_xfer(struct i2c_adapter *adapter, u16 addr,
if (ret)
goto pm_free;
- timeout = wait_for_completion_timeout(&i2c_dev->complete,
- i2c_dev->adap.timeout);
+ time_left = wait_for_completion_timeout(&i2c_dev->complete,
+ i2c_dev->adap.timeout);
ret = f7_msg->result;
if (ret) {
if (i2c_dev->use_dma)
@@ -1826,7 +1826,7 @@ static int stm32f7_i2c_smbus_xfer(struct i2c_adapter *adapter, u16 addr,
goto pm_free;
}
- if (!timeout) {
+ if (!time_left) {
dev_dbg(dev, "Access to slave 0x%x timed out\n", f7_msg->addr);
if (i2c_dev->use_dma)
dmaengine_terminate_sync(dma->chan_using);
--
2.43.0
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]>
---
drivers/i2c/busses/i2c-stm32f4.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/i2c/busses/i2c-stm32f4.c b/drivers/i2c/busses/i2c-stm32f4.c
index 859ac0cf7f6c..f8b12be6ef55 100644
--- a/drivers/i2c/busses/i2c-stm32f4.c
+++ b/drivers/i2c/busses/i2c-stm32f4.c
@@ -681,7 +681,7 @@ static int stm32f4_i2c_xfer_msg(struct stm32f4_i2c_dev *i2c_dev,
{
struct stm32f4_i2c_msg *f4_msg = &i2c_dev->msg;
void __iomem *reg = i2c_dev->base + STM32F4_I2C_CR1;
- unsigned long timeout;
+ unsigned long time_left;
u32 mask;
int ret;
@@ -706,11 +706,11 @@ static int stm32f4_i2c_xfer_msg(struct stm32f4_i2c_dev *i2c_dev,
stm32f4_i2c_set_bits(reg, STM32F4_I2C_CR1_START);
}
- timeout = wait_for_completion_timeout(&i2c_dev->complete,
- i2c_dev->adap.timeout);
+ time_left = wait_for_completion_timeout(&i2c_dev->complete,
+ i2c_dev->adap.timeout);
ret = f4_msg->result;
- if (!timeout)
+ if (!time_left)
ret = -ETIMEDOUT;
return ret;
--
2.43.0
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 <[email protected]>
---
drivers/i2c/busses/i2c-s3c2410.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/i2c/busses/i2c-s3c2410.c b/drivers/i2c/busses/i2c-s3c2410.c
index 275f7c42165c..01419c738cfc 100644
--- a/drivers/i2c/busses/i2c-s3c2410.c
+++ b/drivers/i2c/busses/i2c-s3c2410.c
@@ -685,7 +685,7 @@ static void s3c24xx_i2c_wait_idle(struct s3c24xx_i2c *i2c)
static int s3c24xx_i2c_doxfer(struct s3c24xx_i2c *i2c,
struct i2c_msg *msgs, int num)
{
- unsigned long timeout = 0;
+ long time_left = 0;
int ret;
ret = s3c24xx_i2c_set_master(i2c);
@@ -715,7 +715,7 @@ static int s3c24xx_i2c_doxfer(struct s3c24xx_i2c *i2c,
dev_err(i2c->dev, "deal with arbitration loss\n");
}
} else {
- timeout = wait_event_timeout(i2c->wait, i2c->msg_num == 0, HZ * 5);
+ time_left = wait_event_timeout(i2c->wait, i2c->msg_num == 0, HZ * 5);
}
ret = i2c->msg_idx;
@@ -724,7 +724,7 @@ static int s3c24xx_i2c_doxfer(struct s3c24xx_i2c *i2c,
* Having these next two as dev_err() makes life very
* noisy when doing an i2cdetect
*/
- if (timeout == 0)
+ if (time_left == 0)
dev_dbg(i2c->dev, "timeout\n");
else if (ret != num)
dev_dbg(i2c->dev, "incomplete xfer (%d)\n", ret);
--
2.43.0
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]>
---
drivers/i2c/busses/i2c-imx-lpi2c.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/i2c/busses/i2c-imx-lpi2c.c b/drivers/i2c/busses/i2c-imx-lpi2c.c
index 36e8f6196a87..0197786892a2 100644
--- a/drivers/i2c/busses/i2c-imx-lpi2c.c
+++ b/drivers/i2c/busses/i2c-imx-lpi2c.c
@@ -307,11 +307,11 @@ static int lpi2c_imx_master_disable(struct lpi2c_imx_struct *lpi2c_imx)
static int lpi2c_imx_msg_complete(struct lpi2c_imx_struct *lpi2c_imx)
{
- unsigned long timeout;
+ unsigned long time_left;
- timeout = wait_for_completion_timeout(&lpi2c_imx->complete, HZ);
+ time_left = wait_for_completion_timeout(&lpi2c_imx->complete, HZ);
- return timeout ? 0 : -ETIMEDOUT;
+ return time_left ? 0 : -ETIMEDOUT;
}
static int lpi2c_imx_txfifo_empty(struct lpi2c_imx_struct *lpi2c_imx)
--
2.43.0
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 <[email protected]>
---
drivers/i2c/busses/i2c-rk3x.c | 11 ++++++-----
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/drivers/i2c/busses/i2c-rk3x.c b/drivers/i2c/busses/i2c-rk3x.c
index 8c7367f289d3..beca61700c89 100644
--- a/drivers/i2c/busses/i2c-rk3x.c
+++ b/drivers/i2c/busses/i2c-rk3x.c
@@ -1060,7 +1060,8 @@ static int rk3x_i2c_xfer_common(struct i2c_adapter *adap,
struct i2c_msg *msgs, int num, bool polling)
{
struct rk3x_i2c *i2c = (struct rk3x_i2c *)adap->algo_data;
- unsigned long timeout, flags;
+ unsigned long flags;
+ long time_left;
u32 val;
int ret = 0;
int i;
@@ -1092,20 +1093,20 @@ static int rk3x_i2c_xfer_common(struct i2c_adapter *adap,
if (!polling) {
rk3x_i2c_start(i2c);
- timeout = wait_event_timeout(i2c->wait, !i2c->busy,
- msecs_to_jiffies(WAIT_TIMEOUT));
+ time_left = wait_event_timeout(i2c->wait, !i2c->busy,
+ msecs_to_jiffies(WAIT_TIMEOUT));
} else {
disable_irq(i2c->irq);
rk3x_i2c_start(i2c);
- timeout = rk3x_i2c_wait_xfer_poll(i2c);
+ time_left = rk3x_i2c_wait_xfer_poll(i2c);
enable_irq(i2c->irq);
}
spin_lock_irqsave(&i2c->lock, flags);
- if (timeout == 0) {
+ if (time_left == 0) {
/* Force a STOP condition without interrupt */
i2c_writel(i2c, 0, REG_IEN);
val = i2c_readl(i2c, REG_CON) & REG_CON_TUNING_MASK;
--
2.43.0
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 <[email protected]>
---
drivers/i2c/busses/i2c-pxa.c | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/drivers/i2c/busses/i2c-pxa.c b/drivers/i2c/busses/i2c-pxa.c
index 888ca636f3f3..f495560bd99c 100644
--- a/drivers/i2c/busses/i2c-pxa.c
+++ b/drivers/i2c/busses/i2c-pxa.c
@@ -826,7 +826,7 @@ static inline void i2c_pxa_stop_message(struct pxa_i2c *i2c)
static int i2c_pxa_send_mastercode(struct pxa_i2c *i2c)
{
u32 icr;
- long timeout;
+ long time_left;
spin_lock_irq(&i2c->lock);
i2c->highmode_enter = true;
@@ -837,12 +837,12 @@ static int i2c_pxa_send_mastercode(struct pxa_i2c *i2c)
writel(icr, _ICR(i2c));
spin_unlock_irq(&i2c->lock);
- timeout = wait_event_timeout(i2c->wait,
- i2c->highmode_enter == false, HZ * 1);
+ time_left = wait_event_timeout(i2c->wait,
+ i2c->highmode_enter == false, HZ * 1);
i2c->highmode_enter = false;
- return (timeout == 0) ? I2C_RETRY : 0;
+ return (time_left == 0) ? I2C_RETRY : 0;
}
/*
@@ -1050,7 +1050,7 @@ static irqreturn_t i2c_pxa_handler(int this_irq, void *dev_id)
*/
static int i2c_pxa_do_xfer(struct pxa_i2c *i2c, struct i2c_msg *msg, int num)
{
- long timeout;
+ long time_left;
int ret;
/*
@@ -1095,7 +1095,7 @@ static int i2c_pxa_do_xfer(struct pxa_i2c *i2c, struct i2c_msg *msg, int num)
/*
* The rest of the processing occurs in the interrupt handler.
*/
- timeout = wait_event_timeout(i2c->wait, i2c->msg_num == 0, HZ * 5);
+ time_left = wait_event_timeout(i2c->wait, i2c->msg_num == 0, HZ * 5);
i2c_pxa_stop_message(i2c);
/*
@@ -1103,7 +1103,7 @@ static int i2c_pxa_do_xfer(struct pxa_i2c *i2c, struct i2c_msg *msg, int num)
*/
ret = i2c->msg_idx;
- if (!timeout && i2c->msg_num) {
+ if (!time_left && i2c->msg_num) {
i2c_pxa_scream_blue_murder(i2c, "timeout with active message");
i2c_recover_bus(&i2c->adap);
ret = I2C_RETRY;
--
2.43.0
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]>
---
drivers/i2c/busses/i2c-synquacer.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/i2c/busses/i2c-synquacer.c b/drivers/i2c/busses/i2c-synquacer.c
index bbea521b05dd..bf9bcfefa30e 100644
--- a/drivers/i2c/busses/i2c-synquacer.c
+++ b/drivers/i2c/busses/i2c-synquacer.c
@@ -311,7 +311,7 @@ static int synquacer_i2c_doxfer(struct synquacer_i2c *i2c,
struct i2c_msg *msgs, int num)
{
unsigned char bsr;
- unsigned long timeout;
+ unsigned long time_left;
int ret;
synquacer_i2c_hw_init(i2c);
@@ -335,9 +335,9 @@ static int synquacer_i2c_doxfer(struct synquacer_i2c *i2c,
return ret;
}
- timeout = wait_for_completion_timeout(&i2c->completion,
- msecs_to_jiffies(i2c->timeout_ms));
- if (timeout == 0) {
+ time_left = wait_for_completion_timeout(&i2c->completion,
+ msecs_to_jiffies(i2c->timeout_ms));
+ if (time_left == 0) {
dev_dbg(i2c->dev, "timeout\n");
return -EAGAIN;
}
--
2.43.0
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]>
---
drivers/i2c/busses/i2c-jz4780.c | 22 +++++++++++-----------
1 file changed, 11 insertions(+), 11 deletions(-)
diff --git a/drivers/i2c/busses/i2c-jz4780.c b/drivers/i2c/busses/i2c-jz4780.c
index 55035cca0ae5..7951891d6b97 100644
--- a/drivers/i2c/busses/i2c-jz4780.c
+++ b/drivers/i2c/busses/i2c-jz4780.c
@@ -565,7 +565,7 @@ static inline int jz4780_i2c_xfer_read(struct jz4780_i2c *i2c,
int idx)
{
int ret = 0;
- long timeout;
+ unsigned long time_left;
int wait_time = JZ4780_I2C_TIMEOUT * (len + 5);
unsigned short tmp;
unsigned long flags;
@@ -600,10 +600,10 @@ static inline int jz4780_i2c_xfer_read(struct jz4780_i2c *i2c,
spin_unlock_irqrestore(&i2c->lock, flags);
- timeout = wait_for_completion_timeout(&i2c->trans_waitq,
- msecs_to_jiffies(wait_time));
+ time_left = wait_for_completion_timeout(&i2c->trans_waitq,
+ msecs_to_jiffies(wait_time));
- if (!timeout) {
+ if (!time_left) {
dev_err(&i2c->adap.dev, "irq read timeout\n");
dev_dbg(&i2c->adap.dev, "send cmd count:%d %d\n",
i2c->cmd, i2c->cmd_buf[i2c->cmd]);
@@ -627,7 +627,7 @@ static inline int jz4780_i2c_xfer_write(struct jz4780_i2c *i2c,
{
int ret = 0;
int wait_time = JZ4780_I2C_TIMEOUT * (len + 5);
- long timeout;
+ unsigned long time_left;
unsigned short tmp;
unsigned long flags;
@@ -655,14 +655,14 @@ static inline int jz4780_i2c_xfer_write(struct jz4780_i2c *i2c,
spin_unlock_irqrestore(&i2c->lock, flags);
- timeout = wait_for_completion_timeout(&i2c->trans_waitq,
- msecs_to_jiffies(wait_time));
- if (timeout && !i2c->stop_hold) {
+ time_left = wait_for_completion_timeout(&i2c->trans_waitq,
+ msecs_to_jiffies(wait_time));
+ if (time_left && !i2c->stop_hold) {
unsigned short i2c_sta;
int write_in_process;
- timeout = JZ4780_I2C_TIMEOUT * 100;
- for (; timeout > 0; timeout--) {
+ time_left = JZ4780_I2C_TIMEOUT * 100;
+ for (; time_left > 0; time_left--) {
i2c_sta = jz4780_i2c_readw(i2c, JZ4780_I2C_STA);
write_in_process = (i2c_sta & JZ4780_I2C_STA_MSTACT) ||
@@ -673,7 +673,7 @@ static inline int jz4780_i2c_xfer_write(struct jz4780_i2c *i2c,
}
}
- if (!timeout) {
+ if (!time_left) {
dev_err(&i2c->adap.dev, "write wait timeout\n");
ret = -EIO;
}
--
2.43.0
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]>
---
drivers/i2c/busses/i2c-qcom-geni.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/i2c/busses/i2c-qcom-geni.c b/drivers/i2c/busses/i2c-qcom-geni.c
index 090b4846ed62..0a8b95ce35f7 100644
--- a/drivers/i2c/busses/i2c-qcom-geni.c
+++ b/drivers/i2c/busses/i2c-qcom-geni.c
@@ -586,7 +586,8 @@ static int geni_i2c_gpi_xfer(struct geni_i2c_dev *gi2c, struct i2c_msg msgs[], i
{
struct dma_slave_config config = {};
struct gpi_i2c_config peripheral = {};
- int i, ret = 0, timeout;
+ int i, ret = 0;
+ unsigned long time_left;
dma_addr_t tx_addr, rx_addr;
void *tx_buf = NULL, *rx_buf = NULL;
const struct geni_i2c_clk_fld *itr = gi2c->clk_fld;
@@ -629,8 +630,8 @@ static int geni_i2c_gpi_xfer(struct geni_i2c_dev *gi2c, struct i2c_msg msgs[], i
dma_async_issue_pending(gi2c->tx_c);
- timeout = wait_for_completion_timeout(&gi2c->done, XFER_TIMEOUT);
- if (!timeout)
+ time_left = wait_for_completion_timeout(&gi2c->done, XFER_TIMEOUT);
+ if (!time_left)
gi2c->err = -ETIMEDOUT;
if (gi2c->err) {
--
2.43.0
> Subject: [PATCH 05/15] i2c: imx-lpi2c: 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]>
Reviewed-by: Peng Fan <[email protected]>
> ---
> drivers/i2c/busses/i2c-imx-lpi2c.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/i2c/busses/i2c-imx-lpi2c.c b/drivers/i2c/busses/i2c-imx-
> lpi2c.c
> index 36e8f6196a87..0197786892a2 100644
> --- a/drivers/i2c/busses/i2c-imx-lpi2c.c
> +++ b/drivers/i2c/busses/i2c-imx-lpi2c.c
> @@ -307,11 +307,11 @@ static int lpi2c_imx_master_disable(struct
> lpi2c_imx_struct *lpi2c_imx)
>
> static int lpi2c_imx_msg_complete(struct lpi2c_imx_struct *lpi2c_imx) {
> - unsigned long timeout;
> + unsigned long time_left;
>
> - timeout = wait_for_completion_timeout(&lpi2c_imx->complete, HZ);
> + time_left = wait_for_completion_timeout(&lpi2c_imx->complete, HZ);
>
> - return timeout ? 0 : -ETIMEDOUT;
> + return time_left ? 0 : -ETIMEDOUT;
> }
>
> static int lpi2c_imx_txfifo_empty(struct lpi2c_imx_struct *lpi2c_imx)
> --
> 2.43.0
>
Hello Wolfram,
On Sat, Apr 27, 2024 at 10:35:59PM +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.
>
> Signed-off-by: Wolfram Sang <[email protected]>
I like the improvement in general and looking at this driver in more
detail this looks right.
Reviewed-by: Uwe Kleine-K?nig <[email protected]>
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-K?nig |
Industrial Linux Solutions | https://www.pengutronix.de/ |
On Sat, 27 Apr 2024 at 22:36, Wolfram Sang
<[email protected]> 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.
>
> Signed-off-by: Wolfram Sang <[email protected]>
Acked-by: Ard Biesheuvel <[email protected]>
> ---
> drivers/i2c/busses/i2c-synquacer.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/i2c/busses/i2c-synquacer.c b/drivers/i2c/busses/i2c-synquacer.c
> index bbea521b05dd..bf9bcfefa30e 100644
> --- a/drivers/i2c/busses/i2c-synquacer.c
> +++ b/drivers/i2c/busses/i2c-synquacer.c
> @@ -311,7 +311,7 @@ static int synquacer_i2c_doxfer(struct synquacer_i2c *i2c,
> struct i2c_msg *msgs, int num)
> {
> unsigned char bsr;
> - unsigned long timeout;
> + unsigned long time_left;
> int ret;
>
> synquacer_i2c_hw_init(i2c);
> @@ -335,9 +335,9 @@ static int synquacer_i2c_doxfer(struct synquacer_i2c *i2c,
> return ret;
> }
>
> - timeout = wait_for_completion_timeout(&i2c->completion,
> - msecs_to_jiffies(i2c->timeout_ms));
> - if (timeout == 0) {
> + time_left = wait_for_completion_timeout(&i2c->completion,
> + msecs_to_jiffies(i2c->timeout_ms));
> + if (time_left == 0) {
> dev_dbg(i2c->dev, "timeout\n");
> return -EAGAIN;
> }
> --
> 2.43.0
>
On 27/04/2024 21:36, 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 <[email protected]>
> ---
> drivers/i2c/busses/i2c-qcom-geni.c | 7 ++++---
> 1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/i2c/busses/i2c-qcom-geni.c b/drivers/i2c/busses/i2c-qcom-geni.c
> index 090b4846ed62..0a8b95ce35f7 100644
> --- a/drivers/i2c/busses/i2c-qcom-geni.c
> +++ b/drivers/i2c/busses/i2c-qcom-geni.c
> @@ -586,7 +586,8 @@ static int geni_i2c_gpi_xfer(struct geni_i2c_dev *gi2c, struct i2c_msg msgs[], i
> {
> struct dma_slave_config config = {};
> struct gpi_i2c_config peripheral = {};
> - int i, ret = 0, timeout;
> + int i, ret = 0;
> + unsigned long time_left;
> dma_addr_t tx_addr, rx_addr;
> void *tx_buf = NULL, *rx_buf = NULL;
> const struct geni_i2c_clk_fld *itr = gi2c->clk_fld;
> @@ -629,8 +630,8 @@ static int geni_i2c_gpi_xfer(struct geni_i2c_dev *gi2c, struct i2c_msg msgs[], i
>
> dma_async_issue_pending(gi2c->tx_c);
>
> - timeout = wait_for_completion_timeout(&gi2c->done, XFER_TIMEOUT);
> - if (!timeout)
> + time_left = wait_for_completion_timeout(&gi2c->done, XFER_TIMEOUT);
> + if (!time_left)
> gi2c->err = -ETIMEDOUT;
>
> if (gi2c->err) {
Reviewed-by: Bryan O'Donoghue <[email protected]>
Hi Wolfram,
On Sat, Apr 27, 2024 at 10:35:52PM +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_*() 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.
>
> This is the I2C part of a tree-wide series. The rest of the patches can
> be found here (slightly WIP):
>
> git://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux.git i2c/time_left
>
> Because these patches are generated, they need manual audit. So, I will
> send them step by step. This is part 1 and also a call for opinions if
> this is a desirable change. But at least in the I2C realm, I really want
> to have it proper.
>
> Build bot is happy with these patches and I also compile tested them
> (except two). No functional changes intended.
It all looks good, I will give people a few days more for reviews
and comments and then I'll take it in.
Andi
On Sat, Apr 27, 2024 at 10:36:04PM +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.
>
> Fix to the proper variable type 'unsigned long' while here.
>
> Signed-off-by: Wolfram Sang <[email protected]>
Reviewed-by: Bjorn Andersson <[email protected]>
Regards,
Bjorn
> ---
> drivers/i2c/busses/i2c-qcom-geni.c | 7 ++++---
> 1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/i2c/busses/i2c-qcom-geni.c b/drivers/i2c/busses/i2c-qcom-geni.c
> index 090b4846ed62..0a8b95ce35f7 100644
> --- a/drivers/i2c/busses/i2c-qcom-geni.c
> +++ b/drivers/i2c/busses/i2c-qcom-geni.c
> @@ -586,7 +586,8 @@ static int geni_i2c_gpi_xfer(struct geni_i2c_dev *gi2c, struct i2c_msg msgs[], i
> {
> struct dma_slave_config config = {};
> struct gpi_i2c_config peripheral = {};
> - int i, ret = 0, timeout;
> + int i, ret = 0;
> + unsigned long time_left;
> dma_addr_t tx_addr, rx_addr;
> void *tx_buf = NULL, *rx_buf = NULL;
> const struct geni_i2c_clk_fld *itr = gi2c->clk_fld;
> @@ -629,8 +630,8 @@ static int geni_i2c_gpi_xfer(struct geni_i2c_dev *gi2c, struct i2c_msg msgs[], i
>
> dma_async_issue_pending(gi2c->tx_c);
>
> - timeout = wait_for_completion_timeout(&gi2c->done, XFER_TIMEOUT);
> - if (!timeout)
> + time_left = wait_for_completion_timeout(&gi2c->done, XFER_TIMEOUT);
> + if (!time_left)
> gi2c->err = -ETIMEDOUT;
>
> if (gi2c->err) {
> --
> 2.43.0
>
On 27/4/24 22:36, 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 <[email protected]>
> ---
> drivers/i2c/busses/i2c-jz4780.c | 22 +++++++++++-----------
> 1 file changed, 11 insertions(+), 11 deletions(-)
Reviewed-by: Philippe Mathieu-Daudé <[email protected]>
Le samedi 27 avril 2024 à 22:36 +0200, Wolfram Sang a écrit :
> 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]>
Acked-by: Paul Cercueil <[email protected]>
Cheers,
-Paul
> ---
> drivers/i2c/busses/i2c-jz4780.c | 22 +++++++++++-----------
> 1 file changed, 11 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/i2c/busses/i2c-jz4780.c
> b/drivers/i2c/busses/i2c-jz4780.c
> index 55035cca0ae5..7951891d6b97 100644
> --- a/drivers/i2c/busses/i2c-jz4780.c
> +++ b/drivers/i2c/busses/i2c-jz4780.c
> @@ -565,7 +565,7 @@ static inline int jz4780_i2c_xfer_read(struct
> jz4780_i2c *i2c,
> int idx)
> {
> int ret = 0;
> - long timeout;
> + unsigned long time_left;
> int wait_time = JZ4780_I2C_TIMEOUT * (len + 5);
> unsigned short tmp;
> unsigned long flags;
> @@ -600,10 +600,10 @@ static inline int jz4780_i2c_xfer_read(struct
> jz4780_i2c *i2c,
>
> spin_unlock_irqrestore(&i2c->lock, flags);
>
> - timeout = wait_for_completion_timeout(&i2c->trans_waitq,
> -
> msecs_to_jiffies(wait_time));
> + time_left = wait_for_completion_timeout(&i2c->trans_waitq,
> + msecs_to_jiffies(wai
> t_time));
>
> - if (!timeout) {
> + if (!time_left) {
> dev_err(&i2c->adap.dev, "irq read timeout\n");
> dev_dbg(&i2c->adap.dev, "send cmd count:%d %d\n",
> i2c->cmd, i2c->cmd_buf[i2c->cmd]);
> @@ -627,7 +627,7 @@ static inline int jz4780_i2c_xfer_write(struct
> jz4780_i2c *i2c,
> {
> int ret = 0;
> int wait_time = JZ4780_I2C_TIMEOUT * (len + 5);
> - long timeout;
> + unsigned long time_left;
> unsigned short tmp;
> unsigned long flags;
>
> @@ -655,14 +655,14 @@ static inline int jz4780_i2c_xfer_write(struct
> jz4780_i2c *i2c,
>
> spin_unlock_irqrestore(&i2c->lock, flags);
>
> - timeout = wait_for_completion_timeout(&i2c->trans_waitq,
> -
> msecs_to_jiffies(wait_time));
> - if (timeout && !i2c->stop_hold) {
> + time_left = wait_for_completion_timeout(&i2c->trans_waitq,
> + msecs_to_jiffies(wai
> t_time));
> + if (time_left && !i2c->stop_hold) {
> unsigned short i2c_sta;
> int write_in_process;
>
> - timeout = JZ4780_I2C_TIMEOUT * 100;
> - for (; timeout > 0; timeout--) {
> + time_left = JZ4780_I2C_TIMEOUT * 100;
> + for (; time_left > 0; time_left--) {
> i2c_sta = jz4780_i2c_readw(i2c,
> JZ4780_I2C_STA);
>
> write_in_process = (i2c_sta &
> JZ4780_I2C_STA_MSTACT) ||
> @@ -673,7 +673,7 @@ static inline int jz4780_i2c_xfer_write(struct
> jz4780_i2c *i2c,
> }
> }
>
> - if (!timeout) {
> + if (!time_left) {
> dev_err(&i2c->adap.dev, "write wait timeout\n");
> ret = -EIO;
> }
Hi Wolfram,
On Sat, Apr 27, 2024 at 10:35:52PM +0200, Wolfram Sang wrote:
> There is a confusing pattern in the kernel to use a variable named 'timeout' to
there was a little checkpatch warning here for the line being
over 75 characters, but I went ahead anyway and pushed the whole
series to i2c/i2c-host.
Thanks,
Andi
> store the result of wait_for_*() 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.
>
> This is the I2C part of a tree-wide series. The rest of the patches can
> be found here (slightly WIP):
>
> git://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux.git i2c/time_left
>
> Because these patches are generated, they need manual audit. So, I will
> send them step by step. This is part 1 and also a call for opinions if
> this is a desirable change. But at least in the I2C realm, I really want
> to have it proper.
>
> Build bot is happy with these patches and I also compile tested them
> (except two). No functional changes intended.
>
> Wolfram Sang (15):
> i2c: amd-mp2-plat: use 'time_left' variable with
> wait_for_completion_timeout()
> i2c: digicolor: use 'time_left' variable with
> wait_for_completion_timeout()
> i2c: exynos5: use 'time_left' variable with
> wait_for_completion_timeout()
> i2c: hix5hd2: use 'time_left' variable with
> wait_for_completion_timeout()
> i2c: imx-lpi2c: use 'time_left' variable with
> wait_for_completion_timeout()
> i2c: omap: use 'time_left' variable with wait_for_completion_timeout()
> i2c: st: use 'time_left' variable with wait_for_completion_timeout()
> i2c: stm32f4: use 'time_left' variable with
> wait_for_completion_timeout()
> i2c: stm32f7: use 'time_left' variable with
> wait_for_completion_timeout()
> i2c: synquacer: use 'time_left' variable with
> wait_for_completion_timeout()
> i2c: jz4780: use 'time_left' variable with
> wait_for_completion_timeout()
> i2c: qcom-geni: use 'time_left' variable with
> wait_for_completion_timeout()
> i2c: rk3x: use 'time_left' variable with wait_event_timeout()
> i2c: s3c2410: use 'time_left' variable with wait_event_timeout()
> i2c: pxa: use 'time_left' variable with wait_event_timeout()
>
> --
> 2.43.0
>