| Message ID | 20210517174953.19404-1-shruthi.sanil@intel.com |
|---|---|
| Headers | show |
| Series | Intel Keem Bay WDT bug fixes | expand |
Hi Roeck, I have addressed your review comments in the v2 version of this patch series. Could you please review? If no comments, can I get an Acked-by tag? Thanks! Regards, Shruthi > -----Original Message----- > From: Sanil, Shruthi <shruthi.sanil@intel.com> > Sent: Monday, May 17, 2021 11:20 PM > To: wim@linux-watchdog.org; linux@roeck-us.net; linux- > watchdog@vger.kernel.org; linux-kernel@vger.kernel.org > Cc: andriy.shevchenko@linux.intel.com; kris.pan@linux.intel.com; > mgross@linux.intel.com; Thokala, Srikanth <srikanth.thokala@intel.com>; > Raja Subramanian, Lakshmi Bai <lakshmi.bai.raja.subramanian@intel.com>; > Sangannavar, Mallikarjunappa <mallikarjunappa.sangannavar@intel.com>; > Sanil, Shruthi <shruthi.sanil@intel.com> > Subject: [PATCH v2 0/9] Intel Keem Bay WDT bug fixes > > From: Shruthi Sanil <shruthi.sanil@intel.com> > > The series of patches include the below bug fixes in the Intel Keem Bay > watchdog timer driver: > > Patch 1/9: > - Update WDT pre-timeout during the initialization > The pretimeout register has a default reset value. Hence > when a smaller WDT timeout is set which would be lesser than the > default pretimeout, the system behaves abnormally, starts > triggering the pretimeout interrupt even when the WDT is > not enabled, most of the times leading to system crash. > Hence an update in the pre-timeout is also required for the > default timeout that is being configured. > > Patch 2/9: > - Upadate WDT pretimeout for every update in timeout > The pre-timeout value to be programmed to the register has to be > calculated and updated for every change in the timeout value. > Else the threshold time wouldn't be calculated to its > corresponding timeout. > > Patch 3/9: > - Update pretimeout to 0 in the TH ISR > The pretimeout has to be updated to 0 during the ISR of the > ThresHold interrupt. Else the TH interrupt would be triggerred for > every tick until the timeout. > > Patch 4/9: > - Clear either the TO or TH interrupt bit > During the interrupt service routine of the TimeOut interrupt and > the ThresHold interrupt, the respective interrupt clear bit > have to be cleared and not both. > > Patch 5/9: > - Remove timeout update in the WDT start function > Removed set timeout from the start WDT function. There is a function > defined to set the timeout. Hence no need to set the timeout again in > start function as the timeout would have been already updated > before calling the start/enable. > > Patch 6/9: > - Removed timeout update in the TO ISR > In the TO ISR removed updating the Timeout value because > its not serving any purpose as the timer would have already expired > and the system would be rebooting. > > Patch 7/9: > - MACRO for WDT enable and disable values > Introduced MACRO's for WDT enable and disable values for better > readability > > Patch 8/9: > - WDT SMC handler MACRO name update > Updated the WDT SMC handler MACRO name to make it clear that its > a ARM SMC handler that helps in clearing the WDT interrupt bit. > > Patch 9/9: > - Typo corrections and other blank operations > Corrected typos, aligned the tabs and added new lines > wherever required for better readability > > Changes since v1: > - Dropped a patch with the incorrect fix > regarding the WDT suspend/resume function. > > Shruthi Sanil (9): > watchdog: keembay: Update WDT pre-timeout during the initialization > watchdog: keembay: Upadate WDT pretimeout for every update in timeout > watchdog: keembay: Update pretimeout to zero in the TH ISR > watchdog: keembay: Clear either the TO or TH interrupt bit > watchdog: keembay: Remove timeout update in the WDT start function > watchdog: keembay: Removed timeout update in the TO ISR > watchdog: keembay: MACRO for WDT enable and disable values > watchdog: keembay: WDT SMC handler MACRO name update > watchdog: keembay: Typo corrections and other blank operations > > drivers/watchdog/keembay_wdt.c | 34 ++++++++++++++++++++++------------ > 1 file changed, 22 insertions(+), 12 deletions(-) > > > base-commit: 88b06399c9c766c283e070b022b5ceafa4f63f19 > -- > 2.17.1
On Mon, May 24, 2021 at 06:06:35AM +0000, Sanil, Shruthi wrote: > Hi Roeck, > > I have addressed your review comments in the v2 version of this patch series. > Could you please review? > > If no comments, can I get an Acked-by tag? > Thanks! First of all, do not top post! > > From: Sanil, Shruthi <shruthi.sanil@intel.com> > > Sent: Monday, May 17, 2021 11:20 PM AFAICS Guenter had reviewed (as you put his tags into commits). It means that maintainer will pickup patches when they feel it's a good time. > > The series of patches include the below bug fixes in the Intel Keem Bay > > watchdog timer driver: -- With Best Regards, Andy Shevchenko
On 5/24/21 3:36 AM, andriy.shevchenko@linux.intel.com wrote: > On Mon, May 24, 2021 at 06:06:35AM +0000, Sanil, Shruthi wrote: >> Hi Roeck, >> >> I have addressed your review comments in the v2 version of this patch series. >> Could you please review? >> >> If no comments, can I get an Acked-by tag? >> Thanks! > > First of all, do not top post! > >>> From: Sanil, Shruthi <shruthi.sanil@intel.com> >>> Sent: Monday, May 17, 2021 11:20 PM > > AFAICS Guenter had reviewed (as you put his tags into commits). It means that > maintainer will pickup patches when they feel it's a good time. > Yes, and the patches are queued in my own watchdog-next branch. At this point we'll have to wait for the Wim to pick up the series. Guenter
From: Shruthi Sanil <shruthi.sanil@intel.com> The series of patches include the below bug fixes in the Intel Keem Bay watchdog timer driver: Patch 1/9: - Update WDT pre-timeout during the initialization The pretimeout register has a default reset value. Hence when a smaller WDT timeout is set which would be lesser than the default pretimeout, the system behaves abnormally, starts triggering the pretimeout interrupt even when the WDT is not enabled, most of the times leading to system crash. Hence an update in the pre-timeout is also required for the default timeout that is being configured. Patch 2/9: - Upadate WDT pretimeout for every update in timeout The pre-timeout value to be programmed to the register has to be calculated and updated for every change in the timeout value. Else the threshold time wouldn't be calculated to its corresponding timeout. Patch 3/9: - Update pretimeout to 0 in the TH ISR The pretimeout has to be updated to 0 during the ISR of the ThresHold interrupt. Else the TH interrupt would be triggerred for every tick until the timeout. Patch 4/9: - Clear either the TO or TH interrupt bit During the interrupt service routine of the TimeOut interrupt and the ThresHold interrupt, the respective interrupt clear bit have to be cleared and not both. Patch 5/9: - Remove timeout update in the WDT start function Removed set timeout from the start WDT function. There is a function defined to set the timeout. Hence no need to set the timeout again in start function as the timeout would have been already updated before calling the start/enable. Patch 6/9: - Removed timeout update in the TO ISR In the TO ISR removed updating the Timeout value because its not serving any purpose as the timer would have already expired and the system would be rebooting. Patch 7/9: - MACRO for WDT enable and disable values Introduced MACRO's for WDT enable and disable values for better readability Patch 8/9: - WDT SMC handler MACRO name update Updated the WDT SMC handler MACRO name to make it clear that its a ARM SMC handler that helps in clearing the WDT interrupt bit. Patch 9/9: - Typo corrections and other blank operations Corrected typos, aligned the tabs and added new lines wherever required for better readability Changes since v1: - Dropped a patch with the incorrect fix regarding the WDT suspend/resume function. Shruthi Sanil (9): watchdog: keembay: Update WDT pre-timeout during the initialization watchdog: keembay: Upadate WDT pretimeout for every update in timeout watchdog: keembay: Update pretimeout to zero in the TH ISR watchdog: keembay: Clear either the TO or TH interrupt bit watchdog: keembay: Remove timeout update in the WDT start function watchdog: keembay: Removed timeout update in the TO ISR watchdog: keembay: MACRO for WDT enable and disable values watchdog: keembay: WDT SMC handler MACRO name update watchdog: keembay: Typo corrections and other blank operations drivers/watchdog/keembay_wdt.c | 34 ++++++++++++++++++++++------------ 1 file changed, 22 insertions(+), 12 deletions(-) base-commit: 88b06399c9c766c283e070b022b5ceafa4f63f19