| Message ID | 20191213081230.23494-4-bibby.hsieh@mediatek.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | add power control in i2c and at24 | expand |
pt., 13 gru 2019 o 09:47 Bibby Hsieh <bibby.hsieh@mediatek.com> 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@mediatek.com> > --- > 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
On Fri, 2019-12-13 at 10:02 +0100, Bartosz Golaszewski wrote: > pt., 13 gru 2019 o 09:47 Bibby Hsieh <bibby.hsieh@mediatek.com> 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@mediatek.com> > > --- > > 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. Ok, will remove it. > > > + 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. > Ok. > > + 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? > Sorry for missing it again... I will add invert next version. Bibby > 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
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"); + 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"); + 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); 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), },
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@mediatek.com> --- drivers/misc/eeprom/at24.c | 40 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 40 insertions(+)