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 <sam...@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
>
signature.asc
Description: PGP signature
