Messages in this thread Patch in this message | | | Date | Wed, 6 Jul 2022 14:29:48 +0200 | From | Helmut Grohne <> | Subject | [PATCH v4 1/2] mfd: da9062: enable being a system-power-controller |
| |
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
| |