[PATCH 2.6 ] MPT Fusion driver 3.01.02 update

[PATCH 2.6 ] MPT Fusion driver 3.01.02 update

[Moore, Eric Dean]

[-- Attachment #1: Type: text/plain, Size: 762 bytes --]

This is an update for the MPT Fusion drivers 2.6 kernel.
Version 3.01.02.

Changelog:

(1) Andi Kleen[ak@suse.de]
put warning "Device (0:0:0) reported QUEUE_FULL!" into debug messages

(2) Alexander Stohr[Alexander.Stohr@gmx.de]
fix warnings from mptscsih_setup when driver isn't compiled as module

(3) Randy.Dunlap[rddunlap@osdl.org]
Remove unnecessary min/max macros and change calls to 
use kernel.h macros instead.


You can download full source as well as the patches for this 
driver at:
ftp://ftp.lsil.com/HostAdapterDrivers/linux/FiberChannel/2.6-patches/3.01.02


Eric Moore
Staff Engineer
Standard Storage Products Division
LSI Logic Corporation
4420 Arrowswest Drive
Colorado Springs, CO 80907
Email: emoore@lsil.com
Web: http://www.lsilogic.com/




[-- Attachment #2: mptlinux-3.01.02.patch --]
[-- Type: application/octet-stream, Size: 9360 bytes --]

diff -uarN b/drivers/message/fusion/mptbase.c a/drivers/message/fusion/mptbase.c
--- b/drivers/message/fusion/mptbase.c	2004-03-17 17:11:04.000000000 -0700
+++ a/drivers/message/fusion/mptbase.c	2004-03-17 12:18:39.000000000 -0700
@@ -2567,10 +2567,10 @@
 			 * Set values for this IOC's request & reply frame sizes,
 			 * and request & reply queue depths...
 			 */
-			ioc->req_sz = MIN(MPT_DEFAULT_FRAME_SIZE, facts->RequestFrameSize * 4);
-			ioc->req_depth = MIN(MPT_MAX_REQ_DEPTH, facts->GlobalCredits);
+			ioc->req_sz = min(MPT_DEFAULT_FRAME_SIZE, facts->RequestFrameSize * 4);
+			ioc->req_depth = min_t(int, MPT_MAX_REQ_DEPTH, facts->GlobalCredits);
 			ioc->reply_sz = MPT_REPLY_FRAME_SIZE;
-			ioc->reply_depth = MIN(MPT_DEFAULT_REPLY_DEPTH, facts->ReplyQueueDepth);
+			ioc->reply_depth = min_t(int, MPT_DEFAULT_REPLY_DEPTH, facts->ReplyQueueDepth);
 
 			dprintk((MYIOC_s_INFO_FMT "reply_sz=%3d, reply_depth=%4d\n",
 				ioc->name, ioc->reply_sz, ioc->reply_depth));
@@ -4065,7 +4065,7 @@
 		/*
 		 * Copy out the cached reply...
 		 */
-		for (ii=0; ii < MIN(replyBytes/2,mptReply->MsgLength*2); ii++)
+		for (ii=0; ii < min(replyBytes/2,mptReply->MsgLength*2); ii++)
 			u16reply[ii] = ioc->hs_reply[ii];
 	} else {
 		return -99;
@@ -4312,7 +4312,7 @@
 
 			if ((rc = mpt_config(ioc, &cfg)) == 0) {
 				/* save the data */
-				copy_sz = MIN(sizeof(LANPage0_t), data_sz);
+				copy_sz = min_t(int, sizeof(LANPage0_t), data_sz);
 				memcpy(&ioc->lan_cnfg_page0, ppage0_alloc, copy_sz);
 
 			}
@@ -4357,7 +4357,7 @@
 
 		if ((rc = mpt_config(ioc, &cfg)) == 0) {
 			/* save the data */
-			copy_sz = MIN(sizeof(LANPage1_t), data_sz);
+			copy_sz = min_t(int, sizeof(LANPage1_t), data_sz);
 			memcpy(&ioc->lan_cnfg_page1, ppage1_alloc, copy_sz);
 		}
 
@@ -4426,7 +4426,7 @@
 		if ((rc = mpt_config(ioc, &cfg)) == 0) {
 			/* save the data */
 			pp0dest = &ioc->fc_port_page0[portnum];
-			copy_sz = MIN(sizeof(FCPortPage0_t), data_sz);
+			copy_sz = min_t(int, sizeof(FCPortPage0_t), data_sz);
 			memcpy(pp0dest, ppage0_alloc, copy_sz);
 
 			/*
diff -uarN b/drivers/message/fusion/mptbase.h a/drivers/message/fusion/mptbase.h
--- b/drivers/message/fusion/mptbase.h	2004-03-17 17:11:04.000000000 -0700
+++ a/drivers/message/fusion/mptbase.h	2004-03-17 12:18:39.000000000 -0700
@@ -81,8 +81,8 @@
 #define COPYRIGHT	"Copyright (c) 1999-2004 " MODULEAUTHOR
 #endif
 
-#define MPT_LINUX_VERSION_COMMON	"3.01.01"
-#define MPT_LINUX_PACKAGE_NAME		"@(#)mptlinux-3.01.01"
+#define MPT_LINUX_VERSION_COMMON	"3.01.02"
+#define MPT_LINUX_PACKAGE_NAME		"@(#)mptlinux-3.01.02"
 #define WHAT_MAGIC_STRING		"@" "(" "#" ")"
 
 #define show_mptmod_ver(s,ver)  \
@@ -1061,13 +1061,6 @@
 /*
  *  More (public) macros...
  */
-#ifndef MIN
-#define MIN(a, b)   (((a) < (b)) ? (a) : (b))
-#endif
-#ifndef MAX
-#define MAX(a, b)   (((a) > (b)) ? (a) : (b))
-#endif
-
 #ifndef offsetof
 #define offsetof(t, m)	((size_t) (&((t *)0)->m))
 #endif
diff -uarN b/drivers/message/fusion/mptctl.c a/drivers/message/fusion/mptctl.c
--- b/drivers/message/fusion/mptctl.c	2004-03-17 17:11:04.000000000 -0700
+++ a/drivers/message/fusion/mptctl.c	2004-03-17 12:18:39.000000000 -0700
@@ -285,7 +285,7 @@
 				dctlprintk((MYIOC_s_INFO_FMT ": Copying Reply Frame @%p to IOC!\n",
 						ioc->name, reply));
 				memcpy(ioc->ioctl->ReplyFrame, reply,
-					MIN(ioc->reply_sz, 4*reply->u.reply.MsgLength));
+					min(ioc->reply_sz, 4*reply->u.reply.MsgLength));
 				ioc->ioctl->status |= MPT_IOCTL_STATUS_RF_VALID;
 
 				/* Set the command status to GOOD if IOC Status is GOOD
@@ -336,7 +336,7 @@
 			// NOTE: Expects/requires non-Turbo reply!
 			dctlprintk((MYIOC_s_INFO_FMT ":Caching MPI_FUNCTION_FW_DOWNLOAD reply!\n",
 				ioc->name));
-			memcpy(fwReplyBuffer, reply, MIN(sizeof(fwReplyBuffer), 4*reply->u.reply.MsgLength));
+			memcpy(fwReplyBuffer, reply, min_t(int, sizeof(fwReplyBuffer), 4*reply->u.reply.MsgLength));
 			ReplyMsg = (pMPIDefaultReply_t) fwReplyBuffer;
 		}
 	}
@@ -991,7 +991,7 @@
 	MptSge_t	*sgl;
 	int		 numfrags = 0;
 	int		 fragcnt = 0;
-	int		 alloc_sz = MIN(bytes,MAX_KMALLOC_SZ);	// avoid kernel warning msg!
+	int		 alloc_sz = min(bytes,MAX_KMALLOC_SZ);	// avoid kernel warning msg!
 	int		 bytes_allocd = 0;
 	int		 this_alloc;
 	dma_addr_t	 pa;					// phys addr
@@ -1036,7 +1036,7 @@
 	sgl = sglbuf;
 	sg_spill = ((ioc->req_sz - sge_offset)/(sizeof(dma_addr_t) + sizeof(u32))) - 1;
 	while (bytes_allocd < bytes) {
-		this_alloc = MIN(alloc_sz, bytes-bytes_allocd);
+		this_alloc = min(alloc_sz, bytes-bytes_allocd);
 		buflist[buflist_ent].len = this_alloc;
 		buflist[buflist_ent].kptr = pci_alloc_consistent(ioc->pcidev,
 								 this_alloc,
@@ -2293,9 +2293,9 @@
 		 */
 		if (ioc->ioctl->status & MPT_IOCTL_STATUS_RF_VALID) {
 			if (karg.maxReplyBytes < ioc->reply_sz) {
-				 sz = MIN(karg.maxReplyBytes, 4*ioc->ioctl->ReplyFrame[2]);
+				 sz = min(karg.maxReplyBytes, 4*ioc->ioctl->ReplyFrame[2]);
 			} else {
-				 sz = MIN(ioc->reply_sz, 4*ioc->ioctl->ReplyFrame[2]);
+				 sz = min(ioc->reply_sz, 4*ioc->ioctl->ReplyFrame[2]);
 			}
 
 			if (sz > 0) {
@@ -2314,7 +2314,7 @@
 		/* If valid sense data, copy to user.
 		 */
 		if (ioc->ioctl->status & MPT_IOCTL_STATUS_SENSE_VALID) {
-			sz = MIN(karg.maxSenseBytes, MPT_SENSE_BUFFER_SIZE);
+			sz = min(karg.maxSenseBytes, MPT_SENSE_BUFFER_SIZE);
 			if (sz > 0) {
 				if (copy_to_user((char *)karg.senseDataPtr, ioc->ioctl->sense, sz)) {
 					printk(KERN_ERR "%s@%d::mptctl_do_mpt_command - "
diff -uarN b/drivers/message/fusion/mptscsih.c a/drivers/message/fusion/mptscsih.c
--- b/drivers/message/fusion/mptscsih.c	2004-03-17 17:11:04.000000000 -0700
+++ a/drivers/message/fusion/mptscsih.c	2004-03-17 12:18:39.000000000 -0700
@@ -196,8 +196,9 @@
 static void	mptscsih_dv_parms(MPT_SCSI_HOST *hd, DVPARAMETERS *dv,void *pPage);
 static void	mptscsih_fillbuf(char *buffer, int size, int index, int width);
 #endif
+#ifdef MODULE
 static int	mptscsih_setup(char *str);
-
+#endif
 /* module entry point */
 static int  __init   mptscsih_init  (void);
 static void __exit   mptscsih_exit  (void);
@@ -1366,8 +1367,8 @@
 
 		if (sc->device && sc->device->host != NULL && sc->device->host->hostdata != NULL)
 			ioc_str = ((MPT_SCSI_HOST *)sc->device->host->hostdata)->ioc->name;
-		printk(MYIOC_s_WARN_FMT "Device (%d:%d:%d) reported QUEUE_FULL!\n",
-				ioc_str, 0, sc->device->id, sc->device->lun);
+		dprintk((MYIOC_s_WARN_FMT "Device (%d:%d:%d) reported QUEUE_FULL!\n",
+				ioc_str, 0, sc->device->id, sc->device->lun));
 		last_queue_full = time;
 	}
 }
@@ -4519,7 +4520,7 @@
 					if (nfactor < pspi_data->minSyncFactor )
 						nfactor = pspi_data->minSyncFactor;
 
-					factor = MAX (factor, nfactor);
+					factor = max(factor, nfactor);
 					if (factor == MPT_ASYNC)
 						offset = 0;
 				} else {
@@ -4727,7 +4728,7 @@
 		maxid = ioc->sh->max_id - 1;
 	} else if (ioc->sh) {
 		id = target_id;
-		maxid = MIN(id, ioc->sh->max_id - 1);
+		maxid = min_t(int, id, ioc->sh->max_id - 1);
 	}
 
 	for (; id <= maxid; id++) {
@@ -5134,7 +5135,7 @@
 				sense_data = ((u8 *)hd->ioc->sense_buf_pool +
 					(req_idx * MPT_SENSE_BUFFER_ALLOC));
 
-				sz = MIN (pReq->SenseBufferLength,
+				sz = min_t(int, pReq->SenseBufferLength,
 							SCSI_STD_SENSE_BYTES);
 				memcpy(hd->pLocal->sense, sense_data, sz);
 
@@ -5786,7 +5787,7 @@
 				ioc->spi_data.forceDv &= ~MPT_SCSICFG_RELOAD_IOC_PG3;
 			}
 
-			maxid = MIN (ioc->sh->max_id, MPT_MAX_SCSI_DEVICES);
+			maxid = min_t(int, ioc->sh->max_id, MPT_MAX_SCSI_DEVICES);
 
 			for (id = 0; id < maxid; id++) {
 				spin_lock_irqsave(&dvtaskQ_lock, flags);
@@ -6509,7 +6510,7 @@
 	if (echoBufSize > 0) {
 		iocmd.flags |= MPT_ICFLAG_ECHO;
 		if (dataBufSize > 0)
-			bufsize = MIN(echoBufSize, dataBufSize);
+			bufsize = min(echoBufSize, dataBufSize);
 		else
 			bufsize = echoBufSize;
 	} else if (dataBufSize == 0)
@@ -6520,7 +6521,7 @@
 
 	/* Data buffers for write-read-compare test max 1K.
 	 */
-	sz = MIN(bufsize, 1024);
+	sz = min(bufsize, 1024);
 
 	/* --- loop ----
 	 * On first pass, always issue a reserve.
@@ -6875,9 +6876,9 @@
 		}
 
 		/* limit by adapter capabilities */
-		width = MIN(width, hd->ioc->spi_data.maxBusWidth);
-		offset = MIN(offset, hd->ioc->spi_data.maxSyncOffset);
-		factor = MAX(factor, hd->ioc->spi_data.minSyncFactor);
+		width = min(width, hd->ioc->spi_data.maxBusWidth);
+		offset = min(offset, hd->ioc->spi_data.maxSyncOffset);
+		factor = max(factor, hd->ioc->spi_data.minSyncFactor);
 
 		/* Check Consistency */
 		if (offset && (factor < MPT_ULTRA2) && !width)
@@ -7217,19 +7218,22 @@
 #define	ARG_SEP	','
 #endif
 
+#ifdef MODULE
 static char setup_token[] __initdata =
 	"dv:"
 	"width:"
 	"factor:"
 	"saf-te:"
        ;	/* DO NOT REMOVE THIS ';' */
-
+#endif
+       
 #define OPT_DV			1
 #define OPT_MAX_WIDTH		2
 #define OPT_MIN_SYNC_FACTOR	3
 #define OPT_SAF_TE		4
 
 /*=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=*/
+#ifdef MODULE
 static int
 get_setup_token(char *p)
 {
@@ -7298,7 +7302,7 @@
 	}
 	return 1;
 }
-
+#endif
 /*=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=*/
 
 

Re: [PATCH 2.6 ] MPT Fusion driver 3.01.02 update

[Jeff Garzik]

Moore, Eric Dean wrote:
> This is an update for the MPT Fusion drivers 2.6 kernel.
> Version 3.01.02.
> 
> Changelog:
> 
> (1) Andi Kleen[ak@suse.de]
> put warning "Device (0:0:0) reported QUEUE_FULL!" into debug messages
> 
> (2) Alexander Stohr[Alexander.Stohr@gmx.de]
> fix warnings from mptscsih_setup when driver isn't compiled as module
> 
> (3) Randy.Dunlap[rddunlap@osdl.org]
> Remove unnecessary min/max macros and change calls to 
> use kernel.h macros instead.

Here are some wrappers to remove:

static inline void CHIPREG_PIO_WRITE32(volatile u32 *a, u32 v)
{
         outl(v, (unsigned long)a);
}

static inline u32 CHIPREG_PIO_READ32(volatile u32 *a)
{
         return inl((unsigned long)a);
}


Also, I'm not sure about these wrappers, but it's mainly as style issue 
due to the "if (PortIo)" test.  You should remove the 'volatile' 
markers, at least.


static inline u32 CHIPREG_READ32(volatile u32 *a)
{
         if (PortIo)
                 return inl((unsigned long)a);
         else
                 return readl(a);
}

static inline void CHIPREG_WRITE32(volatile u32 *a, u32 v)
{
         if (PortIo)
                 outl(v, (unsigned long)a);
         else
                 writel(v, a);
}

Re: [PATCH 2.6 ] MPT Fusion driver 3.01.02 update

[Andi Kleen]

On Wed, 17 Mar 2004 14:43:21 -0500
"Moore, Eric Dean" <Emoore@lsil.com> wrote:

> This is an update for the MPT Fusion drivers 2.6 kernel.
> Version 3.01.02.
> 
This doesn't seem to fix the bug I'm running into all the time:

you set a 64bit consistent dma mask, but assume in PrimeIocFifos 
that two calls to pci_alloc_consistent() end in the same 4GB 
segment, which cannot work.

As a workaround i just reverted it to a 4GB consistent mask for now.

-Andi

Re: [PATCH 2.6 ] MPT Fusion driver 3.01.02 update

[Moore, Eric Dean]

Andi -

The fix in 3.01.02 was your request for commenting out the
"queue is full message" which you posted to the
mailing list earlier in the last week.  This has nothing to do with
with todays issue.

Regarding todays issue.  There is a work around for your
issue which I have already suggested earlier today. That is
to modify the mpt fusion Makefile, by uncommenting out
the FIFO FIX define.  This work around will set a dma mask
for less 4GB, thus we don't hit the error condition in PrimeFifo's
when the two memory allocation are on different 4gb boundaries, and
the failure displays "-3" error code as yours did in the original dmesg.

I have provided this temp fix to HP a few weeks ago.
I plan to start work soon to solve this issue by combining these three
FIFOs into one memory allocation( the third FIFO allocation is
in mptscsih.c, that is the chain buffers).

Eric Moore




----- Original Message ----- 
From: "Andi Kleen" <ak@suse.de>
To: "Moore, Eric Dean" <Emoore@lsil.com>
Cc: <linux-scsi@vger.kernel.org>; <James.Bottomley@steeleye.com>;
<Alexander.Stohr@gmx.de>; <rddunlap@osdl.org>
Sent: Wednesday, March 17, 2004 6:09 PM
Subject: Re: [PATCH 2.6 ] MPT Fusion driver 3.01.02 update


> On Wed, 17 Mar 2004 14:43:21 -0500
> "Moore, Eric Dean" <Emoore@lsil.com> wrote:
>
> > This is an update for the MPT Fusion drivers 2.6 kernel.
> > Version 3.01.02.
> >
> This doesn't seem to fix the bug I'm running into all the time:
>
> you set a 64bit consistent dma mask, but assume in PrimeIocFifos
> that two calls to pci_alloc_consistent() end in the same 4GB
> segment, which cannot work.
>
> As a workaround i just reverted it to a 4GB consistent mask for now.
>
> -Andi
>
>
>

Re: [PATCH 2.6 ] MPT Fusion driver 3.01.02 update

[Andi Kleen]

On Wed, 17 Mar 2004 18:38:51 -0700
"Moore, Eric Dean" <emoore@lsil.com> wrote:

> Andi -
> 
> The fix in 3.01.02 was your request for commenting out the
> "queue is full message" which you posted to the
> mailing list earlier in the last week.  This has nothing to do with
> with todays issue.

I merged that. Thanks.
 
> Regarding todays issue.  There is a work around for your
> issue which I have already suggested earlier today. That is
> to modify the mpt fusion Makefile, by uncommenting out
> the FIFO FIX define.  This work around will set a dma mask
> for less 4GB, thus we don't hit the error condition in PrimeFifo's

Ok, I did that already independently. 

Thanks,
-Andi

RE: [PATCH 2.6 ] MPT Fusion driver 3.01.02 update

[Moore, Eric Dean]

The wrappers are there because the system
interface registers could be memory or IO mapped
registers depending on the pci device.  The PIO 
wrappers could be removed, but I think its there 
to be uniform with the other wrappers.  The PIO 
wrappers there for firmware download which 
always using IO registers. Id would rather not
remove the volatile since the registers could
be memory mapped, and may break things when
driver compiled with optimized kernel.

Eric


On Wednesday, March 17, 2004 1:21 PM, Jeff Garzik wrote:

> 
> 
> Moore, Eric Dean wrote:
> > This is an update for the MPT Fusion drivers 2.6 kernel.
> > Version 3.01.02.
> > 
> > Changelog:
> > 
> > (1) Andi Kleen[ak@suse.de]
> > put warning "Device (0:0:0) reported QUEUE_FULL!" into 
> debug messages
> > 
> > (2) Alexander Stohr[Alexander.Stohr@gmx.de]
> > fix warnings from mptscsih_setup when driver isn't compiled 
> as module
> > 
> > (3) Randy.Dunlap[rddunlap@osdl.org]
> > Remove unnecessary min/max macros and change calls to 
> > use kernel.h macros instead.
> 
> Here are some wrappers to remove:
> 
> static inline void CHIPREG_PIO_WRITE32(volatile u32 *a, u32 v)
> {
>          outl(v, (unsigned long)a);
> }
> 
> static inline u32 CHIPREG_PIO_READ32(volatile u32 *a)
> {
>          return inl((unsigned long)a);
> }
> 
> 
> Also, I'm not sure about these wrappers, but it's mainly as 
> style issue 
> due to the "if (PortIo)" test.  You should remove the 'volatile' 
> markers, at least.
> 
> 
> static inline u32 CHIPREG_READ32(volatile u32 *a)
> {
>          if (PortIo)
>                  return inl((unsigned long)a);
>          else
>                  return readl(a);
> }
> 
> static inline void CHIPREG_WRITE32(volatile u32 *a, u32 v)
> {
>          if (PortIo)
>                  outl(v, (unsigned long)a);
>          else
>                  writel(v, a);
> }
> 
> 

Re: [PATCH 2.6 ] MPT Fusion driver 3.01.02 update

[Jeff Garzik]

Moore, Eric Dean wrote:
> The wrappers are there because the system
> interface registers could be memory or IO mapped
> registers depending on the pci device.  The PIO 

Yes, I understand what the wrappers are doing :)  That's why I said it 
was mainly a style issue I was bringing up.  Depending on the hardware 
requirements and installed userbase, it may be better to make PortIO a 
compile-time option.  In general MMIO is preferred.


> wrappers could be removed, but I think its there 
> to be uniform with the other wrappers.  The PIO 
> wrappers there for firmware download which 

If it's directly wrapping a Linux API call, then the Linux API call 
should be used directly in the code.  Wrappers in general inhibit 
compiler optimization opportunities, and occasionally hide bugs.  They 
hinder long term maintainance, and slow down code reviews.

With every vendor-written driver inventing its own wrappers, the problem 
gets unscalable real quick.


> always using IO registers. Id would rather not
> remove the volatile since the registers could
> be memory mapped, and may break things when
> driver compiled with optimized kernel.

No, all that is needed is included in the read[bwl] and write[bwl] 
definitions.  'volatile' occasionally hides bugs, and good code is 
completely volatile-free:  always explicitly flush MMIO writes, and 
employ memory barriers, rather than depending on the compiler to do so.

The compiler does not have 100% of the knowledge necessary -- 
particularly for MMIO writes -- to presume whether a flush is needed or 
not.  Portable code _must_ follow a writel() call with the readl(), to 
guarantee that all PCI- and CPU-posted/delayed writes are flushed 
completely to the PCI device.

	Jeff

RE: [PATCH 2.6 ] MPT Fusion driver 3.01.02 update

[Moore, Eric Dean]

agreed. I will use a better method, instead
of wrappers. I will start work on that.

FC909 is IO-mapped only, and all the other cards
support both MMIO and IO.  The FC909 is much older
card, and it sounds like nobody is using it (just
confirmed by my manager). 

Therefore - I can remove support for this card, which
means the PortIO module parameter can be removed.

Eric


On Wednesday, March 17, 2004 3:15 PM, Jeff Garzik wrote:
> 
> 
> Moore, Eric Dean wrote:
> > The wrappers are there because the system
> > interface registers could be memory or IO mapped
> > registers depending on the pci device.  The PIO 
> 
> Yes, I understand what the wrappers are doing :)  That's why 
> I said it 
> was mainly a style issue I was bringing up.  Depending on the 
> hardware 
> requirements and installed userbase, it may be better to make 
> PortIO a 
> compile-time option.  In general MMIO is preferred.
> 
> 
> > wrappers could be removed, but I think its there 
> > to be uniform with the other wrappers.  The PIO 
> > wrappers there for firmware download which 
> 
> If it's directly wrapping a Linux API call, then the Linux API call 
> should be used directly in the code.  Wrappers in general inhibit 
> compiler optimization opportunities, and occasionally hide 
> bugs.  They 
> hinder long term maintainance, and slow down code reviews.
> 
> With every vendor-written driver inventing its own wrappers, 
> the problem 
> gets unscalable real quick.
> 
> 
> > always using IO registers. Id would rather not
> > remove the volatile since the registers could
> > be memory mapped, and may break things when
> > driver compiled with optimized kernel.
> 
> No, all that is needed is included in the read[bwl] and write[bwl] 
> definitions.  'volatile' occasionally hides bugs, and good code is 
> completely volatile-free:  always explicitly flush MMIO writes, and 
> employ memory barriers, rather than depending on the compiler 
> to do so.
> 
> The compiler does not have 100% of the knowledge necessary -- 
> particularly for MMIO writes -- to presume whether a flush is 
> needed or 
> not.  Portable code _must_ follow a writel() call with the 
> readl(), to 
> guarantee that all PCI- and CPU-posted/delayed writes are flushed 
> completely to the PCI device.
> 
> 	Jeff
> 
> 
> 
>