Re: [PATCH 5/5] 2.6.13-rc5-mm1, driver for IBM Automatic Server Restart watchdog

[Andrey Panin]

On 229, 08 17, 2005 at 01:14:15PM -0700, Andrew Morton wrote:
> Andrey Panin <pazke@xxxxxxxxx> 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
pazke@xxxxxxxxx		| PGP key: wwwkeys.pgp.net

Attachment: signature.asc
Description: Digital signature