[PATCH] i2c-mpc: do not allow interruptions when waiting for I2C to complete
The i2c_wait() function is using wait_event_interruptible_timeout() to wait for
the I2C controller to signal that it has completed an I2C bus operation. If
the process that causes the I2C operation terminated abruptly, the wait will
be interrupted, returning an error. It is better to let the I2C operation
finished before the process exits.
It is safe to use wait_event_timeout() instead, because the timeout will allow
the process to exit if the I2C bus hangs. It's also better to allow the
I2C operation to finish, because unacknowledged I2C operations can cause the
I2C bus to hang.
Signed-off-by: Timur Tabi
---
A similar change should probably be done to i2c-cpm.c, and maybe all other
I2C drivers. Not many use wait_event_interruptible_timeout().
drivers/i2c/busses/i2c-mpc.c |9 +++--
1 files changed, 3 insertions(+), 6 deletions(-)
diff --git a/drivers/i2c/busses/i2c-mpc.c b/drivers/i2c/busses/i2c-mpc.c
index a9a45fc..c0ace48 100644
--- a/drivers/i2c/busses/i2c-mpc.c
+++ b/drivers/i2c/busses/i2c-mpc.c
@@ -70,7 +70,7 @@ static irqreturn_t mpc_i2c_isr(int irq, void *dev_id)
/* Read again to allow register to stabilise */
i2c->interrupt = readb(i2c->base + MPC_I2C_SR);
writeb(0, i2c->base + MPC_I2C_SR);
- wake_up_interruptible(&i2c->queue);
+ wake_up(&i2c->queue);
}
return IRQ_HANDLED;
}
@@ -115,13 +115,10 @@ static int i2c_wait(struct mpc_i2c *i2c, unsigned
timeout, int writing)
writeb(0, i2c->base + MPC_I2C_SR);
} else {
/* Interrupt mode */
- result = wait_event_interruptible_timeout(i2c->queue,
+ result = wait_event_timeout(i2c->queue,
(i2c->interrupt & CSR_MIF), timeout * HZ);
- if (unlikely(result < 0)) {
- pr_debug("I2C: wait interrupted\n");
- writeccr(i2c, 0);
- } else if (unlikely(!(i2c->interrupt & CSR_MIF))) {
+ if (unlikely(!(i2c->interrupt & CSR_MIF))) {
pr_debug("I2C: wait timeout\n");
writeccr(i2c, 0);
result = -ETIMEDOUT;
--
1.5.5
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH] i2c-mpc: do not allow interruptions when waiting for I2C to complete
On Fri, Feb 6, 2009 at 8:00 AM, Timur Tabi wrote: > The i2c_wait() function is using wait_event_interruptible_timeout() to wait > for > the I2C controller to signal that it has completed an I2C bus operation. If > the process that causes the I2C operation terminated abruptly, the wait will > be interrupted, returning an error. It is better to let the I2C operation > finished before the process exits. > > It is safe to use wait_event_timeout() instead, because the timeout will allow > the process to exit if the I2C bus hangs. It's also better to allow the > I2C operation to finish, because unacknowledged I2C operations can cause the > I2C bus to hang. > > Signed-off-by: Timur Tabi Jean, Could you pick up this patch for 2.6.30? -- Timur Tabi Linux kernel developer at Freescale ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH] i2c-mpc: do not allow interruptions when waiting for I2C to complete
On Fri, Feb 06, 2009 at 08:00:37AM -0600, Timur Tabi wrote: > The i2c_wait() function is using wait_event_interruptible_timeout() to wait > for > the I2C controller to signal that it has completed an I2C bus operation. If > the process that causes the I2C operation terminated abruptly, the wait will > be interrupted, returning an error. It is better to let the I2C operation > finished before the process exits. > > It is safe to use wait_event_timeout() instead, because the timeout will allow > the process to exit if the I2C bus hangs. It's also better to allow the > I2C operation to finish, because unacknowledged I2C operations can cause the > I2C bus to hang. > > Signed-off-by: Timur Tabi Acked-by: Wolfram Sang -- Pengutronix e.K. | Wolfram Sang| Industrial Linux Solutions | http://www.pengutronix.de/ | signature.asc Description: Digital signature ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH] i2c-mpc: do not allow interruptions when waiting for I2C to complete
On Tue, 10 Feb 2009 08:59:57 -0600, Timur Tabi wrote: > On Fri, Feb 6, 2009 at 8:00 AM, Timur Tabi wrote: > > The i2c_wait() function is using wait_event_interruptible_timeout() to wait > > for > > the I2C controller to signal that it has completed an I2C bus operation. If > > the process that causes the I2C operation terminated abruptly, the wait will > > be interrupted, returning an error. It is better to let the I2C operation > > finished before the process exits. > > > > It is safe to use wait_event_timeout() instead, because the timeout will > > allow > > the process to exit if the I2C bus hangs. It's also better to allow the > > I2C operation to finish, because unacknowledged I2C operations can cause the > > I2C bus to hang. > > > > Signed-off-by: Timur Tabi > > Jean, > > Could you pick up this patch for 2.6.30? No, that's something for either Ben Dooks (Cc'd) or the powerpc tree. -- Jean Delvare ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH] i2c-mpc: do not allow interruptions when waiting for I2C to complete
Jean Delvare wrote: > No, that's something for either Ben Dooks (Cc'd) or the powerpc tree. This patch has nothing to do with ARM, so Kumar will pick it up, if you ACK it. -- Timur Tabi Linux kernel developer at Freescale ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH] i2c-mpc: do not allow interruptions when waiting for I2C to complete
On Tue, 10 Feb 2009 10:01:54 -0600, Timur Tabi wrote: > Jean Delvare wrote: > > > No, that's something for either Ben Dooks (Cc'd) or the powerpc tree. > > This patch has nothing to do with ARM, so Kumar will pick it up, if you > ACK it. Why are you mentioning ARM? >From MAINTAINERS: I2C SUBSYSTEM P: Jean Delvare (PC drivers, core) M: kh...@linux-fr.org P: Ben Dooks (embedded platforms) M: ben-li...@fluff.org To the best of my knowledge i2c-mpc falls into the "embedded platforms" category and is thus under Ben's responsibility. Thanks, -- Jean Delvare ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH] i2c-mpc: do not allow interruptions when waiting for I2C to complete
Jean Delvare wrote: > To the best of my knowledge i2c-mpc falls into the "embedded platforms" > category and is thus under Ben's responsibility. I don't see his Signed-off-by on any patch to i2c-mpc.c for the past three years (at least). I do see yours, however. You have directly applied similar patches in the recent past, so of course I will assume that you would apply this one. -- Timur Tabi Linux kernel developer at Freescale ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH] i2c-mpc: do not allow interruptions when waiting for I2C to complete
> I don't see his Signed-off-by on any patch to i2c-mpc.c for the past > three years (at least). I do see yours, however. You have directly Ben is the I2C-"embedded"-maintainer. This job was created less than a year ago, this is why you don't see sob from him regarding this driver. Regards, Wolfram -- Pengutronix e.K. | Wolfram Sang| Industrial Linux Solutions | http://www.pengutronix.de/ | signature.asc Description: Digital signature ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH] i2c-mpc: do not allow interruptions when waiting for I2C to complete
On Tue, Feb 10, 2009 at 10:21:04AM -0600, Timur Tabi wrote: > Jean Delvare wrote: > > > To the best of my knowledge i2c-mpc falls into the "embedded platforms" > > category and is thus under Ben's responsibility. > > I don't see his Signed-off-by on any patch to i2c-mpc.c for the past > three years (at least). I do see yours, however. You have directly > applied similar patches in the recent past, so of course I will assume > that you would apply this one. My appointment as an embedded maintainer is relatively recent. I do try and review patches, and sometimes these patches get sent to another maintainer. -- Ben (b...@fluff.org, http://www.fluff.org/) 'a smiley only costs 4 bytes' ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH] i2c-mpc: do not allow interruptions when waiting for I2C to complete
On Fri, Feb 06, 2009 at 08:00:37AM -0600, Timur Tabi wrote:
> The i2c_wait() function is using wait_event_interruptible_timeout() to wait
> for
> the I2C controller to signal that it has completed an I2C bus operation. If
> the process that causes the I2C operation terminated abruptly, the wait will
> be interrupted, returning an error. It is better to let the I2C operation
> finished before the process exits.
>
> It is safe to use wait_event_timeout() instead, because the timeout will allow
> the process to exit if the I2C bus hangs. It's also better to allow the
> I2C operation to finish, because unacknowledged I2C operations can cause the
> I2C bus to hang.
>
> Signed-off-by: Timur Tabi
Acked-by: Ben Dooks
> ---
>
> A similar change should probably be done to i2c-cpm.c, and maybe all other
> I2C drivers. Not many use wait_event_interruptible_timeout().
>
> drivers/i2c/busses/i2c-mpc.c |9 +++--
> 1 files changed, 3 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/i2c/busses/i2c-mpc.c b/drivers/i2c/busses/i2c-mpc.c
> index a9a45fc..c0ace48 100644
> --- a/drivers/i2c/busses/i2c-mpc.c
> +++ b/drivers/i2c/busses/i2c-mpc.c
> @@ -70,7 +70,7 @@ static irqreturn_t mpc_i2c_isr(int irq, void *dev_id)
> /* Read again to allow register to stabilise */
> i2c->interrupt = readb(i2c->base + MPC_I2C_SR);
> writeb(0, i2c->base + MPC_I2C_SR);
> - wake_up_interruptible(&i2c->queue);
> + wake_up(&i2c->queue);
> }
> return IRQ_HANDLED;
> }
> @@ -115,13 +115,10 @@ static int i2c_wait(struct mpc_i2c *i2c, unsigned
> timeout, int writing)
> writeb(0, i2c->base + MPC_I2C_SR);
> } else {
> /* Interrupt mode */
> - result = wait_event_interruptible_timeout(i2c->queue,
> + result = wait_event_timeout(i2c->queue,
> (i2c->interrupt & CSR_MIF), timeout * HZ);
>
> - if (unlikely(result < 0)) {
> - pr_debug("I2C: wait interrupted\n");
> - writeccr(i2c, 0);
> - } else if (unlikely(!(i2c->interrupt & CSR_MIF))) {
> + if (unlikely(!(i2c->interrupt & CSR_MIF))) {
> pr_debug("I2C: wait timeout\n");
> writeccr(i2c, 0);
> result = -ETIMEDOUT;
> --
> 1.5.5
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
Ben (b...@fluff.org, http://www.fluff.org/)
'a smiley only costs 4 bytes'
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH] i2c-mpc: do not allow interruptions when waiting for I2C to complete
On Feb 6, 2009, at 8:00 AM, Timur Tabi wrote: The i2c_wait() function is using wait_event_interruptible_timeout() to wait for the I2C controller to signal that it has completed an I2C bus operation. If the process that causes the I2C operation terminated abruptly, the wait will be interrupted, returning an error. It is better to let the I2C operation finished before the process exits. It is safe to use wait_event_timeout() instead, because the timeout will allow the process to exit if the I2C bus hangs. It's also better to allow the I2C operation to finish, because unacknowledged I2C operations can cause the I2C bus to hang. Signed-off-by: Timur Tabi --- A similar change should probably be done to i2c-cpm.c, and maybe all other I2C drivers. Not many use wait_event_interruptible_timeout(). drivers/i2c/busses/i2c-mpc.c |9 +++-- 1 files changed, 3 insertions(+), 6 deletions(-) applied to next - k ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
