From mboxrd@z Thu Jan 1 00:00:00 1970 Return-path: Received: from bh-25.webhostbox.net ([208.91.199.152]:42163 "EHLO bh-25.webhostbox.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750699AbcBEXyp (ORCPT ); Fri, 5 Feb 2016 18:54:45 -0500 Subject: Re: [PATCH v10 5/5] Watchdog: ARM SBSA Generic Watchdog half timeout panic support To: Fu Wei References: <1454519923-25230-1-git-send-email-fu.wei@linaro.org> <1454519923-25230-6-git-send-email-fu.wei@linaro.org> <56B23883.7000501@codeaurora.org> <56B23E99.1030604@codeaurora.org> <56B2423B.1020109@codeaurora.org> <56B24642.8090105@codeaurora.org> <56B24AB5.3070001@codeaurora.org> <56B2DEE4.7060901@roeck-us.net> <56B4B4E2.40105@roeck-us.net> Cc: Timur Tabi , Rob Herring , =?UTF-8?Q?Pawe=c5=82_Moll?= , Mark Rutland , Ian Campbell , Kumar Gala , Wim Van Sebroeck , Jon Corbet , Catalin Marinas , Will Deacon , Suravee Suthikulpanit , LKML , linux-watchdog@vger.kernel.org, linux-doc@vger.kernel.org, devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org, Linaro ACPI Mailman List , rruigrok@codeaurora.org, "Abdulhamid, Harb" , Christopher Covington , Dave Young , Pratyush Anand , G Gregory , Al Stone , Hanjun Guo , Jon Masters , Arnd Bergmann , Leo Duran , Sudeep Holla From: Guenter Roeck Message-ID: <56B5363D.20200@roeck-us.net> Date: Fri, 5 Feb 2016 15:54:37 -0800 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Sender: linux-watchdog-owner@vger.kernel.org List-Id: linux-watchdog@vger.kernel.org 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 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. > > panic_enabled only can be configured when module is loaded, so we > don't need to update it. > > max_timeout will only be set up in the init stage. > > Does it make sense ? :-) > Not sure I understand your problem or question. max_timeout will have to reflect the correct maximum timeout, under all circumstances. It will have to be set to the correct value before the watchdog driver is registered. Guenter From mboxrd@z Thu Jan 1 00:00:00 1970 From: linux@roeck-us.net (Guenter Roeck) Date: Fri, 5 Feb 2016 15:54:37 -0800 Subject: [PATCH v10 5/5] Watchdog: ARM SBSA Generic Watchdog half timeout panic support In-Reply-To: References: <1454519923-25230-1-git-send-email-fu.wei@linaro.org> <1454519923-25230-6-git-send-email-fu.wei@linaro.org> <56B23883.7000501@codeaurora.org> <56B23E99.1030604@codeaurora.org> <56B2423B.1020109@codeaurora.org> <56B24642.8090105@codeaurora.org> <56B24AB5.3070001@codeaurora.org> <56B2DEE4.7060901@roeck-us.net> <56B4B4E2.40105@roeck-us.net> Message-ID: <56B5363D.20200@roeck-us.net> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org 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 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. > > panic_enabled only can be configured when module is loaded, so we > don't need to update it. > > max_timeout will only be set up in the init stage. > > Does it make sense ? :-) > Not sure I understand your problem or question. max_timeout will have to reflect the correct maximum timeout, under all circumstances. It will have to be set to the correct value before the watchdog driver is registered. Guenter From mboxrd@z Thu Jan 1 00:00:00 1970 From: Guenter Roeck Subject: Re: [PATCH v10 5/5] Watchdog: ARM SBSA Generic Watchdog half timeout panic support Date: Fri, 5 Feb 2016 15:54:37 -0800 Message-ID: <56B5363D.20200@roeck-us.net> References: <1454519923-25230-1-git-send-email-fu.wei@linaro.org> <1454519923-25230-6-git-send-email-fu.wei@linaro.org> <56B23883.7000501@codeaurora.org> <56B23E99.1030604@codeaurora.org> <56B2423B.1020109@codeaurora.org> <56B24642.8090105@codeaurora.org> <56B24AB5.3070001@codeaurora.org> <56B2DEE4.7060901@roeck-us.net> <56B4B4E2.40105@roeck-us.net> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: Sender: linux-doc-owner@vger.kernel.org To: Fu Wei Cc: Timur Tabi , Rob Herring , =?UTF-8?Q?Pawe=c5=82_Moll?= , Mark Rutland , Ian Campbell , Kumar Gala , Wim Van Sebroeck , Jon Corbet , Catalin Marinas , Will Deacon , Suravee Suthikulpanit , LKML , linux-watchdog@vger.kernel.org, linux-doc@vger.kernel.org, devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org, Linaro ACPI Mailman List , rruigrok@codeaurora.org, "Abdulhamid, Harb" , Christopher Covington , Dave Young , Pratyush Anand , G Gregory , Al List-Id: devicetree@vger.kernel.org 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 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. > > panic_enabled only can be configured when module is loaded, so we > don't need to update it. > > max_timeout will only be set up in the init stage. > > Does it make sense ? :-) > Not sure I understand your problem or question. max_timeout will have to reflect the correct maximum timeout, under all circumstances. It will have to be set to the correct value before the watchdog driver is registered. Guenter