| Message ID | 20230327-tegra-pmic-reboot-v6-4-af44a4cd82e9@skidata.com |
|---|---|
| State | Handled Elsewhere |
| Headers | show |
| Series | mfd: tps6586x: register restart handler | expand |
On Tue, 09 May 2023, 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(-) Do the 2 MFD patches depend on the others? For my own reference (apply this as-is to your sign-off block): Acked-for-MFD-by: Lee Jones <lee@kernel.org>
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/
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 --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;