Support pre-timeout when the bark irq is avaible.
This is the fifth version of the patchset addressing all the review
issues to date:
v5:
include linux/bits.h
pretimeout only enables IRQs if value != 0
remove unnecessary subtract operation
add clarity to the conditional in the probe function
revert the irq registration changes
v4:
remove unnecessary include and private variable
provide macro for WDT EN register values
use pretimeout as per its API intent
handle EPROBE_DEFER on get_irq
modify the irq registration as per pm8916_wdt
v3
remove unecessary variable from the driver's private storage
v2:
register the pre-timeout notifier.
With the second patch in the set, I took the oportunity to do some
cleanup in the same code base removing an unnecesary variable from the
driver's private storage.
Jorge Ramirez-Ortiz (2):
watchdog: qcom: support pre-timeout when the bark irq is available
watchdog: qcom: remove unnecessary variable from private storage
drivers/watchdog/qcom-wdt.c | 85 +++++++++++++++++++++++++++++++------
1 file changed, 72 insertions(+), 13 deletions(-)
--
2.23.0
Use the bark interrupt as the pre-timeout notifier whenever this
interrupt is available.
By default, the pretimeout notification shall occur one second earlier
than the timeout.
Signed-off-by: Jorge Ramirez-Ortiz <[email protected]>
---
drivers/watchdog/qcom-wdt.c | 70 ++++++++++++++++++++++++++++++++++---
1 file changed, 65 insertions(+), 5 deletions(-)
diff --git a/drivers/watchdog/qcom-wdt.c b/drivers/watchdog/qcom-wdt.c
index 7be7f87be28f..935c78a882a3 100644
--- a/drivers/watchdog/qcom-wdt.c
+++ b/drivers/watchdog/qcom-wdt.c
@@ -1,8 +1,10 @@
// SPDX-License-Identifier: GPL-2.0-only
/* Copyright (c) 2014, The Linux Foundation. All rights reserved.
*/
+#include <linux/bits.h>
#include <linux/clk.h>
#include <linux/delay.h>
+#include <linux/interrupt.h>
#include <linux/io.h>
#include <linux/kernel.h>
#include <linux/module.h>
@@ -19,6 +21,9 @@ enum wdt_reg {
WDT_BITE_TIME,
};
+#define QCOM_WDT_ENABLE BIT(0)
+#define QCOM_WDT_ENABLE_IRQ BIT(1)
+
static const u32 reg_offset_data_apcs_tmr[] = {
[WDT_RST] = 0x38,
[WDT_EN] = 0x40,
@@ -54,15 +59,35 @@ struct qcom_wdt *to_qcom_wdt(struct watchdog_device *wdd)
return container_of(wdd, struct qcom_wdt, wdd);
}
+static inline int qcom_get_enable(struct watchdog_device *wdd)
+{
+ int enable = QCOM_WDT_ENABLE;
+
+ if (wdd->pretimeout)
+ enable |= QCOM_WDT_ENABLE_IRQ;
+
+ return enable;
+}
+
+static irqreturn_t qcom_wdt_isr(int irq, void *arg)
+{
+ struct watchdog_device *wdd = arg;
+
+ watchdog_notify_pretimeout(wdd);
+
+ return IRQ_HANDLED;
+}
+
static int qcom_wdt_start(struct watchdog_device *wdd)
{
struct qcom_wdt *wdt = to_qcom_wdt(wdd);
+ unsigned int bark = wdd->timeout - wdd->pretimeout;
writel(0, wdt_addr(wdt, WDT_EN));
writel(1, wdt_addr(wdt, WDT_RST));
- writel(wdd->timeout * wdt->rate, wdt_addr(wdt, WDT_BARK_TIME));
+ writel(bark * wdt->rate, wdt_addr(wdt, WDT_BARK_TIME));
writel(wdd->timeout * wdt->rate, wdt_addr(wdt, WDT_BITE_TIME));
- writel(1, wdt_addr(wdt, WDT_EN));
+ writel(qcom_get_enable(wdd), wdt_addr(wdt, WDT_EN));
return 0;
}
@@ -89,6 +114,13 @@ static int qcom_wdt_set_timeout(struct watchdog_device *wdd,
return qcom_wdt_start(wdd);
}
+static int qcom_wdt_set_pretimeout(struct watchdog_device *wdd,
+ unsigned int timeout)
+{
+ wdd->pretimeout = timeout;
+ return qcom_wdt_start(wdd);
+}
+
static int qcom_wdt_restart(struct watchdog_device *wdd, unsigned long action,
void *data)
{
@@ -105,7 +137,7 @@ static int qcom_wdt_restart(struct watchdog_device *wdd, unsigned long action,
writel(1, wdt_addr(wdt, WDT_RST));
writel(timeout, wdt_addr(wdt, WDT_BARK_TIME));
writel(timeout, wdt_addr(wdt, WDT_BITE_TIME));
- writel(1, wdt_addr(wdt, WDT_EN));
+ writel(QCOM_WDT_ENABLE, wdt_addr(wdt, WDT_EN));
/*
* Actually make sure the above sequence hits hardware before sleeping.
@@ -121,6 +153,7 @@ static const struct watchdog_ops qcom_wdt_ops = {
.stop = qcom_wdt_stop,
.ping = qcom_wdt_ping,
.set_timeout = qcom_wdt_set_timeout,
+ .set_pretimeout = qcom_wdt_set_pretimeout,
.restart = qcom_wdt_restart,
.owner = THIS_MODULE,
};
@@ -133,6 +166,15 @@ static const struct watchdog_info qcom_wdt_info = {
.identity = KBUILD_MODNAME,
};
+static const struct watchdog_info qcom_wdt_pt_info = {
+ .options = WDIOF_KEEPALIVEPING
+ | WDIOF_MAGICCLOSE
+ | WDIOF_SETTIMEOUT
+ | WDIOF_PRETIMEOUT
+ | WDIOF_CARDRESET,
+ .identity = KBUILD_MODNAME,
+};
+
static void qcom_clk_disable_unprepare(void *data)
{
clk_disable_unprepare(data);
@@ -146,7 +188,7 @@ static int qcom_wdt_probe(struct platform_device *pdev)
struct device_node *np = dev->of_node;
const u32 *regs;
u32 percpu_offset;
- int ret;
+ int irq, ret;
regs = of_device_get_match_data(dev);
if (!regs) {
@@ -204,7 +246,25 @@ static int qcom_wdt_probe(struct platform_device *pdev)
return -EINVAL;
}
- wdt->wdd.info = &qcom_wdt_info;
+ /* check if there is pretimeout support */
+ irq = platform_get_irq(pdev, 0);
+ if (irq > 0) {
+ ret = devm_request_irq(dev, irq, qcom_wdt_isr,
+ IRQF_TRIGGER_RISING,
+ "wdt_bark", &wdt->wdd);
+ if (ret)
+ return ret;
+
+ wdt->wdd.info = &qcom_wdt_pt_info;
+ wdt->wdd.pretimeout = 1;
+ } else {
+ if (irq == -EPROBE_DEFER)
+ return -EPROBE_DEFER;
+
+ wdt->wdd.info = &qcom_wdt_info;
+ wdt->wdd.pretimeout = 0;
+ }
+
wdt->wdd.ops = &qcom_wdt_ops;
wdt->wdd.min_timeout = 1;
wdt->wdd.max_timeout = 0x10000000U / wdt->rate;
--
2.23.0
there is no need to continue keeping the clock in private storage.
Signed-off-by: Jorge Ramirez-Ortiz <[email protected]>
---
drivers/watchdog/qcom-wdt.c | 15 +++++++--------
1 file changed, 7 insertions(+), 8 deletions(-)
diff --git a/drivers/watchdog/qcom-wdt.c b/drivers/watchdog/qcom-wdt.c
index 935c78a882a3..e98f5a3d83ea 100644
--- a/drivers/watchdog/qcom-wdt.c
+++ b/drivers/watchdog/qcom-wdt.c
@@ -42,7 +42,6 @@ static const u32 reg_offset_data_kpss[] = {
struct qcom_wdt {
struct watchdog_device wdd;
- struct clk *clk;
unsigned long rate;
void __iomem *base;
const u32 *layout;
@@ -189,6 +188,7 @@ static int qcom_wdt_probe(struct platform_device *pdev)
const u32 *regs;
u32 percpu_offset;
int irq, ret;
+ struct clk *clk;
regs = of_device_get_match_data(dev);
if (!regs) {
@@ -215,19 +215,18 @@ static int qcom_wdt_probe(struct platform_device *pdev)
if (IS_ERR(wdt->base))
return PTR_ERR(wdt->base);
- wdt->clk = devm_clk_get(dev, NULL);
- if (IS_ERR(wdt->clk)) {
+ clk = devm_clk_get(dev, NULL);
+ if (IS_ERR(clk)) {
dev_err(dev, "failed to get input clock\n");
- return PTR_ERR(wdt->clk);
+ return PTR_ERR(clk);
}
- ret = clk_prepare_enable(wdt->clk);
+ ret = clk_prepare_enable(clk);
if (ret) {
dev_err(dev, "failed to setup clock\n");
return ret;
}
- ret = devm_add_action_or_reset(dev, qcom_clk_disable_unprepare,
- wdt->clk);
+ ret = devm_add_action_or_reset(dev, qcom_clk_disable_unprepare, clk);
if (ret)
return ret;
@@ -239,7 +238,7 @@ static int qcom_wdt_probe(struct platform_device *pdev)
* that it would bite before a second elapses it's usefulness is
* limited. Bail if this is the case.
*/
- wdt->rate = clk_get_rate(wdt->clk);
+ wdt->rate = clk_get_rate(clk);
if (wdt->rate == 0 ||
wdt->rate > 0x10000000U) {
dev_err(dev, "invalid clock rate\n");
--
2.23.0
On Fri, Sep 06, 2019 at 10:54:10PM +0200, Jorge Ramirez-Ortiz wrote:
> Use the bark interrupt as the pre-timeout notifier whenever this
> interrupt is available.
>
> By default, the pretimeout notification shall occur one second earlier
> than the timeout.
>
> Signed-off-by: Jorge Ramirez-Ortiz <[email protected]>
Nitpick below, otherwise:
Reviewed-by: Guenter Roeck <[email protected]>
> ---
> drivers/watchdog/qcom-wdt.c | 70 ++++++++++++++++++++++++++++++++++---
> 1 file changed, 65 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/watchdog/qcom-wdt.c b/drivers/watchdog/qcom-wdt.c
> index 7be7f87be28f..935c78a882a3 100644
> --- a/drivers/watchdog/qcom-wdt.c
> +++ b/drivers/watchdog/qcom-wdt.c
> @@ -1,8 +1,10 @@
> // SPDX-License-Identifier: GPL-2.0-only
> /* Copyright (c) 2014, The Linux Foundation. All rights reserved.
> */
> +#include <linux/bits.h>
> #include <linux/clk.h>
> #include <linux/delay.h>
> +#include <linux/interrupt.h>
> #include <linux/io.h>
> #include <linux/kernel.h>
> #include <linux/module.h>
> @@ -19,6 +21,9 @@ enum wdt_reg {
> WDT_BITE_TIME,
> };
>
> +#define QCOM_WDT_ENABLE BIT(0)
> +#define QCOM_WDT_ENABLE_IRQ BIT(1)
> +
> static const u32 reg_offset_data_apcs_tmr[] = {
> [WDT_RST] = 0x38,
> [WDT_EN] = 0x40,
> @@ -54,15 +59,35 @@ struct qcom_wdt *to_qcom_wdt(struct watchdog_device *wdd)
> return container_of(wdd, struct qcom_wdt, wdd);
> }
>
> +static inline int qcom_get_enable(struct watchdog_device *wdd)
> +{
> + int enable = QCOM_WDT_ENABLE;
> +
> + if (wdd->pretimeout)
> + enable |= QCOM_WDT_ENABLE_IRQ;
> +
> + return enable;
> +}
> +
> +static irqreturn_t qcom_wdt_isr(int irq, void *arg)
> +{
> + struct watchdog_device *wdd = arg;
> +
> + watchdog_notify_pretimeout(wdd);
> +
> + return IRQ_HANDLED;
> +}
> +
> static int qcom_wdt_start(struct watchdog_device *wdd)
> {
> struct qcom_wdt *wdt = to_qcom_wdt(wdd);
> + unsigned int bark = wdd->timeout - wdd->pretimeout;
>
> writel(0, wdt_addr(wdt, WDT_EN));
> writel(1, wdt_addr(wdt, WDT_RST));
> - writel(wdd->timeout * wdt->rate, wdt_addr(wdt, WDT_BARK_TIME));
> + writel(bark * wdt->rate, wdt_addr(wdt, WDT_BARK_TIME));
> writel(wdd->timeout * wdt->rate, wdt_addr(wdt, WDT_BITE_TIME));
> - writel(1, wdt_addr(wdt, WDT_EN));
> + writel(qcom_get_enable(wdd), wdt_addr(wdt, WDT_EN));
> return 0;
> }
>
> @@ -89,6 +114,13 @@ static int qcom_wdt_set_timeout(struct watchdog_device *wdd,
> return qcom_wdt_start(wdd);
> }
>
> +static int qcom_wdt_set_pretimeout(struct watchdog_device *wdd,
> + unsigned int timeout)
> +{
> + wdd->pretimeout = timeout;
> + return qcom_wdt_start(wdd);
> +}
> +
> static int qcom_wdt_restart(struct watchdog_device *wdd, unsigned long action,
> void *data)
> {
> @@ -105,7 +137,7 @@ static int qcom_wdt_restart(struct watchdog_device *wdd, unsigned long action,
> writel(1, wdt_addr(wdt, WDT_RST));
> writel(timeout, wdt_addr(wdt, WDT_BARK_TIME));
> writel(timeout, wdt_addr(wdt, WDT_BITE_TIME));
> - writel(1, wdt_addr(wdt, WDT_EN));
> + writel(QCOM_WDT_ENABLE, wdt_addr(wdt, WDT_EN));
>
> /*
> * Actually make sure the above sequence hits hardware before sleeping.
> @@ -121,6 +153,7 @@ static const struct watchdog_ops qcom_wdt_ops = {
> .stop = qcom_wdt_stop,
> .ping = qcom_wdt_ping,
> .set_timeout = qcom_wdt_set_timeout,
> + .set_pretimeout = qcom_wdt_set_pretimeout,
> .restart = qcom_wdt_restart,
> .owner = THIS_MODULE,
> };
> @@ -133,6 +166,15 @@ static const struct watchdog_info qcom_wdt_info = {
> .identity = KBUILD_MODNAME,
> };
>
> +static const struct watchdog_info qcom_wdt_pt_info = {
> + .options = WDIOF_KEEPALIVEPING
> + | WDIOF_MAGICCLOSE
> + | WDIOF_SETTIMEOUT
> + | WDIOF_PRETIMEOUT
> + | WDIOF_CARDRESET,
> + .identity = KBUILD_MODNAME,
> +};
> +
> static void qcom_clk_disable_unprepare(void *data)
> {
> clk_disable_unprepare(data);
> @@ -146,7 +188,7 @@ static int qcom_wdt_probe(struct platform_device *pdev)
> struct device_node *np = dev->of_node;
> const u32 *regs;
> u32 percpu_offset;
> - int ret;
> + int irq, ret;
>
> regs = of_device_get_match_data(dev);
> if (!regs) {
> @@ -204,7 +246,25 @@ static int qcom_wdt_probe(struct platform_device *pdev)
> return -EINVAL;
> }
>
> - wdt->wdd.info = &qcom_wdt_info;
> + /* check if there is pretimeout support */
> + irq = platform_get_irq(pdev, 0);
> + if (irq > 0) {
> + ret = devm_request_irq(dev, irq, qcom_wdt_isr,
> + IRQF_TRIGGER_RISING,
> + "wdt_bark", &wdt->wdd);
> + if (ret)
> + return ret;
> +
> + wdt->wdd.info = &qcom_wdt_pt_info;
> + wdt->wdd.pretimeout = 1;
> + } else {
> + if (irq == -EPROBE_DEFER)
> + return -EPROBE_DEFER;
> +
> + wdt->wdd.info = &qcom_wdt_info;
> + wdt->wdd.pretimeout = 0;
It is not necessary to set pretimeout to 0; the data structure was
allocated with devm_kzalloc(). The compiler won't know that and
generate unnecessary code otherwise.
> + }
> +
> wdt->wdd.ops = &qcom_wdt_ops;
> wdt->wdd.min_timeout = 1;
> wdt->wdd.max_timeout = 0x10000000U / wdt->rate;
On Fri, Sep 06, 2019 at 10:54:11PM +0200, Jorge Ramirez-Ortiz wrote:
> there is no need to continue keeping the clock in private storage.
>
> Signed-off-by: Jorge Ramirez-Ortiz <[email protected]>
Good catch.
Reviewed-by: Guenter Roeck <[email protected]>
> ---
> drivers/watchdog/qcom-wdt.c | 15 +++++++--------
> 1 file changed, 7 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/watchdog/qcom-wdt.c b/drivers/watchdog/qcom-wdt.c
> index 935c78a882a3..e98f5a3d83ea 100644
> --- a/drivers/watchdog/qcom-wdt.c
> +++ b/drivers/watchdog/qcom-wdt.c
> @@ -42,7 +42,6 @@ static const u32 reg_offset_data_kpss[] = {
>
> struct qcom_wdt {
> struct watchdog_device wdd;
> - struct clk *clk;
> unsigned long rate;
> void __iomem *base;
> const u32 *layout;
> @@ -189,6 +188,7 @@ static int qcom_wdt_probe(struct platform_device *pdev)
> const u32 *regs;
> u32 percpu_offset;
> int irq, ret;
> + struct clk *clk;
>
> regs = of_device_get_match_data(dev);
> if (!regs) {
> @@ -215,19 +215,18 @@ static int qcom_wdt_probe(struct platform_device *pdev)
> if (IS_ERR(wdt->base))
> return PTR_ERR(wdt->base);
>
> - wdt->clk = devm_clk_get(dev, NULL);
> - if (IS_ERR(wdt->clk)) {
> + clk = devm_clk_get(dev, NULL);
> + if (IS_ERR(clk)) {
> dev_err(dev, "failed to get input clock\n");
> - return PTR_ERR(wdt->clk);
> + return PTR_ERR(clk);
> }
>
> - ret = clk_prepare_enable(wdt->clk);
> + ret = clk_prepare_enable(clk);
> if (ret) {
> dev_err(dev, "failed to setup clock\n");
> return ret;
> }
> - ret = devm_add_action_or_reset(dev, qcom_clk_disable_unprepare,
> - wdt->clk);
> + ret = devm_add_action_or_reset(dev, qcom_clk_disable_unprepare, clk);
> if (ret)
> return ret;
>
> @@ -239,7 +238,7 @@ static int qcom_wdt_probe(struct platform_device *pdev)
> * that it would bite before a second elapses it's usefulness is
> * limited. Bail if this is the case.
> */
> - wdt->rate = clk_get_rate(wdt->clk);
> + wdt->rate = clk_get_rate(clk);
> if (wdt->rate == 0 ||
> wdt->rate > 0x10000000U) {
> dev_err(dev, "invalid clock rate\n");
On 10/09/19 11:06:55, Guenter Roeck wrote:
> On Fri, Sep 06, 2019 at 10:54:10PM +0200, Jorge Ramirez-Ortiz wrote:
> > Use the bark interrupt as the pre-timeout notifier whenever this
> > interrupt is available.
> >
> > By default, the pretimeout notification shall occur one second earlier
> > than the timeout.
> >
> > Signed-off-by: Jorge Ramirez-Ortiz <[email protected]>
>
> Nitpick below, otherwise:
>
> Reviewed-by: Guenter Roeck <[email protected]>
>
> > ---
> > drivers/watchdog/qcom-wdt.c | 70 ++++++++++++++++++++++++++++++++++---
> > 1 file changed, 65 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/watchdog/qcom-wdt.c b/drivers/watchdog/qcom-wdt.c
> > index 7be7f87be28f..935c78a882a3 100644
> > --- a/drivers/watchdog/qcom-wdt.c
> > +++ b/drivers/watchdog/qcom-wdt.c
> > @@ -1,8 +1,10 @@
> > // SPDX-License-Identifier: GPL-2.0-only
> > /* Copyright (c) 2014, The Linux Foundation. All rights reserved.
> > */
> > +#include <linux/bits.h>
> > #include <linux/clk.h>
> > #include <linux/delay.h>
> > +#include <linux/interrupt.h>
> > #include <linux/io.h>
> > #include <linux/kernel.h>
> > #include <linux/module.h>
> > @@ -19,6 +21,9 @@ enum wdt_reg {
> > WDT_BITE_TIME,
> > };
> >
> > +#define QCOM_WDT_ENABLE BIT(0)
> > +#define QCOM_WDT_ENABLE_IRQ BIT(1)
> > +
> > static const u32 reg_offset_data_apcs_tmr[] = {
> > [WDT_RST] = 0x38,
> > [WDT_EN] = 0x40,
> > @@ -54,15 +59,35 @@ struct qcom_wdt *to_qcom_wdt(struct watchdog_device *wdd)
> > return container_of(wdd, struct qcom_wdt, wdd);
> > }
> >
> > +static inline int qcom_get_enable(struct watchdog_device *wdd)
> > +{
> > + int enable = QCOM_WDT_ENABLE;
> > +
> > + if (wdd->pretimeout)
> > + enable |= QCOM_WDT_ENABLE_IRQ;
> > +
> > + return enable;
> > +}
> > +
> > +static irqreturn_t qcom_wdt_isr(int irq, void *arg)
> > +{
> > + struct watchdog_device *wdd = arg;
> > +
> > + watchdog_notify_pretimeout(wdd);
> > +
> > + return IRQ_HANDLED;
> > +}
> > +
> > static int qcom_wdt_start(struct watchdog_device *wdd)
> > {
> > struct qcom_wdt *wdt = to_qcom_wdt(wdd);
> > + unsigned int bark = wdd->timeout - wdd->pretimeout;
> >
> > writel(0, wdt_addr(wdt, WDT_EN));
> > writel(1, wdt_addr(wdt, WDT_RST));
> > - writel(wdd->timeout * wdt->rate, wdt_addr(wdt, WDT_BARK_TIME));
> > + writel(bark * wdt->rate, wdt_addr(wdt, WDT_BARK_TIME));
> > writel(wdd->timeout * wdt->rate, wdt_addr(wdt, WDT_BITE_TIME));
> > - writel(1, wdt_addr(wdt, WDT_EN));
> > + writel(qcom_get_enable(wdd), wdt_addr(wdt, WDT_EN));
> > return 0;
> > }
> >
> > @@ -89,6 +114,13 @@ static int qcom_wdt_set_timeout(struct watchdog_device *wdd,
> > return qcom_wdt_start(wdd);
> > }
> >
> > +static int qcom_wdt_set_pretimeout(struct watchdog_device *wdd,
> > + unsigned int timeout)
> > +{
> > + wdd->pretimeout = timeout;
> > + return qcom_wdt_start(wdd);
> > +}
> > +
> > static int qcom_wdt_restart(struct watchdog_device *wdd, unsigned long action,
> > void *data)
> > {
> > @@ -105,7 +137,7 @@ static int qcom_wdt_restart(struct watchdog_device *wdd, unsigned long action,
> > writel(1, wdt_addr(wdt, WDT_RST));
> > writel(timeout, wdt_addr(wdt, WDT_BARK_TIME));
> > writel(timeout, wdt_addr(wdt, WDT_BITE_TIME));
> > - writel(1, wdt_addr(wdt, WDT_EN));
> > + writel(QCOM_WDT_ENABLE, wdt_addr(wdt, WDT_EN));
> >
> > /*
> > * Actually make sure the above sequence hits hardware before sleeping.
> > @@ -121,6 +153,7 @@ static const struct watchdog_ops qcom_wdt_ops = {
> > .stop = qcom_wdt_stop,
> > .ping = qcom_wdt_ping,
> > .set_timeout = qcom_wdt_set_timeout,
> > + .set_pretimeout = qcom_wdt_set_pretimeout,
> > .restart = qcom_wdt_restart,
> > .owner = THIS_MODULE,
> > };
> > @@ -133,6 +166,15 @@ static const struct watchdog_info qcom_wdt_info = {
> > .identity = KBUILD_MODNAME,
> > };
> >
> > +static const struct watchdog_info qcom_wdt_pt_info = {
> > + .options = WDIOF_KEEPALIVEPING
> > + | WDIOF_MAGICCLOSE
> > + | WDIOF_SETTIMEOUT
> > + | WDIOF_PRETIMEOUT
> > + | WDIOF_CARDRESET,
> > + .identity = KBUILD_MODNAME,
> > +};
> > +
> > static void qcom_clk_disable_unprepare(void *data)
> > {
> > clk_disable_unprepare(data);
> > @@ -146,7 +188,7 @@ static int qcom_wdt_probe(struct platform_device *pdev)
> > struct device_node *np = dev->of_node;
> > const u32 *regs;
> > u32 percpu_offset;
> > - int ret;
> > + int irq, ret;
> >
> > regs = of_device_get_match_data(dev);
> > if (!regs) {
> > @@ -204,7 +246,25 @@ static int qcom_wdt_probe(struct platform_device *pdev)
> > return -EINVAL;
> > }
> >
> > - wdt->wdd.info = &qcom_wdt_info;
> > + /* check if there is pretimeout support */
> > + irq = platform_get_irq(pdev, 0);
> > + if (irq > 0) {
> > + ret = devm_request_irq(dev, irq, qcom_wdt_isr,
> > + IRQF_TRIGGER_RISING,
> > + "wdt_bark", &wdt->wdd);
> > + if (ret)
> > + return ret;
> > +
> > + wdt->wdd.info = &qcom_wdt_pt_info;
> > + wdt->wdd.pretimeout = 1;
> > + } else {
> > + if (irq == -EPROBE_DEFER)
> > + return -EPROBE_DEFER;
> > +
> > + wdt->wdd.info = &qcom_wdt_info;
> > + wdt->wdd.pretimeout = 0;
>
> It is not necessary to set pretimeout to 0; the data structure was
> allocated with devm_kzalloc(). The compiler won't know that and
> generate unnecessary code otherwise.
will you need me to send another version or could you pick it up as is?
>
> > + }
> > +
> > wdt->wdd.ops = &qcom_wdt_ops;
> > wdt->wdd.min_timeout = 1;
> > wdt->wdd.max_timeout = 0x10000000U / wdt->rate;
On Thu, Sep 12, 2019 at 09:24:54AM +0200, Jorge Ramirez-Ortiz, Linaro wrote:
> On 10/09/19 11:06:55, Guenter Roeck wrote:
> > On Fri, Sep 06, 2019 at 10:54:10PM +0200, Jorge Ramirez-Ortiz wrote:
> > > Use the bark interrupt as the pre-timeout notifier whenever this
> > > interrupt is available.
> > >
> > > By default, the pretimeout notification shall occur one second earlier
> > > than the timeout.
> > >
> > > Signed-off-by: Jorge Ramirez-Ortiz <[email protected]>
> >
> > Nitpick below, otherwise:
> >
> > Reviewed-by: Guenter Roeck <[email protected]>
> >
> > > + wdt->wdd.pretimeout = 0;
> >
> > It is not necessary to set pretimeout to 0; the data structure was
> > allocated with devm_kzalloc(). The compiler won't know that and
> > generate unnecessary code otherwise.
>
> will you need me to send another version or could you pick it up as is?
>
I applied the patch to my watchdog-next branch, with the line removed.
Let's assume that Wim will pick it up from there. If not, and
the line stays in, no real damage.
In short, no need to resubmit.
Guenter