* [PATCH v2 0/4] Further i2c-pxa cleanups
@ 2020-05-11 21:09 Russell King - ARM Linux admin
2020-05-11 21:10 ` [PATCH v2 1/4] i2c: pxa: consolidate i2c_pxa_*xfer() implementations Russell King
` (3 more replies)
0 siblings, 4 replies; 9+ messages in thread
From: Russell King - ARM Linux admin @ 2020-05-11 21:09 UTC (permalink / raw)
To: linux-i2c
Hi,
This small series contains some further cleanups to i2c-pxa:
Patch 1 gets rid of the duplication between i2c_pxa_pio_xfer() and
i2c_pxa_xfer(), which are mostly identical.
Patch 2 avoids the "exhausted retries" spamming the kernel log when
running i2cdetect - this occurs when a slave does not respond. We
keep the existing behaviour for all other retries.
Patch 3 changes the kernel messages printed when timeouts happen so
they are unique - thus allowing identification of which one triggered.
Patch 4 removes an unnecessary show_state() while waiting for the
bus to become free.
v2: fix compile error in patch 1.
drivers/i2c/busses/i2c-pxa.c | 56 +++++++++++++++++++++-----------------------
1 file changed, 27 insertions(+), 29 deletions(-)
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 10.2Mbps down 587kbps up
^ permalink raw reply [flat|nested] 9+ messages in thread* [PATCH v2 1/4] i2c: pxa: consolidate i2c_pxa_*xfer() implementations 2020-05-11 21:09 [PATCH v2 0/4] Further i2c-pxa cleanups Russell King - ARM Linux admin @ 2020-05-11 21:10 ` Russell King 2020-05-12 10:35 ` Wolfram Sang 2020-05-11 21:10 ` [PATCH v2 2/4] i2c: pxa: avoid complaints with non-responsive slaves Russell King ` (2 subsequent siblings) 3 siblings, 1 reply; 9+ messages in thread From: Russell King @ 2020-05-11 21:10 UTC (permalink / raw) To: linux-i2c Most of i2c_pxa_pio_xfer() and i2c_pxa_xfer() are identical; the only differences are that i2c_pxa_pio_xfer() may reset the bus, and they use different underlying transfer functions. The retry loop is the same. Consolidate these two functions. Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk> --- drivers/i2c/busses/i2c-pxa.c | 36 ++++++++++++++++-------------------- 1 file changed, 16 insertions(+), 20 deletions(-) diff --git a/drivers/i2c/busses/i2c-pxa.c b/drivers/i2c/busses/i2c-pxa.c index c1e50c0b9756..46f1cf97d955 100644 --- a/drivers/i2c/busses/i2c-pxa.c +++ b/drivers/i2c/busses/i2c-pxa.c @@ -1102,18 +1102,20 @@ static int i2c_pxa_do_xfer(struct pxa_i2c *i2c, struct i2c_msg *msg, int num) return ret; } -static int i2c_pxa_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[], int num) +static int i2c_pxa_internal_xfer(struct pxa_i2c *i2c, + struct i2c_msg *msgs, int num, + int (*xfer)(struct pxa_i2c *, + struct i2c_msg *, int num)) { - struct pxa_i2c *i2c = adap->algo_data; int ret, i; - for (i = adap->retries; i >= 0; i--) { - ret = i2c_pxa_do_xfer(i2c, msgs, num); + for (i = i2c->adap.retries; i >= 0; i--) { + ret = xfer(i2c, msgs, num); if (ret != I2C_RETRY) goto out; if (i2c_debug) - dev_dbg(&adap->dev, "Retrying transmission\n"); + dev_dbg(&i2c->adap.dev, "Retrying transmission\n"); udelay(100); } i2c_pxa_scream_blue_murder(i2c, "exhausted retries"); @@ -1123,6 +1125,14 @@ static int i2c_pxa_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[], int num return ret; } +static int i2c_pxa_xfer(struct i2c_adapter *adap, + struct i2c_msg msgs[], int num) +{ + struct pxa_i2c *i2c = adap->algo_data; + + return i2c_pxa_internal_xfer(i2c, msgs, num, i2c_pxa_do_xfer); +} + static u32 i2c_pxa_functionality(struct i2c_adapter *adap) { return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL | @@ -1210,7 +1220,6 @@ static int i2c_pxa_pio_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[], int num) { struct pxa_i2c *i2c = adap->algo_data; - int ret, i; /* If the I2C controller is disabled we need to reset it (probably due to a suspend/resume destroying state). We do @@ -1219,20 +1228,7 @@ static int i2c_pxa_pio_xfer(struct i2c_adapter *adap, if (!(readl(_ICR(i2c)) & ICR_IUE)) i2c_pxa_reset(i2c); - for (i = adap->retries; i >= 0; i--) { - ret = i2c_pxa_do_pio_xfer(i2c, msgs, num); - if (ret != I2C_RETRY) - goto out; - - if (i2c_debug) - dev_dbg(&adap->dev, "Retrying transmission\n"); - udelay(100); - } - i2c_pxa_scream_blue_murder(i2c, "exhausted retries"); - ret = -EREMOTEIO; - out: - i2c_pxa_set_slave(i2c, ret); - return ret; + return i2c_pxa_internal_xfer(i2c, msgs, num, i2c_pxa_do_pio_xfer); } static const struct i2c_algorithm i2c_pxa_pio_algorithm = { -- 2.20.1 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH v2 1/4] i2c: pxa: consolidate i2c_pxa_*xfer() implementations 2020-05-11 21:10 ` [PATCH v2 1/4] i2c: pxa: consolidate i2c_pxa_*xfer() implementations Russell King @ 2020-05-12 10:35 ` Wolfram Sang 0 siblings, 0 replies; 9+ messages in thread From: Wolfram Sang @ 2020-05-12 10:35 UTC (permalink / raw) To: Russell King; +Cc: linux-i2c [-- Attachment #1: Type: text/plain, Size: 417 bytes --] On Mon, May 11, 2020 at 10:10:27PM +0100, Russell King wrote: > Most of i2c_pxa_pio_xfer() and i2c_pxa_xfer() are identical; the only > differences are that i2c_pxa_pio_xfer() may reset the bus, and they > use different underlying transfer functions. The retry loop is the > same. Consolidate these two functions. > > Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk> Applied to for-next, thanks! [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v2 2/4] i2c: pxa: avoid complaints with non-responsive slaves 2020-05-11 21:09 [PATCH v2 0/4] Further i2c-pxa cleanups Russell King - ARM Linux admin 2020-05-11 21:10 ` [PATCH v2 1/4] i2c: pxa: consolidate i2c_pxa_*xfer() implementations Russell King @ 2020-05-11 21:10 ` Russell King 2020-05-12 10:35 ` Wolfram Sang 2020-05-11 21:10 ` [PATCH v2 3/4] i2c: pxa: ensure timeout messages are unique Russell King 2020-05-11 21:10 ` [PATCH v2 4/4] i2c: pxa: remove some unnecessary debug Russell King 3 siblings, 1 reply; 9+ messages in thread From: Russell King @ 2020-05-11 21:10 UTC (permalink / raw) To: linux-i2c Running i2cdetect on a PXA I2C adapter is very noisy; it complains whenever a slave fails to respond to the address cycle. Since it is normal to probe for slaves in this way, we should not fill the kernel log. This is especially true with SFP modules that take a while to respond on the I2C bus, and probing via the I2C bus is the only way to detect that they are ready. Fix this by changing the internal transfer return code from I2C_RETRY to a new NO_SLAVE code (mapped to -ENXIO, as per the I2C documentation for this condition, but we still return -EREMOTEIO to the I2C stack to maintain long established driver behaviour.) Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk> --- drivers/i2c/busses/i2c-pxa.c | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/drivers/i2c/busses/i2c-pxa.c b/drivers/i2c/busses/i2c-pxa.c index 46f1cf97d955..f20f8b905793 100644 --- a/drivers/i2c/busses/i2c-pxa.c +++ b/drivers/i2c/busses/i2c-pxa.c @@ -90,6 +90,7 @@ */ #define DEF_TIMEOUT 32 +#define NO_SLAVE (-ENXIO) #define BUS_ERROR (-EREMOTEIO) #define XFER_NAKED (-ECONNREFUSED) #define I2C_RETRY (-2000) /* an error has occurred retry transmit */ @@ -881,7 +882,7 @@ static void i2c_pxa_irq_txempty(struct pxa_i2c *i2c, u32 isr) */ if (isr & ISR_ACKNAK) { if (i2c->msg_ptr == 0 && i2c->msg_idx == 0) - ret = I2C_RETRY; + ret = NO_SLAVE; else ret = XFER_NAKED; } @@ -1109,16 +1110,19 @@ static int i2c_pxa_internal_xfer(struct pxa_i2c *i2c, { int ret, i; - for (i = i2c->adap.retries; i >= 0; i--) { + for (i = 0; ; ) { ret = xfer(i2c, msgs, num); - if (ret != I2C_RETRY) + if (ret != I2C_RETRY && ret != NO_SLAVE) goto out; + if (++i >= i2c->adap.retries) + break; if (i2c_debug) dev_dbg(&i2c->adap.dev, "Retrying transmission\n"); udelay(100); } - i2c_pxa_scream_blue_murder(i2c, "exhausted retries"); + if (ret != NO_SLAVE) + i2c_pxa_scream_blue_murder(i2c, "exhausted retries"); ret = -EREMOTEIO; out: i2c_pxa_set_slave(i2c, ret); -- 2.20.1 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH v2 2/4] i2c: pxa: avoid complaints with non-responsive slaves 2020-05-11 21:10 ` [PATCH v2 2/4] i2c: pxa: avoid complaints with non-responsive slaves Russell King @ 2020-05-12 10:35 ` Wolfram Sang 0 siblings, 0 replies; 9+ messages in thread From: Wolfram Sang @ 2020-05-12 10:35 UTC (permalink / raw) To: Russell King; +Cc: linux-i2c [-- Attachment #1: Type: text/plain, Size: 825 bytes --] On Mon, May 11, 2020 at 10:10:32PM +0100, Russell King wrote: > Running i2cdetect on a PXA I2C adapter is very noisy; it complains > whenever a slave fails to respond to the address cycle. Since it is > normal to probe for slaves in this way, we should not fill the kernel > log. This is especially true with SFP modules that take a while to > respond on the I2C bus, and probing via the I2C bus is the only way to > detect that they are ready. > > Fix this by changing the internal transfer return code from I2C_RETRY > to a new NO_SLAVE code (mapped to -ENXIO, as per the I2C documentation > for this condition, but we still return -EREMOTEIO to the I2C stack to > maintain long established driver behaviour.) > > Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk> Applied to for-next, thanks! [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v2 3/4] i2c: pxa: ensure timeout messages are unique 2020-05-11 21:09 [PATCH v2 0/4] Further i2c-pxa cleanups Russell King - ARM Linux admin 2020-05-11 21:10 ` [PATCH v2 1/4] i2c: pxa: consolidate i2c_pxa_*xfer() implementations Russell King 2020-05-11 21:10 ` [PATCH v2 2/4] i2c: pxa: avoid complaints with non-responsive slaves Russell King @ 2020-05-11 21:10 ` Russell King 2020-05-12 10:35 ` Wolfram Sang 2020-05-11 21:10 ` [PATCH v2 4/4] i2c: pxa: remove some unnecessary debug Russell King 3 siblings, 1 reply; 9+ messages in thread From: Russell King @ 2020-05-11 21:10 UTC (permalink / raw) To: linux-i2c Ensure that the various timeout messages can identify where in the code they were produced from to aid debugging. Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk> --- drivers/i2c/busses/i2c-pxa.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/i2c/busses/i2c-pxa.c b/drivers/i2c/busses/i2c-pxa.c index f20f8b905793..0becab239476 100644 --- a/drivers/i2c/busses/i2c-pxa.c +++ b/drivers/i2c/busses/i2c-pxa.c @@ -1095,7 +1095,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) { - i2c_pxa_scream_blue_murder(i2c, "timeout"); + i2c_pxa_scream_blue_murder(i2c, "timeout with active message"); ret = I2C_RETRY; } @@ -1169,7 +1169,7 @@ static int i2c_pxa_pio_set_master(struct pxa_i2c *i2c) if (timeout < 0) { show_state(i2c); dev_err(&i2c->adap.dev, - "i2c_pxa: timeout waiting for bus free\n"); + "i2c_pxa: timeout waiting for bus free (set_master)\n"); return I2C_RETRY; } @@ -1213,7 +1213,7 @@ static int i2c_pxa_do_pio_xfer(struct pxa_i2c *i2c, out: if (timeout == 0) { - i2c_pxa_scream_blue_murder(i2c, "timeout"); + i2c_pxa_scream_blue_murder(i2c, "timeout (do_pio_xfer)"); ret = I2C_RETRY; } -- 2.20.1 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH v2 3/4] i2c: pxa: ensure timeout messages are unique 2020-05-11 21:10 ` [PATCH v2 3/4] i2c: pxa: ensure timeout messages are unique Russell King @ 2020-05-12 10:35 ` Wolfram Sang 0 siblings, 0 replies; 9+ messages in thread From: Wolfram Sang @ 2020-05-12 10:35 UTC (permalink / raw) To: Russell King; +Cc: linux-i2c [-- Attachment #1: Type: text/plain, Size: 281 bytes --] On Mon, May 11, 2020 at 10:10:37PM +0100, Russell King wrote: > Ensure that the various timeout messages can identify where in the code > they were produced from to aid debugging. > > Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk> Applied to for-next, thanks! [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v2 4/4] i2c: pxa: remove some unnecessary debug 2020-05-11 21:09 [PATCH v2 0/4] Further i2c-pxa cleanups Russell King - ARM Linux admin ` (2 preceding siblings ...) 2020-05-11 21:10 ` [PATCH v2 3/4] i2c: pxa: ensure timeout messages are unique Russell King @ 2020-05-11 21:10 ` Russell King 2020-05-12 10:35 ` Wolfram Sang 3 siblings, 1 reply; 9+ messages in thread From: Russell King @ 2020-05-11 21:10 UTC (permalink / raw) To: linux-i2c Remove unnecessary show_state() in the loop inside i2c_pxa_pio_set_master(), which can be unnecessarily verbose. Remove the i2c_pxa_scream_blue_murder() in i2c_pxa_pio_xfer(), which will trigger if we are probing the I2C bus and a slave does not respond; this is a normal event, and not something to report. Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk> --- drivers/i2c/busses/i2c-pxa.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/drivers/i2c/busses/i2c-pxa.c b/drivers/i2c/busses/i2c-pxa.c index 0becab239476..db739cce93ac 100644 --- a/drivers/i2c/busses/i2c-pxa.c +++ b/drivers/i2c/busses/i2c-pxa.c @@ -1161,10 +1161,8 @@ static int i2c_pxa_pio_set_master(struct pxa_i2c *i2c) /* * Wait for the bus to become free. */ - while (timeout-- && readl(_ISR(i2c)) & (ISR_IBB | ISR_UB)) { + while (timeout-- && readl(_ISR(i2c)) & (ISR_IBB | ISR_UB)) udelay(1000); - show_state(i2c); - } if (timeout < 0) { show_state(i2c); -- 2.20.1 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH v2 4/4] i2c: pxa: remove some unnecessary debug 2020-05-11 21:10 ` [PATCH v2 4/4] i2c: pxa: remove some unnecessary debug Russell King @ 2020-05-12 10:35 ` Wolfram Sang 0 siblings, 0 replies; 9+ messages in thread From: Wolfram Sang @ 2020-05-12 10:35 UTC (permalink / raw) To: Russell King; +Cc: linux-i2c [-- Attachment #1: Type: text/plain, Size: 488 bytes --] On Mon, May 11, 2020 at 10:10:42PM +0100, Russell King wrote: > Remove unnecessary show_state() in the loop inside > i2c_pxa_pio_set_master(), which can be unnecessarily verbose. > > Remove the i2c_pxa_scream_blue_murder() in i2c_pxa_pio_xfer(), which > will trigger if we are probing the I2C bus and a slave does not > respond; this is a normal event, and not something to report. > > Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk> Applied to for-next, thanks! [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2020-05-12 10:35 UTC | newest] Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-05-11 21:09 [PATCH v2 0/4] Further i2c-pxa cleanups Russell King - ARM Linux admin 2020-05-11 21:10 ` [PATCH v2 1/4] i2c: pxa: consolidate i2c_pxa_*xfer() implementations Russell King 2020-05-12 10:35 ` Wolfram Sang 2020-05-11 21:10 ` [PATCH v2 2/4] i2c: pxa: avoid complaints with non-responsive slaves Russell King 2020-05-12 10:35 ` Wolfram Sang 2020-05-11 21:10 ` [PATCH v2 3/4] i2c: pxa: ensure timeout messages are unique Russell King 2020-05-12 10:35 ` Wolfram Sang 2020-05-11 21:10 ` [PATCH v2 4/4] i2c: pxa: remove some unnecessary debug Russell King 2020-05-12 10:35 ` 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 NNTP newsgroup(s).