On 3/16/23 19:47, Benjamin Bara wrote: > From: Benjamin Bara <benjamin.bara@xxxxxxxxxxx> > > 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. > > Use it to implement and register a restart handler/notifier. > The DS does not clarify if only writing the SOFT RST bit has side effects. > Therefore, regmap_update_bits() is used. > > Set an appropriate system_state and disable the IRQs to activate > i2c_in_atomic_xfer_mode(). This avoids the following WARNING: > [ 12.667612] WARNING: CPU: 1 PID: 1 at kernel/rcu/tree_plugin.h:318 rcu_note_context_switch+0x33c/0x6b0 > [ 12.676926] Voluntary context switch within RCU read-side critical section! > ... > [ 12.742376] schedule_timeout from wait_for_completion_timeout+0x90/0x114 > [ 12.749179] wait_for_completion_timeout from tegra_i2c_wait_completion+0x40/0x70 > > Tested on a TPS658640. > > Signed-off-by: Benjamin Bara <benjamin.bara@xxxxxxxxxxx> > --- > drivers/mfd/tps6586x.c | 38 ++++++++++++++++++++++++++++++++++++-- > 1 file changed, 36 insertions(+), 2 deletions(-) > > diff --git a/drivers/mfd/tps6586x.c b/drivers/mfd/tps6586x.c > index 2d947f3f606a..e40c664b5f64 100644 > --- a/drivers/mfd/tps6586x.c > +++ b/drivers/mfd/tps6586x.c > @@ -15,6 +15,7 @@ > #include <linux/interrupt.h> > #include <linux/irq.h> > #include <linux/irqdomain.h> > +#include <linux/irqflags.h> > #include <linux/kernel.h> > #include <linux/module.h> > #include <linux/mutex.h> > @@ -22,6 +23,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> > > @@ -29,6 +31,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) > > @@ -466,6 +469,23 @@ static void tps6586x_power_off(void) > tps6586x_set_bits(tps6586x_dev, TPS6586X_SUPPLYENE, SLEEP_MODE_BIT); > } > > +static int tps6586x_restart_notify(struct notifier_block *this, unsigned long mode, void *data) > +{ > + /* bring pmic into HARD REBOOT state, disable IRQs for atomic i2c RCU */ > + system_state = SYSTEM_RESTART; > + local_irq_disable(); > + tps6586x_set_bits(tps6586x_dev, TPS6586X_SUPPLYENE, SOFT_RST_BIT); > + local_irq_enable(); > + > + mdelay(500); > + return NOTIFY_DONE; > +} > + > +static struct notifier_block tps6586x_restart_handler = { > + .notifier_call = tps6586x_restart_notify, > + .priority = 200, > +}; > + > static void tps6586x_print_version(struct i2c_client *client, int version) > { > const char *name; > @@ -559,9 +579,16 @@ static int tps6586x_i2c_probe(struct i2c_client *client) > goto err_add_devs; > } > > - if (pdata->pm_off && !pm_power_off) { > + if (pdata->pm_off) { > tps6586x_dev = &client->dev; > - pm_power_off = tps6586x_power_off; > + if (!pm_power_off) > + pm_power_off = tps6586x_power_off; > + > + ret = register_restart_handler(&tps6586x_restart_handler); > + if (ret) { > + dev_err(&client->dev, "register restart handler failed: %d\n", ret); > + goto err_add_devs; > + } > } > > return 0; > @@ -582,6 +609,13 @@ static void tps6586x_i2c_remove(struct i2c_client *client) > mfd_remove_devices(tps6586x->dev); > if (client->irq) > free_irq(client->irq, tps6586x); > + > + if (tps6586x_dev == &client->dev) { > + tps6586x_dev = NULL; > + unregister_restart_handler(&tps6586x_restart_handler); > + if (pm_power_off == tps6586x_power_off) > + pm_power_off = NULL; > + } There are newer devm_register_power_off/restart_handler() helpers in kernel [1]. Will be great if you'll use them in v2. [1] https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/include/linux/reboot.h -- Best regards, Dmitry