* [PATCH 00/18] i2c: remove printout on handled timeouts
@ 2024-04-10 11:24 Wolfram Sang
2024-04-10 11:24 ` [PATCH 01/18] i2c: at91-master: " Wolfram Sang
` (11 more replies)
0 siblings, 12 replies; 28+ messages in thread
From: Wolfram Sang @ 2024-04-10 11:24 UTC (permalink / raw
To: linux-i2c
Cc: Wolfram Sang, linux-arm-kernel, linux-arm-msm, linux-kernel,
linux-omap, linux-renesas-soc, linux-rockchip, linux-rpi-kernel,
linux-tegra
While working on another cleanup series, I stumbled over the fact that
some drivers print an error on I2C or SMBus related timeouts. This is
wrong because it may be an expected state. The client driver on top
knows this, so let's keep error handling on this level and remove the
prinouts from controller drivers.
Looking forward to comments,
Wolfram
Wolfram Sang (18):
i2c: at91-master: remove printout on handled timeouts
i2c: bcm-iproc: remove printout on handled timeouts
i2c: bcm2835: remove printout on handled timeouts
i2c: cadence: remove printout on handled timeouts
i2c: davinci: remove printout on handled timeouts
i2c: i801: remove printout on handled timeouts
i2c: img-scb: remove printout on handled timeouts
i2c: ismt: remove printout on handled timeouts
i2c: nomadik: remove printout on handled timeouts
i2c: omap: remove printout on handled timeouts
i2c: qcom-geni: remove printout on handled timeouts
i2c: qup: remove printout on handled timeouts
i2c: rk3x: remove printout on handled timeouts
i2c: sh_mobile: remove printout on handled timeouts
i2c: st: remove printout on handled timeouts
i2c: tegra: remove printout on handled timeouts
i2c: uniphier-f: remove printout on handled timeouts
i2c: uniphier: remove printout on handled timeouts
drivers/i2c/busses/i2c-at91-master.c | 1 -
drivers/i2c/busses/i2c-bcm-iproc.c | 2 --
drivers/i2c/busses/i2c-bcm2835.c | 1 -
drivers/i2c/busses/i2c-cadence.c | 2 --
drivers/i2c/busses/i2c-davinci.c | 1 -
drivers/i2c/busses/i2c-i801.c | 4 ++--
drivers/i2c/busses/i2c-img-scb.c | 5 +----
drivers/i2c/busses/i2c-ismt.c | 1 -
drivers/i2c/busses/i2c-nomadik.c | 7 ++-----
drivers/i2c/busses/i2c-omap.c | 1 -
drivers/i2c/busses/i2c-qcom-geni.c | 5 +----
drivers/i2c/busses/i2c-qup.c | 4 +---
drivers/i2c/busses/i2c-rk3x.c | 3 ---
drivers/i2c/busses/i2c-sh_mobile.c | 1 -
drivers/i2c/busses/i2c-st.c | 5 +----
drivers/i2c/busses/i2c-tegra.c | 2 --
drivers/i2c/busses/i2c-uniphier-f.c | 1 -
drivers/i2c/busses/i2c-uniphier.c | 4 +---
18 files changed, 9 insertions(+), 41 deletions(-)
--
2.43.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH 01/18] i2c: at91-master: remove printout on handled timeouts
2024-04-10 11:24 [PATCH 00/18] i2c: remove printout on handled timeouts Wolfram Sang
@ 2024-04-10 11:24 ` Wolfram Sang
2024-04-10 11:24 ` [PATCH 02/18] i2c: bcm-iproc: " Wolfram Sang
` (10 subsequent siblings)
11 siblings, 0 replies; 28+ messages in thread
From: Wolfram Sang @ 2024-04-10 11:24 UTC (permalink / raw
To: linux-i2c
Cc: Alexandre Belloni, Andi Shyti, linux-kernel, Claudiu Beznea,
Wolfram Sang, Codrin Ciubotariu, linux-arm-kernel
I2C and SMBus timeouts are not something the user needs to be informed
about on controller level. The client driver may know if that really is
a problem and give more detailed information to the user. The controller
should just pass this information upwards. Remove the printout.
Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
drivers/i2c/busses/i2c-at91-master.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/i2c/busses/i2c-at91-master.c b/drivers/i2c/busses/i2c-at91-master.c
index d311981d3e60..ee3b469ddfb9 100644
--- a/drivers/i2c/busses/i2c-at91-master.c
+++ b/drivers/i2c/busses/i2c-at91-master.c
@@ -591,7 +591,6 @@ static int at91_do_twi_transfer(struct at91_twi_dev *dev)
dev->adapter.timeout);
if (time_left == 0) {
dev->transfer_status |= at91_twi_read(dev, AT91_TWI_SR);
- dev_err(dev->dev, "controller timed out\n");
at91_init_twi_bus(dev);
ret = -ETIMEDOUT;
goto error;
--
2.43.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH 02/18] i2c: bcm-iproc: remove printout on handled timeouts
2024-04-10 11:24 [PATCH 00/18] i2c: remove printout on handled timeouts Wolfram Sang
2024-04-10 11:24 ` [PATCH 01/18] i2c: at91-master: " Wolfram Sang
@ 2024-04-10 11:24 ` Wolfram Sang
2024-04-10 11:24 ` [PATCH 03/18] i2c: bcm2835: " Wolfram Sang
` (9 subsequent siblings)
11 siblings, 0 replies; 28+ messages in thread
From: Wolfram Sang @ 2024-04-10 11:24 UTC (permalink / raw
To: linux-i2c
Cc: Wolfram Sang, Andi Shyti, Ray Jui, Scott Branden,
Broadcom internal kernel review list, linux-arm-kernel,
linux-kernel
I2C and SMBus timeouts are not something the user needs to be informed
about on controller level. The client driver may know if that really is
a problem and give more detailed information to the user. The controller
should just pass this information upwards. Remove the printout.
Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
drivers/i2c/busses/i2c-bcm-iproc.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/i2c/busses/i2c-bcm-iproc.c b/drivers/i2c/busses/i2c-bcm-iproc.c
index e905734c26a0..133d02899c6b 100644
--- a/drivers/i2c/busses/i2c-bcm-iproc.c
+++ b/drivers/i2c/busses/i2c-bcm-iproc.c
@@ -811,8 +811,6 @@ static int bcm_iproc_i2c_xfer_wait(struct bcm_iproc_i2c_dev *iproc_i2c,
}
if (!time_left && !iproc_i2c->xfer_is_done) {
- dev_err(iproc_i2c->device, "transaction timed out\n");
-
/* flush both TX/RX FIFOs */
val = BIT(M_FIFO_RX_FLUSH_SHIFT) | BIT(M_FIFO_TX_FLUSH_SHIFT);
iproc_i2c_wr_reg(iproc_i2c, M_FIFO_CTRL_OFFSET, val);
--
2.43.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH 03/18] i2c: bcm2835: remove printout on handled timeouts
2024-04-10 11:24 [PATCH 00/18] i2c: remove printout on handled timeouts Wolfram Sang
2024-04-10 11:24 ` [PATCH 01/18] i2c: at91-master: " Wolfram Sang
2024-04-10 11:24 ` [PATCH 02/18] i2c: bcm-iproc: " Wolfram Sang
@ 2024-04-10 11:24 ` Wolfram Sang
2024-04-10 11:24 ` [PATCH 04/18] i2c: cadence: " Wolfram Sang
` (8 subsequent siblings)
11 siblings, 0 replies; 28+ messages in thread
From: Wolfram Sang @ 2024-04-10 11:24 UTC (permalink / raw
To: linux-i2c
Cc: Wolfram Sang, Andi Shyti, Florian Fainelli,
Broadcom internal kernel review list, Ray Jui, Scott Branden,
linux-rpi-kernel, linux-arm-kernel, linux-kernel
I2C and SMBus timeouts are not something the user needs to be informed
about on controller level. The client driver may know if that really is
a problem and give more detailed information to the user. The controller
should just pass this information upwards. Remove the printout.
Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
drivers/i2c/busses/i2c-bcm2835.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/i2c/busses/i2c-bcm2835.c b/drivers/i2c/busses/i2c-bcm2835.c
index b92de1944221..3045ba82380d 100644
--- a/drivers/i2c/busses/i2c-bcm2835.c
+++ b/drivers/i2c/busses/i2c-bcm2835.c
@@ -370,7 +370,6 @@ static int bcm2835_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[],
if (!time_left) {
bcm2835_i2c_writel(i2c_dev, BCM2835_I2C_C,
BCM2835_I2C_C_CLEAR);
- dev_err(i2c_dev->dev, "i2c transfer timed out\n");
return -ETIMEDOUT;
}
--
2.43.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH 04/18] i2c: cadence: remove printout on handled timeouts
2024-04-10 11:24 [PATCH 00/18] i2c: remove printout on handled timeouts Wolfram Sang
` (2 preceding siblings ...)
2024-04-10 11:24 ` [PATCH 03/18] i2c: bcm2835: " Wolfram Sang
@ 2024-04-10 11:24 ` Wolfram Sang
2024-04-11 7:09 ` Michal Simek
2024-04-10 11:24 ` [PATCH 05/18] i2c: davinci: " Wolfram Sang
` (7 subsequent siblings)
11 siblings, 1 reply; 28+ messages in thread
From: Wolfram Sang @ 2024-04-10 11:24 UTC (permalink / raw
To: linux-i2c
Cc: Wolfram Sang, Michal Simek, Andi Shyti, linux-arm-kernel,
linux-kernel
I2C and SMBus timeouts are not something the user needs to be informed
about on controller level. The client driver may know if that really is
a problem and give more detailed information to the user. The controller
should just pass this information upwards. Remove the printout.
Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
drivers/i2c/busses/i2c-cadence.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/i2c/busses/i2c-cadence.c b/drivers/i2c/busses/i2c-cadence.c
index 4bb7d6756947..1b0006e3b95c 100644
--- a/drivers/i2c/busses/i2c-cadence.c
+++ b/drivers/i2c/busses/i2c-cadence.c
@@ -789,8 +789,6 @@ static int cdns_i2c_process_msg(struct cdns_i2c *id, struct i2c_msg *msg,
time_left = wait_for_completion_timeout(&id->xfer_done, msg_timeout);
if (time_left == 0) {
cdns_i2c_master_reset(adap);
- dev_err(id->adap.dev.parent,
- "timeout waiting on completion\n");
return -ETIMEDOUT;
}
--
2.43.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH 05/18] i2c: davinci: remove printout on handled timeouts
2024-04-10 11:24 [PATCH 00/18] i2c: remove printout on handled timeouts Wolfram Sang
` (3 preceding siblings ...)
2024-04-10 11:24 ` [PATCH 04/18] i2c: cadence: " Wolfram Sang
@ 2024-04-10 11:24 ` Wolfram Sang
2024-04-10 11:24 ` [PATCH 09/18] i2c: nomadik: " Wolfram Sang
` (6 subsequent siblings)
11 siblings, 0 replies; 28+ messages in thread
From: Wolfram Sang @ 2024-04-10 11:24 UTC (permalink / raw
To: linux-i2c
Cc: Wolfram Sang, Bartosz Golaszewski, Andi Shyti, linux-arm-kernel,
linux-kernel
I2C and SMBus timeouts are not something the user needs to be informed
about on controller level. The client driver may know if that really is
a problem and give more detailed information to the user. The controller
should just pass this information upwards. Remove the printout.
Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
drivers/i2c/busses/i2c-davinci.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/i2c/busses/i2c-davinci.c b/drivers/i2c/busses/i2c-davinci.c
index 02b3b1160fb0..7ae611120cfa 100644
--- a/drivers/i2c/busses/i2c-davinci.c
+++ b/drivers/i2c/busses/i2c-davinci.c
@@ -489,7 +489,6 @@ i2c_davinci_xfer_msg(struct i2c_adapter *adap, struct i2c_msg *msg, int stop)
time_left = wait_for_completion_timeout(&dev->cmd_complete,
dev->adapter.timeout);
if (!time_left) {
- dev_err(dev->dev, "controller timed out\n");
i2c_recover_bus(adap);
dev->buf_len = 0;
return -ETIMEDOUT;
--
2.43.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH 09/18] i2c: nomadik: remove printout on handled timeouts
2024-04-10 11:24 [PATCH 00/18] i2c: remove printout on handled timeouts Wolfram Sang
` (4 preceding siblings ...)
2024-04-10 11:24 ` [PATCH 05/18] i2c: davinci: " Wolfram Sang
@ 2024-04-10 11:24 ` Wolfram Sang
2024-04-12 8:39 ` Linus Walleij
2024-04-10 11:24 ` [PATCH 13/18] i2c: rk3x: " Wolfram Sang
` (5 subsequent siblings)
11 siblings, 1 reply; 28+ messages in thread
From: Wolfram Sang @ 2024-04-10 11:24 UTC (permalink / raw
To: linux-i2c
Cc: Wolfram Sang, Linus Walleij, Andi Shyti, linux-arm-kernel,
linux-kernel
I2C and SMBus timeouts are not something the user needs to be informed
about on controller level. The client driver may know if that really is
a problem and give more detailed information to the user. The controller
should just pass this information upwards. Remove the printout.
Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
drivers/i2c/busses/i2c-nomadik.c | 7 ++-----
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/drivers/i2c/busses/i2c-nomadik.c b/drivers/i2c/busses/i2c-nomadik.c
index 4f41a3c7824d..45c6df26fcbf 100644
--- a/drivers/i2c/busses/i2c-nomadik.c
+++ b/drivers/i2c/busses/i2c-nomadik.c
@@ -542,12 +542,9 @@ static int read_i2c(struct nmk_i2c_dev *priv, u16 flags)
xfer_done = nmk_i2c_wait_xfer_done(priv);
- if (!xfer_done) {
- /* Controller timed out */
- dev_err(&priv->adev->dev, "read from slave 0x%x timed out\n",
- priv->cli.slave_adr);
+ if (!xfer_done)
status = -ETIMEDOUT;
- }
+
return status;
}
--
2.43.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH 13/18] i2c: rk3x: remove printout on handled timeouts
2024-04-10 11:24 [PATCH 00/18] i2c: remove printout on handled timeouts Wolfram Sang
` (5 preceding siblings ...)
2024-04-10 11:24 ` [PATCH 09/18] i2c: nomadik: " Wolfram Sang
@ 2024-04-10 11:24 ` Wolfram Sang
2024-04-11 18:59 ` Heiko Stuebner
2024-04-12 21:12 ` Dragan Simic
2024-04-10 11:24 ` [PATCH 15/18] i2c: st: " Wolfram Sang
` (4 subsequent siblings)
11 siblings, 2 replies; 28+ messages in thread
From: Wolfram Sang @ 2024-04-10 11:24 UTC (permalink / raw
To: linux-i2c
Cc: Wolfram Sang, Heiko Stuebner, Andi Shyti, linux-arm-kernel,
linux-rockchip, linux-kernel
I2C and SMBus timeouts are not something the user needs to be informed
about on controller level. The client driver may know if that really is
a problem and give more detailed information to the user. The controller
should just pass this information upwards. Remove the printout.
Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
drivers/i2c/busses/i2c-rk3x.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/drivers/i2c/busses/i2c-rk3x.c b/drivers/i2c/busses/i2c-rk3x.c
index 086fdf262e7b..8c7367f289d3 100644
--- a/drivers/i2c/busses/i2c-rk3x.c
+++ b/drivers/i2c/busses/i2c-rk3x.c
@@ -1106,9 +1106,6 @@ static int rk3x_i2c_xfer_common(struct i2c_adapter *adap,
spin_lock_irqsave(&i2c->lock, flags);
if (timeout == 0) {
- dev_err(i2c->dev, "timeout, ipd: 0x%02x, state: %d\n",
- i2c_readl(i2c, REG_IPD), i2c->state);
-
/* 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
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH 15/18] i2c: st: remove printout on handled timeouts
2024-04-10 11:24 [PATCH 00/18] i2c: remove printout on handled timeouts Wolfram Sang
` (6 preceding siblings ...)
2024-04-10 11:24 ` [PATCH 13/18] i2c: rk3x: " Wolfram Sang
@ 2024-04-10 11:24 ` Wolfram Sang
2024-04-10 11:24 ` [PATCH 17/18] i2c: uniphier-f: " Wolfram Sang
` (3 subsequent siblings)
11 siblings, 0 replies; 28+ messages in thread
From: Wolfram Sang @ 2024-04-10 11:24 UTC (permalink / raw
To: linux-i2c
Cc: Wolfram Sang, Patrice Chotard, Andi Shyti, linux-arm-kernel,
linux-kernel
I2C and SMBus timeouts are not something the user needs to be informed
about on controller level. The client driver may know if that really is
a problem and give more detailed information to the user. The controller
should just pass this information upwards. Remove the printout.
Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
drivers/i2c/busses/i2c-st.c | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/drivers/i2c/busses/i2c-st.c b/drivers/i2c/busses/i2c-st.c
index ce2333408904..dbb93394ff94 100644
--- a/drivers/i2c/busses/i2c-st.c
+++ b/drivers/i2c/busses/i2c-st.c
@@ -689,11 +689,8 @@ static int st_i2c_xfer_msg(struct st_i2c_dev *i2c_dev, struct i2c_msg *msg,
i2c_dev->adap.timeout);
ret = c->result;
- if (!timeout) {
- dev_err(i2c_dev->dev, "Write to slave 0x%x timed out\n",
- c->addr);
+ if (!timeout)
ret = -ETIMEDOUT;
- }
i2c = SSC_I2C_STOPG | SSC_I2C_REPSTRTG;
st_i2c_clr_bits(i2c_dev->base + SSC_I2C, i2c);
--
2.43.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH 17/18] i2c: uniphier-f: remove printout on handled timeouts
2024-04-10 11:24 [PATCH 00/18] i2c: remove printout on handled timeouts Wolfram Sang
` (7 preceding siblings ...)
2024-04-10 11:24 ` [PATCH 15/18] i2c: st: " Wolfram Sang
@ 2024-04-10 11:24 ` Wolfram Sang
2024-04-10 11:24 ` [PATCH 18/18] i2c: uniphier: " Wolfram Sang
` (2 subsequent siblings)
11 siblings, 0 replies; 28+ messages in thread
From: Wolfram Sang @ 2024-04-10 11:24 UTC (permalink / raw
To: linux-i2c
Cc: Wolfram Sang, Andi Shyti, Kunihiko Hayashi, Masami Hiramatsu,
linux-arm-kernel, linux-kernel
I2C and SMBus timeouts are not something the user needs to be informed
about on controller level. The client driver may know if that really is
a problem and give more detailed information to the user. The controller
should just pass this information upwards. Remove the printout.
Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
drivers/i2c/busses/i2c-uniphier-f.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/i2c/busses/i2c-uniphier-f.c b/drivers/i2c/busses/i2c-uniphier-f.c
index dbc91c7c3788..6c3dac2cf568 100644
--- a/drivers/i2c/busses/i2c-uniphier-f.c
+++ b/drivers/i2c/busses/i2c-uniphier-f.c
@@ -358,7 +358,6 @@ static int uniphier_fi2c_master_xfer_one(struct i2c_adapter *adap,
spin_unlock_irqrestore(&priv->lock, flags);
if (!time_left) {
- dev_err(&adap->dev, "transaction timeout.\n");
uniphier_fi2c_recover(priv);
return -ETIMEDOUT;
}
--
2.43.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH 18/18] i2c: uniphier: remove printout on handled timeouts
2024-04-10 11:24 [PATCH 00/18] i2c: remove printout on handled timeouts Wolfram Sang
` (8 preceding siblings ...)
2024-04-10 11:24 ` [PATCH 17/18] i2c: uniphier-f: " Wolfram Sang
@ 2024-04-10 11:24 ` Wolfram Sang
2024-04-22 22:50 ` [PATCH 00/18] i2c: " Andi Shyti
[not found] ` <ZihNbtiVDkxgUDGk@surfacebook.localdomain>
11 siblings, 0 replies; 28+ messages in thread
From: Wolfram Sang @ 2024-04-10 11:24 UTC (permalink / raw
To: linux-i2c
Cc: Wolfram Sang, Andi Shyti, Kunihiko Hayashi, Masami Hiramatsu,
linux-arm-kernel, linux-kernel
I2C and SMBus timeouts are not something the user needs to be informed
about on controller level. The client driver may know if that really is
a problem and give more detailed information to the user. The controller
should just pass this information upwards. Remove the printout.
Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
drivers/i2c/busses/i2c-uniphier.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/i2c/busses/i2c-uniphier.c b/drivers/i2c/busses/i2c-uniphier.c
index 854ac25b5862..e1b4c80e0285 100644
--- a/drivers/i2c/busses/i2c-uniphier.c
+++ b/drivers/i2c/busses/i2c-uniphier.c
@@ -71,10 +71,8 @@ static int uniphier_i2c_xfer_byte(struct i2c_adapter *adap, u32 txdata,
writel(txdata, priv->membase + UNIPHIER_I2C_DTRM);
time_left = wait_for_completion_timeout(&priv->comp, adap->timeout);
- if (unlikely(!time_left)) {
- dev_err(&adap->dev, "transaction timeout\n");
+ if (unlikely(!time_left))
return -ETIMEDOUT;
- }
rxdata = readl(priv->membase + UNIPHIER_I2C_DREC);
if (rxdatap)
--
2.43.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 28+ messages in thread
* Re: [PATCH 04/18] i2c: cadence: remove printout on handled timeouts
2024-04-10 11:24 ` [PATCH 04/18] i2c: cadence: " Wolfram Sang
@ 2024-04-11 7:09 ` Michal Simek
0 siblings, 0 replies; 28+ messages in thread
From: Michal Simek @ 2024-04-11 7:09 UTC (permalink / raw
To: Wolfram Sang, linux-i2c; +Cc: Andi Shyti, linux-arm-kernel, linux-kernel
On 4/10/24 13:24, Wolfram Sang wrote:
> I2C and SMBus timeouts are not something the user needs to be informed
> about on controller level. The client driver may know if that really is
> a problem and give more detailed information to the user. The controller
> should just pass this information upwards. Remove the printout.
>
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
> ---
> drivers/i2c/busses/i2c-cadence.c | 2 --
> 1 file changed, 2 deletions(-)
>
> diff --git a/drivers/i2c/busses/i2c-cadence.c b/drivers/i2c/busses/i2c-cadence.c
> index 4bb7d6756947..1b0006e3b95c 100644
> --- a/drivers/i2c/busses/i2c-cadence.c
> +++ b/drivers/i2c/busses/i2c-cadence.c
> @@ -789,8 +789,6 @@ static int cdns_i2c_process_msg(struct cdns_i2c *id, struct i2c_msg *msg,
> time_left = wait_for_completion_timeout(&id->xfer_done, msg_timeout);
> if (time_left == 0) {
> cdns_i2c_master_reset(adap);
> - dev_err(id->adap.dev.parent,
> - "timeout waiting on completion\n");
> return -ETIMEDOUT;
> }
>
Acked-by: Michal Simek <michal.simek@amd.com>
Thanks,
Michal
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 13/18] i2c: rk3x: remove printout on handled timeouts
2024-04-10 11:24 ` [PATCH 13/18] i2c: rk3x: " Wolfram Sang
@ 2024-04-11 18:59 ` Heiko Stuebner
2024-04-12 21:12 ` Dragan Simic
1 sibling, 0 replies; 28+ messages in thread
From: Heiko Stuebner @ 2024-04-11 18:59 UTC (permalink / raw
To: linux-i2c, Wolfram Sang
Cc: Wolfram Sang, Andi Shyti, linux-arm-kernel, linux-rockchip,
linux-kernel
Am Mittwoch, 10. April 2024, 13:24:27 CEST schrieb Wolfram Sang:
> I2C and SMBus timeouts are not something the user needs to be informed
> about on controller level. The client driver may know if that really is
> a problem and give more detailed information to the user. The controller
> should just pass this information upwards. Remove the printout.
>
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
Acked-by: Heiko Stuebner <heiko@sntech.de>
> ---
> drivers/i2c/busses/i2c-rk3x.c | 3 ---
> 1 file changed, 3 deletions(-)
>
> diff --git a/drivers/i2c/busses/i2c-rk3x.c b/drivers/i2c/busses/i2c-rk3x.c
> index 086fdf262e7b..8c7367f289d3 100644
> --- a/drivers/i2c/busses/i2c-rk3x.c
> +++ b/drivers/i2c/busses/i2c-rk3x.c
> @@ -1106,9 +1106,6 @@ static int rk3x_i2c_xfer_common(struct i2c_adapter *adap,
> spin_lock_irqsave(&i2c->lock, flags);
>
> if (timeout == 0) {
> - dev_err(i2c->dev, "timeout, ipd: 0x%02x, state: %d\n",
> - i2c_readl(i2c, REG_IPD), i2c->state);
> -
> /* Force a STOP condition without interrupt */
> i2c_writel(i2c, 0, REG_IEN);
> val = i2c_readl(i2c, REG_CON) & REG_CON_TUNING_MASK;
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 09/18] i2c: nomadik: remove printout on handled timeouts
2024-04-10 11:24 ` [PATCH 09/18] i2c: nomadik: " Wolfram Sang
@ 2024-04-12 8:39 ` Linus Walleij
0 siblings, 0 replies; 28+ messages in thread
From: Linus Walleij @ 2024-04-12 8:39 UTC (permalink / raw
To: Wolfram Sang; +Cc: linux-i2c, Andi Shyti, linux-arm-kernel, linux-kernel
On Wed, Apr 10, 2024 at 1:25 PM Wolfram Sang
<wsa+renesas@sang-engineering.com> wrote:
> I2C and SMBus timeouts are not something the user needs to be informed
> about on controller level. The client driver may know if that really is
> a problem and give more detailed information to the user. The controller
> should just pass this information upwards. Remove the printout.
>
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
Thanks Wolfram,
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
Yours,
Linus Walleij
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 13/18] i2c: rk3x: remove printout on handled timeouts
2024-04-10 11:24 ` [PATCH 13/18] i2c: rk3x: " Wolfram Sang
2024-04-11 18:59 ` Heiko Stuebner
@ 2024-04-12 21:12 ` Dragan Simic
2024-04-13 6:38 ` Wolfram Sang
1 sibling, 1 reply; 28+ messages in thread
From: Dragan Simic @ 2024-04-12 21:12 UTC (permalink / raw
To: Wolfram Sang
Cc: linux-i2c, Heiko Stuebner, Andi Shyti, linux-arm-kernel,
linux-rockchip, linux-kernel
Hello Wolfram,
On 2024-04-10 13:24, Wolfram Sang wrote:
> I2C and SMBus timeouts are not something the user needs to be informed
> about on controller level. The client driver may know if that really is
> a problem and give more detailed information to the user. The
> controller
> should just pass this information upwards. Remove the printout.
Maybe it would be good to turn it into a debug message, instead of
simply removing it? Maybe not all client drivers handle it correctly,
in which case having an easy way for debugging would be beneficial.
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
> ---
> drivers/i2c/busses/i2c-rk3x.c | 3 ---
> 1 file changed, 3 deletions(-)
>
> diff --git a/drivers/i2c/busses/i2c-rk3x.c
> b/drivers/i2c/busses/i2c-rk3x.c
> index 086fdf262e7b..8c7367f289d3 100644
> --- a/drivers/i2c/busses/i2c-rk3x.c
> +++ b/drivers/i2c/busses/i2c-rk3x.c
> @@ -1106,9 +1106,6 @@ static int rk3x_i2c_xfer_common(struct
> i2c_adapter *adap,
> spin_lock_irqsave(&i2c->lock, flags);
>
> if (timeout == 0) {
> - dev_err(i2c->dev, "timeout, ipd: 0x%02x, state: %d\n",
> - i2c_readl(i2c, REG_IPD), i2c->state);
> -
> /* Force a STOP condition without interrupt */
> i2c_writel(i2c, 0, REG_IEN);
> val = i2c_readl(i2c, REG_CON) & REG_CON_TUNING_MASK;
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 13/18] i2c: rk3x: remove printout on handled timeouts
2024-04-12 21:12 ` Dragan Simic
@ 2024-04-13 6:38 ` Wolfram Sang
2024-04-13 6:44 ` Dragan Simic
0 siblings, 1 reply; 28+ messages in thread
From: Wolfram Sang @ 2024-04-13 6:38 UTC (permalink / raw
To: Dragan Simic
Cc: linux-i2c, Heiko Stuebner, Andi Shyti, linux-arm-kernel,
linux-rockchip, linux-kernel
[-- Attachment #1.1: Type: text/plain, Size: 274 bytes --]
> Maybe it would be good to turn it into a debug message, instead of
> simply removing it? Maybe not all client drivers handle it correctly,
> in which case having an easy way for debugging would be beneficial.
Hmm, but it still returns -ETIMEDOUT to distinguish cases?
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
[-- Attachment #2: Type: text/plain, Size: 176 bytes --]
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 13/18] i2c: rk3x: remove printout on handled timeouts
2024-04-13 6:38 ` Wolfram Sang
@ 2024-04-13 6:44 ` Dragan Simic
2024-04-13 7:10 ` Wolfram Sang
2024-04-13 7:58 ` Heiko Stübner
0 siblings, 2 replies; 28+ messages in thread
From: Dragan Simic @ 2024-04-13 6:44 UTC (permalink / raw
To: Dragan Simic
Cc: linux-i2c, Heiko Stuebner, Andi Shyti, linux-arm-kernel,
linux-rockchip, linux-kernel
On 2024-04-13 08:38, Wolfram Sang wrote:
>> Maybe it would be good to turn it into a debug message, instead of
>> simply removing it? Maybe not all client drivers handle it correctly,
>> in which case having an easy way for debugging would be beneficial.
>
> Hmm, but it still returns -ETIMEDOUT to distinguish cases?
Sure, but I think that having such an additional debug facility
can only help and save the people from adding temporary printk()s
while debugging.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 13/18] i2c: rk3x: remove printout on handled timeouts
2024-04-13 6:44 ` Dragan Simic
@ 2024-04-13 7:10 ` Wolfram Sang
2024-04-13 8:07 ` Dragan Simic
2024-04-13 7:58 ` Heiko Stübner
1 sibling, 1 reply; 28+ messages in thread
From: Wolfram Sang @ 2024-04-13 7:10 UTC (permalink / raw
To: Dragan Simic
Cc: linux-i2c, Heiko Stuebner, Andi Shyti, linux-arm-kernel,
linux-rockchip, linux-kernel
[-- Attachment #1.1: Type: text/plain, Size: 720 bytes --]
Hi Dragan,
> Sure, but I think that having such an additional debug facility
> can only help and save the people from adding temporary printk()s
> while debugging.
Mileages, I guess. I like temporary printouts for temporary debug
sessions. This way the upstream source code stays slim. In my experience
with I2C over the years, debugging happens with developers anyhow.
Logfiles from users which help developers create patches are very rare.
And those users are usually capable enough to add debug patches.
Given these experiences (which may be different in other subsystems), I
don't see why we should carry the bloat.
That said, I'll leave the final decision to the driver maintainers.
Happy hacking,
Wolfram
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
[-- Attachment #2: Type: text/plain, Size: 176 bytes --]
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 13/18] i2c: rk3x: remove printout on handled timeouts
2024-04-13 6:44 ` Dragan Simic
2024-04-13 7:10 ` Wolfram Sang
@ 2024-04-13 7:58 ` Heiko Stübner
2024-04-13 8:12 ` Dragan Simic
2024-04-13 14:35 ` Wolfram Sang
1 sibling, 2 replies; 28+ messages in thread
From: Heiko Stübner @ 2024-04-13 7:58 UTC (permalink / raw
To: Dragan Simic, Dragan Simic
Cc: linux-i2c, Andi Shyti, linux-arm-kernel, linux-rockchip,
linux-kernel
Am Samstag, 13. April 2024, 08:44:41 CEST schrieb Dragan Simic:
> On 2024-04-13 08:38, Wolfram Sang wrote:
> >> Maybe it would be good to turn it into a debug message, instead of
> >> simply removing it? Maybe not all client drivers handle it correctly,
> >> in which case having an easy way for debugging would be beneficial.
> >
> > Hmm, but it still returns -ETIMEDOUT to distinguish cases?
>
> Sure, but I think that having such an additional debug facility
> can only help and save the people from adding temporary printk()s
> while debugging.
Also we're talking about two lines of code, I wouldn't call that bloat ;-)
I was thinking about dev_dbg vs. removal too, but hadn't a clear
favorite.
So essentially Dragan is tipping the scale and I guess dev_dbg might be
the nicer way to go.
Heiko
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 13/18] i2c: rk3x: remove printout on handled timeouts
2024-04-13 7:10 ` Wolfram Sang
@ 2024-04-13 8:07 ` Dragan Simic
0 siblings, 0 replies; 28+ messages in thread
From: Dragan Simic @ 2024-04-13 8:07 UTC (permalink / raw
To: Dragan Simic
Cc: linux-i2c, Heiko Stuebner, Andi Shyti, linux-arm-kernel,
linux-rockchip, linux-kernel
Hello Wolfram,
On 2024-04-13 09:10, Wolfram Sang wrote:
> Hi Dragan,
>
>> Sure, but I think that having such an additional debug facility
>> can only help and save the people from adding temporary printk()s
>> while debugging.
>
> Mileages, I guess. I like temporary printouts for temporary debug
> sessions. This way the upstream source code stays slim. In my
> experience
> with I2C over the years, debugging happens with developers anyhow.
> Logfiles from users which help developers create patches are very rare.
> And those users are usually capable enough to add debug patches.
> Given these experiences (which may be different in other subsystems), I
> don't see why we should carry the bloat.
Yup, adding some printk()s while debugging is pretty much inevitable,
and having full debugging available would add a lot of code, regardless
of the subsystem.
> That said, I'll leave the final decision to the driver maintainers.
Agreed.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 13/18] i2c: rk3x: remove printout on handled timeouts
2024-04-13 7:58 ` Heiko Stübner
@ 2024-04-13 8:12 ` Dragan Simic
2024-04-13 14:35 ` Wolfram Sang
1 sibling, 0 replies; 28+ messages in thread
From: Dragan Simic @ 2024-04-13 8:12 UTC (permalink / raw
To: Heiko Stübner
Cc: linux-i2c, Andi Shyti, linux-arm-kernel, linux-rockchip,
linux-kernel
Hello Heiko,
On 2024-04-13 09:58, Heiko Stübner wrote:
> Am Samstag, 13. April 2024, 08:44:41 CEST schrieb Dragan Simic:
>> On 2024-04-13 08:38, Wolfram Sang wrote:
>> >> Maybe it would be good to turn it into a debug message, instead of
>> >> simply removing it? Maybe not all client drivers handle it correctly,
>> >> in which case having an easy way for debugging would be beneficial.
>> >
>> > Hmm, but it still returns -ETIMEDOUT to distinguish cases?
>>
>> Sure, but I think that having such an additional debug facility
>> can only help and save the people from adding temporary printk()s
>> while debugging.
>
> Also we're talking about two lines of code, I wouldn't call that bloat
> ;-)
> I was thinking about dev_dbg vs. removal too, but hadn't a clear
> favorite.
>
> So essentially Dragan is tipping the scale and I guess dev_dbg might be
> the nicer way to go.
Yes, the code for printing the message is already there and it's only
a couple of lines, so it might be a good idea to recycle it. :)
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 13/18] i2c: rk3x: remove printout on handled timeouts
2024-04-13 7:58 ` Heiko Stübner
2024-04-13 8:12 ` Dragan Simic
@ 2024-04-13 14:35 ` Wolfram Sang
2024-04-16 19:02 ` Andi Shyti
1 sibling, 1 reply; 28+ messages in thread
From: Wolfram Sang @ 2024-04-13 14:35 UTC (permalink / raw
To: Heiko Stübner
Cc: Dragan Simic, linux-i2c, Andi Shyti, linux-arm-kernel,
linux-rockchip, linux-kernel
[-- Attachment #1.1: Type: text/plain, Size: 568 bytes --]
> Also we're talking about two lines of code, I wouldn't call that bloat ;-)
With this patch, yes. But once you allow debug code, it is hard to draw
a line which debug is still okay and which is too fine-grained. And then
you end up with a lot. Over the years, I developed the tendency to try
to have less but meaningful error printouts. But I don't enforce it
strictly because it is too much bike-shedding discussion.
In case of this error printout here, it is just wrong. But, see, it also
came from this tendency I don't like to have printouts for every error.
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
[-- Attachment #2: Type: text/plain, Size: 176 bytes --]
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 13/18] i2c: rk3x: remove printout on handled timeouts
2024-04-13 14:35 ` Wolfram Sang
@ 2024-04-16 19:02 ` Andi Shyti
0 siblings, 0 replies; 28+ messages in thread
From: Andi Shyti @ 2024-04-16 19:02 UTC (permalink / raw
To: Wolfram Sang, Heiko Stübner, Dragan Simic, linux-i2c,
linux-arm-kernel, linux-rockchip, linux-kernel
Hi,
On Sat, Apr 13, 2024 at 04:35:06PM +0200, Wolfram Sang wrote:
> > Also we're talking about two lines of code, I wouldn't call that bloat ;-)
>
> With this patch, yes. But once you allow debug code, it is hard to draw
> a line which debug is still okay and which is too fine-grained. And then
> you end up with a lot. Over the years, I developed the tendency to try
> to have less but meaningful error printouts. But I don't enforce it
> strictly because it is too much bike-shedding discussion.
>
> In case of this error printout here, it is just wrong. But, see, it also
> came from this tendency I don't like to have printouts for every error.
I agree with Wolfram here. Debug messages are OK if they are
providing real useful information to a final product.
Besides, as I explained earlier, the patter:
dev_dbg("timed out")
return -ETIMEDOUT;
(print a debug but return error) doesn't make much sense.
But, I this is all personal preference. So that I can also leave
it to the specific maintainer.
From my side all patches in this series are r-b'ed, except for
patch 6 where there are 3 dev_dbg in a row stating the same
thing.
Thanks Dragan and Heiko for your feedback.
Andi
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 00/18] i2c: remove printout on handled timeouts
2024-04-10 11:24 [PATCH 00/18] i2c: remove printout on handled timeouts Wolfram Sang
` (9 preceding siblings ...)
2024-04-10 11:24 ` [PATCH 18/18] i2c: uniphier: " Wolfram Sang
@ 2024-04-22 22:50 ` Andi Shyti
[not found] ` <ZihNbtiVDkxgUDGk@surfacebook.localdomain>
11 siblings, 0 replies; 28+ messages in thread
From: Andi Shyti @ 2024-04-22 22:50 UTC (permalink / raw
To: Wolfram Sang
Cc: linux-i2c, linux-arm-kernel, linux-arm-msm, linux-kernel,
linux-omap, linux-renesas-soc, linux-rockchip, linux-rpi-kernel,
linux-tegra
Hi Wolfram,
On Wed, Apr 10, 2024 at 01:24:14PM +0200, Wolfram Sang wrote:
> While working on another cleanup series, I stumbled over the fact that
> some drivers print an error on I2C or SMBus related timeouts. This is
> wrong because it may be an expected state. The client driver on top
> knows this, so let's keep error handling on this level and remove the
> prinouts from controller drivers.
>
> Looking forward to comments,
>
> Wolfram
Applyed everything but patch 6 in i2c/i2c-host on
git://git.kernel.org/pub/scm/linux/kernel/git/andi.shyti/linux.git
Thank you,
Andi
Patches applied
===============
[01/18] i2c: at91-master: remove printout on handled timeouts
commit: 74cce8ed33aeac91f397d642901c94520e574f8b
[02/18] i2c: bcm-iproc: remove printout on handled timeouts
commit: 9f98914320f3e332487042aa73bbbfacc1dc9896
[03/18] i2c: bcm2835: remove printout on handled timeouts
commit: ab17612ffb60bf07e4268448e022576d42833bf7
[04/18] i2c: cadence: remove printout on handled timeouts
commit: 7aaff22d3e939c5187512188d7e27eb5e93ae41e
[05/18] i2c: davinci: remove printout on handled timeouts
commit: dc72daa5cdf1c6ffebaef0c6df1f4cdeb15cadd6
[07/18] i2c: img-scb: remove printout on handled timeouts
commit: 3e720ba5e30d6dd1b22e0f8a23f1697d438092b8
[08/18] i2c: ismt: remove printout on handled timeouts
commit: 800a297370161bda70a34cb00eb0fa2f0345b75f
[09/18] i2c: nomadik: remove printout on handled timeouts
commit: 26fbd3025cbce49cb3dd71f3a10239f69546b3c2
[10/18] i2c: omap: remove printout on handled timeouts
commit: d3f24197d8125b2bf75162ec5cc270fd68f894f4
[11/18] i2c: qcom-geni: remove printout on handled timeouts
commit: 4677d9f5c98f1c2825de142de5df08621ea340b3
[12/18] i2c: qup: remove printout on handled timeouts
commit: e28ec7512496848e8a340889c512a0167949dc8f
[13/18] i2c: rk3x: remove printout on handled timeouts
commit: 1cf7a7b3c944f727f34453a132b8899685e32f81
[14/18] i2c: sh_mobile: remove printout on handled timeouts
commit: 31fb960bf8a424c47a5bf4568685e058c9d6f24d
[15/18] i2c: st: remove printout on handled timeouts
commit: bff862e67260f779b2188e4b39c1a9f9989532ee
[16/18] i2c: tegra: remove printout on handled timeouts
commit: 5ea641d9ea5ee1b3536f8b75e658e3bf2c2a548e
[17/18] i2c: uniphier-f: remove printout on handled timeouts
commit: c31bc8e162890cda38d045e73ff0004119ab28e7
[18/18] i2c: uniphier: remove printout on handled timeouts
commit: 507a2da9539cdb839a1a2e57bfcca644bcfe0f03
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 00/18] i2c: remove printout on handled timeouts
[not found] ` <ZihNbtiVDkxgUDGk@surfacebook.localdomain>
@ 2024-04-24 9:00 ` Wolfram Sang
2024-04-24 12:41 ` Andy Shevchenko
0 siblings, 1 reply; 28+ messages in thread
From: Wolfram Sang @ 2024-04-24 9:00 UTC (permalink / raw
To: Andy Shevchenko
Cc: linux-i2c, linux-arm-kernel, linux-arm-msm, linux-kernel,
linux-omap, linux-renesas-soc, linux-rockchip, linux-rpi-kernel,
linux-tegra
[-- Attachment #1.1: Type: text/plain, Size: 1547 bytes --]
Hi Andy,
On Wed, Apr 24, 2024 at 03:08:14AM +0300, Andy Shevchenko wrote:
> Wed, Apr 10, 2024 at 01:24:14PM +0200, Wolfram Sang kirjoitti:
> > While working on another cleanup series, I stumbled over the fact that
> > some drivers print an error on I2C or SMBus related timeouts. This is
> > wrong because it may be an expected state. The client driver on top
> > knows this, so let's keep error handling on this level and remove the
> > prinouts from controller drivers.
> >
> > Looking forward to comments,
>
> I do not see an equivalent change in I²C core.
There shouldn't be. The core neither knows if it is okay or not. The
client driver knows.
> IIRC in our case (DW or i801 or iSMT) we often have this message as the only
Often? How often?
> one that points to the issues (on non-debug level), it will be much harder to
> debug for our customers with this going away.
The proper fix is to print the error in the client driver. Maybe this
needs a helper for client drivers which they can use like:
i2c_report_error(client, retval, flags);
The other thing which is also more helpful IMO is that we have
trace_events for __i2c_transfer. There, you can see what was happening
on the bus before the timeout. It can easily be that, if device X
times out, it was because of the transfer before to device Y which locks
up the bus. A simple "Bus timed out" message will not help you a lot
there.
And, keep in mind the false positives I mentioned in the coverletter.
All the best,
Wolfram
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
[-- Attachment #2: Type: text/plain, Size: 176 bytes --]
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 00/18] i2c: remove printout on handled timeouts
2024-04-24 9:00 ` Wolfram Sang
@ 2024-04-24 12:41 ` Andy Shevchenko
2024-04-24 12:44 ` Andy Shevchenko
0 siblings, 1 reply; 28+ messages in thread
From: Andy Shevchenko @ 2024-04-24 12:41 UTC (permalink / raw
To: Wolfram Sang, Andy Shevchenko, linux-i2c, linux-arm-kernel,
linux-arm-msm, linux-kernel, linux-omap, linux-renesas-soc,
linux-rockchip, linux-rpi-kernel, linux-tegra
On Wed, Apr 24, 2024 at 12:00 PM Wolfram Sang
<wsa+renesas@sang-engineering.com> wrote:
> On Wed, Apr 24, 2024 at 03:08:14AM +0300, Andy Shevchenko wrote:
> > Wed, Apr 10, 2024 at 01:24:14PM +0200, Wolfram Sang kirjoitti:
> > > While working on another cleanup series, I stumbled over the fact that
> > > some drivers print an error on I2C or SMBus related timeouts. This is
> > > wrong because it may be an expected state. The client driver on top
> > > knows this, so let's keep error handling on this level and remove the
> > > prinouts from controller drivers.
> > >
> > > Looking forward to comments,
> >
> > I do not see an equivalent change in I²C core.
>
> There shouldn't be. The core neither knows if it is okay or not. The
> client driver knows.
>
> > IIRC in our case (DW or i801 or iSMT) we often have this message as the only
>
> Often? How often?
Once in a couple of months I assume. Last time it was a few weeks ago
that there was a report and they pointed to this very message which
was helpful.
> > one that points to the issues (on non-debug level), it will be much harder to
> > debug for our customers with this going away.
>
> The proper fix is to print the error in the client driver. Maybe this
> needs a helper for client drivers which they can use like:
>
> i2c_report_error(client, retval, flags);
>
> The other thing which is also more helpful IMO is that we have
> trace_events for __i2c_transfer. There, you can see what was happening
> on the bus before the timeout. It can easily be that, if device X
> times out, it was because of the transfer before to device Y which locks
> up the bus. A simple "Bus timed out" message will not help you a lot
> there.
The trace events are good to have, not sure if production kernels have
them enabled, though.
> And, keep in mind the false positives I mentioned in the coverletter.
--
With Best Regards,
Andy Shevchenko
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 00/18] i2c: remove printout on handled timeouts
2024-04-24 12:41 ` Andy Shevchenko
@ 2024-04-24 12:44 ` Andy Shevchenko
2024-04-27 18:03 ` Wolfram Sang
0 siblings, 1 reply; 28+ messages in thread
From: Andy Shevchenko @ 2024-04-24 12:44 UTC (permalink / raw
To: Wolfram Sang, Andy Shevchenko, linux-i2c, linux-arm-kernel,
linux-arm-msm, linux-kernel, linux-omap, linux-renesas-soc,
linux-rockchip, linux-rpi-kernel, linux-tegra
On Wed, Apr 24, 2024 at 3:41 PM Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
> On Wed, Apr 24, 2024 at 12:00 PM Wolfram Sang
> <wsa+renesas@sang-engineering.com> wrote:
> > On Wed, Apr 24, 2024 at 03:08:14AM +0300, Andy Shevchenko wrote:
> > > Wed, Apr 10, 2024 at 01:24:14PM +0200, Wolfram Sang kirjoitti:
> > > > While working on another cleanup series, I stumbled over the fact that
> > > > some drivers print an error on I2C or SMBus related timeouts. This is
> > > > wrong because it may be an expected state. The client driver on top
> > > > knows this, so let's keep error handling on this level and remove the
> > > > prinouts from controller drivers.
> > > >
> > > > Looking forward to comments,
> > >
> > > I do not see an equivalent change in I²C core.
> >
> > There shouldn't be. The core neither knows if it is okay or not. The
> > client driver knows.
> >
> > > IIRC in our case (DW or i801 or iSMT) we often have this message as the only
> >
> > Often? How often?
>
> Once in a couple of months I assume. Last time it was a few weeks ago
> that there was a report and they pointed to this very message which
> was helpful.
>
> > > one that points to the issues (on non-debug level), it will be much harder to
> > > debug for our customers with this going away.
> >
> > The proper fix is to print the error in the client driver. Maybe this
> > needs a helper for client drivers which they can use like:
> >
> > i2c_report_error(client, retval, flags);
> >
> > The other thing which is also more helpful IMO is that we have
> > trace_events for __i2c_transfer. There, you can see what was happening
> > on the bus before the timeout. It can easily be that, if device X
> > times out, it was because of the transfer before to device Y which locks
> > up the bus. A simple "Bus timed out" message will not help you a lot
> > there.
>
> The trace events are good to have, not sure if production kernels have
> them enabled, though.
Ah, and to accent on the usefulness of the message that happens before
one thinks about enabling trace events. How usual is that we _expect_
something to fail? Deeper debugging usually happens after we have
noticed the issue. I'm not sure if there is an equivalent to signal
about a problem without expecting it to happen. Is that -ETIMEDOUT
being converted to some message somewhere?
> > And, keep in mind the false positives I mentioned in the coverletter.
--
With Best Regards,
Andy Shevchenko
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 00/18] i2c: remove printout on handled timeouts
2024-04-24 12:44 ` Andy Shevchenko
@ 2024-04-27 18:03 ` Wolfram Sang
0 siblings, 0 replies; 28+ messages in thread
From: Wolfram Sang @ 2024-04-27 18:03 UTC (permalink / raw
To: Andy Shevchenko
Cc: linux-i2c, linux-arm-kernel, linux-arm-msm, linux-kernel,
linux-omap, linux-renesas-soc, linux-rockchip, linux-rpi-kernel,
linux-tegra
[-- Attachment #1.1: Type: text/plain, Size: 186 bytes --]
> about a problem without expecting it to happen. Is that -ETIMEDOUT
> being converted to some message somewhere?
As said initially, the place for that is the client driver, I'd say.
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
[-- Attachment #2: Type: text/plain, Size: 176 bytes --]
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 28+ messages in thread
end of thread, other threads:[~2024-04-27 18:04 UTC | newest]
Thread overview: 28+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-04-10 11:24 [PATCH 00/18] i2c: remove printout on handled timeouts Wolfram Sang
2024-04-10 11:24 ` [PATCH 01/18] i2c: at91-master: " Wolfram Sang
2024-04-10 11:24 ` [PATCH 02/18] i2c: bcm-iproc: " Wolfram Sang
2024-04-10 11:24 ` [PATCH 03/18] i2c: bcm2835: " Wolfram Sang
2024-04-10 11:24 ` [PATCH 04/18] i2c: cadence: " Wolfram Sang
2024-04-11 7:09 ` Michal Simek
2024-04-10 11:24 ` [PATCH 05/18] i2c: davinci: " Wolfram Sang
2024-04-10 11:24 ` [PATCH 09/18] i2c: nomadik: " Wolfram Sang
2024-04-12 8:39 ` Linus Walleij
2024-04-10 11:24 ` [PATCH 13/18] i2c: rk3x: " Wolfram Sang
2024-04-11 18:59 ` Heiko Stuebner
2024-04-12 21:12 ` Dragan Simic
2024-04-13 6:38 ` Wolfram Sang
2024-04-13 6:44 ` Dragan Simic
2024-04-13 7:10 ` Wolfram Sang
2024-04-13 8:07 ` Dragan Simic
2024-04-13 7:58 ` Heiko Stübner
2024-04-13 8:12 ` Dragan Simic
2024-04-13 14:35 ` Wolfram Sang
2024-04-16 19:02 ` Andi Shyti
2024-04-10 11:24 ` [PATCH 15/18] i2c: st: " Wolfram Sang
2024-04-10 11:24 ` [PATCH 17/18] i2c: uniphier-f: " Wolfram Sang
2024-04-10 11:24 ` [PATCH 18/18] i2c: uniphier: " Wolfram Sang
2024-04-22 22:50 ` [PATCH 00/18] i2c: " Andi Shyti
[not found] ` <ZihNbtiVDkxgUDGk@surfacebook.localdomain>
2024-04-24 9:00 ` Wolfram Sang
2024-04-24 12:41 ` Andy Shevchenko
2024-04-24 12:44 ` Andy Shevchenko
2024-04-27 18:03 ` Wolfram Sang
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).