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 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 > #include > #include > +#include > #include > #include > #include > @@ -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 >