diff mbox series

[v6,4/5] mfd: tps6586x: use devm-based power off handler

Message ID 20230327-tegra-pmic-reboot-v6-4-af44a4cd82e9@skidata.com
State Superseded
Headers show
Series mfd: tps6586x: register restart handler | expand

Commit Message

Benjamin Bara May 9, 2023, 7:03 p.m. UTC
From: Benjamin Bara <benjamin.bara@skidata.com>

Convert the power off handler to a devm-based power off handler.

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

Comments

Benjamin Bara May 18, 2023, 11:15 a.m. UTC | #1
On Thu, 18 May 2023 at 11:43, Lee Jones <lee@kernel.org> wrote:
> Do the 2 MFD patches depend on the others?

They depend on 3/5, which is an extension to [1] and makes the
respective device available to its sys-off handler.

1/5 and 2/5 avoid a warning which is shown if the handler is called from
an emergency restart (e.g. panic()). The reason behind it is that the
i2c transfer currently doesn't recognize that it should be atomic in
this phase and utilizes the DMA instead, which schedules out while
waiting for completion ("Voluntary context switch within RCU read-side
critical section!").

[1] https://lore.kernel.org/lkml/20220509233235.995021-4-dmitry.osipenko@collabora.com/
Dmitry Osipenko May 18, 2023, 11:40 a.m. UTC | #2
On 5/9/23 22:03, Benjamin Bara wrote:
> From: Benjamin Bara <benjamin.bara@skidata.com>
> 
> Convert the power off handler to a devm-based power off handler.
> 
> Signed-off-by: Benjamin Bara <benjamin.bara@skidata.com>
> ---
>  drivers/mfd/tps6586x.c | 29 +++++++++++++++++++++--------
>  1 file changed, 21 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/mfd/tps6586x.c b/drivers/mfd/tps6586x.c
> index 2d947f3f606a..b12c9e18970a 100644
> --- a/drivers/mfd/tps6586x.c
> +++ b/drivers/mfd/tps6586x.c
> @@ -22,6 +22,7 @@
>  #include <linux/err.h>
>  #include <linux/i2c.h>
>  #include <linux/platform_device.h>
> +#include <linux/reboot.h>
>  #include <linux/regmap.h>
>  #include <linux/of.h>
>  
> @@ -457,13 +458,21 @@ static const struct regmap_config tps6586x_regmap_config = {
>  	.cache_type = REGCACHE_RBTREE,
>  };
>  
> -static struct device *tps6586x_dev;
> -static void tps6586x_power_off(void)
> +static int tps6586x_power_off_handler(struct sys_off_data *data)
>  {
> -	if (tps6586x_clr_bits(tps6586x_dev, TPS6586X_SUPPLYENE, EXITSLREQ_BIT))
> -		return;
> +	int ret;
> +
> +	/* Put the PMIC into sleep state. This takes at least 20ms. */
> +	ret = tps6586x_clr_bits(data->dev, TPS6586X_SUPPLYENE, EXITSLREQ_BIT);
> +	if (ret)
> +		return notifier_from_errno(ret);
> +
> +	ret = tps6586x_set_bits(data->dev, TPS6586X_SUPPLYENE, SLEEP_MODE_BIT);
> +	if (ret)
> +		return notifier_from_errno(ret);
>  
> -	tps6586x_set_bits(tps6586x_dev, TPS6586X_SUPPLYENE, SLEEP_MODE_BIT);
> +	mdelay(50);
> +	return notifier_from_errno(-ETIME);
>  }
>  
>  static void tps6586x_print_version(struct i2c_client *client, int version)
> @@ -559,9 +568,13 @@ static int tps6586x_i2c_probe(struct i2c_client *client)
>  		goto err_add_devs;
>  	}
>  
> -	if (pdata->pm_off && !pm_power_off) {
> -		tps6586x_dev = &client->dev;
> -		pm_power_off = tps6586x_power_off;
> +	if (pdata->pm_off) {
> +		ret = devm_register_power_off_handler(&client->dev, &tps6586x_power_off_handler,
> +						      NULL);
> +		if (ret) {
> +			dev_err(&client->dev, "register power off handler failed: %d\n", ret);
> +			goto err_add_devs;
> +		}
>  	}
>  
>  	return 0;
> 

Reviewed-by: Dmitry Osipenko <dmitry.osipenko@collabora.com>
diff mbox series

Patch

diff --git a/drivers/mfd/tps6586x.c b/drivers/mfd/tps6586x.c
index 2d947f3f606a..b12c9e18970a 100644
--- a/drivers/mfd/tps6586x.c
+++ b/drivers/mfd/tps6586x.c
@@ -22,6 +22,7 @@ 
 #include <linux/err.h>
 #include <linux/i2c.h>
 #include <linux/platform_device.h>
+#include <linux/reboot.h>
 #include <linux/regmap.h>
 #include <linux/of.h>
 
@@ -457,13 +458,21 @@  static const struct regmap_config tps6586x_regmap_config = {
 	.cache_type = REGCACHE_RBTREE,
 };
 
-static struct device *tps6586x_dev;
-static void tps6586x_power_off(void)
+static int tps6586x_power_off_handler(struct sys_off_data *data)
 {
-	if (tps6586x_clr_bits(tps6586x_dev, TPS6586X_SUPPLYENE, EXITSLREQ_BIT))
-		return;
+	int ret;
+
+	/* Put the PMIC into sleep state. This takes at least 20ms. */
+	ret = tps6586x_clr_bits(data->dev, TPS6586X_SUPPLYENE, EXITSLREQ_BIT);
+	if (ret)
+		return notifier_from_errno(ret);
+
+	ret = tps6586x_set_bits(data->dev, TPS6586X_SUPPLYENE, SLEEP_MODE_BIT);
+	if (ret)
+		return notifier_from_errno(ret);
 
-	tps6586x_set_bits(tps6586x_dev, TPS6586X_SUPPLYENE, SLEEP_MODE_BIT);
+	mdelay(50);
+	return notifier_from_errno(-ETIME);
 }
 
 static void tps6586x_print_version(struct i2c_client *client, int version)
@@ -559,9 +568,13 @@  static int tps6586x_i2c_probe(struct i2c_client *client)
 		goto err_add_devs;
 	}
 
-	if (pdata->pm_off && !pm_power_off) {
-		tps6586x_dev = &client->dev;
-		pm_power_off = tps6586x_power_off;
+	if (pdata->pm_off) {
+		ret = devm_register_power_off_handler(&client->dev, &tps6586x_power_off_handler,
+						      NULL);
+		if (ret) {
+			dev_err(&client->dev, "register power off handler failed: %d\n", ret);
+			goto err_add_devs;
+		}
 	}
 
 	return 0;