| Message ID | 20211124083258.2606511-2-andrej.picej@norik.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [1/2] mfd: da9062: make register CONFIG_I writable | expand |
On 24 November 2021 08:33, Andrej Picej wrote: > From: Stefan Christ <s.christ@phytec.de> > > In the default fuse configuration the watchdog poweroffs the system > instead of resetting it on a watchdog timeout. This patch changes the > behavior. Now the board is reseted and reboots. > > Note: This patch requires that the config register CONFIG_I is > configured as writable in the da9062 multi function device. I'm a little concerned about forcing this change in the driver. There may be platforms which don't want the PMIC to perform a full reset through OTP re-read and if we hard code this change then that's impacting those platforms. If we want/need this then I think it should probably be a DT binding for da9061/2 which then indicates the behaviour we want. NRES_MODE bit also plays a part here as it controls whether or not the nRESET line state is changed as part of the power-down/up process. I'm assuming for your setup this bit is 0? > > Signed-off-by: Stefan Christ <s.christ@phytec.de> > Signed-off-by: Andrej Picej <andrej.picej@norik.com> > --- > drivers/watchdog/da9062_wdt.c | 14 ++++++++++++++ > 1 file changed, 14 insertions(+) > > diff --git a/drivers/watchdog/da9062_wdt.c b/drivers/watchdog/da9062_wdt.c > index f02cbd530538..a04613e68f4a 100644 > --- a/drivers/watchdog/da9062_wdt.c > +++ b/drivers/watchdog/da9062_wdt.c > @@ -87,6 +87,20 @@ static int da9062_wdt_start(struct watchdog_device *wdd) > unsigned int selector; > int ret; > > + /* > + * Use da9062's SHUTDOWN mode instead of POWERDOWN for > watchdog reset. > + * On timeout the PMIC should reset the system, not powering it > + * off. > + */ > + ret = regmap_update_bits(wdt->hw->regmap, > + DA9062AA_CONFIG_I, > + DA9062AA_WATCHDOG_SD_MASK, > + DA9062AA_WATCHDOG_SD_MASK); > + if (ret) > + dev_err(wdt->hw->dev, > + "failed to set wdt reset mode. Expect poweroff on > watchdog reset: %d\n", > + ret); > + > selector = da9062_wdt_timeout_to_sel(wdt->wdtdev.timeout); > ret = da9062_wdt_update_timeout_register(wdt, selector); > if (ret) > -- > 2.25.1
Hi Adam, On 26. 11. 21 14:28, Adam Thomson wrote: > > I'm a little concerned about forcing this change in the driver. There may be > platforms which don't want the PMIC to perform a full reset through OTP re-read > and if we hard code this change then that's impacting those platforms. If we > want/need this then I think it should probably be a DT binding for da9061/2 > which then indicates the behaviour we want. Ok, I see the impact this might have on the platforms that are relying on the current default setting. I will start on the DT binding implementation and submit a new patch. > > NRES_MODE bit also plays a part here as it controls whether or not the nRESET > line state is changed as part of the power-down/up process. I'm assuming for > your setup this bit is 0? > We leave NRES_MODE as it is, 0 by default I guess? So do you want a separate dt binding for NRES_MODE? BR, Andrej
On 29 November 2021 12:12, Andrej Picej wrote: > > I'm a little concerned about forcing this change in the driver. There may be > > platforms which don't want the PMIC to perform a full reset through OTP re- > read > > and if we hard code this change then that's impacting those platforms. If we > > want/need this then I think it should probably be a DT binding for da9061/2 > > which then indicates the behaviour we want. > > Ok, I see the impact this might have on the platforms that are relying > on the current default setting. I will start on the DT binding > implementation and submit a new patch. Thanks. Probably the safest route. > > > > > NRES_MODE bit also plays a part here as it controls whether or not the nRESET > > line state is changed as part of the power-down/up process. I'm assuming for > > your setup this bit is 0? > > > We leave NRES_MODE as it is, 0 by default I guess? So do you want a > separate dt binding for NRES_MODE? I think the setting very much depends on the customers OTP config selection. For now we can leave this option. Just wanted to highlight it.
diff --git a/drivers/watchdog/da9062_wdt.c b/drivers/watchdog/da9062_wdt.c index f02cbd530538..a04613e68f4a 100644 --- a/drivers/watchdog/da9062_wdt.c +++ b/drivers/watchdog/da9062_wdt.c @@ -87,6 +87,20 @@ static int da9062_wdt_start(struct watchdog_device *wdd) unsigned int selector; int ret; + /* + * Use da9062's SHUTDOWN mode instead of POWERDOWN for watchdog reset. + * On timeout the PMIC should reset the system, not powering it + * off. + */ + ret = regmap_update_bits(wdt->hw->regmap, + DA9062AA_CONFIG_I, + DA9062AA_WATCHDOG_SD_MASK, + DA9062AA_WATCHDOG_SD_MASK); + if (ret) + dev_err(wdt->hw->dev, + "failed to set wdt reset mode. Expect poweroff on watchdog reset: %d\n", + ret); + selector = da9062_wdt_timeout_to_sel(wdt->wdtdev.timeout); ret = da9062_wdt_update_timeout_register(wdt, selector); if (ret)