Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Subject: Re: [PATCH] watchdog: qcom: add support for the bark interrupt
To:     Bjorn Andersson <bjorn.andersson@linaro.org>
Cc:     agross@kernel.org, wim@linux-watchdog.org,
        linux-arm-msm@vger.kernel.org, linux-watchdog@vger.kernel.org,
        linux-kernel@vger.kernel.org
References: <20190905162135.2618-1-jorge.ramirez-ortiz@linaro.org>
 <20190905163919.GA1676@tuxbook-pro>
From:   Jorge Ramirez <jorge.ramirez-ortiz@linaro.org>
Message-ID: <eda57fef-5fd7-85d9-68e6-deec621a7360@linaro.org>
Date:   Thu, 5 Sep 2019 18:48:34 +0200
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101
 Thunderbird/60.8.0
MIME-Version: 1.0
In-Reply-To: <20190905163919.GA1676@tuxbook-pro>
Content-Type: text/plain; charset=utf-8
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On 9/5/19 18:39, Bjorn Andersson wrote:
> On Thu 05 Sep 09:21 PDT 2019, Jorge Ramirez-Ortiz wrote:
> 
>> Use the bark interrupt to notify the bark event. Since the bark and bite
>> timeouts are identical, increase the bite timeout by one second so
>> that the bark event can be logged to the console.
>>
> 
> Afaict you should tie the bark to the "pretimeout" in the watchdog
> framework , which would allow the user to specify a pretimeout and
> configure what should happen at the bark (just a pr_alert() or panic()).

yes, you are right. will send v2 based on this.
thanks!

> 
> Regards,
> Bjorn
> 
>> Signed-off-by: Jorge Ramirez-Ortiz <jorge.ramirez-ortiz@linaro.org>
>> ---
>>  drivers/watchdog/qcom-wdt.c | 42 ++++++++++++++++++++++++++++++++++---
>>  1 file changed, 39 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/watchdog/qcom-wdt.c b/drivers/watchdog/qcom-wdt.c
>> index 7be7f87be28f..5eaf92084b93 100644
>> --- a/drivers/watchdog/qcom-wdt.c
>> +++ b/drivers/watchdog/qcom-wdt.c
>> @@ -10,6 +10,7 @@
>>  #include <linux/platform_device.h>
>>  #include <linux/watchdog.h>
>>  #include <linux/of_device.h>
>> +#include <linux/interrupt.h>
>>  
>>  enum wdt_reg {
>>  	WDT_RST,
>> @@ -41,6 +42,8 @@ struct qcom_wdt {
>>  	unsigned long		rate;
>>  	void __iomem		*base;
>>  	const u32		*layout;
>> +	unsigned int		irq;
>> +	const struct device	*dev;
>>  };
>>  
>>  static void __iomem *wdt_addr(struct qcom_wdt *wdt, enum wdt_reg reg)
>> @@ -54,15 +57,37 @@ struct qcom_wdt *to_qcom_wdt(struct watchdog_device *wdd)
>>  	return container_of(wdd, struct qcom_wdt, wdd);
>>  }
>>  
>> +static inline int qcom_wdt_enable(struct qcom_wdt *wdt)
>> +{
>> +	if (wdt->irq < 0)
>> +		return 1;
>> +
>> +	/* enable timeout with interrupt */
>> +	return 3;
>> +}
>> +
>> +static irqreturn_t qcom_wdt_irq(int irq, void *cookie)
>> +{
>> +	struct qcom_wdt *wdt =  (struct qcom_wdt *) cookie;
>> +
>> +	dev_warn(wdt->dev, "barking, one second countdown to reset\n");
>> +
>> +	return IRQ_HANDLED;
>> +}
>> +
>>  static int qcom_wdt_start(struct watchdog_device *wdd)
>>  {
>>  	struct qcom_wdt *wdt = to_qcom_wdt(wdd);
>> +	unsigned int bark, bite;
>> +
>> +	bark = wdd->timeout;
>> +	bite = wdt->irq < 0 ? bark : bark + 1;
>>  
>>  	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(wdd->timeout * wdt->rate, wdt_addr(wdt, WDT_BITE_TIME));
>> -	writel(1, wdt_addr(wdt, WDT_EN));
>> +	writel(bark * wdt->rate, wdt_addr(wdt, WDT_BARK_TIME));
>> +	writel(bite * wdt->rate, wdt_addr(wdt, WDT_BITE_TIME));
>> +	writel(qcom_wdt_enable(wdt), wdt_addr(wdt, WDT_EN));
>>  	return 0;
>>  }
>>  
>> @@ -210,10 +235,21 @@ static int qcom_wdt_probe(struct platform_device *pdev)
>>  	wdt->wdd.max_timeout = 0x10000000U / wdt->rate;
>>  	wdt->wdd.parent = dev;
>>  	wdt->layout = regs;
>> +	wdt->dev = &pdev->dev;
>>  
>>  	if (readl(wdt_addr(wdt, WDT_STS)) & 1)
>>  		wdt->wdd.bootstatus = WDIOF_CARDRESET;
>>  
>> +	wdt->irq = platform_get_irq(pdev, 0);
>> +	if (wdt->irq >= 0) {
>> +		ret = devm_request_irq(&pdev->dev, wdt->irq, qcom_wdt_irq,
>> +				       IRQF_TRIGGER_RISING, "wdog_bark", wdt);
>> +		if (ret) {
>> +			dev_err(&pdev->dev, "failed to request irq\n");
>> +			return ret;
>> +		}
>> +	}
>> +
>>  	/*
>>  	 * If 'timeout-sec' unspecified in devicetree, assume a 30 second
>>  	 * default, unless the max timeout is less than 30 seconds, then use
>> -- 
>> 2.23.0
>>
>