On 1/26/21 7:02 AM, Sai Prakash Ranjan wrote:
> As per register documentation, QCOM_WDT_ENABLE_IRQ which is BIT(1)
> of watchdog control register is wakeup interrupt enable bit and
> not related to bark interrupt at all, BIT(0) is used for that.
> So remove incorrect usage of this bit when supporting bark irq for
> pre-timeout notification. Currently with this bit set and bark
> interrupt specified, pre-timeout notification and/or watchdog
> reset/bite does not occur.
>
> Fixes: 36375491a439 ("watchdog: qcom: support pre-timeout when the bark irq is available")
> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Sai Prakash Ranjan <saiprakash.ranjan@xxxxxxxxxxxxxx>
Assuming that pretimeout _does_ work with this patch applied,
Reviewed-by: Guenter Roeck <linux@xxxxxxxxxxxx>
> ---
>
> Reading the conversations from when qcom pre-timeout support was
> added [1], Bjorn already had mentioned it was not right to touch this
> bit, not sure which SoC the pre-timeout was tested on at that time,
> but I have tested this on SDM845, SM8150, SC7180 and watchdog bark
> and bite does not occur with enabling this bit with the bark irq
> specified in DT.
>
> [1] https://lore.kernel.org/linux-watchdog/20190906174009.GC11938@tuxbook-pro/
>
> ---
> drivers/watchdog/qcom-wdt.c | 13 +------------
> 1 file changed, 1 insertion(+), 12 deletions(-)
>
> diff --git a/drivers/watchdog/qcom-wdt.c b/drivers/watchdog/qcom-wdt.c
> index 7cf0f2ec649b..e38a87ffe5f5 100644
> --- a/drivers/watchdog/qcom-wdt.c
> +++ b/drivers/watchdog/qcom-wdt.c
> @@ -22,7 +22,6 @@ enum wdt_reg {
> };
>
> #define QCOM_WDT_ENABLE BIT(0)
> -#define QCOM_WDT_ENABLE_IRQ BIT(1)
>
> static const u32 reg_offset_data_apcs_tmr[] = {
> [WDT_RST] = 0x38,
> @@ -63,16 +62,6 @@ 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;
> @@ -91,7 +80,7 @@ static int qcom_wdt_start(struct watchdog_device *wdd)
> writel(1, wdt_addr(wdt, WDT_RST));
> writel(bark * wdt->rate, wdt_addr(wdt, WDT_BARK_TIME));
> writel(wdd->timeout * wdt->rate, wdt_addr(wdt, WDT_BITE_TIME));
> - writel(qcom_get_enable(wdd), wdt_addr(wdt, WDT_EN));
> + writel(QCOM_WDT_ENABLE, wdt_addr(wdt, WDT_EN));
> return 0;
> }
>
>
