Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Subject: Re: [PATCH v4 00/10] watchdog/hpwdt: Update driver to use watchdog
 core.
To:     Jerry Hoemann <jerry.hoemann@hpe.com>, wim@linux-watchdog.org
Cc:     linux-watchdog@vger.kernel.org, linux-kernel@vger.kernel.org,
        rwright@hpe.com, maurice.a.saldivar@hpe.com, mingo@kernel.org,
        marcus.folkesson@gmail.com
References: <20180225213259.2861-1-jerry.hoemann@hpe.com>
From:   Guenter Roeck <linux@roeck-us.net>
Message-ID: <30f31e66-3d32-516f-f0a4-f3fba13b5677@roeck-us.net>
Date:   Sun, 25 Feb 2018 16:50:18 -0800
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101
 Thunderbird/52.6.0
MIME-Version: 1.0
In-Reply-To: <20180225213259.2861-1-jerry.hoemann@hpe.com>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On 02/25/2018 01:32 PM, Jerry Hoemann wrote:
> == v4 ==
> 
> Modifications from prior version:
> 
> Manual merging and patch reordering has caused minor white
> space and diff changes.  Such change shouldn't have effected
> the logic and are omitted below:
> 
For the most part looks good except for the nitpicks, and the
change log below no longer matches reality since you added a
new patch 1 which is not reflected in the patch numbering
(you added the reference to the new patch at the end, so
according to the log you now have two patches #1).

Thanks,
Guenter

> Patch 001  Remove legacy NMI sourcing.
> 1) Updated patch documentation to reflect Spectre concerns.
> 2) Restored comment at #endif
> 
> Patch 002 watchdog/hpwdt: remove include files no longer
> 1) Squashed onto Patch001.
> 
> Patch 0003: watchdog/hpwdt: Update nmi_panic message.
> 1) Unchanged
> 
> 
> Patch 0004:  watchdog/hpwdt: white space changes
> 1) Redacted.
> 
> Patch 0005: watchdog/hpwdt: Update Module info.
> 1) Now Patch 0004
> 2) Update only module description to reflect branding changes.
> 3) Rest of patch redacted.
> 
> Patch 0006 Select CORE
> 1) Merged with 0007
> 
> Patch 0007 Modify to use watchdog core
> 1) squashed on Patch 0006.
> 2) Now Patch 0005
> 3) Requested white space changes.
> 4) Use "wdd" for watchdog_device like other watchdog drivers do
> 
> Patch 0008 Programable-Pretimeout
> 1) hpwdt_settimeout re-worked to take into account timeout <= pretimeout.
>     Patterned after watchdog_set_timeout.
> 2) hpwdt_set_pretimeout re-worked to take into account requested
>     pretimeout <= timeout.
> 3) Clarify patch doc to reflect above.
> 
> Patch 0009 condition-early-return-of-NMI
> 1) Requested syntatic surgar.
> 2) Now Patch 0006
> 
> Patch 0010 remove-allow_kdump
> 1) Now Patch 0007
> 
> Patch 0011 Add-dynamic-debug
> 1) Now Patch 0009
> 2) Typo fix
> 3) New dev_dbg resulting from changes to set timeout and pretimeout.
> 
> Patch 0012 Update driver version.
> 1) Now Patch 0010
> 
>   
> New Patch
> 1) Patch 0001 Update Copyright
> 
> 
> 
> == v3 ==
> 
> Incorperating code review feedback.
> 
> 1) Patch 0003: Use existing hex_byte_pack instead of creating new function.
> 2) Patch 0005: Redacted change in module_param permission.
> 3) Patch 0006: switch from pr_debug etc., to dev_dbg where possible.
> 4) Patch 0006: No longer updating soft_margin post module load.
> 5) Patch 0006: Initialize hpwdt_dev.parent before registering watchdog.
> 6) Patch 0006: Redacted change to dev_info message w.r.t. allow_kdump
> 7) Patch 0006 & 0007: Reorder patches to maintain bisectability.
> 8) Patch 0008: Change pr_debug to dev_dbg
> 9) Patch 0010: Change dev_info message w.r.t. allow_kdump where feature
> 		is removed.
> 
> Note, I am explicitly ignoring the checkpatch error on Patch 0008
> for specifying permisson of "0" instead of "0000".
> 
> == v2 ==
> 
> 1) Fix compiler error when CONFIG_HPWDT_NMI_DECODING is not defined.
> 
> 2) Break out driver version change to its own patch (0011).
> 
> 
> == v1 ==
> 
> The primary purposes of this patch set are to
> 
> 1) Update the hpwdt driver to use the watchdog core.
> 2) Reduce complexity by removing unnecessary features.
> 3) Add customer requested features like optional pretimeout.
> 4) Enhance readability/maintainability of the driver.
> 
> The size of the resultant driver is reduced from over 900
> lines to 350 lines.
> 
> Patch 1& 2 remove legacy NMI sourcing.
> Patch 3	adds useful indication of NMI cause to panic message
> Patch 4 & 5 are general cleanup
> Patch 6 & 7 updates the driver to user the watchdog core.
> Patch 8 makes the pretimeout NMI programmable.
> Patch 9 modifies whether the NMI handler claims the NMI.
> Patch 10 retracts the allow_kdump module parameter.
> 
> 
> 
> Jerry Hoemann (10):
>    watchdog/hpwdt: Update copyright.
>    watchdog/hpwdt: Remove legacy NMI sourcing.
>    watchdog/hpwdt: Update nmi_panic message.
>    watchdog/hpwdt: Update Module info.
>    watchdog/hpwdt: Modify to use watchdog core.
>    watchdog/hpwdt: condition early return of NMI handler on iLO5
>    watchdog/hpwdt: remove allow_kdump module parameter.
>    watchdog/hpwdt: Programable Pretimeout NMI
>    watchdog/hpwdt: Add dynamic debug
>    watchdog/hpwdt: Update driver version.
> 
>   drivers/watchdog/Kconfig |   1 +
>   drivers/watchdog/hpwdt.c | 789 +++++++----------------------------------------
>   2 files changed, 121 insertions(+), 669 deletions(-)
>