Re: [PATCHv3 3/4] watchdog: rti-wdt: attach to running watchdog during probe
On 7/14/20 12:13 AM, Tero Kristo wrote: [ ... ] >>> Well, you can re-program it but... It does not take effect until next >>> window starts, so basically we need to take care of the currently running >>> window anyways after which re-programming it doesn't make much sense >>> anymore. And handling the switch from one setup to next would add extra >>> complexity. >>> >> >> Seems to me that hardware team really made an effort to make the >> watchdog as difficult to program as possible :-(. > > Yea, it is surprisingly difficult to program... when watchdogs in principle > are extremely simple pieces of HW. This claims to be some automotive grade > watchdog, which means it has the window in place. > We are getting a bit off topic, but that almost sounds like "automotive" translates to "it must be hard to program". I can not think of a valid reason for such a window, no matter the circumstances. Either case, dealing with the window in the kernel in that situation (ie if some specification states that the window must exist) would be the wrong solution and circumvent the reason for having it in the first place. Not that this matters here, of course. I am just wondering what the impact would be if the driver is indeed used in such an application. For example, some compliance test code might try to ping the watchdog outside the window to test its reaction. Such test code would fail if the ping is delayed by the kernel. Guenter
Re: [PATCHv3 3/4] watchdog: rti-wdt: attach to running watchdog during probe
On 14/07/2020 10:06, Guenter Roeck wrote:
On 7/13/20 11:50 PM, Tero Kristo wrote:
On 14/07/2020 08:15, Guenter Roeck wrote:
On 7/13/20 6:18 AM, Tero Kristo wrote:
If the RTI watchdog is running already during probe, the driver must
configure itself to match the HW. Window size and timeout is probed from
hardware, and the last keepalive ping is adjusted to match it also.
Signed-off-by: Tero Kristo
---
drivers/watchdog/rti_wdt.c | 111 +
1 file changed, 101 insertions(+), 10 deletions(-)
diff --git a/drivers/watchdog/rti_wdt.c b/drivers/watchdog/rti_wdt.c
index d456dd72d99a..45dfc546e371 100644
--- a/drivers/watchdog/rti_wdt.c
+++ b/drivers/watchdog/rti_wdt.c
@@ -35,7 +35,11 @@
#define RTIWWDRX_NMI 0xa
-#define RTIWWDSIZE_50P 0x50
+#define RTIWWDSIZE_50P 0x50
+#define RTIWWDSIZE_25P 0x500
+#define RTIWWDSIZE_12P5 0x5000
+#define RTIWWDSIZE_6P25 0x5
+#define RTIWWDSIZE_3P125 0x50
#define WDENABLE_KEY 0xa98559da
@@ -48,7 +52,7 @@
#define DWDST BIT(1)
-static int heartbeat;
+static int heartbeat = DEFAULT_HEARTBEAT;
/*
* struct to hold data for each WDT device
@@ -79,11 +83,9 @@ static int rti_wdt_start(struct watchdog_device *wdd)
* be petted during the open window; not too early or not too late.
* The HW configuration options only allow for the open window size
* to be 50% or less than that; we obviouly want to configure the open
- * window as large as possible so we select the 50% option. To avoid
- * any glitches, we accommodate 5% safety margin also, so we setup
- * the min_hw_hearbeat at 55% of the timeout period.
+ * window as large as possible so we select the 50% option.
*/
- wdd->min_hw_heartbeat_ms = 11 * wdd->timeout * 1000 / 20;
+ wdd->min_hw_heartbeat_ms = 500 * wdd->timeout;
/* Generate NMI when wdt expires */
writel_relaxed(RTIWWDRX_NMI, wdt->base + RTIWWDRXCTRL);
@@ -110,7 +112,48 @@ static int rti_wdt_ping(struct watchdog_device *wdd)
return 0;
}
-static unsigned int rti_wdt_get_timeleft(struct watchdog_device *wdd)
+static int rti_wdt_setup_hw_hb(struct watchdog_device *wdd, u32 wsize)
+{
+ /*
+ * RTI only supports a windowed mode, where the watchdog can only
+ * be petted during the open window; not too early or not too late.
+ * The HW configuration options only allow for the open window size
+ * to be 50% or less than that.
+ */
+ switch (wsize) {
+ case RTIWWDSIZE_50P:
+ /* 50% open window => 50% min heartbeat */
+ wdd->min_hw_heartbeat_ms = 500 * heartbeat;
+ break;
+
+ case RTIWWDSIZE_25P:
+ /* 25% open window => 75% min heartbeat */
+ wdd->min_hw_heartbeat_ms = 750 * heartbeat;
+ break;
+
+ case RTIWWDSIZE_12P5:
+ /* 12.5% open window => 87.5% min heartbeat */
+ wdd->min_hw_heartbeat_ms = 875 * heartbeat;
+ break;
+
+ case RTIWWDSIZE_6P25:
+ /* 6.5% open window => 93.5% min heartbeat */
+ wdd->min_hw_heartbeat_ms = 935 * heartbeat;
+ break;
+
+ case RTIWWDSIZE_3P125:
+ /* 3.125% open window => 96.9% min heartbeat */
+ wdd->min_hw_heartbeat_ms = 969 * heartbeat;
+ break;
+
+ default:
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
+static unsigned int rti_wdt_get_timeleft_ms(struct watchdog_device *wdd)
{
u64 timer_counter;
u32 val;
@@ -123,11 +166,18 @@ static unsigned int rti_wdt_get_timeleft(struct
watchdog_device *wdd)
timer_counter = readl_relaxed(wdt->base + RTIDWDCNTR);
+ timer_counter *= 1000;
+
do_div(timer_counter, wdt->freq);
return timer_counter;
}
+static unsigned int rti_wdt_get_timeleft(struct watchdog_device *wdd)
+{
+ return rti_wdt_get_timeleft_ms(wdd) / 1000;
+}
+
static const struct watchdog_info rti_wdt_info = {
.options = WDIOF_KEEPALIVEPING,
.identity = "K3 RTI Watchdog",
@@ -148,6 +198,7 @@ static int rti_wdt_probe(struct platform_device *pdev)
struct watchdog_device *wdd;
struct rti_wdt_device *wdt;
struct clk *clk;
+ u32 last_ping = 0;
wdt = devm_kzalloc(dev, sizeof(*wdt), GFP_KERNEL);
if (!wdt)
@@ -169,6 +220,14 @@ static int rti_wdt_probe(struct platform_device *pdev)
return -EINVAL;
}
+ /*
+ * If watchdog is running at 32k clock, it is not accurate.
+ * Adjust frequency down in this case so that we don't pet
+ * the watchdog too often.
+ */
+ if (wdt->freq < 32768)
+ wdt->freq = wdt->freq * 9 / 10;
+
So this is now only a problem is the window size was set restrictively
in the BOS/ROMMON. Meaning it is still a problem. How is this better than
before ? Why not just always set the window size to something reasonable ?
Re-programming of the watchdog only takes effect at the next ping, then and
only then will it adjust the
Re: [PATCHv3 3/4] watchdog: rti-wdt: attach to running watchdog during probe
On 7/13/20 11:50 PM, Tero Kristo wrote:
> On 14/07/2020 08:15, Guenter Roeck wrote:
>> On 7/13/20 6:18 AM, Tero Kristo wrote:
>>> If the RTI watchdog is running already during probe, the driver must
>>> configure itself to match the HW. Window size and timeout is probed from
>>> hardware, and the last keepalive ping is adjusted to match it also.
>>>
>>> Signed-off-by: Tero Kristo
>>> ---
>>> drivers/watchdog/rti_wdt.c | 111 +
>>> 1 file changed, 101 insertions(+), 10 deletions(-)
>>>
>>> diff --git a/drivers/watchdog/rti_wdt.c b/drivers/watchdog/rti_wdt.c
>>> index d456dd72d99a..45dfc546e371 100644
>>> --- a/drivers/watchdog/rti_wdt.c
>>> +++ b/drivers/watchdog/rti_wdt.c
>>> @@ -35,7 +35,11 @@
>>> #define RTIWWDRX_NMI 0xa
>>> -#define RTIWWDSIZE_50P 0x50
>>> +#define RTIWWDSIZE_50P 0x50
>>> +#define RTIWWDSIZE_25P 0x500
>>> +#define RTIWWDSIZE_12P5 0x5000
>>> +#define RTIWWDSIZE_6P25 0x5
>>> +#define RTIWWDSIZE_3P125 0x50
>>> #define WDENABLE_KEY 0xa98559da
>>> @@ -48,7 +52,7 @@
>>> #define DWDST BIT(1)
>>> -static int heartbeat;
>>> +static int heartbeat = DEFAULT_HEARTBEAT;
>>> /*
>>> * struct to hold data for each WDT device
>>> @@ -79,11 +83,9 @@ static int rti_wdt_start(struct watchdog_device *wdd)
>>> * be petted during the open window; not too early or not too late.
>>> * The HW configuration options only allow for the open window size
>>> * to be 50% or less than that; we obviouly want to configure the open
>>> - * window as large as possible so we select the 50% option. To avoid
>>> - * any glitches, we accommodate 5% safety margin also, so we setup
>>> - * the min_hw_hearbeat at 55% of the timeout period.
>>> + * window as large as possible so we select the 50% option.
>>> */
>>> - wdd->min_hw_heartbeat_ms = 11 * wdd->timeout * 1000 / 20;
>>> + wdd->min_hw_heartbeat_ms = 500 * wdd->timeout;
>>> /* Generate NMI when wdt expires */
>>> writel_relaxed(RTIWWDRX_NMI, wdt->base + RTIWWDRXCTRL);
>>> @@ -110,7 +112,48 @@ static int rti_wdt_ping(struct watchdog_device *wdd)
>>> return 0;
>>> }
>>> -static unsigned int rti_wdt_get_timeleft(struct watchdog_device *wdd)
>>> +static int rti_wdt_setup_hw_hb(struct watchdog_device *wdd, u32 wsize)
>>> +{
>>> + /*
>>> + * RTI only supports a windowed mode, where the watchdog can only
>>> + * be petted during the open window; not too early or not too late.
>>> + * The HW configuration options only allow for the open window size
>>> + * to be 50% or less than that.
>>> + */
>>> + switch (wsize) {
>>> + case RTIWWDSIZE_50P:
>>> + /* 50% open window => 50% min heartbeat */
>>> + wdd->min_hw_heartbeat_ms = 500 * heartbeat;
>>> + break;
>>> +
>>> + case RTIWWDSIZE_25P:
>>> + /* 25% open window => 75% min heartbeat */
>>> + wdd->min_hw_heartbeat_ms = 750 * heartbeat;
>>> + break;
>>> +
>>> + case RTIWWDSIZE_12P5:
>>> + /* 12.5% open window => 87.5% min heartbeat */
>>> + wdd->min_hw_heartbeat_ms = 875 * heartbeat;
>>> + break;
>>> +
>>> + case RTIWWDSIZE_6P25:
>>> + /* 6.5% open window => 93.5% min heartbeat */
>>> + wdd->min_hw_heartbeat_ms = 935 * heartbeat;
>>> + break;
>>> +
>>> + case RTIWWDSIZE_3P125:
>>> + /* 3.125% open window => 96.9% min heartbeat */
>>> + wdd->min_hw_heartbeat_ms = 969 * heartbeat;
>>> + break;
>>> +
>>> + default:
>>> + return -EINVAL;
>>> + }
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static unsigned int rti_wdt_get_timeleft_ms(struct watchdog_device *wdd)
>>> {
>>> u64 timer_counter;
>>> u32 val;
>>> @@ -123,11 +166,18 @@ static unsigned int rti_wdt_get_timeleft(struct
>>> watchdog_device *wdd)
>>> timer_counter = readl_relaxed(wdt->base + RTIDWDCNTR);
>>> + timer_counter *= 1000;
>>> +
>>> do_div(timer_counter, wdt->freq);
>>> return timer_counter;
>>> }
>>> +static unsigned int rti_wdt_get_timeleft(struct watchdog_device *wdd)
>>> +{
>>> + return rti_wdt_get_timeleft_ms(wdd) / 1000;
>>> +}
>>> +
>>> static const struct watchdog_info rti_wdt_info = {
>>> .options = WDIOF_KEEPALIVEPING,
>>> .identity = "K3 RTI Watchdog",
>>> @@ -148,6 +198,7 @@ static int rti_wdt_probe(struct platform_device *pdev)
>>> struct watchdog_device *wdd;
>>> struct rti_wdt_device *wdt;
>>> struct clk *clk;
>>> + u32 last_ping = 0;
>>> wdt = devm_kzalloc(dev, sizeof(*wdt), GFP_KERNEL);
>>> if (!wdt)
>>> @@ -169,6 +220,14 @@ static int rti_wdt_probe(struct platform_device *pdev)
>>> return -EINVAL;
>>> }
>>> + /*
>>> + * If watchdog is running at 32k clock, it is not accurate.
>>> + * Adjust frequency down in this case so that we don't pet
>>> + * the watchdog too often.
>>> + */
Re: [PATCHv3 3/4] watchdog: rti-wdt: attach to running watchdog during probe
On 14/07/2020 08:15, Guenter Roeck wrote:
On 7/13/20 6:18 AM, Tero Kristo wrote:
If the RTI watchdog is running already during probe, the driver must
configure itself to match the HW. Window size and timeout is probed from
hardware, and the last keepalive ping is adjusted to match it also.
Signed-off-by: Tero Kristo
---
drivers/watchdog/rti_wdt.c | 111 +
1 file changed, 101 insertions(+), 10 deletions(-)
diff --git a/drivers/watchdog/rti_wdt.c b/drivers/watchdog/rti_wdt.c
index d456dd72d99a..45dfc546e371 100644
--- a/drivers/watchdog/rti_wdt.c
+++ b/drivers/watchdog/rti_wdt.c
@@ -35,7 +35,11 @@
#define RTIWWDRX_NMI 0xa
-#define RTIWWDSIZE_50P 0x50
+#define RTIWWDSIZE_50P 0x50
+#define RTIWWDSIZE_25P 0x500
+#define RTIWWDSIZE_12P50x5000
+#define RTIWWDSIZE_6P250x5
+#define RTIWWDSIZE_3P125 0x50
#define WDENABLE_KEY 0xa98559da
@@ -48,7 +52,7 @@
#define DWDST BIT(1)
-static int heartbeat;
+static int heartbeat = DEFAULT_HEARTBEAT;
/*
* struct to hold data for each WDT device
@@ -79,11 +83,9 @@ static int rti_wdt_start(struct watchdog_device *wdd)
* be petted during the open window; not too early or not too late.
* The HW configuration options only allow for the open window size
* to be 50% or less than that; we obviouly want to configure the open
-* window as large as possible so we select the 50% option. To avoid
-* any glitches, we accommodate 5% safety margin also, so we setup
-* the min_hw_hearbeat at 55% of the timeout period.
+* window as large as possible so we select the 50% option.
*/
- wdd->min_hw_heartbeat_ms = 11 * wdd->timeout * 1000 / 20;
+ wdd->min_hw_heartbeat_ms = 500 * wdd->timeout;
/* Generate NMI when wdt expires */
writel_relaxed(RTIWWDRX_NMI, wdt->base + RTIWWDRXCTRL);
@@ -110,7 +112,48 @@ static int rti_wdt_ping(struct watchdog_device *wdd)
return 0;
}
-static unsigned int rti_wdt_get_timeleft(struct watchdog_device *wdd)
+static int rti_wdt_setup_hw_hb(struct watchdog_device *wdd, u32 wsize)
+{
+ /*
+* RTI only supports a windowed mode, where the watchdog can only
+* be petted during the open window; not too early or not too late.
+* The HW configuration options only allow for the open window size
+* to be 50% or less than that.
+*/
+ switch (wsize) {
+ case RTIWWDSIZE_50P:
+ /* 50% open window => 50% min heartbeat */
+ wdd->min_hw_heartbeat_ms = 500 * heartbeat;
+ break;
+
+ case RTIWWDSIZE_25P:
+ /* 25% open window => 75% min heartbeat */
+ wdd->min_hw_heartbeat_ms = 750 * heartbeat;
+ break;
+
+ case RTIWWDSIZE_12P5:
+ /* 12.5% open window => 87.5% min heartbeat */
+ wdd->min_hw_heartbeat_ms = 875 * heartbeat;
+ break;
+
+ case RTIWWDSIZE_6P25:
+ /* 6.5% open window => 93.5% min heartbeat */
+ wdd->min_hw_heartbeat_ms = 935 * heartbeat;
+ break;
+
+ case RTIWWDSIZE_3P125:
+ /* 3.125% open window => 96.9% min heartbeat */
+ wdd->min_hw_heartbeat_ms = 969 * heartbeat;
+ break;
+
+ default:
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
+static unsigned int rti_wdt_get_timeleft_ms(struct watchdog_device *wdd)
{
u64 timer_counter;
u32 val;
@@ -123,11 +166,18 @@ static unsigned int rti_wdt_get_timeleft(struct
watchdog_device *wdd)
timer_counter = readl_relaxed(wdt->base + RTIDWDCNTR);
+ timer_counter *= 1000;
+
do_div(timer_counter, wdt->freq);
return timer_counter;
}
+static unsigned int rti_wdt_get_timeleft(struct watchdog_device *wdd)
+{
+ return rti_wdt_get_timeleft_ms(wdd) / 1000;
+}
+
static const struct watchdog_info rti_wdt_info = {
.options = WDIOF_KEEPALIVEPING,
.identity = "K3 RTI Watchdog",
@@ -148,6 +198,7 @@ static int rti_wdt_probe(struct platform_device *pdev)
struct watchdog_device *wdd;
struct rti_wdt_device *wdt;
struct clk *clk;
+ u32 last_ping = 0;
wdt = devm_kzalloc(dev, sizeof(*wdt), GFP_KERNEL);
if (!wdt)
@@ -169,6 +220,14 @@ static int rti_wdt_probe(struct platform_device *pdev)
return -EINVAL;
}
+ /*
+* If watchdog is running at 32k clock, it is not accurate.
+* Adjust frequency down in this case so that we don't pet
+* the watchdog too often.
+*/
+ if (wdt->freq < 32768)
+ wdt->freq = wdt->freq * 9 / 10;
+
So this is now only a problem is the window size was set restrictively
in the BOS/ROMMON. Meaning it is still a problem. How is this better than
before ? Why not just always set the window size to som
Re: [PATCHv3 3/4] watchdog: rti-wdt: attach to running watchdog during probe
On 7/13/20 6:18 AM, Tero Kristo wrote:
> If the RTI watchdog is running already during probe, the driver must
> configure itself to match the HW. Window size and timeout is probed from
> hardware, and the last keepalive ping is adjusted to match it also.
>
> Signed-off-by: Tero Kristo
> ---
> drivers/watchdog/rti_wdt.c | 111 +
> 1 file changed, 101 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/watchdog/rti_wdt.c b/drivers/watchdog/rti_wdt.c
> index d456dd72d99a..45dfc546e371 100644
> --- a/drivers/watchdog/rti_wdt.c
> +++ b/drivers/watchdog/rti_wdt.c
> @@ -35,7 +35,11 @@
>
> #define RTIWWDRX_NMI 0xa
>
> -#define RTIWWDSIZE_50P 0x50
> +#define RTIWWDSIZE_50P 0x50
> +#define RTIWWDSIZE_25P 0x500
> +#define RTIWWDSIZE_12P5 0x5000
> +#define RTIWWDSIZE_6P25 0x5
> +#define RTIWWDSIZE_3P125 0x50
>
> #define WDENABLE_KEY 0xa98559da
>
> @@ -48,7 +52,7 @@
>
> #define DWDSTBIT(1)
>
> -static int heartbeat;
> +static int heartbeat = DEFAULT_HEARTBEAT;
>
> /*
> * struct to hold data for each WDT device
> @@ -79,11 +83,9 @@ static int rti_wdt_start(struct watchdog_device *wdd)
>* be petted during the open window; not too early or not too late.
>* The HW configuration options only allow for the open window size
>* to be 50% or less than that; we obviouly want to configure the open
> - * window as large as possible so we select the 50% option. To avoid
> - * any glitches, we accommodate 5% safety margin also, so we setup
> - * the min_hw_hearbeat at 55% of the timeout period.
> + * window as large as possible so we select the 50% option.
>*/
> - wdd->min_hw_heartbeat_ms = 11 * wdd->timeout * 1000 / 20;
> + wdd->min_hw_heartbeat_ms = 500 * wdd->timeout;
>
> /* Generate NMI when wdt expires */
> writel_relaxed(RTIWWDRX_NMI, wdt->base + RTIWWDRXCTRL);
> @@ -110,7 +112,48 @@ static int rti_wdt_ping(struct watchdog_device *wdd)
> return 0;
> }
>
> -static unsigned int rti_wdt_get_timeleft(struct watchdog_device *wdd)
> +static int rti_wdt_setup_hw_hb(struct watchdog_device *wdd, u32 wsize)
> +{
> + /*
> + * RTI only supports a windowed mode, where the watchdog can only
> + * be petted during the open window; not too early or not too late.
> + * The HW configuration options only allow for the open window size
> + * to be 50% or less than that.
> + */
> + switch (wsize) {
> + case RTIWWDSIZE_50P:
> + /* 50% open window => 50% min heartbeat */
> + wdd->min_hw_heartbeat_ms = 500 * heartbeat;
> + break;
> +
> + case RTIWWDSIZE_25P:
> + /* 25% open window => 75% min heartbeat */
> + wdd->min_hw_heartbeat_ms = 750 * heartbeat;
> + break;
> +
> + case RTIWWDSIZE_12P5:
> + /* 12.5% open window => 87.5% min heartbeat */
> + wdd->min_hw_heartbeat_ms = 875 * heartbeat;
> + break;
> +
> + case RTIWWDSIZE_6P25:
> + /* 6.5% open window => 93.5% min heartbeat */
> + wdd->min_hw_heartbeat_ms = 935 * heartbeat;
> + break;
> +
> + case RTIWWDSIZE_3P125:
> + /* 3.125% open window => 96.9% min heartbeat */
> + wdd->min_hw_heartbeat_ms = 969 * heartbeat;
> + break;
> +
> + default:
> + return -EINVAL;
> + }
> +
> + return 0;
> +}
> +
> +static unsigned int rti_wdt_get_timeleft_ms(struct watchdog_device *wdd)
> {
> u64 timer_counter;
> u32 val;
> @@ -123,11 +166,18 @@ static unsigned int rti_wdt_get_timeleft(struct
> watchdog_device *wdd)
>
> timer_counter = readl_relaxed(wdt->base + RTIDWDCNTR);
>
> + timer_counter *= 1000;
> +
> do_div(timer_counter, wdt->freq);
>
> return timer_counter;
> }
>
> +static unsigned int rti_wdt_get_timeleft(struct watchdog_device *wdd)
> +{
> + return rti_wdt_get_timeleft_ms(wdd) / 1000;
> +}
> +
> static const struct watchdog_info rti_wdt_info = {
> .options = WDIOF_KEEPALIVEPING,
> .identity = "K3 RTI Watchdog",
> @@ -148,6 +198,7 @@ static int rti_wdt_probe(struct platform_device *pdev)
> struct watchdog_device *wdd;
> struct rti_wdt_device *wdt;
> struct clk *clk;
> + u32 last_ping = 0;
>
> wdt = devm_kzalloc(dev, sizeof(*wdt), GFP_KERNEL);
> if (!wdt)
> @@ -169,6 +220,14 @@ static int rti_wdt_probe(struct platform_device *pdev)
> return -EINVAL;
> }
>
> + /*
> + * If watchdog is running at 32k clock, it is not accurate.
> + * Adjust frequency down in this case so that we don't pet
> + * the watchdog too often.
> + */
> + if (wdt->freq < 32768)
> + wdt->freq = wdt->freq * 9 / 10;
> +
So this is now only a problem is the window size was set restrictively
in t
Re: [PATCHv3 3/4] watchdog: rti-wdt: attach to running watchdog during probe
Hi Tero, I love your patch! Yet something to improve: [auto build test ERROR on hwmon/hwmon-next] [also build test ERROR on linux/master linus/master v5.8-rc5 next-20200713] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use as documented in https://git-scm.com/docs/git-format-patch] url: https://github.com/0day-ci/linux/commits/Tero-Kristo/watchdog-rti-support-attach-to-running-timer/20200713-213531 base: https://git.kernel.org/pub/scm/linux/kernel/git/groeck/linux-staging.git hwmon-next config: mips-allmodconfig (attached as .config) compiler: mips-linux-gcc (GCC) 9.3.0 reproduce (this is a W=1 build): wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # save the attached .config to linux build tree COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=mips If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot All errors (new ones prefixed by >>, old ones prefixed by <<): >> ERROR: modpost: "__divdi3" [drivers/watchdog/rti_wdt.ko] undefined! >> ERROR: modpost: "__udivdi3" [drivers/watchdog/rti_wdt.ko] undefined! --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org .config.gz Description: application/gzip
Re: [PATCHv3 3/4] watchdog: rti-wdt: attach to running watchdog during probe
Hi Tero, I love your patch! Yet something to improve: [auto build test ERROR on hwmon/hwmon-next] [also build test ERROR on linux/master linus/master v5.8-rc5 next-20200713] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use as documented in https://git-scm.com/docs/git-format-patch] url: https://github.com/0day-ci/linux/commits/Tero-Kristo/watchdog-rti-support-attach-to-running-timer/20200713-213531 base: https://git.kernel.org/pub/scm/linux/kernel/git/groeck/linux-staging.git hwmon-next config: mips-allyesconfig (attached as .config) compiler: mips-linux-gcc (GCC) 9.3.0 reproduce (this is a W=1 build): wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # save the attached .config to linux build tree COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=mips If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot All errors (new ones prefixed by >>): arch/mips/kernel/head.o: in function `dtb_found': (.ref.text+0xc8): relocation truncated to fit: R_MIPS_26 against `start_kernel' init/main.o: in function `set_reset_devices': main.c:(.init.text+0x20): relocation truncated to fit: R_MIPS_26 against `_mcount' main.c:(.init.text+0x30): relocation truncated to fit: R_MIPS_26 against `__sanitizer_cov_trace_pc' init/main.o: in function `debug_kernel': main.c:(.init.text+0x9c): relocation truncated to fit: R_MIPS_26 against `_mcount' main.c:(.init.text+0xac): relocation truncated to fit: R_MIPS_26 against `__sanitizer_cov_trace_pc' init/main.o: in function `quiet_kernel': main.c:(.init.text+0x118): relocation truncated to fit: R_MIPS_26 against `_mcount' main.c:(.init.text+0x128): relocation truncated to fit: R_MIPS_26 against `__sanitizer_cov_trace_pc' init/main.o: in function `init_setup': main.c:(.init.text+0x1a4): relocation truncated to fit: R_MIPS_26 against `_mcount' main.c:(.init.text+0x1c8): relocation truncated to fit: R_MIPS_26 against `__sanitizer_cov_trace_pc' main.c:(.init.text+0x1e8): relocation truncated to fit: R_MIPS_26 against `__sanitizer_cov_trace_pc' main.c:(.init.text+0x1fc): additional relocation overflows omitted from the output mips-linux-ld: drivers/watchdog/rti_wdt.o: in function `rti_wdt_probe': >> rti_wdt.c:(.text.rti_wdt_probe+0x33c): undefined reference to `__udivdi3' >> mips-linux-ld: rti_wdt.c:(.text.rti_wdt_probe+0x35c): undefined reference to >> `__udivdi3' --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org .config.gz Description: application/gzip
[PATCHv3 3/4] watchdog: rti-wdt: attach to running watchdog during probe
If the RTI watchdog is running already during probe, the driver must
configure itself to match the HW. Window size and timeout is probed from
hardware, and the last keepalive ping is adjusted to match it also.
Signed-off-by: Tero Kristo
---
drivers/watchdog/rti_wdt.c | 111 +
1 file changed, 101 insertions(+), 10 deletions(-)
diff --git a/drivers/watchdog/rti_wdt.c b/drivers/watchdog/rti_wdt.c
index d456dd72d99a..45dfc546e371 100644
--- a/drivers/watchdog/rti_wdt.c
+++ b/drivers/watchdog/rti_wdt.c
@@ -35,7 +35,11 @@
#define RTIWWDRX_NMI 0xa
-#define RTIWWDSIZE_50P 0x50
+#define RTIWWDSIZE_50P 0x50
+#define RTIWWDSIZE_25P 0x500
+#define RTIWWDSIZE_12P50x5000
+#define RTIWWDSIZE_6P250x5
+#define RTIWWDSIZE_3P125 0x50
#define WDENABLE_KEY 0xa98559da
@@ -48,7 +52,7 @@
#define DWDST BIT(1)
-static int heartbeat;
+static int heartbeat = DEFAULT_HEARTBEAT;
/*
* struct to hold data for each WDT device
@@ -79,11 +83,9 @@ static int rti_wdt_start(struct watchdog_device *wdd)
* be petted during the open window; not too early or not too late.
* The HW configuration options only allow for the open window size
* to be 50% or less than that; we obviouly want to configure the open
-* window as large as possible so we select the 50% option. To avoid
-* any glitches, we accommodate 5% safety margin also, so we setup
-* the min_hw_hearbeat at 55% of the timeout period.
+* window as large as possible so we select the 50% option.
*/
- wdd->min_hw_heartbeat_ms = 11 * wdd->timeout * 1000 / 20;
+ wdd->min_hw_heartbeat_ms = 500 * wdd->timeout;
/* Generate NMI when wdt expires */
writel_relaxed(RTIWWDRX_NMI, wdt->base + RTIWWDRXCTRL);
@@ -110,7 +112,48 @@ static int rti_wdt_ping(struct watchdog_device *wdd)
return 0;
}
-static unsigned int rti_wdt_get_timeleft(struct watchdog_device *wdd)
+static int rti_wdt_setup_hw_hb(struct watchdog_device *wdd, u32 wsize)
+{
+ /*
+* RTI only supports a windowed mode, where the watchdog can only
+* be petted during the open window; not too early or not too late.
+* The HW configuration options only allow for the open window size
+* to be 50% or less than that.
+*/
+ switch (wsize) {
+ case RTIWWDSIZE_50P:
+ /* 50% open window => 50% min heartbeat */
+ wdd->min_hw_heartbeat_ms = 500 * heartbeat;
+ break;
+
+ case RTIWWDSIZE_25P:
+ /* 25% open window => 75% min heartbeat */
+ wdd->min_hw_heartbeat_ms = 750 * heartbeat;
+ break;
+
+ case RTIWWDSIZE_12P5:
+ /* 12.5% open window => 87.5% min heartbeat */
+ wdd->min_hw_heartbeat_ms = 875 * heartbeat;
+ break;
+
+ case RTIWWDSIZE_6P25:
+ /* 6.5% open window => 93.5% min heartbeat */
+ wdd->min_hw_heartbeat_ms = 935 * heartbeat;
+ break;
+
+ case RTIWWDSIZE_3P125:
+ /* 3.125% open window => 96.9% min heartbeat */
+ wdd->min_hw_heartbeat_ms = 969 * heartbeat;
+ break;
+
+ default:
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
+static unsigned int rti_wdt_get_timeleft_ms(struct watchdog_device *wdd)
{
u64 timer_counter;
u32 val;
@@ -123,11 +166,18 @@ static unsigned int rti_wdt_get_timeleft(struct
watchdog_device *wdd)
timer_counter = readl_relaxed(wdt->base + RTIDWDCNTR);
+ timer_counter *= 1000;
+
do_div(timer_counter, wdt->freq);
return timer_counter;
}
+static unsigned int rti_wdt_get_timeleft(struct watchdog_device *wdd)
+{
+ return rti_wdt_get_timeleft_ms(wdd) / 1000;
+}
+
static const struct watchdog_info rti_wdt_info = {
.options = WDIOF_KEEPALIVEPING,
.identity = "K3 RTI Watchdog",
@@ -148,6 +198,7 @@ static int rti_wdt_probe(struct platform_device *pdev)
struct watchdog_device *wdd;
struct rti_wdt_device *wdt;
struct clk *clk;
+ u32 last_ping = 0;
wdt = devm_kzalloc(dev, sizeof(*wdt), GFP_KERNEL);
if (!wdt)
@@ -169,6 +220,14 @@ static int rti_wdt_probe(struct platform_device *pdev)
return -EINVAL;
}
+ /*
+* If watchdog is running at 32k clock, it is not accurate.
+* Adjust frequency down in this case so that we don't pet
+* the watchdog too often.
+*/
+ if (wdt->freq < 32768)
+ wdt->freq = wdt->freq * 9 / 10;
+
pm_runtime_enable(dev);
ret = pm_runtime_get_sync(dev);
if (ret) {
@@ -185,11 +244,8 @@ static int rti_wdt_probe(struct platform_device *pdev)
wdd->min_timeout = 1;
wdd->max_hw_heartbeat_ms = (WDT_PRELOAD_MAX << WDT_PRELOAD_SHIFT)
