RE: [Patch V5] i2c: imx: add runtime pm support to improve the performance
From: Heiner Kallweit mailto:hkallwe...@gmail.com Sent: Friday, August 28, 2015 1:50 PM To: Gao Pan-B54642; w...@the-dreams.de Cc: linux-i2c@vger.kernel.org; Li Frank-B20596; Duan Fugang-B38611; u.kleine-koe...@pengutronix.de; ker...@pengutronix.de Subject: Re: [Patch V5] i2c: imx: add runtime pm support to improve the performance Am 27.08.2015 um 09:06 schrieb Gao Pandy: From: Heiner Kallweit mailto:hkallwe...@gmail.com Sent: Thursday, August 27, 2015 1:50 PM To: Gao Pan-B54642; w...@the-dreams.de Cc: linux-i2c@vger.kernel.org; Li Frank-B20596; Duan Fugang-B38611; u.kleine-koe...@pengutronix.de; ker...@pengutronix.de Subject: Re: [Patch V5] i2c: imx: add runtime pm support to improve the performance Am 27.08.2015 um 06:01 schrieb Gao Pandy: From: Heiner Kallweit mailto:hkallwe...@gmail.com Sent: Thursday, August 27, 2015 2:52 AM To: Gao Pan-B54642; w...@the-dreams.de Cc: linux-i2c@vger.kernel.org; Li Frank-B20596; Duan Fugang-B38611; u.kleine-koe...@pengutronix.de; ker...@pengutronix.de Subject: Re: [Patch V5] i2c: imx: add runtime pm support to improve the performance Am 26.08.2015 um 07:23 schrieb Gao Pan: In our former i2c driver, i2c clk is enabled and disabled in xfer function, which contributes to power saving. However, the clk enable process brings a busy wait delay until the core is stable. As a result, the performance is sacrificed. To weigh the power consumption and i2c bus performance, runtime pm is the good solution for it. The clk is enabled when a i2c transfer starts, and disabled after a specifically defined delay. Without the patch the test case (many eeprom reads) executes with approx: real 1m7.735s user 0m0.488s sys 0m20.040s With the patch the same test case (many eeprom reads) executes with approx: real 0m54.241s user 0m0.440s sys 0m5.920s Signed-off-by: Fugang Duan b38...@freescale.com Signed-off-by: Gao Pan b54...@freescale.com [wsa: fixed some indentation] Signed-off-by: Wolfram Sang w...@the-dreams.de --- V2: As Uwe Kleine-König's suggestion, the version do below changes: -call clk_prepare_enable in probe to avoid never enabling clock if CONFIG_PM is disabled -enable clock before request IRQ in probe -remove the pm staff in i2c_imx_isr V3: -pm_runtime_get_sync returns 0 as error V4: -add pm_runtime_set_active before pm_runtime_enable -replace pm_runtime_put_autosuspend with pm_runtime_autosuspend in probe -add error disposal when i2c_add_numbered_adapter fails V5: -clean up and disable runtime PM when i2c_add_numbered_adapter fails -use pm_runtime_get and pm_runtime_put_autosuspend in probe drivers/i2c/busses/i2c-imx.c | 91 1 file changed, 76 insertions(+), 15 deletions(-) diff --git a/drivers/i2c/busses/i2c-imx.c b/drivers/i2c/busses/i2c-imx.c index 785aa67..8940a89 100644 --- a/drivers/i2c/busses/i2c-imx.c +++ b/drivers/i2c/busses/i2c-imx.c @@ -53,6 +53,7 @@ #include linux/platform_device.h #include linux/sched.h #include linux/slab.h +#include linux/pm_runtime.h /** Defines ** ** ** ** ** */ @@ -118,6 +119,8 @@ #define I2CR_IEN_OPCODE_0 0x0 #define I2CR_IEN_OPCODE_1 I2CR_IEN +#define I2C_PM_TIMEOUT 10 /* ms */ + /** Variables ** ** ** ** */ @@ -520,9 +523,6 @@ static int i2c_imx_start(struct imx_i2c_struct *i2c_imx) i2c_imx_set_clk(i2c_imx); - result = clk_prepare_enable(i2c_imx-clk); - if (result) - return result; imx_i2c_write_reg(i2c_imx-ifdr, i2c_imx, IMX_I2C_IFDR); /* Enable I2C controller */ imx_i2c_write_reg(i2c_imx-hwdata-i2sr_clr_opcode, i2c_imx, IMX_I2C_I2SR); @@ -575,7 +575,6 @@ static void i2c_imx_stop(struct imx_i2c_struct *i2c_imx) /* Disable I2C controller */ temp = i2c_imx-hwdata-i2cr_ien_opcode ^ I2CR_IEN, imx_i2c_write_reg(temp, i2c_imx, IMX_I2C_I2CR); - clk_disable_unprepare(i2c_imx-clk); } static irqreturn_t i2c_imx_isr(int irq, void *dev_id) @@ -894,6 +893,10 @@ static int i2c_imx_xfer(struct i2c_adapter *adapter, dev_dbg(i2c_imx-adapter.dev, %s\n, __func__); + result = pm_runtime_get_sync(i2c_imx-adapter.dev.parent); + if (result 0) + goto out; + /* Start I2C transfer */ result = i2c_imx_start(i2c_imx); if (result) @@ -950,6 +953,10 @@ fail0: /* Stop I2C transfer */ i2c_imx_stop(i2c_imx); +out: + pm_runtime_mark_last_busy(i2c_imx-adapter.dev.parent
Re: [Patch V5] i2c: imx: add runtime pm support to improve the performance
Am 27.08.2015 um 09:06 schrieb Gao Pandy: From: Heiner Kallweit mailto:hkallwe...@gmail.com Sent: Thursday, August 27, 2015 1:50 PM To: Gao Pan-B54642; w...@the-dreams.de Cc: linux-i2c@vger.kernel.org; Li Frank-B20596; Duan Fugang-B38611; u.kleine-koe...@pengutronix.de; ker...@pengutronix.de Subject: Re: [Patch V5] i2c: imx: add runtime pm support to improve the performance Am 27.08.2015 um 06:01 schrieb Gao Pandy: From: Heiner Kallweit mailto:hkallwe...@gmail.com Sent: Thursday, August 27, 2015 2:52 AM To: Gao Pan-B54642; w...@the-dreams.de Cc: linux-i2c@vger.kernel.org; Li Frank-B20596; Duan Fugang-B38611; u.kleine-koe...@pengutronix.de; ker...@pengutronix.de Subject: Re: [Patch V5] i2c: imx: add runtime pm support to improve the performance Am 26.08.2015 um 07:23 schrieb Gao Pan: In our former i2c driver, i2c clk is enabled and disabled in xfer function, which contributes to power saving. However, the clk enable process brings a busy wait delay until the core is stable. As a result, the performance is sacrificed. To weigh the power consumption and i2c bus performance, runtime pm is the good solution for it. The clk is enabled when a i2c transfer starts, and disabled after a specifically defined delay. Without the patch the test case (many eeprom reads) executes with approx: real 1m7.735s user 0m0.488s sys 0m20.040s With the patch the same test case (many eeprom reads) executes with approx: real 0m54.241s user 0m0.440s sys 0m5.920s Signed-off-by: Fugang Duan b38...@freescale.com Signed-off-by: Gao Pan b54...@freescale.com [wsa: fixed some indentation] Signed-off-by: Wolfram Sang w...@the-dreams.de --- V2: As Uwe Kleine-König's suggestion, the version do below changes: -call clk_prepare_enable in probe to avoid never enabling clock if CONFIG_PM is disabled -enable clock before request IRQ in probe -remove the pm staff in i2c_imx_isr V3: -pm_runtime_get_sync returns 0 as error V4: -add pm_runtime_set_active before pm_runtime_enable -replace pm_runtime_put_autosuspend with pm_runtime_autosuspend in probe -add error disposal when i2c_add_numbered_adapter fails V5: -clean up and disable runtime PM when i2c_add_numbered_adapter fails -use pm_runtime_get and pm_runtime_put_autosuspend in probe drivers/i2c/busses/i2c-imx.c | 91 1 file changed, 76 insertions(+), 15 deletions(-) diff --git a/drivers/i2c/busses/i2c-imx.c b/drivers/i2c/busses/i2c-imx.c index 785aa67..8940a89 100644 --- a/drivers/i2c/busses/i2c-imx.c +++ b/drivers/i2c/busses/i2c-imx.c @@ -53,6 +53,7 @@ #include linux/platform_device.h #include linux/sched.h #include linux/slab.h +#include linux/pm_runtime.h /** Defines ** */ @@ -118,6 +119,8 @@ #define I2CR_IEN_OPCODE_00x0 #define I2CR_IEN_OPCODE_1I2CR_IEN +#define I2C_PM_TIMEOUT 10 /* ms */ + /** Variables ** ** */ @@ -520,9 +523,6 @@ static int i2c_imx_start(struct imx_i2c_struct *i2c_imx) i2c_imx_set_clk(i2c_imx); - result = clk_prepare_enable(i2c_imx-clk); - if (result) - return result; imx_i2c_write_reg(i2c_imx-ifdr, i2c_imx, IMX_I2C_IFDR); /* Enable I2C controller */ imx_i2c_write_reg(i2c_imx-hwdata-i2sr_clr_opcode, i2c_imx, IMX_I2C_I2SR); @@ -575,7 +575,6 @@ static void i2c_imx_stop(struct imx_i2c_struct *i2c_imx) /* Disable I2C controller */ temp = i2c_imx-hwdata-i2cr_ien_opcode ^ I2CR_IEN, imx_i2c_write_reg(temp, i2c_imx, IMX_I2C_I2CR); - clk_disable_unprepare(i2c_imx-clk); } static irqreturn_t i2c_imx_isr(int irq, void *dev_id) @@ -894,6 +893,10 @@ static int i2c_imx_xfer(struct i2c_adapter *adapter, dev_dbg(i2c_imx-adapter.dev, %s\n, __func__); + result = pm_runtime_get_sync(i2c_imx-adapter.dev.parent); + if (result 0) + goto out; + /* Start I2C transfer */ result = i2c_imx_start(i2c_imx); if (result) @@ -950,6 +953,10 @@ fail0: /* Stop I2C transfer */ i2c_imx_stop(i2c_imx); +out: + pm_runtime_mark_last_busy(i2c_imx-adapter.dev.parent); + pm_runtime_put_autosuspend(i2c_imx-adapter.dev.parent); + dev_dbg(i2c_imx-adapter.dev, %s exit with: %s: %d\n, __func__, (result 0) ? error : success msg, (result 0) ? result : num); @@ -1020,15 +1027,17 @@ static int i2c_imx_probe(struct platform_device *pdev) ret = clk_prepare_enable(i2c_imx-clk); if (ret) { - dev_err(pdev-dev, can't enable I2C clock\n); + dev_err(pdev-dev, can't enable I2C clock, ret=%d\n, ret); return ret; } + /* Request IRQ */ ret = devm_request_irq(pdev-dev, irq, i2c_imx_isr, 0
RE: [Patch V5] i2c: imx: add runtime pm support to improve the performance
From: Heiner Kallweit mailto:hkallwe...@gmail.com Sent: Thursday, August 27, 2015 1:50 PM To: Gao Pan-B54642; w...@the-dreams.de Cc: linux-i2c@vger.kernel.org; Li Frank-B20596; Duan Fugang-B38611; u.kleine-koe...@pengutronix.de; ker...@pengutronix.de Subject: Re: [Patch V5] i2c: imx: add runtime pm support to improve the performance Am 27.08.2015 um 06:01 schrieb Gao Pandy: From: Heiner Kallweit mailto:hkallwe...@gmail.com Sent: Thursday, August 27, 2015 2:52 AM To: Gao Pan-B54642; w...@the-dreams.de Cc: linux-i2c@vger.kernel.org; Li Frank-B20596; Duan Fugang-B38611; u.kleine-koe...@pengutronix.de; ker...@pengutronix.de Subject: Re: [Patch V5] i2c: imx: add runtime pm support to improve the performance Am 26.08.2015 um 07:23 schrieb Gao Pan: In our former i2c driver, i2c clk is enabled and disabled in xfer function, which contributes to power saving. However, the clk enable process brings a busy wait delay until the core is stable. As a result, the performance is sacrificed. To weigh the power consumption and i2c bus performance, runtime pm is the good solution for it. The clk is enabled when a i2c transfer starts, and disabled after a specifically defined delay. Without the patch the test case (many eeprom reads) executes with approx: real 1m7.735s user 0m0.488s sys 0m20.040s With the patch the same test case (many eeprom reads) executes with approx: real 0m54.241s user 0m0.440s sys 0m5.920s Signed-off-by: Fugang Duan b38...@freescale.com Signed-off-by: Gao Pan b54...@freescale.com [wsa: fixed some indentation] Signed-off-by: Wolfram Sang w...@the-dreams.de --- V2: As Uwe Kleine-König's suggestion, the version do below changes: -call clk_prepare_enable in probe to avoid never enabling clock if CONFIG_PM is disabled -enable clock before request IRQ in probe -remove the pm staff in i2c_imx_isr V3: -pm_runtime_get_sync returns 0 as error V4: -add pm_runtime_set_active before pm_runtime_enable -replace pm_runtime_put_autosuspend with pm_runtime_autosuspend in probe -add error disposal when i2c_add_numbered_adapter fails V5: -clean up and disable runtime PM when i2c_add_numbered_adapter fails -use pm_runtime_get and pm_runtime_put_autosuspend in probe drivers/i2c/busses/i2c-imx.c | 91 1 file changed, 76 insertions(+), 15 deletions(-) diff --git a/drivers/i2c/busses/i2c-imx.c b/drivers/i2c/busses/i2c-imx.c index 785aa67..8940a89 100644 --- a/drivers/i2c/busses/i2c-imx.c +++ b/drivers/i2c/busses/i2c-imx.c @@ -53,6 +53,7 @@ #include linux/platform_device.h #include linux/sched.h #include linux/slab.h +#include linux/pm_runtime.h /** Defines ** */ @@ -118,6 +119,8 @@ #define I2CR_IEN_OPCODE_00x0 #define I2CR_IEN_OPCODE_1I2CR_IEN +#define I2C_PM_TIMEOUT 10 /* ms */ + /** Variables ** ** */ @@ -520,9 +523,6 @@ static int i2c_imx_start(struct imx_i2c_struct *i2c_imx) i2c_imx_set_clk(i2c_imx); - result = clk_prepare_enable(i2c_imx-clk); - if (result) - return result; imx_i2c_write_reg(i2c_imx-ifdr, i2c_imx, IMX_I2C_IFDR); /* Enable I2C controller */ imx_i2c_write_reg(i2c_imx-hwdata-i2sr_clr_opcode, i2c_imx, IMX_I2C_I2SR); @@ -575,7 +575,6 @@ static void i2c_imx_stop(struct imx_i2c_struct *i2c_imx) /* Disable I2C controller */ temp = i2c_imx-hwdata-i2cr_ien_opcode ^ I2CR_IEN, imx_i2c_write_reg(temp, i2c_imx, IMX_I2C_I2CR); - clk_disable_unprepare(i2c_imx-clk); } static irqreturn_t i2c_imx_isr(int irq, void *dev_id) @@ -894,6 +893,10 @@ static int i2c_imx_xfer(struct i2c_adapter *adapter, dev_dbg(i2c_imx-adapter.dev, %s\n, __func__); + result = pm_runtime_get_sync(i2c_imx-adapter.dev.parent); + if (result 0) + goto out; + /* Start I2C transfer */ result = i2c_imx_start(i2c_imx); if (result) @@ -950,6 +953,10 @@ fail0: /* Stop I2C transfer */ i2c_imx_stop(i2c_imx); +out: + pm_runtime_mark_last_busy(i2c_imx-adapter.dev.parent); + pm_runtime_put_autosuspend(i2c_imx-adapter.dev.parent); + dev_dbg(i2c_imx-adapter.dev, %s exit with: %s: %d\n, __func__, (result 0) ? error : success msg, (result 0) ? result : num); @@ -1020,15 +1027,17 @@ static int i2c_imx_probe(struct platform_device *pdev) ret = clk_prepare_enable(i2c_imx-clk); if (ret) { - dev_err(pdev-dev, can't enable I2C clock\n); + dev_err(pdev-dev, can't enable I2C clock, ret=%d\n, ret); return ret
Re: [Patch V5] i2c: imx: add runtime pm support to improve the performance
Am 26.08.2015 um 07:23 schrieb Gao Pan: In our former i2c driver, i2c clk is enabled and disabled in xfer function, which contributes to power saving. However, the clk enable process brings a busy wait delay until the core is stable. As a result, the performance is sacrificed. To weigh the power consumption and i2c bus performance, runtime pm is the good solution for it. The clk is enabled when a i2c transfer starts, and disabled after a specifically defined delay. Without the patch the test case (many eeprom reads) executes with approx: real 1m7.735s user 0m0.488s sys 0m20.040s With the patch the same test case (many eeprom reads) executes with approx: real 0m54.241s user 0m0.440s sys 0m5.920s Signed-off-by: Fugang Duan b38...@freescale.com Signed-off-by: Gao Pan b54...@freescale.com [wsa: fixed some indentation] Signed-off-by: Wolfram Sang w...@the-dreams.de --- V2: As Uwe Kleine-König's suggestion, the version do below changes: -call clk_prepare_enable in probe to avoid never enabling clock if CONFIG_PM is disabled -enable clock before request IRQ in probe -remove the pm staff in i2c_imx_isr V3: -pm_runtime_get_sync returns 0 as error V4: -add pm_runtime_set_active before pm_runtime_enable -replace pm_runtime_put_autosuspend with pm_runtime_autosuspend in probe -add error disposal when i2c_add_numbered_adapter fails V5: -clean up and disable runtime PM when i2c_add_numbered_adapter fails -use pm_runtime_get and pm_runtime_put_autosuspend in probe drivers/i2c/busses/i2c-imx.c | 91 1 file changed, 76 insertions(+), 15 deletions(-) diff --git a/drivers/i2c/busses/i2c-imx.c b/drivers/i2c/busses/i2c-imx.c index 785aa67..8940a89 100644 --- a/drivers/i2c/busses/i2c-imx.c +++ b/drivers/i2c/busses/i2c-imx.c @@ -53,6 +53,7 @@ #include linux/platform_device.h #include linux/sched.h #include linux/slab.h +#include linux/pm_runtime.h /** Defines ***/ @@ -118,6 +119,8 @@ #define I2CR_IEN_OPCODE_00x0 #define I2CR_IEN_OPCODE_1I2CR_IEN +#define I2C_PM_TIMEOUT 10 /* ms */ + /** Variables ** ***/ @@ -520,9 +523,6 @@ static int i2c_imx_start(struct imx_i2c_struct *i2c_imx) i2c_imx_set_clk(i2c_imx); - result = clk_prepare_enable(i2c_imx-clk); - if (result) - return result; imx_i2c_write_reg(i2c_imx-ifdr, i2c_imx, IMX_I2C_IFDR); /* Enable I2C controller */ imx_i2c_write_reg(i2c_imx-hwdata-i2sr_clr_opcode, i2c_imx, IMX_I2C_I2SR); @@ -575,7 +575,6 @@ static void i2c_imx_stop(struct imx_i2c_struct *i2c_imx) /* Disable I2C controller */ temp = i2c_imx-hwdata-i2cr_ien_opcode ^ I2CR_IEN, imx_i2c_write_reg(temp, i2c_imx, IMX_I2C_I2CR); - clk_disable_unprepare(i2c_imx-clk); } static irqreturn_t i2c_imx_isr(int irq, void *dev_id) @@ -894,6 +893,10 @@ static int i2c_imx_xfer(struct i2c_adapter *adapter, dev_dbg(i2c_imx-adapter.dev, %s\n, __func__); + result = pm_runtime_get_sync(i2c_imx-adapter.dev.parent); + if (result 0) + goto out; + /* Start I2C transfer */ result = i2c_imx_start(i2c_imx); if (result) @@ -950,6 +953,10 @@ fail0: /* Stop I2C transfer */ i2c_imx_stop(i2c_imx); +out: + pm_runtime_mark_last_busy(i2c_imx-adapter.dev.parent); + pm_runtime_put_autosuspend(i2c_imx-adapter.dev.parent); + dev_dbg(i2c_imx-adapter.dev, %s exit with: %s: %d\n, __func__, (result 0) ? error : success msg, (result 0) ? result : num); @@ -1020,15 +1027,17 @@ static int i2c_imx_probe(struct platform_device *pdev) ret = clk_prepare_enable(i2c_imx-clk); if (ret) { - dev_err(pdev-dev, can't enable I2C clock\n); + dev_err(pdev-dev, can't enable I2C clock, ret=%d\n, ret); return ret; } + /* Request IRQ */ ret = devm_request_irq(pdev-dev, irq, i2c_imx_isr, 0, pdev-name, i2c_imx); if (ret) { dev_err(pdev-dev, can't claim irq %d\n, irq); - goto clk_disable; + clk_disable_unprepare(i2c_imx-clk); + return ret; } /* Init queue */ @@ -1037,6 +1046,18 @@ static int i2c_imx_probe(struct platform_device *pdev) /* Set up adapter data */ i2c_set_adapdata(i2c_imx-adapter, i2c_imx); + /* Set up platform driver data */ + platform_set_drvdata(pdev, i2c_imx); + + pm_runtime_set_autosuspend_delay(pdev-dev, I2C_PM_TIMEOUT); +
Re: [Patch V5] i2c: imx: add runtime pm support to improve the performance
Am 27.08.2015 um 06:01 schrieb Gao Pandy: From: Heiner Kallweit mailto:hkallwe...@gmail.com Sent: Thursday, August 27, 2015 2:52 AM To: Gao Pan-B54642; w...@the-dreams.de Cc: linux-i2c@vger.kernel.org; Li Frank-B20596; Duan Fugang-B38611; u.kleine-koe...@pengutronix.de; ker...@pengutronix.de Subject: Re: [Patch V5] i2c: imx: add runtime pm support to improve the performance Am 26.08.2015 um 07:23 schrieb Gao Pan: In our former i2c driver, i2c clk is enabled and disabled in xfer function, which contributes to power saving. However, the clk enable process brings a busy wait delay until the core is stable. As a result, the performance is sacrificed. To weigh the power consumption and i2c bus performance, runtime pm is the good solution for it. The clk is enabled when a i2c transfer starts, and disabled after a specifically defined delay. Without the patch the test case (many eeprom reads) executes with approx: real 1m7.735s user 0m0.488s sys 0m20.040s With the patch the same test case (many eeprom reads) executes with approx: real 0m54.241s user 0m0.440s sys 0m5.920s Signed-off-by: Fugang Duan b38...@freescale.com Signed-off-by: Gao Pan b54...@freescale.com [wsa: fixed some indentation] Signed-off-by: Wolfram Sang w...@the-dreams.de --- V2: As Uwe Kleine-König's suggestion, the version do below changes: -call clk_prepare_enable in probe to avoid never enabling clock if CONFIG_PM is disabled -enable clock before request IRQ in probe -remove the pm staff in i2c_imx_isr V3: -pm_runtime_get_sync returns 0 as error V4: -add pm_runtime_set_active before pm_runtime_enable -replace pm_runtime_put_autosuspend with pm_runtime_autosuspend in probe -add error disposal when i2c_add_numbered_adapter fails V5: -clean up and disable runtime PM when i2c_add_numbered_adapter fails -use pm_runtime_get and pm_runtime_put_autosuspend in probe drivers/i2c/busses/i2c-imx.c | 91 1 file changed, 76 insertions(+), 15 deletions(-) diff --git a/drivers/i2c/busses/i2c-imx.c b/drivers/i2c/busses/i2c-imx.c index 785aa67..8940a89 100644 --- a/drivers/i2c/busses/i2c-imx.c +++ b/drivers/i2c/busses/i2c-imx.c @@ -53,6 +53,7 @@ #include linux/platform_device.h #include linux/sched.h #include linux/slab.h +#include linux/pm_runtime.h /** Defines ** */ @@ -118,6 +119,8 @@ #define I2CR_IEN_OPCODE_0 0x0 #define I2CR_IEN_OPCODE_1 I2CR_IEN +#define I2C_PM_TIMEOUT 10 /* ms */ + /** Variables ** ** */ @@ -520,9 +523,6 @@ static int i2c_imx_start(struct imx_i2c_struct *i2c_imx) i2c_imx_set_clk(i2c_imx); - result = clk_prepare_enable(i2c_imx-clk); - if (result) - return result; imx_i2c_write_reg(i2c_imx-ifdr, i2c_imx, IMX_I2C_IFDR); /* Enable I2C controller */ imx_i2c_write_reg(i2c_imx-hwdata-i2sr_clr_opcode, i2c_imx, IMX_I2C_I2SR); @@ -575,7 +575,6 @@ static void i2c_imx_stop(struct imx_i2c_struct *i2c_imx) /* Disable I2C controller */ temp = i2c_imx-hwdata-i2cr_ien_opcode ^ I2CR_IEN, imx_i2c_write_reg(temp, i2c_imx, IMX_I2C_I2CR); - clk_disable_unprepare(i2c_imx-clk); } static irqreturn_t i2c_imx_isr(int irq, void *dev_id) @@ -894,6 +893,10 @@ static int i2c_imx_xfer(struct i2c_adapter *adapter, dev_dbg(i2c_imx-adapter.dev, %s\n, __func__); + result = pm_runtime_get_sync(i2c_imx-adapter.dev.parent); + if (result 0) + goto out; + /* Start I2C transfer */ result = i2c_imx_start(i2c_imx); if (result) @@ -950,6 +953,10 @@ fail0: /* Stop I2C transfer */ i2c_imx_stop(i2c_imx); +out: + pm_runtime_mark_last_busy(i2c_imx-adapter.dev.parent); + pm_runtime_put_autosuspend(i2c_imx-adapter.dev.parent); + dev_dbg(i2c_imx-adapter.dev, %s exit with: %s: %d\n, __func__, (result 0) ? error : success msg, (result 0) ? result : num); @@ -1020,15 +1027,17 @@ static int i2c_imx_probe(struct platform_device *pdev) ret = clk_prepare_enable(i2c_imx-clk); if (ret) { - dev_err(pdev-dev, can't enable I2C clock\n); + dev_err(pdev-dev, can't enable I2C clock, ret=%d\n, ret); return ret; } + /* Request IRQ */ ret = devm_request_irq(pdev-dev, irq, i2c_imx_isr, 0, pdev-name, i2c_imx); if (ret) { dev_err(pdev-dev, can't claim irq %d\n, irq); - goto clk_disable; + clk_disable_unprepare(i2c_imx-clk); + return ret; } /* Init queue */ @@ -1037,6 +1046,18 @@ static int i2c_imx_probe(struct platform_device *pdev) /* Set up
RE: [Patch V5] i2c: imx: add runtime pm support to improve the performance
From: Heiner Kallweit mailto:hkallwe...@gmail.com Sent: Thursday, August 27, 2015 2:52 AM To: Gao Pan-B54642; w...@the-dreams.de Cc: linux-i2c@vger.kernel.org; Li Frank-B20596; Duan Fugang-B38611; u.kleine-koe...@pengutronix.de; ker...@pengutronix.de Subject: Re: [Patch V5] i2c: imx: add runtime pm support to improve the performance Am 26.08.2015 um 07:23 schrieb Gao Pan: In our former i2c driver, i2c clk is enabled and disabled in xfer function, which contributes to power saving. However, the clk enable process brings a busy wait delay until the core is stable. As a result, the performance is sacrificed. To weigh the power consumption and i2c bus performance, runtime pm is the good solution for it. The clk is enabled when a i2c transfer starts, and disabled after a specifically defined delay. Without the patch the test case (many eeprom reads) executes with approx: real 1m7.735s user 0m0.488s sys 0m20.040s With the patch the same test case (many eeprom reads) executes with approx: real 0m54.241s user 0m0.440s sys 0m5.920s Signed-off-by: Fugang Duan b38...@freescale.com Signed-off-by: Gao Pan b54...@freescale.com [wsa: fixed some indentation] Signed-off-by: Wolfram Sang w...@the-dreams.de --- V2: As Uwe Kleine-König's suggestion, the version do below changes: -call clk_prepare_enable in probe to avoid never enabling clock if CONFIG_PM is disabled -enable clock before request IRQ in probe -remove the pm staff in i2c_imx_isr V3: -pm_runtime_get_sync returns 0 as error V4: -add pm_runtime_set_active before pm_runtime_enable -replace pm_runtime_put_autosuspend with pm_runtime_autosuspend in probe -add error disposal when i2c_add_numbered_adapter fails V5: -clean up and disable runtime PM when i2c_add_numbered_adapter fails -use pm_runtime_get and pm_runtime_put_autosuspend in probe drivers/i2c/busses/i2c-imx.c | 91 1 file changed, 76 insertions(+), 15 deletions(-) diff --git a/drivers/i2c/busses/i2c-imx.c b/drivers/i2c/busses/i2c-imx.c index 785aa67..8940a89 100644 --- a/drivers/i2c/busses/i2c-imx.c +++ b/drivers/i2c/busses/i2c-imx.c @@ -53,6 +53,7 @@ #include linux/platform_device.h #include linux/sched.h #include linux/slab.h +#include linux/pm_runtime.h /** Defines ** */ @@ -118,6 +119,8 @@ #define I2CR_IEN_OPCODE_0 0x0 #define I2CR_IEN_OPCODE_1 I2CR_IEN +#define I2C_PM_TIMEOUT 10 /* ms */ + /** Variables ** ** */ @@ -520,9 +523,6 @@ static int i2c_imx_start(struct imx_i2c_struct *i2c_imx) i2c_imx_set_clk(i2c_imx); - result = clk_prepare_enable(i2c_imx-clk); - if (result) - return result; imx_i2c_write_reg(i2c_imx-ifdr, i2c_imx, IMX_I2C_IFDR); /* Enable I2C controller */ imx_i2c_write_reg(i2c_imx-hwdata-i2sr_clr_opcode, i2c_imx, IMX_I2C_I2SR); @@ -575,7 +575,6 @@ static void i2c_imx_stop(struct imx_i2c_struct *i2c_imx) /* Disable I2C controller */ temp = i2c_imx-hwdata-i2cr_ien_opcode ^ I2CR_IEN, imx_i2c_write_reg(temp, i2c_imx, IMX_I2C_I2CR); - clk_disable_unprepare(i2c_imx-clk); } static irqreturn_t i2c_imx_isr(int irq, void *dev_id) @@ -894,6 +893,10 @@ static int i2c_imx_xfer(struct i2c_adapter *adapter, dev_dbg(i2c_imx-adapter.dev, %s\n, __func__); + result = pm_runtime_get_sync(i2c_imx-adapter.dev.parent); + if (result 0) + goto out; + /* Start I2C transfer */ result = i2c_imx_start(i2c_imx); if (result) @@ -950,6 +953,10 @@ fail0: /* Stop I2C transfer */ i2c_imx_stop(i2c_imx); +out: + pm_runtime_mark_last_busy(i2c_imx-adapter.dev.parent); + pm_runtime_put_autosuspend(i2c_imx-adapter.dev.parent); + dev_dbg(i2c_imx-adapter.dev, %s exit with: %s: %d\n, __func__, (result 0) ? error : success msg, (result 0) ? result : num); @@ -1020,15 +1027,17 @@ static int i2c_imx_probe(struct platform_device *pdev) ret = clk_prepare_enable(i2c_imx-clk); if (ret) { - dev_err(pdev-dev, can't enable I2C clock\n); + dev_err(pdev-dev, can't enable I2C clock, ret=%d\n, ret); return ret; } + /* Request IRQ */ ret = devm_request_irq(pdev-dev, irq, i2c_imx_isr, 0, pdev-name, i2c_imx); if (ret) { dev_err(pdev-dev, can't claim irq %d\n, irq); - goto clk_disable; + clk_disable_unprepare(i2c_imx-clk); + return ret; } /* Init queue
[Patch V5] i2c: imx: add runtime pm support to improve the performance
In our former i2c driver, i2c clk is enabled and disabled in xfer function, which contributes to power saving. However, the clk enable process brings a busy wait delay until the core is stable. As a result, the performance is sacrificed. To weigh the power consumption and i2c bus performance, runtime pm is the good solution for it. The clk is enabled when a i2c transfer starts, and disabled after a specifically defined delay. Without the patch the test case (many eeprom reads) executes with approx: real 1m7.735s user 0m0.488s sys 0m20.040s With the patch the same test case (many eeprom reads) executes with approx: real 0m54.241s user 0m0.440s sys 0m5.920s Signed-off-by: Fugang Duan b38...@freescale.com Signed-off-by: Gao Pan b54...@freescale.com [wsa: fixed some indentation] Signed-off-by: Wolfram Sang w...@the-dreams.de --- V2: As Uwe Kleine-König's suggestion, the version do below changes: -call clk_prepare_enable in probe to avoid never enabling clock if CONFIG_PM is disabled -enable clock before request IRQ in probe -remove the pm staff in i2c_imx_isr V3: -pm_runtime_get_sync returns 0 as error V4: -add pm_runtime_set_active before pm_runtime_enable -replace pm_runtime_put_autosuspend with pm_runtime_autosuspend in probe -add error disposal when i2c_add_numbered_adapter fails V5: -clean up and disable runtime PM when i2c_add_numbered_adapter fails -use pm_runtime_get and pm_runtime_put_autosuspend in probe drivers/i2c/busses/i2c-imx.c | 91 1 file changed, 76 insertions(+), 15 deletions(-) diff --git a/drivers/i2c/busses/i2c-imx.c b/drivers/i2c/busses/i2c-imx.c index 785aa67..8940a89 100644 --- a/drivers/i2c/busses/i2c-imx.c +++ b/drivers/i2c/busses/i2c-imx.c @@ -53,6 +53,7 @@ #include linux/platform_device.h #include linux/sched.h #include linux/slab.h +#include linux/pm_runtime.h /** Defines ***/ @@ -118,6 +119,8 @@ #define I2CR_IEN_OPCODE_0 0x0 #define I2CR_IEN_OPCODE_1 I2CR_IEN +#define I2C_PM_TIMEOUT 10 /* ms */ + /** Variables ** ***/ @@ -520,9 +523,6 @@ static int i2c_imx_start(struct imx_i2c_struct *i2c_imx) i2c_imx_set_clk(i2c_imx); - result = clk_prepare_enable(i2c_imx-clk); - if (result) - return result; imx_i2c_write_reg(i2c_imx-ifdr, i2c_imx, IMX_I2C_IFDR); /* Enable I2C controller */ imx_i2c_write_reg(i2c_imx-hwdata-i2sr_clr_opcode, i2c_imx, IMX_I2C_I2SR); @@ -575,7 +575,6 @@ static void i2c_imx_stop(struct imx_i2c_struct *i2c_imx) /* Disable I2C controller */ temp = i2c_imx-hwdata-i2cr_ien_opcode ^ I2CR_IEN, imx_i2c_write_reg(temp, i2c_imx, IMX_I2C_I2CR); - clk_disable_unprepare(i2c_imx-clk); } static irqreturn_t i2c_imx_isr(int irq, void *dev_id) @@ -894,6 +893,10 @@ static int i2c_imx_xfer(struct i2c_adapter *adapter, dev_dbg(i2c_imx-adapter.dev, %s\n, __func__); + result = pm_runtime_get_sync(i2c_imx-adapter.dev.parent); + if (result 0) + goto out; + /* Start I2C transfer */ result = i2c_imx_start(i2c_imx); if (result) @@ -950,6 +953,10 @@ fail0: /* Stop I2C transfer */ i2c_imx_stop(i2c_imx); +out: + pm_runtime_mark_last_busy(i2c_imx-adapter.dev.parent); + pm_runtime_put_autosuspend(i2c_imx-adapter.dev.parent); + dev_dbg(i2c_imx-adapter.dev, %s exit with: %s: %d\n, __func__, (result 0) ? error : success msg, (result 0) ? result : num); @@ -1020,15 +1027,17 @@ static int i2c_imx_probe(struct platform_device *pdev) ret = clk_prepare_enable(i2c_imx-clk); if (ret) { - dev_err(pdev-dev, can't enable I2C clock\n); + dev_err(pdev-dev, can't enable I2C clock, ret=%d\n, ret); return ret; } + /* Request IRQ */ ret = devm_request_irq(pdev-dev, irq, i2c_imx_isr, 0, pdev-name, i2c_imx); if (ret) { dev_err(pdev-dev, can't claim irq %d\n, irq); - goto clk_disable; + clk_disable_unprepare(i2c_imx-clk); + return ret; } /* Init queue */ @@ -1037,6 +1046,18 @@ static int i2c_imx_probe(struct platform_device *pdev) /* Set up adapter data */ i2c_set_adapdata(i2c_imx-adapter, i2c_imx); + /* Set up platform driver data */ + platform_set_drvdata(pdev, i2c_imx); + + pm_runtime_set_autosuspend_delay(pdev-dev, I2C_PM_TIMEOUT); + pm_runtime_use_autosuspend(pdev-dev); + pm_runtime_set_active(pdev-dev); + pm_runtime_enable(pdev-dev); + +