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

There are a couple of boards which use a tps6586x as
"ti,system-power-controller", e.g. the tegra20-tamonten.dtsi.
For these, the only registered restart handler is the warm reboot via
tegra's PMC. As the bootloader of the tegra20 requires the VDE, it must
be ensured that VDE is enabled (which is the case after a cold reboot).
For the "normal reboot", this is basically the case since
8f0c714ad9be1ef774c98e8819a7a571451cb019.
However, this workaround is not executed in case of an emergency restart.
In case of an emergency restart, the system now simply hangs in the
bootloader, as VDE is not enabled (as not in use).

The TPS658629-Q1 (unfortunately the only TPS6586x with public data sheet)
provides a SOFT RST bit in the SUPPLYENE reg to request a (cold) reboot.
This avoids the hang-up.

Tested on a TPS658640.

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

diff --git a/drivers/mfd/tps6586x.c b/drivers/mfd/tps6586x.c
index 93f1bf440191..c8adf6a08277 100644
--- a/drivers/mfd/tps6586x.c
+++ b/drivers/mfd/tps6586x.c
@@ -30,6 +30,7 @@
 #include <linux/mfd/tps6586x.h>
 
 #define TPS6586X_SUPPLYENE	0x14
+#define SOFT_RST_BIT		BIT(0)
 #define EXITSLREQ_BIT		BIT(1)
 #define SLEEP_MODE_BIT		BIT(3)
 
@@ -470,6 +471,17 @@ static int tps6586x_power_off_handler(struct sys_off_data *data)
 	return tps6586x_set_bits(tps6586x_dev, TPS6586X_SUPPLYENE, SLEEP_MODE_BIT);
 }
 
+static int tps6586x_restart_handler(struct sys_off_data *data)
+{
+	struct device *tps6586x_dev = data->cb_data;
+
+	/* bring pmic into HARD REBOOT state */
+	tps6586x_set_bits(tps6586x_dev, TPS6586X_SUPPLYENE, SOFT_RST_BIT);
+
+	mdelay(500);
+	return 0;
+}
+
 static void tps6586x_print_version(struct i2c_client *client, int version)
 {
 	const char *name;
@@ -570,6 +582,13 @@ static int tps6586x_i2c_probe(struct i2c_client *client)
 			dev_err(&client->dev, "register power off handler failed: %d\n", ret);
 			goto err_add_devs;
 		}
+
+		ret = devm_register_restart_handler(&client->dev, &tps6586x_restart_handler,
+						    &client->dev);
+		if (ret) {
+			dev_err(&client->dev, "register restart handler failed: %d\n", ret);
+			goto err_add_devs;
+		}
 	}
 
 	return 0;

-- 
2.34.1

On 3/27/23 16:57, Benjamin Bara wrote:
> From: Benjamin Bara <benjamin.bara@skidata.com>
> 
> There are a couple of boards which use a tps6586x as
> "ti,system-power-controller", e.g. the tegra20-tamonten.dtsi.
> For these, the only registered restart handler is the warm reboot via
> tegra's PMC. As the bootloader of the tegra20 requires the VDE, it must
> be ensured that VDE is enabled (which is the case after a cold reboot).
> For the "normal reboot", this is basically the case since
> 8f0c714ad9be1ef774c98e8819a7a571451cb019.
> However, this workaround is not executed in case of an emergency restart.
> In case of an emergency restart, the system now simply hangs in the
> bootloader, as VDE is not enabled (as not in use).
> 
> The TPS658629-Q1 (unfortunately the only TPS6586x with public data sheet)
> provides a SOFT RST bit in the SUPPLYENE reg to request a (cold) reboot.
> This avoids the hang-up.
> 
> Tested on a TPS658640.
> 
> Signed-off-by: Benjamin Bara <benjamin.bara@skidata.com>
> ---
>  drivers/mfd/tps6586x.c | 19 +++++++++++++++++++
>  1 file changed, 19 insertions(+)
> 
> diff --git a/drivers/mfd/tps6586x.c b/drivers/mfd/tps6586x.c
> index 93f1bf440191..c8adf6a08277 100644
> --- a/drivers/mfd/tps6586x.c
> +++ b/drivers/mfd/tps6586x.c
> @@ -30,6 +30,7 @@
>  #include <linux/mfd/tps6586x.h>
>  
>  #define TPS6586X_SUPPLYENE	0x14
> +#define SOFT_RST_BIT		BIT(0)
>  #define EXITSLREQ_BIT		BIT(1)
>  #define SLEEP_MODE_BIT		BIT(3)
>  
> @@ -470,6 +471,17 @@ static int tps6586x_power_off_handler(struct sys_off_data *data)
>  	return tps6586x_set_bits(tps6586x_dev, TPS6586X_SUPPLYENE, SLEEP_MODE_BIT);
>  }
>  
> +static int tps6586x_restart_handler(struct sys_off_data *data)
> +{
> +	struct device *tps6586x_dev = data->cb_data;
> +
> +	/* bring pmic into HARD REBOOT state */
> +	tps6586x_set_bits(tps6586x_dev, TPS6586X_SUPPLYENE, SOFT_RST_BIT);
> +
> +	mdelay(500);
Is this delay needed? There is no delay in a case of power off.

-- 
Best regards,
Dmitry

Thanks for the feedback!

The DS states: "When the reboot request state is set an internal timer TWAIT
(10ms typ) is started. The reboot request ends when t > TWAIT."

Therefore, my intention was to wait a little bit before starting the next
handler in the chain. I can do some tests without the mdelay, but otherwise
will reduce it to 15ms in the next patch. What do you think about it?

BR,
Benjamin

On 4/3/23 09:50, Benjamin Bara wrote:
> Thanks for the feedback!
> 
> The DS states: "When the reboot request state is set an internal timer TWAIT
> (10ms typ) is started. The reboot request ends when t > TWAIT."
> 
> Therefore, my intention was to wait a little bit before starting the next
> handler in the chain. I can do some tests without the mdelay, but otherwise
> will reduce it to 15ms in the next patch. What do you think about it?

Sounds good. Please add a clarifying comment for the delay to the code.

-- 
Best regards,
Dmitry