[PATCH 0/6] regulator: pca9450: register restart handlers

[PATCH 0/6] regulator: pca9450: register restart handlers

[Benjamin Bara]

Hi!

This series implements two restart handler registrations for the pca9450
(6/6). As the pca9450 supports both, cold and warm resets, and there
exist at least one other PMIC implementation which also implements a
warm and a cold reset (tps65219), 1-5/6 should simplify/clarify the
distinction process between warm/soft and cold/hard resets/restarts.
Instead of deciding in the handler, this should be done during
registration. The series is a follow-up to Dmitry's feedback, regarding
checking the reboot_mode in the handler [1].

This series is based on linux-next and 6/6 depends on [2].

Thanks & best regards,
Benjamin

[1] https://lore.kernel.org/all/7eddaf8c-ab04-7670-fc45-15f0fce5eff2@collabora.com/
[2] https://lore.kernel.org/all/20230327-tegra-pmic-reboot-v7-3-18699d5dcd76@skidata.com/

---
Benjamin Bara (6):
      kernel/reboot: distinguish between cold and warm
      mfd: rk8xx: Specify restart mode
      soc/tegra: pmc: Specify restart mode
      mfd: tps65219: Specify restart mode
      kernel/reboot: remove generic restart mode
      regulator: pca9450: register restart handlers

 drivers/mfd/rk8xx-core.c              |  3 +-
 drivers/mfd/tps65219.c                | 17 ++++++--
 drivers/regulator/pca9450-regulator.c | 59 +++++++++++++++++++++++++
 drivers/soc/tegra/pmc.c               |  2 +-
 include/linux/reboot.h                | 23 +++++++---
 include/linux/regulator/pca9450.h     |  7 +++
 kernel/reboot.c                       | 82 +++++++++++++++++++++++++++++------
 7 files changed, 167 insertions(+), 26 deletions(-)
---
base-commit: 4d2c646ac07cf4a35ef1c4a935a1a4fd6c6b1a36
change-id: 20230724-pca9450-reboot-0b32218fc7a2

Best regards,
-- 
Benjamin Bara <benjamin.bara@skidata.com>

[PATCH 1/6] kernel/reboot: distinguish between cold and warm

[Benjamin Bara]

From: Benjamin Bara <benjamin.bara@skidata.com>

As a couple of PMICs support warm and cold resets (e.g. tps65219 or
pca9450) and the 'reboot_mode' kernel parameter allows to specify the
reboot mode, distinguish between cold and warm reboot handlers.
This is done by maintaining two different restart handler lists. When
the mode is unspecified, the cold restart handlers are preferred
(independent of their actual priority).

Signed-off-by: Benjamin Bara <benjamin.bara@skidata.com>
---
 include/linux/reboot.h | 22 ++++++++++++
 kernel/reboot.c        | 92 ++++++++++++++++++++++++++++++++++++++++++++++----
 2 files changed, 108 insertions(+), 6 deletions(-)

diff --git a/include/linux/reboot.h b/include/linux/reboot.h
index 2b6bb593be5b..05199aedb696 100644
--- a/include/linux/reboot.h
+++ b/include/linux/reboot.h
@@ -119,6 +119,20 @@ enum sys_off_mode {
 	 * Handlers restart system. Handlers are disallowed to sleep.
 	 */
 	SYS_OFF_MODE_RESTART,
+
+	/**
+	 * @SYS_OFF_MODE_RESTART_COLD:
+	 *
+	 * Handlers cold restart system. Handlers are disallowed to sleep.
+	 */
+	SYS_OFF_MODE_RESTART_COLD,
+
+	/**
+	 * @SYS_OFF_MODE_RESTART_WARM:
+	 *
+	 * Handlers warm restart system. Handlers are disallowed to sleep.
+	 */
+	SYS_OFF_MODE_RESTART_WARM,
 };
 
 /**
@@ -157,6 +171,14 @@ int devm_register_restart_handler(struct device *dev,
 				  int (*callback)(struct sys_off_data *data),
 				  void *cb_data);
 
+int devm_register_cold_restart_handler(struct device *dev,
+				       int (*callback)(struct sys_off_data *data),
+				       void *cb_data);
+
+int devm_register_warm_restart_handler(struct device *dev,
+				       int (*callback)(struct sys_off_data *data),
+				       void *cb_data);
+
 int register_platform_power_off(void (*power_off)(void));
 void unregister_platform_power_off(void (*power_off)(void));
 
diff --git a/kernel/reboot.c b/kernel/reboot.c
index 3bba88c7ffc6..d0d3faf70a79 100644
--- a/kernel/reboot.c
+++ b/kernel/reboot.c
@@ -146,9 +146,15 @@ EXPORT_SYMBOL(devm_register_reboot_notifier);
 
 /*
  *	Notifier list for kernel code which wants to be called
- *	to restart the system.
+ *	to cold restart the system.
  */
-static ATOMIC_NOTIFIER_HEAD(restart_handler_list);
+static ATOMIC_NOTIFIER_HEAD(cold_restart_handler_list);
+
+/*
+ *	Notifier list for kernel code which wants to be called
+ *	to warm restart the system.
+ */
+static ATOMIC_NOTIFIER_HEAD(warm_restart_handler_list);
 
 /**
  *	register_restart_handler - Register function to be called to reset
@@ -190,7 +196,11 @@ static ATOMIC_NOTIFIER_HEAD(restart_handler_list);
  */
 int register_restart_handler(struct notifier_block *nb)
 {
-	return atomic_notifier_chain_register(&restart_handler_list, nb);
+	/*
+	 * Store all non-devm-based handlers in the warm list to ensure that the
+	 * "specified" handler are preferred over the "unspecified" ones.
+	 */
+	return atomic_notifier_chain_register(&warm_restart_handler_list, nb);
 }
 EXPORT_SYMBOL(register_restart_handler);
 
@@ -205,7 +215,14 @@ EXPORT_SYMBOL(register_restart_handler);
  */
 int unregister_restart_handler(struct notifier_block *nb)
 {
-	return atomic_notifier_chain_unregister(&restart_handler_list, nb);
+	int ret;
+
+	ret = atomic_notifier_chain_unregister(&warm_restart_handler_list, nb);
+	if (ret == -ENOENT)
+		ret = atomic_notifier_chain_unregister(&cold_restart_handler_list,
+						       nb);
+
+	return ret;
 }
 EXPORT_SYMBOL(unregister_restart_handler);
 
@@ -222,7 +239,18 @@ EXPORT_SYMBOL(unregister_restart_handler);
  */
 void do_kernel_restart(char *cmd)
 {
-	atomic_notifier_call_chain(&restart_handler_list, reboot_mode, cmd);
+	/*
+	 * REBOOT_GPIO can be either cold or warm -> let handler decide.
+	 * Prefer cold reboot if mode not specified.
+	 */
+	if (reboot_mode == REBOOT_UNDEFINED || reboot_mode == REBOOT_GPIO ||
+	    reboot_mode == REBOOT_HARD || reboot_mode == REBOOT_COLD)
+		atomic_notifier_call_chain(&cold_restart_handler_list,
+					   reboot_mode, cmd);
+	if (reboot_mode == REBOOT_UNDEFINED || reboot_mode == REBOOT_GPIO ||
+	    reboot_mode == REBOOT_SOFT || reboot_mode == REBOOT_WARM)
+		atomic_notifier_call_chain(&warm_restart_handler_list,
+					   reboot_mode, cmd);
 }
 
 void migrate_to_reboot_cpu(void)
@@ -414,7 +442,15 @@ register_sys_off_handler(enum sys_off_mode mode,
 		break;
 
 	case SYS_OFF_MODE_RESTART:
-		handler->list = &restart_handler_list;
+		handler->list = &warm_restart_handler_list;
+		break;
+
+	case SYS_OFF_MODE_RESTART_COLD:
+		handler->list = &cold_restart_handler_list;
+		break;
+
+	case SYS_OFF_MODE_RESTART_WARM:
+		handler->list = &warm_restart_handler_list;
 		break;
 
 	default:
@@ -560,6 +596,50 @@ int devm_register_restart_handler(struct device *dev,
 }
 EXPORT_SYMBOL_GPL(devm_register_restart_handler);
 
+/**
+ *	devm_register_cold_restart_handler - Register cold restart handler
+ *	@dev: Device that registers callback
+ *	@callback: Callback function
+ *	@cb_data: Callback's argument
+ *
+ *	Registers resource-managed sys-off handler with a default priority
+ *	and using cold restart mode.
+ *
+ *	Returns zero on success, or error code on failure.
+ */
+int devm_register_cold_restart_handler(struct device *dev,
+				       int (*callback)(struct sys_off_data *data),
+				       void *cb_data)
+{
+	return devm_register_sys_off_handler(dev,
+					     SYS_OFF_MODE_RESTART_COLD,
+					     SYS_OFF_PRIO_DEFAULT,
+					     callback, cb_data);
+}
+EXPORT_SYMBOL_GPL(devm_register_cold_restart_handler);
+
+/**
+ *	devm_register_warm_restart_handler - Register warm restart handler
+ *	@dev: Device that registers callback
+ *	@callback: Callback function
+ *	@cb_data: Callback's argument
+ *
+ *	Registers resource-managed sys-off handler with a default priority
+ *	and using warm restart mode.
+ *
+ *	Returns zero on success, or error code on failure.
+ */
+int devm_register_warm_restart_handler(struct device *dev,
+				       int (*callback)(struct sys_off_data *data),
+				       void *cb_data)
+{
+	return devm_register_sys_off_handler(dev,
+					     SYS_OFF_MODE_RESTART_WARM,
+					     SYS_OFF_PRIO_DEFAULT,
+					     callback, cb_data);
+}
+EXPORT_SYMBOL_GPL(devm_register_warm_restart_handler);
+
 static struct sys_off_handler *platform_power_off_handler;
 
 static int platform_power_off_notify(struct sys_off_data *data)

-- 
2.34.1

[PATCH 2/6] mfd: rk8xx: Specify restart mode

[Benjamin Bara]

From: Benjamin Bara <benjamin.bara@skidata.com>

Specify the implemented restart handler as a cold one.

Signed-off-by: Benjamin Bara <benjamin.bara@skidata.com>
---
 drivers/mfd/rk8xx-core.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/mfd/rk8xx-core.c b/drivers/mfd/rk8xx-core.c
index e8fc9e2ab1d0..15e8e6a9943a 100644
--- a/drivers/mfd/rk8xx-core.c
+++ b/drivers/mfd/rk8xx-core.c
@@ -697,7 +697,8 @@ int rk8xx_probe(struct device *dev, int variant, unsigned int irq, struct regmap
 		case RK809_ID:
 		case RK817_ID:
 			ret = devm_register_sys_off_handler(dev,
-							    SYS_OFF_MODE_RESTART, SYS_OFF_PRIO_HIGH,
+							    SYS_OFF_MODE_RESTART_COLD,
+							    SYS_OFF_PRIO_HIGH,
 							    &rk808_restart, rk808);
 			if (ret)
 				dev_warn(dev, "failed to register rst handler, %d\n", ret);

-- 
2.34.1

Re: [PATCH 2/6] mfd: rk8xx: Specify restart mode

[Lee Jones]

On Thu, 27 Jul 2023, Benjamin Bara wrote:

> From: Benjamin Bara <benjamin.bara@skidata.com>
> 
> Specify the implemented restart handler as a cold one.

 * The current implementation is an issue because ...
 * If this is not fixed, the following will happen ...
 * By fixing this in this way and not another way has ... advantages ...

> Signed-off-by: Benjamin Bara <benjamin.bara@skidata.com>
> ---
>  drivers/mfd/rk8xx-core.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/mfd/rk8xx-core.c b/drivers/mfd/rk8xx-core.c
> index e8fc9e2ab1d0..15e8e6a9943a 100644
> --- a/drivers/mfd/rk8xx-core.c
> +++ b/drivers/mfd/rk8xx-core.c
> @@ -697,7 +697,8 @@ int rk8xx_probe(struct device *dev, int variant, unsigned int irq, struct regmap
>  		case RK809_ID:
>  		case RK817_ID:
>  			ret = devm_register_sys_off_handler(dev,
> -							    SYS_OFF_MODE_RESTART, SYS_OFF_PRIO_HIGH,
> +							    SYS_OFF_MODE_RESTART_COLD,
> +							    SYS_OFF_PRIO_HIGH,
>  							    &rk808_restart, rk808);
>  			if (ret)
>  				dev_warn(dev, "failed to register rst handler, %d\n", ret);
> 
> -- 
> 2.34.1
> 

-- 
Lee Jones [李琼斯]

[PATCH 3/6] soc/tegra: pmc: Specify restart mode

[Benjamin Bara]

From: Benjamin Bara <benjamin.bara@skidata.com>

Specify the implemented restart handler as a warm one.

Signed-off-by: Benjamin Bara <benjamin.bara@skidata.com>
---
 drivers/soc/tegra/pmc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/soc/tegra/pmc.c b/drivers/soc/tegra/pmc.c
index 162f52456f65..4f42febb9b0f 100644
--- a/drivers/soc/tegra/pmc.c
+++ b/drivers/soc/tegra/pmc.c
@@ -2962,7 +2962,7 @@ static int tegra_pmc_probe(struct platform_device *pdev)
 	}
 
 	err = devm_register_sys_off_handler(&pdev->dev,
-					    SYS_OFF_MODE_RESTART,
+					    SYS_OFF_MODE_RESTART_WARM,
 					    SYS_OFF_PRIO_LOW,
 					    tegra_pmc_restart_handler, NULL);
 	if (err) {

-- 
2.34.1

Re: [PATCH 3/6] soc/tegra: pmc: Specify restart mode

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 382 bytes --]

On Thu, Jul 27, 2023 at 05:26:38PM +0200, Benjamin Bara wrote:
> From: Benjamin Bara <benjamin.bara@skidata.com>
> 
> Specify the implemented restart handler as a warm one.
> 
> Signed-off-by: Benjamin Bara <benjamin.bara@skidata.com>
> ---
>  drivers/soc/tegra/pmc.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Acked-by: Thierry Reding <treding@nvidia.com>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH 4/6] mfd: tps65219: Specify restart mode

[Benjamin Bara]

From: Benjamin Bara <benjamin.bara@skidata.com>

As the tps65219 supports both, a cold and a warm reset, register a cold
and a warm restart handler.

Signed-off-by: Benjamin Bara <benjamin.bara@skidata.com>
---
 drivers/mfd/tps65219.c | 17 +++++++++++++----
 1 file changed, 13 insertions(+), 4 deletions(-)

diff --git a/drivers/mfd/tps65219.c b/drivers/mfd/tps65219.c
index 0e0c42e4fdfc..85752b93256e 100644
--- a/drivers/mfd/tps65219.c
+++ b/drivers/mfd/tps65219.c
@@ -278,12 +278,21 @@ static int tps65219_probe(struct i2c_client *client)
 		}
 	}
 
-	ret = devm_register_restart_handler(tps->dev,
-					    tps65219_restart_handler,
-					    tps);
+	ret = devm_register_cold_restart_handler(tps->dev,
+						 tps65219_restart_handler,
+						 tps);
 
 	if (ret) {
-		dev_err(tps->dev, "cannot register restart handler, %d\n", ret);
+		dev_err(tps->dev, "cannot register cold restart handler, %d\n", ret);
+		return ret;
+	}
+
+	ret = devm_register_warm_restart_handler(tps->dev,
+						 tps65219_restart_handler,
+						 tps);
+
+	if (ret) {
+		dev_err(tps->dev, "cannot register warm restart handler, %d\n", ret);
 		return ret;
 	}
 

-- 
2.34.1

[PATCH 5/6] kernel/reboot: remove generic restart mode

[Benjamin Bara]

From: Benjamin Bara <benjamin.bara@skidata.com>

Remove the "unspecified" restart mode, as it is not in use anymore. New
handler registrations should use the specified "cold" or "warm" instead.

Signed-off-by: Benjamin Bara <benjamin.bara@skidata.com>
---
 include/linux/reboot.h | 11 -----------
 kernel/reboot.c        | 26 --------------------------
 2 files changed, 37 deletions(-)

diff --git a/include/linux/reboot.h b/include/linux/reboot.h
index 05199aedb696..ad1a7b4026c0 100644
--- a/include/linux/reboot.h
+++ b/include/linux/reboot.h
@@ -113,13 +113,6 @@ enum sys_off_mode {
 	 */
 	SYS_OFF_MODE_RESTART_PREPARE,
 
-	/**
-	 * @SYS_OFF_MODE_RESTART:
-	 *
-	 * Handlers restart system. Handlers are disallowed to sleep.
-	 */
-	SYS_OFF_MODE_RESTART,
-
 	/**
 	 * @SYS_OFF_MODE_RESTART_COLD:
 	 *
@@ -167,10 +160,6 @@ int devm_register_power_off_handler(struct device *dev,
 				    int (*callback)(struct sys_off_data *data),
 				    void *cb_data);
 
-int devm_register_restart_handler(struct device *dev,
-				  int (*callback)(struct sys_off_data *data),
-				  void *cb_data);
-
 int devm_register_cold_restart_handler(struct device *dev,
 				       int (*callback)(struct sys_off_data *data),
 				       void *cb_data);
diff --git a/kernel/reboot.c b/kernel/reboot.c
index d0d3faf70a79..072489e48636 100644
--- a/kernel/reboot.c
+++ b/kernel/reboot.c
@@ -441,10 +441,6 @@ register_sys_off_handler(enum sys_off_mode mode,
 		handler->blocking = true;
 		break;
 
-	case SYS_OFF_MODE_RESTART:
-		handler->list = &warm_restart_handler_list;
-		break;
-
 	case SYS_OFF_MODE_RESTART_COLD:
 		handler->list = &cold_restart_handler_list;
 		break;
@@ -574,28 +570,6 @@ int devm_register_power_off_handler(struct device *dev,
 }
 EXPORT_SYMBOL_GPL(devm_register_power_off_handler);
 
-/**
- *	devm_register_restart_handler - Register restart handler
- *	@dev: Device that registers callback
- *	@callback: Callback function
- *	@cb_data: Callback's argument
- *
- *	Registers resource-managed sys-off handler with a default priority
- *	and using restart mode.
- *
- *	Returns zero on success, or error code on failure.
- */
-int devm_register_restart_handler(struct device *dev,
-				  int (*callback)(struct sys_off_data *data),
-				  void *cb_data)
-{
-	return devm_register_sys_off_handler(dev,
-					     SYS_OFF_MODE_RESTART,
-					     SYS_OFF_PRIO_DEFAULT,
-					     callback, cb_data);
-}
-EXPORT_SYMBOL_GPL(devm_register_restart_handler);
-
 /**
  *	devm_register_cold_restart_handler - Register cold restart handler
  *	@dev: Device that registers callback

-- 
2.34.1

[PATCH 6/6] regulator: pca9450: register restart handlers

[Benjamin Bara]

From: Benjamin Bara <benjamin.bara@skidata.com>

The PCA9450 supports both, a warm and a cold reset. Implement both and
register the respective handlers.

Signed-off-by: Benjamin Bara <benjamin.bara@skidata.com>
---
 drivers/regulator/pca9450-regulator.c | 59 +++++++++++++++++++++++++++++++++++
 include/linux/regulator/pca9450.h     |  7 +++++
 2 files changed, 66 insertions(+)

diff --git a/drivers/regulator/pca9450-regulator.c b/drivers/regulator/pca9450-regulator.c
index 2ab365d2749f..86903d677bf6 100644
--- a/drivers/regulator/pca9450-regulator.c
+++ b/drivers/regulator/pca9450-regulator.c
@@ -38,6 +38,11 @@ struct pca9450 {
 	int irq;
 };
 
+static inline struct pca9450 *dev_to_pca9450(struct device *dev)
+{
+	return dev_get_drvdata(dev);
+}
+
 static const struct regmap_range pca9450_status_range = {
 	.range_min = PCA9450_REG_INT1,
 	.range_max = PCA9450_REG_PWRON_STAT,
@@ -219,6 +224,42 @@ static int pca9450_set_dvs_levels(struct device_node *np,
 	return ret;
 }
 
+static int pca9450_cold_reset(struct pca9450 *pca9450)
+{
+	int ret;
+
+	ret = regmap_write(pca9450->regmap, PCA9450_REG_SWRST,
+			   SWRST_RESET_COLD_LDO12);
+	if (ret)
+		return ret;
+
+	/* t_RESTART is 250 ms. */
+	mdelay(500);
+	return -ETIME;
+}
+
+static int pca9450_warm_reset(struct pca9450 *pca9450)
+{
+	int ret;
+
+	ret = regmap_write(pca9450->regmap, PCA9450_REG_SWRST,
+			   SWRST_RESET_WARM);
+	if (ret)
+		return ret;
+
+	/* t_RESET is 20 ms. */
+	mdelay(50);
+	return -ETIME;
+}
+
+static int pca9450_restart_handler(struct sys_off_data *data)
+{
+	int (*handler)(struct pca9450 *) = data->cb_data;
+	struct pca9450 *pca9450 = dev_to_pca9450(data->dev);
+
+	return handler(pca9450);
+}
+
 static const struct pca9450_regulator_desc pca9450a_regulators[] = {
 	{
 		.desc = {
@@ -845,6 +886,24 @@ static int pca9450_i2c_probe(struct i2c_client *i2c)
 		return PTR_ERR(pca9450->sd_vsel_gpio);
 	}
 
+	ret = devm_register_cold_restart_handler(pca9450->dev,
+						 pca9450_restart_handler,
+						 pca9450_cold_reset);
+	if (ret) {
+		dev_err(&i2c->dev, "register cold restart handler failed: %d\n",
+			ret);
+		return ret;
+	}
+
+	ret = devm_register_warm_restart_handler(pca9450->dev,
+						 pca9450_restart_handler,
+						 pca9450_warm_reset);
+	if (ret) {
+		dev_err(&i2c->dev, "register warm restart handler failed: %d\n",
+			ret);
+		return ret;
+	}
+
 	dev_info(&i2c->dev, "%s probed.\n",
 		type == PCA9450_TYPE_PCA9450A ? "pca9450a" : "pca9450bc");
 
diff --git a/include/linux/regulator/pca9450.h b/include/linux/regulator/pca9450.h
index 505c908dbb81..a72fd4942d5f 100644
--- a/include/linux/regulator/pca9450.h
+++ b/include/linux/regulator/pca9450.h
@@ -93,6 +93,13 @@ enum {
 	PCA9450_MAX_REGISTER	    = 0x2F,
 };
 
+/* PCA9450 SW_RST bits */
+#define SWRST_NOACTION			0x00
+#define SWRST_RESET_REGS		0x05
+#define SWRST_RESET_COLD_LDO12		0x14
+#define SWRST_RESET_WARM		0x35
+#define SWRST_RESET_COLD		0x64
+
 /* PCA9450 BUCK ENMODE bits */
 #define BUCK_ENMODE_OFF			0x00
 #define BUCK_ENMODE_ONREQ		0x01

-- 
2.34.1

Re: [PATCH 6/6] regulator: pca9450: register restart handlers

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 275 bytes --]

On Thu, Jul 27, 2023 at 05:26:41PM +0200, Benjamin Bara wrote:
> From: Benjamin Bara <benjamin.bara@skidata.com>
> 
> The PCA9450 supports both, a warm and a cold reset. Implement both and
> register the respective handlers.

Acked-by: Mark Brown <broonie@kernel.org>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]