linux-kernel - Re: [PATCH 0/5] watchdog: eiois200_wdt: Add EIO-IS200 Watchdog Driver

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <ce810ce8-f93c-4a9c-9d14-1e8f8f8c3e2b@roeck-us.net>
Date:   Fri, 6 Oct 2023 07:16:44 -0700
From:   Guenter Roeck <linux@...ck-us.net>
To:     Wenkai <advantech.susiteam@...il.com>
Cc:     wenkai.chung@...antech.com.tw, Susi.Driver@...antech.com,
        Wim Van Sebroeck <wim@...ux-watchdog.org>,
        linux-kernel@...r.kernel.org, linux-watchdog@...r.kernel.org
Subject: Re: [PATCH 0/5] watchdog: eiois200_wdt: Add EIO-IS200 Watchdog Driver

On Fri, Oct 06, 2023 at 05:27:48PM +0800, Wenkai wrote:
> 
> 
> Guenter Roeck 於 10/6/2023 11:02 AM 寫道:
> > On Thu, Oct 05, 2023 at 04:51:18PM +0800, advantech.susiteam@...il.com wrote:
> > > From: Wenkai <advantech.susiteam@...il.com>
> > > 
> > > This patch series aims to add support for the Advantech EIO-IS200
> > > Embedded Controller's watchdog timer to the Linux kernel. The EIO-IS200
> > > is a widely used embedded controller, and this series introduces a
> > > native driver for its watchdog timer functionality within the Linux
> > > ecosystem.
> > > 
> > I am not going to review this patch series. This is just ne watchdog driver.
> > One patch is sufficient.
> > 
> > Guenter
> Hi Guenter,
> 
> Advantech's EIO-IS200 watchdog supports 5 output pins: RESET, Power
> Button, SCI, IRQ, and GPIO. The most traditional scenario is that the
> Pretimeout triggers IRQ, and the timeout triggers RESET.
> 
> However, unfortunately, for industrial usages, there are various use
> cases, which require certain mechanisms and logic to manage which signal
> is output when Pretimeout and timeout expire. I am concerned that
> consolidating all these features into a single patch for upstream may
> lead to confusion and make the source code less readable and
> understandable.
> 

The 1st patch in your series doesn't even compile. I don't call that
understandable.

Oh, it fails to compile because you include a non-existing file from
../mfd directly and because you select a non-existing configuration option
instead of depending on it.

None of those is even remotely acceptable. Are you seriously sending me
a series of patches that don't even build to review ?

> Therefore, I have divided the implementation into 5 separate patches,
> aiming to make the code more comprehensible and acceptable. If it's
> acceptable to you, I am more than willing to provide a single patch as
> per your preference.
> 
Frankly, your series is one more nail in the coffin. I am now seriously
considering to resign as co-maintainer of the watchdog subsystem.

Guenter