* [PATCH v9 1/4] dt-binding: eeprom: at24: add vcc-supply property
2019-12-16 8:04 [PATCH v9 0/4] add power control in i2c and at24 Bibby Hsieh
@ 2019-12-16 8:04 ` Bibby Hsieh
2019-12-16 8:04 ` [PATCH v9 2/4] dt-binding: i2c: add bus-supply property Bibby Hsieh
` (2 subsequent siblings)
3 siblings, 0 replies; 9+ messages in thread
From: Bibby Hsieh @ 2019-12-16 8:04 UTC (permalink / raw)
To: Wolfram Sang, Bartosz Golaszewski, linux-i2c
Cc: tfiga, drinkcat, srv_heupstream, robh+dt, mark.rutland,
devicetree, Bibby Hsieh
In some platforms, they disable the power-supply of eeprom due
to power consumption reduction. This patch add vcc-supply property.
Signed-off-by: Bibby Hsieh <bibby.hsieh@mediatek.com>
Reviewed-by: Rob Herring <robh@kernel.org>
---
Documentation/devicetree/bindings/eeprom/at24.yaml | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/Documentation/devicetree/bindings/eeprom/at24.yaml b/Documentation/devicetree/bindings/eeprom/at24.yaml
index e8778560d966..c04a3b8a6eba 100644
--- a/Documentation/devicetree/bindings/eeprom/at24.yaml
+++ b/Documentation/devicetree/bindings/eeprom/at24.yaml
@@ -167,6 +167,10 @@ properties:
minimum: 1
maximum: 8
+ vcc-supply:
+ description:
+ phandle of the regulator that provides the supply voltage.
+
required:
- compatible
- reg
--
2.18.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v9 2/4] dt-binding: i2c: add bus-supply property
2019-12-16 8:04 [PATCH v9 0/4] add power control in i2c and at24 Bibby Hsieh
2019-12-16 8:04 ` [PATCH v9 1/4] dt-binding: eeprom: at24: add vcc-supply property Bibby Hsieh
@ 2019-12-16 8:04 ` Bibby Hsieh
2019-12-18 14:47 ` Rob Herring
2019-12-16 8:04 ` [PATCH v9 3/4] misc: eeprom: at24: support pm_runtime control Bibby Hsieh
2019-12-16 8:04 ` [PATCH v9 4/4] i2c: core: support bus regulator controlling in adapter Bibby Hsieh
3 siblings, 1 reply; 9+ messages in thread
From: Bibby Hsieh @ 2019-12-16 8:04 UTC (permalink / raw)
To: Wolfram Sang, Bartosz Golaszewski, linux-i2c
Cc: tfiga, drinkcat, srv_heupstream, robh+dt, mark.rutland,
devicetree, Bibby Hsieh
In some platforms, they disable the power-supply of i2c due
to power consumption reduction. This patch add bus-supply property.
Signed-off-by: Bibby Hsieh <bibby.hsieh@mediatek.com>
---
Documentation/devicetree/bindings/i2c/i2c.txt | 3 +++
1 file changed, 3 insertions(+)
diff --git a/Documentation/devicetree/bindings/i2c/i2c.txt b/Documentation/devicetree/bindings/i2c/i2c.txt
index 9a53df4243c6..89d827805e89 100644
--- a/Documentation/devicetree/bindings/i2c/i2c.txt
+++ b/Documentation/devicetree/bindings/i2c/i2c.txt
@@ -98,6 +98,9 @@ wants to support one of the below features, it should adapt the bindings below.
Names of map programmable addresses.
It can contain any map needing another address than default one.
+- bus-supply
+ phandle to the regulator that provides power to i2c.
+
Binding may contain optional "interrupts" property, describing interrupts
used by the device. I2C core will assign "irq" interrupt (or the very first
interrupt if not using interrupt names) as primary interrupt for the slave.
--
2.18.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH v9 2/4] dt-binding: i2c: add bus-supply property
2019-12-16 8:04 ` [PATCH v9 2/4] dt-binding: i2c: add bus-supply property Bibby Hsieh
@ 2019-12-18 14:47 ` Rob Herring
0 siblings, 0 replies; 9+ messages in thread
From: Rob Herring @ 2019-12-18 14:47 UTC (permalink / raw)
To: Bibby Hsieh
Cc: Wolfram Sang, Bartosz Golaszewski, linux-i2c, tfiga, drinkcat,
srv_heupstream, robh+dt, mark.rutland, devicetree, Bibby Hsieh
On Mon, 16 Dec 2019 16:04:43 +0800, Bibby Hsieh wrote:
> In some platforms, they disable the power-supply of i2c due
> to power consumption reduction. This patch add bus-supply property.
>
> Signed-off-by: Bibby Hsieh <bibby.hsieh@mediatek.com>
> ---
> Documentation/devicetree/bindings/i2c/i2c.txt | 3 +++
> 1 file changed, 3 insertions(+)
>
Reviewed-by: Rob Herring <robh@kernel.org>
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v9 3/4] misc: eeprom: at24: support pm_runtime control
2019-12-16 8:04 [PATCH v9 0/4] add power control in i2c and at24 Bibby Hsieh
2019-12-16 8:04 ` [PATCH v9 1/4] dt-binding: eeprom: at24: add vcc-supply property Bibby Hsieh
2019-12-16 8:04 ` [PATCH v9 2/4] dt-binding: i2c: add bus-supply property Bibby Hsieh
@ 2019-12-16 8:04 ` Bibby Hsieh
2019-12-19 10:50 ` Bartosz Golaszewski
2019-12-16 8:04 ` [PATCH v9 4/4] i2c: core: support bus regulator controlling in adapter Bibby Hsieh
3 siblings, 1 reply; 9+ messages in thread
From: Bibby Hsieh @ 2019-12-16 8:04 UTC (permalink / raw)
To: Wolfram Sang, Bartosz Golaszewski, linux-i2c
Cc: tfiga, drinkcat, srv_heupstream, robh+dt, mark.rutland,
devicetree, Bibby Hsieh
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 | 38 ++++++++++++++++++++++++++++++++++++++
1 file changed, 38 insertions(+)
diff --git a/drivers/misc/eeprom/at24.c b/drivers/misc/eeprom/at24.c
index 0681d5fdd538..6ba23a7e4da1 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,10 @@ 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))
+ 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 +707,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 vcc regulator\n");
+ return err;
+ }
+
/* enable runtime pm */
pm_runtime_set_active(dev);
pm_runtime_enable(dev);
@@ -713,6 +725,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 +742,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),
},
--
2.18.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH v9 3/4] misc: eeprom: at24: support pm_runtime control
2019-12-16 8:04 ` [PATCH v9 3/4] misc: eeprom: at24: support pm_runtime control Bibby Hsieh
@ 2019-12-19 10:50 ` Bartosz Golaszewski
2020-01-06 7:09 ` Bibby Hsieh
0 siblings, 1 reply; 9+ messages in thread
From: Bartosz Golaszewski @ 2019-12-19 10:50 UTC (permalink / raw)
To: Bibby Hsieh
Cc: Wolfram Sang, linux-i2c, Tomasz Figa, Nicolas Boichat,
srv_heupstream, Rob Herring, Mark Rutland, linux-devicetree
pon., 16 gru 2019 o 09:04 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 | 38 ++++++++++++++++++++++++++++++++++++++
> 1 file changed, 38 insertions(+)
>
> diff --git a/drivers/misc/eeprom/at24.c b/drivers/misc/eeprom/at24.c
> index 0681d5fdd538..6ba23a7e4da1 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,10 @@ 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))
> + 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 +707,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 vcc regulator\n");
> + return err;
> + }
> +
> /* enable runtime pm */
> pm_runtime_set_active(dev);
> pm_runtime_enable(dev);
> @@ -713,6 +725,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 +742,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);
Did you at least *try* to compile this? Because if you did, you'd
notice it doesn't build.
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
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v9 3/4] misc: eeprom: at24: support pm_runtime control
2019-12-19 10:50 ` Bartosz Golaszewski
@ 2020-01-06 7:09 ` Bibby Hsieh
0 siblings, 0 replies; 9+ messages in thread
From: Bibby Hsieh @ 2020-01-06 7:09 UTC (permalink / raw)
To: Bartosz Golaszewski
Cc: Wolfram Sang, linux-i2c, Tomasz Figa, Nicolas Boichat,
srv_heupstream, Rob Herring, Mark Rutland, linux-devicetree
On Thu, 2019-12-19 at 11:50 +0100, Bartosz Golaszewski wrote:
> pon., 16 gru 2019 o 09:04 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 | 38 ++++++++++++++++++++++++++++++++++++++
> > 1 file changed, 38 insertions(+)
> >
> > diff --git a/drivers/misc/eeprom/at24.c b/drivers/misc/eeprom/at24.c
> > index 0681d5fdd538..6ba23a7e4da1 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,10 @@ 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))
> > + 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 +707,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 vcc regulator\n");
> > + return err;
> > + }
> > +
> > /* enable runtime pm */
> > pm_runtime_set_active(dev);
> > pm_runtime_enable(dev);
> > @@ -713,6 +725,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 +742,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);
>
> Did you at least *try* to compile this? Because if you did, you'd
> notice it doesn't build.
>
> Bart
Yep, I try to compile them... but I forget to enable at24 in
defconfig... Sorry, I will fix them in my next version.
Bibby
>
> > 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
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v9 4/4] i2c: core: support bus regulator controlling in adapter
2019-12-16 8:04 [PATCH v9 0/4] add power control in i2c and at24 Bibby Hsieh
` (2 preceding siblings ...)
2019-12-16 8:04 ` [PATCH v9 3/4] misc: eeprom: at24: support pm_runtime control Bibby Hsieh
@ 2019-12-16 8:04 ` Bibby Hsieh
2019-12-16 11:58 ` Tomasz Figa
3 siblings, 1 reply; 9+ messages in thread
From: Bibby Hsieh @ 2019-12-16 8:04 UTC (permalink / raw)
To: Wolfram Sang, Bartosz Golaszewski, linux-i2c
Cc: tfiga, drinkcat, srv_heupstream, robh+dt, mark.rutland,
devicetree, Bibby Hsieh
Although in the most platforms, the bus power of i2c
are alway on, some platforms disable the i2c bus power
in order to meet low power request.
We get and enable bulk regulator in i2c adapter device.
Signed-off-by: Bibby Hsieh <bibby.hsieh@mediatek.com>
---
drivers/i2c/i2c-core-base.c | 65 +++++++++++++++++++++++++++++++++++++
include/linux/i2c.h | 3 ++
2 files changed, 68 insertions(+)
diff --git a/drivers/i2c/i2c-core-base.c b/drivers/i2c/i2c-core-base.c
index 9333c865d4a9..e95ebd0af200 100644
--- a/drivers/i2c/i2c-core-base.c
+++ b/drivers/i2c/i2c-core-base.c
@@ -306,6 +306,7 @@ static int i2c_smbus_host_notify_to_irq(const struct i2c_client *client)
static int i2c_device_probe(struct device *dev)
{
struct i2c_client *client = i2c_verify_client(dev);
+ struct i2c_adapter *adap = client->adapter;
struct i2c_driver *driver;
int status;
@@ -371,6 +372,12 @@ static int i2c_device_probe(struct device *dev)
dev_dbg(dev, "probe\n");
+ status = regulator_enable(adap->bus_reg);
+ if (status != 0) {
+ dev_err(&adap->dev, "Failed to enable power regulator\n");
+ goto err_clear_wakeup_irq;
+ }
+
status = of_clk_set_defaults(dev->of_node, false);
if (status < 0)
goto err_clear_wakeup_irq;
@@ -407,6 +414,7 @@ static int i2c_device_probe(struct device *dev)
static int i2c_device_remove(struct device *dev)
{
struct i2c_client *client = i2c_verify_client(dev);
+ struct i2c_adapter *adap = client->adapter;
struct i2c_driver *driver;
int status = 0;
@@ -420,6 +428,8 @@ static int i2c_device_remove(struct device *dev)
}
dev_pm_domain_detach(&client->dev, true);
+ if (!pm_runtime_status_suspended(&adap->dev))
+ regulator_disable(adap->bus_reg);
dev_pm_clear_wake_irq(&client->dev);
device_init_wakeup(&client->dev, false);
@@ -431,6 +441,54 @@ static int i2c_device_remove(struct device *dev)
return status;
}
+#ifdef CONFIG_PM_SLEEP
+static int i2c_resume(struct device *dev)
+{
+ struct i2c_client *client = i2c_verify_client(dev);
+ struct i2c_adapter *adap = client->adapter;
+
+ return pm_generic_resume(&adap->dev);
+}
+
+static int i2c_suspend(struct device *dev)
+{
+ struct i2c_client *client = i2c_verify_client(dev);
+ struct i2c_adapter *adap = client->adapter;
+
+ return pm_generic_suspend(&adap->dev);
+}
+#endif
+
+#ifdef CONFIG_PM
+static int i2c_runtime_resume(struct device *dev)
+{
+ struct i2c_client *client = i2c_verify_client(dev);
+ struct i2c_adapter *adap = client->adapter;
+
+ pm_generic_runtime_resume(&adap->dev);
+
+ return regulator_enable(adap->bus_reg);
+}
+
+static int i2c_runtime_suspend(struct device *dev)
+{
+ struct i2c_client *client = i2c_verify_client(dev);
+ struct i2c_adapter *adap = client->adapter;
+
+ pm_generic_runtime_suspend(&adap->dev);
+
+ if (!pm_runtime_status_suspended(&adap->dev))
+ return regulator_disable(client->adapter->bus_reg);
+
+ return 0;
+}
+#endif
+
+static const struct dev_pm_ops i2c_device_pm = {
+ SET_SYSTEM_SLEEP_PM_OPS(i2c_suspend, i2c_resume)
+ SET_RUNTIME_PM_OPS(i2c_runtime_suspend, i2c_runtime_resume, NULL)
+};
+
static void i2c_device_shutdown(struct device *dev)
{
struct i2c_client *client = i2c_verify_client(dev);
@@ -488,6 +546,7 @@ struct bus_type i2c_bus_type = {
.probe = i2c_device_probe,
.remove = i2c_device_remove,
.shutdown = i2c_device_shutdown,
+ .pm = &i2c_device_pm,
};
EXPORT_SYMBOL_GPL(i2c_bus_type);
@@ -1351,6 +1410,11 @@ static int i2c_register_adapter(struct i2c_adapter *adap)
goto out_reg;
dev_dbg(&adap->dev, "adapter [%s] registered\n", adap->name);
+ adap->bus_reg = devm_regulator_get(&adap->dev, "bus");
+ if (IS_ERR(adap->bus_reg)) {
+ res = PTR_ERR(adap->bus_reg);
+ goto out_reg;
+ }
pm_runtime_no_callbacks(&adap->dev);
pm_suspend_ignore_children(&adap->dev, true);
@@ -1580,6 +1644,7 @@ void i2c_del_adapter(struct i2c_adapter *adap)
dev_dbg(&adap->dev, "adapter [%s] unregistered\n", adap->name);
pm_runtime_disable(&adap->dev);
+ devm_regulator_put(adap->bus_reg);
i2c_host_notify_irq_teardown(adap);
diff --git a/include/linux/i2c.h b/include/linux/i2c.h
index d2f786706657..833b81a680da 100644
--- a/include/linux/i2c.h
+++ b/include/linux/i2c.h
@@ -15,6 +15,7 @@
#include <linux/device.h> /* for struct device */
#include <linux/sched.h> /* for completion */
#include <linux/mutex.h>
+#include <linux/regulator/consumer.h>
#include <linux/rtmutex.h>
#include <linux/irqdomain.h> /* for Host Notify IRQ */
#include <linux/of.h> /* for struct device_node */
@@ -330,6 +331,7 @@ struct i2c_client {
int init_irq; /* irq set at initialization */
int irq; /* irq issued by device */
struct list_head detected;
+
#if IS_ENABLED(CONFIG_I2C_SLAVE)
i2c_slave_cb_t slave_cb; /* callback for slave mode */
#endif
@@ -723,6 +725,7 @@ struct i2c_adapter {
const struct i2c_adapter_quirks *quirks;
struct irq_domain *host_notify_domain;
+ struct regulator *bus_reg;
};
#define to_i2c_adapter(d) container_of(d, struct i2c_adapter, dev)
--
2.18.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH v9 4/4] i2c: core: support bus regulator controlling in adapter
2019-12-16 8:04 ` [PATCH v9 4/4] i2c: core: support bus regulator controlling in adapter Bibby Hsieh
@ 2019-12-16 11:58 ` Tomasz Figa
0 siblings, 0 replies; 9+ messages in thread
From: Tomasz Figa @ 2019-12-16 11:58 UTC (permalink / raw)
To: Bibby Hsieh
Cc: Wolfram Sang, Bartosz Golaszewski, linux-i2c, Nicolas Boichat,
srv_heupstream, Rob Herring, Mark Rutland, linux-devicetree
Hi Bibby,
On Mon, Dec 16, 2019 at 5:04 PM Bibby Hsieh <bibby.hsieh@mediatek.com> wrote:
>
> Although in the most platforms, the bus power of i2c
> are alway on, some platforms disable the i2c bus power
> in order to meet low power request.
>
> We get and enable bulk regulator in i2c adapter device.
>
> Signed-off-by: Bibby Hsieh <bibby.hsieh@mediatek.com>
> ---
> drivers/i2c/i2c-core-base.c | 65 +++++++++++++++++++++++++++++++++++++
> include/linux/i2c.h | 3 ++
> 2 files changed, 68 insertions(+)
>
Thanks for the patch! Please see my comments below.
> diff --git a/drivers/i2c/i2c-core-base.c b/drivers/i2c/i2c-core-base.c
> index 9333c865d4a9..e95ebd0af200 100644
> --- a/drivers/i2c/i2c-core-base.c
> +++ b/drivers/i2c/i2c-core-base.c
> @@ -306,6 +306,7 @@ static int i2c_smbus_host_notify_to_irq(const struct i2c_client *client)
> static int i2c_device_probe(struct device *dev)
> {
> struct i2c_client *client = i2c_verify_client(dev);
> + struct i2c_adapter *adap = client->adapter;
> struct i2c_driver *driver;
> int status;
>
> @@ -371,6 +372,12 @@ static int i2c_device_probe(struct device *dev)
>
> dev_dbg(dev, "probe\n");
>
> + status = regulator_enable(adap->bus_reg);
> + if (status != 0) {
> + dev_err(&adap->dev, "Failed to enable power regulator\n");
> + goto err_clear_wakeup_irq;
> + }
> +
> status = of_clk_set_defaults(dev->of_node, false);
> if (status < 0)
> goto err_clear_wakeup_irq;
> @@ -407,6 +414,7 @@ static int i2c_device_probe(struct device *dev)
> static int i2c_device_remove(struct device *dev)
> {
> struct i2c_client *client = i2c_verify_client(dev);
> + struct i2c_adapter *adap = client->adapter;
> struct i2c_driver *driver;
> int status = 0;
>
> @@ -420,6 +428,8 @@ static int i2c_device_remove(struct device *dev)
> }
>
> dev_pm_domain_detach(&client->dev, true);
> + if (!pm_runtime_status_suspended(&adap->dev))
> + regulator_disable(adap->bus_reg);
>
> dev_pm_clear_wake_irq(&client->dev);
> device_init_wakeup(&client->dev, false);
> @@ -431,6 +441,54 @@ static int i2c_device_remove(struct device *dev)
> return status;
> }
>
> +#ifdef CONFIG_PM_SLEEP
> +static int i2c_resume(struct device *dev)
> +{
> + struct i2c_client *client = i2c_verify_client(dev);
> + struct i2c_adapter *adap = client->adapter;
We need to ensure here that if the slave device was not runtime
suspended before the system suspend, the regulator is enabled before
the slave's resume callback is called.
> +
> + return pm_generic_resume(&adap->dev);
> +}
> +
> +static int i2c_suspend(struct device *dev)
> +{
> + struct i2c_client *client = i2c_verify_client(dev);
> + struct i2c_adapter *adap = client->adapter;
> +
> + return pm_generic_suspend(&adap->dev);
We need to ensure that the regulator is disabled when the system suspends.
> +}
> +#endif
> +
> +#ifdef CONFIG_PM
> +static int i2c_runtime_resume(struct device *dev)
> +{
> + struct i2c_client *client = i2c_verify_client(dev);
> + struct i2c_adapter *adap = client->adapter;
> +
> + pm_generic_runtime_resume(&adap->dev);
Why adap->dev? This callback is expected to execute a PM operation on
the I2C slave device.
Also, don't we need some error handling here?
> +
> + return regulator_enable(adap->bus_reg);
> +}
> +
> +static int i2c_runtime_suspend(struct device *dev)
> +{
> + struct i2c_client *client = i2c_verify_client(dev);
> + struct i2c_adapter *adap = client->adapter;
> +
> + pm_generic_runtime_suspend(&adap->dev);
Ditto.
> +
> + if (!pm_runtime_status_suspended(&adap->dev))
Since we just executed a suspend operation on the device, how is it
possible that it isn't suspended?
> + return regulator_disable(client->adapter->bus_reg);
> +
> + return 0;
> +}
> +#endif
> +
> +static const struct dev_pm_ops i2c_device_pm = {
> + SET_SYSTEM_SLEEP_PM_OPS(i2c_suspend, i2c_resume)
> + SET_RUNTIME_PM_OPS(i2c_runtime_suspend, i2c_runtime_resume, NULL)
> +};
> +
> static void i2c_device_shutdown(struct device *dev)
> {
> struct i2c_client *client = i2c_verify_client(dev);
> @@ -488,6 +546,7 @@ struct bus_type i2c_bus_type = {
> .probe = i2c_device_probe,
> .remove = i2c_device_remove,
> .shutdown = i2c_device_shutdown,
> + .pm = &i2c_device_pm,
> };
> EXPORT_SYMBOL_GPL(i2c_bus_type);
>
> @@ -1351,6 +1410,11 @@ static int i2c_register_adapter(struct i2c_adapter *adap)
> goto out_reg;
>
> dev_dbg(&adap->dev, "adapter [%s] registered\n", adap->name);
> + adap->bus_reg = devm_regulator_get(&adap->dev, "bus");
> + if (IS_ERR(adap->bus_reg)) {
> + res = PTR_ERR(adap->bus_reg);
> + goto out_reg;
> + }
>
> pm_runtime_no_callbacks(&adap->dev);
> pm_suspend_ignore_children(&adap->dev, true);
> @@ -1580,6 +1644,7 @@ void i2c_del_adapter(struct i2c_adapter *adap)
> dev_dbg(&adap->dev, "adapter [%s] unregistered\n", adap->name);
>
> pm_runtime_disable(&adap->dev);
> + devm_regulator_put(adap->bus_reg);
>
> i2c_host_notify_irq_teardown(adap);
>
> diff --git a/include/linux/i2c.h b/include/linux/i2c.h
> index d2f786706657..833b81a680da 100644
> --- a/include/linux/i2c.h
> +++ b/include/linux/i2c.h
> @@ -15,6 +15,7 @@
> #include <linux/device.h> /* for struct device */
> #include <linux/sched.h> /* for completion */
> #include <linux/mutex.h>
> +#include <linux/regulator/consumer.h>
> #include <linux/rtmutex.h>
> #include <linux/irqdomain.h> /* for Host Notify IRQ */
> #include <linux/of.h> /* for struct device_node */
> @@ -330,6 +331,7 @@ struct i2c_client {
> int init_irq; /* irq set at initialization */
> int irq; /* irq issued by device */
> struct list_head detected;
> +
Unnecessary change.
Best regards,
Tomasz
^ permalink raw reply [flat|nested] 9+ messages in thread