diff mbox

[PATCHv2,2/9] i2c: xiic: move the xiic_process to thread context

Message ID 1434554299-23443-3-git-send-email-shubhraj@xilinx.com
State Accepted
Headers show

Commit Message

Shubhrajyoti Datta June 17, 2015, 3:18 p.m. UTC 
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@xilinx.com>
---
 drivers/i2c/busses/i2c-xiic.c |   33 ++++++++++++++++++++-------------
 1 files changed, 20 insertions(+), 13 deletions(-)

Comments

Wolfram Sang July 9, 2015, 5:31 p.m. UTC | #1 
>  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?
Shubhrajyoti Datta July 10, 2015, 5:08 a.m. UTC | #2 
On Thu, Jul 9, 2015 at 11:01 PM, Wolfram Sang <wsa@the-dreams.de> 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.
--
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
Wolfram Sang July 10, 2015, 7:53 a.m. UTC | #3 
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@the-dreams.de> 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
Shubhrajyoti Datta July 13, 2015, 6:28 a.m. UTC | #4 
On Fri, Jul 10, 2015 at 1:23 PM, Wolfram Sang <wsa@the-dreams.de> 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@the-dreams.de> 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
>
--
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 mbox

Patch

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;