I2C: Fix unhandled fault in i2c-omap controller

PATCH In review Tony Lindgren 3 participants 8 messages Mar 13, 2008 – Mar 17, 2008 2 messages not in the archive

Subsystems (4)

I2C subsystem host drivers OMAP2+ support OMAP1 support OMAP I2C driver

lore

Share & tools

lore

Tony Lindgren

author maintainer

2008-03-13 15:51 UTC #0 lore raw

Hi Jean,

Here's an omap I2C fix that would be nice to get into 2.6.25 if still possible.

Regards,

Tony

commit 8e4f19286e68d38d589451ad2a10545f2c40032d

Author: Tony Lindgren <[email protected]>
Date:   Mon Feb 18 20:16:00 2008 -0800

I2C: Fix unhandled fault in i2c-omap controller

If an I2C interrupt happens between disabling interface clock and functional clock, the interrupt handler will produce an external abort on non-linefetch error when trying to access driver registers while interface clock is disabled.

This patch fixes the problem by saving and disabling i2c-omap interrupt before turning off the clocks. Also disable functional clock before the interface clock as suggested by Paul Walmsley.

Patch also renames enable/disable_clocks functions to unidle/idle functions. Note that the driver is currently not taking advantage of the idle interrupts. To use the idle interrupts, driver would have to enable interface clock based on the idle interrupt and dev->idle flag.

Cc Paul Walmsley <[email protected]>

Signed-off-by Tony Lindgren <[email protected]>

drivers/i2c/busses/i2c-omap.c +28 -8

index da66397..862dd69 100644
@@ -128,6 +128,8 @@ struct omap_i2c_dev {
 	size_t			buf_len;
 	struct i2c_adapter	adapter;
 	unsigned		rev1:1;
+	unsigned		idle:1;
+	u16			iestate;	/* Saved interrupt register */
 };
 
 static inline void omap_i2c_write_reg(struct omap_i2c_dev *i2c_dev,

@@ -174,18 +176,30 @@ static void omap_i2c_put_clocks(struct omap_i2c_dev *dev)
 	}
 }
 
-static void omap_i2c_enable_clocks(struct omap_i2c_dev *dev)
+static void omap_i2c_unidle(struct omap_i2c_dev *dev)
 {
 	if (dev->iclk != NULL)
 		clk_enable(dev->iclk);
 	clk_enable(dev->fclk);
+	if (dev->iestate)
+		omap_i2c_write_reg(dev, OMAP_I2C_IE_REG, dev->iestate);
+	dev->idle = 0;
 }
 
-static void omap_i2c_disable_clocks(struct omap_i2c_dev *dev)
+static void omap_i2c_idle(struct omap_i2c_dev *dev)
 {
+	u16 iv;
+
+	dev->idle = 1;
+	dev->iestate = omap_i2c_read_reg(dev, OMAP_I2C_IE_REG);
+	omap_i2c_write_reg(dev, OMAP_I2C_IE_REG, 0);
+	if (dev->rev1)
+		iv = omap_i2c_read_reg(dev, OMAP_I2C_IV_REG);
+	else
+		omap_i2c_write_reg(dev, OMAP_I2C_STAT_REG, dev->iestate);
+	clk_disable(dev->fclk);
 	if (dev->iclk != NULL)
 		clk_disable(dev->iclk);
-	clk_disable(dev->fclk);
 }
 
 static int omap_i2c_init(struct omap_i2c_dev *dev)

@@ -360,7 +374,7 @@ omap_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[], int num)
 	int i;
 	int r;
 
-	omap_i2c_enable_clocks(dev);
+	omap_i2c_unidle(dev);
 
 	if ((r = omap_i2c_wait_for_bb(dev)) < 0)
 		goto out;

@@ -374,7 +388,7 @@ omap_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[], int num)
 	if (r == 0)
 		r = num;
 out:
-	omap_i2c_disable_clocks(dev);
+	omap_i2c_idle(dev);
 	return r;
 }

@@ -403,6 +417,9 @@ omap_i2c_rev1_isr(int this_irq, void *dev_id)
 	struct omap_i2c_dev *dev = dev_id;
 	u16 iv, w;
 
+	if (dev->idle)
+		return IRQ_NONE;
+
 	iv = omap_i2c_read_reg(dev, OMAP_I2C_IV_REG);
 	switch (iv) {
 	case 0x00:	/* None */

@@ -457,6 +474,9 @@ omap_i2c_isr(int this_irq, void *dev_id)
 	u16 stat, w;
 	int count = 0;
 
+	if (dev->idle)
+		return IRQ_NONE;
+
 	bits = omap_i2c_read_reg(dev, OMAP_I2C_IE_REG);
 	while ((stat = (omap_i2c_read_reg(dev, OMAP_I2C_STAT_REG))) & bits) {
 		dev_dbg(dev->dev, "IRQ (ISR = 0x%04x)\n", stat);

@@ -575,7 +595,7 @@ omap_i2c_probe(struct platform_device *pdev)
 	if ((r = omap_i2c_get_clocks(dev)) != 0)
 		goto err_free_mem;
 
-	omap_i2c_enable_clocks(dev);
+	omap_i2c_unidle(dev);
 
 	if (cpu_is_omap15xx())
 		dev->rev1 = omap_i2c_read_reg(dev, OMAP_I2C_REV_REG) < 0x20;

@@ -610,7 +630,7 @@ omap_i2c_probe(struct platform_device *pdev)
 		goto err_free_irq;
 	}
 
-	omap_i2c_disable_clocks(dev);
+	omap_i2c_idle(dev);
 
 	return 0;

@@ -618,7 +638,7 @@ err_free_irq:
 	free_irq(dev->irq, dev);
 err_unuse_clocks:
 	omap_i2c_write_reg(dev, OMAP_I2C_CON_REG, 0);
-	omap_i2c_disable_clocks(dev);
+	omap_i2c_idle(dev);
 	omap_i2c_put_clocks(dev);
 err_free_mem:
 	platform_set_drvdata(pdev, NULL);

Jean Delvare

2008-03-14 15:53 UTC #2 lore raw

In reply to a message not in the archive

Hi Tony,

Quoted 2 lines · Tony Lindgren

Here's an omap I2C fix that would be nice to get into 2.6.25 if still possible.

Yeah, that's possible, however the patch isn't totally trivial so I would appreciate if someone from the OMAP community could review it publicly and ack it. Then I'll push it to Linus quickly.

Signature · 6 lines

Jean Delvare

_______________________________________________
i2c mailing list
[email protected]
http://lists.lm-sensors.org/mailman/listinfo/i2c

Jean Delvare

2008-03-14 17:52 UTC #4 lore raw

In reply to a message not in the archive

Quoted 4 lines · Tony Lindgren

Hi Jean,
Here's an omap I2C fix that would be nice to get into 2.6.25 if still possible.

Review:

Quoted 52 lines

commit 8e4f19286e68d38d589451ad2a10545f2c40032d
Author: Tony Lindgren <[email protected]>
Date:   Mon Feb 18 20:16:00 2008 -0800
I2C: Fix unhandled fault in i2c-omap controller
If an I2C interrupt happens between disabling interface clock and functional clock, the interrupt handler will produce an external abort on non-linefetch error when trying to access driver registers while interface clock is disabled.
This patch fixes the problem by saving and disabling i2c-omap interrupt before turning off the clocks. Also disable functional clock before the interface clock as suggested by Paul Walmsley.
Patch also renames enable/disable_clocks functions to unidle/idle functions. Note that the driver is currently not taking advantage of the idle interrupts. To use the idle interrupts, driver would have to enable interface clock based on the idle interrupt and dev->idle flag.
Cc Paul Walmsley <[email protected]>
Signed-off-by Tony Lindgren <[email protected]>
drivers/i2c/busses/i2c-omap.c +6 -1
index da66397..862dd69 100644
@@ -128,6 +128,8 @@ struct omap_i2c_dev {
 	size_t			buf_len;
 	struct i2c_adapter	adapter;
 	unsigned		rev1:1;
+	unsigned		idle:1;
+	u16			iestate;	/* Saved interrupt register */
 };
 
 static inline void omap_i2c_write_reg(struct omap_i2c_dev *i2c_dev,
@@ -174,18 +176,30 @@ static void omap_i2c_put_clocks(struct omap_i2c_dev *dev)
 	}
 }
 
-static void omap_i2c_enable_clocks(struct omap_i2c_dev *dev)
+static void omap_i2c_unidle(struct omap_i2c_dev *dev)
 {
 	if (dev->iclk != NULL)
 		clk_enable(dev->iclk);
 	clk_enable(dev->fclk);
+	if (dev->iestate)
+		omap_i2c_write_reg(dev, OMAP_I2C_IE_REG, dev->iestate);
+	dev->idle = 0;
 }
 

drivers/i2c/busses/i2c-omap.c static void omap_i2c_put_clocks(struct omap_i2c_dev *dev) · line 197

Quoted 10 lines

drivers/i2c/busses/i2c-omap.c +8 -1

-static void omap_i2c_disable_clocks(struct omap_i2c_dev *dev)
+static void omap_i2c_idle(struct omap_i2c_dev *dev)
 {
+	u16 iv;
+
+	dev->idle = 1;
+	dev->iestate = omap_i2c_read_reg(dev, OMAP_I2C_IE_REG);
+	omap_i2c_write_reg(dev, OMAP_I2C_IE_REG, 0);
+	if (dev->rev1)
+		iv = omap_i2c_read_reg(dev, OMAP_I2C_IV_REG);

Reading OMAP_I2C_IV_REG but not doing anything with the value? This deserves at least a comment if this is done on purpose (but then I don't think you need the iv variable at all.)

Quoted 66 lines

drivers/i2c/busses/i2c-omap.c +13 -5

+	else
+		omap_i2c_write_reg(dev, OMAP_I2C_STAT_REG, dev->iestate);
+	clk_disable(dev->fclk);
 	if (dev->iclk != NULL)
 		clk_disable(dev->iclk);
-	clk_disable(dev->fclk);
 }
 
 static int omap_i2c_init(struct omap_i2c_dev *dev)

@@ -360,7 +374,7 @@ omap_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[], int num)
 	int i;
 	int r;
 
-	omap_i2c_enable_clocks(dev);
+	omap_i2c_unidle(dev);
 
 	if ((r = omap_i2c_wait_for_bb(dev)) < 0)
 		goto out;

@@ -374,7 +388,7 @@ omap_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[], int num)
 	if (r == 0)
 		r = num;
 out:
-	omap_i2c_disable_clocks(dev);
+	omap_i2c_idle(dev);
 	return r;
 }

@@ -403,6 +417,9 @@ omap_i2c_rev1_isr(int this_irq, void *dev_id)
 	struct omap_i2c_dev *dev = dev_id;
 	u16 iv, w;
 
+	if (dev->idle)
+		return IRQ_NONE;
+
 	iv = omap_i2c_read_reg(dev, OMAP_I2C_IV_REG);
 	switch (iv) {
 	case 0x00:	/* None */

@@ -457,6 +474,9 @@ omap_i2c_isr(int this_irq, void *dev_id)
 	u16 stat, w;
 	int count = 0;
 
+	if (dev->idle)
+		return IRQ_NONE;
+
 	bits = omap_i2c_read_reg(dev, OMAP_I2C_IE_REG);
 	while ((stat = (omap_i2c_read_reg(dev, OMAP_I2C_STAT_REG))) & bits) {
 		dev_dbg(dev->dev, "IRQ (ISR = 0x%04x)\n", stat);

@@ -575,7 +595,7 @@ omap_i2c_probe(struct platform_device *pdev)
 	if ((r = omap_i2c_get_clocks(dev)) != 0)
 		goto err_free_mem;
 
-	omap_i2c_enable_clocks(dev);
+	omap_i2c_unidle(dev);
 
 	if (cpu_is_omap15xx())
 		dev->rev1 = omap_i2c_read_reg(dev, OMAP_I2C_REV_REG) < 0x20;

@@ -610,7 +630,7 @@ omap_i2c_probe(struct platform_device *pdev)
 		goto err_free_irq;
 	}
 
-	omap_i2c_disable_clocks(dev);
+	omap_i2c_idle(dev);
 
 	return 0;

drivers/i2c/busses/i2c-omap.c err_free_irq: · line 644

Quoted 8 lines

drivers/i2c/busses/i2c-omap.c +1 -1

@@ -618,7 +638,7 @@ err_free_irq:
 	free_irq(dev->irq, dev);
 err_unuse_clocks:
 	omap_i2c_write_reg(dev, OMAP_I2C_CON_REG, 0);
-	omap_i2c_disable_clocks(dev);
+	omap_i2c_idle(dev);
 	omap_i2c_put_clocks(dev);
 err_free_mem:
 	platform_set_drvdata(pdev, NULL);

The rest of the patch looks OK to me.

Signature · 6 lines

Jean Delvare

_______________________________________________
i2c mailing list
[email protected]
http://lists.lm-sensors.org/mailman/listinfo/i2c

Tony Lindgren

author maintainer

2008-03-17 06:39 UTC #3 lore raw

In reply to Jean Delvare

Quoted 9 lines · Jean Delvare

Hi Tony,
Quoted 2 lines · Tony Lindgren
Here's an omap I2C fix that would be nice to get into 2.6.25 if still possible.
Yeah, that's possible, however the patch isn't totally trivial so I would appreciate if someone from the OMAP community could review it publicly and ack it. Then I'll push it to Linus quickly.

Well we've had this fix tested in linux-omap for about a month now and it really fixes the problem. I'll add that to the comments.

I don't know if anybody else has seen this problem repeatedly on omaps as it's quite rare condition, but if somebody else was seeing it, please Ack here! :)

Regards,

Tony

Tony Lindgren

author maintainer

2008-03-17 06:46 UTC #5 lore raw

In reply to Jean Delvare

Quoted 7 lines · Jean Delvare

Quoted 4 lines · Tony Lindgren
Hi Jean,
Here's an omap I2C fix that would be nice to get into 2.6.25 if still possible.
Review:

Thanks for looking throught it.

Quoted 53 lines

Quoted 53 lines
commit 8e4f19286e68d38d589451ad2a10545f2c40032d
Author: Tony Lindgren <[email protected]>
Date:   Mon Feb 18 20:16:00 2008 -0800
I2C: Fix unhandled fault in i2c-omap controller
If an I2C interrupt happens between disabling interface clock and functional clock, the interrupt handler will produce an external abort on non-linefetch error when trying to access driver registers while interface clock is disabled.
This patch fixes the problem by saving and disabling i2c-omap interrupt before turning off the clocks. Also disable functional clock before the interface clock as suggested by Paul Walmsley.
Patch also renames enable/disable_clocks functions to unidle/idle functions. Note that the driver is currently not taking advantage of the idle interrupts. To use the idle interrupts, driver would have to enable interface clock based on the idle interrupt and dev->idle flag.
Cc Paul Walmsley <[email protected]>
Signed-off-by Tony Lindgren <[email protected]>
drivers/i2c/busses/i2c-omap.c +6 -1
index da66397..862dd69 100644
@@ -128,6 +128,8 @@ struct omap_i2c_dev {
 	size_t			buf_len;
 	struct i2c_adapter	adapter;
 	unsigned		rev1:1;
+	unsigned		idle:1;
+	u16			iestate;	/* Saved interrupt register */
 };
 
 static inline void omap_i2c_write_reg(struct omap_i2c_dev *i2c_dev,
@@ -174,18 +176,30 @@ static void omap_i2c_put_clocks(struct omap_i2c_dev *dev)
 	}
 }
 
-static void omap_i2c_enable_clocks(struct omap_i2c_dev *dev)
+static void omap_i2c_unidle(struct omap_i2c_dev *dev)
 {
 	if (dev->iclk != NULL)
 		clk_enable(dev->iclk);
 	clk_enable(dev->fclk);
+	if (dev->iestate)
+		omap_i2c_write_reg(dev, OMAP_I2C_IE_REG, dev->iestate);
+	dev->idle = 0;
 }
 

Quoted 14 lines

drivers/i2c/busses/i2c-omap.c static void omap_i2c_put_clocks(struct omap_i2c_dev *dev) · line 197
Quoted 10 lines
drivers/i2c/busses/i2c-omap.c +8 -1
-static void omap_i2c_disable_clocks(struct omap_i2c_dev *dev)
+static void omap_i2c_idle(struct omap_i2c_dev *dev)
 {
+	u16 iv;
+
+	dev->idle = 1;
+	dev->iestate = omap_i2c_read_reg(dev, OMAP_I2C_IE_REG);
+	omap_i2c_write_reg(dev, OMAP_I2C_IE_REG, 0);
+	if (dev->rev1)
+		iv = omap_i2c_read_reg(dev, OMAP_I2C_IV_REG);
Reading OMAP_I2C_IV_REG but not doing anything with the value? This deserves at least a comment if this is done on purpose (but then I don't think you need the iv variable at all.)

Yeah for rev1 of the controller reading of IV (Interrupt Vector) clears the interrupts. I'll add a comment there.

Quoted 80 lines

Quoted 67 lines

drivers/i2c/busses/i2c-omap.c +13 -5

+	else
+		omap_i2c_write_reg(dev, OMAP_I2C_STAT_REG, dev->iestate);
+	clk_disable(dev->fclk);
 	if (dev->iclk != NULL)
 		clk_disable(dev->iclk);
-	clk_disable(dev->fclk);
 }
 
 static int omap_i2c_init(struct omap_i2c_dev *dev)

@@ -360,7 +374,7 @@ omap_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[], int num)
 	int i;
 	int r;
 
-	omap_i2c_enable_clocks(dev);
+	omap_i2c_unidle(dev);
 
 	if ((r = omap_i2c_wait_for_bb(dev)) < 0)
 		goto out;

@@ -374,7 +388,7 @@ omap_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[], int num)
 	if (r == 0)
 		r = num;
 out:
-	omap_i2c_disable_clocks(dev);
+	omap_i2c_idle(dev);
 	return r;
 }

@@ -403,6 +417,9 @@ omap_i2c_rev1_isr(int this_irq, void *dev_id)
 	struct omap_i2c_dev *dev = dev_id;
 	u16 iv, w;
 
+	if (dev->idle)
+		return IRQ_NONE;
+
 	iv = omap_i2c_read_reg(dev, OMAP_I2C_IV_REG);
 	switch (iv) {
 	case 0x00:	/* None */

@@ -457,6 +474,9 @@ omap_i2c_isr(int this_irq, void *dev_id)
 	u16 stat, w;
 	int count = 0;
 
+	if (dev->idle)
+		return IRQ_NONE;
+
 	bits = omap_i2c_read_reg(dev, OMAP_I2C_IE_REG);
 	while ((stat = (omap_i2c_read_reg(dev, OMAP_I2C_STAT_REG))) & bits) {
 		dev_dbg(dev->dev, "IRQ (ISR = 0x%04x)\n", stat);

@@ -575,7 +595,7 @@ omap_i2c_probe(struct platform_device *pdev)
 	if ((r = omap_i2c_get_clocks(dev)) != 0)
 		goto err_free_mem;
 
-	omap_i2c_enable_clocks(dev);
+	omap_i2c_unidle(dev);
 
 	if (cpu_is_omap15xx())
 		dev->rev1 = omap_i2c_read_reg(dev, OMAP_I2C_REV_REG) < 0x20;

@@ -610,7 +630,7 @@ omap_i2c_probe(struct platform_device *pdev)
 		goto err_free_irq;
 	}
 
-	omap_i2c_disable_clocks(dev);
+	omap_i2c_idle(dev);
 
 	return 0;

drivers/i2c/busses/i2c-omap.c err_free_irq: · line 644

Quoted 8 lines

drivers/i2c/busses/i2c-omap.c +1 -1

@@ -618,7 +638,7 @@ err_free_irq:
 	free_irq(dev->irq, dev);
 err_unuse_clocks:
 	omap_i2c_write_reg(dev, OMAP_I2C_CON_REG, 0);
-	omap_i2c_disable_clocks(dev);
+	omap_i2c_idle(dev);
 	omap_i2c_put_clocks(dev);
 err_free_mem:
 	platform_set_drvdata(pdev, NULL);

The rest of the patch looks OK to me.

Signature · 1 line

Jean Delvare

Tony Lindgren

author maintainer

2008-03-17 07:19 UTC #6 lore raw

PATCH 1/1 I2C: Fix unhandled fault in i2c-omap controller, take #2

Hi,

Here's an updated version

Regards,

Tony

commit 94a3ce6a20c3920e0e43991d38506134791e9d72

Author: Tony Lindgren <[email protected]>
Date:   Mon Feb 18 20:16:00 2008 -0800

I2C: Fix unhandled fault in i2c-omap controller

This patch fixes the problem by saving and disabling i2c-omap interrupt before turning off the clocks. Also disable functional clock before the interface clock as suggested by Paul Walmsley.

This patch has been tested in linux-omap tree with various omaps.

Cc [email protected]

Cc Paul Walmsley <[email protected]>

Signed-off-by Tony Lindgren <[email protected]>

drivers/i2c/busses/i2c-omap.c +28 -8

index da66397..7ba3177 100644
@@ -128,6 +128,8 @@ struct omap_i2c_dev {
 	size_t			buf_len;
 	struct i2c_adapter	adapter;
 	unsigned		rev1:1;
+	unsigned		idle:1;
+	u16			iestate;	/* Saved interrupt register */
 };
 
 static inline void omap_i2c_write_reg(struct omap_i2c_dev *i2c_dev,

@@ -174,18 +176,30 @@ static void omap_i2c_put_clocks(struct omap_i2c_dev *dev)
 	}
 }
 
-static void omap_i2c_enable_clocks(struct omap_i2c_dev *dev)
+static void omap_i2c_unidle(struct omap_i2c_dev *dev)
 {
 	if (dev->iclk != NULL)
 		clk_enable(dev->iclk);
 	clk_enable(dev->fclk);
+	if (dev->iestate)
+		omap_i2c_write_reg(dev, OMAP_I2C_IE_REG, dev->iestate);
+	dev->idle = 0;
 }
 
-static void omap_i2c_disable_clocks(struct omap_i2c_dev *dev)
+static void omap_i2c_idle(struct omap_i2c_dev *dev)
 {
+	u16 iv;
+
+	dev->idle = 1;
+	dev->iestate = omap_i2c_read_reg(dev, OMAP_I2C_IE_REG);
+	omap_i2c_write_reg(dev, OMAP_I2C_IE_REG, 0);
+	if (dev->rev1)
+		iv = omap_i2c_read_reg(dev, OMAP_I2C_IV_REG);	/* Read clears */
+	else
+		omap_i2c_write_reg(dev, OMAP_I2C_STAT_REG, dev->iestate);
+	clk_disable(dev->fclk);
 	if (dev->iclk != NULL)
 		clk_disable(dev->iclk);
-	clk_disable(dev->fclk);
 }
 
 static int omap_i2c_init(struct omap_i2c_dev *dev)

@@ -360,7 +374,7 @@ omap_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[], int num)
 	int i;
 	int r;
 
-	omap_i2c_enable_clocks(dev);
+	omap_i2c_unidle(dev);
 
 	if ((r = omap_i2c_wait_for_bb(dev)) < 0)
 		goto out;

@@ -374,7 +388,7 @@ omap_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[], int num)
 	if (r == 0)
 		r = num;
 out:
-	omap_i2c_disable_clocks(dev);
+	omap_i2c_idle(dev);
 	return r;
 }

@@ -403,6 +417,9 @@ omap_i2c_rev1_isr(int this_irq, void *dev_id)
 	struct omap_i2c_dev *dev = dev_id;
 	u16 iv, w;
 
+	if (dev->idle)
+		return IRQ_NONE;
+
 	iv = omap_i2c_read_reg(dev, OMAP_I2C_IV_REG);
 	switch (iv) {
 	case 0x00:	/* None */

@@ -457,6 +474,9 @@ omap_i2c_isr(int this_irq, void *dev_id)
 	u16 stat, w;
 	int count = 0;
 
+	if (dev->idle)
+		return IRQ_NONE;
+
 	bits = omap_i2c_read_reg(dev, OMAP_I2C_IE_REG);
 	while ((stat = (omap_i2c_read_reg(dev, OMAP_I2C_STAT_REG))) & bits) {
 		dev_dbg(dev->dev, "IRQ (ISR = 0x%04x)\n", stat);

@@ -575,7 +595,7 @@ omap_i2c_probe(struct platform_device *pdev)
 	if ((r = omap_i2c_get_clocks(dev)) != 0)
 		goto err_free_mem;
 
-	omap_i2c_enable_clocks(dev);
+	omap_i2c_unidle(dev);
 
 	if (cpu_is_omap15xx())
 		dev->rev1 = omap_i2c_read_reg(dev, OMAP_I2C_REV_REG) < 0x20;

@@ -610,7 +630,7 @@ omap_i2c_probe(struct platform_device *pdev)
 		goto err_free_irq;
 	}
 
-	omap_i2c_disable_clocks(dev);
+	omap_i2c_idle(dev);
 
 	return 0;

@@ -618,7 +638,7 @@ err_free_irq:
 	free_irq(dev->irq, dev);
 err_unuse_clocks:
 	omap_i2c_write_reg(dev, OMAP_I2C_CON_REG, 0);
-	omap_i2c_disable_clocks(dev);
+	omap_i2c_idle(dev);
 	omap_i2c_put_clocks(dev);
 err_free_mem:
 	platform_set_drvdata(pdev, NULL);

David Brownell

2008-03-17 08:07 UTC #7 lore raw

Re: [PATCH] I2C: Fix unhandled fault in i2c-omap controller, take #2

Quoted 12 lines · Tony Lindgren

(quoted diff) +0 -0

@@ -360,7 +374,7 @@ omap_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[], int num)
         int i;
         int r;

-       omap_i2c_enable_clocks(dev);
+       omap_i2c_unidle(dev);

        if ((r = omap_i2c_wait_for_bb(dev)) < 0)
                goto out;

drivers/i2c/busses/i2c-omap.c +0 -0

@@ -374,7 +388,7 @@ omap_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[], int num)
         if (r == 0)
                 r = num;

Quoted 6 lines

out:
-       omap_i2c_disable_clocks(dev);
+       omap_i2c_idle(dev);
        return r;
}

Wouldn't it be nice -- from a pure power management perspective of course! -- if more drivers were this careful about idling their clocks when they have no work to do. ;)

- Dave

--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Jean Delvare

2008-03-17 11:05 UTC #9 lore raw

Re: [PATCH] I2C: Fix unhandled fault in i2c-omap controller, take #2

In reply to a message not in the archive

Quoted 3 lines · Tony Lindgren

Hi,
Here's an updated version

Applied and queued for 2.6.25, thanks.

Signature · 6 lines

Jean Delvare

_______________________________________________
i2c mailing list
[email protected]
http://lists.lm-sensors.org/mailman/listinfo/i2c