* [PATCHv9 00/10] I2C fixes
@ 2012-05-02 14:32 Shubhrajyoti D
2012-05-02 14:32 ` [PATCHv9 01/10] I2C: OMAP: make omap_i2c_unidle/idle functions depend on CONFIG_PM_RUNTIME Shubhrajyoti D
` (12 more replies)
0 siblings, 13 replies; 24+ messages in thread
From: Shubhrajyoti D @ 2012-05-02 14:32 UTC (permalink / raw)
To: linux-arm-kernel
The patch series does the following
- Warn fixes if CONFIG_PM_RUNTIME is not selected.
- In case of i2c remove register access was done without any
get_sync fix the same.
- Folds a patch from Tasslehoff to prevent any merge conflicts.
- Prevents the XDUF flag to be set if the underflow condition is not met.
- As per discussion in [1] .Adds a patch to rename the 1p153 errata and
use the unique id instead as the section number in the recent errata
docs has changed.
v9:
Fix the comments from Wolfram Sang
[1] http://www.spinics.net/lists/linux-i2c/msg07607.html
Tested on omap4sdp and omap3sdp.
The following changes since commit b821861b905a79f71746945237968c3382d99adc:
Merge tag 'ktest-for-v3.4-rc5' of git://git.kernel.org/pub/scm/linux/kernel/git/rostedt/linux-ktest (2012-05-01 19:43:34 -0700)
are available in the git repository at:
git://gitorious.org/linus-tree/linus-tree.git i2c_omap-fixes
Shubhrajyoti D (9):
I2C: OMAP: make omap_i2c_unidle/idle functions depend on CONFIG_PM_RUNTIME
I2C: OMAP: Fix the mismatch of pm_runtime enable and disable
I2C: OMAP: Fix the interrupt clearing in OMAP4
I2C: OMAP: Prevent the register access after pm_runtime_put in probe
I2C: OMAP: Don't check if wait_for_completion_timeout() returns less than zero
I2C: OMAP: Fix the crash in i2c remove
I2C: OMAP: Handle error check for pm runtime
I2C: OMAP: Do not set the XUDF(Transmit underflow) if the underflow is not reached
I2C: OMAP: Rename the 1p153 to the erratum id i462
Tasslehoff Kjappfot (1):
I2C: OMAP: prevent the overwrite of the errata flags
drivers/i2c/busses/i2c-omap.c | 127 ++++++++++++++++++++---------------------
1 files changed, 62 insertions(+), 65 deletions(-)
--
1.7.5.4
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCHv9 01/10] I2C: OMAP: make omap_i2c_unidle/idle functions depend on CONFIG_PM_RUNTIME
2012-05-02 14:32 [PATCHv9 00/10] I2C fixes Shubhrajyoti D
@ 2012-05-02 14:32 ` Shubhrajyoti D
2012-05-02 14:32 ` [PATCHv9 02/10] I2C: OMAP: Fix the mismatch of pm_runtime enable and disable Shubhrajyoti D
` (11 subsequent siblings)
12 siblings, 0 replies; 24+ messages in thread
From: Shubhrajyoti D @ 2012-05-02 14:32 UTC (permalink / raw)
To: linux-arm-kernel
The functions omap_i2c_unidle/idle are called from omap_i2c_runtime_resume
and omap_i2c_runtime_suspend which is compiled for CONFIG_PM_RUNTIME.
This patch removes the omap_i2c_unidle/idle functions and folds them
into the runtime callbacks.
This fixes the below warn when CONFIG_PM_RUNTIME is not defined
CC arch/arm/mach-omap2/board-ti8168evm.o
drivers/i2c/busses/i2c-omap.c:272: warning: 'omap_i2c_unidle' defined but not used
drivers/i2c/busses/i2c-omap.c:293: warning: 'omap_i2c_idle' defined but not used
CC net/ipv4/ip_forward.o
Signed-off-by: Shubhrajyoti D <shubhrajyoti@ti.com>
---
drivers/i2c/busses/i2c-omap.c | 75 +++++++++++++++++-----------------------
1 files changed, 32 insertions(+), 43 deletions(-)
diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c
index 801df60..4f4188d 100644
--- a/drivers/i2c/busses/i2c-omap.c
+++ b/drivers/i2c/busses/i2c-omap.c
@@ -269,47 +269,6 @@ static inline u16 omap_i2c_read_reg(struct omap_i2c_dev *i2c_dev, int reg)
(i2c_dev->regs[reg] << i2c_dev->reg_shift));
}
-static void omap_i2c_unidle(struct omap_i2c_dev *dev)
-{
- if (dev->flags & OMAP_I2C_FLAG_RESET_REGS_POSTIDLE) {
- omap_i2c_write_reg(dev, OMAP_I2C_CON_REG, 0);
- omap_i2c_write_reg(dev, OMAP_I2C_PSC_REG, dev->pscstate);
- omap_i2c_write_reg(dev, OMAP_I2C_SCLL_REG, dev->scllstate);
- omap_i2c_write_reg(dev, OMAP_I2C_SCLH_REG, dev->sclhstate);
- omap_i2c_write_reg(dev, OMAP_I2C_BUF_REG, dev->bufstate);
- omap_i2c_write_reg(dev, OMAP_I2C_SYSC_REG, dev->syscstate);
- omap_i2c_write_reg(dev, OMAP_I2C_WE_REG, dev->westate);
- omap_i2c_write_reg(dev, OMAP_I2C_CON_REG, OMAP_I2C_CON_EN);
- }
-
- /*
- * Don't write to this register if the IE state is 0 as it can
- * cause deadlock.
- */
- if (dev->iestate)
- omap_i2c_write_reg(dev, OMAP_I2C_IE_REG, dev->iestate);
-}
-
-static void omap_i2c_idle(struct omap_i2c_dev *dev)
-{
- u16 iv;
-
- dev->iestate = omap_i2c_read_reg(dev, OMAP_I2C_IE_REG);
- if (dev->dtrev == OMAP_I2C_IP_VERSION_2)
- omap_i2c_write_reg(dev, OMAP_I2C_IP_V2_IRQENABLE_CLR, 1);
- else
- omap_i2c_write_reg(dev, OMAP_I2C_IE_REG, 0);
-
- if (dev->rev < OMAP_I2C_OMAP1_REV_2) {
- iv = omap_i2c_read_reg(dev, OMAP_I2C_IV_REG); /* Read clears */
- } else {
- omap_i2c_write_reg(dev, OMAP_I2C_STAT_REG, dev->iestate);
-
- /* Flush posted write */
- omap_i2c_read_reg(dev, OMAP_I2C_STAT_REG);
- }
-}
-
static int omap_i2c_init(struct omap_i2c_dev *dev)
{
u16 psc = 0, scll = 0, sclh = 0, buf = 0;
@@ -1163,8 +1122,22 @@ static int omap_i2c_runtime_suspend(struct device *dev)
{
struct platform_device *pdev = to_platform_device(dev);
struct omap_i2c_dev *_dev = platform_get_drvdata(pdev);
+ u16 iv;
+
+ _dev->iestate = omap_i2c_read_reg(_dev, OMAP_I2C_IE_REG);
+ if (_dev->dtrev == OMAP_I2C_IP_VERSION_2)
+ omap_i2c_write_reg(_dev, OMAP_I2C_IP_V2_IRQENABLE_CLR, 1);
+ else
+ omap_i2c_write_reg(_dev, OMAP_I2C_IE_REG, 0);
+
+ if (_dev->rev < OMAP_I2C_OMAP1_REV_2) {
+ iv = omap_i2c_read_reg(_dev, OMAP_I2C_IV_REG); /* Read clears */
+ } else {
+ omap_i2c_write_reg(_dev, OMAP_I2C_STAT_REG, _dev->iestate);
- omap_i2c_idle(_dev);
+ /* Flush posted write */
+ omap_i2c_read_reg(_dev, OMAP_I2C_STAT_REG);
+ }
return 0;
}
@@ -1174,7 +1147,23 @@ static int omap_i2c_runtime_resume(struct device *dev)
struct platform_device *pdev = to_platform_device(dev);
struct omap_i2c_dev *_dev = platform_get_drvdata(pdev);
- omap_i2c_unidle(_dev);
+ if (_dev->flags & OMAP_I2C_FLAG_RESET_REGS_POSTIDLE) {
+ omap_i2c_write_reg(_dev, OMAP_I2C_CON_REG, 0);
+ omap_i2c_write_reg(_dev, OMAP_I2C_PSC_REG, _dev->pscstate);
+ omap_i2c_write_reg(_dev, OMAP_I2C_SCLL_REG, _dev->scllstate);
+ omap_i2c_write_reg(_dev, OMAP_I2C_SCLH_REG, _dev->sclhstate);
+ omap_i2c_write_reg(_dev, OMAP_I2C_BUF_REG, _dev->bufstate);
+ omap_i2c_write_reg(_dev, OMAP_I2C_SYSC_REG, _dev->syscstate);
+ omap_i2c_write_reg(_dev, OMAP_I2C_WE_REG, _dev->westate);
+ omap_i2c_write_reg(_dev, OMAP_I2C_CON_REG, OMAP_I2C_CON_EN);
+ }
+
+ /*
+ * Don't write to this register if the IE state is 0 as it can
+ * cause deadlock.
+ */
+ if (_dev->iestate)
+ omap_i2c_write_reg(_dev, OMAP_I2C_IE_REG, _dev->iestate);
return 0;
}
--
1.7.5.4
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCHv9 02/10] I2C: OMAP: Fix the mismatch of pm_runtime enable and disable
2012-05-02 14:32 [PATCHv9 00/10] I2C fixes Shubhrajyoti D
2012-05-02 14:32 ` [PATCHv9 01/10] I2C: OMAP: make omap_i2c_unidle/idle functions depend on CONFIG_PM_RUNTIME Shubhrajyoti D
@ 2012-05-02 14:32 ` Shubhrajyoti D
2012-05-25 21:57 ` Kevin Hilman
2012-05-02 14:32 ` [PATCHv9 03/10] I2C: OMAP: Fix the interrupt clearing in OMAP4 Shubhrajyoti D
` (10 subsequent siblings)
12 siblings, 1 reply; 24+ messages in thread
From: Shubhrajyoti D @ 2012-05-02 14:32 UTC (permalink / raw)
To: linux-arm-kernel
Currently the i2c driver calls the pm_runtime_enable and never
the disable. This may cause a warning when pm_runtime_enable
checks for the count match.Attempting to fix the same by calling
pm_runtime_disable in the error and the remove path.
Cc: Kevin Hilman <khilman@ti.com>
Cc: Rajendra Nayak <rnayak@ti.com>
Signed-off-by: Shubhrajyoti D <shubhrajyoti@ti.com>
---
drivers/i2c/busses/i2c-omap.c | 2 ++
1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c
index 4f4188d..c851672 100644
--- a/drivers/i2c/busses/i2c-omap.c
+++ b/drivers/i2c/busses/i2c-omap.c
@@ -1090,6 +1090,7 @@ err_unuse_clocks:
omap_i2c_write_reg(dev, OMAP_I2C_CON_REG, 0);
pm_runtime_put(dev->dev);
iounmap(dev->base);
+ pm_runtime_disable(&pdev->dev);
err_free_mem:
platform_set_drvdata(pdev, NULL);
kfree(dev);
@@ -1110,6 +1111,7 @@ omap_i2c_remove(struct platform_device *pdev)
free_irq(dev->irq, dev);
i2c_del_adapter(&dev->adapter);
omap_i2c_write_reg(dev, OMAP_I2C_CON_REG, 0);
+ pm_runtime_disable(&pdev->dev);
iounmap(dev->base);
kfree(dev);
mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
--
1.7.5.4
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCHv9 03/10] I2C: OMAP: Fix the interrupt clearing in OMAP4
2012-05-02 14:32 [PATCHv9 00/10] I2C fixes Shubhrajyoti D
2012-05-02 14:32 ` [PATCHv9 01/10] I2C: OMAP: make omap_i2c_unidle/idle functions depend on CONFIG_PM_RUNTIME Shubhrajyoti D
2012-05-02 14:32 ` [PATCHv9 02/10] I2C: OMAP: Fix the mismatch of pm_runtime enable and disable Shubhrajyoti D
@ 2012-05-02 14:32 ` Shubhrajyoti D
2012-05-02 14:32 ` [PATCHv9 04/10] I2C: OMAP: Prevent the register access after pm_runtime_put in probe Shubhrajyoti D
` (9 subsequent siblings)
12 siblings, 0 replies; 24+ messages in thread
From: Shubhrajyoti D @ 2012-05-02 14:32 UTC (permalink / raw)
To: linux-arm-kernel
On OMAP4 we were writing 1 to IRQENABLE_CLR which cleared only
the arbitration lost interrupt. The patch intends to fix the same by writing 0
to the IE register clearing all interrupts.
This is based on the work done by Vikram Pandita <vikram.pandita@ti.com>.
The changes from the original patch ...
- Does not use the IRQENABLE_CLR register to clear as it is not mentioned
to be legacy register IRQENABLE_CLR helps in atomically
setting/clearing specific interrupts, instead use the OMAP_I2C_IE_REG as we
are clearing all interrupts.
Cc: Vikram Pandita <vikram.pandita@ti.com>
Signed-off-by: Shubhrajyoti D <shubhrajyoti@ti.com>
---
drivers/i2c/busses/i2c-omap.c | 6 ++----
1 files changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c
index c851672..bf07ffd 100644
--- a/drivers/i2c/busses/i2c-omap.c
+++ b/drivers/i2c/busses/i2c-omap.c
@@ -1127,10 +1127,8 @@ static int omap_i2c_runtime_suspend(struct device *dev)
u16 iv;
_dev->iestate = omap_i2c_read_reg(_dev, OMAP_I2C_IE_REG);
- if (_dev->dtrev == OMAP_I2C_IP_VERSION_2)
- omap_i2c_write_reg(_dev, OMAP_I2C_IP_V2_IRQENABLE_CLR, 1);
- else
- omap_i2c_write_reg(_dev, OMAP_I2C_IE_REG, 0);
+
+ omap_i2c_write_reg(_dev, OMAP_I2C_IE_REG, 0);
if (_dev->rev < OMAP_I2C_OMAP1_REV_2) {
iv = omap_i2c_read_reg(_dev, OMAP_I2C_IV_REG); /* Read clears */
--
1.7.5.4
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCHv9 04/10] I2C: OMAP: Prevent the register access after pm_runtime_put in probe
2012-05-02 14:32 [PATCHv9 00/10] I2C fixes Shubhrajyoti D
` (2 preceding siblings ...)
2012-05-02 14:32 ` [PATCHv9 03/10] I2C: OMAP: Fix the interrupt clearing in OMAP4 Shubhrajyoti D
@ 2012-05-02 14:32 ` Shubhrajyoti D
2012-05-02 14:32 ` [PATCHv9 05/10] I2C: OMAP: Don't check if wait_for_completion_timeout() returns less than zero Shubhrajyoti D
` (8 subsequent siblings)
12 siblings, 0 replies; 24+ messages in thread
From: Shubhrajyoti D @ 2012-05-02 14:32 UTC (permalink / raw)
To: linux-arm-kernel
Currently in probe
pm_runtime_put(dev->dev);
...
/* i2c device drivers may be active on return from add_adapter() */
adap->nr = pdev->id;
r = i2c_add_numbered_adapter(adap);
if (r) {
dev_err(dev->dev, "failure adding adapter\n");
goto err_free_irq;
}
...
return 0;
err_free_irq:
free_irq(dev->irq, dev);
err_unuse_clocks:
omap_i2c_write_reg(dev, OMAP_I2C_CON_REG, 0);
pm_runtime_put(dev->dev);
This may access the i2c registers without the clocks in the error cases.
Fix the same by moving the pm_runtime_put after the error check.
Signed-off-by: Shubhrajyoti D <shubhrajyoti@ti.com>
---
drivers/i2c/busses/i2c-omap.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c
index bf07ffd..1777d79 100644
--- a/drivers/i2c/busses/i2c-omap.c
+++ b/drivers/i2c/busses/i2c-omap.c
@@ -1061,8 +1061,6 @@ omap_i2c_probe(struct platform_device *pdev)
dev_info(dev->dev, "bus %d rev%d.%d.%d at %d kHz\n", pdev->id,
dev->dtrev, dev->rev >> 4, dev->rev & 0xf, dev->speed);
- pm_runtime_put(dev->dev);
-
adap = &dev->adapter;
i2c_set_adapdata(adap, dev);
adap->owner = THIS_MODULE;
@@ -1082,6 +1080,8 @@ omap_i2c_probe(struct platform_device *pdev)
of_i2c_register_devices(adap);
+ pm_runtime_put(dev->dev);
+
return 0;
err_free_irq:
--
1.7.5.4
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCHv9 05/10] I2C: OMAP: Don't check if wait_for_completion_timeout() returns less than zero
2012-05-02 14:32 [PATCHv9 00/10] I2C fixes Shubhrajyoti D
` (3 preceding siblings ...)
2012-05-02 14:32 ` [PATCHv9 04/10] I2C: OMAP: Prevent the register access after pm_runtime_put in probe Shubhrajyoti D
@ 2012-05-02 14:32 ` Shubhrajyoti D
2012-05-02 14:32 ` [PATCHv9 06/10] I2C: OMAP: Fix the crash in i2c remove Shubhrajyoti D
` (7 subsequent siblings)
12 siblings, 0 replies; 24+ messages in thread
From: Shubhrajyoti D @ 2012-05-02 14:32 UTC (permalink / raw)
To: linux-arm-kernel
By definition, wait_for_completion_timeout() returns an unsigned value and
therefore, it is not necessary to check if the return value is less than zero
as this is not possible.
This is based on a patch from Jon Hunter <jon-hunter@ti.com>
Changes from his patch
- Declare a long as the wait_for_completion_timeout returns long.
Original patch is
http://git.omapzoom.org/?p=kernel/omap.git;a=commitdiff;h=ea02cece7b0000bc736e60c4188a11aaa74bc6e6
Signed-off-by: Jon Hunter <jon-hunter@ti.com>
Signed-off-by: Shubhrajyoti D <shubhrajyoti@ti.com>
---
drivers/i2c/busses/i2c-omap.c | 10 ++++------
1 files changed, 4 insertions(+), 6 deletions(-)
diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c
index 1777d79..fec8d5c 100644
--- a/drivers/i2c/busses/i2c-omap.c
+++ b/drivers/i2c/busses/i2c-omap.c
@@ -473,7 +473,7 @@ static int omap_i2c_xfer_msg(struct i2c_adapter *adap,
struct i2c_msg *msg, int stop)
{
struct omap_i2c_dev *dev = i2c_get_adapdata(adap);
- int r;
+ unsigned long timeout;
u16 w;
dev_dbg(dev->dev, "addr: 0x%04x, len: %d, flags: 0x%x, stop: %d\n",
@@ -541,12 +541,10 @@ static int omap_i2c_xfer_msg(struct i2c_adapter *adap,
* REVISIT: We should abort the transfer on signals, but the bus goes
* into arbitration and we're currently unable to recover from it.
*/
- r = wait_for_completion_timeout(&dev->cmd_complete,
- OMAP_I2C_TIMEOUT);
+ timeout = wait_for_completion_timeout(&dev->cmd_complete,
+ OMAP_I2C_TIMEOUT);
dev->buf_len = 0;
- if (r < 0)
- return r;
- if (r == 0) {
+ if (timeout == 0) {
dev_err(dev->dev, "controller timed out\n");
omap_i2c_init(dev);
return -ETIMEDOUT;
--
1.7.5.4
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCHv9 06/10] I2C: OMAP: Fix the crash in i2c remove
2012-05-02 14:32 [PATCHv9 00/10] I2C fixes Shubhrajyoti D
` (4 preceding siblings ...)
2012-05-02 14:32 ` [PATCHv9 05/10] I2C: OMAP: Don't check if wait_for_completion_timeout() returns less than zero Shubhrajyoti D
@ 2012-05-02 14:32 ` Shubhrajyoti D
2012-05-12 18:10 ` Wolfram Sang
2012-05-02 14:32 ` [PATCHv9 07/10] I2C: OMAP: Handle error check for pm runtime Shubhrajyoti D
` (6 subsequent siblings)
12 siblings, 1 reply; 24+ messages in thread
From: Shubhrajyoti D @ 2012-05-02 14:32 UTC (permalink / raw)
To: linux-arm-kernel
In omap_i2c_remove we are accessing the I2C_CON register without
enabling the clocks. Fix the same by enabling the clocks and disabling
it.
This fixes the following crash.
[ 154.723022] ------------[ cut here ]------------
[ 154.725677] WARNING: at arch/arm/mach-omap2/omap_l3_noc.c:112 l3_interrupt_handler+0x1b4/0x1c4()
[ 154.725677] L3 custom error: MASTER:MPU TARGET:L4 PER2
[ 154.742614] Modules linked in: i2c_omap(-)
[ 154.746948] Backtrace:
[ 154.746948] [<c0013078>] (dump_backtrace+0x0/0x110) from [<c026c158>] (dump_stack+0x18/0x1c)
[ 154.752716] r6:00000070 r5:c002c43c r4:df9b9e98 r3:df9b8000
[ 154.764465] [<c026c140>] (dump_stack+0x0/0x1c) from [<c0041a2c>] (warn_slowpath_common+0x5c/0x6c)
[ 154.768341] [<c00419d0>] (warn_slowpath_common+0x0/0x6c) from [<c0041ae0>] (warn_slowpath_fmt+0x38/0x40)
[ 154.776153] r8:00000180 r7:c0361594 r6:c0379b48 r5:00080003 r4:e0838b00
[ 154.790771] r3:00000009
[ 154.791778] [<c0041aa8>] (warn_slowpath_fmt+0x0/0x40) from [<c002c43c>] (l3_interrupt_handler+0x1b4/0x1c4)
[ 154.803710] r3:c0361598 r2:c02ef74c
[ 154.807403] [<c002c288>] (l3_interrupt_handler+0x0/0x1c4) from [<c0085f44>] (handle_irq_event_percpu+0x58/0
[ 154.818237] r8:0000002a r7:00000000 r6:00000000 r5:df808054 r4:df8893c0
[ 154.825378] [<c0085eec>] (handle_irq_event_percpu+0x0/0x188) from [<c00860b8>] (handle_irq_event+0x44/0x64)
[ 154.835662] [<c0086074>] (handle_irq_event+0x0/0x64) from [<c0088ec0>] (handle_fasteoi_irq+0xa4/0x10c)
[ 154.845458] r6:0000002a r5:df808054 r4:df808000 r3:c034a150
[ 154.846466] [<c0088e1c>] (handle_fasteoi_irq+0x0/0x10c) from [<c0085ed0>] (generic_handle_irq+0x30/0x38)
[ 154.854278] r5:c034aa48 r4:0000002a
[ 154.862091] [<c0085ea0>] (generic_handle_irq+0x0/0x38) from [<c000fd38>] (handle_IRQ+0x60/0xc0)
[ 154.874450] r4:c034ea70 r3:000001f8
[ 154.878234] [<c000fcd8>] (handle_IRQ+0x0/0xc0) from [<c0008478>] (gic_handle_irq+0x20/0x5c)
[ 154.887023] r7:ffffff40 r6:df9b9fb0 r5:c034e2b4 r4:0000001a
[ 154.887054] [<c0008458>] (gic_handle_irq+0x0/0x5c) from [<c000ea80>] (__irq_usr+0x40/0x60)
[ 154.901153] Exception stack(0xdf9b9fb0 to 0xdf9b9ff8)
[ 154.907104] 9fa0: beaf1f04 4006be00 0000000f 0000000c
[ 154.915710] 9fc0: 4006c000 00000000 00008034 ffffff40 00000007 00000000 00000000 0007b8d7
[ 154.916778] 9fe0: 00000000 beaf1b68 0000d23c 4005baf0 80000010 ffffffff
[ 154.931335] r6:ffffffff r5:80000010 r4:4005baf0 r3:beaf1f04
[ 154.937316] ---[ end trace 1b75b31a2719ed21 ]--
Cc: Kevin Hilman <khilman@ti.com>
Cc: Rajendra Nayak <rnayak@ti.com>
Signed-off-by: Shubhrajyoti D <shubhrajyoti@ti.com>
---
drivers/i2c/busses/i2c-omap.c | 2 ++
1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c
index fec8d5c..44e8cfa 100644
--- a/drivers/i2c/busses/i2c-omap.c
+++ b/drivers/i2c/busses/i2c-omap.c
@@ -1108,7 +1108,9 @@ omap_i2c_remove(struct platform_device *pdev)
free_irq(dev->irq, dev);
i2c_del_adapter(&dev->adapter);
+ pm_runtime_get_sync(&pdev->dev);
omap_i2c_write_reg(dev, OMAP_I2C_CON_REG, 0);
+ pm_runtime_put(&pdev->dev);
pm_runtime_disable(&pdev->dev);
iounmap(dev->base);
kfree(dev);
--
1.7.5.4
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCHv9 07/10] I2C: OMAP: Handle error check for pm runtime
2012-05-02 14:32 [PATCHv9 00/10] I2C fixes Shubhrajyoti D
` (5 preceding siblings ...)
2012-05-02 14:32 ` [PATCHv9 06/10] I2C: OMAP: Fix the crash in i2c remove Shubhrajyoti D
@ 2012-05-02 14:32 ` Shubhrajyoti D
2012-05-25 22:06 ` Kevin Hilman
2012-05-02 14:32 ` [PATCHv9 08/10] I2C: OMAP: prevent the overwrite of the errata flags Shubhrajyoti D
` (5 subsequent siblings)
12 siblings, 1 reply; 24+ messages in thread
From: Shubhrajyoti D @ 2012-05-02 14:32 UTC (permalink / raw)
To: linux-arm-kernel
If PM runtime get_sync fails return with the error
so that no further reads/writes goes through the interface.
This will avoid possible abort. Add a error message in case
of failure with the cause of the failure.
Signed-off-by: Shubhrajyoti D <shubhrajyoti@ti.com>
---
drivers/i2c/busses/i2c-omap.c | 14 +++++++++++---
1 files changed, 11 insertions(+), 3 deletions(-)
diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c
index 44e8cfa..a72874e 100644
--- a/drivers/i2c/busses/i2c-omap.c
+++ b/drivers/i2c/busses/i2c-omap.c
@@ -585,7 +585,9 @@ omap_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[], int num)
int i;
int r;
- pm_runtime_get_sync(dev->dev);
+ r = pm_runtime_get_sync(dev->dev);
+ if (r < 0)
+ return r;
r = omap_i2c_wait_for_bb(dev);
if (r < 0)
@@ -1011,7 +1013,9 @@ omap_i2c_probe(struct platform_device *pdev)
dev->regs = (u8 *)reg_map_ip_v1;
pm_runtime_enable(dev->dev);
- pm_runtime_get_sync(dev->dev);
+ r = pm_runtime_get_sync(dev->dev);
+ if (r < 0)
+ goto err_free_mem;
dev->rev = omap_i2c_read_reg(dev, OMAP_I2C_REV_REG) & 0xff;
@@ -1103,12 +1107,16 @@ omap_i2c_remove(struct platform_device *pdev)
{
struct omap_i2c_dev *dev = platform_get_drvdata(pdev);
struct resource *mem;
+ int ret;
platform_set_drvdata(pdev, NULL);
free_irq(dev->irq, dev);
i2c_del_adapter(&dev->adapter);
- pm_runtime_get_sync(&pdev->dev);
+ ret = pm_runtime_get_sync(&pdev->dev);
+ if (ret < 0)
+ return ret;
+
omap_i2c_write_reg(dev, OMAP_I2C_CON_REG, 0);
pm_runtime_put(&pdev->dev);
pm_runtime_disable(&pdev->dev);
--
1.7.5.4
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCHv9 08/10] I2C: OMAP: prevent the overwrite of the errata flags
2012-05-02 14:32 [PATCHv9 00/10] I2C fixes Shubhrajyoti D
` (6 preceding siblings ...)
2012-05-02 14:32 ` [PATCHv9 07/10] I2C: OMAP: Handle error check for pm runtime Shubhrajyoti D
@ 2012-05-02 14:32 ` Shubhrajyoti D
2012-05-02 14:32 ` [PATCHv9 09/10] I2C: OMAP: Do not set the XUDF(Transmit underflow) if the underflow is not reached Shubhrajyoti D
` (4 subsequent siblings)
12 siblings, 0 replies; 24+ messages in thread
From: Shubhrajyoti D @ 2012-05-02 14:32 UTC (permalink / raw)
To: linux-arm-kernel
From: Tasslehoff Kjappfot <tasskjapp@gmail.com>
i2c_probe set the dev->errata flag, but omap_i2c_init cleared the flag again.
Prevent the overwrite of the errata flags.Move the errata handling to a unified
place in probe to prevent such errors. Thus preventing the overwrite of erratum
1p153.
Signed-off-by: Tasslehoff Kjappfot <tasskjapp@gmail.com>
Signed-off-by: Shubhrajyoti D <shubhrajyoti@ti.com>
---
drivers/i2c/busses/i2c-omap.c | 10 +++++-----
1 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c
index a72874e..43b0efb 100644
--- a/drivers/i2c/busses/i2c-omap.c
+++ b/drivers/i2c/busses/i2c-omap.c
@@ -427,11 +427,6 @@ static int omap_i2c_init(struct omap_i2c_dev *dev)
/* Take the I2C module out of reset: */
omap_i2c_write_reg(dev, OMAP_I2C_CON_REG, OMAP_I2C_CON_EN);
- dev->errata = 0;
-
- if (dev->flags & OMAP_I2C_FLAG_APPLY_ERRATA_I207)
- dev->errata |= I2C_OMAP_ERRATA_I207;
-
/* Enable interrupts */
dev->iestate = (OMAP_I2C_IE_XRDY | OMAP_I2C_IE_RRDY |
OMAP_I2C_IE_ARDY | OMAP_I2C_IE_NACK |
@@ -1019,6 +1014,11 @@ omap_i2c_probe(struct platform_device *pdev)
dev->rev = omap_i2c_read_reg(dev, OMAP_I2C_REV_REG) & 0xff;
+ dev->errata = 0;
+
+ if (dev->flags & OMAP_I2C_FLAG_APPLY_ERRATA_I207)
+ dev->errata |= I2C_OMAP_ERRATA_I207;
+
if (dev->rev <= OMAP_I2C_REV_ON_3430)
dev->errata |= I2C_OMAP3_1P153;
--
1.7.5.4
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCHv9 09/10] I2C: OMAP: Do not set the XUDF(Transmit underflow) if the underflow is not reached
2012-05-02 14:32 [PATCHv9 00/10] I2C fixes Shubhrajyoti D
` (7 preceding siblings ...)
2012-05-02 14:32 ` [PATCHv9 08/10] I2C: OMAP: prevent the overwrite of the errata flags Shubhrajyoti D
@ 2012-05-02 14:32 ` Shubhrajyoti D
2012-05-25 22:09 ` Kevin Hilman
2012-05-02 14:32 ` [PATCHv9 10/10] I2C: OMAP: Rename the 1p153 to the erratum id i462 Shubhrajyoti D
` (3 subsequent siblings)
12 siblings, 1 reply; 24+ messages in thread
From: Shubhrajyoti D @ 2012-05-02 14:32 UTC (permalink / raw)
To: linux-arm-kernel
Currently in the 1.153 errata handling while waiting for transmitter
underflow if NACK is got the XUDF(Transmit underflow) flag is also set.
The flag is set after wait for the condition is over.
Cc: Alexander Shishkin <virtuoso@slind.org>
Acked-by: Moiz Sonasath <m-sonasath@ti.com>
Signed-off-by: Shubhrajyoti D <shubhrajyoti@ti.com>
---
drivers/i2c/busses/i2c-omap.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c
index 43b0efb..c0aa16b 100644
--- a/drivers/i2c/busses/i2c-omap.c
+++ b/drivers/i2c/busses/i2c-omap.c
@@ -730,7 +730,6 @@ static int errata_omap3_1p153(struct omap_i2c_dev *dev, u16 *stat, int *err)
if (*stat & (OMAP_I2C_STAT_NACK | OMAP_I2C_STAT_AL)) {
omap_i2c_ack_stat(dev, *stat & (OMAP_I2C_STAT_XRDY |
OMAP_I2C_STAT_XDR));
- *err |= OMAP_I2C_STAT_XUDF;
return -ETIMEDOUT;
}
@@ -743,6 +742,7 @@ static int errata_omap3_1p153(struct omap_i2c_dev *dev, u16 *stat, int *err)
return 0;
}
+ *err |= OMAP_I2C_STAT_XUDF;
return 0;
}
--
1.7.5.4
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCHv9 10/10] I2C: OMAP: Rename the 1p153 to the erratum id i462
2012-05-02 14:32 [PATCHv9 00/10] I2C fixes Shubhrajyoti D
` (8 preceding siblings ...)
2012-05-02 14:32 ` [PATCHv9 09/10] I2C: OMAP: Do not set the XUDF(Transmit underflow) if the underflow is not reached Shubhrajyoti D
@ 2012-05-02 14:32 ` Shubhrajyoti D
2012-05-12 18:10 ` [PATCHv9 00/10] I2C fixes Wolfram Sang
` (2 subsequent siblings)
12 siblings, 0 replies; 24+ messages in thread
From: Shubhrajyoti D @ 2012-05-02 14:32 UTC (permalink / raw)
To: linux-arm-kernel
The section number in the recent errata document has changed.
Rename the erratum 1p153 to the unique id i462 instead, so that
it is easier to reference. Also change the function name and comments
to reflect the same.
Cc: Jon Hunter <jon-hunter@ti.com>
Signed-off-by: Shubhrajyoti D <shubhrajyoti@ti.com>
---
drivers/i2c/busses/i2c-omap.c | 12 ++++++------
1 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c
index c0aa16b..0f536b2 100644
--- a/drivers/i2c/busses/i2c-omap.c
+++ b/drivers/i2c/busses/i2c-omap.c
@@ -173,7 +173,7 @@ enum {
/* Errata definitions */
#define I2C_OMAP_ERRATA_I207 (1 << 0)
-#define I2C_OMAP3_1P153 (1 << 1)
+#define I2C_OMAP_ERRATA_I462 (1 << 1)
struct omap_i2c_dev {
struct device *dev;
@@ -718,11 +718,11 @@ omap_i2c_omap1_isr(int this_irq, void *dev_id)
#endif
/*
- * OMAP3430 Errata 1.153: When an XRDY/XDR is hit, wait for XUDF before writing
+ * OMAP3430 Errata i462: When an XRDY/XDR is hit, wait for XUDF before writing
* data to DATA_REG. Otherwise some data bytes can be lost while transferring
* them from the memory to the I2C interface.
*/
-static int errata_omap3_1p153(struct omap_i2c_dev *dev, u16 *stat, int *err)
+static int errata_omap3_i462(struct omap_i2c_dev *dev, u16 *stat, int *err)
{
unsigned long timeout = 10000;
@@ -881,8 +881,8 @@ complete:
break;
}
- if ((dev->errata & I2C_OMAP3_1P153) &&
- errata_omap3_1p153(dev, &stat, &err))
+ if ((dev->errata & I2C_OMAP_ERRATA_I462) &&
+ errata_omap3_i462(dev, &stat, &err))
goto complete;
omap_i2c_write_reg(dev, OMAP_I2C_DATA_REG, w);
@@ -1020,7 +1020,7 @@ omap_i2c_probe(struct platform_device *pdev)
dev->errata |= I2C_OMAP_ERRATA_I207;
if (dev->rev <= OMAP_I2C_REV_ON_3430)
- dev->errata |= I2C_OMAP3_1P153;
+ dev->errata |= I2C_OMAP_ERRATA_I462;
if (!(dev->flags & OMAP_I2C_FLAG_NO_FIFO)) {
u16 s;
--
1.7.5.4
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCHv9 06/10] I2C: OMAP: Fix the crash in i2c remove
2012-05-02 14:32 ` [PATCHv9 06/10] I2C: OMAP: Fix the crash in i2c remove Shubhrajyoti D
@ 2012-05-12 18:10 ` Wolfram Sang
2012-05-14 11:26 ` Shubhrajyoti
2012-05-25 21:51 ` Kevin Hilman
0 siblings, 2 replies; 24+ messages in thread
From: Wolfram Sang @ 2012-05-12 18:10 UTC (permalink / raw)
To: linux-arm-kernel
On Wed, May 02, 2012 at 08:02:11PM +0530, Shubhrajyoti D wrote:
> In omap_i2c_remove we are accessing the I2C_CON register without
> enabling the clocks. Fix the same by enabling the clocks and disabling
> it.
> This fixes the following crash.
> [ 154.723022] ------------[ cut here ]------------
> [ 154.725677] WARNING: at arch/arm/mach-omap2/omap_l3_noc.c:112 l3_interrupt_handler+0x1b4/0x1c4()
> [ 154.725677] L3 custom error: MASTER:MPU TARGET:L4 PER2
> [ 154.742614] Modules linked in: i2c_omap(-)
> [ 154.746948] Backtrace:
> [ 154.746948] [<c0013078>] (dump_backtrace+0x0/0x110) from [<c026c158>] (dump_stack+0x18/0x1c)
> [ 154.752716] r6:00000070 r5:c002c43c r4:df9b9e98 r3:df9b8000
> [ 154.764465] [<c026c140>] (dump_stack+0x0/0x1c) from [<c0041a2c>] (warn_slowpath_common+0x5c/0x6c)
> [ 154.768341] [<c00419d0>] (warn_slowpath_common+0x0/0x6c) from [<c0041ae0>] (warn_slowpath_fmt+0x38/0x40)
> [ 154.776153] r8:00000180 r7:c0361594 r6:c0379b48 r5:00080003 r4:e0838b00
> [ 154.790771] r3:00000009
> [ 154.791778] [<c0041aa8>] (warn_slowpath_fmt+0x0/0x40) from [<c002c43c>] (l3_interrupt_handler+0x1b4/0x1c4)
> [ 154.803710] r3:c0361598 r2:c02ef74c
> [ 154.807403] [<c002c288>] (l3_interrupt_handler+0x0/0x1c4) from [<c0085f44>] (handle_irq_event_percpu+0x58/0
> [ 154.818237] r8:0000002a r7:00000000 r6:00000000 r5:df808054 r4:df8893c0
> [ 154.825378] [<c0085eec>] (handle_irq_event_percpu+0x0/0x188) from [<c00860b8>] (handle_irq_event+0x44/0x64)
> [ 154.835662] [<c0086074>] (handle_irq_event+0x0/0x64) from [<c0088ec0>] (handle_fasteoi_irq+0xa4/0x10c)
> [ 154.845458] r6:0000002a r5:df808054 r4:df808000 r3:c034a150
> [ 154.846466] [<c0088e1c>] (handle_fasteoi_irq+0x0/0x10c) from [<c0085ed0>] (generic_handle_irq+0x30/0x38)
> [ 154.854278] r5:c034aa48 r4:0000002a
> [ 154.862091] [<c0085ea0>] (generic_handle_irq+0x0/0x38) from [<c000fd38>] (handle_IRQ+0x60/0xc0)
> [ 154.874450] r4:c034ea70 r3:000001f8
> [ 154.878234] [<c000fcd8>] (handle_IRQ+0x0/0xc0) from [<c0008478>] (gic_handle_irq+0x20/0x5c)
> [ 154.887023] r7:ffffff40 r6:df9b9fb0 r5:c034e2b4 r4:0000001a
> [ 154.887054] [<c0008458>] (gic_handle_irq+0x0/0x5c) from [<c000ea80>] (__irq_usr+0x40/0x60)
> [ 154.901153] Exception stack(0xdf9b9fb0 to 0xdf9b9ff8)
> [ 154.907104] 9fa0: beaf1f04 4006be00 0000000f 0000000c
> [ 154.915710] 9fc0: 4006c000 00000000 00008034 ffffff40 00000007 00000000 00000000 0007b8d7
> [ 154.916778] 9fe0: 00000000 beaf1b68 0000d23c 4005baf0 80000010 ffffffff
> [ 154.931335] r6:ffffffff r5:80000010 r4:4005baf0 r3:beaf1f04
> [ 154.937316] ---[ end trace 1b75b31a2719ed21 ]--
>
> Cc: Kevin Hilman <khilman@ti.com>
> Cc: Rajendra Nayak <rnayak@ti.com>
> Signed-off-by: Shubhrajyoti D <shubhrajyoti@ti.com>
I'd really like a comment from the PM experts if each and every driver
has to ensure that the clocks are enabled on remove like this?
> ---
> drivers/i2c/busses/i2c-omap.c | 2 ++
> 1 files changed, 2 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c
> index fec8d5c..44e8cfa 100644
> --- a/drivers/i2c/busses/i2c-omap.c
> +++ b/drivers/i2c/busses/i2c-omap.c
> @@ -1108,7 +1108,9 @@ omap_i2c_remove(struct platform_device *pdev)
>
> free_irq(dev->irq, dev);
> i2c_del_adapter(&dev->adapter);
> + pm_runtime_get_sync(&pdev->dev);
> omap_i2c_write_reg(dev, OMAP_I2C_CON_REG, 0);
> + pm_runtime_put(&pdev->dev);
> pm_runtime_disable(&pdev->dev);
> iounmap(dev->base);
> kfree(dev);
> --
> 1.7.5.4
>
--
Pengutronix e.K. | Wolfram Sang |
Industrial Linux Solutions | http://www.pengutronix.de/ |
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20120512/8e44dbae/attachment.sig>
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCHv9 00/10] I2C fixes
2012-05-02 14:32 [PATCHv9 00/10] I2C fixes Shubhrajyoti D
` (9 preceding siblings ...)
2012-05-02 14:32 ` [PATCHv9 10/10] I2C: OMAP: Rename the 1p153 to the erratum id i462 Shubhrajyoti D
@ 2012-05-12 18:10 ` Wolfram Sang
2012-05-25 22:13 ` Kevin Hilman
2012-05-25 23:40 ` Kevin Hilman
12 siblings, 0 replies; 24+ messages in thread
From: Wolfram Sang @ 2012-05-12 18:10 UTC (permalink / raw)
To: linux-arm-kernel
On Wed, May 02, 2012 at 08:02:05PM +0530, Shubhrajyoti D wrote:
>
> The patch series does the following
>
> - Warn fixes if CONFIG_PM_RUNTIME is not selected.
> - In case of i2c remove register access was done without any
> get_sync fix the same.
> - Folds a patch from Tasslehoff to prevent any merge conflicts.
> - Prevents the XDUF flag to be set if the underflow condition is not met.
> - As per discussion in [1] .Adds a patch to rename the 1p153 errata and
> use the unique id instead as the section number in the recent errata
> docs has changed.
>
> v9:
> Fix the comments from Wolfram Sang
Patch 2 has my comment not addressed, so I stopped reviewing. It is
probably more helpful (and easier for me, too) if you do a changelog per
patch (and not of the whole series), then you can immediately see if
that specific changelog matches the current patch. 'git send-email
--annotate' might be helpful here.
Thanks,
Wolfram
--
Pengutronix e.K. | Wolfram Sang |
Industrial Linux Solutions | http://www.pengutronix.de/ |
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20120512/c4db5ea4/attachment.sig>
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCHv9 06/10] I2C: OMAP: Fix the crash in i2c remove
2012-05-12 18:10 ` Wolfram Sang
@ 2012-05-14 11:26 ` Shubhrajyoti
2012-05-25 21:51 ` Kevin Hilman
1 sibling, 0 replies; 24+ messages in thread
From: Shubhrajyoti @ 2012-05-14 11:26 UTC (permalink / raw)
To: linux-arm-kernel
On Saturday 12 May 2012 11:40 PM, Wolfram Sang wrote:
>> Cc: Kevin Hilman <khilman@ti.com>
>> > Cc: Rajendra Nayak <rnayak@ti.com>
>> > Signed-off-by: Shubhrajyoti D <shubhrajyoti@ti.com>
> I'd really like a comment from the PM experts if each and every driver
> has to ensure that the clocks are enabled on remove like this?
Just resent cc ing linux-pm
BTW also found that
Some others are also doing the same. eg:
drivers/mmc/host/omap_hsmmc.c
static int __devexit omap_hsmmc_remove(struct platform_device *pdev)
{
struct omap_hsmmc_host *host = platform_get_drvdata(pdev);
struct resource *res;
pm_runtime_get_sync(host->dev);
mmc_remove_host(host->mmc);
2. drivers/usb/musb/musb_core.c
static void musb_shutdown(struct platform_device *pdev)
{
struct musb *musb = dev_to_musb(&pdev->dev);
unsigned long flags;
pm_runtime_get_sync(musb->controller);
Anyways will wait for feedback.
>
>> > ---
>> > drivers/i2c/busses/i2c-omap.c |
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCHv9 06/10] I2C: OMAP: Fix the crash in i2c remove
2012-05-12 18:10 ` Wolfram Sang
2012-05-14 11:26 ` Shubhrajyoti
@ 2012-05-25 21:51 ` Kevin Hilman
1 sibling, 0 replies; 24+ messages in thread
From: Kevin Hilman @ 2012-05-25 21:51 UTC (permalink / raw)
To: linux-arm-kernel
Hi Wolfram,
Wolfram Sang <w.sang@pengutronix.de> writes:
> On Wed, May 02, 2012 at 08:02:11PM +0530, Shubhrajyoti D wrote:
>> In omap_i2c_remove we are accessing the I2C_CON register without
>> enabling the clocks. Fix the same by enabling the clocks and disabling
>> it.
[...]
> I'd really like a comment from the PM experts if each and every driver
> has to ensure that the clocks are enabled on remove like this?
Yes, this is correct.
In fact, this is the goal of runtime PM. The driver itself tells the PM
core (using runtime PM) when the device needs to be accessible and when
it doesn't.
Technically speaking, the it's up to the platform-specific runtime PM
implementation to decide whether or not the clocks are actually disable
or not (e.g. due to wakeup latency requirements, it might decide not to
cut clocks.)
Because of that, the changelog should be reworded to say something like
"ensure device is accessible" instead of "enable the clocks", because
the runtime PM implementation does more than just manage clocks.
Kevin
P.S. It's great to see you helping out maintaining i2c drivers. Thanks!
P.P.S. Before you merge this, I would strongly recommend we wait for a
few more Tested-bys, and a bit more description about how this
was tested. We've been having quite a few problems with
regressions introduced in OMAP drivers that have not been well
reviewed or tested.
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCHv9 02/10] I2C: OMAP: Fix the mismatch of pm_runtime enable and disable
2012-05-02 14:32 ` [PATCHv9 02/10] I2C: OMAP: Fix the mismatch of pm_runtime enable and disable Shubhrajyoti D
@ 2012-05-25 21:57 ` Kevin Hilman
2012-05-28 9:59 ` Shubhrajyoti
0 siblings, 1 reply; 24+ messages in thread
From: Kevin Hilman @ 2012-05-25 21:57 UTC (permalink / raw)
To: linux-arm-kernel
Shubhrajyoti D <shubhrajyoti@ti.com> writes:
> Currently the i2c driver calls the pm_runtime_enable and never
> the disable. This may cause a warning when pm_runtime_enable
> checks for the count match.Attempting to fix the same by calling
> pm_runtime_disable in the error and the remove path.
Looks right.
Can you be more specific in the changelog about when the errors/warning
happens? e.g. why pm_runtime_enable() is called again? Is this on
module unload/reload?
Other than that
Acked-by: Kevin Hilman <khilman@ti.com>
> Cc: Kevin Hilman <khilman@ti.com>
> Cc: Rajendra Nayak <rnayak@ti.com>
> Signed-off-by: Shubhrajyoti D <shubhrajyoti@ti.com>
> ---
> drivers/i2c/busses/i2c-omap.c | 2 ++
> 1 files changed, 2 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c
> index 4f4188d..c851672 100644
> --- a/drivers/i2c/busses/i2c-omap.c
> +++ b/drivers/i2c/busses/i2c-omap.c
> @@ -1090,6 +1090,7 @@ err_unuse_clocks:
> omap_i2c_write_reg(dev, OMAP_I2C_CON_REG, 0);
> pm_runtime_put(dev->dev);
> iounmap(dev->base);
> + pm_runtime_disable(&pdev->dev);
> err_free_mem:
> platform_set_drvdata(pdev, NULL);
> kfree(dev);
> @@ -1110,6 +1111,7 @@ omap_i2c_remove(struct platform_device *pdev)
> free_irq(dev->irq, dev);
> i2c_del_adapter(&dev->adapter);
> omap_i2c_write_reg(dev, OMAP_I2C_CON_REG, 0);
> + pm_runtime_disable(&pdev->dev);
> iounmap(dev->base);
> kfree(dev);
> mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCHv9 07/10] I2C: OMAP: Handle error check for pm runtime
2012-05-02 14:32 ` [PATCHv9 07/10] I2C: OMAP: Handle error check for pm runtime Shubhrajyoti D
@ 2012-05-25 22:06 ` Kevin Hilman
2012-05-28 11:22 ` Shubhrajyoti
0 siblings, 1 reply; 24+ messages in thread
From: Kevin Hilman @ 2012-05-25 22:06 UTC (permalink / raw)
To: linux-arm-kernel
Shubhrajyoti D <shubhrajyoti@ti.com> writes:
> If PM runtime get_sync fails return with the error
> so that no further reads/writes goes through the interface.
> This will avoid possible abort. Add a error message in case
> of failure with the cause of the failure.
>
> Signed-off-by: Shubhrajyoti D <shubhrajyoti@ti.com>
> ---
> drivers/i2c/busses/i2c-omap.c | 14 +++++++++++---
> 1 files changed, 11 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c
> index 44e8cfa..a72874e 100644
> --- a/drivers/i2c/busses/i2c-omap.c
> +++ b/drivers/i2c/busses/i2c-omap.c
> @@ -585,7 +585,9 @@ omap_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[], int num)
> int i;
> int r;
>
> - pm_runtime_get_sync(dev->dev);
> + r = pm_runtime_get_sync(dev->dev);
> + if (r < 0)
> + return r;
nit: please use IS_ERR_VALUE()
Kevin
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCHv9 09/10] I2C: OMAP: Do not set the XUDF(Transmit underflow) if the underflow is not reached
2012-05-02 14:32 ` [PATCHv9 09/10] I2C: OMAP: Do not set the XUDF(Transmit underflow) if the underflow is not reached Shubhrajyoti D
@ 2012-05-25 22:09 ` Kevin Hilman
0 siblings, 0 replies; 24+ messages in thread
From: Kevin Hilman @ 2012-05-25 22:09 UTC (permalink / raw)
To: linux-arm-kernel
Shubhrajyoti D <shubhrajyoti@ti.com> writes:
> Currently in the 1.153 errata handling while waiting for transmitter
> underflow if NACK is got the XUDF(Transmit underflow) flag is also set.
-EOVERFLOW
This sentence needs a rewrite and some punctuation. It does not read well.
> The flag is set after wait for the condition is over.
>
> Cc: Alexander Shishkin <virtuoso@slind.org>
> Acked-by: Moiz Sonasath <m-sonasath@ti.com>
> Signed-off-by: Shubhrajyoti D <shubhrajyoti@ti.com>
> ---
> drivers/i2c/busses/i2c-omap.c | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c
> index 43b0efb..c0aa16b 100644
> --- a/drivers/i2c/busses/i2c-omap.c
> +++ b/drivers/i2c/busses/i2c-omap.c
> @@ -730,7 +730,6 @@ static int errata_omap3_1p153(struct omap_i2c_dev *dev, u16 *stat, int *err)
> if (*stat & (OMAP_I2C_STAT_NACK | OMAP_I2C_STAT_AL)) {
> omap_i2c_ack_stat(dev, *stat & (OMAP_I2C_STAT_XRDY |
> OMAP_I2C_STAT_XDR));
> - *err |= OMAP_I2C_STAT_XUDF;
> return -ETIMEDOUT;
> }
>
> @@ -743,6 +742,7 @@ static int errata_omap3_1p153(struct omap_i2c_dev *dev, u16 *stat, int *err)
> return 0;
> }
>
> + *err |= OMAP_I2C_STAT_XUDF;
> return 0;
> }
Kevin
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCHv9 00/10] I2C fixes
2012-05-02 14:32 [PATCHv9 00/10] I2C fixes Shubhrajyoti D
` (10 preceding siblings ...)
2012-05-12 18:10 ` [PATCHv9 00/10] I2C fixes Wolfram Sang
@ 2012-05-25 22:13 ` Kevin Hilman
2012-05-28 9:52 ` Shubhrajyoti
2012-05-25 23:40 ` Kevin Hilman
12 siblings, 1 reply; 24+ messages in thread
From: Kevin Hilman @ 2012-05-25 22:13 UTC (permalink / raw)
To: linux-arm-kernel
Shubhrajyoti D <shubhrajyoti@ti.com> writes:
> The patch series does the following
>
> - Warn fixes if CONFIG_PM_RUNTIME is not selected.
> - In case of i2c remove register access was done without any
> get_sync fix the same.
> - Folds a patch from Tasslehoff to prevent any merge conflicts.
> - Prevents the XDUF flag to be set if the underflow condition is not met.
> - As per discussion in [1] .Adds a patch to rename the 1p153 errata and
> use the unique id instead as the section number in the recent errata
> docs has changed.
>
> v9:
> Fix the comments from Wolfram Sang
>
> [1] http://www.spinics.net/lists/linux-i2c/msg07607.html
>
> Tested on omap4sdp and omap3sdp.
Can you also describe how it was tested?
With the runtime PM changes, does it still hit full-chip retention in
idle and suspend after these changes?
I had a few minor comments on this version, otherwise feel free add
Reviewed-by: Kevin Hilman <khilman@ti.com>
That being said, before this is merged, I woudl like to see some more
non-author Tested-bys. We've been having lots of regressions of late
from OMAP drivers that are not being sufficiently tested before
merging. We need to ensure proper testing before merge.
Other testers should also report what platforms they tested on, and how
it was tested.
Thanks,
Kevin
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCHv9 00/10] I2C fixes
2012-05-02 14:32 [PATCHv9 00/10] I2C fixes Shubhrajyoti D
` (11 preceding siblings ...)
2012-05-25 22:13 ` Kevin Hilman
@ 2012-05-25 23:40 ` Kevin Hilman
2012-05-28 9:54 ` Shubhrajyoti
12 siblings, 1 reply; 24+ messages in thread
From: Kevin Hilman @ 2012-05-25 23:40 UTC (permalink / raw)
To: linux-arm-kernel
+Neil Brown
Shubhrajyoti D <shubhrajyoti@ti.com> writes:
> The patch series does the following
>
> - Warn fixes if CONFIG_PM_RUNTIME is not selected.
> - In case of i2c remove register access was done without any
> get_sync fix the same.
> - Folds a patch from Tasslehoff to prevent any merge conflicts.
> - Prevents the XDUF flag to be set if the underflow condition is not met.
> - As per discussion in [1] .Adds a patch to rename the 1p153 errata and
> use the unique id instead as the section number in the recent errata
> docs has changed.
>
Shubhrajyoti,
Can you add one more patch to this series.
The patch below from Neil Brown has been circulating for awhile, and
I've been using it locally for awhile now too. It would help if it got
into this series and got some broader testing.
Thanks,
Kevin
>From 0c6effd8356e6273c294490a576551ef37ae6799 Mon Sep 17 00:00:00 2001
From: NeilBrown <neilb@suse.de>
Date: Fri, 30 Dec 2011 12:40:30 +1100
Subject: [PATCH] OMAP/I2C - Fix timeout problem during suspend.
On a board with OMAP3 processor and TWL4030 Power management,
we need to talk to the TWL4030 during late suspend but cannot
because the I2C interrupt is disabled (as late suspend disables
interrupt).
e.g. I get messages like:
[ 62.161102] musb-omap2430 musb-omap2430: LATE power domain suspend
[ 63.167205] omap_i2c omap_i2c.1: controller timed out
[ 63.183044] twl: i2c_read failed to transfer all messages
[ 64.182861] omap_i2c omap_i2c.1: controller timed out
[ 64.198455] twl: i2c_write failed to transfer all messages
[ 65.198455] omap_i2c omap_i2c.1: controller timed out
[ 65.203765] twl: i2c_write failed to transfer all messages
The stack shows omap2430_runtime_suspend calling twl4030_set_suspend
which tries to power-down the USB PHY (twl4030_phy_suspend ->
twl4030_phy_power -> __twl4030_phy_power which as a nice WARN_ON
that helps).
Then we get the same in resume:
[ 69.603912] musb-omap2430 musb-omap2430: EARLY power domain resume
[ 70.610473] omap_i2c omap_i2c.1: controller timed out
[ 70.626129] twl: i2c_write failed to transfer all messages
etc.
So don't disable interrupts for I2C.
Acked-by: Kevin Hilman <khilman@ti.com>
Tested-by: Kevin Hilman <khilman@ti.com>
Signed-off-by: NeilBrown <neilb@suse.de>
---
drivers/i2c/busses/i2c-omap.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c
index 801df60..e024c50 100644
--- a/drivers/i2c/busses/i2c-omap.c
+++ b/drivers/i2c/busses/i2c-omap.c
@@ -1092,7 +1092,7 @@ omap_i2c_probe(struct platform_device *pdev)
isr = (dev->rev < OMAP_I2C_OMAP1_REV_2) ? omap_i2c_omap1_isr :
omap_i2c_isr;
- r = request_irq(dev->irq, isr, 0, pdev->name, dev);
+ r = request_irq(dev->irq, isr, IRQF_NO_SUSPEND, pdev->name, dev);
if (r) {
dev_err(dev->dev, "failure requesting irq %i\n", dev->irq);
--
1.7.9.2
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCHv9 00/10] I2C fixes
2012-05-25 22:13 ` Kevin Hilman
@ 2012-05-28 9:52 ` Shubhrajyoti
0 siblings, 0 replies; 24+ messages in thread
From: Shubhrajyoti @ 2012-05-28 9:52 UTC (permalink / raw)
To: linux-arm-kernel
Hi Kevin,
On Saturday 26 May 2012 03:43 AM, Kevin Hilman wrote:
> Shubhrajyoti D <shubhrajyoti@ti.com> writes:
>
>> The patch series does the following
>>
>> - Warn fixes if CONFIG_PM_RUNTIME is not selected.
>> - In case of i2c remove register access was done without any
>> get_sync fix the same.
>> - Folds a patch from Tasslehoff to prevent any merge conflicts.
>> - Prevents the XDUF flag to be set if the underflow condition is not met.
>> - As per discussion in [1] .Adds a patch to rename the 1p153 errata and
>> use the unique id instead as the section number in the recent errata
>> docs has changed.
>>
>> v9:
>> Fix the comments from Wolfram Sang
>>
>> [1] http://www.spinics.net/lists/linux-i2c/msg07607.html
>>
>> Tested on omap4sdp and omap3sdp.
> Can you also describe how it was tested?
I did basic functionality tests using i2c-tools.
>
> With the runtime PM changes, does it still hit full-chip retention in
> idle and suspend after these changes?
Will check.
>
> I had a few minor comments on this version,
Will fixup and resend.
> otherwise feel free add
>
> Reviewed-by: Kevin Hilman <khilman@ti.com>
Thanks for your review.
>
> That being said, before this is merged, I woudl like to see some more
> non-author Tested-bys. We've been having lots of regressions of late
> from OMAP drivers that are not being sufficiently tested before
> merging. We need to ensure proper testing before merge.
>
> Other testers should also report what platforms they tested on, and how
> it was tested.
>
> Thanks,
>
> Kevin
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCHv9 00/10] I2C fixes
2012-05-25 23:40 ` Kevin Hilman
@ 2012-05-28 9:54 ` Shubhrajyoti
0 siblings, 0 replies; 24+ messages in thread
From: Shubhrajyoti @ 2012-05-28 9:54 UTC (permalink / raw)
To: linux-arm-kernel
On Saturday 26 May 2012 05:10 AM, Kevin Hilman wrote:
> Shubhrajyoti,
>
> Can you add one more patch to this series.
Yes will add it.
Thanks,
Shubhro
>
> The patch below from Neil Brown has been circulating for awhile, and
> I've been using it locally for awhile now too. It would help if it got
> into this series and got some broader testing.
>
> Thanks,
>
> Kevin
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCHv9 02/10] I2C: OMAP: Fix the mismatch of pm_runtime enable and disable
2012-05-25 21:57 ` Kevin Hilman
@ 2012-05-28 9:59 ` Shubhrajyoti
0 siblings, 0 replies; 24+ messages in thread
From: Shubhrajyoti @ 2012-05-28 9:59 UTC (permalink / raw)
To: linux-arm-kernel
On Saturday 26 May 2012 03:27 AM, Kevin Hilman wrote:
> Looks right.
>
> Can you be more specific in the changelog about when the errors/warning
> happens?
By the way the count cribs Unbalanced pm_runtime_enable!
is what happens.
I found it by review, I did the testing with this patch.
Since there was an access without get_sync I was seeing
some other issues.
Did not test with the fix and without this patch.
> e.g. why pm_runtime_enable() is called again? Is this on
> module unload/reload?
>
> Other than that
>
> Acked-by: Kevin Hilman <khilman@ti.com>
Thanks.
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCHv9 07/10] I2C: OMAP: Handle error check for pm runtime
2012-05-25 22:06 ` Kevin Hilman
@ 2012-05-28 11:22 ` Shubhrajyoti
0 siblings, 0 replies; 24+ messages in thread
From: Shubhrajyoti @ 2012-05-28 11:22 UTC (permalink / raw)
To: linux-arm-kernel
On Saturday 26 May 2012 03:36 AM, Kevin Hilman wrote:
>> + r = pm_runtime_get_sync(dev->dev);
>> > + if (r < 0)
>> > + return r;
> nit: please use IS_ERR_VALUE()
Will fix that and resend.
> Kevin
^ permalink raw reply [flat|nested] 24+ messages in thread
end of thread, other threads:[~2012-05-28 11:22 UTC | newest]
Thread overview: 24+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-05-02 14:32 [PATCHv9 00/10] I2C fixes Shubhrajyoti D
2012-05-02 14:32 ` [PATCHv9 01/10] I2C: OMAP: make omap_i2c_unidle/idle functions depend on CONFIG_PM_RUNTIME Shubhrajyoti D
2012-05-02 14:32 ` [PATCHv9 02/10] I2C: OMAP: Fix the mismatch of pm_runtime enable and disable Shubhrajyoti D
2012-05-25 21:57 ` Kevin Hilman
2012-05-28 9:59 ` Shubhrajyoti
2012-05-02 14:32 ` [PATCHv9 03/10] I2C: OMAP: Fix the interrupt clearing in OMAP4 Shubhrajyoti D
2012-05-02 14:32 ` [PATCHv9 04/10] I2C: OMAP: Prevent the register access after pm_runtime_put in probe Shubhrajyoti D
2012-05-02 14:32 ` [PATCHv9 05/10] I2C: OMAP: Don't check if wait_for_completion_timeout() returns less than zero Shubhrajyoti D
2012-05-02 14:32 ` [PATCHv9 06/10] I2C: OMAP: Fix the crash in i2c remove Shubhrajyoti D
2012-05-12 18:10 ` Wolfram Sang
2012-05-14 11:26 ` Shubhrajyoti
2012-05-25 21:51 ` Kevin Hilman
2012-05-02 14:32 ` [PATCHv9 07/10] I2C: OMAP: Handle error check for pm runtime Shubhrajyoti D
2012-05-25 22:06 ` Kevin Hilman
2012-05-28 11:22 ` Shubhrajyoti
2012-05-02 14:32 ` [PATCHv9 08/10] I2C: OMAP: prevent the overwrite of the errata flags Shubhrajyoti D
2012-05-02 14:32 ` [PATCHv9 09/10] I2C: OMAP: Do not set the XUDF(Transmit underflow) if the underflow is not reached Shubhrajyoti D
2012-05-25 22:09 ` Kevin Hilman
2012-05-02 14:32 ` [PATCHv9 10/10] I2C: OMAP: Rename the 1p153 to the erratum id i462 Shubhrajyoti D
2012-05-12 18:10 ` [PATCHv9 00/10] I2C fixes Wolfram Sang
2012-05-25 22:13 ` Kevin Hilman
2012-05-28 9:52 ` Shubhrajyoti
2012-05-25 23:40 ` Kevin Hilman
2012-05-28 9:54 ` Shubhrajyoti
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).