Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:4 as permitted sender) client-ip=2620:137:e000::3:4;
Sender: Guenter Roeck <groeck7@gmail.com>
Date:   Fri, 6 Oct 2023 07:16:44 -0700
From:   Guenter Roeck <linux@roeck-us.net>
To:     Wenkai <advantech.susiteam@gmail.com>
Cc:     wenkai.chung@advantech.com.tw, Susi.Driver@advantech.com,
        Wim Van Sebroeck <wim@linux-watchdog.org>,
        linux-kernel@vger.kernel.org, linux-watchdog@vger.kernel.org
Subject: Re: [PATCH 0/5] watchdog: eiois200_wdt: Add EIO-IS200 Watchdog Driver
Message-ID: <ce810ce8-f93c-4a9c-9d14-1e8f8f8c3e2b@roeck-us.net>
References: <cover.1696495372.git.advantech.susiteam@gmail.com>
 <d7df3c7b-730a-4d09-8f15-3cc8591c8092@roeck-us.net>
 <b08d6cf6-cd48-86d7-a959-290fc4de092c@gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <b08d6cf6-cd48-86d7-a959-290fc4de092c@gmail.com>
Precedence: bulk

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@gmail.com wrote:
> > > From: Wenkai <advantech.susiteam@gmail.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