* [PATCH V3] i2c-imx: do not allow interruptions when waiting for I2C to complete
[not found] ` <4C1F0FC7.60605-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
@ 2010-06-21 7:27 ` Marc Kleine-Budde
2010-07-15 7:49 ` [PATCH V3 RESEND] " Marc Kleine-Budde
` (4 subsequent siblings)
5 siblings, 0 replies; 12+ messages in thread
From: Marc Kleine-Budde @ 2010-06-21 7:27 UTC (permalink / raw)
To: ben-linux-elnMNo+KYs3YtjvyW6yDsg
Cc: linux-i2c-u79uwXL29TY76Z2rM5mHXA, Wolfram Sang, Marc Kleine-Budde
The i2c_imx_trx_complete() 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 receives a signal, the wait will be
interrupted, returning an error. It is better to let the I2C operation
finished before handling the signal (i.e. returning into userspace).
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: Marc Kleine-Budde <mkl-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
Reviewed-by: Wolfram Sang <w.sang-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
---
V2: Remove check for "result < 0" as commented by Wolfram Sang.
V3: Remove "result" entirely.
drivers/i2c/busses/i2c-imx.c | 12 +++---------
1 files changed, 3 insertions(+), 9 deletions(-)
diff --git a/drivers/i2c/busses/i2c-imx.c b/drivers/i2c/busses/i2c-imx.c
index d1ff940..4c2a62b 100644
--- a/drivers/i2c/busses/i2c-imx.c
+++ b/drivers/i2c/busses/i2c-imx.c
@@ -159,15 +159,9 @@ static int i2c_imx_bus_busy(struct imx_i2c_struct *i2c_imx, int for_busy)
static int i2c_imx_trx_complete(struct imx_i2c_struct *i2c_imx)
{
- int result;
-
- result = wait_event_interruptible_timeout(i2c_imx->queue,
- i2c_imx->i2csr & I2SR_IIF, HZ / 10);
+ wait_event_timeout(i2c_imx->queue, i2c_imx->i2csr & I2SR_IIF, HZ / 10);
- if (unlikely(result < 0)) {
- dev_dbg(&i2c_imx->adapter.dev, "<%s> result < 0\n", __func__);
- return result;
- } else if (unlikely(!(i2c_imx->i2csr & I2SR_IIF))) {
+ if (unlikely(!(i2c_imx->i2csr & I2SR_IIF))) {
dev_dbg(&i2c_imx->adapter.dev, "<%s> Timeout\n", __func__);
return -ETIMEDOUT;
}
@@ -295,7 +289,7 @@ static irqreturn_t i2c_imx_isr(int irq, void *dev_id)
i2c_imx->i2csr = temp;
temp &= ~I2SR_IIF;
writeb(temp, i2c_imx->base + IMX_I2C_I2SR);
- wake_up_interruptible(&i2c_imx->queue);
+ wake_up(&i2c_imx->queue);
return IRQ_HANDLED;
}
--
1.7.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH V3 RESEND] i2c-imx: do not allow interruptions when waiting for I2C to complete
[not found] ` <4C1F0FC7.60605-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
2010-06-21 7:27 ` [PATCH V3] i2c-imx: do not allow interruptions when waiting for I2C to complete Marc Kleine-Budde
@ 2010-07-15 7:49 ` Marc Kleine-Budde
2010-07-19 8:11 ` [PATCH V3 RESEND2] " Marc Kleine-Budde
` (3 subsequent siblings)
5 siblings, 0 replies; 12+ messages in thread
From: Marc Kleine-Budde @ 2010-07-15 7:49 UTC (permalink / raw)
To: ben-linux-elnMNo+KYs3YtjvyW6yDsg
Cc: linux-i2c-u79uwXL29TY76Z2rM5mHXA, Wolfram Sang, Marc Kleine-Budde
[-- Attachment #1: Type: text/plain, Size: 2382 bytes --]
The i2c_imx_trx_complete() 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 receives a signal, the wait will be
interrupted, returning an error. It is better to let the I2C operation
finished before handling the signal (i.e. returning into userspace).
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: Marc Kleine-Budde <mkl-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
Reviewed-by: Wolfram Sang <w.sang-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
---
V2: Remove check for "result < 0" as commented by Wolfram Sang.
V3: Remove "result" entirely.
drivers/i2c/busses/i2c-imx.c | 12 +++---------
1 files changed, 3 insertions(+), 9 deletions(-)
diff --git a/drivers/i2c/busses/i2c-imx.c b/drivers/i2c/busses/i2c-imx.c
index d1ff940..4c2a62b 100644
--- a/drivers/i2c/busses/i2c-imx.c
+++ b/drivers/i2c/busses/i2c-imx.c
@@ -159,15 +159,9 @@ static int i2c_imx_bus_busy(struct imx_i2c_struct
*i2c_imx, int for_busy)
static int i2c_imx_trx_complete(struct imx_i2c_struct *i2c_imx)
{
- int result;
-
- result = wait_event_interruptible_timeout(i2c_imx->queue,
- i2c_imx->i2csr & I2SR_IIF, HZ / 10);
+ wait_event_timeout(i2c_imx->queue, i2c_imx->i2csr & I2SR_IIF, HZ / 10);
- if (unlikely(result < 0)) {
- dev_dbg(&i2c_imx->adapter.dev, "<%s> result < 0\n", __func__);
- return result;
- } else if (unlikely(!(i2c_imx->i2csr & I2SR_IIF))) {
+ if (unlikely(!(i2c_imx->i2csr & I2SR_IIF))) {
dev_dbg(&i2c_imx->adapter.dev, "<%s> Timeout\n", __func__);
return -ETIMEDOUT;
}
@@ -295,7 +289,7 @@ static irqreturn_t i2c_imx_isr(int irq, void *dev_id)
i2c_imx->i2csr = temp;
temp &= ~I2SR_IIF;
writeb(temp, i2c_imx->base + IMX_I2C_I2SR);
- wake_up_interruptible(&i2c_imx->queue);
+ wake_up(&i2c_imx->queue);
return IRQ_HANDLED;
}
--
1.7.1
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 260 bytes --]
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH V3 RESEND2] i2c-imx: do not allow interruptions when waiting for I2C to complete
[not found] ` <4C1F0FC7.60605-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
2010-06-21 7:27 ` [PATCH V3] i2c-imx: do not allow interruptions when waiting for I2C to complete Marc Kleine-Budde
2010-07-15 7:49 ` [PATCH V3 RESEND] " Marc Kleine-Budde
@ 2010-07-19 8:11 ` Marc Kleine-Budde
2010-09-15 11:45 ` [PATCH V3 RESEND3] " Marc Kleine-Budde
` (2 subsequent siblings)
5 siblings, 0 replies; 12+ messages in thread
From: Marc Kleine-Budde @ 2010-07-19 8:11 UTC (permalink / raw)
To: ben-linux-elnMNo+KYs3YtjvyW6yDsg
Cc: linux-i2c-u79uwXL29TY76Z2rM5mHXA, Wolfram Sang, Marc Kleine-Budde
[-- Attachment #1: Type: text/plain, Size: 2514 bytes --]
Hello Ben,
what about this patch?
cheers, Marc
--------8<--------8<--------8<--------8<--------8<--------8<--------
The i2c_imx_trx_complete() 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 receives a signal, the wait will be
interrupted, returning an error. It is better to let the I2C operation
finished before handling the signal (i.e. returning into userspace).
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: Marc Kleine-Budde <mkl-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
Reviewed-by: Wolfram Sang <w.sang-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
---
V2: Remove check for "result < 0" as commented by Wolfram Sang.
V3: Remove "result" entirely.
drivers/i2c/busses/i2c-imx.c | 12 +++---------
1 files changed, 3 insertions(+), 9 deletions(-)
diff --git a/drivers/i2c/busses/i2c-imx.c b/drivers/i2c/busses/i2c-imx.c
index d1ff940..4c2a62b 100644
--- a/drivers/i2c/busses/i2c-imx.c
+++ b/drivers/i2c/busses/i2c-imx.c
@@ -159,15 +159,9 @@ static int i2c_imx_bus_busy(struct imx_i2c_struct
*i2c_imx, int for_busy)
static int i2c_imx_trx_complete(struct imx_i2c_struct *i2c_imx)
{
- int result;
-
- result = wait_event_interruptible_timeout(i2c_imx->queue,
- i2c_imx->i2csr & I2SR_IIF, HZ / 10);
+ wait_event_timeout(i2c_imx->queue, i2c_imx->i2csr & I2SR_IIF, HZ / 10);
- if (unlikely(result < 0)) {
- dev_dbg(&i2c_imx->adapter.dev, "<%s> result < 0\n", __func__);
- return result;
- } else if (unlikely(!(i2c_imx->i2csr & I2SR_IIF))) {
+ if (unlikely(!(i2c_imx->i2csr & I2SR_IIF))) {
dev_dbg(&i2c_imx->adapter.dev, "<%s> Timeout\n", __func__);
return -ETIMEDOUT;
}
@@ -295,7 +289,7 @@ static irqreturn_t i2c_imx_isr(int irq, void *dev_id)
i2c_imx->i2csr = temp;
temp &= ~I2SR_IIF;
writeb(temp, i2c_imx->base + IMX_I2C_I2SR);
- wake_up_interruptible(&i2c_imx->queue);
+ wake_up(&i2c_imx->queue);
return IRQ_HANDLED;
}
--
1.7.1
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 260 bytes --]
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH V3 RESEND3] i2c-imx: do not allow interruptions when waiting for I2C to complete
[not found] ` <4C1F0FC7.60605-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
` (2 preceding siblings ...)
2010-07-19 8:11 ` [PATCH V3 RESEND2] " Marc Kleine-Budde
@ 2010-09-15 11:45 ` Marc Kleine-Budde
2010-09-24 14:50 ` [PATCH V3 RESEND4] " Marc Kleine-Budde
2010-10-04 14:22 ` [PATCH V3 RESEND5] " Marc Kleine-Budde
5 siblings, 0 replies; 12+ messages in thread
From: Marc Kleine-Budde @ 2010-09-15 11:45 UTC (permalink / raw)
To: ben-linux-elnMNo+KYs3YtjvyW6yDsg
Cc: linux-i2c-u79uwXL29TY76Z2rM5mHXA, Wolfram Sang, Marc Kleine-Budde
[-- Attachment #1: Type: text/plain, Size: 2518 bytes --]
Hello Ben,
what about this patch?
cheers, Marc
--------8<--------8<--------8<--------8<--------8<--------8<--------
The i2c_imx_trx_complete() 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 receives a signal, the wait will be
interrupted, returning an error. It is better to let the I2C operation
finished before handling the signal (i.e. returning into userspace).
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: Marc Kleine-Budde <mkl-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
Reviewed-by: Wolfram Sang <w.sang-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
---
V2: Remove check for "result < 0" as commented by Wolfram Sang.
V3: Remove "result" entirely.
drivers/i2c/busses/i2c-imx.c | 12 +++---------
1 files changed, 3 insertions(+), 9 deletions(-)
diff --git a/drivers/i2c/busses/i2c-imx.c b/drivers/i2c/busses/i2c-imx.c
index d1ff940..4c2a62b 100644
--- a/drivers/i2c/busses/i2c-imx.c
+++ b/drivers/i2c/busses/i2c-imx.c
@@ -159,15 +159,9 @@ static int i2c_imx_bus_busy(struct imx_i2c_struct
*i2c_imx, int for_busy)
static int i2c_imx_trx_complete(struct imx_i2c_struct *i2c_imx)
{
- int result;
-
- result = wait_event_interruptible_timeout(i2c_imx->queue,
- i2c_imx->i2csr & I2SR_IIF, HZ / 10);
+ wait_event_timeout(i2c_imx->queue, i2c_imx->i2csr & I2SR_IIF, HZ / 10);
- if (unlikely(result < 0)) {
- dev_dbg(&i2c_imx->adapter.dev, "<%s> result < 0\n", __func__);
- return result;
- } else if (unlikely(!(i2c_imx->i2csr & I2SR_IIF))) {
+ if (unlikely(!(i2c_imx->i2csr & I2SR_IIF))) {
dev_dbg(&i2c_imx->adapter.dev, "<%s> Timeout\n", __func__);
return -ETIMEDOUT;
}
@@ -295,7 +289,7 @@ static irqreturn_t i2c_imx_isr(int irq, void *dev_id)
i2c_imx->i2csr = temp;
temp &= ~I2SR_IIF;
writeb(temp, i2c_imx->base + IMX_I2C_I2SR);
- wake_up_interruptible(&i2c_imx->queue);
+ wake_up(&i2c_imx->queue);
return IRQ_HANDLED;
}
--
1.7.1
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 262 bytes --]
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH V3 RESEND4] i2c-imx: do not allow interruptions when waiting for I2C to complete
[not found] ` <4C1F0FC7.60605-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
` (3 preceding siblings ...)
2010-09-15 11:45 ` [PATCH V3 RESEND3] " Marc Kleine-Budde
@ 2010-09-24 14:50 ` Marc Kleine-Budde
[not found] ` <4C9CBAA1.5040202-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
2010-10-04 14:22 ` [PATCH V3 RESEND5] " Marc Kleine-Budde
5 siblings, 1 reply; 12+ messages in thread
From: Marc Kleine-Budde @ 2010-09-24 14:50 UTC (permalink / raw)
To: ben-linux-elnMNo+KYs3YtjvyW6yDsg
Cc: linux-i2c-u79uwXL29TY76Z2rM5mHXA, Wolfram Sang, Marc Kleine-Budde
[-- Attachment #1: Type: text/plain, Size: 2522 bytes --]
Hello Ben,
what about this patch?
cheers, Marc
--------8<--------8<--------8<--------8<--------8<--------8<--------
The i2c_imx_trx_complete() 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 receives a signal, the wait will be
interrupted, returning an error. It is better to let the I2C operation
finished before handling the signal (i.e. returning into userspace).
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: Marc Kleine-Budde <mkl-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
Reviewed-by: Wolfram Sang <w.sang-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
---
V2: Remove check for "result < 0" as commented by Wolfram Sang.
V3: Remove "result" entirely.
drivers/i2c/busses/i2c-imx.c | 12 +++---------
1 files changed, 3 insertions(+), 9 deletions(-)
diff --git a/drivers/i2c/busses/i2c-imx.c b/drivers/i2c/busses/i2c-imx.c
index d1ff940..4c2a62b 100644
--- a/drivers/i2c/busses/i2c-imx.c
+++ b/drivers/i2c/busses/i2c-imx.c
@@ -159,15 +159,9 @@ static int i2c_imx_bus_busy(struct imx_i2c_struct
*i2c_imx, int for_busy)
static int i2c_imx_trx_complete(struct imx_i2c_struct *i2c_imx)
{
- int result;
-
- result = wait_event_interruptible_timeout(i2c_imx->queue,
- i2c_imx->i2csr & I2SR_IIF, HZ / 10);
+ wait_event_timeout(i2c_imx->queue, i2c_imx->i2csr & I2SR_IIF, HZ / 10);
- if (unlikely(result < 0)) {
- dev_dbg(&i2c_imx->adapter.dev, "<%s> result < 0\n", __func__);
- return result;
- } else if (unlikely(!(i2c_imx->i2csr & I2SR_IIF))) {
+ if (unlikely(!(i2c_imx->i2csr & I2SR_IIF))) {
dev_dbg(&i2c_imx->adapter.dev, "<%s> Timeout\n", __func__);
return -ETIMEDOUT;
}
@@ -295,7 +289,7 @@ static irqreturn_t i2c_imx_isr(int irq, void *dev_id)
i2c_imx->i2csr = temp;
temp &= ~I2SR_IIF;
writeb(temp, i2c_imx->base + IMX_I2C_I2SR);
- wake_up_interruptible(&i2c_imx->queue);
+ wake_up(&i2c_imx->queue);
return IRQ_HANDLED;
}
--
1.7.1
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 262 bytes --]
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH V3 RESEND5] i2c-imx: do not allow interruptions when waiting for I2C to complete
[not found] ` <4C1F0FC7.60605-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
` (4 preceding siblings ...)
2010-09-24 14:50 ` [PATCH V3 RESEND4] " Marc Kleine-Budde
@ 2010-10-04 14:22 ` Marc Kleine-Budde
5 siblings, 0 replies; 12+ messages in thread
From: Marc Kleine-Budde @ 2010-10-04 14:22 UTC (permalink / raw)
To: ben-linux-elnMNo+KYs3YtjvyW6yDsg
Cc: linux-i2c-u79uwXL29TY76Z2rM5mHXA, Wolfram Sang,
Marc Kleine-Budde, gregkh-l3A5Bk7waGM
Hello Ben,
I've send this patch several times, can you please comment it or apply
it to your git tree. Please send me a note if you do so, then I can
stop bugging you :)
cheers, Marc
--------8<--------8<--------8<--------8<--------8<--------8<--------
The i2c_imx_trx_complete() 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 receives a signal, the wait will be
interrupted, returning an error. It is better to let the I2C operation
finished before handling the signal (i.e. returning into userspace).
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: Marc Kleine-Budde <mkl-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
Reviewed-by: Wolfram Sang <w.sang-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
---
V2: Remove check for "result < 0" as commented by Wolfram Sang.
V3: Remove "result" entirely.
drivers/i2c/busses/i2c-imx.c | 12 +++---------
1 files changed, 3 insertions(+), 9 deletions(-)
diff --git a/drivers/i2c/busses/i2c-imx.c b/drivers/i2c/busses/i2c-imx.c
index d1ff940..4c2a62b 100644
--- a/drivers/i2c/busses/i2c-imx.c
+++ b/drivers/i2c/busses/i2c-imx.c
@@ -159,15 +159,9 @@ static int i2c_imx_bus_busy(struct imx_i2c_struct
*i2c_imx, int for_busy)
static int i2c_imx_trx_complete(struct imx_i2c_struct *i2c_imx)
{
- int result;
-
- result = wait_event_interruptible_timeout(i2c_imx->queue,
- i2c_imx->i2csr & I2SR_IIF, HZ / 10);
+ wait_event_timeout(i2c_imx->queue, i2c_imx->i2csr & I2SR_IIF, HZ / 10);
- if (unlikely(result < 0)) {
- dev_dbg(&i2c_imx->adapter.dev, "<%s> result < 0\n", __func__);
- return result;
- } else if (unlikely(!(i2c_imx->i2csr & I2SR_IIF))) {
+ if (unlikely(!(i2c_imx->i2csr & I2SR_IIF))) {
dev_dbg(&i2c_imx->adapter.dev, "<%s> Timeout\n", __func__);
return -ETIMEDOUT;
}
@@ -295,7 +289,7 @@ static irqreturn_t i2c_imx_isr(int irq, void *dev_id)
i2c_imx->i2csr = temp;
temp &= ~I2SR_IIF;
writeb(temp, i2c_imx->base + IMX_I2C_I2SR);
- wake_up_interruptible(&i2c_imx->queue);
+ wake_up(&i2c_imx->queue);
return IRQ_HANDLED;
}
--
1.7.1
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related [flat|nested] 12+ messages in thread