i2c: omap: Do not enable the irq always
Subsystems (4)
PATCH 1/3 i2c: omap: Do not enable the irq always
Enable the irq in the transfer and disable it after the transfer is done.
--- drivers/i2c/busses/i2c-omap.c | 3 +++ 1 files changed, 3 insertions(+), 0 deletions(-)
drivers/i2c/busses/i2c-omap.c +3 -0
@@ -625,6 +625,7 @@ omap_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[], int num)if (IS_ERR_VALUE (r)) goto out; +enable_irq (dev->irq); r =omap_i2c_wait_for_bb (dev);if (r <0 ) goto out;
@@ -654,6 +655,7 @@ omap_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[], int num)omap_i2c_wait_for_bb (dev); out: +disable_irq (dev->irq);pm_runtime_mark_last_busy (dev->dev);pm_runtime_put_autosuspend (dev->dev);return r;
@@ -1179,6 +1181,7 @@ omap_i2c_probe(struct platform_device *pdev) goto err_unuse_clocks; } +disable_irq (dev->irq); adap = &dev->adapter;i2c_set_adapdata (adap, dev); adap->owner =THIS_MODULE ;
Signature · 1 line
1.7.5.4
PATCH 2/3 i2c: omap: Remove the redundant fifo clear
The omap_i2c_resize_fifo already takes care of the fifo clearing so, remove the (re)clearing of the fifo.
--- drivers/i2c/busses/i2c-omap.c | 5 ----- 1 files changed, 0 insertions(+), 5 deletions(-)
drivers/i2c/busses/i2c-omap.c +0 -5
@@ -524,11 +524,6 @@ static int omap_i2c_xfer_msg(struct i2c_adapter *adap,omap_i2c_write_reg (dev,OMAP_I2C_CNT_REG , dev->buf_len); -/* Clear the FIFO Buffers */ - w =omap_i2c_read_reg (dev,OMAP_I2C_BUF_REG ); - w |=OMAP_I2C_BUF_RXFIF_CLR |OMAP_I2C_BUF_TXFIF_CLR ; -omap_i2c_write_reg (dev,OMAP_I2C_BUF_REG , w); -INIT_COMPLETION (dev->cmd_complete); dev->cmd_err =0 ;
Signature · 1 line
1.7.5.4
PATCH 3/3 i2c: omap: Remove Address as slave wakeup
In reply to Shubhrajyoti DCurrently the slave support is not there in the current i2c. Lets remove the Address as slave wakeup from the Wakeup enable register.
--- drivers/i2c/busses/i2c-omap.c | 7 ++++++- 1 files changed, 6 insertions(+), 1 deletions(-)
drivers/i2c/busses/i2c-omap.c +6 -1
@@ -128,6 +128,11 @@ enum { OMAP_I2C_WE_DRDY_WE | OMAP_I2C_WE_ARDY_WE | \ OMAP_I2C_WE_NACK_WE | OMAP_I2C_WE_AL_WE) +#define OMAP_I2C_WE_ENABLED (OMAP_I2C_WE_XDR_WE | OMAP_I2C_WE_RDR_WE | \ + OMAP_I2C_WE_BF_WE | \ + OMAP_I2C_WE_STC_WE | OMAP_I2C_WE_GC_WE | \ + OMAP_I2C_WE_DRDY_WE | OMAP_I2C_WE_ARDY_WE | \ + OMAP_I2C_WE_NACK_WE | OMAP_I2C_WE_AL_WE)/* I2C Buffer Configuration Register (OMAP_I2C_BUF): */ #define OMAP_I2C_BUF_RDMA_EN (1 << 15)/* RX DMA channel enable */ #define OMAP_I2C_BUF_RXFIF_CLR (1 << 14)/* RX FIFO Clear */
@@ -326,7 +331,7 @@ static int omap_i2c_init(struct omap_i2c_dev *dev)* WFI instruction. * REVISIT: Some wkup sources might not be needed. */ - dev->westate =OMAP_I2C_WE_ALL ; + dev->westate =OMAP_I2C_WE_ENABLED ;omap_i2c_write_reg (dev,OMAP_I2C_WE_REG , dev->westate); }
Signature · 1 line
1.7.5.4
PATCH 1/3 i2c: omap: Do not enable the irq always
In reply to Shubhrajyoti DQuoted 2 lines · Shubhrajyoti D
Enable the irq in the transfer and disable it after the transfer is done.
This description is missing the most vital bits of information. In years to come, someone will wonder "why has this changed" and there's no reason given in the commit log.
Commit logs which are just mere translations from patch to English are evil. Instead, good commit logs briefly state what is being done and then go on to describe _why_ the change is necessary.
Quoted 41 lines
Signed-off-by Shubhrajyoti D <[email protected]>--- drivers/i2c/busses/i2c-omap.c | 3 +++ 1 files changed, 3 insertions(+), 0 deletions(-)drivers/i2c/busses/i2c-omap.c +3
-0@@ -625,6 +625,7 @@ omap_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[], int num)if (IS_ERR_VALUE (r)) goto out; +enable_irq (dev->irq); r =omap_i2c_wait_for_bb (dev);if (r <0 ) goto out;@@ -654,6 +655,7 @@ omap_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[], int num)omap_i2c_wait_for_bb (dev); out: +disable_irq (dev->irq);pm_runtime_mark_last_busy (dev->dev);pm_runtime_put_autosuspend (dev->dev);return r;@@ -1179,6 +1181,7 @@ omap_i2c_probe(struct platform_device *pdev) goto err_unuse_clocks; } +disable_irq (dev->irq); adap = &dev->adapter;i2c_set_adapdata (adap, dev); adap->owner =THIS_MODULE ;Signature · 7 lines
1.7.5.4 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel at lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
PATCH 1/3 i2c: omap: Do not enable the irq always
In reply to Shubhrajyoti D+Kalle, Grygorii, Huzefa
Quoted 21 lines · Shubhrajyoti D
Enable the irq in the transfer and disable it after the transfer is done.
Signed-off-by Shubhrajyoti D <[email protected]>--- drivers/i2c/busses/i2c-omap.c | 3 +++ 1 files changed, 3 insertions(+), 0 deletions(-)drivers/i2c/busses/i2c-omap.c +1
-0@@ -625,6 +625,7 @@ omap_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[], int num)if (IS_ERR_VALUE (r)) goto out; +enable_irq (dev->irq); r =omap_i2c_wait_for_bb (dev);if (r <0 ) goto out;
drivers/i2c/busses/i2c-omap.c omap_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[], int num) · line 658
Quoted 4 lines
drivers/i2c/busses/i2c-omap.c +1
-0@@ -654,6 +655,7 @@ omap_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[], int num)omap_i2c_wait_for_bb (dev); out: +disable_irq (dev->irq);
When using runtime PM with auto-suspend timeouts, why would you disable the IRQ before the runtime suspend handlers have run?
If you really want to do this, you probably should have these in the runtime PM callbacks. But I'll wait until you add a more descriptive changelog before I can really tell why this is being done. Based on the discussion in the patch from Kalle, I'm assuming this is to prevent interrups when I2C is being used by co-processors. If so, plese describe in the changelog.
That being said, doesn't the runtime suspend callback already disable IRQs at the device level instead of the INTC level?
Kevin
Quoted 11 lines
drivers/i2c/busses/i2c-omap.c +1
-0pm_runtime_mark_last_busy (dev->dev);pm_runtime_put_autosuspend (dev->dev);return r;@@ -1179,6 +1181,7 @@ omap_i2c_probe(struct platform_device *pdev) goto err_unuse_clocks; } +disable_irq (dev->irq); adap = &dev->adapter;i2c_set_adapdata (adap, dev); adap->owner =THIS_MODULE ;
PATCH 1/3 i2c: omap: Do not enable the irq always
Quoted 33 lines · Kevin Hilman
+Kalle, Grygorii, Huzefa
Quoted 21 lines · Shubhrajyoti D
Enable the irq in the transfer and disable it after the transfer is done.
Signed-off-by Shubhrajyoti D <[email protected]>--- drivers/i2c/busses/i2c-omap.c | 3 +++ 1 files changed, 3 insertions(+), 0 deletions(-)drivers/i2c/busses/i2c-omap.c +1
-0@@ -625,6 +625,7 @@ omap_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[], int num)if (IS_ERR_VALUE (r)) goto out; +enable_irq (dev->irq); r =omap_i2c_wait_for_bb (dev);if (r <0 ) goto out;drivers/i2c/busses/i2c-omap.c omap_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[], int num) · line 658
Quoted 4 lines
drivers/i2c/busses/i2c-omap.c +1
-0@@ -654,6 +655,7 @@ omap_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[], int num)omap_i2c_wait_for_bb (dev); out: +disable_irq (dev->irq);When using runtime PM with auto-suspend timeouts, why would you disable the IRQ before the runtime suspend handlers have run?
Quoted 2 lines
If you really want to do this, you probably should have these in the runtime PM callbacks.
OK.
Quoted 5 lines
But I'll wait until you add a more descriptive
changelog before I can really tell why this is being done. Based on the discussion in the patch from Kalle, I'm assuming this is to prevent interrups when I2C is being used by co-processors. If so, plese describe in the changelog.
OK.
Quoted 3 lines
That being said, doesn't the runtime suspend callback already disable IRQs at the device level instead of the INTC level?
My idea is that the device may get reconfigured by the other processor. I felt intc is the only way. do you agree?
Quoted 18 lines
Kevin
drivers/i2c/busses/i2c-omap.c omap_i2c_probe(struct platform_device *pdev) · line 1187
Quoted 11 lines
drivers/i2c/busses/i2c-omap.c +1
-0pm_runtime_mark_last_busy (dev->dev);pm_runtime_put_autosuspend (dev->dev);return r;@@ -1179,6 +1181,7 @@ omap_i2c_probe(struct platform_device *pdev) goto err_unuse_clocks; } +disable_irq (dev->irq); adap = &dev->adapter;i2c_set_adapdata (adap, dev); adap->owner =THIS_MODULE ;--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majordomo at vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
PATCH 1/3 i2c: omap: Do not enable the irq always
In reply to Shubhrajyoti D+Kalle, Grygorii, Huzefa
Quoted 21 lines · Shubhrajyoti D
Enable the irq in the transfer and disable it after the transfer is done.
Signed-off-by Shubhrajyoti D <[email protected]>--- drivers/i2c/busses/i2c-omap.c | 3 +++ 1 files changed, 3 insertions(+), 0 deletions(-)drivers/i2c/busses/i2c-omap.c +1
-0@@ -625,6 +625,7 @@ omap_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[], int num)if (IS_ERR_VALUE (r)) goto out; +enable_irq (dev->irq); r =omap_i2c_wait_for_bb (dev);if (r <0 ) goto out;
drivers/i2c/busses/i2c-omap.c omap_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[], int num) · line 658
Quoted 4 lines
drivers/i2c/busses/i2c-omap.c +1
-0@@ -654,6 +655,7 @@ omap_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[], int num)omap_i2c_wait_for_bb (dev); out: +disable_irq (dev->irq);
When using runtime PM with auto-suspend timeouts, why would you disable the IRQ before the runtime suspend handlers have run?
If you really want to do this, you probably should have these in the runtime PM callbacks. But I'll wait until you add a more descriptive changelog before I can really tell why this is being done. Based on the discussion in the patch from Kalle, I'm assuming this is to prevent interrups when I2C is being used by co-processors. If so, plese describe in the changelog.
That being said, doesn't the runtime suspend callback already disable IRQs at the device level instead of the INTC level?
Kevin
Quoted 11 lines
drivers/i2c/busses/i2c-omap.c +1
-0pm_runtime_mark_last_busy (dev->dev);pm_runtime_put_autosuspend (dev->dev);return r;@@ -1179,6 +1181,7 @@ omap_i2c_probe(struct platform_device *pdev) goto err_unuse_clocks; } +disable_irq (dev->irq); adap = &dev->adapter;i2c_set_adapdata (adap, dev); adap->owner =THIS_MODULE ;
PATCH 1/3 i2c: omap: Do not enable the irq always
Quoted 10 lines · Kevin Hilman
When using runtime PM with auto-suspend timeouts, why would you disable the IRQ before the runtime suspend handlers have run?
If you really want to do this, you probably should have these in the runtime PM callbacks. But I'll wait until you add a more descriptive changelog before I can really tell why this is being done. Based on the discussion in the patch from Kalle, I'm assuming this is to prevent interrups when I2C is being used by co-processors. If so, plese describe in the changelog.
Quoted 2 lines
That being said, doesn't the runtime suspend callback already disable IRQs at the device level instead of the INTC level?
I thought of not relying on the intc as the registers may be reconfigured.
Quoted 1 line
Kevin