On 229, 08 17, 2005 at 01:14:15PM -0700, Andrew Morton wrote:
> Andrey Panin <[EMAIL PROTECTED]> wrote:
> >
> > 
> > This patch adds driver for IBM Automatic Server Restart watchdog hardware
> > found in some IBM eServer xSeries machines. This driver is based on the ugly
> > driver provided by IBM. Driver was tested on IBM eServer 226.
> > 
> > ...
> > +
> > +   case ASMTYPE_JASPER:
> > +           type = "Jaspers ";
> > +
> > +           /* FIXME: need to use pci_config_lock here, but it's not 
> > exported */
> 
> That's gregkh material.
> 
> > +
> > +/*         spin_lock_irqsave(&pci_config_lock, flags);*/
> > +
> > +           /* Select the SuperIO chip in the PCI I/O port register */
> > +           outl(0x8000f858, 0xcf8);
> > +
> > +           /* 
> > +            * Read the base address for the SuperIO chip.
> > +            * Only the lower 16 bits are valid, but the address is word 
> > +            * aligned so the last bit must be masked off.
> > +            */
> > +           asr_base = inl(0xcfc) & 0xfffe;
> > +
> > +/*         spin_unlock_irqrestore(&pci_config_lock, flags);*/
> >
> > ...
> >
> > +static int asr_ioctl(struct inode *inode, struct file *file,
> > +                unsigned int cmd, unsigned long arg)
> > +{
> > +   static const struct watchdog_info ident = {
> > +           .options =      WDIOF_KEEPALIVEPING | WDIOF_SETTIMEOUT |
> > +                           WDIOF_MAGICCLOSE,
> > +           .identity =     "IBM ASR"
> > +   };
> > +   void __user *argp = (void __user *)arg;
> > +   int __user *p = argp;
> > +   int heartbeat;
> > +
> > +   switch (cmd) {
> > +           case WDIOC_GETSUPPORT:
> > +                   return copy_to_user(argp, &ident, sizeof(ident)) ? 
> > +                           -EFAULT : 0;
> > +
> > +           case WDIOC_GETSTATUS:
> > +           case WDIOC_GETBOOTSTATUS:
> > +                   return put_user(0, p);
> > +
> > +           case WDIOC_KEEPALIVE:
> > +                   asr_toggle();
> > +                   return 0;
> > +
> > +
> > +           case WDIOC_SETTIMEOUT:
> > +                   if (get_user(heartbeat, p))
> > +                           return -EFAULT;
> > +                   /* Fall */
> > +
> > +           case WDIOC_GETTIMEOUT:
> > +                   heartbeat = 256;
> > +                   return put_user(heartbeat, p);
> 
> Something very wrong is happening with WDIOC_SETTIMEOUT and
> WDIOC_GETTIMEOUT.  They're both no-ops and the effect of WDIOC_SETTIMEOUT
> is immidiately overwritten.

Hardware has fixed timeout value, so WDIOC_SETTIMEOUT is noop and 
WDIOC_GETTIMEOUT
always returns 256.

-- 
Andrey Panin            | Linux and UNIX system administrator
[EMAIL PROTECTED]               | PGP key: wwwkeys.pgp.net

Attachment: signature.asc
Description: Digital signature

Reply via email to