* [PATCH v15 0/2] add power control in i2c @ 2020-05-19 7:27 Bibby Hsieh 2020-05-19 7:27 ` [PATCH v15 1/2] dt-binding: i2c: add bus-supply property Bibby Hsieh 2020-05-19 7:27 ` [PATCH v15 2/2] i2c: core: support bus regulator controlling in adapter Bibby Hsieh 0 siblings, 2 replies; 12+ messages in thread From: Bibby Hsieh @ 2020-05-19 7:27 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 v14: - change the return value in normal condition - access the variable after NULL pointer checking - add ack tag 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 [... 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 | 84 +++++++++++++++++++ include/linux/i2c.h | 2 + 3 files changed, 89 insertions(+) -- 2.18.0 ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v15 1/2] dt-binding: i2c: add bus-supply property 2020-05-19 7:27 [PATCH v15 0/2] add power control in i2c Bibby Hsieh @ 2020-05-19 7:27 ` Bibby Hsieh 2020-05-19 8:48 ` Wolfram Sang 2020-05-19 7:27 ` [PATCH v15 2/2] i2c: core: support bus regulator controlling in adapter Bibby Hsieh 1 sibling, 1 reply; 12+ messages in thread From: Bibby Hsieh @ 2020-05-19 7:27 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> Acked-by: Rob Herring <robh@kernel.org> --- 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 related [flat|nested] 12+ messages in thread
* Re: [PATCH v15 1/2] dt-binding: i2c: add bus-supply property 2020-05-19 7:27 ` [PATCH v15 1/2] dt-binding: i2c: add bus-supply property Bibby Hsieh @ 2020-05-19 8:48 ` Wolfram Sang 2020-05-22 14:59 ` Wolfram Sang 0 siblings, 1 reply; 12+ messages in thread From: Wolfram Sang @ 2020-05-19 8:48 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: 334 bytes --] On Tue, May 19, 2020 at 03:27:28PM +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> > Acked-by: Rob Herring <robh@kernel.org> Applied to for-next, thanks! [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v15 1/2] dt-binding: i2c: add bus-supply property 2020-05-19 8:48 ` Wolfram Sang @ 2020-05-22 14:59 ` Wolfram Sang 0 siblings, 0 replies; 12+ messages in thread From: Wolfram Sang @ 2020-05-22 14: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: 470 bytes --] On Tue, May 19, 2020 at 10:48:00AM +0200, Wolfram Sang wrote: > On Tue, May 19, 2020 at 03:27:28PM +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> > > Acked-by: Rob Herring <robh@kernel.org> > > Applied to for-next, thanks! Reverted because of regression reports in linux-next. [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v15 2/2] i2c: core: support bus regulator controlling in adapter 2020-05-19 7:27 [PATCH v15 0/2] add power control in i2c Bibby Hsieh 2020-05-19 7:27 ` [PATCH v15 1/2] dt-binding: i2c: add bus-supply property Bibby Hsieh @ 2020-05-19 7:27 ` Bibby Hsieh 2020-05-19 8:48 ` Wolfram Sang 2020-05-22 17:59 ` Grygorii Strashko 1 sibling, 2 replies; 12+ messages in thread From: Bibby Hsieh @ 2020-05-19 7:27 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 | 84 +++++++++++++++++++++++++++++++++++++ include/linux/i2c.h | 2 + 2 files changed, 86 insertions(+) diff --git a/drivers/i2c/i2c-core-base.c b/drivers/i2c/i2c-core-base.c index 5cc0b0ec5570..e1cc8d46bc51 100644 --- a/drivers/i2c/i2c-core-base.c +++ b/drivers/i2c/i2c-core-base.c @@ -313,12 +313,14 @@ 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; struct i2c_driver *driver; int status; if (!client) return 0; + adap = client->adapter; driver = to_i2c_driver(dev->driver); client->irq = client->init_irq; @@ -378,6 +380,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,12 +422,14 @@ 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; struct i2c_driver *driver; int status = 0; if (!client || !dev->driver) return 0; + adap = client->adapter; driver = to_i2c_driver(dev->driver); if (driver->remove) { dev_dbg(dev, "remove\n"); @@ -427,6 +437,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 +450,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 0; +} +#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 +573,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 +1412,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 related [flat|nested] 12+ messages in thread
* Re: [PATCH v15 2/2] i2c: core: support bus regulator controlling in adapter 2020-05-19 7:27 ` [PATCH v15 2/2] i2c: core: support bus regulator controlling in adapter Bibby Hsieh @ 2020-05-19 8:48 ` Wolfram Sang 2020-05-22 15:00 ` Wolfram Sang 2020-05-22 17:59 ` Grygorii Strashko 1 sibling, 1 reply; 12+ messages in thread From: Wolfram Sang @ 2020-05-19 8:48 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: 405 bytes --] On Tue, May 19, 2020 at 03:27:29PM +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> Applied to for-next, thanks! I added Tomasz Rev-by: for you. [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v15 2/2] i2c: core: support bus regulator controlling in adapter 2020-05-19 8:48 ` Wolfram Sang @ 2020-05-22 15:00 ` Wolfram Sang 2020-05-25 11:40 ` Tomasz Figa 0 siblings, 1 reply; 12+ messages in thread From: Wolfram Sang @ 2020-05-22 15: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: 642 bytes --] On Tue, May 19, 2020 at 10:48:33AM +0200, Wolfram Sang wrote: > On Tue, May 19, 2020 at 03:27:29PM +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> > > Applied to for-next, thanks! I added Tomasz Rev-by: for you. Reverted because of regression reports in linux-next. I am all open for the change in general, but it looks to me that we should wait another cycle. [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v15 2/2] i2c: core: support bus regulator controlling in adapter 2020-05-22 15:00 ` Wolfram Sang @ 2020-05-25 11:40 ` Tomasz Figa 2020-05-25 12:00 ` Wolfram Sang 0 siblings, 1 reply; 12+ messages in thread From: Tomasz Figa @ 2020-05-25 11:40 UTC (permalink / raw) To: Wolfram Sang, Bibby Hsieh Cc: Bartosz Golaszewski, linux-i2c, Nicolas Boichat, srv_heupstream, Rob Herring, Mark Rutland, linux-devicetree, Rafael J . Wysocki On Fri, May 22, 2020 at 5:00 PM Wolfram Sang <wsa@the-dreams.de> wrote: > > On Tue, May 19, 2020 at 10:48:33AM +0200, Wolfram Sang wrote: > > On Tue, May 19, 2020 at 03:27:29PM +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> > > > > Applied to for-next, thanks! I added Tomasz Rev-by: for you. > > Reverted because of regression reports in linux-next. I am all open for > the change in general, but it looks to me that we should wait another > cycle. > Thanks Wolfram and sorry for the trouble. Bibby, I think we need a bit more thorough testing. We can discuss the details offline. Best regards, Tomasz ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v15 2/2] i2c: core: support bus regulator controlling in adapter 2020-05-25 11:40 ` Tomasz Figa @ 2020-05-25 12:00 ` Wolfram Sang 0 siblings, 0 replies; 12+ messages in thread From: Wolfram Sang @ 2020-05-25 12:00 UTC (permalink / raw) To: Tomasz Figa Cc: Bibby Hsieh, Bartosz Golaszewski, linux-i2c, Nicolas Boichat, srv_heupstream, Rob Herring, Mark Rutland, linux-devicetree, Rafael J . Wysocki [-- Attachment #1: Type: text/plain, Size: 270 bytes --] > > Reverted because of regression reports in linux-next. I am all open for > > the change in general, but it looks to me that we should wait another > > cycle. > > > > Thanks Wolfram and sorry for the trouble. No worries. Glad you guys are working on it. [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v15 2/2] i2c: core: support bus regulator controlling in adapter 2020-05-19 7:27 ` [PATCH v15 2/2] i2c: core: support bus regulator controlling in adapter Bibby Hsieh 2020-05-19 8:48 ` Wolfram Sang @ 2020-05-22 17:59 ` Grygorii Strashko 2020-05-25 11:34 ` Tomasz Figa 1 sibling, 1 reply; 12+ messages in thread From: Grygorii Strashko @ 2020-05-22 17:59 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 19/05/2020 10:27, 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 | 84 +++++++++++++++++++++++++++++++++++++ > include/linux/i2c.h | 2 + > 2 files changed, 86 insertions(+) > > diff --git a/drivers/i2c/i2c-core-base.c b/drivers/i2c/i2c-core-base.c > index 5cc0b0ec5570..e1cc8d46bc51 100644 > --- a/drivers/i2c/i2c-core-base.c > +++ b/drivers/i2c/i2c-core-base.c > @@ -313,12 +313,14 @@ 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; > struct i2c_driver *driver; > int status; > > if (!client) > return 0; > > + adap = client->adapter; > driver = to_i2c_driver(dev->driver); > > client->irq = client->init_irq; > @@ -378,6 +380,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,12 +422,14 @@ 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; > struct i2c_driver *driver; > int status = 0; > > if (!client || !dev->driver) > return 0; > > + adap = client->adapter; > driver = to_i2c_driver(dev->driver); > if (driver->remove) { > dev_dbg(dev, "remove\n"); > @@ -427,6 +437,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); Not sure this check is correct. i2c_device_probe() - regulator_enable - 1 pm_runtime_get() - regulator_enable - 2 i2c_device_remove() - pm_runtime_status_suspended() flase - regulator_disable() - 1 --> still active? Sorry, I probably missing smth. > > dev_pm_clear_wake_irq(&client->dev); > device_init_wakeup(&client->dev, false); > @@ -438,6 +450,72 @@ static int i2c_device_remove(struct device *dev) > return status; > } > [...] -- Best regards, grygorii ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v15 2/2] i2c: core: support bus regulator controlling in adapter 2020-05-22 17:59 ` Grygorii Strashko @ 2020-05-25 11:34 ` Tomasz Figa 2020-05-26 18:03 ` Grygorii Strashko 0 siblings, 1 reply; 12+ messages in thread From: Tomasz Figa @ 2020-05-25 11:34 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 Hi Grygorii, On Fri, May 22, 2020 at 7:59 PM Grygorii Strashko <grygorii.strashko@ti.com> wrote: > > > > On 19/05/2020 10:27, 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 | 84 +++++++++++++++++++++++++++++++++++++ > > include/linux/i2c.h | 2 + > > 2 files changed, 86 insertions(+) > > > > diff --git a/drivers/i2c/i2c-core-base.c b/drivers/i2c/i2c-core-base.c > > index 5cc0b0ec5570..e1cc8d46bc51 100644 > > --- a/drivers/i2c/i2c-core-base.c > > +++ b/drivers/i2c/i2c-core-base.c > > @@ -313,12 +313,14 @@ 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; > > struct i2c_driver *driver; > > int status; > > > > if (!client) > > return 0; > > > > + adap = client->adapter; > > driver = to_i2c_driver(dev->driver); > > > > client->irq = client->init_irq; > > @@ -378,6 +380,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,12 +422,14 @@ 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; > > struct i2c_driver *driver; > > int status = 0; > > > > if (!client || !dev->driver) > > return 0; > > > > + adap = client->adapter; > > driver = to_i2c_driver(dev->driver); > > if (driver->remove) { > > dev_dbg(dev, "remove\n"); > > @@ -427,6 +437,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); > > Not sure this check is correct. > > i2c_device_probe() > - regulator_enable - 1 > > pm_runtime_get() > - regulator_enable - 2 > I believe regulator_enable() wouldn't be called again, because the device was already active in probe. However, I've been having difficulties keeping track of runtime PM semantics under various circumstances (e.g. ACPI vs DT), so can't tell for sure anymore. > i2c_device_remove() > - pm_runtime_status_suspended() flase > - regulator_disable() - 1 --> still active? > > Sorry, I probably missing smth. > > > > > dev_pm_clear_wake_irq(&client->dev); > > device_init_wakeup(&client->dev, false); > > @@ -438,6 +450,72 @@ static int i2c_device_remove(struct device *dev) > > return status; > > } > > > > [...] > > -- > Best regards, > grygorii ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v15 2/2] i2c: core: support bus regulator controlling in adapter 2020-05-25 11:34 ` Tomasz Figa @ 2020-05-26 18:03 ` Grygorii Strashko 0 siblings, 0 replies; 12+ messages in thread From: Grygorii Strashko @ 2020-05-26 18:03 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 25/05/2020 14:34, Tomasz Figa wrote: > Hi Grygorii, > > On Fri, May 22, 2020 at 7:59 PM Grygorii Strashko > <grygorii.strashko@ti.com> wrote: >> >> >> >> On 19/05/2020 10:27, 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 | 84 +++++++++++++++++++++++++++++++++++++ >>> include/linux/i2c.h | 2 + >>> 2 files changed, 86 insertions(+) >>> >>> diff --git a/drivers/i2c/i2c-core-base.c b/drivers/i2c/i2c-core-base.c >>> index 5cc0b0ec5570..e1cc8d46bc51 100644 >>> --- a/drivers/i2c/i2c-core-base.c >>> +++ b/drivers/i2c/i2c-core-base.c >>> @@ -313,12 +313,14 @@ 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; >>> struct i2c_driver *driver; >>> int status; >>> >>> if (!client) >>> return 0; >>> >>> + adap = client->adapter; >>> driver = to_i2c_driver(dev->driver); >>> >>> client->irq = client->init_irq; >>> @@ -378,6 +380,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,12 +422,14 @@ 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; >>> struct i2c_driver *driver; >>> int status = 0; >>> >>> if (!client || !dev->driver) >>> return 0; >>> >>> + adap = client->adapter; >>> driver = to_i2c_driver(dev->driver); >>> if (driver->remove) { >>> dev_dbg(dev, "remove\n"); >>> @@ -427,6 +437,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); >> >> Not sure this check is correct. >> >> i2c_device_probe() >> - regulator_enable - 1 >> >> pm_runtime_get() >> - regulator_enable - 2 >> > > I believe regulator_enable() wouldn't be called again, because the > device was already active in probe. However, I've been having > difficulties keeping track of runtime PM semantics under various > circumstances (e.g. ACPI vs DT), so can't tell for sure anymore. True. I've found pretty useful: - CONFIG_PM_ADVANCED_DEBUG - bind/unbind for such testing. for regulators - num_users\x02 can be checked. > >> i2c_device_remove() >> - pm_runtime_status_suspended() flase >> - regulator_disable() - 1 --> still active? >> >> Sorry, I probably missing smth. >> >>> >>> dev_pm_clear_wake_irq(&client->dev); >>> device_init_wakeup(&client->dev, false); >>> @@ -438,6 +450,72 @@ static int i2c_device_remove(struct device *dev) >>> return status; >>> } >>> >> >> [...] >> >> -- >> Best regards, >> grygorii -- Best regards, grygorii ^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2020-05-26 18:03 UTC | newest] Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-05-19 7:27 [PATCH v15 0/2] add power control in i2c Bibby Hsieh 2020-05-19 7:27 ` [PATCH v15 1/2] dt-binding: i2c: add bus-supply property Bibby Hsieh 2020-05-19 8:48 ` Wolfram Sang 2020-05-22 14:59 ` Wolfram Sang 2020-05-19 7:27 ` [PATCH v15 2/2] i2c: core: support bus regulator controlling in adapter Bibby Hsieh 2020-05-19 8:48 ` Wolfram Sang 2020-05-22 15:00 ` Wolfram Sang 2020-05-25 11:40 ` Tomasz Figa 2020-05-25 12:00 ` Wolfram Sang 2020-05-22 17:59 ` Grygorii Strashko 2020-05-25 11:34 ` Tomasz Figa 2020-05-26 18:03 ` Grygorii Strashko
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.