[PATCH v4 1/2] mfd: da9062: enable being a system-power-controller

[PATCH v4 1/2] mfd: da9062: enable being a system-power-controller

[Helmut Grohne]

The DA9062 can be the device used to power the CPU. In that case, it can
be used to power off the system. In the CONTROL_A register, the M_*_EN
bits must be zero for the corresponding *_EN bits to have an effect. We
zero them all to turn off the system.

Signed-off-by: Helmut Grohne <helmut.grohne@intenta.de>
---
 drivers/mfd/da9062-core.c | 35 +++++++++++++++++++++++++++++++++++
 1 file changed, 35 insertions(+)

Compared to v3, this addresses the following concerns raised by Lee Jones:
 * Consistently start error messages upper case.
 * Pull i2c_get_clientdata out of the call again. In his previous review he
   remarked the joint declaration and call as "passable", which I misunderstood
   as "can be passed to regmap_update_bits directly" while the intended meaning
   was more like "acceptable". This is part is now reverted back to the v2
   form.

Adam Thomson's observation about the use of i2c in atomic context is observable
in reality since a few kernel releases. When powering off the system, I now see
this:

reboot: Power down
------------[ cut here ]------------
WARNING: CPU: 0 PID: 1 at drivers/i2c/i2c-core.h:40 i2c_transfer+0x104/0x134
---[ end trace 0000000000000000 ]---

Looking at the actual warning, this is actually about using non-atomic xfers in
atomic context (i.e. he's totally right). Wolfram Sang worked on this problem
on a more fundamental level and added a patch "i2c: core: introduce callbacks
for atomic transfers" (63b96983a5dd). Unfortunately, the relevant i2c driver in
use here (i2c-cadence) does not include atomic transfer callbacks yet, which is
why I see the warning. However, that only confirms the way of using i2c code in
pm_power_off hooks as the "right" way to do it.

In all of my tests, the system powered off reliably.

I still didn't include Adam Thomson's Reviewed-by from v2, because the code is
still noticeably different.

Thanks for bearing with me

Helmut
diff --git a/drivers/mfd/da9062-core.c b/drivers/mfd/da9062-core.c
index 2774b2cbaea6..94d447769da3 100644
--- a/drivers/mfd/da9062-core.c
+++ b/drivers/mfd/da9062-core.c
@@ -620,6 +620,27 @@ static const struct of_device_id da9062_dt_ids[] = {
 };
 MODULE_DEVICE_TABLE(of, da9062_dt_ids);
 
+static struct i2c_client *da9062_i2c_client;
+
+static void da9062_power_off(void)
+{
+	struct da9062 *chip = i2c_get_clientdata(da9062_i2c_client);
+	int ret;
+
+	ret = regmap_update_bits(
+		chip->regmap,
+		DA9062AA_CONTROL_A,
+		DA9062AA_SYSTEM_EN_MASK | DA9062AA_POWER_EN_MASK |
+			DA9062AA_POWER1_EN_MASK | DA9062AA_M_SYSTEM_EN_MASK |
+			DA9062AA_M_POWER_EN_MASK | DA9062AA_M_POWER1_EN_MASK,
+		0
+	);
+
+	if (ret < 0)
+		dev_err(&da9062_i2c_client->dev,
+			"Failed to power the system off (err=%d)\n", ret);
+}
+
 static int da9062_i2c_probe(struct i2c_client *i2c,
 	const struct i2c_device_id *id)
 {
@@ -720,6 +741,15 @@ static int da9062_i2c_probe(struct i2c_client *i2c,
 		return ret;
 	}
 
+	if (of_device_is_system_power_controller(i2c->dev.of_node)) {
+		if (!pm_power_off) {
+			da9062_i2c_client = i2c;
+			pm_power_off = da9062_power_off;
+		} else {
+			dev_warn(&i2c->dev, "Poweroff callback already assigned\n");
+		}
+	}
+
 	return ret;
 }
 
@@ -727,6 +757,11 @@ static int da9062_i2c_remove(struct i2c_client *i2c)
 {
 	struct da9062 *chip = i2c_get_clientdata(i2c);
 
+	if (pm_power_off == da9062_power_off)
+		pm_power_off = NULL;
+	if (da9062_i2c_client)
+		da9062_i2c_client = NULL;
+
 	mfd_remove_devices(chip->dev);
 	regmap_del_irq_chip(i2c->irq, chip->regmap_irq);
 
-- 
2.30.2
 
Dipl.-Inf. Helmut Grohne
Research and Development
Application Engineering
 
Phone: +49 (371) 24354 0 ∙ Fax:  +49 (371) 24354 020
helmut.grohne@intenta.de ∙ https://www.intenta.de
 
Intenta GmbH ∙ Ahornstraße 55 ∙ 09112 Chemnitz, Germany
Managing Director: Dr.-Ing. Basel Fardi ∙ VAT/USt-IdNr.: DE 275745394
Commercial register: HRB 26404 Amtsgericht Chemnitz

[PATCH v4 2/2] dt-bindings: mfd: da9062 can be a system power controller

[Helmut Grohne]

The DA9062 can be used to power off a system if it is appropriately
wired.

Signed-off-by: Helmut Grohne <helmut.grohne@intenta.de>
Acked-by: Rob Herring <robh@kernel.org>
---
 Documentation/devicetree/bindings/mfd/da9062.txt | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/mfd/da9062.txt b/Documentation/devicetree/bindings/mfd/da9062.txt
index bab0d0e66cb3..4861c3ad97e9 100644
--- a/Documentation/devicetree/bindings/mfd/da9062.txt
+++ b/Documentation/devicetree/bindings/mfd/da9062.txt
@@ -41,6 +41,7 @@ further information on IRQ bindings.
 
 Optional properties:
 
+- system-power-controller
 - gpio-controller : Marks the device as a gpio controller.
 - #gpio-cells     : Should be two. The first cell is the pin number and the
                     second cell is used to specify the gpio polarity.
-- 
2.30.2
 
Dipl.-Inf. Helmut Grohne
Research and Development
Application Engineering
 
Phone: +49 (371) 24354 0 ∙ Fax:  +49 (371) 24354 020
helmut.grohne@intenta.de ∙ https://www.intenta.de
 
Intenta GmbH ∙ Ahornstraße 55 ∙ 09112 Chemnitz, Germany
Managing Director: Dr.-Ing. Basel Fardi ∙ VAT/USt-IdNr.: DE 275745394
Commercial register: HRB 26404 Amtsgericht Chemnitz

RE: [PATCH v4 1/2] mfd: da9062: enable being a system-power-controller

[DLG Adam Thomson]

On 06 July 2022 13:30, Helmut Grohne wrote:

> The DA9062 can be the device used to power the CPU. In that case, it can
> be used to power off the system. In the CONTROL_A register, the M_*_EN
> bits must be zero for the corresponding *_EN bits to have an effect. We
> zero them all to turn off the system.
> 
> Signed-off-by: Helmut Grohne <helmut.grohne@intenta.de>

Reviewed-by: Adam Thomson <DLG-Adam.Thomson.Opensource@dm.renesas.com>

RE: [PATCH v4 2/2] dt-bindings: mfd: da9062 can be a system power controller

[DLG Adam Thomson]

On 06 July 2022 13:30, Helmut Grohne wrote:

> The DA9062 can be used to power off a system if it is appropriately
> wired.
> 
> Signed-off-by: Helmut Grohne <helmut.grohne@intenta.de>
> Acked-by: Rob Herring <robh@kernel.org>

Reviewed-by: Adam Thomson <DLG-Adam.Thomson.Opensource@dm.renesas.com>