Re: [PATCH v10 5/5] Watchdog: ARM SBSA Generic Watchdog half timeout panic support

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 02/05/2016 01:51 AM, Fu Wei wrote:
Hi Guenter,

On 4 February 2016 at 13:17, Guenter Roeck <linux@xxxxxxxxxxxx> wrote:
On 02/03/2016 03:00 PM, Fu Wei wrote:

On 4 February 2016 at 02:45, Timur Tabi <timur@xxxxxxxxxxxxxx> wrote:

Fu Wei wrote:


As you know I have made the pre-timeout support patch, If people like
it, i am happy to go on upstream it separately.

If we want to use pre-timeout here, user only can use get_pretimeout
and disable panic by setting pretimeout to 0
but user can not really set pretimeout, because "pre-timeout  ==
timeout / 2 (always)".
if user want to change pretimeout, he/she has to set_time instead.



Ok, I think patches 4 and 5 should be combined, and I think the Kconfig
entry should be removed and just use panic_enabled.


Agreed.

np, will do



NP, will update this patchset like that ,  thanks :-)


Also, if panic is enabled, the timeout needs to be adjusted accordingly
(to only panic after the entire timeout period has expired, not after
half of it). We can not panic the system after timeout / 2.

OK, my thought is

if panic is enabled :
|--------WOR-------WS0--------WOR-------WS1
|------timeout------(panic)------timeout-----reset

if panic is disabled .
|--------WOR-------WS0--------WOR-------WS1
|---------------------timeout---------------------reset

  panic_enabled only can be configured when module is loaded by module parameter

But user should know that max_timeout(panic_enable) =
max_timeout(panic_disable) / 2


That means you'll have to update max_timeout accordingly.


I am not too happy with the parameter name (panic_enabled). How about
"action", to match machzwd ?

yes, makes sense. Maybe we can do something  like this:

/*
  * action refers to action taken when watchdog gets WS0
  * 0 = SKIP
  * 1 = PANIC
  * defaults to SKIP (0)
  */
static int action;
module_param(action, int, 0);
MODULE_PARM_DESC(action, "after watchdog gets WS0 interrupt, do: "
"0 = SKIP(*)  1 = PANIC");

Yes, though I would suggest to use lower case letters.

Thanks,
Guenter

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]
  Powered by Linux