Re: [PATCH] mfd: tps65219: Add support for soft shutdown via sys-off API

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 3/1/23 10:07 AM, Lee Jones wrote:
On Fri, 03 Feb 2023, Jerome Neanne wrote:

Use new API for power-off mode support:
Link: https://lwn.net/Articles/894511/
Link: https://lore.kernel.org/all/7hfseqa7l0.fsf@xxxxxxxxxxxx/

sys-off API allows support of shutdown handler and restart handler.

Shutdown was not supported before that enhancement.
This is required for platform that are not using PSCI.


Not sure what platform doesn't have PSCI off, since you tested on
AM62-SK I'm guessing you manually disabled the PSCI off for testing?

Anyway I don't see any huge issues with the code itself, small comment below.

Test:
- restart:
   # reboot
   Default is cold reset:
   # cat /sys/kernel/reboot/mode
   Switch boot mode to warm reset:
   # echo warm > /sys/kernel/reboot/mode
- power-off:
   # halt

Tested on AM62-SP-SK board.

Signed-off-by: Jerome Neanne <jneanne@xxxxxxxxxxxx>
Suggested-by: Andrew Davis <afd@xxxxxx>

A review from Andrew would be helpful here.

---
  drivers/mfd/tps65219.c | 45 +++++++++++++++++++++++++++++++-----------
  1 file changed, 34 insertions(+), 11 deletions(-)

diff --git a/drivers/mfd/tps65219.c b/drivers/mfd/tps65219.c
index 0e402fda206b..c134f3f6e202 100644
--- a/drivers/mfd/tps65219.c
+++ b/drivers/mfd/tps65219.c
@@ -25,25 +25,34 @@ static int tps65219_cold_reset(struct tps65219 *tps)
  				  TPS65219_MFP_COLD_RESET_I2C_CTRL_MASK);
  }
-static int tps65219_restart(struct notifier_block *this,
-			    unsigned long reboot_mode, void *cmd)
+static int tps65219_soft_shutdown(struct tps65219 *tps)
  {
-	struct tps65219 *tps;
+	return regmap_update_bits(tps->regmap, TPS65219_REG_MFP_CTRL,
+				  TPS65219_MFP_I2C_OFF_REQ_MASK,
+				  TPS65219_MFP_I2C_OFF_REQ_MASK);
+}
- tps = container_of(this, struct tps65219, nb);
+static int tps65219_power_off_handler(struct sys_off_data *data)
+{
+	tps65219_soft_shutdown(data->cb_data);
+	return NOTIFY_DONE;
+}
+static int tps65219_restart(struct tps65219 *tps,
+			    unsigned long reboot_mode)
+{
  	if (reboot_mode == REBOOT_WARM)
  		tps65219_warm_reset(tps);
  	else
  		tps65219_cold_reset(tps);
-
  	return NOTIFY_DONE;
  }
-static struct notifier_block pmic_rst_restart_nb = {
-	.notifier_call = tps65219_restart,
-	.priority = 200,
-};
+static int tps65219_restart_handler(struct sys_off_data *data)
+{
+	tps65219_restart(data->cb_data, data->mode);
+	return NOTIFY_DONE;
+}
static const struct resource tps65219_pwrbutton_resources[] = {
  	DEFINE_RES_IRQ_NAMED(TPS65219_INT_PB_FALLING_EDGE_DETECT, "falling"),
@@ -269,13 +278,27 @@ static int tps65219_probe(struct i2c_client *client)
  		}
  	}
- tps->nb = pmic_rst_restart_nb;
-	ret = register_restart_handler(&tps->nb);
+	ret = devm_register_sys_off_handler(tps->dev,
+					    SYS_OFF_MODE_RESTART,
+					    SYS_OFF_PRIO_HIGH,

Why not default prio? SYS_OFF_PRIO_DEFAULT

Then you can use this new helper devm_register_restart_handler()

+					    tps65219_restart_handler,
+					    tps);
+
  	if (ret) {
  		dev_err(tps->dev, "cannot register restart handler, %d\n", ret);
  		return ret;
  	}
+ ret = devm_register_sys_off_handler(tps->dev,
+					    SYS_OFF_MODE_POWER_OFF,
+					    SYS_OFF_PRIO_DEFAULT,
+					    tps65219_power_off_handler,
+					    tps);


devm_register_power_off_handler()?

Otherwise I see no major issues,

Reviewed-by: Andrew Davis <afd@xxxxxx>

Andrew

+	if (ret) {
+		dev_err(tps->dev, "failed to register sys-off handler: %d\n",
+			ret);
+		return ret;
+	}
  	return 0;
  }
--
2.34.1





[Index of Archives]     [Linux Arm (vger)]     [ARM Kernel]     [ARM MSM]     [Linux Tegra]     [Linux WPAN Networking]     [Linux Wireless Networking]     [Maemo Users]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Trails]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux