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

From: Guenter Roeck
Date: Fri Oct 06 2023 - 10:16:52 EST


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