* [PATCHv2 0/9] i2c: xiic: cleanups
@ 2015-06-17 15:18 Shubhrajyoti Datta
[not found] ` <1434554299-23443-1-git-send-email-shubhraj-gjFFaj9aHVfQT0dZR+AlfA@public.gmane.org>
0 siblings, 1 reply; 19+ messages in thread
From: Shubhrajyoti Datta @ 2015-06-17 15:18 UTC (permalink / raw)
To: linux-i2c-u79uwXL29TY76Z2rM5mHXA; +Cc: Shubhrajyoti Datta
This is a series of cleanups for the axi iic.
tested on zc702 and microblaze.
Changes from v1
Updated the commit message.
Shubhrajyoti Datta (9):
i2c: xiic: Remove the disabling of interrupts
i2c: xiic: move the xiic_process to thread context
i2c: xiic: Do not reset controller before every transfer
i2c: xiic: Remove the disabling of interrupts
i2c: xiic: Remove busy loop while waiting for bus busy
i2c: xiic: Add a debug msg in case of timeout
i2c: xiic: Remove the Addressed as slave interrupt
i2c: xiic: Service all interrupts in isr
i2c: xiic: Do not continue in case of errors in Rx
drivers/i2c/busses/i2c-xiic.c | 75 +++++++++++++++++++----------------------
1 files changed, 35 insertions(+), 40 deletions(-)
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCHv2 1/9] i2c: xiic: Remove the disabling of interrupts
[not found] ` <1434554299-23443-1-git-send-email-shubhraj-gjFFaj9aHVfQT0dZR+AlfA@public.gmane.org>
@ 2015-06-17 15:18 ` Shubhrajyoti Datta
2015-06-17 15:18 ` [PATCHv2 2/9] i2c: xiic: move the xiic_process to thread context Shubhrajyoti Datta
` (8 subsequent siblings)
9 siblings, 0 replies; 19+ messages in thread
From: Shubhrajyoti Datta @ 2015-06-17 15:18 UTC (permalink / raw)
To: linux-i2c-u79uwXL29TY76Z2rM5mHXA; +Cc: Shubhrajyoti Datta
Currently the interrupts are disabled at the start of the
isr and enabled at the end of the isr. Remove the same.
In case the slave device NACKs the transaction while in the isr
the transfer will continue and the NACK interrupt will arrive
only after the isr is serviced.
Signed-off-by: Shubhrajyoti Datta <shubhraj-gjFFaj9aHVfQT0dZR+AlfA@public.gmane.org>
---
drivers/i2c/busses/i2c-xiic.c | 3 ---
1 files changed, 0 insertions(+), 3 deletions(-)
diff --git a/drivers/i2c/busses/i2c-xiic.c b/drivers/i2c/busses/i2c-xiic.c
index 4dda23f..912780a 100644
--- a/drivers/i2c/busses/i2c-xiic.c
+++ b/drivers/i2c/busses/i2c-xiic.c
@@ -604,14 +604,11 @@ static irqreturn_t xiic_isr(int irq, void *dev_id)
struct xiic_i2c *i2c = dev_id;
spin_lock(&i2c->lock);
- /* disable interrupts globally */
- xiic_setreg32(i2c, XIIC_DGIER_OFFSET, 0);
dev_dbg(i2c->adap.dev.parent, "%s entry\n", __func__);
xiic_process(i2c);
- xiic_setreg32(i2c, XIIC_DGIER_OFFSET, XIIC_GINTR_ENABLE_MASK);
spin_unlock(&i2c->lock);
return IRQ_HANDLED;
--
1.7.1
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCHv2 2/9] i2c: xiic: move the xiic_process to thread context
[not found] ` <1434554299-23443-1-git-send-email-shubhraj-gjFFaj9aHVfQT0dZR+AlfA@public.gmane.org>
2015-06-17 15:18 ` [PATCHv2 1/9] i2c: xiic: Remove the disabling of interrupts Shubhrajyoti Datta
@ 2015-06-17 15:18 ` Shubhrajyoti Datta
[not found] ` <1434554299-23443-3-git-send-email-shubhraj-gjFFaj9aHVfQT0dZR+AlfA@public.gmane.org>
2015-06-17 15:18 ` [PATCHv2 3/9] i2c: xiic: Do not reset controller before every transfer Shubhrajyoti Datta
` (7 subsequent siblings)
9 siblings, 1 reply; 19+ messages in thread
From: Shubhrajyoti Datta @ 2015-06-17 15:18 UTC (permalink / raw)
To: linux-i2c-u79uwXL29TY76Z2rM5mHXA; +Cc: Shubhrajyoti Datta
The xiic_process is a 154 line code that runs in isr context currently
move it to thread context. Also the name xiic_process suggests that the
intension was to run in process context.
Signed-off-by: Shubhrajyoti Datta <shubhraj-gjFFaj9aHVfQT0dZR+AlfA@public.gmane.org>
---
drivers/i2c/busses/i2c-xiic.c | 33 ++++++++++++++++++++-------------
1 files changed, 20 insertions(+), 13 deletions(-)
diff --git a/drivers/i2c/busses/i2c-xiic.c b/drivers/i2c/busses/i2c-xiic.c
index 912780a..1a6e637 100644
--- a/drivers/i2c/busses/i2c-xiic.c
+++ b/drivers/i2c/busses/i2c-xiic.c
@@ -358,8 +358,9 @@ static void xiic_wakeup(struct xiic_i2c *i2c, int code)
wake_up(&i2c->wait);
}
-static void xiic_process(struct xiic_i2c *i2c)
+static irqreturn_t xiic_process(int irq, void *dev_id)
{
+ struct xiic_i2c *i2c = dev_id;
u32 pend, isr, ier;
u32 clr = 0;
@@ -368,6 +369,7 @@ static void xiic_process(struct xiic_i2c *i2c)
* To find which interrupts are pending; AND interrupts pending with
* interrupts masked.
*/
+ spin_lock(&i2c->lock);
isr = xiic_getreg32(i2c, XIIC_IISR_OFFSET);
ier = xiic_getreg32(i2c, XIIC_IIER_OFFSET);
pend = isr & ier;
@@ -378,11 +380,6 @@ static void xiic_process(struct xiic_i2c *i2c)
__func__, xiic_getreg8(i2c, XIIC_SR_REG_OFFSET),
i2c->tx_msg, i2c->nmsgs);
- /* Do not processes a devices interrupts if the device has no
- * interrupts pending
- */
- if (!pend)
- return;
/* Service requesting interrupt */
if ((pend & XIIC_INTR_ARB_LOST_MASK) ||
@@ -502,6 +499,8 @@ out:
dev_dbg(i2c->adap.dev.parent, "%s clr: 0x%x\n", __func__, clr);
xiic_setreg32(i2c, XIIC_IISR_OFFSET, clr);
+ spin_unlock(&i2c->lock);
+ return IRQ_HANDLED;
}
static int xiic_bus_busy(struct xiic_i2c *i2c)
@@ -602,16 +601,21 @@ static void xiic_start_send(struct xiic_i2c *i2c)
static irqreturn_t xiic_isr(int irq, void *dev_id)
{
struct xiic_i2c *i2c = dev_id;
-
- spin_lock(&i2c->lock);
+ u32 pend, isr, ier;
+ irqreturn_t ret = IRQ_HANDLED;
+ /* Do not processes a devices interrupts if the device has no
+ * interrupts pending
+ */
dev_dbg(i2c->adap.dev.parent, "%s entry\n", __func__);
- xiic_process(i2c);
-
- spin_unlock(&i2c->lock);
+ isr = xiic_getreg32(i2c, XIIC_IISR_OFFSET);
+ ier = xiic_getreg32(i2c, XIIC_IIER_OFFSET);
+ pend = isr & ier;
+ if (pend)
+ ret = IRQ_WAKE_THREAD;
- return IRQ_HANDLED;
+ return ret;
}
static void __xiic_start_xfer(struct xiic_i2c *i2c)
@@ -752,7 +756,10 @@ static int xiic_i2c_probe(struct platform_device *pdev)
spin_lock_init(&i2c->lock);
init_waitqueue_head(&i2c->wait);
- ret = devm_request_irq(&pdev->dev, irq, xiic_isr, 0, pdev->name, i2c);
+ ret = devm_request_threaded_irq(&pdev->dev, irq, xiic_isr,
+ xiic_process, IRQF_ONESHOT,
+ pdev->name, i2c);
+
if (ret < 0) {
dev_err(&pdev->dev, "Cannot claim IRQ\n");
return ret;
--
1.7.1
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCHv2 3/9] i2c: xiic: Do not reset controller before every transfer
[not found] ` <1434554299-23443-1-git-send-email-shubhraj-gjFFaj9aHVfQT0dZR+AlfA@public.gmane.org>
2015-06-17 15:18 ` [PATCHv2 1/9] i2c: xiic: Remove the disabling of interrupts Shubhrajyoti Datta
2015-06-17 15:18 ` [PATCHv2 2/9] i2c: xiic: move the xiic_process to thread context Shubhrajyoti Datta
@ 2015-06-17 15:18 ` Shubhrajyoti Datta
2015-06-17 15:18 ` [PATCHv2 4/9] i2c: xiic: Remove the disabling of interrupts Shubhrajyoti Datta
` (6 subsequent siblings)
9 siblings, 0 replies; 19+ messages in thread
From: Shubhrajyoti Datta @ 2015-06-17 15:18 UTC (permalink / raw)
To: linux-i2c-u79uwXL29TY76Z2rM5mHXA; +Cc: Shubhrajyoti Datta
Currently before every transfer the controller is reinitialised.
We are already resetting the controller upon errors so upon every
transfer is a performance kill.
Remove the same.
Signed-off-by: Shubhrajyoti Datta <shubhraj-gjFFaj9aHVfQT0dZR+AlfA@public.gmane.org>
---
drivers/i2c/busses/i2c-xiic.c | 1 -
1 files changed, 0 insertions(+), 1 deletions(-)
diff --git a/drivers/i2c/busses/i2c-xiic.c b/drivers/i2c/busses/i2c-xiic.c
index 1a6e637..92ea52a 100644
--- a/drivers/i2c/busses/i2c-xiic.c
+++ b/drivers/i2c/busses/i2c-xiic.c
@@ -667,7 +667,6 @@ static void xiic_start_xfer(struct xiic_i2c *i2c)
unsigned long flags;
spin_lock_irqsave(&i2c->lock, flags);
- xiic_reinit(i2c);
/* disable interrupts globally */
xiic_setreg32(i2c, XIIC_DGIER_OFFSET, 0);
spin_unlock_irqrestore(&i2c->lock, flags);
--
1.7.1
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCHv2 4/9] i2c: xiic: Remove the disabling of interrupts
[not found] ` <1434554299-23443-1-git-send-email-shubhraj-gjFFaj9aHVfQT0dZR+AlfA@public.gmane.org>
` (2 preceding siblings ...)
2015-06-17 15:18 ` [PATCHv2 3/9] i2c: xiic: Do not reset controller before every transfer Shubhrajyoti Datta
@ 2015-06-17 15:18 ` Shubhrajyoti Datta
2015-06-17 15:18 ` [PATCHv2 5/9] i2c: xiic: Remove busy loop while waiting for bus busy Shubhrajyoti Datta
` (5 subsequent siblings)
9 siblings, 0 replies; 19+ messages in thread
From: Shubhrajyoti Datta @ 2015-06-17 15:18 UTC (permalink / raw)
To: linux-i2c-u79uwXL29TY76Z2rM5mHXA; +Cc: Shubhrajyoti Datta
Currently before every transfer the interrupts are disabled.
So incase the slave nacks in the middle of the transfer the
current transfer is not aborted. Upon enabling the interrupts
conditions like NACK , arbitration lost will not be masked.
Remove the disabling of the interrupts.
Signed-off-by: Shubhrajyoti Datta <shubhraj-gjFFaj9aHVfQT0dZR+AlfA@public.gmane.org>
---
drivers/i2c/busses/i2c-xiic.c | 7 -------
1 files changed, 0 insertions(+), 7 deletions(-)
diff --git a/drivers/i2c/busses/i2c-xiic.c b/drivers/i2c/busses/i2c-xiic.c
index 92ea52a..d9501ab 100644
--- a/drivers/i2c/busses/i2c-xiic.c
+++ b/drivers/i2c/busses/i2c-xiic.c
@@ -664,15 +664,8 @@ static void __xiic_start_xfer(struct xiic_i2c *i2c)
static void xiic_start_xfer(struct xiic_i2c *i2c)
{
- unsigned long flags;
-
- spin_lock_irqsave(&i2c->lock, flags);
- /* disable interrupts globally */
- xiic_setreg32(i2c, XIIC_DGIER_OFFSET, 0);
- spin_unlock_irqrestore(&i2c->lock, flags);
__xiic_start_xfer(i2c);
- xiic_setreg32(i2c, XIIC_DGIER_OFFSET, XIIC_GINTR_ENABLE_MASK);
}
static int xiic_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs, int num)
--
1.7.1
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCHv2 5/9] i2c: xiic: Remove busy loop while waiting for bus busy
[not found] ` <1434554299-23443-1-git-send-email-shubhraj-gjFFaj9aHVfQT0dZR+AlfA@public.gmane.org>
` (3 preceding siblings ...)
2015-06-17 15:18 ` [PATCHv2 4/9] i2c: xiic: Remove the disabling of interrupts Shubhrajyoti Datta
@ 2015-06-17 15:18 ` Shubhrajyoti Datta
2015-06-17 15:18 ` [PATCHv2 6/9] i2c: xiic: Add a debug msg in case of timeout Shubhrajyoti Datta
` (4 subsequent siblings)
9 siblings, 0 replies; 19+ messages in thread
From: Shubhrajyoti Datta @ 2015-06-17 15:18 UTC (permalink / raw)
To: linux-i2c-u79uwXL29TY76Z2rM5mHXA; +Cc: Shubhrajyoti Datta
Remove the busy loop while waiting for bus busy.
Instead let the processor sleep.
Signed-off-by: Shubhrajyoti Datta <shubhraj-gjFFaj9aHVfQT0dZR+AlfA@public.gmane.org>
---
drivers/i2c/busses/i2c-xiic.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/i2c/busses/i2c-xiic.c b/drivers/i2c/busses/i2c-xiic.c
index d9501ab..a83f300 100644
--- a/drivers/i2c/busses/i2c-xiic.c
+++ b/drivers/i2c/busses/i2c-xiic.c
@@ -524,7 +524,7 @@ static int xiic_busy(struct xiic_i2c *i2c)
*/
err = xiic_bus_busy(i2c);
while (err && tries--) {
- mdelay(1);
+ msleep(1);
err = xiic_bus_busy(i2c);
}
--
1.7.1
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCHv2 6/9] i2c: xiic: Add a debug msg in case of timeout
[not found] ` <1434554299-23443-1-git-send-email-shubhraj-gjFFaj9aHVfQT0dZR+AlfA@public.gmane.org>
` (4 preceding siblings ...)
2015-06-17 15:18 ` [PATCHv2 5/9] i2c: xiic: Remove busy loop while waiting for bus busy Shubhrajyoti Datta
@ 2015-06-17 15:18 ` Shubhrajyoti Datta
[not found] ` <1434554299-23443-7-git-send-email-shubhraj-gjFFaj9aHVfQT0dZR+AlfA@public.gmane.org>
2015-06-17 15:18 ` [PATCHv2 7/9] i2c: xiic: Remove the Addressed as slave interrupt Shubhrajyoti Datta
` (3 subsequent siblings)
9 siblings, 1 reply; 19+ messages in thread
From: Shubhrajyoti Datta @ 2015-06-17 15:18 UTC (permalink / raw)
To: linux-i2c-u79uwXL29TY76Z2rM5mHXA; +Cc: Shubhrajyoti Datta
Currently we return silently upon a timeout.
Adding an error message to aid debugability. Not
a functional change.
Signed-off-by: Shubhrajyoti Datta <shubhraj-gjFFaj9aHVfQT0dZR+AlfA@public.gmane.org>
---
drivers/i2c/busses/i2c-xiic.c | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/drivers/i2c/busses/i2c-xiic.c b/drivers/i2c/busses/i2c-xiic.c
index a83f300..66c571e 100644
--- a/drivers/i2c/busses/i2c-xiic.c
+++ b/drivers/i2c/busses/i2c-xiic.c
@@ -692,6 +692,7 @@ static int xiic_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs, int num)
i2c->tx_msg = NULL;
i2c->rx_msg = NULL;
i2c->nmsgs = 0;
+ dev_err(adap->dev.parent, "Controller timed out\n");
return -ETIMEDOUT;
}
}
--
1.7.1
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCHv2 7/9] i2c: xiic: Remove the Addressed as slave interrupt
[not found] ` <1434554299-23443-1-git-send-email-shubhraj-gjFFaj9aHVfQT0dZR+AlfA@public.gmane.org>
` (5 preceding siblings ...)
2015-06-17 15:18 ` [PATCHv2 6/9] i2c: xiic: Add a debug msg in case of timeout Shubhrajyoti Datta
@ 2015-06-17 15:18 ` Shubhrajyoti Datta
2015-06-17 15:18 ` [PATCHv2 8/9] i2c: xiic: Service all interrupts in isr Shubhrajyoti Datta
` (2 subsequent siblings)
9 siblings, 0 replies; 19+ messages in thread
From: Shubhrajyoti Datta @ 2015-06-17 15:18 UTC (permalink / raw)
To: linux-i2c-u79uwXL29TY76Z2rM5mHXA; +Cc: Shubhrajyoti Datta
Currently there is no slave mode support in the driver
also in the isr we just ack it and do nothing.
So disable the AAS interrupt.
Signed-off-by: Shubhrajyoti Datta <shubhraj-gjFFaj9aHVfQT0dZR+AlfA@public.gmane.org>
---
drivers/i2c/busses/i2c-xiic.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/i2c/busses/i2c-xiic.c b/drivers/i2c/busses/i2c-xiic.c
index 66c571e..dd23a78 100644
--- a/drivers/i2c/busses/i2c-xiic.c
+++ b/drivers/i2c/busses/i2c-xiic.c
@@ -283,7 +283,7 @@ static void xiic_reinit(struct xiic_i2c *i2c)
/* Enable interrupts */
xiic_setreg32(i2c, XIIC_DGIER_OFFSET, XIIC_GINTR_ENABLE_MASK);
- xiic_irq_clr_en(i2c, XIIC_INTR_AAS_MASK | XIIC_INTR_ARB_LOST_MASK);
+ xiic_irq_clr_en(i2c, XIIC_INTR_ARB_LOST_MASK);
}
static void xiic_deinit(struct xiic_i2c *i2c)
--
1.7.1
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCHv2 8/9] i2c: xiic: Service all interrupts in isr
[not found] ` <1434554299-23443-1-git-send-email-shubhraj-gjFFaj9aHVfQT0dZR+AlfA@public.gmane.org>
` (6 preceding siblings ...)
2015-06-17 15:18 ` [PATCHv2 7/9] i2c: xiic: Remove the Addressed as slave interrupt Shubhrajyoti Datta
@ 2015-06-17 15:18 ` Shubhrajyoti Datta
2015-06-17 15:18 ` [PATCHv2 9/9] i2c: xiic: Do not continue in case of errors in Rx Shubhrajyoti Datta
2015-07-09 17:35 ` [PATCHv2 0/9] i2c: xiic: cleanups Wolfram Sang
9 siblings, 0 replies; 19+ messages in thread
From: Shubhrajyoti Datta @ 2015-06-17 15:18 UTC (permalink / raw)
To: linux-i2c-u79uwXL29TY76Z2rM5mHXA; +Cc: Shubhrajyoti Datta
Currently only one interrupt is serviced in the isr.
In case the multiple interrupts happen simultenously we service and ack
only one of them. Check for all the causes in the isr and service them.
Signed-off-by: Shubhrajyoti Datta <shubhraj-gjFFaj9aHVfQT0dZR+AlfA@public.gmane.org>
---
drivers/i2c/busses/i2c-xiic.c | 24 ++++++++++--------------
1 files changed, 10 insertions(+), 14 deletions(-)
diff --git a/drivers/i2c/busses/i2c-xiic.c b/drivers/i2c/busses/i2c-xiic.c
index dd23a78..182ea68 100644
--- a/drivers/i2c/busses/i2c-xiic.c
+++ b/drivers/i2c/busses/i2c-xiic.c
@@ -401,11 +401,11 @@ static irqreturn_t xiic_process(int irq, void *dev_id)
if (i2c->tx_msg)
xiic_wakeup(i2c, STATE_ERROR);
-
- } else if (pend & XIIC_INTR_RX_FULL_MASK) {
+ }
+ if (pend & XIIC_INTR_RX_FULL_MASK) {
/* Receive register/FIFO is full */
- clr = XIIC_INTR_RX_FULL_MASK;
+ clr |= XIIC_INTR_RX_FULL_MASK;
if (!i2c->rx_msg) {
dev_dbg(i2c->adap.dev.parent,
"%s unexpexted RX IRQ\n", __func__);
@@ -438,9 +438,10 @@ static irqreturn_t xiic_process(int irq, void *dev_id)
__xiic_start_xfer(i2c);
}
}
- } else if (pend & XIIC_INTR_BNB_MASK) {
+ }
+ if (pend & XIIC_INTR_BNB_MASK) {
/* IIC bus has transitioned to not busy */
- clr = XIIC_INTR_BNB_MASK;
+ clr |= XIIC_INTR_BNB_MASK;
/* The bus is not busy, disable BusNotBusy interrupt */
xiic_irq_dis(i2c, XIIC_INTR_BNB_MASK);
@@ -453,12 +454,12 @@ static irqreturn_t xiic_process(int irq, void *dev_id)
xiic_wakeup(i2c, STATE_DONE);
else
xiic_wakeup(i2c, STATE_ERROR);
-
- } else if (pend & (XIIC_INTR_TX_EMPTY_MASK | XIIC_INTR_TX_HALF_MASK)) {
+ }
+ if (pend & (XIIC_INTR_TX_EMPTY_MASK | XIIC_INTR_TX_HALF_MASK)) {
/* Transmit register/FIFO is empty or ½ empty */
- clr = pend &
- (XIIC_INTR_TX_EMPTY_MASK | XIIC_INTR_TX_HALF_MASK);
+ clr |= (pend &
+ (XIIC_INTR_TX_EMPTY_MASK | XIIC_INTR_TX_HALF_MASK));
if (!i2c->tx_msg) {
dev_dbg(i2c->adap.dev.parent,
@@ -489,11 +490,6 @@ static irqreturn_t xiic_process(int irq, void *dev_id)
* make sure to disable tx half
*/
xiic_irq_dis(i2c, XIIC_INTR_TX_HALF_MASK);
- } else {
- /* got IRQ which is not acked */
- dev_err(i2c->adap.dev.parent, "%s Got unexpected IRQ\n",
- __func__);
- clr = pend;
}
out:
dev_dbg(i2c->adap.dev.parent, "%s clr: 0x%x\n", __func__, clr);
--
1.7.1
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCHv2 9/9] i2c: xiic: Do not continue in case of errors in Rx
[not found] ` <1434554299-23443-1-git-send-email-shubhraj-gjFFaj9aHVfQT0dZR+AlfA@public.gmane.org>
` (7 preceding siblings ...)
2015-06-17 15:18 ` [PATCHv2 8/9] i2c: xiic: Service all interrupts in isr Shubhrajyoti Datta
@ 2015-06-17 15:18 ` Shubhrajyoti Datta
2015-07-09 17:35 ` [PATCHv2 0/9] i2c: xiic: cleanups Wolfram Sang
9 siblings, 0 replies; 19+ messages in thread
From: Shubhrajyoti Datta @ 2015-06-17 15:18 UTC (permalink / raw)
To: linux-i2c-u79uwXL29TY76Z2rM5mHXA; +Cc: Shubhrajyoti Datta
In case of error conditions like Arbitration lost or NACK lets signal
the waiting process.
Handle error cases in the Rx path
Signed-off-by: Shubhrajyoti Datta <shubhraj-gjFFaj9aHVfQT0dZR+AlfA@public.gmane.org>
---
drivers/i2c/busses/i2c-xiic.c | 2 ++
1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/drivers/i2c/busses/i2c-xiic.c b/drivers/i2c/busses/i2c-xiic.c
index 182ea68..c071897 100644
--- a/drivers/i2c/busses/i2c-xiic.c
+++ b/drivers/i2c/busses/i2c-xiic.c
@@ -399,6 +399,8 @@ static irqreturn_t xiic_process(int irq, void *dev_id)
*/
xiic_reinit(i2c);
+ if (i2c->rx_msg)
+ xiic_wakeup(i2c, STATE_ERROR);
if (i2c->tx_msg)
xiic_wakeup(i2c, STATE_ERROR);
}
--
1.7.1
^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCHv2 6/9] i2c: xiic: Add a debug msg in case of timeout
[not found] ` <1434554299-23443-7-git-send-email-shubhraj-gjFFaj9aHVfQT0dZR+AlfA@public.gmane.org>
@ 2015-07-09 17:29 ` Wolfram Sang
2015-07-10 5:11 ` Shubhrajyoti Datta
0 siblings, 1 reply; 19+ messages in thread
From: Wolfram Sang @ 2015-07-09 17:29 UTC (permalink / raw)
To: Shubhrajyoti Datta; +Cc: linux-i2c-u79uwXL29TY76Z2rM5mHXA, Shubhrajyoti Datta
[-- Attachment #1: Type: text/plain, Size: 1344 bytes --]
On Wed, Jun 17, 2015 at 08:48:16PM +0530, Shubhrajyoti Datta wrote:
> Currently we return silently upon a timeout.
> Adding an error message to aid debugability. Not
> a functional change.
>
> Signed-off-by: Shubhrajyoti Datta <shubhraj-gjFFaj9aHVfQT0dZR+AlfA@public.gmane.org>
I am not in favour of this. In case of a stalled bus, this can easily
flood the logs. If it is for debugging, it should also be dev_dbg. And
then, it probably can also be added when needed. I mean the errno is
quite descriptive, no?
> ---
> drivers/i2c/busses/i2c-xiic.c | 1 +
> 1 files changed, 1 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/i2c/busses/i2c-xiic.c b/drivers/i2c/busses/i2c-xiic.c
> index a83f300..66c571e 100644
> --- a/drivers/i2c/busses/i2c-xiic.c
> +++ b/drivers/i2c/busses/i2c-xiic.c
> @@ -692,6 +692,7 @@ static int xiic_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs, int num)
> i2c->tx_msg = NULL;
> i2c->rx_msg = NULL;
> i2c->nmsgs = 0;
> + dev_err(adap->dev.parent, "Controller timed out\n");
> return -ETIMEDOUT;
> }
> }
> --
> 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: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCHv2 2/9] i2c: xiic: move the xiic_process to thread context
[not found] ` <1434554299-23443-3-git-send-email-shubhraj-gjFFaj9aHVfQT0dZR+AlfA@public.gmane.org>
@ 2015-07-09 17:31 ` Wolfram Sang
2015-07-10 5:08 ` Shubhrajyoti Datta
0 siblings, 1 reply; 19+ messages in thread
From: Wolfram Sang @ 2015-07-09 17:31 UTC (permalink / raw)
To: Shubhrajyoti Datta; +Cc: linux-i2c-u79uwXL29TY76Z2rM5mHXA, Shubhrajyoti Datta
[-- Attachment #1: Type: text/plain, Size: 453 bytes --]
> static int xiic_bus_busy(struct xiic_i2c *i2c)
> @@ -602,16 +601,21 @@ static void xiic_start_send(struct xiic_i2c *i2c)
> static irqreturn_t xiic_isr(int irq, void *dev_id)
> {
> struct xiic_i2c *i2c = dev_id;
> -
> - spin_lock(&i2c->lock);
> + u32 pend, isr, ier;
> + irqreturn_t ret = IRQ_HANDLED;
> + /* Do not processes a devices interrupts if the device has no
> + * interrupts pending
> + */
Shouldn't you init 'ret' to IRQ_NONE then?
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCHv2 0/9] i2c: xiic: cleanups
[not found] ` <1434554299-23443-1-git-send-email-shubhraj-gjFFaj9aHVfQT0dZR+AlfA@public.gmane.org>
` (8 preceding siblings ...)
2015-06-17 15:18 ` [PATCHv2 9/9] i2c: xiic: Do not continue in case of errors in Rx Shubhrajyoti Datta
@ 2015-07-09 17:35 ` Wolfram Sang
2015-07-10 5:13 ` Shubhrajyoti Datta
9 siblings, 1 reply; 19+ messages in thread
From: Wolfram Sang @ 2015-07-09 17:35 UTC (permalink / raw)
To: Shubhrajyoti Datta; +Cc: linux-i2c-u79uwXL29TY76Z2rM5mHXA, Shubhrajyoti Datta
[-- Attachment #1: Type: text/plain, Size: 899 bytes --]
On Wed, Jun 17, 2015 at 08:48:10PM +0530, Shubhrajyoti Datta wrote:
> This is a series of cleanups for the axi iic.
> tested on zc702 and microblaze.
>
> Changes from v1
> Updated the commit message.
>
>
> Shubhrajyoti Datta (9):
> i2c: xiic: Remove the disabling of interrupts
> i2c: xiic: move the xiic_process to thread context
> i2c: xiic: Do not reset controller before every transfer
> i2c: xiic: Remove the disabling of interrupts
> i2c: xiic: Remove busy loop while waiting for bus busy
> i2c: xiic: Add a debug msg in case of timeout
> i2c: xiic: Remove the Addressed as slave interrupt
> i2c: xiic: Service all interrupts in isr
> i2c: xiic: Do not continue in case of errors in Rx
Commit messages are a lot better, thanks!
Two minor comments. If you agree to them, I can simply drop patch 6 and
fixup patch 2. Then, no need for resending.
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCHv2 2/9] i2c: xiic: move the xiic_process to thread context
2015-07-09 17:31 ` Wolfram Sang
@ 2015-07-10 5:08 ` Shubhrajyoti Datta
[not found] ` <CAKfKVtGOxWU-66fQSd8D6Ue0Si0koQF-CrR0pVZ8RupMNQ=-GQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
0 siblings, 1 reply; 19+ messages in thread
From: Shubhrajyoti Datta @ 2015-07-10 5:08 UTC (permalink / raw)
To: Wolfram Sang
Cc: Shubhrajyoti Datta, linux-i2c-u79uwXL29TY76Z2rM5mHXA,
Shubhrajyoti Datta
On Thu, Jul 9, 2015 at 11:01 PM, Wolfram Sang <wsa-z923LK4zBo2bacvFa/9K2g@public.gmane.org> wrote:
>> static int xiic_bus_busy(struct xiic_i2c *i2c)
>> @@ -602,16 +601,21 @@ static void xiic_start_send(struct xiic_i2c *i2c)
>> static irqreturn_t xiic_isr(int irq, void *dev_id)
>> {
>> struct xiic_i2c *i2c = dev_id;
>> -
>> - spin_lock(&i2c->lock);
>> + u32 pend, isr, ier;
>> + irqreturn_t ret = IRQ_HANDLED;
>> + /* Do not processes a devices interrupts if the device has no
>> + * interrupts pending
>> + */
>
> Shouldn't you init 'ret' to IRQ_NONE then?
>
Indeed I missed it.
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCHv2 6/9] i2c: xiic: Add a debug msg in case of timeout
2015-07-09 17:29 ` Wolfram Sang
@ 2015-07-10 5:11 ` Shubhrajyoti Datta
0 siblings, 0 replies; 19+ messages in thread
From: Shubhrajyoti Datta @ 2015-07-10 5:11 UTC (permalink / raw)
To: Wolfram Sang
Cc: Shubhrajyoti Datta, linux-i2c-u79uwXL29TY76Z2rM5mHXA,
Shubhrajyoti Datta
On Thu, Jul 9, 2015 at 10:59 PM, Wolfram Sang <wsa-z923LK4zBo2bacvFa/9K2g@public.gmane.org> wrote:
> On Wed, Jun 17, 2015 at 08:48:16PM +0530, Shubhrajyoti Datta wrote:
>> Currently we return silently upon a timeout.
>> Adding an error message to aid debugability. Not
>> a functional change.
>>
>> Signed-off-by: Shubhrajyoti Datta <shubhraj-gjFFaj9aHVfQT0dZR+AlfA@public.gmane.org>
>
> I am not in favour of this. In case of a stalled bus, this can easily
> flood the logs. If it is for debugging, it should also be dev_dbg. And
> then, it probably can also be added when needed. I mean the errno is
> quite descriptive, no?
>
I agree I had added it for debugging.
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCHv2 0/9] i2c: xiic: cleanups
2015-07-09 17:35 ` [PATCHv2 0/9] i2c: xiic: cleanups Wolfram Sang
@ 2015-07-10 5:13 ` Shubhrajyoti Datta
[not found] ` <CAKfKVtE5qfQjxJNDOBXfVLncVYXJjgfXe6Anb6M=dSFkHsTt8w-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
0 siblings, 1 reply; 19+ messages in thread
From: Shubhrajyoti Datta @ 2015-07-10 5:13 UTC (permalink / raw)
To: Wolfram Sang
Cc: Shubhrajyoti Datta, linux-i2c-u79uwXL29TY76Z2rM5mHXA,
Shubhrajyoti Datta
On Thu, Jul 9, 2015 at 11:05 PM, Wolfram Sang <wsa-z923LK4zBo2bacvFa/9K2g@public.gmane.org> wrote:
> On Wed, Jun 17, 2015 at 08:48:10PM +0530, Shubhrajyoti Datta wrote:
>> This is a series of cleanups for the axi iic.
>> tested on zc702 and microblaze.
>>
>> Changes from v1
>> Updated the commit message.
>>
>>
>> Shubhrajyoti Datta (9):
>> i2c: xiic: Remove the disabling of interrupts
>> i2c: xiic: move the xiic_process to thread context
>> i2c: xiic: Do not reset controller before every transfer
>> i2c: xiic: Remove the disabling of interrupts
>> i2c: xiic: Remove busy loop while waiting for bus busy
>> i2c: xiic: Add a debug msg in case of timeout
>> i2c: xiic: Remove the Addressed as slave interrupt
>> i2c: xiic: Service all interrupts in isr
>> i2c: xiic: Do not continue in case of errors in Rx
>
> Commit messages are a lot better, thanks!
>
> Two minor comments. If you agree to them, I can simply drop patch 6 and
> fixup patch 2
I agree anyways 2 was like a debug patch.
>. Then, no need for resending.
>
thanks.
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCHv2 2/9] i2c: xiic: move the xiic_process to thread context
[not found] ` <CAKfKVtGOxWU-66fQSd8D6Ue0Si0koQF-CrR0pVZ8RupMNQ=-GQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
@ 2015-07-10 7:53 ` Wolfram Sang
2015-07-13 6:28 ` Shubhrajyoti Datta
0 siblings, 1 reply; 19+ messages in thread
From: Wolfram Sang @ 2015-07-10 7:53 UTC (permalink / raw)
To: Shubhrajyoti Datta
Cc: Shubhrajyoti Datta, linux-i2c-u79uwXL29TY76Z2rM5mHXA,
Shubhrajyoti Datta
[-- Attachment #1: Type: text/plain, Size: 818 bytes --]
On Fri, Jul 10, 2015 at 10:38:11AM +0530, Shubhrajyoti Datta wrote:
> On Thu, Jul 9, 2015 at 11:01 PM, Wolfram Sang <wsa-z923LK4zBo2bacvFa/9K2g@public.gmane.org> wrote:
> >> static int xiic_bus_busy(struct xiic_i2c *i2c)
> >> @@ -602,16 +601,21 @@ static void xiic_start_send(struct xiic_i2c *i2c)
> >> static irqreturn_t xiic_isr(int irq, void *dev_id)
> >> {
> >> struct xiic_i2c *i2c = dev_id;
> >> -
> >> - spin_lock(&i2c->lock);
> >> + u32 pend, isr, ier;
> >> + irqreturn_t ret = IRQ_HANDLED;
> >> + /* Do not processes a devices interrupts if the device has no
> >> + * interrupts pending
> >> + */
> >
> > Shouldn't you init 'ret' to IRQ_NONE then?
> >
>
> Indeed I missed it.
Can you test this change on HW and report back?
Thanks,
Wolfram
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCHv2 2/9] i2c: xiic: move the xiic_process to thread context
2015-07-10 7:53 ` Wolfram Sang
@ 2015-07-13 6:28 ` Shubhrajyoti Datta
0 siblings, 0 replies; 19+ messages in thread
From: Shubhrajyoti Datta @ 2015-07-13 6:28 UTC (permalink / raw)
To: Wolfram Sang
Cc: Shubhrajyoti Datta, linux-i2c-u79uwXL29TY76Z2rM5mHXA,
Shubhrajyoti Datta
On Fri, Jul 10, 2015 at 1:23 PM, Wolfram Sang <wsa-z923LK4zBo2bacvFa/9K2g@public.gmane.org> wrote:
> On Fri, Jul 10, 2015 at 10:38:11AM +0530, Shubhrajyoti Datta wrote:
>> On Thu, Jul 9, 2015 at 11:01 PM, Wolfram Sang <wsa-z923LK4zBo2bacvFa/9K2g@public.gmane.org> wrote:
>> >> static int xiic_bus_busy(struct xiic_i2c *i2c)
>> >> @@ -602,16 +601,21 @@ static void xiic_start_send(struct xiic_i2c *i2c)
>> >> static irqreturn_t xiic_isr(int irq, void *dev_id)
>> >> {
>> >> struct xiic_i2c *i2c = dev_id;
>> >> -
>> >> - spin_lock(&i2c->lock);
>> >> + u32 pend, isr, ier;
>> >> + irqreturn_t ret = IRQ_HANDLED;
>> >> + /* Do not processes a devices interrupts if the device has no
>> >> + * interrupts pending
>> >> + */
>> >
>> > Shouldn't you init 'ret' to IRQ_NONE then?
>> >
>>
>> Indeed I missed it.
>
> Can you test this change on HW and report back?
I have tested it.
>
> Thanks,
>
> Wolfram
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCHv2 0/9] i2c: xiic: cleanups
[not found] ` <CAKfKVtE5qfQjxJNDOBXfVLncVYXJjgfXe6Anb6M=dSFkHsTt8w-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
@ 2015-07-14 11:59 ` Wolfram Sang
0 siblings, 0 replies; 19+ messages in thread
From: Wolfram Sang @ 2015-07-14 11:59 UTC (permalink / raw)
To: Shubhrajyoti Datta
Cc: Shubhrajyoti Datta, linux-i2c-u79uwXL29TY76Z2rM5mHXA,
Shubhrajyoti Datta
[-- Attachment #1: Type: text/plain, Size: 1178 bytes --]
On Fri, Jul 10, 2015 at 10:43:18AM +0530, Shubhrajyoti Datta wrote:
> On Thu, Jul 9, 2015 at 11:05 PM, Wolfram Sang <wsa-z923LK4zBo2bacvFa/9K2g@public.gmane.org> wrote:
> > On Wed, Jun 17, 2015 at 08:48:10PM +0530, Shubhrajyoti Datta wrote:
> >> This is a series of cleanups for the axi iic.
> >> tested on zc702 and microblaze.
> >>
> >> Changes from v1
> >> Updated the commit message.
> >>
> >>
> >> Shubhrajyoti Datta (9):
> >> i2c: xiic: Remove the disabling of interrupts
> >> i2c: xiic: move the xiic_process to thread context
> >> i2c: xiic: Do not reset controller before every transfer
> >> i2c: xiic: Remove the disabling of interrupts
> >> i2c: xiic: Remove busy loop while waiting for bus busy
> >> i2c: xiic: Add a debug msg in case of timeout
> >> i2c: xiic: Remove the Addressed as slave interrupt
> >> i2c: xiic: Service all interrupts in isr
> >> i2c: xiic: Do not continue in case of errors in Rx
> >
> > Commit messages are a lot better, thanks!
> >
> > Two minor comments. If you agree to them, I can simply drop patch 6 and
> > fixup patch 2
> I agree anyways 2 was like a debug patch.
Applied patches as discussed to for-next, thanks!
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 19+ messages in thread
end of thread, other threads:[~2015-07-14 11:59 UTC | newest]
Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-06-17 15:18 [PATCHv2 0/9] i2c: xiic: cleanups Shubhrajyoti Datta
[not found] ` <1434554299-23443-1-git-send-email-shubhraj-gjFFaj9aHVfQT0dZR+AlfA@public.gmane.org>
2015-06-17 15:18 ` [PATCHv2 1/9] i2c: xiic: Remove the disabling of interrupts Shubhrajyoti Datta
2015-06-17 15:18 ` [PATCHv2 2/9] i2c: xiic: move the xiic_process to thread context Shubhrajyoti Datta
[not found] ` <1434554299-23443-3-git-send-email-shubhraj-gjFFaj9aHVfQT0dZR+AlfA@public.gmane.org>
2015-07-09 17:31 ` Wolfram Sang
2015-07-10 5:08 ` Shubhrajyoti Datta
[not found] ` <CAKfKVtGOxWU-66fQSd8D6Ue0Si0koQF-CrR0pVZ8RupMNQ=-GQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2015-07-10 7:53 ` Wolfram Sang
2015-07-13 6:28 ` Shubhrajyoti Datta
2015-06-17 15:18 ` [PATCHv2 3/9] i2c: xiic: Do not reset controller before every transfer Shubhrajyoti Datta
2015-06-17 15:18 ` [PATCHv2 4/9] i2c: xiic: Remove the disabling of interrupts Shubhrajyoti Datta
2015-06-17 15:18 ` [PATCHv2 5/9] i2c: xiic: Remove busy loop while waiting for bus busy Shubhrajyoti Datta
2015-06-17 15:18 ` [PATCHv2 6/9] i2c: xiic: Add a debug msg in case of timeout Shubhrajyoti Datta
[not found] ` <1434554299-23443-7-git-send-email-shubhraj-gjFFaj9aHVfQT0dZR+AlfA@public.gmane.org>
2015-07-09 17:29 ` Wolfram Sang
2015-07-10 5:11 ` Shubhrajyoti Datta
2015-06-17 15:18 ` [PATCHv2 7/9] i2c: xiic: Remove the Addressed as slave interrupt Shubhrajyoti Datta
2015-06-17 15:18 ` [PATCHv2 8/9] i2c: xiic: Service all interrupts in isr Shubhrajyoti Datta
2015-06-17 15:18 ` [PATCHv2 9/9] i2c: xiic: Do not continue in case of errors in Rx Shubhrajyoti Datta
2015-07-09 17:35 ` [PATCHv2 0/9] i2c: xiic: cleanups Wolfram Sang
2015-07-10 5:13 ` Shubhrajyoti Datta
[not found] ` <CAKfKVtE5qfQjxJNDOBXfVLncVYXJjgfXe6Anb6M=dSFkHsTt8w-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2015-07-14 11:59 ` Wolfram Sang
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.