Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1321937 > unrolled thread

Re: [RFT PATCH v7 9/9] watchdog: dw_wdt: Convert to use watchdog infrastructure

Started byDoug Anderson <dianders@chromium.org>
First post2016-01-29 19:00 +0100
Last post2016-01-29 19:30 +0100
Articles 2 — 2 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.


Contents

  Re: [RFT PATCH v7 9/9] watchdog: dw_wdt: Convert to use watchdog infrastructure Doug Anderson <dianders@chromium.org> - 2016-01-29 19:00 +0100
    Re: [RFT PATCH v7 9/9] watchdog: dw_wdt: Convert to use watchdog  infrastructure Guenter Roeck <linux@roeck-us.net> - 2016-01-29 19:30 +0100

#1321937 — Re: [RFT PATCH v7 9/9] watchdog: dw_wdt: Convert to use watchdog infrastructure

FromDoug Anderson <dianders@chromium.org>
Date2016-01-29 19:00 +0100
SubjectRe: [RFT PATCH v7 9/9] watchdog: dw_wdt: Convert to use watchdog infrastructure
Message-ID<qWluY-6FF-29@gated-at.bofh.it>
Guenter,

On Mon, Jan 25, 2016 at 6:53 PM, Guenter Roeck
<patchwork@patchwork.roeck-us.net> wrote:
> From: Guenter Roeck <linux@roeck-us.net>
>
> Convert driver to use watchdog infrastructure. This includes
> infrastructure support to handle watchdog keepalive if the watchdog
> is running while the watchdog device is closed.
>
> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
>
> ---
> v7: Set max_hw_timeout_ms
>     Rebased to v4.5-rc1
> v6: Added patch
> ---
>  drivers/watchdog/Kconfig  |   1 +
>  drivers/watchdog/dw_wdt.c | 323 +++++++++++++++++-----------------------------
>  2 files changed, 117 insertions(+), 207 deletions(-)

I used the following test environment:
* Rockchip rk3288-based Chromebook
* v4.4-based kernel (specifically commit b1ba57898bfc
("veyron_defconfig: update for 4.5-rc") from Heiko Stuebner "somewhat
stable" development tree
* Pulled in some USB-related fixes (just for my own testing)
* Pulled in Wim's most recent pull request for watchdog subsystem
(ac36856fe432 watchdog: asm9260: remove __init and __exit annotations)
* Applied all 9 of your patches
* Test with daisydog
<https://chromium.googlesource.com/chromiumos/third_party/daisydog/>


I did these tests:

###
# Confirm normal functionality
###

stop daisydog
daisydog -c
# HW watchdog interval is 43 seconds
# /dev/watchdog reported boot status: normal-boot

cat /dev/watchdog
# cat: [ 5401.044582] watchdog watchdog0: watchdog did not stop!
# /dev/watchdog: Invalid argument

for i in $(seq 60); do
   echo $i
   sleep 1
 done
# 1 - 43 printed, then reboot

###
# Try setting interval, confirm all good (no reboots)
# ...then confirm that reboot happens in 10 sec
###

stop daisydog
daisydog -c
# HW watchdog interval is 43 seconds
# /dev/watchdog reported boot status: normal-boot

daisydog -i 10
# HW watchdog interval is 10 seconds
# /dev/watchdog reported boot status: normal-boot

<Ctrl-C>

daisydog -c
# HW watchdog interval is 10 seconds
# /dev/watchdog reported boot status: normal-boot

sleep 60
# No reboots

cat /dev/watchdog
# cat: [ 5401.044582] watchdog watchdog0: watchdog did not stop!
# /dev/watchdog: Invalid argument

for i in $(seq 60); do
   echo $i
   sleep 1
 done
# 1 - 11 printed, then reboot

###
# Try setting interval super long
# (outside of hardware range)
###

stop daisydog
daisydog -i 100 -c
# HW watchdog interval is 86 seconds
# /dev/watchdog reported boot status: normal-boot

cat /dev/watchdog
# cat: [ 5401.044582] watchdog watchdog0: watchdog did not stop!
# /dev/watchdog: Invalid argument

for i in $(seq 200); do
   echo $i
   sleep 1
 done
# 1 - 87 printed, then reboot; seems right to me.

###
# Try with a interval that's in hardware range, but above default
###

stop daisydog
daisydog -i 70 -c
# HW watchdog interval is 86 seconds
# /dev/watchdog reported boot status: normal-boot

cat /dev/watchdog
# cat: [ 5401.044582] watchdog watchdog0: watchdog did not stop!
# /dev/watchdog: Invalid argument

for i in $(seq 200); do
   echo $i
   sleep 1
 done
# 1 - 87 printed, then reboot; seems right to me

###
# Make sure my previous fix not regressed:
# 04b1a62e6bb9 ("watchdog: dw_wdt: keepalive the watchdog at write time")
###

stop daisydog
while true; do daisydog -c > /dev/null; done

# Let the above run for a 2 minutes and see no reboot.

###
# See what happens when you change things midway
###

stop daisydog
daisydog -c
# HW watchdog interval is 43 seconds
# /dev/watchdog reported boot status: normal-boot

cat /dev/watchdog
# cat: [ 5401.044582] watchdog watchdog0: watchdog did not stop!
# /dev/watchdog: Invalid argument

for i in $(seq 30); do
   echo $i
   sleep 1
 done
# 1 - 30 printed

daisydog -c -i 10
# HW watchdog interval is 10 seconds
# /dev/watchdog reported boot status: normal-boot

sleep 60
# No reboot; right since the "-c -i 10" patted + closed properly

===

I also tried the new functionality enabled by your new patch and added
"timeout-sec = <10>" to my device tree.  That seemed to work.  Upon
bootup the watchdog defaulted to 10 seconds.  I also tried setting it
to 86 in the device tree with similar success.

===

The above seem to cover all the cases I can think of.

Tested-by: Douglas Anderson <dianders@chromium.org>

[toc] | [next] | [standalone]


#1321978 — Re: [RFT PATCH v7 9/9] watchdog: dw_wdt: Convert to use watchdog infrastructure

FromGuenter Roeck <linux@roeck-us.net>
Date2016-01-29 19:30 +0100
SubjectRe: [RFT PATCH v7 9/9] watchdog: dw_wdt: Convert to use watchdog infrastructure
Message-ID<qWlXZ-79P-53@gated-at.bofh.it>
In reply to#1321937
Hi Doug,

On 01/29/2016 09:52 AM, Doug Anderson wrote:
> Guenter,
>
> On Mon, Jan 25, 2016 at 6:53 PM, Guenter Roeck
> <patchwork@patchwork.roeck-us.net> wrote:
>> From: Guenter Roeck <linux@roeck-us.net>
>>
>> Convert driver to use watchdog infrastructure. This includes
>> infrastructure support to handle watchdog keepalive if the watchdog
>> is running while the watchdog device is closed.
>>
>> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
>>
>> ---
>> v7: Set max_hw_timeout_ms
>>      Rebased to v4.5-rc1
>> v6: Added patch
>> ---
>>   drivers/watchdog/Kconfig  |   1 +
>>   drivers/watchdog/dw_wdt.c | 323 +++++++++++++++++-----------------------------
>>   2 files changed, 117 insertions(+), 207 deletions(-)
>
> I used the following test environment:
> * Rockchip rk3288-based Chromebook
> * v4.4-based kernel (specifically commit b1ba57898bfc
> ("veyron_defconfig: update for 4.5-rc") from Heiko Stuebner "somewhat
> stable" development tree
> * Pulled in some USB-related fixes (just for my own testing)
> * Pulled in Wim's most recent pull request for watchdog subsystem
> (ac36856fe432 watchdog: asm9260: remove __init and __exit annotations)
> * Applied all 9 of your patches
> * Test with daisydog
> <https://chromium.googlesource.com/chromiumos/third_party/daisydog/>
>
>
> I did these tests:
>
> ###
> # Confirm normal functionality
> ###
>
> stop daisydog
> daisydog -c
> # HW watchdog interval is 43 seconds
> # /dev/watchdog reported boot status: normal-boot
>
> cat /dev/watchdog
> # cat: [ 5401.044582] watchdog watchdog0: watchdog did not stop!
> # /dev/watchdog: Invalid argument
>
> for i in $(seq 60); do
>     echo $i
>     sleep 1
>   done
> # 1 - 43 printed, then reboot
>
> ###
> # Try setting interval, confirm all good (no reboots)
> # ...then confirm that reboot happens in 10 sec
> ###
>
> stop daisydog
> daisydog -c
> # HW watchdog interval is 43 seconds
> # /dev/watchdog reported boot status: normal-boot
>
> daisydog -i 10
> # HW watchdog interval is 10 seconds
> # /dev/watchdog reported boot status: normal-boot
>
> <Ctrl-C>
>
> daisydog -c
> # HW watchdog interval is 10 seconds
> # /dev/watchdog reported boot status: normal-boot
>
> sleep 60
> # No reboots
>
> cat /dev/watchdog
> # cat: [ 5401.044582] watchdog watchdog0: watchdog did not stop!
> # /dev/watchdog: Invalid argument
>
> for i in $(seq 60); do
>     echo $i
>     sleep 1
>   done
> # 1 - 11 printed, then reboot
>
> ###
> # Try setting interval super long
> # (outside of hardware range)
> ###
>
> stop daisydog
> daisydog -i 100 -c
> # HW watchdog interval is 86 seconds
> # /dev/watchdog reported boot status: normal-boot
>
> cat /dev/watchdog
> # cat: [ 5401.044582] watchdog watchdog0: watchdog did not stop!
> # /dev/watchdog: Invalid argument
>
> for i in $(seq 200); do
>     echo $i
>     sleep 1
>   done
> # 1 - 87 printed, then reboot; seems right to me.
>
> ###
> # Try with a interval that's in hardware range, but above default
> ###
>
> stop daisydog
> daisydog -i 70 -c
> # HW watchdog interval is 86 seconds
> # /dev/watchdog reported boot status: normal-boot
>
> cat /dev/watchdog
> # cat: [ 5401.044582] watchdog watchdog0: watchdog did not stop!
> # /dev/watchdog: Invalid argument
>
> for i in $(seq 200); do
>     echo $i
>     sleep 1
>   done
> # 1 - 87 printed, then reboot; seems right to me
>
> ###
> # Make sure my previous fix not regressed:
> # 04b1a62e6bb9 ("watchdog: dw_wdt: keepalive the watchdog at write time")
> ###
>
> stop daisydog
> while true; do daisydog -c > /dev/null; done
>
> # Let the above run for a 2 minutes and see no reboot.
>
> ###
> # See what happens when you change things midway
> ###
>
> stop daisydog
> daisydog -c
> # HW watchdog interval is 43 seconds
> # /dev/watchdog reported boot status: normal-boot
>
> cat /dev/watchdog
> # cat: [ 5401.044582] watchdog watchdog0: watchdog did not stop!
> # /dev/watchdog: Invalid argument
>
> for i in $(seq 30); do
>     echo $i
>     sleep 1
>   done
> # 1 - 30 printed
>
> daisydog -c -i 10
> # HW watchdog interval is 10 seconds
> # /dev/watchdog reported boot status: normal-boot
>
> sleep 60
> # No reboot; right since the "-c -i 10" patted + closed properly
>
> ===
>
> I also tried the new functionality enabled by your new patch and added
> "timeout-sec = <10>" to my device tree.  That seemed to work.  Upon
> bootup the watchdog defaulted to 10 seconds.  I also tried setting it
> to 86 in the device tree with similar success.
>
> ===
>
> The above seem to cover all the cases I can think of.
>
> Tested-by: Douglas Anderson <dianders@chromium.org>
>

Excellent - thanks a lot for the testing!

Guenter

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web