Re: [PATCH v2] OMAP: I2C: Fix the interrupt clearing in OMAP4
Shubhrajyoti D shubhrajy...@ti.com writes: For OMAP4 Interrupt enable register is a legacy register. I don't see anything in the docs mentioning this is legacy. In fact, that register is used extensivly throughout the driver, even for OMAP4. I think the CLR/SET registers were added to aid atomically setting/clearing specific interrupts, but when disabling all, I don't see why I2C_IE cannot be used. For that reason, any reason why the 4430-specific check cannot simply be removed to fix this interrupt clearing bug? To clear the interrupts we were writing 0 to it. This patch still writes 0 to it, so I'm not seeing the point of this comment. However on OMAP4 we were writing 1 to IRQENABLE_CLR which clears only the arbitration lost interrupt. The patch intends to fix the same by writing 1 to all the bits. This is the bug, and should come first in the changelog so readers know what the problem is. Signed-off-by: Shubhrajyoti D shubhrajy...@ti.com I believe this patch was originally from a fix by Vikram Pandita. Even if you've now changed the implementation, please credit the original author (and Cc them) in the changelog. It's common practice (and common courtesy) to say something like Based on an a patch originally from Author email. Thanks. Also, this patch doesn't apply to mainline or linux-omap master. Can you please update? Thanks, Kevin -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] OMAP: I2C: Fix the interrupt clearing in OMAP4
On Tuesday 22 November 2011 12:05 AM, Kevin Hilman wrote: Shubhrajyoti D shubhrajy...@ti.com writes: For OMAP4 Interrupt enable register is a legacy register. I don't see anything in the docs mentioning this is legacy. In fact, that register is used extensivly throughout the driver, even for OMAP4. I think the CLR/SET registers were added to aid atomically setting/clearing specific interrupts, but when disabling all, I don't see why I2C_IE cannot be used. For that reason, any reason why the 4430-specific check cannot simply be removed to fix this interrupt clearing bug? I think IE could be used as well since the CLR is there I thought of using it. To clear the interrupts we were writing 0 to it. This patch still writes 0 to it, so I'm not seeing the point of this comment. However on OMAP4 we were writing 1 to IRQENABLE_CLR which clears only the arbitration lost interrupt. The patch intends to fix the same by writing 1 to all the bits. This is the bug, and should come first in the changelog so readers know what the problem is. Yes will make this the first thing . Signed-off-by: Shubhrajyoti D shubhrajy...@ti.com I believe this patch was originally from a fix by Vikram Pandita. Even if you've now changed the implementation, please credit the original author (and Cc them) in the changelog. It's common practice (and common courtesy) to say something like Based on an a patch originally from Author email. Thanks. Yes missed to copy the second version will update. Also, this patch doesn't apply to mainline or linux-omap master. Can you please update? It is was based on linus tree. Will update. Thanks, Kevin -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2] OMAP: I2C: Fix the interrupt clearing in OMAP4
For OMAP4 Interrupt enable register is a legacy register.
To clear the interrupts we were writing 0 to it. However on OMAP4
we were writing 1 to IRQENABLE_CLR which clears only the arbitration
lost interrupt. The patch intends to fix the same by writing 1
to all the bits.
Signed-off-by: Shubhrajyoti D shubhrajy...@ti.com
---
drivers/i2c/busses/i2c-omap.c | 15 ++-
1 files changed, 14 insertions(+), 1 deletions(-)
diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c
index 2dfb631..bf4376f 100644
--- a/drivers/i2c/busses/i2c-omap.c
+++ b/drivers/i2c/busses/i2c-omap.c
@@ -167,6 +167,10 @@ enum {
#define SYSC_IDLEMODE_SMART0x2
#define SYSC_CLOCKACTIVITY_FCLK0x2
+
+/* Mask to clear all the interrupts */
+#define OMAP_I2C_IRQENABLE_CLR_ALL 0x6FFF
+
/* Errata definitions */
#define I2C_OMAP_ERRATA_I207 (1 0)
#define I2C_OMAP3_1P153(1 1)
@@ -308,8 +312,17 @@ static void omap_i2c_idle(struct omap_i2c_dev *dev)
pdata = pdev-dev.platform_data;
dev-iestate = omap_i2c_read_reg(dev, OMAP_I2C_IE_REG);
+
+ /*
+* The Interrupt enable register is a legacy register for OMAP4.
+* However to clear all the interrupts we could write 0 to Interrupt
+* enable reg or should write 1 to all the bits of the
+* I2C_IRQENABLE_CLR register.
+*/
+
if (dev-rev = OMAP_I2C_REV_ON_4430)
- omap_i2c_write_reg(dev, OMAP_I2C_IRQENABLE_CLR, 1);
+ omap_i2c_write_reg(dev, OMAP_I2C_IRQENABLE_CLR,
+ OMAP_I2C_IRQENABLE_CLR_ALL);
else
omap_i2c_write_reg(dev, OMAP_I2C_IE_REG, 0);
--
1.7.1
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
