I2C big cleanup
Subsystems (4)
v7 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
Previous discussions can be found here
http://www.spinics.net/lists/linux-i2c/msg09482.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_cleanup
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 v7 1/23 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 v7 2/23 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 v7 3/23 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 v7 4/23 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 v7 5/23 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 v7 6/23 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 v7 7/23 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 v7 8/23 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 v7 9/23 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 v7 10/23 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 v7 11/23 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 v7 12/23 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 v7 13/23 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 v7 14/23 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 v7 15/23 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 v7 16/23 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 v7 17/23 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 v7 18/23 i2c: omap: remove redundant status read
In reply to Shubhrajyoti DRemove 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 v7 18/23 i2c: omap: remove redundant status read
Quoted 1 line · Shubhrajyoti D
Remove the redundant read of the status register.
Commit message is just repeating the subject. Why was the read there and why can it be removed now?
Quoted 29 lines
Signed-off-by Shubhrajyoti D <[email protected]>--- 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
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/20120911/ef823e8b/attachment-0001.sig>
PATCH v7 18/23 i2c: omap: remove redundant status read
Quoted 5 lines · Wolfram Sang
Quoted 1 line · Shubhrajyoti D
Remove the redundant read of the status register.
Commit message is just repeating the subject. Why was the read there and why can it be removed now?
It is read and it is not read anywhere below.
Quoted 28 lines
Quoted 28 lines
Signed-off-by Shubhrajyoti D <[email protected]>--- 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 v7 19/23 i2c: omap: always return IRQ_HANDLED
In reply to Shubhrajyoti DFrom: Felipe Balbi <[email protected]>
even if our clocks are disabled, we still
handled the IRQ, so we should return IRQ_HANDLED.
--- drivers/i2c/busses/i2c-omap.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
drivers/i2c/busses/i2c-omap.c +1 -1
@@ -862,7 +862,7 @@ omap_i2c_isr(int this_irq, void *dev_id)int err =0 , count =0 ;if (pm_runtime_suspended (dev->dev)) -return IRQ_NONE ; +return IRQ_HANDLED ; do { bits = omap_i2c_read_reg(dev, OMAP_I2C_IE_REG);
Signature · 1 line
1.7.5.4
PATCH v7 19/23 i2c: omap: always return IRQ_HANDLED
Quoted 7 lines · Shubhrajyoti D
From: Felipe Balbi <[email protected]>
even if our clocks are disabled, we still
handled the IRQ, so we should returnIRQ_HANDLED.Signed-off-by Felipe Balbi <[email protected]>Signed-off-by Shubhrajyoti D <[email protected]>
Can't we just drop this patch since the code gets removed soon anyhow? Or is patch 20/23 changing something I don't see yet?
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/20120911/aed0208f/attachment.sig>
PATCH v7 19/23 i2c: omap: always return IRQ_HANDLED
Quoted 3 lines · Wolfram Sang
Quoted 1 line
Signed-off-by Shubhrajyoti D <[email protected]>Can't we just drop this patch since the code gets removed soon anyhow? Or is patch 20/23 changing something I don't see yet?
yes it is dropped will update.
PATCH v7 20/23 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_HANDLED ; +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 v7 20/23 i2c: omap: switch to threaded IRQ support
Quoted 26 lines · Shubhrajyoti D
From: 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.Signed-off-by Felipe Balbi <[email protected]>[Trivial formating changes]Signed-off-by Shubhrajyoti D <[email protected]>--- drivers/i2c/busses/i2c-omap.c | 43 +++++++++++++++++++++++++++++++++++----- 1 files changed, 37 insertions(+), 6 deletions(-)drivers/i2c/busses/i2c-omap.c +1
-0@@ -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;
drivers/i2c/busses/i2c-omap.c static int omap_i2c_transmit_data(struct omap_i2c_dev *dev, u8 num_bytes, · line 861
Quoted 8 lines
drivers/i2c/busses/i2c-omap.c +2
-1@@ -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 ;
Shouldn't that be IRQ_NONE?
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/20120911/1a30fec3/attachment.sig>
PATCH v7 20/23 i2c: omap: switch to threaded IRQ support
Quoted 6 lines · Wolfram Sang
Quoted 5 lines
-omap_i2c_isr(int
this_irq, void*dev_id)drivers/i2c/busses/i2c-omap.c static int omap_i2c_transmit_data(struct omap_i2c_dev *dev, u8 num_bytes, · line 861
Quoted 4 lines
drivers/i2c/busses/i2c-omap.c +2
-0+omap_i2c_isr(int irq, void *dev_id) { struct omap_i2c_dev *dev = dev_id; +irqreturn_t ret =IRQ_HANDLED ;Shouldn't that be
IRQ_NONE?
Actually we are processing it so I thought it to be ok. Also a similar discussion.
http://lists.infradead.org/pipermail/linux-arm-kernel/2012-June/104422.html
PATCH v7 20/23 i2c: omap: switch to threaded IRQ support
Quoted 8 lines · Shubhrajyoti
Quoted 6 lines · Wolfram Sang
Quoted 5 lines
-omap_i2c_isr(int
this_irq, void*dev_id)drivers/i2c/busses/i2c-omap.c static int omap_i2c_transmit_data(struct omap_i2c_dev *dev, u8 num_bytes, · line 861
Quoted 4 lines
drivers/i2c/busses/i2c-omap.c +2
-0+omap_i2c_isr(int irq, void *dev_id) { struct omap_i2c_dev *dev = dev_id; +irqreturn_t ret =IRQ_HANDLED ;Shouldn't that be
IRQ_NONE?Actually we are processing it so I thought it to be ok.
Are we processing it? There is nothing acknowledged AFAICS. Anyway, we can fix that with a later patch.
Quoted 3 lines
Also a similar discussion.
http://lists.infradead.org/pipermail/linux-arm-kernel/2012-June/104422.html
I totally agree to the things said there.
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/12939203/attachment-0001.sig>
PATCH v7 21/23 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_HANDLED ; -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 v7 22/23 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 v7 23/23 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
v7 cover I2C big cleanup
In reply to Shubhrajyoti DQuoted 21 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
Quoted 2 lines
Previous discussions can be found here
http://www.spinics.net/lists/linux-i2c/msg09482.html
Also this gives better performance
With the patches:
Performance counter stats for '/build/i2c/bin/i2cdump -y -f 1 0x48
b':
78.796376 task-clock # 0.453 CPUs
utilized
516 context-switches # 0.007
M/sec
0 CPU-migrations # 0.000
K/sec
114 page-faults # 0.001 M/sec
0.174011183 seconds time elapsed
Without the patches
Performance counter stats for '/build/i2c/bin/i2cdump -y -f 1 0x48
b':
123.504640 task-clock # 0.049 CPUs
utilized
337 context-switches # 0.003
M/sec
0 CPU-migrations # 0.000
K/sec
144 page-faults # 0.001
M/sec
2.534424040 seconds time elapsed
The auto suspend is probably the one :-)
Quoted 43 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_cleanupFelipe 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(-)
v7 cover I2C big cleanup
Hi,
Quoted 57 lines · Shubhrajyoti
Quoted 21 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 updateQuoted 2 lines
Previous discussions can be found here
http://www.spinics.net/lists/linux-i2c/msg09482.htmlAlso this gives better performance
With the patches:Performance counter stats for '/build/i2c/bin/i2cdump -y -f 1
0x48
b':78.796376 task-clock # 0.453 CPUsutilized
516 context-switches # 0.007
M/sec
0 CPU-migrations # 0.000
K/sec
114 page-faults # 0.001 M/sec0.174011183 seconds time elapsed
Without the patches
Performance counter stats for '/build/i2c/bin/i2cdump -y -f 10x48
b':123.504640 task-clock # 0.049 CPUsutilized
337 context-switches # 0.003
M/sec
0 CPU-migrations # 0.000
K/sec
144 page-faults # 0.001
M/sec
Quoted 3 lines
2.534424040 seconds time elapsedThe auto suspend is probably the one :-)
Awesome, I didn't go as far as checking performance counters :-)
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/20120911/796b2d06/attachment-0001.sig>
v7 cover I2C big cleanup
In reply to Shubhrajyoti DQuoted 12 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:
Quoted 8 lines
- 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
Looks mostly good, thanks. Only a few comments. Oh, and I still get reports about a section mismatch ;)
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/b9d6e8b1/attachment.sig>
v7 cover I2C big cleanup
Quoted 5 lines · Wolfram Sang
Quoted 3 lines
Changes since v6:
Quoted 2 lines
- Fix comments on setting the pdev to NULL.
- Trivial changelog updateLooks mostly good, thanks. Only a few comments. Oh, and I still get reports about a section mismatch ;)
I am using omap2plus_defconfig
$ make --ver GNU Make 3.81 Copyright (C) 2006 Free Software Foundation, Inc. This is free software; see the source for copying conditions. There is NO warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
$ make CONFIG_DEBUG_SECTION_MISMATCH=y uImage .... LD vmlinux.o MODPOST vmlinux.o GEN .version CHK include/generated/compile.h UPD include/generated/compile.h CC init/version.o LD init/built-in.o KSYM .tmp_kallsyms1.o KSYM .tmp_kallsyms2.o LD vmlinux SYSMAP System.map OBJCOPY arch/arm/boot/Image Kernel: arch/arm/boot/Image is ready GZIP arch/arm/boot/compressed/piggy.gzip CC arch/arm/boot/compressed/misc.o CC arch/arm/boot/compressed/decompress.o CC arch/arm/boot/compressed/string.o AS arch/arm/boot/compressed/piggy.gzip.o LD arch/arm/boot/compressed/vmlinux OBJCOPY arch/arm/boot/zImage Kernel: arch/arm/boot/zImage is ready UIMAGE arch/arm/boot/uImage Image Name: Linux-3.6.0-rc5-00022-g49fb6db Created: Wed Sep 12 15:24:21 2012 Image Type: ARM Linux Kernel Image (uncompressed) Data Size: 4074688 Bytes = 3979.19 kB = 3.89 MB Load Address: 80008000 Entry Point: 80008000 Image arch/arm/boot/uImage is ready
I donot see the warning. Am I missing something?
v7 cover I2C big cleanup
Quoted 2 lines
I donot see the warning. Am I missing something?
I deleted my logfiles already. Ignore it for now, if it comes up again with your new series, I will give a more detailed pointer.
Thanks,
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/20120912/fa1222fb/attachment.sig>
v7 cover I2C big cleanup
Quoted 5 lines · Wolfram Sang
Quoted 2 lines
I donot see the warning. Am I missing something?
I deleted my logfiles already. Ignore it for now, if it comes up again with your new series, I will give a more detailed pointer.
Sorry, the section mismatch was not related to I2C it seems:
WARNING: vmlinux.o(.data+0x30958): Section mismatch in reference from the variable rx51_si4713_dev to the (unknown reference) .init.data:(unknown) The variable rx51_si4713_dev references
the (unknown reference) __initdata (unknown)
If the reference is valid then annotate the
variable with __init* or __refdata (see linux/init.h) or name the variable:*_template, *_timer, *_sht, *_ops, *_probe, *_probe_one, *_console
Got it with an "allnoconfig" and then selecting MMU and OMAP.
Regards,
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/20120912/bb3a742e/attachment.sig>
v7 cover I2C big cleanup
Quoted 8 lines · Wolfram Sang
Quoted 5 lines · Wolfram Sang
Quoted 2 lines
I donot see the warning. Am I missing something?
I deleted my logfiles already. Ignore it for now, if it comes up again with your new series, I will give a more detailed pointer.
Sorry, the section mismatch was not related to I2C it seems:
Thanks for the report just sent a patch fixing that.
Quoted 26 lines
WARNING:
vmlinux.o(.data+0x30958): Section mismatch in reference from the variablerx51_si4713_devto the (unknown reference) .init.data:(unknown) The variablerx51_si4713_devreferences
the (unknown reference)__initdata(unknown)
If the reference is valid then annotate the
variable with __init* or__refdata(seelinux/init.h) or name the variable:*_template,*_timer,*_sht,*_ops,*_probe,*_probe_one,*_consoleGot it with an "allnoconfig" and then selecting MMU and OMAP.
Regards,
Wolfram
--
Pengutronix e.K. | Wolfram Sang | Industrial Linux Solutions | http://www.pengutronix.de/ |-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (GNU/Linux)iEYEARECAAYFAlBQkbsACgkQD27XaX1/VRtbuACgkBa0lOIN551eec9TSetVPsCE Ew0AoKizKon3DIILpERWJIwzAXdgRVDc
=T4Yq
-----END PGP SIGNATURE-----