Re: [PATCH 1/3] watchdog: sync linux-omap changes

[Russell King - ARM Linux]

On Mon, Sep 22, 2008 at 08:22:06PM +0200, Wim Van Sebroeck wrote:
> Hi All,
> 
> last item that (for me) is still open is:
> > +struct omap_wdt_dev {
> > +	void __iomem    *base;          /* physical */
> > +	struct device   *dev;
> 
> is dev indeed unused or is it used by platform specific code?
> If this is sorted out this will go into the watchdog-mm tree.

It's unused, but it's a minor point, something that shouldn't stand
in the way of it going into the watchdog tree.  It doesn't cause a
build error and doesn't cause malfunction.  It's just a little untidy
and can be addressed separately.

However, if you want Filipe to redo the patch yet again (risking him
getting pissed off with the number of times round the loop it's taking
for what should be a simple driver) there's also this:

@@ -219,12 +240,12 @@ static long omap_wdt_ioctl(struct file *file, unsigned int+cmd,
                omap_wdt_adjust_timeout(new_margin);

                spin_lock(&wdt_lock);
-               omap_wdt_disable();
-               omap_wdt_set_timeout();
-               omap_wdt_enable();
-
-               omap_wdt_ping();
+               omap_wdt_disable(wdev);
+               omap_wdt_set_timeout(wdev);
+               omap_wdt_enable(wdev);
                spin_unlock(&wdt_lock);
+
+               omap_wdt_ping(wdev);

which is moving omap_wdt_ping() outside of the spin lock, which I
doubt actually causes any problem in real life on OMAP platforms.
Granted that theoretically and logically it's wrong.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/