Message ID | 20230316164703.1157813-1-bbara93@gmail.com |
---|---|
State | Handled Elsewhere |
Headers | show |
Series | mfd: tps6586x: register restart handler | expand |
On 3/16/23 19:47, Benjamin Bara wrote: > From: Benjamin Bara <benjamin.bara@skidata.com> > > 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@skidata.com> > --- > 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
On Sat, Mar 18, 2023 at 8:15 PM Dmitry Osipenko <dmitry.osipenko@collabora.com> wrote: > There are newer devm_register_power_off/restart_handler() helpers in > kernel [1]. Will be great if you'll use them in v2. Thanks! I will change my implementation to use devm_register_restart_handler() and update the current pm_power_off to devm_register_power_off_handler(). Best regards, Benjamin
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; + } } static int __maybe_unused tps6586x_i2c_suspend(struct device *dev)