Date: Sat, 14 Jan 2006 21:08:39 +0100
From: Willy Tarreau <willy@w.ods.org>
To: Alan Cox <alan@lxorguk.ukuu.org.uk>
Cc: "Calin A. Culianu" <calin@ajvar.org>, linux-kernel@vger.kernel.org,
       akpm@osdl.org
Subject: Re: [PATCH] Watchdog: Winsystems EPX-C3 SBC
Message-ID: <20060114200839.GA14036@w.ods.org>
References: <Pine.LNX.4.64.0601132149430.9231@rtlab.med.cornell.edu> <1137266649.23269.2.camel@localhost.localdomain>
Mime-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <1137266649.23269.2.camel@localhost.localdomain>
User-Agent: Mutt/1.5.10i
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 1274
Lines: 38

On Sat, Jan 14, 2006 at 07:24:09PM +0000, Alan Cox wrote:
> Some quick comments:
> 
> +       if (len) {
> +               epx_c3_pet();
> +       }
> 
> Doesn't need brackets (minor style)
> 
> Otherwise it looks excellent but should use request_region and friends
> to claim the two ports it uses.

You just remind me that I had to comment out the request_region in another
watchdog driver I wrote, because the hardware was mapped on I/O port 0xF2
which is within the FPU I/O space :

   00f0-00ff : fpu

I did not know how to correctly fix this problem, and I could live without
the check, but I found it dirty and never proposed it for inclusion.

It's not the first time I notice that write-only hardware share a reserved
I/O range with other components, and I don't know how to cope with this.
Perhaps it sounds logical not to request_region() as the hardware is meant
to be 100% transparent afterall ?

> The see the "Sign your work:" bit of Documentation/SubmittingPatches are
> it looks ready to go in.
> 
> Alan

Willy

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/