lkml.org 
[lkml]   [2002]   [Feb]   [21]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [DRIVER][RFC] SC1200 Watchdog driver
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



-
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/

\
 
 \ /
  Last update: 2005-03-22 13:24    [W:0.071 / U:9.136 seconds]
©2003-2018 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site