Re: [DRIVER][RFC] SC1200 Watchdog driver

Roy Sigurd Karlsbakk
2002-02-24T17:31:05Z

Will this driver shut off the watchdog when /dev/watchdog is closed, or
does it require an explicit shutdown message like Jakob Oestergaard's
sbc60xxwdt driver?

On Thu, 21 Feb 2002, Zwane Mwaikambo wrote:

> On Thu, 21 Feb 2002, Jeff Garzik wrote:
> > > #include <asm/system.h>
> > > #include <linux/notifier.h>
> > > #include <linux/reboot.h>
> > > #include <linux/init.h>
> >
> > try deleting all includes and rebuild this list from scratch... I'll bet
> > it can be made smaller.
>
> Will do.
>
> > > static int sc1200wdt_release(struct inode *inode, struct file *file)
> > > {
> > > lock_kernel();
> > >
> > > /* Disable it on the way out */
> > > sc1200wdt_write_data(WDTO, 0);
> > > up(&open_sem);
> > >
> > > unlock_kernel();
> > >
> > > printk(KERN_INFO PFX "Watchdog disabled\n");
> > > MOD_DEC_USE_COUNT;
> > >
> > > return 0;
> > > }
> >
> > are you certain we need lock_kernel(), unlock_kernel() here?
> > especially with a semaphore...
>
> I'll remove the lock/unlock_kernel from there and shuffle the semaphore
> around.
>
> > > static struct file_operations sc1200wdt_fops =
> > > {
> > > owner: THIS_MODULE,
> > > write: sc1200wdt_write,
> > > ioctl: sc1200wdt_ioctl,
> > > open: sc1200wdt_open,
> > > release: sc1200wdt_release,
> > > };
> >
> > I noticed wdt_pci.c implements ->read, too, why not here as well?
>
> Hmm i see wdt_pci uses its read call for getting temperature status, the
> only thing i can report back is the status of the watchdog, and that i
> currently send back via an ioctl call (WDIOC_GETSTATUS). The chip i have a
> datasheet for doesn't have temperature reporting via watchdog, but there
> are bits (supported by lmsensors) which can do that.
>
> > Look at how i810_rng does its PCI probe. [I'm guessing] Surely this
> > SC1200 hardware has _some_ sort of identifier, like a list of commonly
> > found PCI host bridges, that is better than the simple request_region()
> > provided.
>
> Its an ISAPNP device so we can probe like that (logical device 8), this
> particular module doesn't have PnP support (i was gonna add it later), i
> was wondering wether there was a possible non PnP probe we could do.
>
> > Overall, looks good... nice, clean driver.
>
> Thanks, but i think i wasted my time on this one, there is a driver for
> most of the SC1200 bits (including watchdog) at http://www.nano-system.com/scx200
>
> Cheers,
> Zwane Mwaikambo
>
>
>

--
Roy Sigurd Karlsbakk, Datavaktmester

Computers are like air conditioners.
They stop working when you open Windows.