| Message ID | 1434029032-8811-1-git-send-email-hofrat@osadl.org |
|---|---|
| State | Superseded |
| Headers | show |
On Thu, Jun 11, 2015 at 03:23:52PM +0200, Nicholas Mc Guire wrote: > Change wait_event_timeout() to wait_event_interruptible_timeout() allowing > signals to be received while waiting on event (task state set to > TASK_INTERRUPTIBLE) as the dev_dbg() string indicates that signals > are expected to be possible. While at it fix the type to long (as returned > by schedule_timeout(). > > Signed-off-by: Nicholas Mc Guire <hofrat@osadl.org> > --- > > From the dev_dbg string "wait interrupted" it seems that interrupts are > expected to be on - so most likely this should be > wait_event_interruptible_timeout() and not wait_event_timeout() as the > later return >= 0 always and sets the task state to TASK_UNINTERRUPTIBLE. > Someone that knows the details of this driver needs to confirm this though. Use 'git log drivers/i2c/busses/i2c-octeon.c' and you will find commit 2637e5fd232d421b680757944f613d4b1a36ae26 which disabled the signals. For further digging, you could do 'git log --grep="interruptible" drivers/i2c/busses' to find that signals cause a lot of problems with i2c transactions. Summary: The code still handling signal code simply needs to go. Thanks, Wolfram
On Thu, 11 Jun 2015, Wolfram Sang wrote: > On Thu, Jun 11, 2015 at 03:23:52PM +0200, Nicholas Mc Guire wrote: > > Change wait_event_timeout() to wait_event_interruptible_timeout() allowing > > signals to be received while waiting on event (task state set to > > TASK_INTERRUPTIBLE) as the dev_dbg() string indicates that signals > > are expected to be possible. While at it fix the type to long (as returned > > by schedule_timeout(). > > > > Signed-off-by: Nicholas Mc Guire <hofrat@osadl.org> > > --- > > > > From the dev_dbg string "wait interrupted" it seems that interrupts are > > expected to be on - so most likely this should be > > wait_event_interruptible_timeout() and not wait_event_timeout() as the > > later return >= 0 always and sets the task state to TASK_UNINTERRUPTIBLE. > > Someone that knows the details of this driver needs to confirm this though. > > Use 'git log drivers/i2c/busses/i2c-octeon.c' and you will find commit > 2637e5fd232d421b680757944f613d4b1a36ae26 which disabled the signals. > For further digging, you could do > 'git log --grep="interruptible" drivers/i2c/busses' to find that signals > cause a lot of problems with i2c transactions. > > Summary: The code still handling signal code simply needs to go. > thanks - the output of 'git log --grep="interruptible" drivers/i2c/busses' really make it very clear - sorry for the noise - I had only been looking at the code of this driver to figure out how to resolve the obvious API inconsistency but did not check the broader scope. will fix it up and resend. thx! hofrat -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/i2c/busses/i2c-octeon.c b/drivers/i2c/busses/i2c-octeon.c index 6e75e01..e25e2b8 100644 --- a/drivers/i2c/busses/i2c-octeon.c +++ b/drivers/i2c/busses/i2c-octeon.c @@ -200,19 +200,19 @@ static int octeon_i2c_test_iflg(struct octeon_i2c *i2c) */ static int octeon_i2c_wait(struct octeon_i2c *i2c) { - int result; + long result; octeon_i2c_int_enable(i2c); - result = wait_event_timeout(i2c->queue, - octeon_i2c_test_iflg(i2c), - i2c->adap.timeout); + result = wait_event_interruptible_timeout(i2c->queue, + octeon_i2c_test_iflg(i2c), + i2c->adap.timeout); octeon_i2c_int_disable(i2c); if (result < 0) { dev_dbg(i2c->dev, "%s: wait interrupted\n", __func__); - return result; + return -ERESTARTSYS; } else if (result == 0) { dev_dbg(i2c->dev, "%s: timeout\n", __func__); return -ETIMEDOUT;
Change wait_event_timeout() to wait_event_interruptible_timeout() allowing signals to be received while waiting on event (task state set to TASK_INTERRUPTIBLE) as the dev_dbg() string indicates that signals are expected to be possible. While at it fix the type to long (as returned by schedule_timeout(). Signed-off-by: Nicholas Mc Guire <hofrat@osadl.org> --- From the dev_dbg string "wait interrupted" it seems that interrupts are expected to be on - so most likely this should be wait_event_interruptible_timeout() and not wait_event_timeout() as the later return >= 0 always and sets the task state to TASK_UNINTERRUPTIBLE. Someone that knows the details of this driver needs to confirm this though. Patch was compile tested with cavium_octeon_defconfig (implies CONFIG_I2C_OCTEON=y) Patch is against 4.1-rc7 (localversion-next is -next-20150611) drivers/i2c/busses/i2c-octeon.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)