pt., 13 gru 2019 o 09:47 Bibby Hsieh <bibby.hsieh@xxxxxxxxxxxx> napisał(a): > > Although in the most platforms, the power of eeprom are alway > on, some platforms disable the eeprom power in order to meet > low power request. This patch add the pm_runtime ops to control > power to support all platforms. > > Signed-off-by: Bibby Hsieh <bibby.hsieh@xxxxxxxxxxxx> > --- > drivers/misc/eeprom/at24.c | 40 ++++++++++++++++++++++++++++++++++++++ > 1 file changed, 40 insertions(+) > > diff --git a/drivers/misc/eeprom/at24.c b/drivers/misc/eeprom/at24.c > index 0681d5fdd538..06ae2cc32f79 100644 > --- a/drivers/misc/eeprom/at24.c > +++ b/drivers/misc/eeprom/at24.c > @@ -22,6 +22,7 @@ > #include <linux/nvmem-provider.h> > #include <linux/regmap.h> > #include <linux/pm_runtime.h> > +#include <linux/regulator/consumer.h> > #include <linux/gpio/consumer.h> > > /* Address pointer is 16 bit. */ > @@ -91,6 +92,7 @@ struct at24_data { > > struct gpio_desc *wp_gpio; > > + struct regulator *vcc_reg; > /* > * Some chips tie up multiple I2C addresses; dummy devices reserve > * them for us, and we'll use them with SMBus calls. > @@ -662,6 +664,12 @@ static int at24_probe(struct i2c_client *client) > at24->client[0].client = client; > at24->client[0].regmap = regmap; > > + at24->vcc_reg = devm_regulator_get(dev, "vcc"); > + if (IS_ERR(at24->vcc_reg)) { > + dev_err(dev, "failed to get at24 VCC regulator\n"); The regulator core is quite verbose in its error messages when calling regulator_get() - you don't need to add yours here. Just return the error code. > + return PTR_ERR(at24->vcc_reg); > + } > + > at24->wp_gpio = devm_gpiod_get_optional(dev, "wp", GPIOD_OUT_HIGH); > if (IS_ERR(at24->wp_gpio)) > return PTR_ERR(at24->wp_gpio); > @@ -701,6 +709,12 @@ static int at24_probe(struct i2c_client *client) > > i2c_set_clientdata(client, at24); > > + err = regulator_enable(at24->vcc_reg); > + if (err) { > + dev_err(dev, "Failed to enable at24 vcc regulator\n"); Drop the at24 name - dev_err() will print the device name for you. > + return err; > + } > + > /* enable runtime pm */ > pm_runtime_set_active(dev); > pm_runtime_enable(dev); > @@ -713,6 +727,7 @@ static int at24_probe(struct i2c_client *client) > pm_runtime_idle(dev); > if (err) { > pm_runtime_disable(dev); > + regulator_disable(at24->vcc_reg); > return -ENODEV; > } > > @@ -729,14 +744,39 @@ static int at24_probe(struct i2c_client *client) > static int at24_remove(struct i2c_client *client) > { > pm_runtime_disable(&client->dev); > + if (pm_runtime_status_suspended(&client->dev)) > + regulator_disable(at24->vcc_reg); Why didn't you fix the inverted logic here as I pointed out back in v6 of this series? Bart > pm_runtime_set_suspended(&client->dev); > > return 0; > } > > +static int __maybe_unused at24_suspend(struct device *dev) > +{ > + struct i2c_client *client = to_i2c_client(dev); > + struct at24_data *at24 = i2c_get_clientdata(client); > + > + return regulator_disable(at24->vcc_reg); > +} > + > +static int __maybe_unused at24_resume(struct device *dev) > +{ > + struct i2c_client *client = to_i2c_client(dev); > + struct at24_data *at24 = i2c_get_clientdata(client); > + > + return regulator_enable(at24->vcc_reg); > +} > + > +static const struct dev_pm_ops at24_pm_ops = { > + SET_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend, > + pm_runtime_force_resume) > + SET_RUNTIME_PM_OPS(at24_suspend, at24_resume, NULL) > +}; > + > static struct i2c_driver at24_driver = { > .driver = { > .name = "at24", > + .pm = &at24_pm_ops, > .of_match_table = at24_of_match, > .acpi_match_table = ACPI_PTR(at24_acpi_ids), > }, > -- > 2.18.0