* [PATCH 1/5] i2c: octeon,thunderx: Fix set SCL recovery function
2016-09-21 6:51 [PATCH 0/5] i2c: octeon,thunderx: Recovery fixes and improvements Jan Glauber
@ 2016-09-21 6:51 ` Jan Glauber
2016-09-21 21:00 ` Wolfram Sang
2016-09-21 6:51 ` [PATCH 2/5] i2c: octeon,thunderx: Avoid sending STOP during recovery Jan Glauber
` (3 subsequent siblings)
4 siblings, 1 reply; 15+ messages in thread
From: Jan Glauber @ 2016-09-21 6:51 UTC (permalink / raw)
To: Wolfram Sang; +Cc: linux-kernel, linux-i2c, Dmitry Bazhenov, Jan Glauber
From: Dmitry Bazhenov <dmitry.bazhenov@auriga.com>
The set SCL recovery function unconditionally pulls the SCL line low.
Only pull SCL line low according to val parameter.
Signed-off-by: Dmitry Bazhenov <dmitry.bazhenov@auriga.com>
Signed-off-by: Jan Glauber <jglauber@cavium.com>
[Changed commit message]
---
drivers/i2c/busses/i2c-octeon-core.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/i2c/busses/i2c-octeon-core.c b/drivers/i2c/busses/i2c-octeon-core.c
index f45ea5e..f322242 100644
--- a/drivers/i2c/busses/i2c-octeon-core.c
+++ b/drivers/i2c/busses/i2c-octeon-core.c
@@ -767,7 +767,7 @@ static void octeon_i2c_set_scl(struct i2c_adapter *adap, int val)
{
struct octeon_i2c *i2c = i2c_get_adapdata(adap);
- octeon_i2c_write_int(i2c, TWSI_INT_SCL_OVR);
+ octeon_i2c_write_int(i2c, val ? 0 : TWSI_INT_SCL_OVR);
}
static int octeon_i2c_get_sda(struct i2c_adapter *adap)
--
1.9.1
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH 1/5] i2c: octeon,thunderx: Fix set SCL recovery function
2016-09-21 6:51 ` [PATCH 1/5] i2c: octeon,thunderx: Fix set SCL recovery function Jan Glauber
@ 2016-09-21 21:00 ` Wolfram Sang
0 siblings, 0 replies; 15+ messages in thread
From: Wolfram Sang @ 2016-09-21 21:00 UTC (permalink / raw)
To: Jan Glauber; +Cc: linux-kernel, linux-i2c, Dmitry Bazhenov
[-- Attachment #1: Type: text/plain, Size: 495 bytes --]
On Wed, Sep 21, 2016 at 08:51:02AM +0200, Jan Glauber wrote:
> From: Dmitry Bazhenov <dmitry.bazhenov@auriga.com>
>
> The set SCL recovery function unconditionally pulls the SCL line low.
> Only pull SCL line low according to val parameter.
>
> Signed-off-by: Dmitry Bazhenov <dmitry.bazhenov@auriga.com>
> Signed-off-by: Jan Glauber <jglauber@cavium.com>
> [Changed commit message]
Applied to for-next, thanks!
Minor nit: please only use "octeon:" if you change octeon-core.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 2/5] i2c: octeon,thunderx: Avoid sending STOP during recovery
2016-09-21 6:51 [PATCH 0/5] i2c: octeon,thunderx: Recovery fixes and improvements Jan Glauber
2016-09-21 6:51 ` [PATCH 1/5] i2c: octeon,thunderx: Fix set SCL recovery function Jan Glauber
@ 2016-09-21 6:51 ` Jan Glauber
2016-09-21 21:00 ` Wolfram Sang
2016-09-21 6:51 ` [PATCH 3/5] i2c: octeon,thunderx: Fix high-level controller status check Jan Glauber
` (2 subsequent siblings)
4 siblings, 1 reply; 15+ messages in thread
From: Jan Glauber @ 2016-09-21 6:51 UTC (permalink / raw)
To: Wolfram Sang; +Cc: linux-kernel, linux-i2c, Dmitry Bazhenov, Jan Glauber
From: Dmitry Bazhenov <dmitry.bazhenov@auriga.com>
Due to a bug in the ThunderX I2C hardware sending STOP during
a recovery attempt could lock up the hardware. To work around
this problem do not send STOP at the beginning of the recovery
but use the override registers to bring the TWSI including
the high-level controller out of the bad state.
Signed-off-by: Dmitry Bazhenov <dmitry.bazhenov@auriga.com>
Signed-off-by: Jan Glauber <jglauber@cavium.com>
[Changed commit message]
---
drivers/i2c/busses/i2c-octeon-core.c | 18 ++++++++++++++----
1 file changed, 14 insertions(+), 4 deletions(-)
diff --git a/drivers/i2c/busses/i2c-octeon-core.c b/drivers/i2c/busses/i2c-octeon-core.c
index f322242..7d4df83 100644
--- a/drivers/i2c/busses/i2c-octeon-core.c
+++ b/drivers/i2c/busses/i2c-octeon-core.c
@@ -783,13 +783,14 @@ static void octeon_i2c_prepare_recovery(struct i2c_adapter *adap)
{
struct octeon_i2c *i2c = i2c_get_adapdata(adap);
+ octeon_i2c_hlc_disable(i2c);
+
/*
- * The stop resets the state machine, does not _transmit_ STOP unless
- * engine was active.
+ * Bring control register to a good state regardless
+ * of HLC state.
*/
- octeon_i2c_stop(i2c);
+ octeon_i2c_ctl_write(i2c, TWSI_CTL_ENAB);
- octeon_i2c_hlc_disable(i2c);
octeon_i2c_write_int(i2c, 0);
}
@@ -797,6 +798,15 @@ static void octeon_i2c_unprepare_recovery(struct i2c_adapter *adap)
{
struct octeon_i2c *i2c = i2c_get_adapdata(adap);
+ /*
+ * Generate STOP to finish the unfinished transaction.
+ * Can't generate STOP via the TWSI CTL register
+ * since it could bring the TWSI controller into an inoperable state.
+ */
+ octeon_i2c_write_int(i2c, TWSI_INT_SDA_OVR | TWSI_INT_SCL_OVR);
+ udelay(5);
+ octeon_i2c_write_int(i2c, TWSI_INT_SDA_OVR);
+ udelay(5);
octeon_i2c_write_int(i2c, 0);
}
--
1.9.1
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH 2/5] i2c: octeon,thunderx: Avoid sending STOP during recovery
2016-09-21 6:51 ` [PATCH 2/5] i2c: octeon,thunderx: Avoid sending STOP during recovery Jan Glauber
@ 2016-09-21 21:00 ` Wolfram Sang
0 siblings, 0 replies; 15+ messages in thread
From: Wolfram Sang @ 2016-09-21 21:00 UTC (permalink / raw)
To: Jan Glauber; +Cc: linux-kernel, linux-i2c, Dmitry Bazhenov
[-- Attachment #1: Type: text/plain, Size: 610 bytes --]
On Wed, Sep 21, 2016 at 08:51:03AM +0200, Jan Glauber wrote:
> From: Dmitry Bazhenov <dmitry.bazhenov@auriga.com>
>
> Due to a bug in the ThunderX I2C hardware sending STOP during
> a recovery attempt could lock up the hardware. To work around
> this problem do not send STOP at the beginning of the recovery
> but use the override registers to bring the TWSI including
> the high-level controller out of the bad state.
>
> Signed-off-by: Dmitry Bazhenov <dmitry.bazhenov@auriga.com>
> Signed-off-by: Jan Glauber <jglauber@cavium.com>
> [Changed commit message]
Applied to for-next, thanks!
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 3/5] i2c: octeon,thunderx: Fix high-level controller status check
2016-09-21 6:51 [PATCH 0/5] i2c: octeon,thunderx: Recovery fixes and improvements Jan Glauber
2016-09-21 6:51 ` [PATCH 1/5] i2c: octeon,thunderx: Fix set SCL recovery function Jan Glauber
2016-09-21 6:51 ` [PATCH 2/5] i2c: octeon,thunderx: Avoid sending STOP during recovery Jan Glauber
@ 2016-09-21 6:51 ` Jan Glauber
2016-09-21 21:01 ` Wolfram Sang
2016-09-21 6:51 ` [PATCH 4/5] i2c: octeon,thunderx: Check bus state before starting a transaction Jan Glauber
2016-09-21 6:51 ` [PATCH 5/5] i2c: octeon,thunderx: Limit register access retries Jan Glauber
4 siblings, 1 reply; 15+ messages in thread
From: Jan Glauber @ 2016-09-21 6:51 UTC (permalink / raw)
To: Wolfram Sang; +Cc: linux-kernel, linux-i2c, Jan Glauber
In case the high-level controller (HLC) is used the status code is
reported at a different location. Check that location after HLC
write operations if the ready bit is not set and return an appropriate
error code instead of always returning -EAGAIN.
Signed-off-by: Jan Glauber <jglauber@cavium.com>
---
drivers/i2c/busses/i2c-octeon-core.c | 23 ++++++++++++++---------
1 file changed, 14 insertions(+), 9 deletions(-)
diff --git a/drivers/i2c/busses/i2c-octeon-core.c b/drivers/i2c/busses/i2c-octeon-core.c
index 7d4df83..5e63b17 100644
--- a/drivers/i2c/busses/i2c-octeon-core.c
+++ b/drivers/i2c/busses/i2c-octeon-core.c
@@ -215,7 +215,16 @@ static int octeon_i2c_hlc_wait(struct octeon_i2c *i2c)
static int octeon_i2c_check_status(struct octeon_i2c *i2c, int final_read)
{
- u8 stat = octeon_i2c_stat_read(i2c);
+ u8 stat;
+
+ /*
+ * This is ugly... in HLC mode the status is not in the status register
+ * but in the lower 8 bits of SW_TWSI.
+ */
+ if (i2c->hlc_enabled)
+ stat = __raw_readq(i2c->twsi_base + SW_TWSI(i2c));
+ else
+ stat = octeon_i2c_stat_read(i2c);
switch (stat) {
/* Everything is fine */
@@ -453,7 +462,7 @@ static int octeon_i2c_hlc_read(struct octeon_i2c *i2c, struct i2c_msg *msgs)
cmd = __raw_readq(i2c->twsi_base + SW_TWSI(i2c));
if ((cmd & SW_TWSI_R) == 0)
- return -EAGAIN;
+ return octeon_i2c_check_status(i2c, false);
for (i = 0, j = msgs[0].len - 1; i < msgs[0].len && i < 4; i++, j--)
msgs[0].buf[j] = (cmd >> (8 * i)) & 0xff;
@@ -506,9 +515,7 @@ static int octeon_i2c_hlc_write(struct octeon_i2c *i2c, struct i2c_msg *msgs)
cmd = __raw_readq(i2c->twsi_base + SW_TWSI(i2c));
if ((cmd & SW_TWSI_R) == 0)
- return -EAGAIN;
-
- ret = octeon_i2c_check_status(i2c, false);
+ return octeon_i2c_check_status(i2c, false);
err:
return ret;
@@ -553,7 +560,7 @@ static int octeon_i2c_hlc_comp_read(struct octeon_i2c *i2c, struct i2c_msg *msgs
cmd = __raw_readq(i2c->twsi_base + SW_TWSI(i2c));
if ((cmd & SW_TWSI_R) == 0)
- return -EAGAIN;
+ return octeon_i2c_check_status(i2c, false);
for (i = 0, j = msgs[1].len - 1; i < msgs[1].len && i < 4; i++, j--)
msgs[1].buf[j] = (cmd >> (8 * i)) & 0xff;
@@ -617,9 +624,7 @@ static int octeon_i2c_hlc_comp_write(struct octeon_i2c *i2c, struct i2c_msg *msg
cmd = __raw_readq(i2c->twsi_base + SW_TWSI(i2c));
if ((cmd & SW_TWSI_R) == 0)
- return -EAGAIN;
-
- ret = octeon_i2c_check_status(i2c, false);
+ return octeon_i2c_check_status(i2c, false);
err:
return ret;
--
1.9.1
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH 3/5] i2c: octeon,thunderx: Fix high-level controller status check
2016-09-21 6:51 ` [PATCH 3/5] i2c: octeon,thunderx: Fix high-level controller status check Jan Glauber
@ 2016-09-21 21:01 ` Wolfram Sang
0 siblings, 0 replies; 15+ messages in thread
From: Wolfram Sang @ 2016-09-21 21:01 UTC (permalink / raw)
To: Jan Glauber; +Cc: linux-kernel, linux-i2c
[-- Attachment #1: Type: text/plain, Size: 414 bytes --]
On Wed, Sep 21, 2016 at 08:51:04AM +0200, Jan Glauber wrote:
> In case the high-level controller (HLC) is used the status code is
> reported at a different location. Check that location after HLC
> write operations if the ready bit is not set and return an appropriate
> error code instead of always returning -EAGAIN.
>
> Signed-off-by: Jan Glauber <jglauber@cavium.com>
Applied to for-next, thanks!
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 4/5] i2c: octeon,thunderx: Check bus state before starting a transaction
2016-09-21 6:51 [PATCH 0/5] i2c: octeon,thunderx: Recovery fixes and improvements Jan Glauber
` (2 preceding siblings ...)
2016-09-21 6:51 ` [PATCH 3/5] i2c: octeon,thunderx: Fix high-level controller status check Jan Glauber
@ 2016-09-21 6:51 ` Jan Glauber
2016-09-21 20:55 ` Wolfram Sang
2016-09-21 6:51 ` [PATCH 5/5] i2c: octeon,thunderx: Limit register access retries Jan Glauber
4 siblings, 1 reply; 15+ messages in thread
From: Jan Glauber @ 2016-09-21 6:51 UTC (permalink / raw)
To: Wolfram Sang; +Cc: linux-kernel, linux-i2c, Jan Glauber
Add an additional status check before starting a transaction and,
if required, trigger the recovery if the check fails.
Signed-off-by: Jan Glauber <jglauber@cavium.com>
---
drivers/i2c/busses/i2c-octeon-core.c | 20 ++++++++++++++++++++
1 file changed, 20 insertions(+)
diff --git a/drivers/i2c/busses/i2c-octeon-core.c b/drivers/i2c/busses/i2c-octeon-core.c
index 5e63b17..b21aa3e 100644
--- a/drivers/i2c/busses/i2c-octeon-core.c
+++ b/drivers/i2c/busses/i2c-octeon-core.c
@@ -630,6 +630,22 @@ static int octeon_i2c_hlc_comp_write(struct octeon_i2c *i2c, struct i2c_msg *msg
return ret;
}
+static int octeon_i2c_check_bus(struct octeon_i2c *i2c)
+{
+ int stat, lines;
+
+ stat = octeon_i2c_stat_read(i2c);
+
+ /* get I2C line state */
+ lines = octeon_i2c_read_int(i2c) & (TWSI_INT_SCL | TWSI_INT_SDA);
+
+ if (stat == STAT_IDLE && lines == (TWSI_INT_SCL | TWSI_INT_SDA))
+ return 0;
+
+ /* bus check failed, try to recover */
+ return octeon_i2c_recovery(i2c);
+}
+
/**
* octeon_i2c_xfer - The driver's master_xfer function
* @adap: Pointer to the i2c_adapter structure
@@ -643,6 +659,10 @@ int octeon_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs, int num)
struct octeon_i2c *i2c = i2c_get_adapdata(adap);
int i, ret = 0;
+ ret = octeon_i2c_check_bus(i2c);
+ if (ret)
+ goto out;
+
if (num == 1) {
if (msgs[0].len > 0 && msgs[0].len <= 8) {
if (msgs[0].flags & I2C_M_RD)
--
1.9.1
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH 4/5] i2c: octeon,thunderx: Check bus state before starting a transaction
2016-09-21 6:51 ` [PATCH 4/5] i2c: octeon,thunderx: Check bus state before starting a transaction Jan Glauber
@ 2016-09-21 20:55 ` Wolfram Sang
2016-09-22 16:08 ` Jan Glauber
0 siblings, 1 reply; 15+ messages in thread
From: Wolfram Sang @ 2016-09-21 20:55 UTC (permalink / raw)
To: Jan Glauber; +Cc: linux-kernel, linux-i2c
[-- Attachment #1: Type: text/plain, Size: 287 bytes --]
On Wed, Sep 21, 2016 at 08:51:05AM +0200, Jan Glauber wrote:
> Add an additional status check before starting a transaction and,
> if required, trigger the recovery if the check fails.
>
> Signed-off-by: Jan Glauber <jglauber@cavium.com>
Won't this break multi-master setups?
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 4/5] i2c: octeon,thunderx: Check bus state before starting a transaction
2016-09-21 20:55 ` Wolfram Sang
@ 2016-09-22 16:08 ` Jan Glauber
0 siblings, 0 replies; 15+ messages in thread
From: Jan Glauber @ 2016-09-22 16:08 UTC (permalink / raw)
To: Wolfram Sang; +Cc: linux-kernel, linux-i2c, Bazhenov, Dmitry
On Wed, Sep 21, 2016 at 10:55:41PM +0200, Wolfram Sang wrote:
> On Wed, Sep 21, 2016 at 08:51:05AM +0200, Jan Glauber wrote:
> > Add an additional status check before starting a transaction and,
> > if required, trigger the recovery if the check fails.
> >
> > Signed-off-by: Jan Glauber <jglauber@cavium.com>
>
> Won't this break multi-master setups?
>
I'm afraid yes. I don't have a multi-master setup, but agree that we
should not break it.
How about re-checking if the bus is idle until a timeout
(i2c->adap.timeout ?) happens and only then recover?
Additionaly we can check for arbitration loss as Dmitry did in his
original patch.
--Jan
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 4/5] i2c: octeon,thunderx: Check bus state before starting a transaction
@ 2016-09-22 16:08 ` Jan Glauber
0 siblings, 0 replies; 15+ messages in thread
From: Jan Glauber @ 2016-09-22 16:08 UTC (permalink / raw)
To: Wolfram Sang; +Cc: linux-kernel, linux-i2c, Bazhenov, Dmitry
On Wed, Sep 21, 2016 at 10:55:41PM +0200, Wolfram Sang wrote:
> On Wed, Sep 21, 2016 at 08:51:05AM +0200, Jan Glauber wrote:
> > Add an additional status check before starting a transaction and,
> > if required, trigger the recovery if the check fails.
> >
> > Signed-off-by: Jan Glauber <jglauber@cavium.com>
>
> Won't this break multi-master setups?
>
I'm afraid yes. I don't have a multi-master setup, but agree that we
should not break it.
How about re-checking if the bus is idle until a timeout
(i2c->adap.timeout ?) happens and only then recover?
Additionaly we can check for arbitration loss as Dmitry did in his
original patch.
--Jan
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 5/5] i2c: octeon,thunderx: Limit register access retries
2016-09-21 6:51 [PATCH 0/5] i2c: octeon,thunderx: Recovery fixes and improvements Jan Glauber
` (3 preceding siblings ...)
2016-09-21 6:51 ` [PATCH 4/5] i2c: octeon,thunderx: Check bus state before starting a transaction Jan Glauber
@ 2016-09-21 6:51 ` Jan Glauber
2016-09-21 21:03 ` Wolfram Sang
4 siblings, 1 reply; 15+ messages in thread
From: Jan Glauber @ 2016-09-21 6:51 UTC (permalink / raw)
To: Wolfram Sang; +Cc: linux-kernel, linux-i2c, Jan Glauber
Do not infinitely retry register readq and writeq operations
in order to not lock up the CPU in case the TWSI gets stuck.
Return -EIO in case of a failed data read. For all other
cases just return so subsequent operations will fail
and trigger the recovery.
Signed-off-by: Jan Glauber <jglauber@cavium.com>
---
drivers/i2c/busses/i2c-octeon-core.c | 4 +++-
drivers/i2c/busses/i2c-octeon-core.h | 21 ++++++++++++++++-----
2 files changed, 19 insertions(+), 6 deletions(-)
diff --git a/drivers/i2c/busses/i2c-octeon-core.c b/drivers/i2c/busses/i2c-octeon-core.c
index b21aa3e..6ba0146 100644
--- a/drivers/i2c/busses/i2c-octeon-core.c
+++ b/drivers/i2c/busses/i2c-octeon-core.c
@@ -381,7 +381,9 @@ static int octeon_i2c_read(struct octeon_i2c *i2c, int target,
if (result)
return result;
- data[i] = octeon_i2c_data_read(i2c);
+ data[i] = octeon_i2c_data_read(i2c, &result);
+ if (result)
+ return result;
if (recv_len && i == 0) {
if (data[i] > I2C_SMBUS_BLOCK_MAX + 1)
return -EPROTO;
diff --git a/drivers/i2c/busses/i2c-octeon-core.h b/drivers/i2c/busses/i2c-octeon-core.h
index 87151ea..177d4cc 100644
--- a/drivers/i2c/busses/i2c-octeon-core.h
+++ b/drivers/i2c/busses/i2c-octeon-core.h
@@ -141,11 +141,14 @@ static inline void octeon_i2c_writeq_flush(u64 val, void __iomem *addr)
*/
static inline void octeon_i2c_reg_write(struct octeon_i2c *i2c, u64 eop_reg, u8 data)
{
+ int tries = 100;
u64 tmp;
__raw_writeq(SW_TWSI_V | eop_reg | data, i2c->twsi_base + SW_TWSI(i2c));
do {
tmp = __raw_readq(i2c->twsi_base + SW_TWSI(i2c));
+ if (--tries < 0)
+ return;
} while ((tmp & SW_TWSI_V) != 0);
}
@@ -163,24 +166,32 @@ static inline void octeon_i2c_reg_write(struct octeon_i2c *i2c, u64 eop_reg, u8
*
* The I2C core registers are accessed indirectly via the SW_TWSI CSR.
*/
-static inline u8 octeon_i2c_reg_read(struct octeon_i2c *i2c, u64 eop_reg)
+static inline int octeon_i2c_reg_read(struct octeon_i2c *i2c, u64 eop_reg,
+ int *error)
{
+ int tries = 100;
u64 tmp;
__raw_writeq(SW_TWSI_V | eop_reg | SW_TWSI_R, i2c->twsi_base + SW_TWSI(i2c));
do {
tmp = __raw_readq(i2c->twsi_base + SW_TWSI(i2c));
+ if (--tries < 0) {
+ /* signal that the returned data is invalid */
+ if (error)
+ *error = -EIO;
+ return 0;
+ }
} while ((tmp & SW_TWSI_V) != 0);
return tmp & 0xFF;
}
#define octeon_i2c_ctl_read(i2c) \
- octeon_i2c_reg_read(i2c, SW_TWSI_EOP_TWSI_CTL)
-#define octeon_i2c_data_read(i2c) \
- octeon_i2c_reg_read(i2c, SW_TWSI_EOP_TWSI_DATA)
+ octeon_i2c_reg_read(i2c, SW_TWSI_EOP_TWSI_CTL, NULL)
+#define octeon_i2c_data_read(i2c, error) \
+ octeon_i2c_reg_read(i2c, SW_TWSI_EOP_TWSI_DATA, error)
#define octeon_i2c_stat_read(i2c) \
- octeon_i2c_reg_read(i2c, SW_TWSI_EOP_TWSI_STAT)
+ octeon_i2c_reg_read(i2c, SW_TWSI_EOP_TWSI_STAT, NULL)
/**
* octeon_i2c_read_int - read the TWSI_INT register
--
1.9.1
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH 5/5] i2c: octeon,thunderx: Limit register access retries
2016-09-21 6:51 ` [PATCH 5/5] i2c: octeon,thunderx: Limit register access retries Jan Glauber
@ 2016-09-21 21:03 ` Wolfram Sang
2016-09-22 16:40 ` Jan Glauber
0 siblings, 1 reply; 15+ messages in thread
From: Wolfram Sang @ 2016-09-21 21:03 UTC (permalink / raw)
To: Jan Glauber; +Cc: linux-kernel, linux-i2c
[-- Attachment #1: Type: text/plain, Size: 490 bytes --]
On Wed, Sep 21, 2016 at 08:51:06AM +0200, Jan Glauber wrote:
> Do not infinitely retry register readq and writeq operations
> in order to not lock up the CPU in case the TWSI gets stuck.
>
> Return -EIO in case of a failed data read. For all other
> cases just return so subsequent operations will fail
> and trigger the recovery.
>
> Signed-off-by: Jan Glauber <jglauber@cavium.com>
I didn't really check, but have you considered using
readq_poll_timeout() from iopoll.h?
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 5/5] i2c: octeon,thunderx: Limit register access retries
2016-09-21 21:03 ` Wolfram Sang
@ 2016-09-22 16:40 ` Jan Glauber
0 siblings, 0 replies; 15+ messages in thread
From: Jan Glauber @ 2016-09-22 16:40 UTC (permalink / raw)
To: Wolfram Sang; +Cc: linux-kernel, linux-i2c, Bazhenov, Dmitry
On Wed, Sep 21, 2016 at 11:03:35PM +0200, Wolfram Sang wrote:
> On Wed, Sep 21, 2016 at 08:51:06AM +0200, Jan Glauber wrote:
> > Do not infinitely retry register readq and writeq operations
> > in order to not lock up the CPU in case the TWSI gets stuck.
> >
> > Return -EIO in case of a failed data read. For all other
> > cases just return so subsequent operations will fail
> > and trigger the recovery.
> >
> > Signed-off-by: Jan Glauber <jglauber@cavium.com>
>
> I didn't really check, but have you considered using
> readq_poll_timeout() from iopoll.h?
>
Indeed, readq_poll_timeout() fits quite well here. It will lose some cycles
on mips but I'm not convinced that matters with i2c.
That would be the first user of readq_poll_timeout() in the kernel :)
--Jan
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 5/5] i2c: octeon,thunderx: Limit register access retries
@ 2016-09-22 16:40 ` Jan Glauber
0 siblings, 0 replies; 15+ messages in thread
From: Jan Glauber @ 2016-09-22 16:40 UTC (permalink / raw)
To: Wolfram Sang; +Cc: linux-kernel, linux-i2c, Bazhenov, Dmitry
On Wed, Sep 21, 2016 at 11:03:35PM +0200, Wolfram Sang wrote:
> On Wed, Sep 21, 2016 at 08:51:06AM +0200, Jan Glauber wrote:
> > Do not infinitely retry register readq and writeq operations
> > in order to not lock up the CPU in case the TWSI gets stuck.
> >
> > Return -EIO in case of a failed data read. For all other
> > cases just return so subsequent operations will fail
> > and trigger the recovery.
> >
> > Signed-off-by: Jan Glauber <jglauber@cavium.com>
>
> I didn't really check, but have you considered using
> readq_poll_timeout() from iopoll.h?
>
Indeed, readq_poll_timeout() fits quite well here. It will lose some cycles
on mips but I'm not convinced that matters with i2c.
That would be the first user of readq_poll_timeout() in the kernel :)
--Jan
^ permalink raw reply [flat|nested] 15+ messages in thread