* [PATCH] i2c: mv64xxx: Add runtime PM support @ 2021-01-03 10:51 Samuel Holland 2021-01-28 9:22 ` Wolfram Sang 2021-02-01 22:07 ` Wolfram Sang 0 siblings, 2 replies; 4+ messages in thread From: Samuel Holland @ 2021-01-03 10:51 UTC (permalink / raw) To: Gregory CLEMENT, linux-i2c Cc: Maxime Ripard, Chen-Yu Tsai, Jernej Skrabec, Ondrej Jirman, linux-kernel, linux-sunxi, Samuel Holland To save power, gate the clock when the bus is inactive, during system sleep, and during shutdown. On some platforms, specifically Allwinner A13/A20, gating the clock implicitly resets the module as well. Since the module already needs to be reset after some suspend/resume cycles, it is simple enough to reset it during every runtime suspend/resume. Because the bus may be used by wakeup source IRQ threads, it needs to be functional as soon as IRQs are enabled. Thus, its system PM hooks need to run in the noirq phase. Signed-off-by: Samuel Holland <samuel@sholland.org> --- drivers/i2c/busses/i2c-mv64xxx.c | 120 ++++++++++++++++++++----------- 1 file changed, 78 insertions(+), 42 deletions(-) diff --git a/drivers/i2c/busses/i2c-mv64xxx.c b/drivers/i2c/busses/i2c-mv64xxx.c index 5cfe70aedced..b03c344323d1 100644 --- a/drivers/i2c/busses/i2c-mv64xxx.c +++ b/drivers/i2c/busses/i2c-mv64xxx.c @@ -18,6 +18,7 @@ #include <linux/mv643xx_i2c.h> #include <linux/platform_device.h> #include <linux/pinctrl/consumer.h> +#include <linux/pm_runtime.h> #include <linux/reset.h> #include <linux/io.h> #include <linux/of.h> @@ -717,6 +718,10 @@ mv64xxx_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[], int num) struct mv64xxx_i2c_data *drv_data = i2c_get_adapdata(adap); int rc, ret = num; + rc = pm_runtime_resume_and_get(&adap->dev); + if (rc) + return rc; + BUG_ON(drv_data->msgs != NULL); drv_data->msgs = msgs; drv_data->num_msgs = num; @@ -732,6 +737,9 @@ mv64xxx_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[], int num) drv_data->num_msgs = 0; drv_data->msgs = NULL; + pm_runtime_mark_last_busy(&adap->dev); + pm_runtime_put_autosuspend(&adap->dev); + return ret; } @@ -828,7 +836,6 @@ mv64xxx_of_config(struct mv64xxx_i2c_data *drv_data, rc = PTR_ERR(drv_data->rstc); goto out; } - reset_control_deassert(drv_data->rstc); /* Its not yet defined how timeouts will be specified in device tree. * So hard code the value to 1 second. @@ -893,6 +900,32 @@ static int mv64xxx_i2c_init_recovery_info(struct mv64xxx_i2c_data *drv_data, return 0; } +static int +mv64xxx_i2c_runtime_suspend(struct device *dev) +{ + struct mv64xxx_i2c_data *drv_data = dev_get_drvdata(dev); + + reset_control_assert(drv_data->rstc); + clk_disable_unprepare(drv_data->reg_clk); + clk_disable_unprepare(drv_data->clk); + + return 0; +} + +static int +mv64xxx_i2c_runtime_resume(struct device *dev) +{ + struct mv64xxx_i2c_data *drv_data = dev_get_drvdata(dev); + + clk_prepare_enable(drv_data->clk); + clk_prepare_enable(drv_data->reg_clk); + reset_control_reset(drv_data->rstc); + + mv64xxx_i2c_hw_init(drv_data); + + return 0; +} + static int mv64xxx_i2c_probe(struct platform_device *pd) { @@ -920,18 +953,22 @@ mv64xxx_i2c_probe(struct platform_device *pd) /* Not all platforms have clocks */ drv_data->clk = devm_clk_get(&pd->dev, NULL); - if (PTR_ERR(drv_data->clk) == -EPROBE_DEFER) - return -EPROBE_DEFER; - if (!IS_ERR(drv_data->clk)) - clk_prepare_enable(drv_data->clk); + if (IS_ERR(drv_data->clk)) { + if (PTR_ERR(drv_data->clk) == -EPROBE_DEFER) + return -EPROBE_DEFER; + drv_data->clk = NULL; + } drv_data->reg_clk = devm_clk_get(&pd->dev, "reg"); - if (PTR_ERR(drv_data->reg_clk) == -EPROBE_DEFER) - return -EPROBE_DEFER; - if (!IS_ERR(drv_data->reg_clk)) - clk_prepare_enable(drv_data->reg_clk); + if (IS_ERR(drv_data->reg_clk)) { + if (PTR_ERR(drv_data->reg_clk) == -EPROBE_DEFER) + return -EPROBE_DEFER; + drv_data->reg_clk = NULL; + } drv_data->irq = platform_get_irq(pd, 0); + if (drv_data->irq < 0) + return drv_data->irq; if (pdata) { drv_data->freq_m = pdata->freq_m; @@ -942,16 +979,12 @@ mv64xxx_i2c_probe(struct platform_device *pd) } else if (pd->dev.of_node) { rc = mv64xxx_of_config(drv_data, &pd->dev); if (rc) - goto exit_clk; - } - if (drv_data->irq < 0) { - rc = drv_data->irq; - goto exit_reset; + return rc; } rc = mv64xxx_i2c_init_recovery_info(drv_data, &pd->dev); if (rc == -EPROBE_DEFER) - goto exit_reset; + return rc; drv_data->adapter.dev.parent = &pd->dev; drv_data->adapter.algo = &mv64xxx_i2c_algo; @@ -962,7 +995,14 @@ mv64xxx_i2c_probe(struct platform_device *pd) platform_set_drvdata(pd, drv_data); i2c_set_adapdata(&drv_data->adapter, drv_data); - mv64xxx_i2c_hw_init(drv_data); + pm_runtime_set_autosuspend_delay(&pd->dev, MSEC_PER_SEC); + pm_runtime_use_autosuspend(&pd->dev); + pm_runtime_enable(&pd->dev); + if (!pm_runtime_enabled(&pd->dev)) { + rc = mv64xxx_i2c_runtime_resume(&pd->dev); + if (rc) + goto exit_disable_pm; + } rc = request_irq(drv_data->irq, mv64xxx_i2c_intr, 0, MV64XXX_I2C_CTLR_NAME, drv_data); @@ -970,7 +1010,7 @@ mv64xxx_i2c_probe(struct platform_device *pd) dev_err(&drv_data->adapter.dev, "mv64xxx: Can't register intr handler irq%d: %d\n", drv_data->irq, rc); - goto exit_reset; + goto exit_disable_pm; } else if ((rc = i2c_add_numbered_adapter(&drv_data->adapter)) != 0) { dev_err(&drv_data->adapter.dev, "mv64xxx: Can't add i2c adapter, rc: %d\n", -rc); @@ -981,54 +1021,50 @@ mv64xxx_i2c_probe(struct platform_device *pd) exit_free_irq: free_irq(drv_data->irq, drv_data); -exit_reset: - reset_control_assert(drv_data->rstc); -exit_clk: - clk_disable_unprepare(drv_data->reg_clk); - clk_disable_unprepare(drv_data->clk); +exit_disable_pm: + pm_runtime_disable(&pd->dev); + if (!pm_runtime_status_suspended(&pd->dev)) + mv64xxx_i2c_runtime_suspend(&pd->dev); return rc; } static int -mv64xxx_i2c_remove(struct platform_device *dev) +mv64xxx_i2c_remove(struct platform_device *pd) { - struct mv64xxx_i2c_data *drv_data = platform_get_drvdata(dev); + struct mv64xxx_i2c_data *drv_data = platform_get_drvdata(pd); i2c_del_adapter(&drv_data->adapter); free_irq(drv_data->irq, drv_data); - reset_control_assert(drv_data->rstc); - clk_disable_unprepare(drv_data->reg_clk); - clk_disable_unprepare(drv_data->clk); + pm_runtime_disable(&pd->dev); + if (!pm_runtime_status_suspended(&pd->dev)) + mv64xxx_i2c_runtime_suspend(&pd->dev); return 0; } -#ifdef CONFIG_PM -static int mv64xxx_i2c_resume(struct device *dev) +static void +mv64xxx_i2c_shutdown(struct platform_device *pd) { - struct mv64xxx_i2c_data *drv_data = dev_get_drvdata(dev); - - mv64xxx_i2c_hw_init(drv_data); - - return 0; + pm_runtime_disable(&pd->dev); + if (!pm_runtime_status_suspended(&pd->dev)) + mv64xxx_i2c_runtime_suspend(&pd->dev); } -static const struct dev_pm_ops mv64xxx_i2c_pm = { - .resume = mv64xxx_i2c_resume, +static const struct dev_pm_ops mv64xxx_i2c_pm_ops = { + SET_RUNTIME_PM_OPS(mv64xxx_i2c_runtime_suspend, + mv64xxx_i2c_runtime_resume, NULL) + SET_NOIRQ_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend, + pm_runtime_force_resume) }; -#define mv64xxx_i2c_pm_ops (&mv64xxx_i2c_pm) -#else -#define mv64xxx_i2c_pm_ops NULL -#endif - static struct platform_driver mv64xxx_i2c_driver = { .probe = mv64xxx_i2c_probe, .remove = mv64xxx_i2c_remove, + .shutdown = mv64xxx_i2c_shutdown, .driver = { .name = MV64XXX_I2C_CTLR_NAME, - .pm = mv64xxx_i2c_pm_ops, + .pm = &mv64xxx_i2c_pm_ops, .of_match_table = mv64xxx_i2c_of_match_table, }, }; -- 2.26.2 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] i2c: mv64xxx: Add runtime PM support 2021-01-03 10:51 [PATCH] i2c: mv64xxx: Add runtime PM support Samuel Holland @ 2021-01-28 9:22 ` Wolfram Sang 2021-01-28 15:47 ` Ondřej Jirman 2021-02-01 22:07 ` Wolfram Sang 1 sibling, 1 reply; 4+ messages in thread From: Wolfram Sang @ 2021-01-28 9:22 UTC (permalink / raw) To: Samuel Holland Cc: Gregory CLEMENT, linux-i2c, Maxime Ripard, Chen-Yu Tsai, Jernej Skrabec, Ondrej Jirman, linux-kernel, linux-sunxi [-- Attachment #1: Type: text/plain, Size: 8061 bytes --] On Sun, Jan 03, 2021 at 04:51:46AM -0600, Samuel Holland wrote: > To save power, gate the clock when the bus is inactive, during system > sleep, and during shutdown. On some platforms, specifically Allwinner > A13/A20, gating the clock implicitly resets the module as well. Since > the module already needs to be reset after some suspend/resume cycles, > it is simple enough to reset it during every runtime suspend/resume. > > Because the bus may be used by wakeup source IRQ threads, it needs to > be functional as soon as IRQs are enabled. Thus, its system PM hooks > need to run in the noirq phase. > > Signed-off-by: Samuel Holland <samuel@sholland.org> Gregory, what do you think? > --- > drivers/i2c/busses/i2c-mv64xxx.c | 120 ++++++++++++++++++++----------- > 1 file changed, 78 insertions(+), 42 deletions(-) > > diff --git a/drivers/i2c/busses/i2c-mv64xxx.c b/drivers/i2c/busses/i2c-mv64xxx.c > index 5cfe70aedced..b03c344323d1 100644 > --- a/drivers/i2c/busses/i2c-mv64xxx.c > +++ b/drivers/i2c/busses/i2c-mv64xxx.c > @@ -18,6 +18,7 @@ > #include <linux/mv643xx_i2c.h> > #include <linux/platform_device.h> > #include <linux/pinctrl/consumer.h> > +#include <linux/pm_runtime.h> > #include <linux/reset.h> > #include <linux/io.h> > #include <linux/of.h> > @@ -717,6 +718,10 @@ mv64xxx_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[], int num) > struct mv64xxx_i2c_data *drv_data = i2c_get_adapdata(adap); > int rc, ret = num; > > + rc = pm_runtime_resume_and_get(&adap->dev); > + if (rc) > + return rc; > + > BUG_ON(drv_data->msgs != NULL); > drv_data->msgs = msgs; > drv_data->num_msgs = num; > @@ -732,6 +737,9 @@ mv64xxx_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[], int num) > drv_data->num_msgs = 0; > drv_data->msgs = NULL; > > + pm_runtime_mark_last_busy(&adap->dev); > + pm_runtime_put_autosuspend(&adap->dev); > + > return ret; > } > > @@ -828,7 +836,6 @@ mv64xxx_of_config(struct mv64xxx_i2c_data *drv_data, > rc = PTR_ERR(drv_data->rstc); > goto out; > } > - reset_control_deassert(drv_data->rstc); > > /* Its not yet defined how timeouts will be specified in device tree. > * So hard code the value to 1 second. > @@ -893,6 +900,32 @@ static int mv64xxx_i2c_init_recovery_info(struct mv64xxx_i2c_data *drv_data, > return 0; > } > > +static int > +mv64xxx_i2c_runtime_suspend(struct device *dev) > +{ > + struct mv64xxx_i2c_data *drv_data = dev_get_drvdata(dev); > + > + reset_control_assert(drv_data->rstc); > + clk_disable_unprepare(drv_data->reg_clk); > + clk_disable_unprepare(drv_data->clk); > + > + return 0; > +} > + > +static int > +mv64xxx_i2c_runtime_resume(struct device *dev) > +{ > + struct mv64xxx_i2c_data *drv_data = dev_get_drvdata(dev); > + > + clk_prepare_enable(drv_data->clk); > + clk_prepare_enable(drv_data->reg_clk); > + reset_control_reset(drv_data->rstc); > + > + mv64xxx_i2c_hw_init(drv_data); > + > + return 0; > +} > + > static int > mv64xxx_i2c_probe(struct platform_device *pd) > { > @@ -920,18 +953,22 @@ mv64xxx_i2c_probe(struct platform_device *pd) > > /* Not all platforms have clocks */ > drv_data->clk = devm_clk_get(&pd->dev, NULL); > - if (PTR_ERR(drv_data->clk) == -EPROBE_DEFER) > - return -EPROBE_DEFER; > - if (!IS_ERR(drv_data->clk)) > - clk_prepare_enable(drv_data->clk); > + if (IS_ERR(drv_data->clk)) { > + if (PTR_ERR(drv_data->clk) == -EPROBE_DEFER) > + return -EPROBE_DEFER; > + drv_data->clk = NULL; > + } > > drv_data->reg_clk = devm_clk_get(&pd->dev, "reg"); > - if (PTR_ERR(drv_data->reg_clk) == -EPROBE_DEFER) > - return -EPROBE_DEFER; > - if (!IS_ERR(drv_data->reg_clk)) > - clk_prepare_enable(drv_data->reg_clk); > + if (IS_ERR(drv_data->reg_clk)) { > + if (PTR_ERR(drv_data->reg_clk) == -EPROBE_DEFER) > + return -EPROBE_DEFER; > + drv_data->reg_clk = NULL; > + } > > drv_data->irq = platform_get_irq(pd, 0); > + if (drv_data->irq < 0) > + return drv_data->irq; > > if (pdata) { > drv_data->freq_m = pdata->freq_m; > @@ -942,16 +979,12 @@ mv64xxx_i2c_probe(struct platform_device *pd) > } else if (pd->dev.of_node) { > rc = mv64xxx_of_config(drv_data, &pd->dev); > if (rc) > - goto exit_clk; > - } > - if (drv_data->irq < 0) { > - rc = drv_data->irq; > - goto exit_reset; > + return rc; > } > > rc = mv64xxx_i2c_init_recovery_info(drv_data, &pd->dev); > if (rc == -EPROBE_DEFER) > - goto exit_reset; > + return rc; > > drv_data->adapter.dev.parent = &pd->dev; > drv_data->adapter.algo = &mv64xxx_i2c_algo; > @@ -962,7 +995,14 @@ mv64xxx_i2c_probe(struct platform_device *pd) > platform_set_drvdata(pd, drv_data); > i2c_set_adapdata(&drv_data->adapter, drv_data); > > - mv64xxx_i2c_hw_init(drv_data); > + pm_runtime_set_autosuspend_delay(&pd->dev, MSEC_PER_SEC); > + pm_runtime_use_autosuspend(&pd->dev); > + pm_runtime_enable(&pd->dev); > + if (!pm_runtime_enabled(&pd->dev)) { > + rc = mv64xxx_i2c_runtime_resume(&pd->dev); > + if (rc) > + goto exit_disable_pm; > + } > > rc = request_irq(drv_data->irq, mv64xxx_i2c_intr, 0, > MV64XXX_I2C_CTLR_NAME, drv_data); > @@ -970,7 +1010,7 @@ mv64xxx_i2c_probe(struct platform_device *pd) > dev_err(&drv_data->adapter.dev, > "mv64xxx: Can't register intr handler irq%d: %d\n", > drv_data->irq, rc); > - goto exit_reset; > + goto exit_disable_pm; > } else if ((rc = i2c_add_numbered_adapter(&drv_data->adapter)) != 0) { > dev_err(&drv_data->adapter.dev, > "mv64xxx: Can't add i2c adapter, rc: %d\n", -rc); > @@ -981,54 +1021,50 @@ mv64xxx_i2c_probe(struct platform_device *pd) > > exit_free_irq: > free_irq(drv_data->irq, drv_data); > -exit_reset: > - reset_control_assert(drv_data->rstc); > -exit_clk: > - clk_disable_unprepare(drv_data->reg_clk); > - clk_disable_unprepare(drv_data->clk); > +exit_disable_pm: > + pm_runtime_disable(&pd->dev); > + if (!pm_runtime_status_suspended(&pd->dev)) > + mv64xxx_i2c_runtime_suspend(&pd->dev); > > return rc; > } > > static int > -mv64xxx_i2c_remove(struct platform_device *dev) > +mv64xxx_i2c_remove(struct platform_device *pd) > { > - struct mv64xxx_i2c_data *drv_data = platform_get_drvdata(dev); > + struct mv64xxx_i2c_data *drv_data = platform_get_drvdata(pd); > > i2c_del_adapter(&drv_data->adapter); > free_irq(drv_data->irq, drv_data); > - reset_control_assert(drv_data->rstc); > - clk_disable_unprepare(drv_data->reg_clk); > - clk_disable_unprepare(drv_data->clk); > + pm_runtime_disable(&pd->dev); > + if (!pm_runtime_status_suspended(&pd->dev)) > + mv64xxx_i2c_runtime_suspend(&pd->dev); > > return 0; > } > > -#ifdef CONFIG_PM > -static int mv64xxx_i2c_resume(struct device *dev) > +static void > +mv64xxx_i2c_shutdown(struct platform_device *pd) > { > - struct mv64xxx_i2c_data *drv_data = dev_get_drvdata(dev); > - > - mv64xxx_i2c_hw_init(drv_data); > - > - return 0; > + pm_runtime_disable(&pd->dev); > + if (!pm_runtime_status_suspended(&pd->dev)) > + mv64xxx_i2c_runtime_suspend(&pd->dev); > } > > -static const struct dev_pm_ops mv64xxx_i2c_pm = { > - .resume = mv64xxx_i2c_resume, > +static const struct dev_pm_ops mv64xxx_i2c_pm_ops = { > + SET_RUNTIME_PM_OPS(mv64xxx_i2c_runtime_suspend, > + mv64xxx_i2c_runtime_resume, NULL) > + SET_NOIRQ_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend, > + pm_runtime_force_resume) > }; > > -#define mv64xxx_i2c_pm_ops (&mv64xxx_i2c_pm) > -#else > -#define mv64xxx_i2c_pm_ops NULL > -#endif > - > static struct platform_driver mv64xxx_i2c_driver = { > .probe = mv64xxx_i2c_probe, > .remove = mv64xxx_i2c_remove, > + .shutdown = mv64xxx_i2c_shutdown, > .driver = { > .name = MV64XXX_I2C_CTLR_NAME, > - .pm = mv64xxx_i2c_pm_ops, > + .pm = &mv64xxx_i2c_pm_ops, > .of_match_table = mv64xxx_i2c_of_match_table, > }, > }; > -- > 2.26.2 > [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] i2c: mv64xxx: Add runtime PM support 2021-01-28 9:22 ` Wolfram Sang @ 2021-01-28 15:47 ` Ondřej Jirman 0 siblings, 0 replies; 4+ messages in thread From: Ondřej Jirman @ 2021-01-28 15:47 UTC (permalink / raw) To: Wolfram Sang Cc: Samuel Holland, Gregory CLEMENT, linux-i2c, Maxime Ripard, Chen-Yu Tsai, Jernej Skrabec, linux-kernel, linux-sunxi Hello Samuel, On Thu, Jan 28, 2021 at 10:22:08AM +0100, Wolfram Sang wrote: > On Sun, Jan 03, 2021 at 04:51:46AM -0600, Samuel Holland wrote: > > To save power, gate the clock when the bus is inactive, during system > > sleep, and during shutdown. On some platforms, specifically Allwinner > > A13/A20, gating the clock implicitly resets the module as well. Since > > the module already needs to be reset after some suspend/resume cycles, > > it is simple enough to reset it during every runtime suspend/resume. > > > > Because the bus may be used by wakeup source IRQ threads, it needs to > > be functional as soon as IRQs are enabled. Thus, its system PM hooks > > need to run in the noirq phase. > > > > Signed-off-by: Samuel Holland <samuel@sholland.org> Tested-by: Ondrej Jirman <megous@megous.com> I tried it on H3, H5, H6, A64, A83T and A13 based devices. This time it works on my A13 pocketbook, too. I get "mmc_erase: group start error -110, status 0x0" in dmesg on my 2 Orange Pi 3's soon after boot on 5.11-rc4 + this patch. Though I do get these without this patch too. So those timeouts are unrelated. I think it's comming from some f2fs background gc job doing trim and mmc sometimes times out during that. It doesn't lead to corruption. kind regards, o. > Gregory, what do you think? > > > --- > > drivers/i2c/busses/i2c-mv64xxx.c | 120 ++++++++++++++++++++----------- > > 1 file changed, 78 insertions(+), 42 deletions(-) > > > > diff --git a/drivers/i2c/busses/i2c-mv64xxx.c b/drivers/i2c/busses/i2c-mv64xxx.c > > index 5cfe70aedced..b03c344323d1 100644 > > --- a/drivers/i2c/busses/i2c-mv64xxx.c > > +++ b/drivers/i2c/busses/i2c-mv64xxx.c > > @@ -18,6 +18,7 @@ > > #include <linux/mv643xx_i2c.h> > > #include <linux/platform_device.h> > > #include <linux/pinctrl/consumer.h> > > +#include <linux/pm_runtime.h> > > #include <linux/reset.h> > > #include <linux/io.h> > > #include <linux/of.h> > > @@ -717,6 +718,10 @@ mv64xxx_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[], int num) > > struct mv64xxx_i2c_data *drv_data = i2c_get_adapdata(adap); > > int rc, ret = num; > > > > + rc = pm_runtime_resume_and_get(&adap->dev); > > + if (rc) > > + return rc; > > + > > BUG_ON(drv_data->msgs != NULL); > > drv_data->msgs = msgs; > > drv_data->num_msgs = num; > > @@ -732,6 +737,9 @@ mv64xxx_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[], int num) > > drv_data->num_msgs = 0; > > drv_data->msgs = NULL; > > > > + pm_runtime_mark_last_busy(&adap->dev); > > + pm_runtime_put_autosuspend(&adap->dev); > > + > > return ret; > > } > > > > @@ -828,7 +836,6 @@ mv64xxx_of_config(struct mv64xxx_i2c_data *drv_data, > > rc = PTR_ERR(drv_data->rstc); > > goto out; > > } > > - reset_control_deassert(drv_data->rstc); > > > > /* Its not yet defined how timeouts will be specified in device tree. > > * So hard code the value to 1 second. > > @@ -893,6 +900,32 @@ static int mv64xxx_i2c_init_recovery_info(struct mv64xxx_i2c_data *drv_data, > > return 0; > > } > > > > +static int > > +mv64xxx_i2c_runtime_suspend(struct device *dev) > > +{ > > + struct mv64xxx_i2c_data *drv_data = dev_get_drvdata(dev); > > + > > + reset_control_assert(drv_data->rstc); > > + clk_disable_unprepare(drv_data->reg_clk); > > + clk_disable_unprepare(drv_data->clk); > > + > > + return 0; > > +} > > + > > +static int > > +mv64xxx_i2c_runtime_resume(struct device *dev) > > +{ > > + struct mv64xxx_i2c_data *drv_data = dev_get_drvdata(dev); > > + > > + clk_prepare_enable(drv_data->clk); > > + clk_prepare_enable(drv_data->reg_clk); > > + reset_control_reset(drv_data->rstc); > > + > > + mv64xxx_i2c_hw_init(drv_data); > > + > > + return 0; > > +} > > + > > static int > > mv64xxx_i2c_probe(struct platform_device *pd) > > { > > @@ -920,18 +953,22 @@ mv64xxx_i2c_probe(struct platform_device *pd) > > > > /* Not all platforms have clocks */ > > drv_data->clk = devm_clk_get(&pd->dev, NULL); > > - if (PTR_ERR(drv_data->clk) == -EPROBE_DEFER) > > - return -EPROBE_DEFER; > > - if (!IS_ERR(drv_data->clk)) > > - clk_prepare_enable(drv_data->clk); > > + if (IS_ERR(drv_data->clk)) { > > + if (PTR_ERR(drv_data->clk) == -EPROBE_DEFER) > > + return -EPROBE_DEFER; > > + drv_data->clk = NULL; > > + } > > > > drv_data->reg_clk = devm_clk_get(&pd->dev, "reg"); > > - if (PTR_ERR(drv_data->reg_clk) == -EPROBE_DEFER) > > - return -EPROBE_DEFER; > > - if (!IS_ERR(drv_data->reg_clk)) > > - clk_prepare_enable(drv_data->reg_clk); > > + if (IS_ERR(drv_data->reg_clk)) { > > + if (PTR_ERR(drv_data->reg_clk) == -EPROBE_DEFER) > > + return -EPROBE_DEFER; > > + drv_data->reg_clk = NULL; > > + } > > > > drv_data->irq = platform_get_irq(pd, 0); > > + if (drv_data->irq < 0) > > + return drv_data->irq; > > > > if (pdata) { > > drv_data->freq_m = pdata->freq_m; > > @@ -942,16 +979,12 @@ mv64xxx_i2c_probe(struct platform_device *pd) > > } else if (pd->dev.of_node) { > > rc = mv64xxx_of_config(drv_data, &pd->dev); > > if (rc) > > - goto exit_clk; > > - } > > - if (drv_data->irq < 0) { > > - rc = drv_data->irq; > > - goto exit_reset; > > + return rc; > > } > > > > rc = mv64xxx_i2c_init_recovery_info(drv_data, &pd->dev); > > if (rc == -EPROBE_DEFER) > > - goto exit_reset; > > + return rc; > > > > drv_data->adapter.dev.parent = &pd->dev; > > drv_data->adapter.algo = &mv64xxx_i2c_algo; > > @@ -962,7 +995,14 @@ mv64xxx_i2c_probe(struct platform_device *pd) > > platform_set_drvdata(pd, drv_data); > > i2c_set_adapdata(&drv_data->adapter, drv_data); > > > > - mv64xxx_i2c_hw_init(drv_data); > > + pm_runtime_set_autosuspend_delay(&pd->dev, MSEC_PER_SEC); > > + pm_runtime_use_autosuspend(&pd->dev); > > + pm_runtime_enable(&pd->dev); > > + if (!pm_runtime_enabled(&pd->dev)) { > > + rc = mv64xxx_i2c_runtime_resume(&pd->dev); > > + if (rc) > > + goto exit_disable_pm; > > + } > > > > rc = request_irq(drv_data->irq, mv64xxx_i2c_intr, 0, > > MV64XXX_I2C_CTLR_NAME, drv_data); > > @@ -970,7 +1010,7 @@ mv64xxx_i2c_probe(struct platform_device *pd) > > dev_err(&drv_data->adapter.dev, > > "mv64xxx: Can't register intr handler irq%d: %d\n", > > drv_data->irq, rc); > > - goto exit_reset; > > + goto exit_disable_pm; > > } else if ((rc = i2c_add_numbered_adapter(&drv_data->adapter)) != 0) { > > dev_err(&drv_data->adapter.dev, > > "mv64xxx: Can't add i2c adapter, rc: %d\n", -rc); > > @@ -981,54 +1021,50 @@ mv64xxx_i2c_probe(struct platform_device *pd) > > > > exit_free_irq: > > free_irq(drv_data->irq, drv_data); > > -exit_reset: > > - reset_control_assert(drv_data->rstc); > > -exit_clk: > > - clk_disable_unprepare(drv_data->reg_clk); > > - clk_disable_unprepare(drv_data->clk); > > +exit_disable_pm: > > + pm_runtime_disable(&pd->dev); > > + if (!pm_runtime_status_suspended(&pd->dev)) > > + mv64xxx_i2c_runtime_suspend(&pd->dev); > > > > return rc; > > } > > > > static int > > -mv64xxx_i2c_remove(struct platform_device *dev) > > +mv64xxx_i2c_remove(struct platform_device *pd) > > { > > - struct mv64xxx_i2c_data *drv_data = platform_get_drvdata(dev); > > + struct mv64xxx_i2c_data *drv_data = platform_get_drvdata(pd); > > > > i2c_del_adapter(&drv_data->adapter); > > free_irq(drv_data->irq, drv_data); > > - reset_control_assert(drv_data->rstc); > > - clk_disable_unprepare(drv_data->reg_clk); > > - clk_disable_unprepare(drv_data->clk); > > + pm_runtime_disable(&pd->dev); > > + if (!pm_runtime_status_suspended(&pd->dev)) > > + mv64xxx_i2c_runtime_suspend(&pd->dev); > > > > return 0; > > } > > > > -#ifdef CONFIG_PM > > -static int mv64xxx_i2c_resume(struct device *dev) > > +static void > > +mv64xxx_i2c_shutdown(struct platform_device *pd) > > { > > - struct mv64xxx_i2c_data *drv_data = dev_get_drvdata(dev); > > - > > - mv64xxx_i2c_hw_init(drv_data); > > - > > - return 0; > > + pm_runtime_disable(&pd->dev); > > + if (!pm_runtime_status_suspended(&pd->dev)) > > + mv64xxx_i2c_runtime_suspend(&pd->dev); > > } > > > > -static const struct dev_pm_ops mv64xxx_i2c_pm = { > > - .resume = mv64xxx_i2c_resume, > > +static const struct dev_pm_ops mv64xxx_i2c_pm_ops = { > > + SET_RUNTIME_PM_OPS(mv64xxx_i2c_runtime_suspend, > > + mv64xxx_i2c_runtime_resume, NULL) > > + SET_NOIRQ_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend, > > + pm_runtime_force_resume) > > }; > > > > -#define mv64xxx_i2c_pm_ops (&mv64xxx_i2c_pm) > > -#else > > -#define mv64xxx_i2c_pm_ops NULL > > -#endif > > - > > static struct platform_driver mv64xxx_i2c_driver = { > > .probe = mv64xxx_i2c_probe, > > .remove = mv64xxx_i2c_remove, > > + .shutdown = mv64xxx_i2c_shutdown, > > .driver = { > > .name = MV64XXX_I2C_CTLR_NAME, > > - .pm = mv64xxx_i2c_pm_ops, > > + .pm = &mv64xxx_i2c_pm_ops, > > .of_match_table = mv64xxx_i2c_of_match_table, > > }, > > }; > > -- > > 2.26.2 > > ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] i2c: mv64xxx: Add runtime PM support 2021-01-03 10:51 [PATCH] i2c: mv64xxx: Add runtime PM support Samuel Holland 2021-01-28 9:22 ` Wolfram Sang @ 2021-02-01 22:07 ` Wolfram Sang 1 sibling, 0 replies; 4+ messages in thread From: Wolfram Sang @ 2021-02-01 22:07 UTC (permalink / raw) To: Samuel Holland Cc: Gregory CLEMENT, linux-i2c, Maxime Ripard, Chen-Yu Tsai, Jernej Skrabec, Ondrej Jirman, linux-kernel, linux-sunxi [-- Attachment #1: Type: text/plain, Size: 726 bytes --] On Sun, Jan 03, 2021 at 04:51:46AM -0600, Samuel Holland wrote: > To save power, gate the clock when the bus is inactive, during system > sleep, and during shutdown. On some platforms, specifically Allwinner > A13/A20, gating the clock implicitly resets the module as well. Since > the module already needs to be reset after some suspend/resume cycles, > it is simple enough to reset it during every runtime suspend/resume. > > Because the bus may be used by wakeup source IRQ threads, it needs to > be functional as soon as IRQs are enabled. Thus, its system PM hooks > need to run in the noirq phase. > > Signed-off-by: Samuel Holland <samuel@sholland.org> Applied to for-next with Gregory's ack, thanks! [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2021-02-01 22:08 UTC | newest] Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-01-03 10:51 [PATCH] i2c: mv64xxx: Add runtime PM support Samuel Holland 2021-01-28 9:22 ` Wolfram Sang 2021-01-28 15:47 ` Ondřej Jirman 2021-02-01 22:07 ` Wolfram Sang
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).