All of lore.kernel.org
 help / color / mirror / Atom feed
From: Guenter Roeck <linux@roeck-us.net>
To: Wim Coekaerts <wim.coekaerts@oracle.com>, wim@iguana.be
Cc: linux-watchdog@vger.kernel.org, sparclinux@vger.kernel.org
Subject: Re: [PATCH v3] Add sun4v_wdt watchdog driver
Date: Mon, 25 Jan 2016 13:15:08 -0800	[thread overview]
Message-ID: <56A6905C.4070106@roeck-us.net> (raw)
In-Reply-To: <56A68F1B.80605@oracle.com>

On 01/25/2016 01:09 PM, Wim Coekaerts wrote:
> On 01/25/2016 01:02 PM, Guenter Roeck wrote:
>> On 01/25/2016 12:38 PM, Wim Coekaerts wrote:
>>> This driver adds sparc hypervisor watchdog support. Timeout is set in
>>> milliseconds since that is the granularity supported and it honors
>>> the settings of both the watchdog-resolution and watchdog-max-timeout
>>> MD properties.
>>>
>>> Default timeout is set at 60 seconds or 60000ms. The range is between
>>> 1 second and 180 seconds. The default resolution is 1000ms.
>>>
>>> Signed-off-by: Wim Coekaerts <wim.coekaerts@oracle.com>
>>> ---
>>> Changes in v3:
>>>    - Modify sun4v_mach_set_watchdog to allow for NULL and remove
>>>     time_remaining
>>>    - Cleanup includes
>>>    - Consolidate _start and _ping into _ping since they were the same
>>>    - Fix checkpatch warnings
>>>    - Remove pr_info()s
>>>    - Clean up return codes to be standard kernel return values
>>>    - Consolidate _init and _probe into _init
>>>     Don't use platform_device anymore because this is really
>>>     just a driver that depends on a hv call. This now looks more
>>>     like softdog.
>>>    - Cleanly check for sun4v architecture and remove extra tests
>>>    - Convert to ms timer support and honor resolution
>>>     since most drivers use seconds I added _ms to make it clear
>>
>> You can not redefine the watchdog ABI to mean milli-seconds instead of seconds.
>
> ugh ok. Then it makes honoring resolutions a bit useless (comment on
> previous patch about having a fixed 1s resolution)
>

Why ? You can still round the resolution to seconds, and use the milli-second
resolution internally while keeping the ABI stable. Many other drivers have a
hardware watchdog resolution which differs from 1 second.

Guenter


WARNING: multiple messages have this Message-ID (diff)
From: Guenter Roeck <linux@roeck-us.net>
To: Wim Coekaerts <wim.coekaerts@oracle.com>, wim@iguana.be
Cc: linux-watchdog@vger.kernel.org, sparclinux@vger.kernel.org
Subject: Re: [PATCH v3] Add sun4v_wdt watchdog driver
Date: Mon, 25 Jan 2016 21:15:08 +0000	[thread overview]
Message-ID: <56A6905C.4070106@roeck-us.net> (raw)
In-Reply-To: <56A68F1B.80605@oracle.com>

On 01/25/2016 01:09 PM, Wim Coekaerts wrote:
> On 01/25/2016 01:02 PM, Guenter Roeck wrote:
>> On 01/25/2016 12:38 PM, Wim Coekaerts wrote:
>>> This driver adds sparc hypervisor watchdog support. Timeout is set in
>>> milliseconds since that is the granularity supported and it honors
>>> the settings of both the watchdog-resolution and watchdog-max-timeout
>>> MD properties.
>>>
>>> Default timeout is set at 60 seconds or 60000ms. The range is between
>>> 1 second and 180 seconds. The default resolution is 1000ms.
>>>
>>> Signed-off-by: Wim Coekaerts <wim.coekaerts@oracle.com>
>>> ---
>>> Changes in v3:
>>>    - Modify sun4v_mach_set_watchdog to allow for NULL and remove
>>>     time_remaining
>>>    - Cleanup includes
>>>    - Consolidate _start and _ping into _ping since they were the same
>>>    - Fix checkpatch warnings
>>>    - Remove pr_info()s
>>>    - Clean up return codes to be standard kernel return values
>>>    - Consolidate _init and _probe into _init
>>>     Don't use platform_device anymore because this is really
>>>     just a driver that depends on a hv call. This now looks more
>>>     like softdog.
>>>    - Cleanly check for sun4v architecture and remove extra tests
>>>    - Convert to ms timer support and honor resolution
>>>     since most drivers use seconds I added _ms to make it clear
>>
>> You can not redefine the watchdog ABI to mean milli-seconds instead of seconds.
>
> ugh ok. Then it makes honoring resolutions a bit useless (comment on
> previous patch about having a fixed 1s resolution)
>

Why ? You can still round the resolution to seconds, and use the milli-second
resolution internally while keeping the ABI stable. Many other drivers have a
hardware watchdog resolution which differs from 1 second.

Guenter


  reply	other threads:[~2016-01-25 21:15 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-01-25 20:38 [PATCH v3] Add sun4v_wdt watchdog driver Wim Coekaerts
2016-01-25 20:38 ` Wim Coekaerts
2016-01-25 21:02 ` Guenter Roeck
2016-01-25 21:02   ` Guenter Roeck
2016-01-25 21:09   ` Wim Coekaerts
2016-01-25 21:09     ` Wim Coekaerts
2016-01-25 21:15     ` Guenter Roeck [this message]
2016-01-25 21:15       ` Guenter Roeck

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=56A6905C.4070106@roeck-us.net \
    --to=linux@roeck-us.net \
    --cc=linux-watchdog@vger.kernel.org \
    --cc=sparclinux@vger.kernel.org \
    --cc=wim.coekaerts@oracle.com \
    --cc=wim@iguana.be \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.