| Message ID | 20240821201935.1698146-1-jm@ti.com |
|---|---|
| State | New |
| Headers | show |
| Series | watchdog: rti_wdt: Allow timeout config with timeout-sec | expand |
On Wed, Aug 21, 2024 at 03:19:35PM -0500, Judith Mendez wrote: > Currently rti_wdt does not allow timeout to be configured > via DT property timeout-sec, so fix watchdog_init_timeout > to be able to use timeout-sec. > > Signed-off-by: Judith Mendez <jm@ti.com> > --- > drivers/watchdog/rti_wdt.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/watchdog/rti_wdt.c b/drivers/watchdog/rti_wdt.c > index 8e1be7ba01039..7260c67e60a25 100644 > --- a/drivers/watchdog/rti_wdt.c > +++ b/drivers/watchdog/rti_wdt.c > @@ -332,7 +332,8 @@ static int rti_wdt_probe(struct platform_device *pdev) > memunmap(vaddr); > } > > - watchdog_init_timeout(wdd, heartbeat, dev); > + wdd->timeout = heartbeat; The proper fix would be to initialize the 'heartbeat' variable with 0, set wdt->timeout to DEFAULT_HEARTBEAT, and to keep passing 'heartbeat' to watchdog_init_timeout(). That used to be done but was explicitly changed in an earlier commit, presumably on purpose. I am not inclined to accept a (partial) revert unless the author of commit 5527483f confirms that this is acceptable and desirable. Otherwise we'll just end up in an edit war, which I really don't want to get into. Guenter ` > + watchdog_init_timeout(wdd, 0, dev); > > ret = watchdog_register_device(wdd); > if (ret) { > > base-commit: 860bbe8e618fd62446309e286ab4a83d38201c0a > -- > 2.46.0 >
Hi Guenter, On 8/21/24 6:28 PM, Guenter Roeck wrote: > On Wed, Aug 21, 2024 at 03:19:35PM -0500, Judith Mendez wrote: >> Currently rti_wdt does not allow timeout to be configured >> via DT property timeout-sec, so fix watchdog_init_timeout >> to be able to use timeout-sec. >> >> Signed-off-by: Judith Mendez <jm@ti.com> >> --- >> drivers/watchdog/rti_wdt.c | 3 ++- >> 1 file changed, 2 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/watchdog/rti_wdt.c b/drivers/watchdog/rti_wdt.c >> index 8e1be7ba01039..7260c67e60a25 100644 >> --- a/drivers/watchdog/rti_wdt.c >> +++ b/drivers/watchdog/rti_wdt.c >> @@ -332,7 +332,8 @@ static int rti_wdt_probe(struct platform_device *pdev) >> memunmap(vaddr); >> } >> >> - watchdog_init_timeout(wdd, heartbeat, dev); >> + wdd->timeout = heartbeat; > > The proper fix would be to initialize the 'heartbeat' variable with 0, > set wdt->timeout to DEFAULT_HEARTBEAT, and to keep passing 'heartbeat' > to watchdog_init_timeout(). That used to be done but was explicitly > changed in an earlier commit, presumably on purpose. I am not inclined > to accept a (partial) revert unless the author of commit 5527483f confirms > that this is acceptable and desirable. Otherwise we'll just end up in an > edit war, which I really don't want to get into. You are right, I completely missed this change in this commit due to the subject line not being related. I will try to align with Tero Kristo to see if this is something we can add back. Thanks. ~ Judith > > Guenter > ` > >> + watchdog_init_timeout(wdd, 0, dev); >> >> ret = watchdog_register_device(wdd); >> if (ret) { >> >> base-commit: 860bbe8e618fd62446309e286ab4a83d38201c0a >> -- >> 2.46.0 >>
diff --git a/drivers/watchdog/rti_wdt.c b/drivers/watchdog/rti_wdt.c index 8e1be7ba01039..7260c67e60a25 100644 --- a/drivers/watchdog/rti_wdt.c +++ b/drivers/watchdog/rti_wdt.c @@ -332,7 +332,8 @@ static int rti_wdt_probe(struct platform_device *pdev) memunmap(vaddr); } - watchdog_init_timeout(wdd, heartbeat, dev); + wdd->timeout = heartbeat; + watchdog_init_timeout(wdd, 0, dev); ret = watchdog_register_device(wdd); if (ret) {
Currently rti_wdt does not allow timeout to be configured via DT property timeout-sec, so fix watchdog_init_timeout to be able to use timeout-sec. Signed-off-by: Judith Mendez <jm@ti.com> --- drivers/watchdog/rti_wdt.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) base-commit: 860bbe8e618fd62446309e286ab4a83d38201c0a