I2C big cleanup
Subsystems (4)
v8 cover I2C big cleanup
Changes since v1:
- removed tabification on patch 6/17
- removed dev_err() which was introduced on patch 09/17
Changes since v2:
- do not set full fifo depth in the RDR interrupt.
- some changelog updates.
- rebase to the Wolfram's tree.
Changes since v3:
- Remove a redundant read of status register
- Read the dev->buf_len variable instead of the register as the information of the remaining bytes is there.
Changes since v4:
- Ack the arbitration lost.
- Rebase to the i2c-embedded/for-next branch.
Changes since v5:
- Rebase to latest mainline
- Added some more cleanup patches so as have a consolidated series.
Changes since v6:
- Fix comments on setting the pdev to NULL.
- Trivial changelog update
Changes since v7:
- Remove a patch as the code is getting changed anyways
- Changelogs update.
Previous discussions can be found here
http://www.spinics.net/lists/linux-i2c/msg09764.html
This is the cleanup only series.
Tested on omap4sdp and 3430sdp.
The following changes since commit 55d512e245bc7699a8800e23df1a24195dd08217:
Linux 3.6-rc5 (2012-09-08 16:43:45 -0700)
are available in the git repository at:
git://gitorious.org/linus-tree/linus-tree.git for_3.7/i2c/big_cleanups
Felipe Balbi (22):
i2c: omap: switch to devm_* API
i2c: omap: simplify num_bytes handling
i2c: omap: decrease indentation level on data handling
i2c: omap: add blank lines
i2c: omap: simplify omap_i2c_ack_stat()
i2c: omap: split out [XR]DR and [XR]RDY
i2c: omap: improve i462 errata handling
i2c: omap: re-factor receive/transmit data loop
i2c: omap: switch over to do {} while loop
i2c: omap: ack IRQ in parts
i2c: omap: switch to platform_get_irq()
i2c: omap: bus: add a receiver flag
i2c: omap: simplify errata check
i2c: omap: always return IRQ_HANDLED
i2c: omap: simplify IRQ exit path
i2c: omap: resize fifos before each message
i2c: omap: get rid of the "complete" label
i2c: omap: always return IRQ_HANDLED
i2c: omap: switch to threaded IRQ support
i2c: omap: remove unnecessary pm_runtime_suspended check
i2c: omap: switch over to autosuspend API
i2c: omap: sanitize exit path
Shubhrajyoti D (1):
i2c: omap: remove redundant status read
drivers/i2c/busses/i2c-omap.c | 442 +++++++++++++++++++++++++---------------- 1 files changed, 271 insertions(+), 171 deletions(-)
Signature · 1 line
1.7.5.4
PATCH v8 1/22 i2c: omap: switch to devm_* API
From: Felipe Balbi <[email protected]>
that helps deleting some boiler plate code
and lets driver-core manage our resources
for us.
--- drivers/i2c/busses/i2c-omap.c | 41 ++++++++++++----------------------------- 1 files changed, 12 insertions(+), 29 deletions(-)
drivers/i2c/busses/i2c-omap.c +12 -29
@@ -943,7 +943,7 @@ omap_i2c_probe(struct platform_device *pdev) {struct omap_i2c_dev *dev;struct i2c_adapter *adap; -struct resource *mem, *irq, *ioarea; +struct resource *mem, *irq;struct omap_i2c_bus_platform_data *pdata = pdev->dev.platform_data;struct device_node *node = pdev->dev.of_node;const struct of_device_id *match;
@@ -962,17 +962,16 @@ omap_i2c_probe(struct platform_device *pdev)return -ENODEV ; } - ioarea =request_mem_region (mem->start,resource_size (mem), - pdev->name); -if (!ioarea) { -dev_err (&pdev->dev,"I2C region already claimed\n" ); -return -EBUSY ; + dev =devm_kzalloc (&pdev->dev,sizeof (struct omap_i2c_dev ),GFP_KERNEL ); +if (!dev) { +dev_err (&pdev->dev,"Menory allocation failed\n" ); +return -ENOMEM ; } - dev =kzalloc (sizeof (struct omap_i2c_dev ),GFP_KERNEL ); -if (!dev) { - r = -ENOMEM ; - goto err_release_region; + dev->base =devm_request_and_ioremap (&pdev->dev, mem); +if (!dev->base) { +dev_err (&pdev->dev,"I2C region already claimed\n" ); +return -ENOMEM ; } match =of_match_device (of_match_ptr (omap_i2c_of_match), &pdev->dev);
@@ -995,11 +994,6 @@ omap_i2c_probe(struct platform_device *pdev) dev->dev = &pdev->dev; dev->irq = irq->start; - dev->base =ioremap (mem->start,resource_size (mem)); -if (!dev->base) { - r = -ENOMEM ; - goto err_free_mem; - }platform_set_drvdata (pdev, dev);init_completion (&dev->cmd_complete);
@@ -1057,7 +1051,8 @@ 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,IRQF_NO_SUSPEND , pdev->name, dev); + r =devm_request_irq (&pdev->dev, dev->irq, isr,IRQF_NO_SUSPEND , + pdev->name, dev);if (r) {dev_err (dev->dev,"failure requesting irq %i\n" , dev->irq);
@@ -1081,7 +1076,7 @@ omap_i2c_probe(struct platform_device *pdev) r =i2c_add_numbered_adapter (adap);if (r) {dev_err (dev->dev,"failure adding adapter\n" ); - goto err_free_irq; + goto err_unuse_clocks; }of_i2c_register_devices (adap);
@@ -1090,18 +1085,12 @@ omap_i2c_probe(struct platform_device *pdev)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); -iounmap (dev->base);pm_runtime_disable (&pdev->dev); err_free_mem:platform_set_drvdata (pdev,NULL ); -kfree (dev); -err_release_region: -release_mem_region (mem->start,resource_size (mem));return r; }
@@ -1109,12 +1098,10 @@ err_release_region: static int __devexit 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); ret =pm_runtime_get_sync (&pdev->dev);if (IS_ERR_VALUE (ret))
@@ -1123,10 +1110,6 @@ static int __devexit omap_i2c_remove(struct platform_device *pdev)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); - mem =platform_get_resource (pdev,IORESOURCE_MEM ,0 ); -release_mem_region (mem->start,resource_size (mem));return 0 ; }
Signature · 1 line
1.7.5.4
PATCH v8 2/22 i2c: omap: simplify num_bytes handling
In reply to Shubhrajyoti DFrom: Felipe Balbi <[email protected]>
trivial patch, no functional changes
If the fifo is disabled or fifo_size is 0 the num_bytes is set to 1. Else it is set to fifo_size or in case of a draining interrupt the remaining bytes in the buff stat. So the zero check is redundant and can be safely optimised.
--- drivers/i2c/busses/i2c-omap.c | 6 ++---- 1 files changed, 2 insertions(+), 4 deletions(-)
drivers/i2c/busses/i2c-omap.c +2 -4
@@ -812,8 +812,7 @@ complete: OMAP_I2C_BUFSTAT_REG) >> 8) & 0x3F; } -while (num_bytes) { - num_bytes--; +while (num_bytes--) { w =omap_i2c_read_reg (dev,OMAP_I2C_DATA_REG );if (dev->buf_len) { *dev->buf++ = w;
@@ -855,8 +854,7 @@ complete: OMAP_I2C_BUFSTAT_REG) &0x3F ; } -while (num_bytes) { - num_bytes--; +while (num_bytes--) { w =0 ;if (dev->buf_len) { w = *dev->buf++;
Signature · 1 line
1.7.5.4
PATCH v8 3/22 i2c: omap: decrease indentation level on data handling
In reply to Shubhrajyoti DFrom: Felipe Balbi <[email protected]>
The patch intends to decrease the indentation level on the data handling
by using the fact that else of if (dev->buf_len) is same as if (!dev->buf_len)
if (dev->buf_len) {
aaa;
} else {
bbb;
break;
}to
if (!dev->buf_len) {
bbb;
break;
}aaa;
Hence no functional changes.
--- drivers/i2c/busses/i2c-omap.c | 63 ++++++++++++++++++++--------------------- 1 files changed, 31 insertions(+), 32 deletions(-)
drivers/i2c/busses/i2c-omap.c +31 -32
@@ -813,22 +813,7 @@ complete: >> 8) & 0x3F; }while (num_bytes--) { - w =omap_i2c_read_reg (dev,OMAP_I2C_DATA_REG ); -if (dev->buf_len) { - *dev->buf++ = w; - dev->buf_len--; -/* -* Data reg in 2430, omap3 and -* omap4 is 8 bit wide -*/ -if (dev->flags & -OMAP_I2C_FLAG_16BIT_DATA_REG ) { -if (dev->buf_len) { - *dev->buf++ = w >>8 ; - dev->buf_len--; - } - } - }else { +if (!dev->buf_len) {if (stat &OMAP_I2C_STAT_RRDY )dev_err (dev->dev, "RRDY IRQ while no data"
@@ -839,6 +824,21 @@ complete: " requested\n");break ; } + + w =omap_i2c_read_reg (dev,OMAP_I2C_DATA_REG ); + *dev->buf++ = w; + dev->buf_len--; +/* +* Data reg in 2430, omap3 and +* omap4 is 8 bit wide +*/ +if (dev->flags & +OMAP_I2C_FLAG_16BIT_DATA_REG ) { +if (dev->buf_len) { + *dev->buf++ = w >>8 ; + dev->buf_len--; + } + } }omap_i2c_ack_stat (dev, stat & (OMAP_I2C_STAT_RRDY |OMAP_I2C_STAT_RDR ));
@@ -855,22 +855,7 @@ complete: &0x3F ; }while (num_bytes--) { - w =0 ; -if (dev->buf_len) { - w = *dev->buf++; - dev->buf_len--; -/* -* Data reg in 2430, omap3 and -* omap4 is 8 bit wide -*/ -if (dev->flags & -OMAP_I2C_FLAG_16BIT_DATA_REG ) { -if (dev->buf_len) { - w |= *dev->buf++ <<8 ; - dev->buf_len--; - } - } - }else { +if (!dev->buf_len) {if (stat &OMAP_I2C_STAT_XRDY )dev_err (dev->dev, "XRDY IRQ while no "
@@ -882,6 +867,20 @@ complete:break ; } + w = *dev->buf++; + dev->buf_len--; +/* +* Data reg in 2430, omap3 and +* omap4 is 8 bit wide +*/ +if (dev->flags & +OMAP_I2C_FLAG_16BIT_DATA_REG ) { +if (dev->buf_len) { + w |= *dev->buf++ <<8 ; + dev->buf_len--; + } + } +if ((dev->errata &I2C_OMAP_ERRATA_I462 ) &&errata_omap3_i462 (dev, &stat, &err)) goto complete;
Signature · 1 line
1.7.5.4
PATCH v8 4/22 i2c: omap: add blank lines
In reply to Shubhrajyoti DFrom: Felipe Balbi <[email protected]>
trivial patch to aid readability. No functional
changes.
--- drivers/i2c/busses/i2c-omap.c | 5 +++++ 1 files changed, 5 insertions(+), 0 deletions(-)
drivers/i2c/busses/i2c-omap.c +5 -0
@@ -786,6 +786,7 @@ complete:dev_err (dev->dev,"Arbitration lost\n" ); err |=OMAP_I2C_STAT_AL ; } +/* * ProDB0017052: Clear ARDY bit twice */
@@ -798,6 +799,7 @@ complete:omap_i2c_complete_cmd (dev, err);return IRQ_HANDLED ; } +if (stat & (OMAP_I2C_STAT_RRDY |OMAP_I2C_STAT_RDR )) {u8 num_bytes =1 ;
@@ -844,6 +846,7 @@ complete: stat & (OMAP_I2C_STAT_RRDY | OMAP_I2C_STAT_RDR));continue ; } +if (stat & (OMAP_I2C_STAT_XRDY |OMAP_I2C_STAT_XDR )) {u8 num_bytes =1 ;if (dev->fifo_size) {
@@ -891,10 +894,12 @@ complete: stat & (OMAP_I2C_STAT_XRDY | OMAP_I2C_STAT_XDR));continue ; } +if (stat &OMAP_I2C_STAT_ROVR ) {dev_err (dev->dev,"Receive overrun\n" ); dev->cmd_err |=OMAP_I2C_STAT_ROVR ; } +if (stat &OMAP_I2C_STAT_XUDF ) {dev_err (dev->dev,"Transmit underflow\n" ); dev->cmd_err |=OMAP_I2C_STAT_XUDF ;
Signature · 1 line
1.7.5.4
PATCH v8 5/22 i2c: omap: simplify omap_i2c_ack_stat()
In reply to Shubhrajyoti DFrom: Felipe Balbi <[email protected]>
stat & BIT(1) is the same as BIT(1), so let's
simplify things a bit by removing "stat &" from
all omap_i2c_ack_stat() calls.
Code snippet (extremely simplified):
if (stat & NACK) {
...
omap_i2c_ack_stat(dev, stat & NACK);
}if (stat & RDR) {
...
omap_i2c_ack_stat(dev, stat & RDR);
}and so on. The tricky place is only WRT errata handling, for example:
if (*stat & (NACK | AL)) {
omap_i2c_ack_stat(dev, *stat & (XRDY | XDR));
...
}but in this case, the errata says we must clear XRDY and XDR if that errata triggers, so if they just got enabled or not, it doesn't matter.
Another tricky place is RDR | RRDY (likewise for XDR | XRDY):
if (stat & (RDR | RRDY)) {
...
omap_i2c_ack_stat(dev, stat & (RDR | RRDY));
}again here there will be no issues because those IRQs never fire simultaneously and one will only after after we have handled the previous, that's because the same FIFO is used anyway and we won't shift data into FIFO until we tell the IP "hey, I'm done with the FIFO, you can shift more data"
--- drivers/i2c/busses/i2c-omap.c | 19 ++++++++++--------- 1 files changed, 10 insertions(+), 9 deletions(-)
drivers/i2c/busses/i2c-omap.c +10 -9
@@ -731,7 +731,7 @@ static int errata_omap3_i462(struct omap_i2c_dev *dev, u16 *stat, int *err)while (--timeout && !(*stat &OMAP_I2C_STAT_XUDF )) {if (*stat & (OMAP_I2C_STAT_NACK |OMAP_I2C_STAT_AL )) { -omap_i2c_ack_stat (dev, *stat & (OMAP_I2C_STAT_XRDY | +omap_i2c_ack_stat (dev, (OMAP_I2C_STAT_XRDY |OMAP_I2C_STAT_XDR ));return -ETIMEDOUT ; }
@@ -792,10 +792,11 @@ complete:*/ if (stat & (OMAP_I2C_STAT_ARDY |OMAP_I2C_STAT_NACK |OMAP_I2C_STAT_AL )) { -omap_i2c_ack_stat (dev, stat & - (OMAP_I2C_STAT_RRDY |OMAP_I2C_STAT_RDR | -OMAP_I2C_STAT_XRDY |OMAP_I2C_STAT_XDR | -OMAP_I2C_STAT_ARDY )); +omap_i2c_ack_stat (dev, (OMAP_I2C_STAT_RRDY | +OMAP_I2C_STAT_RDR | +OMAP_I2C_STAT_XRDY | +OMAP_I2C_STAT_XDR | +OMAP_I2C_STAT_ARDY ));omap_i2c_complete_cmd (dev, err);return IRQ_HANDLED ; }
@@ -842,8 +843,8 @@ complete: } } } -omap_i2c_ack_stat (dev, - stat & (OMAP_I2C_STAT_RRDY |OMAP_I2C_STAT_RDR )); +omap_i2c_ack_stat (dev, (OMAP_I2C_STAT_RRDY | +OMAP_I2C_STAT_RDR ));continue ; }
@@ -890,8 +891,8 @@ complete:omap_i2c_write_reg (dev,OMAP_I2C_DATA_REG , w); } -omap_i2c_ack_stat (dev, - stat & (OMAP_I2C_STAT_XRDY |OMAP_I2C_STAT_XDR )); +omap_i2c_ack_stat (dev, (OMAP_I2C_STAT_XRDY | +OMAP_I2C_STAT_XDR ));continue ; }
Signature · 1 line
1.7.5.4
PATCH v8 6/22 i2c: omap: split out [XR]DR and [XR]RDY
In reply to Shubhrajyoti DFrom: Felipe Balbi <[email protected]>
While they do pretty much the same thing, there are a few peculiarities. Specially WRT erratas, it's best to split those out and re-factor the read/write loop to another function which both cases call.
This last part will be done on another patch.
While at that, also avoid an unncessary register read since dev->fifo_len will always contain the correct amount of data to be transferred.
--- drivers/i2c/busses/i2c-omap.c | 126 ++++++++++++++++++++++++++++++----------- 1 files changed, 92 insertions(+), 34 deletions(-)
drivers/i2c/busses/i2c-omap.c +92 -34
@@ -801,36 +801,62 @@ complete:return IRQ_HANDLED ; } -if (stat & (OMAP_I2C_STAT_RRDY |OMAP_I2C_STAT_RDR )) { +if (stat &OMAP_I2C_STAT_RDR ) {u8 num_bytes =1 ; +if (dev->fifo_size) + num_bytes = dev->buf_len; + +while (num_bytes--) { +if (!dev->buf_len) { +dev_err (dev->dev, +"RDR IRQ while no data" +" requested\n" ); +break ; + } + + w =omap_i2c_read_reg (dev,OMAP_I2C_DATA_REG ); + *dev->buf++ = w; + dev->buf_len--; + +/* +* Data reg in 2430, omap3 and +* omap4 is 8 bit wide +*/ +if (dev->flags & +OMAP_I2C_FLAG_16BIT_DATA_REG ) { +if (dev->buf_len) { + *dev->buf++ = w >>8 ; + dev->buf_len--; + } + } + } +if (dev->errata &I2C_OMAP_ERRATA_I207 )i2c_omap_errata_i207 (dev, stat); -if (dev->fifo_size) { -if (stat &OMAP_I2C_STAT_RRDY ) - num_bytes = dev->fifo_size; -else /* read RXSTAT on RDR interrupt */ - num_bytes = (omap_i2c_read_reg (dev, -OMAP_I2C_BUFSTAT_REG ) - >>8 ) &0x3F ; - } +omap_i2c_ack_stat (dev,OMAP_I2C_STAT_RDR ); +continue ; + } + +if (stat &OMAP_I2C_STAT_RRDY ) { +u8 num_bytes =1 ; + +if (dev->fifo_size) + num_bytes = dev->fifo_size; +while (num_bytes--) {if (!dev->buf_len) { -if (stat &OMAP_I2C_STAT_RRDY ) -dev_err (dev->dev, +dev_err (dev->dev,"RRDY IRQ while no data" -" requested\n" ); -if (stat &OMAP_I2C_STAT_RDR ) -dev_err (dev->dev, -"RDR IRQ while no data" -" requested\n" ); +" requested\n" );break ; } w =omap_i2c_read_reg (dev,OMAP_I2C_DATA_REG ); *dev->buf++ = w; dev->buf_len--; +/* * Data reg in 2430, omap3 and * omap4 is 8 bit wide
@@ -843,36 +869,68 @@ complete: } } } -omap_i2c_ack_stat (dev, (OMAP_I2C_STAT_RRDY | -OMAP_I2C_STAT_RDR )); + +omap_i2c_ack_stat (dev,OMAP_I2C_STAT_RRDY );continue ; } -if (stat & (OMAP_I2C_STAT_XRDY |OMAP_I2C_STAT_XDR )) { +if (stat &OMAP_I2C_STAT_XDR ) {u8 num_bytes =1 ; -if (dev->fifo_size) { -if (stat &OMAP_I2C_STAT_XRDY ) - num_bytes = dev->fifo_size; -else /* read TXSTAT on XDR interrupt */ - num_bytes =omap_i2c_read_reg (dev, -OMAP_I2C_BUFSTAT_REG ) - &0x3F ; + +if (dev->fifo_size) + num_bytes = dev->buf_len; + +while (num_bytes--) { +if (!dev->buf_len) { +dev_err (dev->dev, +"XDR IRQ while no " +"data to send\n" ); +break ; + } + + w = *dev->buf++; + dev->buf_len--; + +/* +* Data reg in 2430, omap3 and +* omap4 is 8 bit wide +*/ +if (dev->flags & +OMAP_I2C_FLAG_16BIT_DATA_REG ) { +if (dev->buf_len) { + w |= *dev->buf++ <<8 ; + dev->buf_len--; + } + } + +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); } + +omap_i2c_ack_stat (dev,OMAP_I2C_STAT_XDR ); +continue ; + } + +if (stat &OMAP_I2C_STAT_XRDY ) { +u8 num_bytes =1 ; + +if (dev->fifo_size) + num_bytes = dev->fifo_size; +while (num_bytes--) {if (!dev->buf_len) { -if (stat &OMAP_I2C_STAT_XRDY ) -dev_err (dev->dev, +dev_err (dev->dev,"XRDY IRQ while no " "data to send\n" ); -if (stat &OMAP_I2C_STAT_XDR ) -dev_err (dev->dev, -"XDR IRQ while no " -"data to send\n" );break ; } w = *dev->buf++; dev->buf_len--; +/* * Data reg in 2430, omap3 and * omap4 is 8 bit wide
@@ -891,8 +949,8 @@ complete:omap_i2c_write_reg (dev,OMAP_I2C_DATA_REG , w); } -omap_i2c_ack_stat (dev, (OMAP_I2C_STAT_XRDY | -OMAP_I2C_STAT_XDR )); + +omap_i2c_ack_stat (dev,OMAP_I2C_STAT_XRDY );continue ; }
Signature · 1 line
1.7.5.4
PATCH v8 7/22 i2c: omap: improve i462 errata handling
In reply to Shubhrajyoti DFrom: Felipe Balbi <[email protected]>
Make it not depend on ISR's local variables
in order to make it easier to re-factor the
transmit data loop.
Also since we are waiting for XUDF(Transmitter underflow) just before writing data lets not flag the underflow.
This is anyways going to go once we write
the data.
--- drivers/i2c/busses/i2c-omap.c | 43 ++++++++++++++++++++++++++++------------ 1 files changed, 30 insertions(+), 13 deletions(-)
drivers/i2c/busses/i2c-omap.c +30 -13
@@ -725,27 +725,30 @@ omap_i2c_omap1_isr(int this_irq, void *dev_id)* 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_i462 (struct omap_i2c_dev *dev,u16 *stat,int *err) +static int errata_omap3_i462 (struct omap_i2c_dev *dev) {unsigned long timeout =10000 ; +u16 stat; -while (--timeout && !(*stat &OMAP_I2C_STAT_XUDF )) { -if (*stat & (OMAP_I2C_STAT_NACK |OMAP_I2C_STAT_AL )) { +do { + stat =omap_i2c_read_reg (dev,OMAP_I2C_STAT_REG ); +if (stat &OMAP_I2C_STAT_XUDF ) +break ; + +if (stat & (OMAP_I2C_STAT_NACK |OMAP_I2C_STAT_AL )) {omap_i2c_ack_stat (dev, (OMAP_I2C_STAT_XRDY |OMAP_I2C_STAT_XDR )); -return -ETIMEDOUT ; +return -EIO ; }cpu_relax (); - *stat =omap_i2c_read_reg (dev,OMAP_I2C_STAT_REG ); - } + }while (--timeout);if (!timeout) {dev_err (dev->dev,"timeout waiting on XUDF bit\n" );return 0 ; } - *err |=OMAP_I2C_STAT_XUDF ;return 0 ; }
@@ -903,9 +906,16 @@ complete: } } -if ((dev->errata &I2C_OMAP_ERRATA_I462 ) && -errata_omap3_i462 (dev, &stat, &err)) - goto complete; +if (dev->errata &I2C_OMAP_ERRATA_I462 ) { +int ret; + + ret =errata_omap3_i462 (dev); + stat =omap_i2c_read_reg (dev, +OMAP_I2C_STAT_REG ); + +if (ret <0 ) + goto complete; + }omap_i2c_write_reg (dev,OMAP_I2C_DATA_REG , w); }
@@ -943,9 +953,16 @@ complete: } } -if ((dev->errata &I2C_OMAP_ERRATA_I462 ) && -errata_omap3_i462 (dev, &stat, &err)) - goto complete; +if (dev->errata &I2C_OMAP_ERRATA_I462 ) { +int ret; + + ret =errata_omap3_i462 (dev); + stat =omap_i2c_read_reg (dev, +OMAP_I2C_STAT_REG ); + +if (ret <0 ) + goto complete; + }omap_i2c_write_reg (dev,OMAP_I2C_DATA_REG , w); }
Signature · 1 line
1.7.5.4
PATCH v8 8/22 i2c: omap: re-factor receive/transmit data loop
In reply to Shubhrajyoti DFrom: Felipe Balbi <[email protected]>
re-factor the common parts to a separate function, so that code is easier to read and understand.
No functional changes.
--- drivers/i2c/busses/i2c-omap.c | 204 ++++++++++++++++------------------------ 1 files changed, 82 insertions(+), 122 deletions(-)
drivers/i2c/busses/i2c-omap.c +82 -122
@@ -752,12 +752,81 @@ static int errata_omap3_i462(struct omap_i2c_dev *dev)return 0 ; } +static void omap_i2c_receive_data (struct omap_i2c_dev *dev,u8 num_bytes, +bool is_rdr) +{ +u16 w; + +while (num_bytes--) { +if (!dev->buf_len) { +dev_err (dev->dev,"%s without data" , + is_rdr ?"RDR" :"RRDY" ); +break ; + } + + w =omap_i2c_read_reg (dev,OMAP_I2C_DATA_REG ); + *dev->buf++ = w; + dev->buf_len--; + +/* +* Data reg in 2430, omap3 and +* omap4 is 8 bit wide +*/ +if (dev->flags &OMAP_I2C_FLAG_16BIT_DATA_REG ) { +if (dev->buf_len) { + *dev->buf++ = w >>8 ; + dev->buf_len--; + } + } + } +} + +static int omap_i2c_transmit_data (struct omap_i2c_dev *dev,u8 num_bytes, +bool is_xdr) +{ +u16 w; + +while (num_bytes--) { +if (!dev->buf_len) { +dev_err (dev->dev,"%s without data" , + is_xdr ?"XDR" :"XRDY" ); +break ; + } + + w = *dev->buf++; + dev->buf_len--; + +/* +* Data reg in 2430, omap3 and +* omap4 is 8 bit wide +*/ +if (dev->flags &OMAP_I2C_FLAG_16BIT_DATA_REG ) { +if (dev->buf_len) { + w |= *dev->buf++ <<8 ; + dev->buf_len--; + } + } + +if (dev->errata &I2C_OMAP_ERRATA_I462 ) { +int ret; + + ret =errata_omap3_i462 (dev); +if (ret <0 ) +return ret; + } + +omap_i2c_write_reg (dev,OMAP_I2C_DATA_REG , w); + } + +return 0 ; +} +static irqreturn_t omap_i2c_isr (int this_irq,void *dev_id) {struct omap_i2c_dev *dev = dev_id;u16 bits; -u16 stat, w; +u16 stat;int err, count =0 ;if (pm_runtime_suspended (dev->dev))
@@ -810,30 +879,7 @@ complete:if (dev->fifo_size) num_bytes = dev->buf_len; -while (num_bytes--) { -if (!dev->buf_len) { -dev_err (dev->dev, -"RDR IRQ while no data" -" requested\n" ); -break ; - } - - w =omap_i2c_read_reg (dev,OMAP_I2C_DATA_REG ); - *dev->buf++ = w; - dev->buf_len--; - -/* -* Data reg in 2430, omap3 and -* omap4 is 8 bit wide -*/ -if (dev->flags & -OMAP_I2C_FLAG_16BIT_DATA_REG ) { -if (dev->buf_len) { - *dev->buf++ = w >>8 ; - dev->buf_len--; - } - } - } +omap_i2c_receive_data (dev, num_bytes, true);if (dev->errata &I2C_OMAP_ERRATA_I207 )i2c_omap_errata_i207 (dev, stat);
@@ -848,77 +894,22 @@ complete:if (dev->fifo_size) num_bytes = dev->fifo_size; -while (num_bytes--) { -if (!dev->buf_len) { -dev_err (dev->dev, -"RRDY IRQ while no data" -" requested\n" ); -break ; - } - - w =omap_i2c_read_reg (dev,OMAP_I2C_DATA_REG ); - *dev->buf++ = w; - dev->buf_len--; - -/* -* Data reg in 2430, omap3 and -* omap4 is 8 bit wide -*/ -if (dev->flags & -OMAP_I2C_FLAG_16BIT_DATA_REG ) { -if (dev->buf_len) { - *dev->buf++ = w >>8 ; - dev->buf_len--; - } - } - } - +omap_i2c_receive_data (dev, num_bytes, false);omap_i2c_ack_stat (dev,OMAP_I2C_STAT_RRDY );continue ; }if (stat &OMAP_I2C_STAT_XDR ) {u8 num_bytes =1 ; +int ret;if (dev->fifo_size) num_bytes = dev->buf_len; -while (num_bytes--) { -if (!dev->buf_len) { -dev_err (dev->dev, -"XDR IRQ while no " -"data to send\n" ); -break ; - } - - w = *dev->buf++; - dev->buf_len--; - -/* -* Data reg in 2430, omap3 and -* omap4 is 8 bit wide -*/ -if (dev->flags & -OMAP_I2C_FLAG_16BIT_DATA_REG ) { -if (dev->buf_len) { - w |= *dev->buf++ <<8 ; - dev->buf_len--; - } - } - -if (dev->errata &I2C_OMAP_ERRATA_I462 ) { -int ret; - - ret =errata_omap3_i462 (dev); - stat =omap_i2c_read_reg (dev, -OMAP_I2C_STAT_REG ); - -if (ret <0 ) - goto complete; - } - -omap_i2c_write_reg (dev,OMAP_I2C_DATA_REG , w); - } + ret =omap_i2c_transmit_data (dev, num_bytes, true); + stat =omap_i2c_read_reg (dev,OMAP_I2C_STAT_REG ); +if (ret <0 ) + goto complete;omap_i2c_ack_stat (dev,OMAP_I2C_STAT_XDR );continue ;
@@ -926,46 +917,15 @@ complete:if (stat &OMAP_I2C_STAT_XRDY ) {u8 num_bytes =1 ; +int ret;if (dev->fifo_size) num_bytes = dev->fifo_size; -while (num_bytes--) { -if (!dev->buf_len) { -dev_err (dev->dev, -"XRDY IRQ while no " -"data to send\n" ); -break ; - } - - w = *dev->buf++; - dev->buf_len--; - -/* -* Data reg in 2430, omap3 and -* omap4 is 8 bit wide -*/ -if (dev->flags & -OMAP_I2C_FLAG_16BIT_DATA_REG ) { -if (dev->buf_len) { - w |= *dev->buf++ <<8 ; - dev->buf_len--; - } - } - -if (dev->errata &I2C_OMAP_ERRATA_I462 ) { -int ret; - - ret =errata_omap3_i462 (dev); - stat =omap_i2c_read_reg (dev, -OMAP_I2C_STAT_REG ); - -if (ret <0 ) - goto complete; - } - -omap_i2c_write_reg (dev,OMAP_I2C_DATA_REG , w); - } + ret =omap_i2c_transmit_data (dev, num_bytes, false); + stat =omap_i2c_read_reg (dev,OMAP_I2C_STAT_REG ); +if (ret <0 ) + goto complete;omap_i2c_ack_stat (dev,OMAP_I2C_STAT_XRDY );continue ;
Signature · 1 line
1.7.5.4
PATCH v8 9/22 i2c: omap: switch over to do {} while loop
In reply to Shubhrajyoti DFrom: Felipe Balbi <[email protected]>
this will make sure that we execute at least once. No functional changes otherwise.
--- drivers/i2c/busses/i2c-omap.c | 20 ++++++++++++++------ 1 files changed, 14 insertions(+), 6 deletions(-)
drivers/i2c/busses/i2c-omap.c +14 -6
@@ -827,20 +827,28 @@ omap_i2c_isr(int this_irq, void *dev_id)struct omap_i2c_dev *dev = dev_id;u16 bits;u16 stat; -int err, count =0 ; +int err =0 , count =0 ;if (pm_runtime_suspended (dev->dev))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) { + do { + bits = omap_i2c_read_reg(dev, OMAP_I2C_IE_REG); + stat = omap_i2c_read_reg(dev, OMAP_I2C_STAT_REG); + stat &= bits; + + if (!stat) { + /* my work here is done */ + return IRQ_HANDLED; + } + dev_dbg(dev->dev, "IRQ (ISR = 0x%04x)\n", stat); if (count++ == 100) { dev_warn(dev->dev, "Too much work in one IRQ\n"); -break ; + omap_i2c_complete_cmd(dev, err); + return IRQ_HANDLED; } - err =0 ; complete: /* * Ack the stat in one go, but [R/X]DR and [R/X]RDY should be
@@ -940,7 +948,7 @@ complete:dev_err (dev->dev,"Transmit underflow\n" ); dev->cmd_err |=OMAP_I2C_STAT_XUDF ; } - } + }while (stat);return count ?IRQ_HANDLED :IRQ_NONE ; }
Signature · 1 line
1.7.5.4
PATCH v8 10/22 i2c: omap: ack IRQ in parts
In reply to Shubhrajyoti DFrom: Felipe Balbi <[email protected]>
According to flow diagrams on OMAP TRMs,
we should ACK the IRQ as they happen.
--- drivers/i2c/busses/i2c-omap.c | 28 ++++++++++++++++------------ 1 files changed, 16 insertions(+), 12 deletions(-)
drivers/i2c/busses/i2c-omap.c +16 -12
@@ -850,21 +850,19 @@ omap_i2c_isr(int this_irq, void *dev_id) } complete: -/* -* Ack the stat in one go, but [R/X]DR and [R/X]RDY should be -* acked after the data operation is complete. -* Ref: TRM SWPU114Q Figure 18-31 -*/ -omap_i2c_write_reg (dev,OMAP_I2C_STAT_REG , stat & - ~(OMAP_I2C_STAT_RRDY |OMAP_I2C_STAT_RDR | -OMAP_I2C_STAT_XRDY |OMAP_I2C_STAT_XDR )); - -if (stat &OMAP_I2C_STAT_NACK ) +if (stat &OMAP_I2C_STAT_NACK ) { err |=OMAP_I2C_STAT_NACK ; +omap_i2c_ack_stat (dev,OMAP_I2C_STAT_NACK ); +omap_i2c_complete_cmd (dev, err); +return IRQ_HANDLED ; + }if (stat &OMAP_I2C_STAT_AL ) {dev_err (dev->dev,"Arbitration lost\n" ); err |=OMAP_I2C_STAT_AL ; +omap_i2c_ack_stat (dev,OMAP_I2C_STAT_AL ); +omap_i2c_complete_cmd (dev, err); +return IRQ_HANDLED ; }/*
@@ -941,12 +939,18 @@ complete:if (stat &OMAP_I2C_STAT_ROVR ) {dev_err (dev->dev,"Receive overrun\n" ); - dev->cmd_err |=OMAP_I2C_STAT_ROVR ; + err |=OMAP_I2C_STAT_ROVR ; +omap_i2c_ack_stat (dev,OMAP_I2C_STAT_ROVR ); +omap_i2c_complete_cmd (dev, err); +return IRQ_HANDLED ; }if (stat &OMAP_I2C_STAT_XUDF ) {dev_err (dev->dev,"Transmit underflow\n" ); - dev->cmd_err |=OMAP_I2C_STAT_XUDF ; + err |=OMAP_I2C_STAT_XUDF ; +omap_i2c_ack_stat (dev,OMAP_I2C_STAT_XUDF ); +omap_i2c_complete_cmd (dev, err); +return IRQ_HANDLED ; } }while (stat);
Signature · 1 line
1.7.5.4
PATCH v8 11/22 i2c: omap: switch to platform_get_irq()
In reply to Shubhrajyoti DFrom: Felipe Balbi <[email protected]>
that's a nice helper from drivers core which will give us the exact IRQ number, instead of a pointer to an IRQ resource.
--- drivers/i2c/busses/i2c-omap.c | 12 +++++++----- 1 files changed, 7 insertions(+), 5 deletions(-)
drivers/i2c/busses/i2c-omap.c +7 -5
@@ -993,11 +993,12 @@ omap_i2c_probe(struct platform_device *pdev) {struct omap_i2c_dev *dev;struct i2c_adapter *adap; -struct resource *mem, *irq; +struct resource *mem;struct omap_i2c_bus_platform_data *pdata = pdev->dev.platform_data;struct device_node *node = pdev->dev.of_node;const struct of_device_id *match;irq_handler_t isr; +int irq;int r;/* NOTE: driver uses the static register mapping */
@@ -1006,10 +1007,11 @@ omap_i2c_probe(struct platform_device *pdev)dev_err (&pdev->dev,"no mem resource?\n" );return -ENODEV ; } - irq =platform_get_resource (pdev,IORESOURCE_IRQ ,0 ); -if (!irq) { + + irq =platform_get_irq (pdev,0 ); +if (irq <0 ) {dev_err (&pdev->dev,"no irq resource?\n" ); -return -ENODEV ; +return irq; } dev =devm_kzalloc (&pdev->dev,sizeof (struct omap_i2c_dev ),GFP_KERNEL );
@@ -1043,7 +1045,7 @@ omap_i2c_probe(struct platform_device *pdev) } dev->dev = &pdev->dev; - dev->irq = irq->start; + dev->irq = irq;platform_set_drvdata (pdev, dev);init_completion (&dev->cmd_complete);
Signature · 1 line
1.7.5.4
PATCH v8 12/22 i2c: omap: bus: add a receiver flag
In reply to Shubhrajyoti DFrom: Felipe Balbi <[email protected]>
that way we can ignore TX IRQs while in receiver mode and ignore RX IRQs while in transmitter mode.
--- drivers/i2c/busses/i2c-omap.c | 8 ++++++++ 1 files changed, 8 insertions(+), 0 deletions(-)
drivers/i2c/busses/i2c-omap.c +8 -0
@@ -199,6 +199,7 @@ struct omap_i2c_dev {*/ u8 rev; unsigned b_hw:1; /* bad h/w fixes */ + unsigned receiver:1; /* true when we're in receiver mode */u16 iestate;/* Saved interrupt register */ u16 pscstate;u16 scllstate;
@@ -492,6 +493,7 @@ static int omap_i2c_xfer_msg(struct i2c_adapter *adap,INIT_COMPLETION (dev->cmd_complete); dev->cmd_err =0 ; + dev->receiver = !!(msg->flags &I2C_M_RD ); w =OMAP_I2C_CON_EN |OMAP_I2C_CON_MST |OMAP_I2C_CON_STT ;
@@ -837,6 +839,12 @@ omap_i2c_isr(int this_irq, void *dev_id) stat =omap_i2c_read_reg (dev,OMAP_I2C_STAT_REG ); stat &= bits; +/* If we're in receiver mode, ignore XDR/XRDY */ +if (dev->receiver) + stat &= ~(OMAP_I2C_STAT_XDR |OMAP_I2C_STAT_XRDY ); +else + stat &= ~(OMAP_I2C_STAT_RDR |OMAP_I2C_STAT_RRDY ); +if (!stat) {/* my work here is done */ return IRQ_HANDLED ;
Signature · 1 line
1.7.5.4
PATCH v8 13/22 i2c: omap: simplify errata check
In reply to Shubhrajyoti DFrom: Felipe Balbi <[email protected]>
omap_i2c_dev is allocated with kzalloc(),
so we need not initialize b_hw to zero.
--- drivers/i2c/busses/i2c-omap.c | 4 +--- 1 files changed, 1 insertions(+), 3 deletions(-)
drivers/i2c/busses/i2c-omap.c +1 -3
@@ -1095,9 +1095,7 @@ omap_i2c_probe(struct platform_device *pdev) dev->fifo_size = (dev->fifo_size /2 ); -if (dev->rev >=OMAP_I2C_REV_ON_3630_4430 ) - dev->b_hw =0 ;/* Disable hardware fixes */ -else +if (dev->rev <OMAP_I2C_REV_ON_3630_4430 ) dev->b_hw =1 ;/* Enable hardware fixes */ /* calculate wakeup latency constraint for MPU */
Signature · 1 line
1.7.5.4
PATCH v8 14/22 i2c: omap: always return IRQ_HANDLED
In reply to Shubhrajyoti DFrom: Felipe Balbi <[email protected]>
Always return IRQ_HANDLED otherwise we could get our IRQ line disabled due to many spurious IRQs.
--- drivers/i2c/busses/i2c-omap.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
drivers/i2c/busses/i2c-omap.c +1 -1
@@ -962,7 +962,7 @@ complete: } }while (stat); -return count ?IRQ_HANDLED :IRQ_NONE ; +return IRQ_HANDLED ; }static const struct i2c_algorithm omap_i2c_algo = {
Signature · 1 line
1.7.5.4
PATCH v8 15/22 i2c: omap: simplify IRQ exit path
In reply to Shubhrajyoti DFrom: Felipe Balbi <[email protected]>
instead of having multiple return points, use
a goto statement to make that clearer.
--- drivers/i2c/busses/i2c-omap.c | 20 ++++++++------------ 1 files changed, 8 insertions(+), 12 deletions(-)
drivers/i2c/busses/i2c-omap.c +8 -12
@@ -853,24 +853,21 @@ omap_i2c_isr(int this_irq, void *dev_id)dev_dbg (dev->dev,"IRQ (ISR = 0x%04x)\n" , stat);if (count++ ==100 ) {dev_warn (dev->dev,"Too much work in one IRQ\n" ); -omap_i2c_complete_cmd (dev, err); -return IRQ_HANDLED ; + goto out; } complete:if (stat &OMAP_I2C_STAT_NACK ) { err |=OMAP_I2C_STAT_NACK ;omap_i2c_ack_stat (dev,OMAP_I2C_STAT_NACK ); -omap_i2c_complete_cmd (dev, err); -return IRQ_HANDLED ; + goto out; }if (stat &OMAP_I2C_STAT_AL ) {dev_err (dev->dev,"Arbitration lost\n" ); err |=OMAP_I2C_STAT_AL ;omap_i2c_ack_stat (dev,OMAP_I2C_STAT_AL ); -omap_i2c_complete_cmd (dev, err); -return IRQ_HANDLED ; + goto out; }/*
@@ -883,8 +880,7 @@ complete:OMAP_I2C_STAT_XRDY |OMAP_I2C_STAT_XDR | OMAP_I2C_STAT_ARDY)); -omap_i2c_complete_cmd (dev, err); -return IRQ_HANDLED ; + goto out; }if (stat &OMAP_I2C_STAT_RDR ) {
@@ -949,19 +945,19 @@ complete:dev_err (dev->dev,"Receive overrun\n" ); err |=OMAP_I2C_STAT_ROVR ;omap_i2c_ack_stat (dev,OMAP_I2C_STAT_ROVR ); -omap_i2c_complete_cmd (dev, err); -return IRQ_HANDLED ; + goto out; }if (stat &OMAP_I2C_STAT_XUDF ) {dev_err (dev->dev,"Transmit underflow\n" ); err |=OMAP_I2C_STAT_XUDF ;omap_i2c_ack_stat (dev,OMAP_I2C_STAT_XUDF ); -omap_i2c_complete_cmd (dev, err); -return IRQ_HANDLED ; + goto out; } }while (stat); +out: +omap_i2c_complete_cmd (dev, err);return IRQ_HANDLED ; }
Signature · 1 line
1.7.5.4
PATCH v8 16/22 i2c: omap: resize fifos before each message
In reply to Shubhrajyoti DFrom: Felipe Balbi <[email protected]>
This patch will try to avoid the usage of draining feature by reconfiguring the FIFO the start condition of each message based on the message's size.
By doing that, we will be better utilizing the FIFO when doing big transfers.
While at that also drop the now unneeded check for dev->buf_len as we always know the amount of data to be transmitted.
--- drivers/i2c/busses/i2c-omap.c | 83 +++++++++++++++++++++++++---------------- 1 files changed, 51 insertions(+), 32 deletions(-)
drivers/i2c/busses/i2c-omap.c +51 -32
@@ -193,6 +193,7 @@ struct omap_i2c_dev {u8 *regs;size_t buf_len;struct i2c_adapter adapter; +u8 threshold;u8 fifo_size;/* use as flag and value * fifo_size==0 implies no fifo * if set, should be trsh+1
@@ -418,13 +419,6 @@ static int omap_i2c_init(struct omap_i2c_dev *dev)omap_i2c_write_reg (dev,OMAP_I2C_SCLL_REG , scll);omap_i2c_write_reg (dev,OMAP_I2C_SCLH_REG , sclh); -if (dev->fifo_size) { -/* Note: setup required fifo size - 1. RTRSH and XTRSH */ - buf = (dev->fifo_size -1 ) <<8 |OMAP_I2C_BUF_RXFIF_CLR | - (dev->fifo_size -1 ) |OMAP_I2C_BUF_TXFIF_CLR ; -omap_i2c_write_reg (dev,OMAP_I2C_BUF_REG , buf); - } -/* Take the I2C module out of reset: */ omap_i2c_write_reg (dev,OMAP_I2C_CON_REG ,OMAP_I2C_CON_EN );
@@ -462,6 +456,45 @@ static int omap_i2c_wait_for_bb(struct omap_i2c_dev *dev)return 0 ; } +static void omap_i2c_resize_fifo (struct omap_i2c_dev *dev,u8 size,bool is_rx) +{ +u16 buf; + +if (dev->flags &OMAP_I2C_FLAG_NO_FIFO ) +return ; + +/* +* Set up notification threshold based on message size. We're doing +* this to try and avoid draining feature as much as possible. Whenever +* we have big messages to transfer (bigger than our total fifo size) +* then we might use draining feature to transfer the remaining bytes. +*/ + + dev->threshold =clamp (size, (u8 )1 , dev->fifo_size); + + buf =omap_i2c_read_reg (dev,OMAP_I2C_BUF_REG ); + +if (is_rx) { +/* Clear RX Threshold */ + buf &= ~(0x3f <<8 ); + buf |= ((dev->threshold -1 ) <<8 ) |OMAP_I2C_BUF_RXFIF_CLR ; + }else { +/* Clear TX Threshold */ + buf &= ~0x3f ; + buf |= (dev->threshold -1 ) |OMAP_I2C_BUF_TXFIF_CLR ; + } + +omap_i2c_write_reg (dev,OMAP_I2C_BUF_REG , buf); + +if (dev->rev <OMAP_I2C_REV_ON_3630_4430 ) + dev->b_hw =1 ;/* Enable hardware fixes */ + +/* calculate wakeup latency constraint for MPU */ +if (dev->set_mpu_wkup_lat !=NULL ) + dev->latency = (1000000 * dev->threshold) / + (1000 * dev->speed /8 ); +} +/* * Low level master read/write transaction. */
@@ -478,6 +511,9 @@ static int omap_i2c_xfer_msg(struct i2c_adapter *adap,if (msg->len ==0 )return -EINVAL ; + dev->receiver = !!(msg->flags &I2C_M_RD ); +omap_i2c_resize_fifo (dev, msg->len, dev->receiver); +omap_i2c_write_reg (dev,OMAP_I2C_SA_REG , msg->addr);/* REVISIT: Could the STB bit of I2C_CON be used with probing? */
@@ -493,7 +529,6 @@ static int omap_i2c_xfer_msg(struct i2c_adapter *adap,INIT_COMPLETION (dev->cmd_complete); dev->cmd_err =0 ; - dev->receiver = !!(msg->flags &I2C_M_RD ); w =OMAP_I2C_CON_EN |OMAP_I2C_CON_MST |OMAP_I2C_CON_STT ;
@@ -760,12 +795,6 @@ static void omap_i2c_receive_data(struct omap_i2c_dev *dev, u8 num_bytes,u16 w;while (num_bytes--) { -if (!dev->buf_len) { -dev_err (dev->dev,"%s without data" , - is_rdr ?"RDR" :"RRDY" ); -break ; - } - w =omap_i2c_read_reg (dev,OMAP_I2C_DATA_REG ); *dev->buf++ = w; dev->buf_len--;
@@ -775,10 +804,8 @@ static void omap_i2c_receive_data(struct omap_i2c_dev *dev, u8 num_bytes,* omap4 is 8 bit wide */ if (dev->flags &OMAP_I2C_FLAG_16BIT_DATA_REG ) { -if (dev->buf_len) { - *dev->buf++ = w >>8 ; - dev->buf_len--; - } + *dev->buf++ = w >>8 ; + dev->buf_len--; } } }
@@ -789,12 +816,6 @@ static int omap_i2c_transmit_data(struct omap_i2c_dev *dev, u8 num_bytes,u16 w;while (num_bytes--) { -if (!dev->buf_len) { -dev_err (dev->dev,"%s without data" , - is_xdr ?"XDR" :"XRDY" ); -break ; - } - w = *dev->buf++; dev->buf_len--;
@@ -803,10 +824,8 @@ static int omap_i2c_transmit_data(struct omap_i2c_dev *dev, u8 num_bytes,* omap4 is 8 bit wide */ if (dev->flags &OMAP_I2C_FLAG_16BIT_DATA_REG ) { -if (dev->buf_len) { - w |= *dev->buf++ <<8 ; - dev->buf_len--; - } + w |= *dev->buf++ <<8 ; + dev->buf_len--; }if (dev->errata &I2C_OMAP_ERRATA_I462 ) {
@@ -901,8 +920,8 @@ complete:if (stat &OMAP_I2C_STAT_RRDY ) {u8 num_bytes =1 ; -if (dev->fifo_size) - num_bytes = dev->fifo_size; +if (dev->threshold) + num_bytes = dev->threshold;omap_i2c_receive_data (dev, num_bytes, false);omap_i2c_ack_stat (dev,OMAP_I2C_STAT_RRDY );
@@ -929,8 +948,8 @@ complete:u8 num_bytes =1 ;int ret; -if (dev->fifo_size) - num_bytes = dev->fifo_size; +if (dev->threshold) + num_bytes = dev->threshold; ret =omap_i2c_transmit_data (dev, num_bytes, false); stat =omap_i2c_read_reg (dev,OMAP_I2C_STAT_REG );
Signature · 1 line
1.7.5.4
PATCH v8 17/22 i2c: omap: get rid of the "complete" label
In reply to Shubhrajyoti DFrom: Felipe Balbi <[email protected]>
we can ack stat and complete the command from
the errata handling itself.
--- drivers/i2c/busses/i2c-omap.c | 16 +++++++++++++--- 1 files changed, 13 insertions(+), 3 deletions(-)
drivers/i2c/busses/i2c-omap.c +13 -3
@@ -775,6 +775,17 @@ static int errata_omap3_i462(struct omap_i2c_dev *dev)if (stat & (OMAP_I2C_STAT_NACK |OMAP_I2C_STAT_AL )) {omap_i2c_ack_stat (dev, (OMAP_I2C_STAT_XRDY |OMAP_I2C_STAT_XDR )); +if (stat &OMAP_I2C_STAT_NACK ) { + dev->cmd_err |=OMAP_I2C_STAT_NACK ; +omap_i2c_ack_stat (dev,OMAP_I2C_STAT_NACK ); + } + +if (stat &OMAP_I2C_STAT_AL ) { +dev_err (dev->dev,"Arbitration lost\n" ); + dev->cmd_err |=OMAP_I2C_STAT_AL ; +omap_i2c_ack_stat (dev,OMAP_I2C_STAT_NACK ); + } +return -EIO ; }
@@ -875,7 +886,6 @@ omap_i2c_isr(int this_irq, void *dev_id) goto out; } -complete:if (stat &OMAP_I2C_STAT_NACK ) { err |=OMAP_I2C_STAT_NACK ;omap_i2c_ack_stat (dev,OMAP_I2C_STAT_NACK );
@@ -938,7 +948,7 @@ complete: ret =omap_i2c_transmit_data (dev, num_bytes, true); stat =omap_i2c_read_reg (dev,OMAP_I2C_STAT_REG );if (ret <0 ) - goto complete; + goto out;omap_i2c_ack_stat (dev,OMAP_I2C_STAT_XDR );continue ;
@@ -954,7 +964,7 @@ complete: ret =omap_i2c_transmit_data (dev, num_bytes, false); stat =omap_i2c_read_reg (dev,OMAP_I2C_STAT_REG );if (ret <0 ) - goto complete; + goto out;omap_i2c_ack_stat (dev,OMAP_I2C_STAT_XRDY );continue ;
Signature · 1 line
1.7.5.4
PATCH v8 18/22 i2c: omap: remove redundant status read
In reply to Shubhrajyoti DCurrently omap_i2c_ack_stat doesn't use the stat variable. After the read of the I2C_STAT_REG it is not used. Remove the redundant read of the status register.
--- drivers/i2c/busses/i2c-omap.c | 2 -- 1 files changed, 0 insertions(+), 2 deletions(-)
drivers/i2c/busses/i2c-omap.c +0 -2
@@ -946,7 +946,6 @@ omap_i2c_isr(int this_irq, void *dev_id) num_bytes = dev->buf_len; ret =omap_i2c_transmit_data (dev, num_bytes, true); - stat =omap_i2c_read_reg (dev,OMAP_I2C_STAT_REG );if (ret <0 ) goto out;
@@ -962,7 +961,6 @@ omap_i2c_isr(int this_irq, void *dev_id) num_bytes = dev->threshold; ret =omap_i2c_transmit_data (dev, num_bytes, false); - stat =omap_i2c_read_reg (dev,OMAP_I2C_STAT_REG );if (ret <0 ) goto out;
Signature · 1 line
1.7.5.4
PATCH v8 19/22 i2c: omap: switch to threaded IRQ support
In reply to Shubhrajyoti DFrom: Felipe Balbi <[email protected]>
for OMAP2, we can easily switch over to threaded
IRQs on the I2C driver. This will allow us to
spend less time in hardirq context.
--- drivers/i2c/busses/i2c-omap.c | 43 +++++++++++++++++++++++++++++++++++----- 1 files changed, 37 insertions(+), 6 deletions(-)
drivers/i2c/busses/i2c-omap.c +37 -6
@@ -176,6 +176,7 @@ enum {#define I2C_OMAP_ERRATA_I462 (1 << 1)struct omap_i2c_dev { +spinlock_t lock;/* IRQ synchronization */ struct device *dev;void __iomem *base;/* virtual */ int irq;
@@ -854,9 +855,30 @@ static int omap_i2c_transmit_data(struct omap_i2c_dev *dev, u8 num_bytes, }static irqreturn_t -omap_i2c_isr (int this_irq,void *dev_id) +omap_i2c_isr (int irq,void *dev_id) {struct omap_i2c_dev *dev = dev_id; +irqreturn_t ret =IRQ_HANDLED ; +u16 mask; +u16 stat; + +spin_lock (&dev->lock); + mask =omap_i2c_read_reg (dev,OMAP_I2C_IE_REG ); + stat =omap_i2c_read_reg (dev,OMAP_I2C_STAT_REG ); + +if (stat & mask) + ret =IRQ_WAKE_THREAD ; + +spin_unlock (&dev->lock); + +return ret; +} + +static irqreturn_t +omap_i2c_isr_thread (int this_irq,void *dev_id) +{ +struct omap_i2c_dev *dev = dev_id; +unsigned long flags;u16 bits;u16 stat;int err =0 , count =0 ;
@@ -864,6 +886,7 @@ omap_i2c_isr(int this_irq, void *dev_id)if (pm_runtime_suspended (dev->dev))return IRQ_NONE ; +spin_lock_irqsave (&dev->lock, flags); do { bits = omap_i2c_read_reg(dev, OMAP_I2C_IE_REG); stat = omap_i2c_read_reg(dev, OMAP_I2C_STAT_REG);
@@ -877,6 +900,7 @@ omap_i2c_isr(int this_irq, void *dev_id)if (!stat) {/* my work here is done */ +spin_unlock_irqrestore (&dev->lock, flags);return IRQ_HANDLED ; }
@@ -985,6 +1009,8 @@ omap_i2c_isr(int this_irq, void *dev_id) out:omap_i2c_complete_cmd (dev, err); +spin_unlock_irqrestore (&dev->lock, flags); +return IRQ_HANDLED ; }
@@ -1028,7 +1054,6 @@ omap_i2c_probe(struct platform_device *pdev)struct omap_i2c_bus_platform_data *pdata = pdev->dev.platform_data;struct device_node *node = pdev->dev.of_node;const struct of_device_id *match; -irq_handler_t isr;int irq;int r;
@@ -1078,6 +1103,8 @@ omap_i2c_probe(struct platform_device *pdev) dev->dev = &pdev->dev; dev->irq = irq; +spin_lock_init (&dev->lock); +platform_set_drvdata (pdev, dev);init_completion (&dev->cmd_complete);
@@ -1130,10 +1157,14 @@ omap_i2c_probe(struct platform_device *pdev)/* reset ASAP, clearing any IRQs */ omap_i2c_init (dev); - isr = (dev->rev <OMAP_I2C_OMAP1_REV_2 ) ? omap_i2c_omap1_isr : - omap_i2c_isr; - r =devm_request_irq (&pdev->dev, dev->irq, isr,IRQF_NO_SUSPEND , - pdev->name, dev); +if (dev->rev <OMAP_I2C_OMAP1_REV_2 ) + r =devm_request_irq (&pdev->dev, dev->irq, omap_i2c_omap1_isr, +IRQF_NO_SUSPEND , pdev->name, dev); +else + r =devm_request_threaded_irq (&pdev->dev, dev->irq, + omap_i2c_isr, omap_i2c_isr_thread, +IRQF_NO_SUSPEND |IRQF_ONESHOT , + pdev->name, dev);if (r) {dev_err (dev->dev,"failure requesting irq %i\n" , dev->irq);
Signature · 1 line
1.7.5.4
PATCH v8 20/22 i2c: omap: remove unnecessary pm_runtime_suspended check
In reply to Shubhrajyoti DFrom: Felipe Balbi <[email protected]>
before starting any messages we call pm_runtime_get_sync() which will make sure that by the time we program a transfer and our IRQ handler gets called, we're not suspended anymore.
--- drivers/i2c/busses/i2c-omap.c | 3 --- 1 files changed, 0 insertions(+), 3 deletions(-)
drivers/i2c/busses/i2c-omap.c +0 -3
@@ -883,9 +883,6 @@ omap_i2c_isr_thread(int this_irq, void *dev_id)u16 stat;int err =0 , count =0 ; -if (pm_runtime_suspended (dev->dev)) -return IRQ_NONE ; -spin_lock_irqsave (&dev->lock, flags); do { bits = omap_i2c_read_reg(dev, OMAP_I2C_IE_REG);
Signature · 1 line
1.7.5.4
PATCH v8 21/22 i2c: omap: switch over to autosuspend API
In reply to Shubhrajyoti DFrom: Felipe Balbi <[email protected]>
this helps us reduce unnecessary pm transitions in case we have another i2c message starting soon.
--- drivers/i2c/busses/i2c-omap.c | 12 ++++++++++-- 1 files changed, 10 insertions(+), 2 deletions(-)
drivers/i2c/busses/i2c-omap.c +10 -2
@@ -55,6 +55,9 @@/* timeout waiting for the controller to respond */ #define OMAP_I2C_TIMEOUT (msecs_to_jiffies(1000)) +/* timeout for pm runtime autosuspend */ +#define OMAP_I2C_PM_TIMEOUT 1000/* ms */ +/* For OMAP3 I2C_IV has changed to I2C_WE (wakeup enable) */ enum {OMAP_I2C_REV_REG =0 ,
@@ -645,7 +648,8 @@ omap_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[], int num)omap_i2c_wait_for_bb (dev); out: -pm_runtime_put (dev->dev); +pm_runtime_mark_last_busy (dev->dev); +pm_runtime_put_autosuspend (dev->dev);return r; }
@@ -1113,6 +1117,9 @@ omap_i2c_probe(struct platform_device *pdev) dev->regs = (u8 *)reg_map_ip_v1;pm_runtime_enable (dev->dev); +pm_runtime_set_autosuspend_delay (dev->dev,OMAP_I2C_PM_TIMEOUT ); +pm_runtime_use_autosuspend (dev->dev); + r =pm_runtime_get_sync (dev->dev);if (IS_ERR_VALUE (r)) goto err_free_mem;
@@ -1190,7 +1197,8 @@ omap_i2c_probe(struct platform_device *pdev)of_i2c_register_devices (adap); -pm_runtime_put (dev->dev); +pm_runtime_mark_last_busy (dev->dev); +pm_runtime_put_autosuspend (dev->dev);return 0 ;
Signature · 1 line
1.7.5.4
PATCH v8 21/22 i2c: omap: switch over to autosuspend API
Quoted 7 lines · Shubhrajyoti D
From: Felipe Balbi <[email protected]>
this helps us reduce unnecessary pm transitions in case we have another i2c message starting soon.
Signed-off-by Felipe Balbi <[email protected]>Signed-off-by Shubhrajyoti D <[email protected]>
I tracked the PM regression down to this patch.
Quoted 19 lines
--- drivers/i2c/busses/i2c-omap.c | 12 ++++++++++-- 1 files changed, 10 insertions(+), 2 deletions(-)drivers/i2c/busses/i2c-omap.c +3
-0@@ -55,6 +55,9 @@/* timeout waiting for the controller to respond */ #define OMAP_I2C_TIMEOUT (msecs_to_jiffies(1000)) +/* timeout for pm runtime autosuspend */ +#define OMAP_I2C_PM_TIMEOUT 1000/* ms */ +/* For OMAP3 I2C_IV has changed to I2C_WE (wakeup enable) */ enum {OMAP_I2C_REV_REG =0 ,
drivers/i2c/busses/i2c-omap.c omap_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[], int num) · line 652
Quoted 6 lines
drivers/i2c/busses/i2c-omap.c +2
-1@@ -645,7 +648,8 @@ omap_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[], int num)omap_i2c_wait_for_bb (dev); out: -pm_runtime_put (dev->dev); +pm_runtime_mark_last_busy (dev->dev); +pm_runtime_put_autosuspend (dev->dev);
Reverting this change allows CORE to hit retention again.
I didn't debug this any further, so I'm not sure exactly why the async suspend works but not the autosuspend.
Kevin
PATCH v8 22/22 i2c: omap: sanitize exit path
In reply to Shubhrajyoti DFrom: Felipe Balbi <[email protected]>
move the goto out label one line down, so that it can be used when stat is read as zero. All other exits, can be done with a break statement.
While at that, also break out as soon as we complete draining IRQ, since at that time we know we transferred everything there was to be transferred.
--- drivers/i2c/busses/i2c-omap.c | 26 +++++++++++++------------- 1 files changed, 13 insertions(+), 13 deletions(-)
drivers/i2c/busses/i2c-omap.c +13 -13
@@ -901,27 +901,26 @@ omap_i2c_isr_thread(int this_irq, void *dev_id)if (!stat) {/* my work here is done */ -spin_unlock_irqrestore (&dev->lock, flags); -return IRQ_HANDLED ; + goto out; }dev_dbg (dev->dev,"IRQ (ISR = 0x%04x)\n" , stat);if (count++ ==100 ) {dev_warn (dev->dev,"Too much work in one IRQ\n" ); - goto out; +break ; }if (stat &OMAP_I2C_STAT_NACK ) { err |=OMAP_I2C_STAT_NACK ;omap_i2c_ack_stat (dev,OMAP_I2C_STAT_NACK ); - goto out; +break ; }if (stat &OMAP_I2C_STAT_AL ) {dev_err (dev->dev,"Arbitration lost\n" ); err |=OMAP_I2C_STAT_AL ;omap_i2c_ack_stat (dev,OMAP_I2C_STAT_AL ); - goto out; +break ; }/*
@@ -934,7 +933,7 @@ omap_i2c_isr_thread(int this_irq, void *dev_id)OMAP_I2C_STAT_XRDY |OMAP_I2C_STAT_XDR | OMAP_I2C_STAT_ARDY)); - goto out; +break ; }if (stat &OMAP_I2C_STAT_RDR ) {
@@ -949,7 +948,7 @@ omap_i2c_isr_thread(int this_irq, void *dev_id)i2c_omap_errata_i207 (dev, stat);omap_i2c_ack_stat (dev,OMAP_I2C_STAT_RDR ); -continue ; +break ; }if (stat &OMAP_I2C_STAT_RRDY ) {
@@ -972,10 +971,10 @@ omap_i2c_isr_thread(int this_irq, void *dev_id) ret =omap_i2c_transmit_data (dev, num_bytes, true);if (ret <0 ) - goto out; +break ;omap_i2c_ack_stat (dev,OMAP_I2C_STAT_XDR ); -continue ; +break ; }if (stat &OMAP_I2C_STAT_XRDY ) {
@@ -987,7 +986,7 @@ omap_i2c_isr_thread(int this_irq, void *dev_id) ret =omap_i2c_transmit_data (dev, num_bytes, false);if (ret <0 ) - goto out; +break ;omap_i2c_ack_stat (dev,OMAP_I2C_STAT_XRDY );continue ;
@@ -997,19 +996,20 @@ omap_i2c_isr_thread(int this_irq, void *dev_id)dev_err (dev->dev,"Receive overrun\n" ); err |=OMAP_I2C_STAT_ROVR ;omap_i2c_ack_stat (dev,OMAP_I2C_STAT_ROVR ); - goto out; +break ; }if (stat &OMAP_I2C_STAT_XUDF ) {dev_err (dev->dev,"Transmit underflow\n" ); err |=OMAP_I2C_STAT_XUDF ;omap_i2c_ack_stat (dev,OMAP_I2C_STAT_XUDF ); - goto out; +break ; } }while (stat); -out:omap_i2c_complete_cmd (dev, err); + +out:spin_unlock_irqrestore (&dev->lock, flags);return IRQ_HANDLED ;
Signature · 1 line
1.7.5.4
Wolfram Sang
Appliedv8 cover I2C big cleanup
In reply to Shubhrajyoti DQuoted 29 lines · Shubhrajyoti D
Changes since v1:
- removed tabification on patch 6/17
- removeddev_err()which was introduced on patch 09/17
Changes since v2:
- do not set full fifo depth in the RDR interrupt.
- some changelog updates.
- rebase to the Wolfram's tree.
Changes since v3:
- Remove a redundant read of status register
- Read thedev->buf_lenvariable instead of the register as the information of the remaining bytes is there.
Changes since v4:
- Ack the arbitration lost.
- Rebase to the i2c-embedded/for-next branch.
Changes since v5:
- Rebase to latest mainline
- Added some more cleanup patches so as have a consolidated series.
Changes since v6:
- Fix comments on setting the pdev to NULL.
- Trivial changelog update
Changes since v7:
- Remove a patch as the code is getting changed anyways
- Changelogs update.Previous discussions can be found here
http://www.spinics.net/lists/linux-i2c/msg09764.htmlThis is the cleanup only series.
Quoted 8 lines
Tested on omap4sdp and 3430sdp.
The following changes since commit
55d512e245bc7699a8800e23df1a24195dd08217:Linux 3.6-rc5 (2012-09-08 16:43:45 -0700)
are available in the git repository at:
git://gitorious.org/linus-tree/linus-tree.gitfor_3.7/i2c/big_cleanups
Pushed to -next. Thanks to all involved!
Signature · 9 lines
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/20120912/629622f5/attachment.sig>
v8 cover I2C big cleanup
Quoted 9 lines · Wolfram Sang
Quoted 8 lines
Tested on omap4sdp and 3430sdp.
Quoted 7 lines
The following changes since commit
55d512e245bc7699a8800e23df1a24195dd08217:Linux 3.6-rc5 (2012-09-08 16:43:45 -0700)
are available in the git repository at:
git://gitorious.org/linus-tree/linus-tree.gitfor_3.7/i2c/big_cleanupsPushed to -next. Thanks to all involved!
Thanks.
v8 cover I2C big cleanup
In reply to Wolfram SangQuoted 1 line · Wolfram Sang
On Wed, Sep 12, 2012 at 04:27:54PM +0530, Shubhrajyoti D wrote:
[...]
Quoted 4 lines
Quoted 4 lines
This is the cleanup only series.
Tested on omap4sdp and 3430sdp.
Quoted 8 lines
Quoted 6 lines
The following changes since commit
55d512e245bc7699a8800e23df1a24195dd08217:Linux 3.6-rc5 (2012-09-08 16:43:45 -0700)
are available in the git repository at:
git://gitorious.org/linus-tree/linus-tree.gitfor_3.7/i2c/big_cleanupsPushed to -next. Thanks to all involved!
Sorry to be late to the party (again), but still catching up after some time off.
Unfortunately, this series causes PM regressions on several OMAP platforms. I hope we can hold off on this until those issues are addressed.
We *really* need to have some more thorough testing (especially PM) before merging large series like this.
Kevin
v8 cover I2C big cleanup
Quoted 17 lines · Kevin Hilman
Quoted 1 line · Wolfram Sang
On Wed, Sep 12, 2012 at 04:27:54PM +0530, Shubhrajyoti D wrote:
[...]
Quoted 11 lines
Quoted 10 lines
This is the cleanup only series.
Tested on omap4sdp and 3430sdp.
The following changes since commit
55d512e245bc7699a8800e23df1a24195dd08217:Linux 3.6-rc5 (2012-09-08 16:43:45 -0700)
are available in the git repository at:
git://gitorious.org/linus-tree/linus-tree.gitfor_3.7/i2c/big_cleanups
Quoted 8 lines
Quoted 1 line
Pushed to -next. Thanks to all involved!
Sorry to be late to the party (again), but still catching up after some time off.
Unfortunately, this series causes PM regressions on several OMAP platforms. I hope we can hold off on this until those issues are addressed.
I tracked the regression down to [PATCHv8 21/22] (see reply there.)
Since this series is already merged, I suggest that the problem patch be reverted, at least for v3.7 and until the problem is better understood and tested.
With that patch reverted, all my PM tests are passing. Feel free to add:
Kevin
v8 cover I2C big cleanup
Hi Kevin,
Quoted 3 lines
Since this series is already merged, I suggest that the problem patch be reverted, at least for v3.7 and until the problem is better understood and tested.
I thought I'll give you guys some more days to fix the problem before reverting.
Quoted 5 lines
With that patch reverted, all my PM tests are passing. Feel free to add:
Tested-by Kevin Hilman <[email protected]>
Thanks. That indeed increases confidence in this series. I won't add the tags to the tree, though, since that would mean rebasing.
All the best,
Wolfram
Signature · 9 lines
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/20120913/a775360b/attachment.sig>
v8 cover I2C big cleanup
In reply to Kevin HilmanQuoted 2 lines · Kevin Hilman
Kevin Hilman <[email protected]> writes:
[...]
Quoted 7 lines
Quoted 6 lines
Sorry to be late to the party (again), but still catching up after some time off.
Unfortunately, this series causes PM regressions on several OMAP platforms. I hope we can hold off on this until those issues are addressed.
Quoted 8 lines
I tracked the regression down to [PATCHv8 21/22] (see reply there.)
Since this series is already merged, I suggest that the problem patch be reverted, at least for v3.7 and until the problem is better understood and tested.
With that patch reverted, all my PM tests are passing. Feel free to add:
OK, the i2c series is off the hook.
Felipe and I spent a little time tracking this down. Felipe suggested that there might be a driver with periodic i2c activity keeping I2C awake, and thus preventing CORE retention. He was right.
On the boards where I'm seeing the failure, the RTC is firing every second, and since the RTC is on the I2C connected PMIC, the PMIC IRQ reads and the RTC reads are causing lots of I2C activity every second.
With the new autosuspend feature, that is enough to keep the I2C active continually and prevent CORE retention.
So, all that to say, from my PoV, this series can go in as is. The PM problem is caused by the RTC driver someplace.
Thanks Felipe,
Kevin
v8 cover I2C big cleanup
Hi,
Quoted 19 lines · Kevin Hilman
Quoted 2 lines · Kevin Hilman
Kevin Hilman <[email protected]> writes:
[...]
Quoted 13 lines
Quoted 6 lines
Sorry to be late to the party (again), but still catching up after some time off.
Unfortunately, this series causes PM regressions on several OMAP platforms. I hope we can hold off on this until those issues are addressed.
I tracked the regression down to [PATCHv8 21/22] (see reply there.)
Since this series is already merged, I suggest that the problem patch be reverted, at least for v3.7 and until the problem is better understood and tested.
Quoted 8 lines
Quoted 2 lines
With that patch reverted, all my PM tests are passing. Feel free to add:
OK, the i2c series is off the hook.
Felipe and I spent a little time tracking this down. Felipe suggested that there might be a driver with periodic i2c activity keeping I2C awake, and thus preventing CORE retention. He was right.
FYI, the original idea came from Shubhro. We agreed that would be the only way i2c would be prevented from idling.
Signature · 8 lines
balbi -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 836 bytes Desc: Digital signature URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20120913/df783ae7/attachment-0001.sig>
v8 cover I2C big cleanup
Quoted 25 lines · Felipe Balbi
Hi,
Quoted 22 lines · Kevin Hilman
Quoted 2 lines · Kevin Hilman
Kevin Hilman <[email protected]> writes:
[...]
Quoted 15 lines
Quoted 6 lines
Sorry to be late to the party (again), but still catching up after some time off.
Unfortunately, this series causes PM regressions on several OMAP platforms. I hope we can hold off on this until those issues are addressed.
I tracked the regression down to [PATCHv8 21/22] (see reply there.)
Since this series is already merged, I suggest that the problem patch be reverted, at least for v3.7 and until the problem is better understood and tested.
With that patch reverted, all my PM tests are passing. Feel free to add:
Quoted 8 lines
Quoted 5 lines
OK, the i2c series is off the hook.
Felipe and I spent a little time tracking this down. Felipe suggested that there might be a driver with periodic i2c activity keeping I2C awake, and thus preventing CORE retention. He was right.
FYI, the original idea came from Shubhro. We agreed that would be the only way i2c would be prevented from idling.
Great, thanks Shubhro!
Also, FYI, I just submitted a patch to the TWL RTC driver which was the source of all the I2C activity since it's on the I2C-connected PMIC.
Thanks for the help and suggestions,
Kevin
[1] https://groups.google.com/forum/#!topic/rtc-linux/sFbYmAzCRLQ
v8 cover I2C big cleanup
Quoted 34 lines · Kevin Hilman
Quoted 23 lines · Felipe Balbi
Hi,
Quoted 20 lines · Kevin Hilman
Quoted 2 lines · Kevin Hilman
Kevin Hilman <[email protected]> writes:
[...]
Quoted 14 lines
Quoted 6 lines
Sorry to be late to the party (again), but still catching up after some time off.
Unfortunately, this series causes PM regressions on several OMAP platforms. I hope we can hold off on this until those issues are addressed.
I tracked the regression down to [PATCHv8 21/22] (see reply there.)
Since this series is already merged, I suggest that the problem patch be reverted, at least for v3.7 and until the problem is better understood and tested.
With that patch reverted, all my PM tests are passing. Feel free to add:
Quoted 7 lines
Quoted 5 lines
OK, the i2c series is off the hook.
Felipe and I spent a little time tracking this down. Felipe suggested that there might be a driver with periodic i2c activity keeping I2C awake, and thus preventing CORE retention. He was right.
FYI, the original idea came from Shubhro. We agreed that would be the only way i2c would be prevented from idling.
Great, thanks Shubhro!
Quoted 8 lines
Also, FYI, I just submitted a patch to the TWL RTC driver which was the source of all the I2C activity since it's on the I2C-connected PMIC.
Thanks for the help and suggestions,
Kevin
[1] https://groups.google.com/forum/#!topic/rtc-linux/sFbYmAzCRLQ
Thanks for the testing and the patch.
v8 cover I2C big cleanup
In reply to Shubhrajyoti DShubhrajyoti D <[email protected]> writes:
[...]
Quoted 4 lines
This is the cleanup only series.
Tested on omap4sdp and 3430sdp.
It would be extremely helpful if you would describe how this was tested. And for me, it would be a source of extreme joy if you described any PM testing.
I gave this some additional PM testing on 3430/n900, 3530/Overo, 3730/OveroSTORM, 3730/Beagle-xM and 4430/Panda.
I tested v3.6-rc5 which passed all PM tests and then added this series (by merging the i2c-embedded/i2c-next branch.) PM tests then fail.
At least on 3530/Overo and 3730/Overo, CORE no longer hits retenion (or off) during idle.
The easy way to notice this is to see that even when no i2c transactions are happening, the runtime PM status for omap_i2c.1 is remains 'active':
# cat omap_i2c.*/power/runtime_status active suspended
Of course that means that clocks are never gated during idle, and CORE will never hit retention.
I noticed it because my PM tests detected that the CORE powerdomain was not transitioning to retention (or off) during idle.
To reproduce, simply enable UART timeouts so CORE will hit retention:
echo 3000 > /sys/devices/platform/omap_uart.0/power/autosuspend_delay_ms echo 3000 > /sys/devices/platform/omap_uart.1/power/autosuspend_delay_ms echo 3000 > /sys/devices/platform/omap_uart.2/power/autosuspend_delay_ms echo 3000 > /sys/devices/platform/omap_uart.3/power/autosuspend_delay_ms
and check the core_pwrm state counters:
cat /debug/pm_debug/count
wait > 3 seconds for the UART autosuspend timers to kick in. At this point, CORE should be transitioning to retenion in idle (determined by noticing that the RET counter for core_pwrdm is increasing.)
With $SUBJECT series applied, you'll notice that CORE is not transitioning.
Kevin
Quoted 37 lines
The following changes since commit
55d512e245bc7699a8800e23df1a24195dd08217:Linux 3.6-rc5 (2012-09-08 16:43:45 -0700)
are available in the git repository at:
git://gitorious.org/linus-tree/linus-tree.gitfor_3.7/i2c/big_cleanupsFelipe Balbi (22):
i2c: omap: switch to devm_* API
i2c: omap: simplifynum_byteshandling
i2c: omap: decrease indentation level on data handling
i2c: omap: add blank lines
i2c: omap: simplifyomap_i2c_ack_stat()
i2c: omap: split out [XR]DR and [XR]RDY
i2c: omap: improve i462 errata handling
i2c: omap: re-factor receive/transmit data loop
i2c: omap: switch over to do {} while loop
i2c: omap: ack IRQ in parts
i2c: omap: switch toplatform_get_irq()
i2c: omap: bus: add a receiver flag
i2c: omap: simplify errata check
i2c: omap: always returnIRQ_HANDLED
i2c: omap: simplify IRQ exit path
i2c: omap: resize fifos before each message
i2c: omap: get rid of the "complete" label
i2c: omap: always returnIRQ_HANDLED
i2c: omap: switch to threaded IRQ support
i2c: omap: remove unnecessarypm_runtime_suspendedcheck
i2c: omap: switch over to autosuspend API
i2c: omap: sanitize exit pathShubhrajyoti D (1):
i2c: omap: remove redundant status readdrivers/i2c/busses/i2c-omap.c | 442 +++++++++++++++++++++++++---------------- 1 files changed, 271 insertions(+), 171 deletions(-)
v8 cover I2C big cleanup
Quoted 42 lines · Kevin Hilman
Shubhrajyoti D <[email protected]> writes:
[...]
Quoted 4 lines
This is the cleanup only series.
Tested on omap4sdp and 3430sdp.
It would be extremely helpful if you would describe how this was tested. And for me, it would be a source of extreme joy if you described any PM testing.
I gave this some additional PM testing on 3430/n900, 3530/Overo, 3730/OveroSTORM, 3730/Beagle-xM and 4430/Panda.
I tested v3.6-rc5 which passed all PM tests and then added this series (by merging the i2c-embedded/i2c-next branch.) PM tests then fail.
At least on 3530/Overo and 3730/Overo, CORE no longer hits retenion (or off) during idle.
The easy way to notice this is to see that even when no i2c transactions are happening, the runtime PM status for omap_i2c.1 is remains 'active':
# cat omap_i2c.*/power/runtime_status active suspendedOf course that means that clocks are never gated during idle, and CORE will never hit retention.
I noticed it because my PM tests detected that the CORE powerdomain was not transitioning to retention (or off) during idle.
To reproduce, simply enable UART timeouts so CORE will hit retention:
echo 3000 >
/sys/devices/platform/omap_uart.0/power/autosuspend_delay_msecho 3000 >/sys/devices/platform/omap_uart.1/power/autosuspend_delay_msecho 3000 >/sys/devices/platform/omap_uart.2/power/autosuspend_delay_msecho 3000 >/sys/devices/platform/omap_uart.3/power/autosuspend_delay_ms
Quoted 7 lines
and check the
core_pwrmstate counters:cat /debug/pm_debug/count
wait > 3 seconds for the UART autosuspend timers to kick in. At this point, CORE should be transitioning to retenion in idle (determined by noticing that the RET counter for
core_pwrdmis increasing.)
I just ran same tests on pandaboard and i2c is suspended actually, though no power domain transitions to RET. Do we not have retention while idle on pandaboard ?
See below for a few results... First I ran this script to set everything up:
#!/bin/sh
echo "mounting proc"
mount -t proc none /proc
echo "mounting sysfs"
mount -t sysfs none /sys
echo "mounting debugfs"
mount -t debugfs none /debug
echo "UART autosuspend delay"
echo 3000 > /sys/devices/platform/omap_uart.0/power/autosuspend_delay_ms echo 3000 > /sys/devices/platform/omap_uart.1/power/autosuspend_delay_ms echo 3000 > /sys/devices/platform/omap_uart.2/power/autosuspend_delay_ms echo 3000 > /sys/devices/platform/omap_uart.3/power/autosuspend_delay_ms
echo "check omap_i2c.* status"
cat /sys/devices/platform/omap_i2c.*/power/runtime_status
echo "check CORE pm counters"
grep "^core_pwrdm" /debug/pm_debug/count
Then I triggered a few i2c transfers by reading regulator status through sysfs:
# cat /sys/class/regulator/regulator.*/status [ 507.469299] omap_i2c omap_i2c.1: omap_i2c_runtime_resume normal normal off off normal normal normal normal normal off # [ 509.010711] omap_i2c omap_i2c.1: omap_i2c_runtime_suspend cat /sys/devices/platform/omap_i2c.*/power/runtime_status suspended suspended suspended suspended
notice the prints I've added... I don't know what could go wrong only on Overo that i2c doesn't suspend. Can you add this debugging patch just for me to see if, at least, omap_i2c_runtime_suspend is being called ?
drivers/i2c/busses/i2c-omap.c +4 -0
@@ -1238,6 +1238,8 @@ static int omap_i2c_runtime_suspend(struct device *dev)struct omap_i2c_dev *_dev =platform_get_drvdata (pdev);u16 iv; +dev_info (dev,"%s\n" , __func__); + _dev->iestate =omap_i2c_read_reg (_dev,OMAP_I2C_IE_REG );omap_i2c_write_reg (_dev,OMAP_I2C_IE_REG ,0 );
@@ -1277,6 +1279,8 @@ static int omap_i2c_runtime_resume(struct device *dev)if (_dev->iestate)omap_i2c_write_reg (_dev,OMAP_I2C_IE_REG , _dev->iestate); +dev_info (dev,"%s\n" , __func__); + return 0; }#endif /* CONFIG_PM_RUNTIME */
this is on top of Wolfram's for-next branch, btw. Maybe you should also add:
CONFIG_I2C_DEBUG_CORE=yCONFIG_I2C_DEBUG_ALGO=yCONFIG_I2C_DEBUG_BUS=y
to your .config just in case.
cheers
Signature · 8 lines
balbi -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 836 bytes Desc: Digital signature URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20120913/c04d14f5/attachment-0001.sig>
v8 cover I2C big cleanup
Felipe Balbi <[email protected]> writes:
[...]
Quoted 3 lines
I just ran same tests on pandaboard and i2c is suspended actually, though no power domain transitions to RET. Do we not have retention while idle on pandaboard ?
Not for CORE. Only CPUx and MPU domains will be transitioning on OMAP4, and then, only with CPUidle enabled.
Until we have CORE retention support in mainline (we're almost there) OMAP4 is not a useful test platform for device PM.
Kevin
v8 cover I2C big cleanup
In reply to Kevin HilmanQuoted 42 lines · Kevin Hilman
Shubhrajyoti D <[email protected]> writes:
[...]
Quoted 3 lines
This is the cleanup only series.
Tested on omap4sdp and 3430sdp.
It would be extremely helpful if you would describe how this was tested. And for me, it would be a source of extreme joy if you described any PM testing.
I gave this some additional PM testing on 3430/n900, 3530/Overo, 3730/OveroSTORM, 3730/Beagle-xM and 4430/Panda.
I tested v3.6-rc5 which passed all PM tests and then added this series (by merging the i2c-embedded/i2c-next branch.) PM tests then fail.
At least on 3530/Overo and 3730/Overo, CORE no longer hits retenion (or off) during idle.
The easy way to notice this is to see that even when no i2c transactions are happening, the runtime PM status for omap_i2c.1 is remains 'active':
# cat omap_i2c.*/power/runtime_status active suspendedOf course that means that clocks are never gated during idle, and CORE will never hit retention.
I noticed it because my PM tests detected that the CORE powerdomain was not transitioning to retention (or off) during idle.
To reproduce, simply enable UART timeouts so CORE will hit retention:
echo 3000 >
/sys/devices/platform/omap_uart.0/power/autosuspend_delay_msecho 3000 >/sys/devices/platform/omap_uart.1/power/autosuspend_delay_msecho 3000 >/sys/devices/platform/omap_uart.2/power/autosuspend_delay_msecho 3000 >/sys/devices/platform/omap_uart.3/power/autosuspend_delay_msand check the
core_pwrmstate counters:
Quoted 8 lines
cat /debug/pm_debug/count
wait > 3 seconds for the UART autosuspend timers to kick in. At this point, CORE should be transitioning to retenion in idle (determined by noticing that the RET counter for
core_pwrdmis increasing.)With $SUBJECT series applied, you'll notice that CORE is not transitioning.
However I do not see the issue,let me know if I missed something see below.
On my 3430sdp
/sys/bus/platform/devices # cat omap_i2c.*/power/runtime_status
suspended
suspended
suspended/sys/bus/platform/devices #/sys/bus/platform/devices # cat omap_i2c.*/power/autosuspend_delay_ms
1000
1000
1000/sys/bus/platform/devices # echo 3000 >/sys/devices/platform/omap_uart.0/power/autosuspend_delay_ms/sys/bus/platform/devices # echo 3000 >/sys/devices/platform/omap_uart.1/power/autosuspend_delay_ms/sys/bus/platform/devices # echo 3000 >/sys/devices/platform/omap_uart.2/power/autosuspend_delay_ms/sys/bus/platform/devices #/sys/bus/platform/devices # cat /debug/pm_debug/countusbhost_pwrdm
(ON),OFF:0,RET:1373,INA:0,ON:1374,RET-LOGIC-OFF:0,RET-MEMBANK1-OFF:0sgx_pwrdm (OFF),OFF:1,RET:0,INA:0,ON:1,RET-LOGIC-OFF:0,RET-MEMBANK1-OFF:0core_pwrdm (ON),OFF:0,RET:1,INA:0,ON:2,RET-LOGIC-OFF:0,RET-MEMBANK1-OFF:0,RET-MEMBANK2-OFF:0 per_pwrdm (ON),OFF:0,RET:79,INA:0,ON:80,RET-LOGIC-OFF:0,RET-MEMBANK1-OFF:0dss_pwrdm
(ON),OFF:0,RET:1373,INA:0,ON:1374,RET-LOGIC-OFF:0,RET-MEMBANK1-OFF:0cam_pwrdm (RET),OFF:0,RET:1,INA:0,ON:1,RET-LOGIC-OFF:0,RET-MEMBANK1-OFF:0neon_pwrdm (ON),OFF:0,RET:1373,INA:0,ON:1374,RET-LOGIC-OFF:0mpu_pwrdm
(ON),OFF:0,RET:1373,INA:0,ON:1374,RET-LOGIC-OFF:0,RET-MEMBANK1-OFF:0iva2_pwrdm (RET),OFF:0,RET:1,INA:0,ON:1,RET-LOGIC-OFF:0,RET-MEMBANK1-OFF:0,RET-MEMBANK2-OFF:0,RET-MEMBANK3-OFF:0,RE0 usbhost_clkdm->usbhost_pwrdm (1)sgx_clkdm->sgx_pwrdm (0)per_clkdm->per_pwrdm (13)cam_clkdm->cam_pwrdm (0)dss_clkdm->dss_pwrdm (1)d2d_clkdm->core_pwrdm (0)iva2_clkdm->iva2_pwrdm (0)mpu_clkdm->mpu_pwrdm (0)core_l4_clkdm->core_pwrdm (23)core_l3_clkdm->core_pwrdm (4)neon_clkdm->neon_pwrdm (0)/sys/bus/platform/devices #/sys/bus/platform/devices #/sys/bus/platform/devices # sleep 5/sys/bus/platform/devices # cat /debug/pm_debug/countusbhost_pwrdm
(ON),OFF:0,RET:1512,INA:0,ON:1513,RET-LOGIC-OFF:0,RET-MEMBANK1-OFF:0sgx_pwrdm (OFF),OFF:1,RET:0,INA:0,ON:1,RET-LOGIC-OFF:0,RET-MEMBANK1-OFF:0core_pwrdm (ON),OFF:0,RET:12,INA:0,ON:13,RET-LOGIC-OFF:0,RET-MEMBANK1-OFF:0,RET-MEMBANK2-OFF:0 per_pwrdm (ON),OFF:0,RET:218,INA:0,ON:219,RET-LOGIC-OFF:0,RET-MEMBANK1-OFF:0dss_pwrdm
(ON),OFF:0,RET:1512,INA:0,ON:1513,RET-LOGIC-OFF:0,RET-MEMBANK1-OFF:0cam_pwrdm (RET),OFF:0,RET:1,INA:0,ON:1,RET-LOGIC-OFF:0,RET-MEMBANK1-OFF:0neon_pwrdm (ON),OFF:0,RET:1512,INA:0,ON:1513,RET-LOGIC-OFF:0mpu_pwrdm
(ON),OFF:0,RET:1512,INA:0,ON:1513,RET-LOGIC-OFF:0,RET-MEMBANK1-OFF:0iva2_pwrdm (RET),OFF:0,RET:1,INA:0,ON:1,RET-LOGIC-OFF:0,RET-MEMBANK1-OFF:0,RET-MEMBANK2-OFF:0,RET-MEMBANK3-OFF:0,RE0 usbhost_clkdm->usbhost_pwrdm (1)sgx_clkdm->sgx_pwrdm (0)per_clkdm->per_pwrdm (13)cam_clkdm->cam_pwrdm (0)dss_clkdm->dss_pwrdm (1)d2d_clkdm->core_pwrdm (0)iva2_clkdm->iva2_pwrdm (0)mpu_clkdm->mpu_pwrdm (0)core_l4_clkdm->core_pwrdm (23)core_l3_clkdm->core_pwrdm (4)neon_clkdm->neon_pwrdm (0)/sys/bus/platform/devices # cat omap_i2c.*/power/runtime_status
suspended
suspended
suspended/sys/bus/platform/devices #
Quoted 40 lines
Kevin
Quoted 37 lines
The following changes since commit
55d512e245bc7699a8800e23df1a24195dd08217:Linux 3.6-rc5 (2012-09-08 16:43:45 -0700)
are available in the git repository at:
git://gitorious.org/linus-tree/linus-tree.gitfor_3.7/i2c/big_cleanupsFelipe Balbi (22):
i2c: omap: switch to devm_* API
i2c: omap: simplifynum_byteshandling
i2c: omap: decrease indentation level on data handling
i2c: omap: add blank lines
i2c: omap: simplifyomap_i2c_ack_stat()
i2c: omap: split out [XR]DR and [XR]RDY
i2c: omap: improve i462 errata handling
i2c: omap: re-factor receive/transmit data loop
i2c: omap: switch over to do {} while loop
i2c: omap: ack IRQ in parts
i2c: omap: switch toplatform_get_irq()
i2c: omap: bus: add a receiver flag
i2c: omap: simplify errata check
i2c: omap: always returnIRQ_HANDLED
i2c: omap: simplify IRQ exit path
i2c: omap: resize fifos before each message
i2c: omap: get rid of the "complete" label
i2c: omap: always returnIRQ_HANDLED
i2c: omap: switch to threaded IRQ support
i2c: omap: remove unnecessarypm_runtime_suspendedcheck
i2c: omap: switch over to autosuspend API
i2c: omap: sanitize exit pathShubhrajyoti D (1):
i2c: omap: remove redundant status readdrivers/i2c/busses/i2c-omap.c | 442 +++++++++++++++++++++++++---------------- 1 files changed, 271 insertions(+), 171 deletions(-)
v8 cover I2C big cleanup
Hi,
Quoted 55 lines · Shubhrajyoti
Quoted 42 lines · Kevin Hilman
Shubhrajyoti D <[email protected]> writes:
[...]
Quoted 3 lines
This is the cleanup only series.
Tested on omap4sdp and 3430sdp.
It would be extremely helpful if you would describe how this was tested. And for me, it would be a source of extreme joy if you described any PM testing.
I gave this some additional PM testing on 3430/n900, 3530/Overo, 3730/OveroSTORM, 3730/Beagle-xM and 4430/Panda.
I tested v3.6-rc5 which passed all PM tests and then added this series (by merging the i2c-embedded/i2c-next branch.) PM tests then fail.
At least on 3530/Overo and 3730/Overo, CORE no longer hits retenion (or off) during idle.
The easy way to notice this is to see that even when no i2c transactions are happening, the runtime PM status for omap_i2c.1 is remains 'active':
# cat omap_i2c.*/power/runtime_status active suspendedOf course that means that clocks are never gated during idle, and CORE will never hit retention.
I noticed it because my PM tests detected that the CORE powerdomain was not transitioning to retention (or off) during idle.
To reproduce, simply enable UART timeouts so CORE will hit retention:
echo 3000 >
/sys/devices/platform/omap_uart.0/power/autosuspend_delay_msecho 3000 >/sys/devices/platform/omap_uart.1/power/autosuspend_delay_msecho 3000 >/sys/devices/platform/omap_uart.2/power/autosuspend_delay_msecho 3000 >/sys/devices/platform/omap_uart.3/power/autosuspend_delay_msand check the
core_pwrmstate counters:Quoted 8 lines
cat /debug/pm_debug/count
wait > 3 seconds for the UART autosuspend timers to kick in. At this point, CORE should be transitioning to retenion in idle (determined by noticing that the RET counter for
core_pwrdmis increasing.)With $SUBJECT series applied, you'll notice that CORE is not transitioning.
However I do not see the issue,let me know if I missed something see below.
On my 3430sdp
Quoted 9 lines
/sys/bus/platform/devices# cat omap_i2c.*/power/runtime_status suspended
suspended
suspended/sys/bus/platform/devices#/sys/bus/platform/devices# cat omap_i2c.*/power/autosuspend_delay_ms 1000
1000
1000
I just revived my very, very old 3503 overo and while i2c isn't active, I don't see transitions on core power domain:
# mount -t proc none /proc # mount -t sysfs none /sys # mount -t debugfs none /debug # echo 3000 > /sys/devices/platform/omap_uart.0/power/autosuspend_delay_ms # echo 3000 > /sys/devices/platform/omap_uart.1/power/autosuspend_delay_ms # echo 3000 > /sys/devices/platform/omap_uart.2/power/autosuspend_delay_ms # cat /sys/devices/platform/omap_i2c.*/power/runtime_status suspended suspended # grep ^core_pwrdm /debug/pm_debug/count && sleep 5 && grep ^core_pwrdm /debug/pm_debug/count core_pwrdm (ON),OFF:0,RET:0,INA:0,ON:1,RET-LOGIC-OFF:0,RET-MEMBANK1-OFF:0,RET-MEMBANK2-OFF:0 core_pwrdm (ON),OFF:0,RET:0,INA:0,ON:1,RET-LOGIC-OFF:0,RET-MEMBANK1-OFF:0,RET-MEMBANK2-OFF:0
Weird, but I don't think i2c is to blame here (unless I'm missing something). I can see that after i2c transfers, my suspend function is called and right after that i2c is suspended:
# cat /sys/class/regulator/regulator.*/status && sleep 1 && cat /sys/devices/platform/omap_i2c.*/power/runtime_status [ 411.072998] omap_i2c omap_i2c.1: omap_i2c_runtime_resume normal normal normal [ 412.075347] omap_i2c omap_i2c.1: omap_i2c_runtime_suspend suspended suspended
What am I missing to get any pm transitions to happen with my overo ?
# cat /debug/pm_debug/count && sleep 5 && cat /debug/pm_debug/count usbhost_pwrdm (ON),OFF:0,RET:0,INA:0,ON:1,RET-LOGIC-OFF:0,RET-MEMBANK1-OFF:0 sgx_pwrdm (OFF),OFF:1,RET:0,INA:0,ON:0,RET-LOGIC-OFF:0,RET-MEMBANK1-OFF:0 core_pwrdm (ON),OFF:0,RET:0,INA:0,ON:1,RET-LOGIC-OFF:0,RET-MEMBANK1-OFF:0,RET-MEMBANK2-OFF:0 per_pwrdm (ON),OFF:0,RET:0,INA:0,ON:1,RET-LOGIC-OFF:0,RET-MEMBANK1-OFF:0 dss_pwrdm (ON),OFF:0,RET:0,INA:0,ON:1,RET-LOGIC-OFF:0,RET-MEMBANK1-OFF:0 cam_pwrdm (RET),OFF:0,RET:1,INA:0,ON:1,RET-LOGIC-OFF:0,RET-MEMBANK1-OFF:0 neon_pwrdm (ON),OFF:0,RET:2379,INA:0,ON:2380,RET-LOGIC-OFF:0 mpu_pwrdm (ON),OFF:0,RET:2377,INA:0,ON:2378,RET-LOGIC-OFF:0,RET-MEMBANK1-OFF:0 iva2_pwrdm (ON),OFF:0,RET:0,INA:0,ON:1,RET-LOGIC-OFF:0,RET-MEMBANK1-OFF:0,RET-MEMBANK2-OFF:0,RET-MEMBANK3-OFF:0,RET-MEMBANK4-OFF:0 usbhost_clkdm->usbhost_pwrdm (1) sgx_clkdm->sgx_pwrdm (0) per_clkdm->per_pwrdm (19) cam_clkdm->cam_pwrdm (0) dss_clkdm->dss_pwrdm (1) d2d_clkdm->core_pwrdm (0) iva2_clkdm->iva2_pwrdm (0) mpu_clkdm->mpu_pwrdm (0) core_l4_clkdm->core_pwrdm (21) core_l3_clkdm->core_pwrdm (4) neon_clkdm->neon_pwrdm (0) usbhost_pwrdm (ON),OFF:0,RET:0,INA:0,ON:1,RET-LOGIC-OFF:0,RET-MEMBANK1-OFF:0 sgx_pwrdm (OFF),OFF:1,RET:0,INA:0,ON:0,RET-LOGIC-OFF:0,RET-MEMBANK1-OFF:0 core_pwrdm (ON),OFF:0,RET:0,INA:0,ON:1,RET-LOGIC-OFF:0,RET-MEMBANK1-OFF:0,RET-MEMBANK2-OFF:0 per_pwrdm (ON),OFF:0,RET:0,INA:0,ON:1,RET-LOGIC-OFF:0,RET-MEMBANK1-OFF:0 dss_pwrdm (ON),OFF:0,RET:0,INA:0,ON:1,RET-LOGIC-OFF:0,RET-MEMBANK1-OFF:0 cam_pwrdm (RET),OFF:0,RET:1,INA:0,ON:1,RET-LOGIC-OFF:0,RET-MEMBANK1-OFF:0 neon_pwrdm (ON),OFF:0,RET:2442,INA:0,ON:2443,RET-LOGIC-OFF:0 mpu_pwrdm (ON),OFF:0,RET:2440,INA:0,ON:2441,RET-LOGIC-OFF:0,RET-MEMBANK1-OFF:0 iva2_pwrdm (ON),OFF:0,RET:0,INA:0,ON:1,RET-LOGIC-OFF:0,RET-MEMBANK1-OFF:0,RET-MEMBANK2-OFF:0,RET-MEMBANK3-OFF:0,RET-MEMBANK4-OFF:0 usbhost_clkdm->usbhost_pwrdm (1) sgx_clkdm->sgx_pwrdm (0) per_clkdm->per_pwrdm (19) cam_clkdm->cam_pwrdm (0) dss_clkdm->dss_pwrdm (1) d2d_clkdm->core_pwrdm (0) iva2_clkdm->iva2_pwrdm (0) mpu_clkdm->mpu_pwrdm (0) core_l4_clkdm->core_pwrdm (21) core_l3_clkdm->core_pwrdm (4) neon_clkdm->neon_pwrdm (0)
Signature · 8 lines
balbi -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 836 bytes Desc: Digital signature URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20120913/b90d41b7/attachment-0001.sig>
v8 cover I2C big cleanup
In reply to ShubhrajyotiQuoted 2 lines · Shubhrajyoti
Quoted 2 lines
At least on 3530/Overo and 3730/Overo, CORE no longer hits retenion (or off) during idle.
I dont have an Overo, anyways will see if I can get one
However on beagleXm even off count increases
/ # grep "^core_pwrdm" /debug/pm_debug/countcore_pwrdm (ON),OFF:48,RET:856,INA:0,ON:905,RET-LOGIC-OFF:0,RET-MEMBANK1-OFF:0,RET-MEMBANK2-OFF:0 / # cat /sys/devices/platform/omap_i2c.*/power/runtime_status
suspended
suspended
/ #
/ #
/ # cat /sys/devices/platform/omap_i2c.*/power/autosuspend_delay_ms
1000
1000
/ #
/ #
/ # grep "^core_pwrdm" /debug/pm_debug/countcore_pwrdm (ON),OFF:218,RET:856,INA:0,ON:1075,RET-LOGIC-OFF:0,RET-MEMBANK1-OFF:0,RET-MEMBANK2-OFF:0 / #