Linux-i2c Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH v19 0/6] add power control in i2c
@ 2021-04-14 17:29 Hsin-Yi Wang
  2021-04-14 17:29 ` [PATCH v19 1/6] i2c: core: support bus regulator controlling in adapter Hsin-Yi Wang
                   ` (5 more replies)
  0 siblings, 6 replies; 12+ messages in thread
From: Hsin-Yi Wang @ 2021-04-14 17:29 UTC (permalink / raw)
  To: Wolfram Sang, Matthias Brugger, Rob Herring, Bartosz Golaszewski
  Cc: linux-i2c, Qii Wang, devicetree, linux-arm-kernel,
	linux-mediatek, linux-kernel, Greg Kroah-Hartman, Mark Brown,
	Marek Szyprowski, Bibby Hsieh, Arnd Bergmann, Jani Nikula,
	Joonas Lahtinen, intel-gfx

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 v18:
 - Fix a function name conflict with drivers/gpu/drm/i915/selftests/i915_gem.c

Changes since v17:
 - Add a patch to fix unbalanced regulator disabling.
 - Add dts patch.

Changes since v16:
 - request regulator in device instead of in the core.
 - control regulator only if it's provided.

Changes since v15:
 - Squash the fix[1] for v15.
[1] https://patchwork.ozlabs.org/project/linux-i2c/patch/20200522101327.13456-1-m.szyprowski@samsung.com/

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 (1):
  i2c: core: support bus regulator controlling in adapter

Hsin-Yi Wang (5):
  dt-binding: i2c: mt65xx: add vbus-supply property
  i2c: mediatek: mt65xx: add optional vbus-supply
  misc: eeprom: at24: check suspend status before disable regulator
  arm64: dts: mt8183: add supply name for eeprom
  drm/i915/selftests: Rename functions names

 .../devicetree/bindings/i2c/i2c-mt65xx.txt    |  1 +
 .../dts/mediatek/mt8183-kukui-kakadu.dtsi     |  4 +
 .../dts/mediatek/mt8183-kukui-kodama.dtsi     |  4 +
 .../boot/dts/mediatek/mt8183-kukui-krane.dtsi |  4 +
 drivers/gpu/drm/i915/selftests/i915_gem.c     | 10 +--
 drivers/i2c/busses/i2c-mt65xx.c               |  7 ++
 drivers/i2c/i2c-core-base.c                   | 88 +++++++++++++++++++
 drivers/misc/eeprom/at24.c                    |  6 +-
 include/linux/i2c.h                           |  2 +
 9 files changed, 119 insertions(+), 7 deletions(-)

-- 
2.31.1.295.g9ea45b61b8-goog


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

* [PATCH v19 1/6] i2c: core: support bus regulator controlling in adapter
  2021-04-14 17:29 [PATCH v19 0/6] add power control in i2c Hsin-Yi Wang
@ 2021-04-14 17:29 ` Hsin-Yi Wang
  2021-04-14 17:29 ` [PATCH v19 2/6] dt-binding: i2c: mt65xx: add vbus-supply property Hsin-Yi Wang
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 12+ messages in thread
From: Hsin-Yi Wang @ 2021-04-14 17:29 UTC (permalink / raw)
  To: Wolfram Sang, Matthias Brugger, Rob Herring, Bartosz Golaszewski
  Cc: linux-i2c, Qii Wang, devicetree, linux-arm-kernel,
	linux-mediatek, linux-kernel, Greg Kroah-Hartman, Mark Brown,
	Marek Szyprowski, Bibby Hsieh, Arnd Bergmann, Jani Nikula,
	Joonas Lahtinen, intel-gfx

From: Bibby Hsieh <bibby.hsieh@mediatek.com>

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 can control bulk regulator if it is provided in i2c
adapter device.

Signed-off-by: Bibby Hsieh <bibby.hsieh@mediatek.com>
Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
Signed-off-by: Hsin-Yi Wang <hsinyi@chromium.org>
---
 drivers/i2c/i2c-core-base.c | 88 +++++++++++++++++++++++++++++++++++++
 include/linux/i2c.h         |  2 +
 2 files changed, 90 insertions(+)

diff --git a/drivers/i2c/i2c-core-base.c b/drivers/i2c/i2c-core-base.c
index 24c8f11bac73..c34920f30c5a 100644
--- a/drivers/i2c/i2c-core-base.c
+++ b/drivers/i2c/i2c-core-base.c
@@ -461,12 +461,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;
 	client->irq = client->init_irq;
 
 	if (!client->irq) {
@@ -532,6 +534,14 @@ static int i2c_device_probe(struct device *dev)
 
 	dev_dbg(dev, "probe\n");
 
+	if (adap->bus_regulator) {
+		status = regulator_enable(adap->bus_regulator);
+		if (status < 0) {
+			dev_err(&adap->dev, "Failed to enable bus regulator\n");
+			goto err_clear_wakeup_irq;
+		}
+	}
+
 	status = of_clk_set_defaults(dev->of_node, false);
 	if (status < 0)
 		goto err_clear_wakeup_irq;
@@ -589,8 +599,10 @@ static int i2c_device_probe(struct device *dev)
 static int i2c_device_remove(struct device *dev)
 {
 	struct i2c_client	*client = to_i2c_client(dev);
+	struct i2c_adapter      *adap;
 	struct i2c_driver	*driver;
 
+	adap = client->adapter;
 	driver = to_i2c_driver(dev->driver);
 	if (driver->remove) {
 		int status;
@@ -605,6 +617,8 @@ static int i2c_device_remove(struct device *dev)
 	devres_release_group(&client->dev, client->devres_group_id);
 
 	dev_pm_domain_detach(&client->dev, true);
+	if (!pm_runtime_status_suspended(&client->dev) && adap->bus_regulator)
+		regulator_disable(adap->bus_regulator);
 
 	dev_pm_clear_wake_irq(&client->dev);
 	device_init_wakeup(&client->dev, false);
@@ -617,6 +631,79 @@ static int i2c_device_remove(struct device *dev)
 	return 0;
 }
 
+#ifdef CONFIG_PM_SLEEP
+static int i2c_resume_early(struct device *dev)
+{
+	struct i2c_client *client = i2c_verify_client(dev);
+	int err;
+
+	if (!client || !client->adapter->bus_regulator)
+		return 0;
+
+	if (!pm_runtime_status_suspended(&client->dev)) {
+		err = regulator_enable(client->adapter->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);
+	int err;
+
+	if (!client || !client->adapter->bus_regulator)
+		return 0;
+
+	err = pm_generic_suspend_late(&client->dev);
+	if (err)
+		return err;
+
+	if (!pm_runtime_status_suspended(&client->dev))
+		return regulator_disable(client->adapter->bus_regulator);
+
+	return 0;
+}
+#endif
+
+#ifdef CONFIG_PM
+static int i2c_runtime_resume(struct device *dev)
+{
+	struct i2c_client *client = i2c_verify_client(dev);
+	int err;
+
+	if (!client || !client->adapter->bus_regulator)
+		return 0;
+
+	err = regulator_enable(client->adapter->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);
+	int err;
+
+	if (!client || !client->adapter->bus_regulator)
+		return 0;
+
+	err = pm_generic_runtime_suspend(&client->dev);
+	if (err)
+		return err;
+
+	return regulator_disable(client->adapter->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);
@@ -674,6 +761,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);
 
diff --git a/include/linux/i2c.h b/include/linux/i2c.h
index e8f2ac8c9c3d..953a4eecb88f 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 */
@@ -729,6 +730,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.31.1.295.g9ea45b61b8-goog


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

* [PATCH v19 2/6] dt-binding: i2c: mt65xx: add vbus-supply property
  2021-04-14 17:29 [PATCH v19 0/6] add power control in i2c Hsin-Yi Wang
  2021-04-14 17:29 ` [PATCH v19 1/6] i2c: core: support bus regulator controlling in adapter Hsin-Yi Wang
@ 2021-04-14 17:29 ` Hsin-Yi Wang
  2021-04-14 22:19   ` [Intel-gfx] " Rob Herring
  2021-04-14 17:29 ` [PATCH v19 3/6] i2c: mediatek: mt65xx: add optional vbus-supply Hsin-Yi Wang
                   ` (3 subsequent siblings)
  5 siblings, 1 reply; 12+ messages in thread
From: Hsin-Yi Wang @ 2021-04-14 17:29 UTC (permalink / raw)
  To: Wolfram Sang, Matthias Brugger, Rob Herring, Bartosz Golaszewski
  Cc: linux-i2c, Qii Wang, devicetree, linux-arm-kernel,
	linux-mediatek, linux-kernel, Greg Kroah-Hartman, Mark Brown,
	Marek Szyprowski, Bibby Hsieh, Arnd Bergmann, Jani Nikula,
	Joonas Lahtinen, intel-gfx

Add vbus-supply property for mt65xx. The regulator can be passed into
core and turned off during suspend/sleep to reduce power consumption.

Signed-off-by: Hsin-Yi Wang <hsinyi@chromium.org>
---
 Documentation/devicetree/bindings/i2c/i2c-mt65xx.txt | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/i2c/i2c-mt65xx.txt b/Documentation/devicetree/bindings/i2c/i2c-mt65xx.txt
index 7f0194fdd0cc..2c45647e9f0b 100644
--- a/Documentation/devicetree/bindings/i2c/i2c-mt65xx.txt
+++ b/Documentation/devicetree/bindings/i2c/i2c-mt65xx.txt
@@ -32,6 +32,7 @@ Optional properties:
   - mediatek,have-pmic: platform can control i2c form special pmic side.
     Only mt6589 and mt8135 support this feature.
   - mediatek,use-push-pull: IO config use push-pull mode.
+  - vbus-supply: phandle to the regulator that provides power to SCL/SDA.
 
 Example:
 
-- 
2.31.1.295.g9ea45b61b8-goog


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

* [PATCH v19 3/6] i2c: mediatek: mt65xx: add optional vbus-supply
  2021-04-14 17:29 [PATCH v19 0/6] add power control in i2c Hsin-Yi Wang
  2021-04-14 17:29 ` [PATCH v19 1/6] i2c: core: support bus regulator controlling in adapter Hsin-Yi Wang
  2021-04-14 17:29 ` [PATCH v19 2/6] dt-binding: i2c: mt65xx: add vbus-supply property Hsin-Yi Wang
@ 2021-04-14 17:29 ` Hsin-Yi Wang
  2021-04-14 17:29 ` [PATCH v19 4/6] misc: eeprom: at24: check suspend status before disable regulator Hsin-Yi Wang
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 12+ messages in thread
From: Hsin-Yi Wang @ 2021-04-14 17:29 UTC (permalink / raw)
  To: Wolfram Sang, Matthias Brugger, Rob Herring, Bartosz Golaszewski
  Cc: linux-i2c, Qii Wang, devicetree, linux-arm-kernel,
	linux-mediatek, linux-kernel, Greg Kroah-Hartman, Mark Brown,
	Marek Szyprowski, Bibby Hsieh, Arnd Bergmann, Jani Nikula,
	Joonas Lahtinen, intel-gfx

Add vbus-supply which provides power to SCL/SDA. Pass this regulator
into core so it can be turned on/off for low power mode support.

Signed-off-by: Hsin-Yi Wang <hsinyi@chromium.org>
---
 drivers/i2c/busses/i2c-mt65xx.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/i2c/busses/i2c-mt65xx.c b/drivers/i2c/busses/i2c-mt65xx.c
index 2ffd2f354d0a..82f2b6716005 100644
--- a/drivers/i2c/busses/i2c-mt65xx.c
+++ b/drivers/i2c/busses/i2c-mt65xx.c
@@ -1215,6 +1215,13 @@ static int mtk_i2c_probe(struct platform_device *pdev)
 	i2c->adap.quirks = i2c->dev_comp->quirks;
 	i2c->adap.timeout = 2 * HZ;
 	i2c->adap.retries = 1;
+	i2c->adap.bus_regulator = devm_regulator_get_optional(&pdev->dev, "vbus");
+	if (IS_ERR(i2c->adap.bus_regulator)) {
+		if (PTR_ERR(i2c->adap.bus_regulator) == -ENODEV)
+			i2c->adap.bus_regulator = NULL;
+		else
+			return PTR_ERR(i2c->adap.bus_regulator);
+	}
 
 	ret = mtk_i2c_parse_dt(pdev->dev.of_node, i2c);
 	if (ret)
-- 
2.31.1.295.g9ea45b61b8-goog


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

* [PATCH v19 4/6] misc: eeprom: at24: check suspend status before disable regulator
  2021-04-14 17:29 [PATCH v19 0/6] add power control in i2c Hsin-Yi Wang
                   ` (2 preceding siblings ...)
  2021-04-14 17:29 ` [PATCH v19 3/6] i2c: mediatek: mt65xx: add optional vbus-supply Hsin-Yi Wang
@ 2021-04-14 17:29 ` Hsin-Yi Wang
  2021-04-16 14:09   ` Bartosz Golaszewski
  2021-04-14 17:29 ` [PATCH v19 5/6] arm64: dts: mt8183: add supply name for eeprom Hsin-Yi Wang
  2021-04-14 17:29 ` [PATCH v19 6/6] drm/i915/selftests: Rename functions names Hsin-Yi Wang
  5 siblings, 1 reply; 12+ messages in thread
From: Hsin-Yi Wang @ 2021-04-14 17:29 UTC (permalink / raw)
  To: Wolfram Sang, Matthias Brugger, Rob Herring, Bartosz Golaszewski
  Cc: linux-i2c, Qii Wang, devicetree, linux-arm-kernel,
	linux-mediatek, linux-kernel, Greg Kroah-Hartman, Mark Brown,
	Marek Szyprowski, Bibby Hsieh, Arnd Bergmann, Jani Nikula,
	Joonas Lahtinen, intel-gfx

cd5676db0574 ("misc: eeprom: at24: support pm_runtime control") disables
regulator in runtime suspend. If runtime suspend is called before
regulator disable, it will results in regulator unbalanced disabling.

Signed-off-by: Hsin-Yi Wang <hsinyi@chromium.org>
---
 drivers/misc/eeprom/at24.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/misc/eeprom/at24.c b/drivers/misc/eeprom/at24.c
index 926408b41270..7a6f01ace78a 100644
--- a/drivers/misc/eeprom/at24.c
+++ b/drivers/misc/eeprom/at24.c
@@ -763,7 +763,8 @@ static int at24_probe(struct i2c_client *client)
 	at24->nvmem = devm_nvmem_register(dev, &nvmem_config);
 	if (IS_ERR(at24->nvmem)) {
 		pm_runtime_disable(dev);
-		regulator_disable(at24->vcc_reg);
+		if (!pm_runtime_status_suspended(dev))
+			regulator_disable(at24->vcc_reg);
 		return PTR_ERR(at24->nvmem);
 	}
 
@@ -774,7 +775,8 @@ static int at24_probe(struct i2c_client *client)
 	err = at24_read(at24, 0, &test_byte, 1);
 	if (err) {
 		pm_runtime_disable(dev);
-		regulator_disable(at24->vcc_reg);
+		if (!pm_runtime_status_suspended(dev))
+			regulator_disable(at24->vcc_reg);
 		return -ENODEV;
 	}
 
-- 
2.31.1.295.g9ea45b61b8-goog


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

* [PATCH v19 5/6] arm64: dts: mt8183: add supply name for eeprom
  2021-04-14 17:29 [PATCH v19 0/6] add power control in i2c Hsin-Yi Wang
                   ` (3 preceding siblings ...)
  2021-04-14 17:29 ` [PATCH v19 4/6] misc: eeprom: at24: check suspend status before disable regulator Hsin-Yi Wang
@ 2021-04-14 17:29 ` Hsin-Yi Wang
  2021-04-14 17:29 ` [PATCH v19 6/6] drm/i915/selftests: Rename functions names Hsin-Yi Wang
  5 siblings, 0 replies; 12+ messages in thread
From: Hsin-Yi Wang @ 2021-04-14 17:29 UTC (permalink / raw)
  To: Wolfram Sang, Matthias Brugger, Rob Herring, Bartosz Golaszewski
  Cc: linux-i2c, Qii Wang, devicetree, linux-arm-kernel,
	linux-mediatek, linux-kernel, Greg Kroah-Hartman, Mark Brown,
	Marek Szyprowski, Bibby Hsieh, Arnd Bergmann, Jani Nikula,
	Joonas Lahtinen, intel-gfx

Add supplies for eeprom for mt8183 boards.

Signed-off-by: Hsin-Yi Wang <hsinyi@chromium.org>
---
 arch/arm64/boot/dts/mediatek/mt8183-kukui-kakadu.dtsi | 4 ++++
 arch/arm64/boot/dts/mediatek/mt8183-kukui-kodama.dtsi | 4 ++++
 arch/arm64/boot/dts/mediatek/mt8183-kukui-krane.dtsi  | 4 ++++
 3 files changed, 12 insertions(+)

diff --git a/arch/arm64/boot/dts/mediatek/mt8183-kukui-kakadu.dtsi b/arch/arm64/boot/dts/mediatek/mt8183-kukui-kakadu.dtsi
index b442e38a3156..28966a65391b 100644
--- a/arch/arm64/boot/dts/mediatek/mt8183-kukui-kakadu.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt8183-kukui-kakadu.dtsi
@@ -88,11 +88,13 @@ &i2c2 {
 	pinctrl-0 = <&i2c2_pins>;
 	status = "okay";
 	clock-frequency = <400000>;
+	vbus-supply = <&mt6358_vcamio_reg>;
 
 	eeprom@58 {
 		compatible = "atmel,24c32";
 		reg = <0x58>;
 		pagesize = <32>;
+		vcc-supply = <&mt6358_vcama2_reg>;
 	};
 };
 
@@ -101,11 +103,13 @@ &i2c4 {
 	pinctrl-0 = <&i2c4_pins>;
 	status = "okay";
 	clock-frequency = <400000>;
+	vbus-supply = <&mt6358_vcn18_reg>;
 
 	eeprom@54 {
 		compatible = "atmel,24c32";
 		reg = <0x54>;
 		pagesize = <32>;
+		vcc-supply = <&mt6358_vcn18_reg>;
 	};
 };
 
diff --git a/arch/arm64/boot/dts/mediatek/mt8183-kukui-kodama.dtsi b/arch/arm64/boot/dts/mediatek/mt8183-kukui-kodama.dtsi
index 2f5234a16ead..3aa79403c0c2 100644
--- a/arch/arm64/boot/dts/mediatek/mt8183-kukui-kodama.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt8183-kukui-kodama.dtsi
@@ -62,11 +62,13 @@ &i2c2 {
 	pinctrl-0 = <&i2c2_pins>;
 	status = "okay";
 	clock-frequency = <400000>;
+	vbus-supply = <&mt6358_vcamio_reg>;
 
 	eeprom@58 {
 		compatible = "atmel,24c64";
 		reg = <0x58>;
 		pagesize = <32>;
+		vcc-supply = <&mt6358_vcamio_reg>;
 	};
 };
 
@@ -75,11 +77,13 @@ &i2c4 {
 	pinctrl-0 = <&i2c4_pins>;
 	status = "okay";
 	clock-frequency = <400000>;
+	vbus-supply = <&mt6358_vcn18_reg>;
 
 	eeprom@54 {
 		compatible = "atmel,24c64";
 		reg = <0x54>;
 		pagesize = <32>;
+		vcc-supply = <&mt6358_vcn18_reg>;
 	};
 };
 
diff --git a/arch/arm64/boot/dts/mediatek/mt8183-kukui-krane.dtsi b/arch/arm64/boot/dts/mediatek/mt8183-kukui-krane.dtsi
index fbc471ccf805..30c183c96a54 100644
--- a/arch/arm64/boot/dts/mediatek/mt8183-kukui-krane.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt8183-kukui-krane.dtsi
@@ -71,11 +71,13 @@ &i2c2 {
 	pinctrl-0 = <&i2c2_pins>;
 	status = "okay";
 	clock-frequency = <400000>;
+	vbus-supply = <&mt6358_vcamio_reg>;
 
 	eeprom@58 {
 		compatible = "atmel,24c32";
 		reg = <0x58>;
 		pagesize = <32>;
+		vcc-supply = <&mt6358_vcama2_reg>;
 	};
 };
 
@@ -84,11 +86,13 @@ &i2c4 {
 	pinctrl-0 = <&i2c4_pins>;
 	status = "okay";
 	clock-frequency = <400000>;
+	vbus-supply = <&mt6358_vcn18_reg>;
 
 	eeprom@54 {
 		compatible = "atmel,24c32";
 		reg = <0x54>;
 		pagesize = <32>;
+		vcc-supply = <&mt6358_vcn18_reg>;
 	};
 };
 
-- 
2.31.1.295.g9ea45b61b8-goog


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

* [PATCH v19 6/6] drm/i915/selftests: Rename functions names
  2021-04-14 17:29 [PATCH v19 0/6] add power control in i2c Hsin-Yi Wang
                   ` (4 preceding siblings ...)
  2021-04-14 17:29 ` [PATCH v19 5/6] arm64: dts: mt8183: add supply name for eeprom Hsin-Yi Wang
@ 2021-04-14 17:29 ` Hsin-Yi Wang
  2021-04-16 14:23   ` Jani Nikula
  5 siblings, 1 reply; 12+ messages in thread
From: Hsin-Yi Wang @ 2021-04-14 17:29 UTC (permalink / raw)
  To: Wolfram Sang, Matthias Brugger, Rob Herring, Bartosz Golaszewski
  Cc: linux-i2c, Qii Wang, devicetree, linux-arm-kernel,
	linux-mediatek, linux-kernel, Greg Kroah-Hartman, Mark Brown,
	Marek Szyprowski, Bibby Hsieh, Arnd Bergmann, Jani Nikula,
	Joonas Lahtinen, intel-gfx

pm_resume and pm_suspend might be conflict with the ones defined in
include/linux/suspend.h. Rename pm_resume{suspend} to
i915_pm_resume{suspend} since they are only used here.

Signed-off-by: Hsin-Yi Wang <hsinyi@chromium.org>
Reported-by: kernel test robot <lkp@intel.com>
---
 drivers/gpu/drm/i915/selftests/i915_gem.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/selftests/i915_gem.c b/drivers/gpu/drm/i915/selftests/i915_gem.c
index dc394fb7ccfa..525afda9d31f 100644
--- a/drivers/gpu/drm/i915/selftests/i915_gem.c
+++ b/drivers/gpu/drm/i915/selftests/i915_gem.c
@@ -94,7 +94,7 @@ static int pm_prepare(struct drm_i915_private *i915)
 	return 0;
 }
 
-static void pm_suspend(struct drm_i915_private *i915)
+static void i915_pm_suspend(struct drm_i915_private *i915)
 {
 	intel_wakeref_t wakeref;
 
@@ -116,7 +116,7 @@ static void pm_hibernate(struct drm_i915_private *i915)
 	}
 }
 
-static void pm_resume(struct drm_i915_private *i915)
+static void i915_pm_resume(struct drm_i915_private *i915)
 {
 	intel_wakeref_t wakeref;
 
@@ -152,12 +152,12 @@ static int igt_gem_suspend(void *arg)
 	if (err)
 		goto out;
 
-	pm_suspend(i915);
+	i915_pm_suspend(i915);
 
 	/* Here be dragons! Note that with S3RST any S3 may become S4! */
 	simulate_hibernate(i915);
 
-	pm_resume(i915);
+	i915_pm_resume(i915);
 
 	err = switch_to_context(ctx);
 out:
@@ -192,7 +192,7 @@ static int igt_gem_hibernate(void *arg)
 	/* Here be dragons! */
 	simulate_hibernate(i915);
 
-	pm_resume(i915);
+	i915_pm_resume(i915);
 
 	err = switch_to_context(ctx);
 out:
-- 
2.31.1.295.g9ea45b61b8-goog


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

* Re: [Intel-gfx] [PATCH v19 2/6] dt-binding: i2c: mt65xx: add vbus-supply property
  2021-04-14 17:29 ` [PATCH v19 2/6] dt-binding: i2c: mt65xx: add vbus-supply property Hsin-Yi Wang
@ 2021-04-14 22:19   ` Rob Herring
  0 siblings, 0 replies; 12+ messages in thread
From: Rob Herring @ 2021-04-14 22:19 UTC (permalink / raw)
  To: Hsin-Yi Wang
  Cc: linux-i2c, Arnd Bergmann, Greg Kroah-Hartman, Matthias Brugger,
	Marek Szyprowski, linux-arm-kernel, Rob Herring, intel-gfx,
	devicetree, Bartosz Golaszewski, Qii Wang, linux-kernel,
	linux-mediatek, Bibby Hsieh, Mark Brown, Wolfram Sang

On Thu, 15 Apr 2021 01:29:12 +0800, Hsin-Yi Wang wrote:
> Add vbus-supply property for mt65xx. The regulator can be passed into
> core and turned off during suspend/sleep to reduce power consumption.
> 
> Signed-off-by: Hsin-Yi Wang <hsinyi@chromium.org>
> ---
>  Documentation/devicetree/bindings/i2c/i2c-mt65xx.txt | 1 +
>  1 file changed, 1 insertion(+)
> 

Acked-by: Rob Herring <robh@kernel.org>

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

* Re: [PATCH v19 4/6] misc: eeprom: at24: check suspend status before disable regulator
  2021-04-14 17:29 ` [PATCH v19 4/6] misc: eeprom: at24: check suspend status before disable regulator Hsin-Yi Wang
@ 2021-04-16 14:09   ` Bartosz Golaszewski
  2021-04-20 13:33     ` Hsin-Yi Wang
  0 siblings, 1 reply; 12+ messages in thread
From: Bartosz Golaszewski @ 2021-04-16 14:09 UTC (permalink / raw)
  To: Hsin-Yi Wang
  Cc: Wolfram Sang, Matthias Brugger, Rob Herring, linux-i2c, Qii Wang,
	linux-devicetree, arm-soc, moderated list:ARM/Mediatek SoC...,
	LKML, Greg Kroah-Hartman, Mark Brown, Marek Szyprowski,
	Bibby Hsieh, Arnd Bergmann, Jani Nikula, Joonas Lahtinen,
	intel-gfx

On Wed, Apr 14, 2021 at 7:29 PM Hsin-Yi Wang <hsinyi@chromium.org> wrote:
>
> cd5676db0574 ("misc: eeprom: at24: support pm_runtime control") disables
> regulator in runtime suspend. If runtime suspend is called before
> regulator disable, it will results in regulator unbalanced disabling.
>
> Signed-off-by: Hsin-Yi Wang <hsinyi@chromium.org>

Please add the Fixes tag.

> ---
>  drivers/misc/eeprom/at24.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/misc/eeprom/at24.c b/drivers/misc/eeprom/at24.c
> index 926408b41270..7a6f01ace78a 100644
> --- a/drivers/misc/eeprom/at24.c
> +++ b/drivers/misc/eeprom/at24.c
> @@ -763,7 +763,8 @@ static int at24_probe(struct i2c_client *client)
>         at24->nvmem = devm_nvmem_register(dev, &nvmem_config);
>         if (IS_ERR(at24->nvmem)) {
>                 pm_runtime_disable(dev);
> -               regulator_disable(at24->vcc_reg);
> +               if (!pm_runtime_status_suspended(dev))
> +                       regulator_disable(at24->vcc_reg);
>                 return PTR_ERR(at24->nvmem);
>         }
>
> @@ -774,7 +775,8 @@ static int at24_probe(struct i2c_client *client)
>         err = at24_read(at24, 0, &test_byte, 1);
>         if (err) {
>                 pm_runtime_disable(dev);
> -               regulator_disable(at24->vcc_reg);
> +               if (!pm_runtime_status_suspended(dev))
> +                       regulator_disable(at24->vcc_reg);
>                 return -ENODEV;
>         }
>
> --
> 2.31.1.295.g9ea45b61b8-goog
>

Acked-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>

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

* Re: [PATCH v19 6/6] drm/i915/selftests: Rename functions names
  2021-04-14 17:29 ` [PATCH v19 6/6] drm/i915/selftests: Rename functions names Hsin-Yi Wang
@ 2021-04-16 14:23   ` Jani Nikula
  2021-04-16 14:42     ` Hsin-Yi Wang
  0 siblings, 1 reply; 12+ messages in thread
From: Jani Nikula @ 2021-04-16 14:23 UTC (permalink / raw)
  To: Hsin-Yi Wang, Wolfram Sang, Matthias Brugger, Rob Herring,
	Bartosz Golaszewski
  Cc: linux-i2c, Qii Wang, devicetree, linux-arm-kernel,
	linux-mediatek, linux-kernel, Greg Kroah-Hartman, Mark Brown,
	Marek Szyprowski, Bibby Hsieh, Arnd Bergmann, Joonas Lahtinen,
	intel-gfx

On Thu, 15 Apr 2021, Hsin-Yi Wang <hsinyi@chromium.org> wrote:
> pm_resume and pm_suspend might be conflict with the ones defined in
> include/linux/suspend.h. Rename pm_resume{suspend} to
> i915_pm_resume{suspend} since they are only used here.

I agree with the rationale here.

Do you need this to be part of your series, or shall we just pick this
up for i915? (We might consider renaming to something else or prefix the
functions with _ though, as we also have existing i915_pm_suspend and
i915_pm_resume elsewhere.)

BR,
Jani.

>
> Signed-off-by: Hsin-Yi Wang <hsinyi@chromium.org>
> Reported-by: kernel test robot <lkp@intel.com>
> ---
>  drivers/gpu/drm/i915/selftests/i915_gem.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/selftests/i915_gem.c b/drivers/gpu/drm/i915/selftests/i915_gem.c
> index dc394fb7ccfa..525afda9d31f 100644
> --- a/drivers/gpu/drm/i915/selftests/i915_gem.c
> +++ b/drivers/gpu/drm/i915/selftests/i915_gem.c
> @@ -94,7 +94,7 @@ static int pm_prepare(struct drm_i915_private *i915)
>  	return 0;
>  }
>  
> -static void pm_suspend(struct drm_i915_private *i915)
> +static void i915_pm_suspend(struct drm_i915_private *i915)
>  {
>  	intel_wakeref_t wakeref;
>  
> @@ -116,7 +116,7 @@ static void pm_hibernate(struct drm_i915_private *i915)
>  	}
>  }
>  
> -static void pm_resume(struct drm_i915_private *i915)
> +static void i915_pm_resume(struct drm_i915_private *i915)
>  {
>  	intel_wakeref_t wakeref;
>  
> @@ -152,12 +152,12 @@ static int igt_gem_suspend(void *arg)
>  	if (err)
>  		goto out;
>  
> -	pm_suspend(i915);
> +	i915_pm_suspend(i915);
>  
>  	/* Here be dragons! Note that with S3RST any S3 may become S4! */
>  	simulate_hibernate(i915);
>  
> -	pm_resume(i915);
> +	i915_pm_resume(i915);
>  
>  	err = switch_to_context(ctx);
>  out:
> @@ -192,7 +192,7 @@ static int igt_gem_hibernate(void *arg)
>  	/* Here be dragons! */
>  	simulate_hibernate(i915);
>  
> -	pm_resume(i915);
> +	i915_pm_resume(i915);
>  
>  	err = switch_to_context(ctx);
>  out:

-- 
Jani Nikula, Intel Open Source Graphics Center

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

* Re: [PATCH v19 6/6] drm/i915/selftests: Rename functions names
  2021-04-16 14:23   ` Jani Nikula
@ 2021-04-16 14:42     ` Hsin-Yi Wang
  0 siblings, 0 replies; 12+ messages in thread
From: Hsin-Yi Wang @ 2021-04-16 14:42 UTC (permalink / raw)
  To: Jani Nikula
  Cc: Wolfram Sang, Matthias Brugger, Rob Herring, Bartosz Golaszewski,
	linux-i2c, Qii Wang, Devicetree List,
	moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
	moderated list:ARM/Mediatek SoC support, lkml,
	Greg Kroah-Hartman, Mark Brown, Marek Szyprowski, Bibby Hsieh,
	Arnd Bergmann, Joonas Lahtinen, intel-gfx

On Fri, Apr 16, 2021 at 10:23 PM Jani Nikula
<jani.nikula@linux.intel.com> wrote:
>
> On Thu, 15 Apr 2021, Hsin-Yi Wang <hsinyi@chromium.org> wrote:
> > pm_resume and pm_suspend might be conflict with the ones defined in
> > include/linux/suspend.h. Rename pm_resume{suspend} to
> > i915_pm_resume{suspend} since they are only used here.
>
> I agree with the rationale here.
>
> Do you need this to be part of your series, or shall we just pick this
> up for i915? (We might consider renaming to something else or prefix the
> functions with _ though, as we also have existing i915_pm_suspend and
> i915_pm_resume elsewhere.)
>

This patch can be separated from the series, thanks.


> BR,
> Jani.
>
> >
> > Signed-off-by: Hsin-Yi Wang <hsinyi@chromium.org>
> > Reported-by: kernel test robot <lkp@intel.com>
> > ---
> >  drivers/gpu/drm/i915/selftests/i915_gem.c | 10 +++++-----
> >  1 file changed, 5 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/selftests/i915_gem.c b/drivers/gpu/drm/i915/selftests/i915_gem.c
> > index dc394fb7ccfa..525afda9d31f 100644
> > --- a/drivers/gpu/drm/i915/selftests/i915_gem.c
> > +++ b/drivers/gpu/drm/i915/selftests/i915_gem.c
> > @@ -94,7 +94,7 @@ static int pm_prepare(struct drm_i915_private *i915)
> >       return 0;
> >  }
> >
> > -static void pm_suspend(struct drm_i915_private *i915)
> > +static void i915_pm_suspend(struct drm_i915_private *i915)
> >  {
> >       intel_wakeref_t wakeref;
> >
> > @@ -116,7 +116,7 @@ static void pm_hibernate(struct drm_i915_private *i915)
> >       }
> >  }
> >
> > -static void pm_resume(struct drm_i915_private *i915)
> > +static void i915_pm_resume(struct drm_i915_private *i915)
> >  {
> >       intel_wakeref_t wakeref;
> >
> > @@ -152,12 +152,12 @@ static int igt_gem_suspend(void *arg)
> >       if (err)
> >               goto out;
> >
> > -     pm_suspend(i915);
> > +     i915_pm_suspend(i915);
> >
> >       /* Here be dragons! Note that with S3RST any S3 may become S4! */
> >       simulate_hibernate(i915);
> >
> > -     pm_resume(i915);
> > +     i915_pm_resume(i915);
> >
> >       err = switch_to_context(ctx);
> >  out:
> > @@ -192,7 +192,7 @@ static int igt_gem_hibernate(void *arg)
> >       /* Here be dragons! */
> >       simulate_hibernate(i915);
> >
> > -     pm_resume(i915);
> > +     i915_pm_resume(i915);
> >
> >       err = switch_to_context(ctx);
> >  out:
>
> --
> Jani Nikula, Intel Open Source Graphics Center

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

* Re: [PATCH v19 4/6] misc: eeprom: at24: check suspend status before disable regulator
  2021-04-16 14:09   ` Bartosz Golaszewski
@ 2021-04-20 13:33     ` Hsin-Yi Wang
  0 siblings, 0 replies; 12+ messages in thread
From: Hsin-Yi Wang @ 2021-04-20 13:33 UTC (permalink / raw)
  To: Bartosz Golaszewski
  Cc: Wolfram Sang, Matthias Brugger, Rob Herring, linux-i2c, Qii Wang,
	linux-devicetree, arm-soc, moderated list:ARM/Mediatek SoC...,
	LKML, Greg Kroah-Hartman, Mark Brown, Marek Szyprowski,
	Bibby Hsieh, Arnd Bergmann, Jani Nikula, Joonas Lahtinen,
	intel-gfx

On Fri, Apr 16, 2021 at 10:09 PM Bartosz Golaszewski
<bgolaszewski@baylibre.com> wrote:
>
> On Wed, Apr 14, 2021 at 7:29 PM Hsin-Yi Wang <hsinyi@chromium.org> wrote:
> >
> > cd5676db0574 ("misc: eeprom: at24: support pm_runtime control") disables
> > regulator in runtime suspend. If runtime suspend is called before
> > regulator disable, it will results in regulator unbalanced disabling.
> >
> > Signed-off-by: Hsin-Yi Wang <hsinyi@chromium.org>
>
> Please add the Fixes tag.
>

Hi,

I resend the patch with the fix tag separately since other patches in
this series are not changed.

https://patchwork.ozlabs.org/project/linux-i2c/patch/20210420133050.377209-1-hsinyi@chromium.org/

Thanks

> > ---
> >  drivers/misc/eeprom/at24.c | 6 ++++--
> >  1 file changed, 4 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/misc/eeprom/at24.c b/drivers/misc/eeprom/at24.c
> > index 926408b41270..7a6f01ace78a 100644
> > --- a/drivers/misc/eeprom/at24.c
> > +++ b/drivers/misc/eeprom/at24.c
> > @@ -763,7 +763,8 @@ static int at24_probe(struct i2c_client *client)
> >         at24->nvmem = devm_nvmem_register(dev, &nvmem_config);
> >         if (IS_ERR(at24->nvmem)) {
> >                 pm_runtime_disable(dev);
> > -               regulator_disable(at24->vcc_reg);
> > +               if (!pm_runtime_status_suspended(dev))
> > +                       regulator_disable(at24->vcc_reg);
> >                 return PTR_ERR(at24->nvmem);
> >         }
> >
> > @@ -774,7 +775,8 @@ static int at24_probe(struct i2c_client *client)
> >         err = at24_read(at24, 0, &test_byte, 1);
> >         if (err) {
> >                 pm_runtime_disable(dev);
> > -               regulator_disable(at24->vcc_reg);
> > +               if (!pm_runtime_status_suspended(dev))
> > +                       regulator_disable(at24->vcc_reg);
> >                 return -ENODEV;
> >         }
> >
> > --
> > 2.31.1.295.g9ea45b61b8-goog
> >
>
> Acked-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>

^ 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 --
2021-04-14 17:29 [PATCH v19 0/6] add power control in i2c Hsin-Yi Wang
2021-04-14 17:29 ` [PATCH v19 1/6] i2c: core: support bus regulator controlling in adapter Hsin-Yi Wang
2021-04-14 17:29 ` [PATCH v19 2/6] dt-binding: i2c: mt65xx: add vbus-supply property Hsin-Yi Wang
2021-04-14 22:19   ` [Intel-gfx] " Rob Herring
2021-04-14 17:29 ` [PATCH v19 3/6] i2c: mediatek: mt65xx: add optional vbus-supply Hsin-Yi Wang
2021-04-14 17:29 ` [PATCH v19 4/6] misc: eeprom: at24: check suspend status before disable regulator Hsin-Yi Wang
2021-04-16 14:09   ` Bartosz Golaszewski
2021-04-20 13:33     ` Hsin-Yi Wang
2021-04-14 17:29 ` [PATCH v19 5/6] arm64: dts: mt8183: add supply name for eeprom Hsin-Yi Wang
2021-04-14 17:29 ` [PATCH v19 6/6] drm/i915/selftests: Rename functions names Hsin-Yi Wang
2021-04-16 14:23   ` Jani Nikula
2021-04-16 14:42     ` Hsin-Yi Wang

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