Linux-i2c Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH v14 0/2] add power control in i2c
@ 2020-04-28  6:18 Bibby Hsieh
  2020-04-28  6:18 ` [PATCH v14 1/2] dt-binding: i2c: add bus-supply property Bibby Hsieh
  2020-04-28  6:18 ` [PATCH v14 2/2] i2c: core: support bus regulator controlling in adapter Bibby Hsieh
  0 siblings, 2 replies; 12+ messages in thread
From: Bibby Hsieh @ 2020-04-28  6:18 UTC (permalink / raw)
  To: Wolfram Sang, Bartosz Golaszewski, linux-i2c
  Cc: tfiga, drinkcat, srv_heupstream, robh+dt, mark.rutland,
	devicetree, Rafael J . Wysocki, Bibby Hsieh

Although in the most platforms, the power of eeprom
and i2c are alway on, some platforms disable the
eeprom and i2c power in order to meet low power request.

This patch add the pm_runtime ops to control power to
support all platforms.

Changes since v13:
 - fixup some logic error

Changes since v12:
 - rebase onto v5.7-rc1
 - change the property description in binding

Changes since v11:
 - use suspend_late/resume_early instead of suspend/resume
 - rebase onto v5.6-rc1

Changes since v10:
 - fixup some worng codes

Changes since v9:
 - fixup build error
 - remove redundant code

Changes since v8:
 - fixup some wrong code
 - remove redundant message

Changes since v7:
 - add binding describe supply property in i2c and at24.
 - move i2c bus supply control in i2c-core.
 - rebase onto v5.5-rc1

        [... snip ...]

Bibby Hsieh (2):
  dt-binding: i2c: add bus-supply property
  i2c: core: support bus regulator controlling in adapter

 Documentation/devicetree/bindings/i2c/i2c.txt |  3 +
 drivers/i2c/i2c-core-base.c                   | 82 +++++++++++++++++++
 include/linux/i2c.h                           |  2 +
 3 files changed, 87 insertions(+)

-- 
2.18.0

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH v14 1/2] dt-binding: i2c: add bus-supply property
  2020-04-28  6:18 [PATCH v14 0/2] add power control in i2c Bibby Hsieh
@ 2020-04-28  6:18 ` Bibby Hsieh
  2020-04-28 15:02   ` Rob Herring
  2020-05-19  6:53   ` Wolfram Sang
  2020-04-28  6:18 ` [PATCH v14 2/2] i2c: core: support bus regulator controlling in adapter Bibby Hsieh
  1 sibling, 2 replies; 12+ messages in thread
From: Bibby Hsieh @ 2020-04-28  6:18 UTC (permalink / raw)
  To: Wolfram Sang, Bartosz Golaszewski, linux-i2c
  Cc: tfiga, drinkcat, srv_heupstream, robh+dt, mark.rutland,
	devicetree, Rafael J . Wysocki, 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..e6b5aafc5dc9 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 SCL/SDA.
+
 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	[flat|nested] 12+ messages in thread

* [PATCH v14 2/2] i2c: core: support bus regulator controlling in adapter
  2020-04-28  6:18 [PATCH v14 0/2] add power control in i2c Bibby Hsieh
  2020-04-28  6:18 ` [PATCH v14 1/2] dt-binding: i2c: add bus-supply property Bibby Hsieh
@ 2020-04-28  6:18 ` Bibby Hsieh
  2020-04-28 11:25   ` Grygorii Strashko
                     ` (2 more replies)
  1 sibling, 3 replies; 12+ messages in thread
From: Bibby Hsieh @ 2020-04-28  6:18 UTC (permalink / raw)
  To: Wolfram Sang, Bartosz Golaszewski, linux-i2c
  Cc: tfiga, drinkcat, srv_heupstream, robh+dt, mark.rutland,
	devicetree, Rafael J . Wysocki, 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 | 82 +++++++++++++++++++++++++++++++++++++
 include/linux/i2c.h         |  2 +
 2 files changed, 84 insertions(+)

diff --git a/drivers/i2c/i2c-core-base.c b/drivers/i2c/i2c-core-base.c
index 5cc0b0ec5570..f81b42a4ed07 100644
--- a/drivers/i2c/i2c-core-base.c
+++ b/drivers/i2c/i2c-core-base.c
@@ -313,6 +313,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;
 
@@ -378,6 +379,12 @@ static int i2c_device_probe(struct device *dev)
 
 	dev_dbg(dev, "probe\n");
 
+	status = regulator_enable(adap->bus_regulator);
+	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;
@@ -414,6 +421,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;
 
@@ -427,6 +435,8 @@ static int i2c_device_remove(struct device *dev)
 	}
 
 	dev_pm_domain_detach(&client->dev, true);
+	if (!pm_runtime_status_suspended(&client->dev))
+		regulator_disable(adap->bus_regulator);
 
 	dev_pm_clear_wake_irq(&client->dev);
 	device_init_wakeup(&client->dev, false);
@@ -438,6 +448,72 @@ static int i2c_device_remove(struct device *dev)
 	return status;
 }
 
+#ifdef CONFIG_PM_SLEEP
+static int i2c_resume_early(struct device *dev)
+{
+	struct i2c_client *client = i2c_verify_client(dev);
+	struct i2c_adapter *adap = client->adapter;
+	int err;
+
+	if (!pm_runtime_status_suspended(&client->dev)) {
+		err = regulator_enable(adap->bus_regulator);
+		if (err)
+			return err;
+	}
+
+	return pm_generic_resume_early(&client->dev);
+}
+
+static int i2c_suspend_late(struct device *dev)
+{
+	struct i2c_client *client = i2c_verify_client(dev);
+	struct i2c_adapter *adap = client->adapter;
+	int err;
+
+	err = pm_generic_suspend_late(&client->dev);
+	if (err)
+		return err;
+
+	if (!pm_runtime_status_suspended(&client->dev))
+		return regulator_disable(adap->bus_regulator);
+
+	return err;
+}
+#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;
+	int err;
+
+	err = regulator_enable(adap->bus_regulator);
+	if (err)
+		return err;
+
+	return pm_generic_runtime_resume(&client->dev);
+}
+
+static int i2c_runtime_suspend(struct device *dev)
+{
+	struct i2c_client *client = i2c_verify_client(dev);
+	struct i2c_adapter *adap = client->adapter;
+	int err;
+
+	err = pm_generic_runtime_suspend(&client->dev);
+	if (err)
+		return err;
+
+	return regulator_disable(adap->bus_regulator);
+}
+#endif
+
+static const struct dev_pm_ops i2c_device_pm = {
+	SET_LATE_SYSTEM_SLEEP_PM_OPS(i2c_suspend_late, i2c_resume_early)
+	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);
@@ -495,6 +571,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);
 
@@ -1333,6 +1410,11 @@ static int i2c_register_adapter(struct i2c_adapter *adap)
 	if (res)
 		goto out_reg;
 
+	adap->bus_regulator = devm_regulator_get(&adap->dev, "bus");
+	if (IS_ERR(adap->bus_regulator)) {
+		res = PTR_ERR(adap->bus_regulator);
+		goto out_reg;
+	}
 	dev_dbg(&adap->dev, "adapter [%s] registered\n", adap->name);
 
 	pm_runtime_no_callbacks(&adap->dev);
diff --git a/include/linux/i2c.h b/include/linux/i2c.h
index 456fc17ecb1c..bc83af0d38d1 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 */
@@ -721,6 +722,7 @@ struct i2c_adapter {
 	const struct i2c_adapter_quirks *quirks;
 
 	struct irq_domain *host_notify_domain;
+	struct regulator *bus_regulator;
 };
 #define to_i2c_adapter(d) container_of(d, struct i2c_adapter, dev)
 
-- 
2.18.0

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v14 2/2] i2c: core: support bus regulator controlling in adapter
  2020-04-28  6:18 ` [PATCH v14 2/2] i2c: core: support bus regulator controlling in adapter Bibby Hsieh
@ 2020-04-28 11:25   ` Grygorii Strashko
  2020-04-28 11:44     ` Tomasz Figa
  2020-04-28 11:45   ` Tomasz Figa
  2020-05-19  6:59   ` Wolfram Sang
  2 siblings, 1 reply; 12+ messages in thread
From: Grygorii Strashko @ 2020-04-28 11:25 UTC (permalink / raw)
  To: Bibby Hsieh, Wolfram Sang, Bartosz Golaszewski, linux-i2c
  Cc: tfiga, drinkcat, srv_heupstream, robh+dt, mark.rutland,
	devicetree, Rafael J . Wysocki



On 28/04/2020 09:18, Bibby Hsieh 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 | 82 +++++++++++++++++++++++++++++++++++++
>   include/linux/i2c.h         |  2 +
>   2 files changed, 84 insertions(+)
> 
> diff --git a/drivers/i2c/i2c-core-base.c b/drivers/i2c/i2c-core-base.c
> index 5cc0b0ec5570..f81b42a4ed07 100644
> --- a/drivers/i2c/i2c-core-base.c
> +++ b/drivers/i2c/i2c-core-base.c
> @@ -313,6 +313,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;
>   
> @@ -378,6 +379,12 @@ static int i2c_device_probe(struct device *dev)
>   
>   	dev_dbg(dev, "probe\n");
>   
> +	status = regulator_enable(adap->bus_regulator);
> +	if (status < 0) {
> +		dev_err(&adap->dev, "Failed to enable power regulator\n");
> +		goto err_clear_wakeup_irq;
> +	}
> +

Sry, but this is confusing.
What if there is separate regulators for I2C device and bus/adapter?

I2C bus is transaction based and usually I2C bus drivers ensures that i2c bus is
in proper state to perform transaction. While I2C devices can be enable, configured and
function without actually interacting with I2C bus unless required (irq for example).

With you change any I2C device will enable and keep bus regulator on all the time it's active
even if there is no I2C interruptions.

Following the problem description it seems
  - i2c bus driver should get regulator and ensure it's enabled for the duration of transaction(s)
  - i2c device should get its own regulator (or the same if shared)  ensure it's enabled for
    the period device is active.


>   	status = of_clk_set_defaults(dev->of_node, false);
>   	if (status < 0)
>   		goto err_clear_wakeup_irq;
> @@ -414,6 +421,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;
>   
> @@ -427,6 +435,8 @@ static int i2c_device_remove(struct device *dev)
>   	}
>   
>   	dev_pm_domain_detach(&client->dev, true);
> +	if (!pm_runtime_status_suspended(&client->dev))
> +		regulator_disable(adap->bus_regulator);
>   
>   	dev_pm_clear_wake_irq(&client->dev);
>   	device_init_wakeup(&client->dev, false);
> @@ -438,6 +448,72 @@ static int i2c_device_remove(struct device *dev)
>   	return status;
>   }
>   
> +#ifdef CONFIG_PM_SLEEP
> +static int i2c_resume_early(struct device *dev)
> +{
> +	struct i2c_client *client = i2c_verify_client(dev);
> +	struct i2c_adapter *adap = client->adapter;
> +	int err;
> +
> +	if (!pm_runtime_status_suspended(&client->dev)) {
> +		err = regulator_enable(adap->bus_regulator);
> +		if (err)
> +			return err;
> +	}
> +
> +	return pm_generic_resume_early(&client->dev);
> +}
> +
> +static int i2c_suspend_late(struct device *dev)
> +{
> +	struct i2c_client *client = i2c_verify_client(dev);
> +	struct i2c_adapter *adap = client->adapter;
> +	int err;
> +
> +	err = pm_generic_suspend_late(&client->dev);
> +	if (err)
> +		return err;
> +
> +	if (!pm_runtime_status_suspended(&client->dev))
> +		return regulator_disable(adap->bus_regulator);
> +
> +	return err;
> +}
> +#endif

Have you considered pm_runtime_force_suspend/pm_runtime_force_resume?

> +
> +#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;
> +	int err;
> +
> +	err = regulator_enable(adap->bus_regulator);
> +	if (err)
> +		return err;
> +
> +	return pm_generic_runtime_resume(&client->dev);
> +}
> +
> +static int i2c_runtime_suspend(struct device *dev)
> +{
> +	struct i2c_client *client = i2c_verify_client(dev);
> +	struct i2c_adapter *adap = client->adapter;
> +	int err;
> +
> +	err = pm_generic_runtime_suspend(&client->dev);
> +	if (err)
> +		return err;
> +
> +	return regulator_disable(adap->bus_regulator);
> +}
> +#endif
> +
> +static const struct dev_pm_ops i2c_device_pm = {
> +	SET_LATE_SYSTEM_SLEEP_PM_OPS(i2c_suspend_late, i2c_resume_early)
> +	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);
> @@ -495,6 +571,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);
>   
> @@ -1333,6 +1410,11 @@ static int i2c_register_adapter(struct i2c_adapter *adap)
>   	if (res)
>   		goto out_reg;
>   
> +	adap->bus_regulator = devm_regulator_get(&adap->dev, "bus");
> +	if (IS_ERR(adap->bus_regulator)) {
> +		res = PTR_ERR(adap->bus_regulator);
> +		goto out_reg;
> +	}
>   	dev_dbg(&adap->dev, "adapter [%s] registered\n", adap->name);
>   
>   	pm_runtime_no_callbacks(&adap->dev);
> diff --git a/include/linux/i2c.h b/include/linux/i2c.h
> index 456fc17ecb1c..bc83af0d38d1 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 */
> @@ -721,6 +722,7 @@ struct i2c_adapter {
>   	const struct i2c_adapter_quirks *quirks;
>   
>   	struct irq_domain *host_notify_domain;
> +	struct regulator *bus_regulator;
>   };
>   #define to_i2c_adapter(d) container_of(d, struct i2c_adapter, dev)
>   
> 

-- 
Best regards,
grygorii

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v14 2/2] i2c: core: support bus regulator controlling in adapter
  2020-04-28 11:25   ` Grygorii Strashko
@ 2020-04-28 11:44     ` Tomasz Figa
  2020-04-28 12:15       ` Grygorii Strashko
  0 siblings, 1 reply; 12+ messages in thread
From: Tomasz Figa @ 2020-04-28 11:44 UTC (permalink / raw)
  To: Grygorii Strashko
  Cc: Bibby Hsieh, Wolfram Sang, Bartosz Golaszewski, linux-i2c,
	Nicolas Boichat, srv_heupstream, Rob Herring, Mark Rutland,
	linux-devicetree, Rafael J . Wysocki

On Tue, Apr 28, 2020 at 1:25 PM Grygorii Strashko
<grygorii.strashko@ti.com> wrote:
>
>
>
> On 28/04/2020 09:18, Bibby Hsieh 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 | 82 +++++++++++++++++++++++++++++++++++++
> >   include/linux/i2c.h         |  2 +
> >   2 files changed, 84 insertions(+)
> >
> > diff --git a/drivers/i2c/i2c-core-base.c b/drivers/i2c/i2c-core-base.c
> > index 5cc0b0ec5570..f81b42a4ed07 100644
> > --- a/drivers/i2c/i2c-core-base.c
> > +++ b/drivers/i2c/i2c-core-base.c
> > @@ -313,6 +313,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;
> >
> > @@ -378,6 +379,12 @@ static int i2c_device_probe(struct device *dev)
> >
> >       dev_dbg(dev, "probe\n");
> >
> > +     status = regulator_enable(adap->bus_regulator);
> > +     if (status < 0) {
> > +             dev_err(&adap->dev, "Failed to enable power regulator\n");
> > +             goto err_clear_wakeup_irq;
> > +     }
> > +
>
> Sry, but this is confusing.
> What if there is separate regulators for I2C device and bus/adapter?
>
> I2C bus is transaction based and usually I2C bus drivers ensures that i2c bus is
> in proper state to perform transaction. While I2C devices can be enable, configured and
> function without actually interacting with I2C bus unless required (irq for example).
>
> With you change any I2C device will enable and keep bus regulator on all the time it's active
> even if there is no I2C interruptions.

The I2C SDA/SCL lines must stay high for all the time the bus is idle.
The regulator in question here is exactly the regulator that drives
the voltage rail the SDA/SCL lines are pulled up to and so it needs to
be enabled all the time any slave device is active and expects the I2C
bus to be in a valid state.

>
> Following the problem description it seems
>   - i2c bus driver should get regulator and ensure it's enabled for the duration of transaction(s)

That's not necessary given the point below, because the slave driver
must not trigger any I2C transactions when the slave device is not
active.

>   - i2c device should get its own regulator (or the same if shared)  ensure it's enabled for
>     the period device is active.

This is a board design aspect and not specific to any particular I2C
slave device, so slave drivers should not be aware of it. This patch
exactly attempts to get this SDA/SCL regulator on behalf of the slave
device.

>
>
> >       status = of_clk_set_defaults(dev->of_node, false);
> >       if (status < 0)
> >               goto err_clear_wakeup_irq;
> > @@ -414,6 +421,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;
> >
> > @@ -427,6 +435,8 @@ static int i2c_device_remove(struct device *dev)
> >       }
> >
> >       dev_pm_domain_detach(&client->dev, true);
> > +     if (!pm_runtime_status_suspended(&client->dev))
> > +             regulator_disable(adap->bus_regulator);
> >
> >       dev_pm_clear_wake_irq(&client->dev);
> >       device_init_wakeup(&client->dev, false);
> > @@ -438,6 +448,72 @@ static int i2c_device_remove(struct device *dev)
> >       return status;
> >   }
> >
> > +#ifdef CONFIG_PM_SLEEP
> > +static int i2c_resume_early(struct device *dev)
> > +{
> > +     struct i2c_client *client = i2c_verify_client(dev);
> > +     struct i2c_adapter *adap = client->adapter;
> > +     int err;
> > +
> > +     if (!pm_runtime_status_suspended(&client->dev)) {
> > +             err = regulator_enable(adap->bus_regulator);
> > +             if (err)
> > +                     return err;
> > +     }
> > +
> > +     return pm_generic_resume_early(&client->dev);
> > +}
> > +
> > +static int i2c_suspend_late(struct device *dev)
> > +{
> > +     struct i2c_client *client = i2c_verify_client(dev);
> > +     struct i2c_adapter *adap = client->adapter;
> > +     int err;
> > +
> > +     err = pm_generic_suspend_late(&client->dev);
> > +     if (err)
> > +             return err;
> > +
> > +     if (!pm_runtime_status_suspended(&client->dev))
> > +             return regulator_disable(adap->bus_regulator);
> > +
> > +     return err;
> > +}
> > +#endif
>
> Have you considered pm_runtime_force_suspend/pm_runtime_force_resume?
>
> > +
> > +#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;
> > +     int err;
> > +
> > +     err = regulator_enable(adap->bus_regulator);
> > +     if (err)
> > +             return err;
> > +
> > +     return pm_generic_runtime_resume(&client->dev);
> > +}
> > +
> > +static int i2c_runtime_suspend(struct device *dev)
> > +{
> > +     struct i2c_client *client = i2c_verify_client(dev);
> > +     struct i2c_adapter *adap = client->adapter;
> > +     int err;
> > +
> > +     err = pm_generic_runtime_suspend(&client->dev);
> > +     if (err)
> > +             return err;
> > +
> > +     return regulator_disable(adap->bus_regulator);
> > +}
> > +#endif
> > +
> > +static const struct dev_pm_ops i2c_device_pm = {
> > +     SET_LATE_SYSTEM_SLEEP_PM_OPS(i2c_suspend_late, i2c_resume_early)
> > +     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);
> > @@ -495,6 +571,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);
> >
> > @@ -1333,6 +1410,11 @@ static int i2c_register_adapter(struct i2c_adapter *adap)
> >       if (res)
> >               goto out_reg;
> >
> > +     adap->bus_regulator = devm_regulator_get(&adap->dev, "bus");
> > +     if (IS_ERR(adap->bus_regulator)) {
> > +             res = PTR_ERR(adap->bus_regulator);
> > +             goto out_reg;
> > +     }
> >       dev_dbg(&adap->dev, "adapter [%s] registered\n", adap->name);
> >
> >       pm_runtime_no_callbacks(&adap->dev);
> > diff --git a/include/linux/i2c.h b/include/linux/i2c.h
> > index 456fc17ecb1c..bc83af0d38d1 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 */
> > @@ -721,6 +722,7 @@ struct i2c_adapter {
> >       const struct i2c_adapter_quirks *quirks;
> >
> >       struct irq_domain *host_notify_domain;
> > +     struct regulator *bus_regulator;
> >   };
> >   #define to_i2c_adapter(d) container_of(d, struct i2c_adapter, dev)
> >
> >
>
> --
> Best regards,
> grygorii

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v14 2/2] i2c: core: support bus regulator controlling in adapter
  2020-04-28  6:18 ` [PATCH v14 2/2] i2c: core: support bus regulator controlling in adapter Bibby Hsieh
  2020-04-28 11:25   ` Grygorii Strashko
@ 2020-04-28 11:45   ` Tomasz Figa
  2020-05-19  6:59   ` Wolfram Sang
  2 siblings, 0 replies; 12+ messages in thread
From: Tomasz Figa @ 2020-04-28 11:45 UTC (permalink / raw)
  To: Bibby Hsieh
  Cc: Wolfram Sang, Bartosz Golaszewski, linux-i2c, Nicolas Boichat,
	srv_heupstream, Rob Herring, Mark Rutland, linux-devicetree,
	Rafael J . Wysocki

On Tue, Apr 28, 2020 at 8:18 AM 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 | 82 +++++++++++++++++++++++++++++++++++++
>  include/linux/i2c.h         |  2 +
>  2 files changed, 84 insertions(+)
>

Reviewed-by: Tomasz Figa <tfiga@chromium.org>

Best regards,
Tomasz

> diff --git a/drivers/i2c/i2c-core-base.c b/drivers/i2c/i2c-core-base.c
> index 5cc0b0ec5570..f81b42a4ed07 100644
> --- a/drivers/i2c/i2c-core-base.c
> +++ b/drivers/i2c/i2c-core-base.c
> @@ -313,6 +313,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;
>
> @@ -378,6 +379,12 @@ static int i2c_device_probe(struct device *dev)
>
>         dev_dbg(dev, "probe\n");
>
> +       status = regulator_enable(adap->bus_regulator);
> +       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;
> @@ -414,6 +421,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;
>
> @@ -427,6 +435,8 @@ static int i2c_device_remove(struct device *dev)
>         }
>
>         dev_pm_domain_detach(&client->dev, true);
> +       if (!pm_runtime_status_suspended(&client->dev))
> +               regulator_disable(adap->bus_regulator);
>
>         dev_pm_clear_wake_irq(&client->dev);
>         device_init_wakeup(&client->dev, false);
> @@ -438,6 +448,72 @@ static int i2c_device_remove(struct device *dev)
>         return status;
>  }
>
> +#ifdef CONFIG_PM_SLEEP
> +static int i2c_resume_early(struct device *dev)
> +{
> +       struct i2c_client *client = i2c_verify_client(dev);
> +       struct i2c_adapter *adap = client->adapter;
> +       int err;
> +
> +       if (!pm_runtime_status_suspended(&client->dev)) {
> +               err = regulator_enable(adap->bus_regulator);
> +               if (err)
> +                       return err;
> +       }
> +
> +       return pm_generic_resume_early(&client->dev);
> +}
> +
> +static int i2c_suspend_late(struct device *dev)
> +{
> +       struct i2c_client *client = i2c_verify_client(dev);
> +       struct i2c_adapter *adap = client->adapter;
> +       int err;
> +
> +       err = pm_generic_suspend_late(&client->dev);
> +       if (err)
> +               return err;
> +
> +       if (!pm_runtime_status_suspended(&client->dev))
> +               return regulator_disable(adap->bus_regulator);
> +
> +       return err;
> +}
> +#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;
> +       int err;
> +
> +       err = regulator_enable(adap->bus_regulator);
> +       if (err)
> +               return err;
> +
> +       return pm_generic_runtime_resume(&client->dev);
> +}
> +
> +static int i2c_runtime_suspend(struct device *dev)
> +{
> +       struct i2c_client *client = i2c_verify_client(dev);
> +       struct i2c_adapter *adap = client->adapter;
> +       int err;
> +
> +       err = pm_generic_runtime_suspend(&client->dev);
> +       if (err)
> +               return err;
> +
> +       return regulator_disable(adap->bus_regulator);
> +}
> +#endif
> +
> +static const struct dev_pm_ops i2c_device_pm = {
> +       SET_LATE_SYSTEM_SLEEP_PM_OPS(i2c_suspend_late, i2c_resume_early)
> +       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);
> @@ -495,6 +571,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);
>
> @@ -1333,6 +1410,11 @@ static int i2c_register_adapter(struct i2c_adapter *adap)
>         if (res)
>                 goto out_reg;
>
> +       adap->bus_regulator = devm_regulator_get(&adap->dev, "bus");
> +       if (IS_ERR(adap->bus_regulator)) {
> +               res = PTR_ERR(adap->bus_regulator);
> +               goto out_reg;
> +       }
>         dev_dbg(&adap->dev, "adapter [%s] registered\n", adap->name);
>
>         pm_runtime_no_callbacks(&adap->dev);
> diff --git a/include/linux/i2c.h b/include/linux/i2c.h
> index 456fc17ecb1c..bc83af0d38d1 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 */
> @@ -721,6 +722,7 @@ struct i2c_adapter {
>         const struct i2c_adapter_quirks *quirks;
>
>         struct irq_domain *host_notify_domain;
> +       struct regulator *bus_regulator;
>  };
>  #define to_i2c_adapter(d) container_of(d, struct i2c_adapter, dev)
>
> --
> 2.18.0

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v14 2/2] i2c: core: support bus regulator controlling in adapter
  2020-04-28 11:44     ` Tomasz Figa
@ 2020-04-28 12:15       ` Grygorii Strashko
  0 siblings, 0 replies; 12+ messages in thread
From: Grygorii Strashko @ 2020-04-28 12:15 UTC (permalink / raw)
  To: Tomasz Figa
  Cc: Bibby Hsieh, Wolfram Sang, Bartosz Golaszewski, linux-i2c,
	Nicolas Boichat, srv_heupstream, Rob Herring, Mark Rutland,
	linux-devicetree, Rafael J . Wysocki



On 28/04/2020 14:44, Tomasz Figa wrote:
> On Tue, Apr 28, 2020 at 1:25 PM Grygorii Strashko
> <grygorii.strashko@ti.com> wrote:
>>
>>
>>
>> On 28/04/2020 09:18, Bibby Hsieh 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 | 82 +++++++++++++++++++++++++++++++++++++
>>>    include/linux/i2c.h         |  2 +
>>>    2 files changed, 84 insertions(+)
>>>
>>> diff --git a/drivers/i2c/i2c-core-base.c b/drivers/i2c/i2c-core-base.c
>>> index 5cc0b0ec5570..f81b42a4ed07 100644
>>> --- a/drivers/i2c/i2c-core-base.c
>>> +++ b/drivers/i2c/i2c-core-base.c
>>> @@ -313,6 +313,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;
>>>
>>> @@ -378,6 +379,12 @@ static int i2c_device_probe(struct device *dev)
>>>
>>>        dev_dbg(dev, "probe\n");
>>>
>>> +     status = regulator_enable(adap->bus_regulator);
>>> +     if (status < 0) {
>>> +             dev_err(&adap->dev, "Failed to enable power regulator\n");
>>> +             goto err_clear_wakeup_irq;
>>> +     }
>>> +
>>
>> Sry, but this is confusing.
>> What if there is separate regulators for I2C device and bus/adapter?
>>
>> I2C bus is transaction based and usually I2C bus drivers ensures that i2c bus is
>> in proper state to perform transaction. While I2C devices can be enable, configured and
>> function without actually interacting with I2C bus unless required (irq for example).
>>
>> With you change any I2C device will enable and keep bus regulator on all the time it's active
>> even if there is no I2C interruptions.
> 
> The I2C SDA/SCL lines must stay high for all the time the bus is idle.
> The regulator in question here is exactly the regulator that drives
> the voltage rail the SDA/SCL lines are pulled up to and so it needs to
> be enabled all the time any slave device is active and expects the I2C
> bus to be in a valid state.
> 
>>
>> Following the problem description it seems
>>    - i2c bus driver should get regulator and ensure it's enabled for the duration of transaction(s)
> 
> That's not necessary given the point below, because the slave driver
> must not trigger any I2C transactions when the slave device is not
> active.
> 
>>    - i2c device should get its own regulator (or the same if shared)  ensure it's enabled for
>>      the period device is active.
> 
> This is a board design aspect and not specific to any particular I2C
> slave device, so slave drivers should not be aware of it. This patch
> exactly attempts to get this SDA/SCL regulator on behalf of the slave
> device.

Thank you for your explanation.



-- 
Best regards,
grygorii

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v14 1/2] dt-binding: i2c: add bus-supply property
  2020-04-28  6:18 ` [PATCH v14 1/2] dt-binding: i2c: add bus-supply property Bibby Hsieh
@ 2020-04-28 15:02   ` Rob Herring
  2020-05-19  6:53   ` Wolfram Sang
  1 sibling, 0 replies; 12+ messages in thread
From: Rob Herring @ 2020-04-28 15:02 UTC (permalink / raw)
  To: Bibby Hsieh
  Cc: Wolfram Sang, Bartosz Golaszewski, linux-i2c, tfiga, drinkcat,
	srv_heupstream, robh+dt, mark.rutland, devicetree,
	Rafael J . Wysocki, Bibby Hsieh

On Tue, 28 Apr 2020 14:18:12 +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(+)
> 

Please add Acked-by/Reviewed-by tags when posting new versions. However,
there's no need to repost patches *only* to add the tags. The upstream
maintainer will do that for acks received on the version they apply.

If a tag was not added on purpose, please state why and what changed.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v14 1/2] dt-binding: i2c: add bus-supply property
  2020-04-28  6:18 ` [PATCH v14 1/2] dt-binding: i2c: add bus-supply property Bibby Hsieh
  2020-04-28 15:02   ` Rob Herring
@ 2020-05-19  6:53   ` Wolfram Sang
  2020-05-19  7:00     ` Wolfram Sang
  1 sibling, 1 reply; 12+ messages in thread
From: Wolfram Sang @ 2020-05-19  6:53 UTC (permalink / raw)
  To: Bibby Hsieh
  Cc: Bartosz Golaszewski, linux-i2c, tfiga, drinkcat, srv_heupstream,
	robh+dt, mark.rutland, devicetree, Rafael J . Wysocki


[-- Attachment #1: Type: text/plain, Size: 380 bytes --]

On Tue, Apr 28, 2020 at 02:18:12PM +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>

Applied to for-next, thanks! +1 for the comments Rob made, please add
given tags if you didn't make further changes.


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v14 2/2] i2c: core: support bus regulator controlling in adapter
  2020-04-28  6:18 ` [PATCH v14 2/2] i2c: core: support bus regulator controlling in adapter Bibby Hsieh
  2020-04-28 11:25   ` Grygorii Strashko
  2020-04-28 11:45   ` Tomasz Figa
@ 2020-05-19  6:59   ` Wolfram Sang
  2020-05-19  7:26     ` Bibby Hsieh
  2 siblings, 1 reply; 12+ messages in thread
From: Wolfram Sang @ 2020-05-19  6:59 UTC (permalink / raw)
  To: Bibby Hsieh
  Cc: Bartosz Golaszewski, linux-i2c, tfiga, drinkcat, srv_heupstream,
	robh+dt, mark.rutland, devicetree, Rafael J . Wysocki


[-- Attachment #1: Type: text/plain, Size: 1695 bytes --]

On Tue, Apr 28, 2020 at 02:18:13PM +0800, Bibby Hsieh 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 | 82 +++++++++++++++++++++++++++++++++++++
>  include/linux/i2c.h         |  2 +
>  2 files changed, 84 insertions(+)
> 
> diff --git a/drivers/i2c/i2c-core-base.c b/drivers/i2c/i2c-core-base.c
> index 5cc0b0ec5570..f81b42a4ed07 100644
> --- a/drivers/i2c/i2c-core-base.c
> +++ b/drivers/i2c/i2c-core-base.c
> @@ -313,6 +313,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;

You are accessing client before the NULL pointer check.


> @@ -414,6 +421,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;

Same here.

> +static int i2c_suspend_late(struct device *dev)
> +{
> +	struct i2c_client *client = i2c_verify_client(dev);
> +	struct i2c_adapter *adap = client->adapter;
> +	int err;
> +
> +	err = pm_generic_suspend_late(&client->dev);
> +	if (err)
> +		return err;
> +
> +	if (!pm_runtime_status_suspended(&client->dev))
> +		return regulator_disable(adap->bus_regulator);
> +
> +	return err;

Can be 'return 0'.


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v14 1/2] dt-binding: i2c: add bus-supply property
  2020-05-19  6:53   ` Wolfram Sang
@ 2020-05-19  7:00     ` Wolfram Sang
  0 siblings, 0 replies; 12+ messages in thread
From: Wolfram Sang @ 2020-05-19  7:00 UTC (permalink / raw)
  To: Bibby Hsieh
  Cc: Bartosz Golaszewski, linux-i2c, tfiga, drinkcat, srv_heupstream,
	robh+dt, mark.rutland, devicetree, Rafael J . Wysocki


[-- Attachment #1: Type: text/plain, Size: 532 bytes --]

On Tue, May 19, 2020 at 08:53:23AM +0200, Wolfram Sang wrote:
> On Tue, Apr 28, 2020 at 02:18:12PM +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>
> 
> Applied to for-next, thanks! +1 for the comments Rob made, please add
> given tags if you didn't make further changes.

Waiting for v15 because I found issues in patch 2, so remoeved again.


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v14 2/2] i2c: core: support bus regulator controlling in adapter
  2020-05-19  6:59   ` Wolfram Sang
@ 2020-05-19  7:26     ` Bibby Hsieh
  0 siblings, 0 replies; 12+ messages in thread
From: Bibby Hsieh @ 2020-05-19  7:26 UTC (permalink / raw)
  To: Wolfram Sang
  Cc: Bartosz Golaszewski, linux-i2c, tfiga, drinkcat, srv_heupstream,
	robh+dt, mark.rutland, devicetree, Rafael J . Wysocki

Hi, Wolfram,

Thanks for the reviewing, I will sent next version for fixing up them.

Bibby

On Tue, 2020-05-19 at 08:59 +0200, Wolfram Sang wrote:
> On Tue, Apr 28, 2020 at 02:18:13PM +0800, Bibby Hsieh 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 | 82 +++++++++++++++++++++++++++++++++++++
> >  include/linux/i2c.h         |  2 +
> >  2 files changed, 84 insertions(+)
> > 
> > diff --git a/drivers/i2c/i2c-core-base.c b/drivers/i2c/i2c-core-base.c
> > index 5cc0b0ec5570..f81b42a4ed07 100644
> > --- a/drivers/i2c/i2c-core-base.c
> > +++ b/drivers/i2c/i2c-core-base.c
> > @@ -313,6 +313,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;
> 
> You are accessing client before the NULL pointer check.
> 
> 
> > @@ -414,6 +421,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;
> 
> Same here.
> 
> > +static int i2c_suspend_late(struct device *dev)
> > +{
> > +	struct i2c_client *client = i2c_verify_client(dev);
> > +	struct i2c_adapter *adap = client->adapter;
> > +	int err;
> > +
> > +	err = pm_generic_suspend_late(&client->dev);
> > +	if (err)
> > +		return err;
> > +
> > +	if (!pm_runtime_status_suspended(&client->dev))
> > +		return regulator_disable(adap->bus_regulator);
> > +
> > +	return err;
> 
> Can be 'return 0'.
> 


^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, back to index

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-04-28  6:18 [PATCH v14 0/2] add power control in i2c Bibby Hsieh
2020-04-28  6:18 ` [PATCH v14 1/2] dt-binding: i2c: add bus-supply property Bibby Hsieh
2020-04-28 15:02   ` Rob Herring
2020-05-19  6:53   ` Wolfram Sang
2020-05-19  7:00     ` Wolfram Sang
2020-04-28  6:18 ` [PATCH v14 2/2] i2c: core: support bus regulator controlling in adapter Bibby Hsieh
2020-04-28 11:25   ` Grygorii Strashko
2020-04-28 11:44     ` Tomasz Figa
2020-04-28 12:15       ` Grygorii Strashko
2020-04-28 11:45   ` Tomasz Figa
2020-05-19  6:59   ` Wolfram Sang
2020-05-19  7:26     ` Bibby Hsieh

Linux-i2c Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-i2c/0 linux-i2c/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-i2c linux-i2c/ https://lore.kernel.org/linux-i2c \
		linux-i2c@vger.kernel.org
	public-inbox-index linux-i2c

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-i2c


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git