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

On 7 February 2016 at 02:57, Guenter Roeck wrote: > On 02/06/2016 10:02 AM, Fu Wei wrote: >> >> Hi Guenter, >> >> On 6 February 2016 at 07:54, Guenter Roeck wrote: >>> >>> On 02/05/2016 10:21 AM, Fu Wei wrote: On 5 February 2016 at 22:42, Guenter Roeck wrote: > > > On

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

On 7 February 2016 at 02:55, Timur Tabi wrote: > Fu Wei wrote: >> >> if (action) { >> wdd->min_timeout = 1; >> wdd->max_timeout = U32_MAX / gwdt->clk; >> } else { >> wdd->min_timeout = 2; >> wdd->max_timeout = U32_MAX / gwdt->clk * 2; >> } > > > Why would the

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

On 02/06/2016 10:02 AM, Fu Wei wrote: Hi Guenter, On 6 February 2016 at 07:54, Guenter Roeck wrote: On 02/05/2016 10:21 AM, Fu Wei wrote: On 5 February 2016 at 22:42, Guenter Roeck wrote: On 02/05/2016 01:51 AM, Fu Wei wrote: Hi Guenter, On 4 February 2016 at 13:17, Guenter Roeck

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

Fu Wei wrote: if (action) { wdd->min_timeout = 1; wdd->max_timeout = U32_MAX / gwdt->clk; } else { wdd->min_timeout = 2; wdd->max_timeout = U32_MAX / gwdt->clk * 2; } Why would the minimum timeout be 2? You can program WOR to timeout in half a second,

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

Hi Guenter, On 6 February 2016 at 07:54, Guenter Roeck wrote: > On 02/05/2016 10:21 AM, Fu Wei wrote: >> >> On 5 February 2016 at 22:42, Guenter Roeck wrote: >>> >>> On 02/05/2016 01:51 AM, Fu Wei wrote: Hi Guenter, On 4 February 2016 at 13:17, Guenter Roeck wrote:

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

Hi Guenter, On 6 February 2016 at 07:54, Guenter Roeck wrote: > On 02/05/2016 10:21 AM, Fu Wei wrote: >> >> On 5 February 2016 at 22:42, Guenter Roeck wrote: >>> >>> On 02/05/2016 01:51 AM, Fu Wei wrote: Hi Guenter, On 4 February

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

Fu Wei wrote: if (action) { wdd->min_timeout = 1; wdd->max_timeout = U32_MAX / gwdt->clk; } else { wdd->min_timeout = 2; wdd->max_timeout = U32_MAX / gwdt->clk * 2; } Why would the minimum timeout be 2? You can program WOR to timeout in half a second,

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

On 02/06/2016 10:02 AM, Fu Wei wrote: Hi Guenter, On 6 February 2016 at 07:54, Guenter Roeck wrote: On 02/05/2016 10:21 AM, Fu Wei wrote: On 5 February 2016 at 22:42, Guenter Roeck wrote: On 02/05/2016 01:51 AM, Fu Wei wrote: Hi Guenter, On 4

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

On 7 February 2016 at 02:55, Timur Tabi wrote: > Fu Wei wrote: >> >> if (action) { >> wdd->min_timeout = 1; >> wdd->max_timeout = U32_MAX / gwdt->clk; >> } else { >> wdd->min_timeout = 2; >> wdd->max_timeout = U32_MAX / gwdt->clk * 2; >> }

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

On 7 February 2016 at 02:57, Guenter Roeck wrote: > On 02/06/2016 10:02 AM, Fu Wei wrote: >> >> Hi Guenter, >> >> On 6 February 2016 at 07:54, Guenter Roeck wrote: >>> >>> On 02/05/2016 10:21 AM, Fu Wei wrote: On 5 February 2016 at 22:42,

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

On 02/05/2016 10:21 AM, Fu Wei wrote: On 5 February 2016 at 22:42, Guenter Roeck wrote: On 02/05/2016 01:51 AM, Fu Wei wrote: Hi Guenter, On 4 February 2016 at 13:17, Guenter Roeck wrote: On 02/03/2016 03:00 PM, Fu Wei wrote: On 4 February 2016 at 02:45, Timur Tabi wrote: Fu Wei

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

On 5 February 2016 at 22:42, Guenter Roeck wrote: > On 02/05/2016 01:51 AM, Fu Wei wrote: >> >> Hi Guenter, >> >> On 4 February 2016 at 13:17, Guenter Roeck wrote: >>> >>> On 02/03/2016 03:00 PM, Fu Wei wrote: On 4 February 2016 at 02:45, Timur Tabi wrote: > > > Fu

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

On 02/05/2016 01:51 AM, Fu Wei wrote: Hi Guenter, On 4 February 2016 at 13:17, Guenter Roeck wrote: On 02/03/2016 03:00 PM, Fu Wei wrote: On 4 February 2016 at 02:45, Timur Tabi wrote: Fu Wei wrote: As you know I have made the pre-timeout support patch, If people like it, i am happy

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

Hello, On Fri, 5 Feb 2016 07:08:23 -0600, Timur Tabi wrote: > > I'm quite certainly missing something completely obvious here, but how > > can you get the WS1 interrupt*after* raising a panic? Aren't all > > interrupts disabled and the system fully halted once you get a panic(), > > especially

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

Thomas Petazzoni wrote: if panic is enabled : >|WOR---WS0WOR---WS1 >|--timeout--(panic)--timeout-reset I'm quite certainly missing something completely obvious here, but how can you get the WS1 interrupt*after* raising a panic? Aren't all interrupts

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

Hello, On Fri, 5 Feb 2016 17:51:52 +0800, Fu Wei wrote: > OK, my thought is > > if panic is enabled : > |WOR---WS0WOR---WS1 > |--timeout--(panic)--timeout-reset I'm quite certainly missing something completely obvious here, but how can you get the WS1

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

Hi Guenter, On 4 February 2016 at 13:17, Guenter Roeck wrote: > On 02/03/2016 03:00 PM, Fu Wei wrote: >> >> On 4 February 2016 at 02:45, Timur Tabi 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

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

On 5 February 2016 at 00:43, Timur Tabi wrote: > Mathieu Poirier wrote: >>> >>> >+#ifdef CONFIG_ARM_SBSA_WATCHDOG_PANIC >>> >+ irq = platform_get_irq(pdev, 0); >>> >+ if (irq < 0) { >>> >+ dev_err(dev, "unable to get ws0 interrupt.\n"); >>> >+ return irq;

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

On 5 February 2016 at 22:42, Guenter Roeck wrote: > On 02/05/2016 01:51 AM, Fu Wei wrote: >> >> Hi Guenter, >> >> On 4 February 2016 at 13:17, Guenter Roeck wrote: >>> >>> On 02/03/2016 03:00 PM, Fu Wei wrote: On 4 February 2016 at 02:45,

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

Hi Guenter, On 4 February 2016 at 13:17, Guenter Roeck wrote: > On 02/03/2016 03:00 PM, Fu Wei wrote: >> >> On 4 February 2016 at 02:45, Timur Tabi wrote: >>> >>> Fu Wei wrote: As you know I have made the pre-timeout support patch, If

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

On 5 February 2016 at 00:43, Timur Tabi wrote: > Mathieu Poirier wrote: >>> >>> >+#ifdef CONFIG_ARM_SBSA_WATCHDOG_PANIC >>> >+ irq = platform_get_irq(pdev, 0); >>> >+ if (irq < 0) { >>> >+ dev_err(dev, "unable to get ws0 interrupt.\n"); >>> >+

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

Hello, On Fri, 5 Feb 2016 17:51:52 +0800, Fu Wei wrote: > OK, my thought is > > if panic is enabled : > |WOR---WS0WOR---WS1 > |--timeout--(panic)--timeout-reset I'm quite certainly missing something completely obvious here, but how can you get the WS1

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

Thomas Petazzoni wrote: if panic is enabled : >|WOR---WS0WOR---WS1 >|--timeout--(panic)--timeout-reset I'm quite certainly missing something completely obvious here, but how can you get the WS1 interrupt*after* raising a panic? Aren't all interrupts

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

Hello, On Fri, 5 Feb 2016 07:08:23 -0600, Timur Tabi wrote: > > I'm quite certainly missing something completely obvious here, but how > > can you get the WS1 interrupt*after* raising a panic? Aren't all > > interrupts disabled and the system fully halted once you get a panic(), > > especially

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

On 02/05/2016 01:51 AM, Fu Wei wrote: Hi Guenter, On 4 February 2016 at 13:17, Guenter Roeck wrote: On 02/03/2016 03:00 PM, Fu Wei wrote: On 4 February 2016 at 02:45, Timur Tabi wrote: Fu Wei wrote: As you know I have made the pre-timeout

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

On 02/05/2016 10:21 AM, Fu Wei wrote: On 5 February 2016 at 22:42, Guenter Roeck wrote: On 02/05/2016 01:51 AM, Fu Wei wrote: Hi Guenter, On 4 February 2016 at 13:17, Guenter Roeck wrote: On 02/03/2016 03:00 PM, Fu Wei wrote: On 4 February 2016

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

On 02/04/2016 08:32 AM, Mathieu Poirier wrote: On 3 February 2016 at 10:18, wrote: From: Fu Wei This patch registers the WS0 interrupt routine to trigger panic, when the watchdog reachs the first stage (the half timeout). This function can help administrator to backup the system context

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

On 3 February 2016 at 10:18, wrote: > From: Fu Wei > > This patch registers the WS0 interrupt routine to trigger panic, > when the watchdog reachs the first stage (the half timeout). > This function can help administrator to backup the system context > info by panic console output or kdump (if

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

Mathieu Poirier wrote: >+#ifdef CONFIG_ARM_SBSA_WATCHDOG_PANIC >+ irq = platform_get_irq(pdev, 0); >+ if (irq < 0) { >+ dev_err(dev, "unable to get ws0 interrupt.\n"); >+ return irq; >+ } >+#endif >+ Can't the driver revert to single stage mode if

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

On 02/04/2016 05:48 AM, Timur Tabi wrote: Guenter Roeck wrote: 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. It's a debugging feature,

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

Guenter Roeck wrote: 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. It's a debugging feature, not an actual watchdog timeout panic. That's

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

Guenter Roeck wrote: 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. It's a debugging feature, not an actual watchdog timeout panic. That's

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

On 02/04/2016 05:48 AM, Timur Tabi wrote: Guenter Roeck wrote: 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. It's a debugging feature,

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

On 02/04/2016 08:32 AM, Mathieu Poirier wrote: On 3 February 2016 at 10:18, wrote: From: Fu Wei This patch registers the WS0 interrupt routine to trigger panic, when the watchdog reachs the first stage (the half timeout). This function can help

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

On 3 February 2016 at 10:18, wrote: > From: Fu Wei > > This patch registers the WS0 interrupt routine to trigger panic, > when the watchdog reachs the first stage (the half timeout). > This function can help administrator to backup the system context > info

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

Mathieu Poirier wrote: >+#ifdef CONFIG_ARM_SBSA_WATCHDOG_PANIC >+ irq = platform_get_irq(pdev, 0); >+ if (irq < 0) { >+ dev_err(dev, "unable to get ws0 interrupt.\n"); >+ return irq; >+ } >+#endif >+ Can't the driver revert to single stage mode if

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

On 02/03/2016 03:00 PM, Fu Wei wrote: On 4 February 2016 at 02:45, Timur Tabi 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

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

On 4 February 2016 at 02:45, Timur Tabi 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

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

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

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

On 4 February 2016 at 02:26, Timur Tabi wrote: > Fu Wei wrote: >> >> Actually, >> the SBSA watchdog driver should support only half-timeout for panic >> the user cannot configure the length of "panic time", He can only >> enable it, and it is automatically set to 1/2 timeout. >> >> we don't

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

Fu Wei wrote: Actually, the SBSA watchdog driver should support only half-timeout for panic the user cannot configure the length of "panic time", He can only enable it, and it is automatically set to 1/2 timeout. we don't need pre-timeout here. Hope I understand you correctly:-) sorry for

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

On 4 February 2016 at 02:08, Timur Tabi wrote: > Fu Wei wrote: >> >> Actually I am taking your suggestion to avoid touching WCV in >> interrupt routine. >> So even we have pre-timeout support , it is useless for this >> panic-on-half-timeout feature, >> because pre-timeout == timeout / 2

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

Fu Wei wrote: Actually I am taking your suggestion to avoid touching WCV in interrupt routine. So even we have pre-timeout support , it is useless for this panic-on-half-timeout feature, because pre-timeout == timeout / 2 (always). So maybe I misunderstand your suggestion, could you let me

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

Hi Timur On 4 February 2016 at 01:53, Timur Tabi wrote: > Fu Wei wrote: >> >> sorry, are you saying : using pre-timeout instead of this half timeout? >> >> But even we have pre-timeout support, pre-timeout == timeout / 2, it >> can not be configured without touch timeout. >> >> if you want

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

Fu Wei wrote: sorry, are you saying : using pre-timeout instead of this half timeout? But even we have pre-timeout support, pre-timeout == timeout / 2, it can not be configured without touch timeout. if you want pre-timeout != timeout / 2, we have to modify WCV in the interrupt routine.

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

Hi Timur, Thanks for your rapid feedback :-) On 4 February 2016 at 01:27, Timur Tabi wrote: > fu@linaro.org wrote: >> >> +#ifdef CONFIG_ARM_SBSA_WATCHDOG_PANIC >> +static bool panic_enabled = true; > > > I think this should default to 'false', because IMHO, this seems like an odd yes, It

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

fu@linaro.org wrote: +#ifdef CONFIG_ARM_SBSA_WATCHDOG_PANIC +static bool panic_enabled = true; I think this should default to 'false', because IMHO, this seems like an odd feature. I'm not crazy about the fact that there's a Kconfig option for it either, but I'm not going to NACK this

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

From: Fu Wei This patch registers the WS0 interrupt routine to trigger panic, when the watchdog reachs the first stage (the half timeout). This function can help administrator to backup the system context info by panic console output or kdump (if supported), once system goes wrong (doesn't feed

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

On 4 February 2016 at 02:45, Timur Tabi 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

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

On 02/03/2016 03:00 PM, Fu Wei wrote: On 4 February 2016 at 02:45, Timur Tabi 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

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

Hi Timur, Thanks for your rapid feedback :-) On 4 February 2016 at 01:27, Timur Tabi wrote: > fu@linaro.org wrote: >> >> +#ifdef CONFIG_ARM_SBSA_WATCHDOG_PANIC >> +static bool panic_enabled = true; > > > I think this should default to 'false', because IMHO, this seems

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

Fu Wei wrote: sorry, are you saying : using pre-timeout instead of this half timeout? But even we have pre-timeout support, pre-timeout == timeout / 2, it can not be configured without touch timeout. if you want pre-timeout != timeout / 2, we have to modify WCV in the interrupt routine.

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

Hi Timur On 4 February 2016 at 01:53, Timur Tabi wrote: > Fu Wei wrote: >> >> sorry, are you saying : using pre-timeout instead of this half timeout? >> >> But even we have pre-timeout support, pre-timeout == timeout / 2, it >> can not be configured without touch timeout.

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

Fu Wei wrote: Actually I am taking your suggestion to avoid touching WCV in interrupt routine. So even we have pre-timeout support , it is useless for this panic-on-half-timeout feature, because pre-timeout == timeout / 2 (always). So maybe I misunderstand your suggestion, could you let me

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

From: Fu Wei This patch registers the WS0 interrupt routine to trigger panic, when the watchdog reachs the first stage (the half timeout). This function can help administrator to backup the system context info by panic console output or kdump (if supported), once system goes

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

fu@linaro.org wrote: +#ifdef CONFIG_ARM_SBSA_WATCHDOG_PANIC +static bool panic_enabled = true; I think this should default to 'false', because IMHO, this seems like an odd feature. I'm not crazy about the fact that there's a Kconfig option for it either, but I'm not going to NACK this

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

On 4 February 2016 at 02:08, Timur Tabi wrote: > Fu Wei wrote: >> >> Actually I am taking your suggestion to avoid touching WCV in >> interrupt routine. >> So even we have pre-timeout support , it is useless for this >> panic-on-half-timeout feature, >> because pre-timeout

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

On 4 February 2016 at 02:26, Timur Tabi wrote: > Fu Wei wrote: >> >> Actually, >> the SBSA watchdog driver should support only half-timeout for panic >> the user cannot configure the length of "panic time", He can only >> enable it, and it is automatically set to 1/2

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

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

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

Fu Wei wrote: Actually, the SBSA watchdog driver should support only half-timeout for panic the user cannot configure the length of "panic time", He can only enable it, and it is automatically set to 1/2 timeout. we don't need pre-timeout here. Hope I understand you correctly:-) sorry for