Re: [PATCH v2 0/8] watchdog: w83627hf: Convert to watchdog infrastructure

From: Guenter Roeck <linux@roeck-us.net>
To: Wim Van Sebroeck <wim@iguana.be>
Cc: linux-watchdog@vger.kernel.org, linux-kernel@vger.kernel.org,
	"Samuel Ortiz" <sameo@linux.intel.com>,
	"Pádraig Brady" <P@draigBrady.com>
Subject: Re: [PATCH v2 0/8] watchdog: w83627hf: Convert to watchdog infrastructure
Date: Fri, 22 Mar 2013 17:28:10 -0700	[thread overview]
Message-ID: <20130323002810.GA26245@roeck-us.net> (raw)
In-Reply-To: <20130322205258.GE7867@spo001.leaseweb.com>

On Fri, Mar 22, 2013 at 09:52:58PM +0100, Wim Van Sebroeck wrote:
> Hi Guenter,
> 
> > Convert to watchdog infrastructure, cleanup, add support for additional
> > chips, and merge with W83697HF and W83697UG watchdog drivers.
> > 
> > Tested with W83627UHG, NCT6775, NCT6776. Additional test feedback
> > for other chips would be appreciated.
> > 
> > Original idea was to prepare the driver for use as a client to an MFD driver,
> > but I found that requesting memory with request_muxed_region works just as well
> > and has less impact. v2 includes the knowledge gained from converting the
> > driver to an MFD client driver, but without the actual conversion.
> > 
> > v2: Tested with W83627UHG
> >     Retain timeout module parameter; use watchdog_init_timeout to set it
> >     Eliminate some cosmetic changes
> >     Drop spinlock.h include
> >     Keep "initialized" message
> >     Don't try to configure WDTO for W83627UHG and W83627EHF
> >     Don't report the nowayout option with the 'initializing' message
> >     Use request_muxed_region to reserve memory range only while needed
> >     Add support for W83627S, W83627DHG-P, W83667HG, and NCT6779
> 
> In 2011 I started something similar but then with the MFD approach in mind.
> Goal was also to clean-up the w836* watchdog drivers and get a clean driver that
> supports all Winbond super-I/O based watchdog drivers.
> 
> I dug op the development code again. I'll post it in a next e-mail so that we can
> see what the best way forward is. Note: I took the MFD approach because:
> 1) all superio shares the similar functions for using the Super-I/O registers.
> 2) Goal is to have low-level driver that support the specific super-I/O chipsets
> and that does the platform stuff for hwmon, watchdog, gpio, ...
> 
Hi Wim,

I started with a similar approach, only I used mfd cells to pass on platform
specific information such as the device type and the superio base address.
I still have the patchset for the mfd driver, in case you are interested.
My code is based on the patches submitted by Rodolfo Giometti a couple
of years ago. Want me to post it ?

What I noticed in my testing is that the superio address range (2e or 4e),
which the drivers currently take as granted, is at least on my systems
(all three of them) reserved by ACPI. Unfortunately that means one can not
use the mfd infrastructure to pass on the superio memory region,
since it checks for acpi conflicts. With that I gave up on the idea and
reverted to using request_muxed_region. That seemed simpler and accomplish
the same as long as all drivers actually use it.

Ultimately, I am open to either approach.

Thanks,
Guenter