I2C: OMAP: fix runtime PM get/put balance on error
Subsystems (4)
PATCH 1/1 I2C: OMAP: fix runtime PM get/put balance on error
ensure pm_runtime_put() is called, on pm_runtime_get_sync() failure.
Without this, after a failed call, the runtime PM usecount will have been incremented, but not decremented causing the usecount to never reach zero after a failure. Thanks to Kevin for educating about it. While at it also fix a missing pm_runtime_disable in the probe error path.
--- drivers/i2c/busses/i2c-omap.c | 7 +++---- 1 files changed, 3 insertions(+), 4 deletions(-)
drivers/i2c/busses/i2c-omap.c +3 -4
@@ -1113,10 +1113,10 @@ err_free_irq:free_irq (dev->irq, dev); err_unuse_clocks:omap_i2c_write_reg (dev,OMAP_I2C_CON_REG ,0 ); +err_free_mem: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:
@@ -1136,10 +1136,9 @@ static int __devexit omap_i2c_remove(struct platform_device *pdev)free_irq (dev->irq, dev);i2c_del_adapter (&dev->adapter); ret =pm_runtime_get_sync (&pdev->dev); -if (IS_ERR_VALUE (ret)) -return ret; +if (!IS_ERR_VALUE (ret)) +omap_i2c_write_reg (dev,OMAP_I2C_CON_REG ,0 ); -omap_i2c_write_reg (dev,OMAP_I2C_CON_REG ,0 );pm_runtime_put (&pdev->dev);pm_runtime_disable (&pdev->dev);iounmap (dev->base);
Signature · 1 line
1.7.5.4
PATCH 1/1 I2C: OMAP: fix runtime PM get/put balance on error
Quoted 8 lines · Shubhrajyoti D
ensure
pm_runtime_put()is called, onpm_runtime_get_sync()failure.Without this, after a failed call, the runtime PM usecount will have been incremented, but not decremented causing the usecount to never reach zero after a failure. Thanks to Kevin for educating about it. While at it also fix a missing
pm_runtime_disablein the probe error path.
This is the same subject and changelog as the patch I sent, but is a different patch.
Please write a new subject and a changelog specific to your patch.
As this changes the error/failure path, please be specific about how the failure modes were tested, and on which platforms.
Quoted 11 lines
Cc Kevin Hilman <[email protected]>Signed-off-by Shubhrajyoti D <[email protected]>--- drivers/i2c/busses/i2c-omap.c | 7 +++---- 1 files changed, 3 insertions(+), 4 deletions(-)drivers/i2c/busses/i2c-omap.c +0
-0
drivers/i2c/busses/i2c-omap.c err_free_irq: · line 1118
Quoted 6 lines
drivers/i2c/busses/i2c-omap.c +1
-0@@ -1113,10 +1113,10 @@ err_free_irq:free_irq (dev->irq, dev); err_unuse_clocks:omap_i2c_write_reg (dev,OMAP_I2C_CON_REG ,0 ); +err_free_mem:pm_runtime_put (dev->dev);iounmap (dev->base);
This doesn't look right. At least one of the gotos for this label, the ioremap has failed.
drivers/i2c/busses/i2c-omap.c static int __devexit omap_i2c_remove(struct platform_device *pdev) · line 1140
Quoted 13 lines
drivers/i2c/busses/i2c-omap.c +2
-3pm_runtime_disable (&pdev->dev); -err_free_mem:platform_set_drvdata (pdev,NULL );kfree (dev); err_release_region:@@ -1136,10 +1136,9 @@ static int __devexit omap_i2c_remove(struct platform_device *pdev)free_irq (dev->irq, dev);i2c_del_adapter (&dev->adapter); ret =pm_runtime_get_sync (&pdev->dev); -if (IS_ERR_VALUE (ret)) -return ret; +if (!IS_ERR_VALUE (ret)) +omap_i2c_write_reg (dev,OMAP_I2C_CON_REG ,0 );
this change isn't described in changelog
drivers/i2c/busses/i2c-omap.c static int __devexit omap_i2c_remove(struct platform_device *pdev) · line 1144
Quoted 4 lines
drivers/i2c/busses/i2c-omap.c +0
-1-omap_i2c_write_reg (dev,OMAP_I2C_CON_REG ,0 );pm_runtime_put (&pdev->dev);pm_runtime_disable (&pdev->dev);iounmap (dev->base);
Kevin
PATCH 1/1 I2C: OMAP: fix runtime PM get/put balance on error
Quoted 10 lines · Kevin Hilman
Quoted 8 lines · Shubhrajyoti D
ensure
pm_runtime_put()is called, onpm_runtime_get_sync()failure.Without this, after a failed call, the runtime PM usecount will have been incremented, but not decremented causing the usecount to never reach zero after a failure. Thanks to Kevin for educating about it. While at it also fix a missing
pm_runtime_disablein the probe error path.
Quoted 4 lines
This is the same subject and changelog as the patch I sent, but is a different patch.
Please write a new subject and a changelog specific to your patch.
OK.
Actually I did that on purpose your patch fixed the xfer call only. I thought that since get_sync increments the count always we could extend the patch to all the callers.
Quoted 3 lines
As this changes the error/failure path, please be specific about how the failure modes were tested, and on which platforms.
I found and fixed by review only.
Didnt really hit the failure case.
Quoted 7 lines
Quoted 7 lines
Cc Kevin Hilman <[email protected]>Signed-off-by Shubhrajyoti D <[email protected]>--- drivers/i2c/busses/i2c-omap.c | 7 +++---- 1 files changed, 3 insertions(+), 4 deletions(-)
Quoted 13 lines
drivers/i2c/busses/i2c-omap.c err_free_irq: · line 1118
Quoted 11 lines
drivers/i2c/busses/i2c-omap.c +1
-0@@ -1113,10 +1113,10 @@ err_free_irq:free_irq (dev->irq, dev); err_unuse_clocks:omap_i2c_write_reg (dev,OMAP_I2C_CON_REG ,0 ); +err_free_mem:pm_runtime_put (dev->dev);iounmap (dev->base);This doesn't look right. At least one of the gotos for this label, the ioremap has failed.
Yes , missed that thanks will fix it.
Quoted 15 lines
Quoted 7 lines
drivers/i2c/busses/i2c-omap.c +0
-1pm_runtime_disable (&pdev->dev); -err_free_mem:platform_set_drvdata (pdev,NULL );kfree (dev); err_release_region:drivers/i2c/busses/i2c-omap.c static int __devexit omap_i2c_remove(struct platform_device *pdev) · line 1140
Quoted 7 lines
drivers/i2c/busses/i2c-omap.c +2
-2@@ -1136,10 +1136,9 @@ static int __devexit omap_i2c_remove(struct platform_device *pdev)free_irq (dev->irq, dev);i2c_del_adapter (&dev->adapter); ret =pm_runtime_get_sync (&pdev->dev); -if (IS_ERR_VALUE (ret)) -return ret; +if (!IS_ERR_VALUE (ret)) +omap_i2c_write_reg (dev,OMAP_I2C_CON_REG ,0 );this change isn't described in changelog
Will do so.
Quoted 6 lines
drivers/i2c/busses/i2c-omap.c static int __devexit omap_i2c_remove(struct platform_device *pdev) · line 1144
Quoted 5 lines
drivers/i2c/busses/i2c-omap.c +0
-1-omap_i2c_write_reg (dev,OMAP_I2C_CON_REG ,0 );pm_runtime_put (&pdev->dev);pm_runtime_disable (&pdev->dev);iounmap (dev->base);Kevin
PATCH 1/1 I2C: OMAP: fix runtime PM get/put balance on error
Quoted 20 lines · Shubhrajyoti
Quoted 10 lines · Kevin Hilman
Quoted 8 lines · Shubhrajyoti D
ensure
pm_runtime_put()is called, onpm_runtime_get_sync()failure.Without this, after a failed call, the runtime PM usecount will have been incremented, but not decremented causing the usecount to never reach zero after a failure. Thanks to Kevin for educating about it. While at it also fix a missing
pm_runtime_disablein the probe error path.Quoted 4 lines
This is the same subject and changelog as the patch I sent, but is a different patch.
Please write a new subject and a changelog specific to your patch.
OK.
Actually I did that on purpose your patch fixed the xfer call only. I thought that sinceget_syncincrements the count always we could extend the patch to all the callers.Quoted 1 line
Quoted 4 lines
Quoted 2 lines
As this changes the error/failure path, please be specific about how the failure modes were tested, and on which platforms.
I found and fixed by review only.
Didnt really hit the failure case.
Which is why you should't just add stuff to other peoples work.
My patch was targetted at the XFER problem during suspend/resume which I found by testing and and proved the fix by testing.
Your patch adds several additional functional changes that were not well described and not tested. That calls for a separate patch with seprate subject/changelog etc.
Kevin