clean up drivers/scsi/pcmcia/nsp_cs

This patch (against 2.6.5) removes some unnecessary macros from drivers/scsi/pcmcia/nsp_cs.c and .h

One Question: Like MIN/MAX and ARRAY_SIZE, it seems that BIT(x) is also
defined a thousand times in the kernel. But which macro should be
used? Here I included linux/input.h, but I do appreciate feedback on which version is the safest.

Best regards
Michael Veeck

diff -Naur linux-2.6.5.org/drivers/scsi/pcmcia/nsp_cs.c linux-2.6.5.new/drivers/scsi/pcmcia/nsp_cs.c
@@ -31,6 +31,7 @@
 #include <linux/module.h>
 #include <linux/kernel.h>
 #include <linux/init.h>
+#include <linux/input.h>
 #include <linux/sched.h>
 #include <linux/slab.h>
 #include <linux/string.h>

@@ -315,7 +316,7 @@
 	int i;
 
 	/* setup sync data */
-	for ( i = 0; i < NUMBER(data->Sync); i++ ) {
+	for (i = 0; i < ARRAY_SIZE(data->Sync); i++) {
 		data->Sync[i] = tmp_sync;
 	}
 }

@@ -600,7 +601,7 @@
 	unsigned int  base = SCpnt->device->host->io_port;
 	nsp_hw_data  *data = (nsp_hw_data *)SCpnt->device->host->hostdata;
 	char	     *buf  = data->MsgBuffer;
-	int	      len  = MIN(MSGBUF_SIZE, data->MsgLen);
+	int	      len  = min(MSGBUF_SIZE, data->MsgLen);
 	int	      ptr;
 	int	      ret;
 

@@ -774,7 +775,7 @@
 			continue;
 		}
 
-		res = MIN(res, SCpnt->SCp.this_residual);
+		res = min(res, SCpnt->SCp.this_residual);
 
 		switch (data->TransferMode) {
 		case MODE_IO32:

@@ -868,7 +869,7 @@
 			continue;
 		}
 
-		res = MIN(SCpnt->SCp.this_residual, WFIFO_CRIT);
+		res = min(SCpnt->SCp.this_residual, WFIFO_CRIT);
 
 		//nsp_dbg(NSP_DEBUG_DATA_IO, "ptr=0x%p this=0x%x res=0x%x", SCpnt->SCp.ptr, SCpnt->SCp.this_residual, res);
 		switch (data->TransferMode) {

@@ -1490,7 +1491,7 @@
 	spin_unlock_irqrestore(&(data->Lock), flags);
 
 	SPRINTF("SDTR status\n");
-	for(id = 0; id < NUMBER(data->Sync); id++) {
+	for (id = 0; id < ARRAY_SIZE(data->Sync); id++) {
 
 		SPRINTF("id %d: ", id);
 

@@ -1534,7 +1535,7 @@
         }
 
 
-	thislength = MIN(thislength, length);
+	thislength = min(thislength, length);
 	*start = buffer + offset;
 
 	return thislength;

diff -Naur linux-2.6.5.org/drivers/scsi/pcmcia/nsp_cs.h linux-2.6.5.new/drivers/scsi/pcmcia/nsp_cs.h
@@ -23,13 +23,6 @@
 #define inline
 */
 
-/************************************
- * Some useful macros...
- */
-#define NUMBER(arr) ((int) (sizeof(arr) / sizeof(arr[0]))) /* from XtNumber() in /usr/X11R6/include/X11/Intrinsic.h */
-#define BIT(x)      (1L << (x))
-#define MIN(a,b)    ((a) > (b) ? (b) : (a))
-
 /* SCSI initiator must be ID 7 */
 #define NSP_INITIATOR_ID  7
 

_______________________________________________
Kernel-janitors mailing list
[email protected]
http://lists.osdl.org/mailman/listinfo/kernel-janitors

On Wed, Apr 07, 2004 at 01:44:00AM +0200, Michael Veeck wrote:

|This patch (against 2.6.5) removes some unnecessary macros from 
|drivers/scsi/pcmcia/nsp_cs.c and .h
|
|One Question: Like MIN/MAX and ARRAY_SIZE, it seems that BIT(x) is also 
| defined a thousand times in the kernel. But which macro should be 
|used? Here I included linux/input.h, but I do appreciate feedback on 
|which version is the safest.

.. the problem occours with the dbg/info/warn macros defined in include/linux/usb.h

there are many other places where macro like that are defined in a different way, why don't we store all these usefull macros to a different file?

[Daniele]

_______________________________________________
Kernel-janitors mailing list
[email protected]
http://lists.osdl.org/mailman/listinfo/kernel-janitors

Well, some (like the ones I mentioned) are already in include/linux/kernel.h. That could be the right place for a safe BIT method I think.

_______________________________________________
Kernel-janitors mailing list
[email protected]
http://lists.osdl.org/mailman/listinfo/kernel-janitors

On Wed, 7 Apr 2004 09:49:11 +0200 Daniele Bellucci wrote:

| On Wed, Apr 07, 2004 at 01:44:00AM +0200, Michael Veeck wrote:
| |This patch (against 2.6.5) removes some unnecessary macros from 
| |drivers/scsi/pcmcia/nsp_cs.c and .h
| |
| |One Question: Like MIN/MAX and ARRAY_SIZE, it seems that BIT(x) is also 
| | defined a thousand times in the kernel. But which macro should be 
| |used? Here I included linux/input.h, but I do appreciate feedback on 
| |which version is the safest.

Having one version of BIT() would make some sense. If there is truly only one version, it should end up in linux/bitops.h. If there is one version per arch, then it should end up in include/asm-*/bitops.h.

Some arch-es use 1ULL to shift, some use 1UL, some use 1. Only input.h does a modulo operator on the shift count AFAICT. I would hope that the '%' isn't needed in general. However, input.h's BIT() is the only one that is non-arch-specific and potentially used (tested) on all arches.

| .. the problem occours with the dbg/info/warn macros defined in include/linux/usb.h
| 
| there are many other places where macro like that are defined in a different way,
| why don't we store all these usefull macros to a different file?

Like we have in include/linux/device.h ? E.g.:

#define dev_err(dev, format, arg...) \
dev_printk(KERN_ERR , dev , format , ## arg)
#define dev_info(dev, format, arg...) \
dev_printk(KERN_INFO , dev , format , ## arg)
#define dev_warn(dev, format, arg...) \
dev_printk(KERN_WARNING , dev , format , ## arg)

However, these require a struct device <dev>.

--
~Randy
"We have met the enemy and he is us." -- Pogo (by Walt Kelly) (Again. Sometimes I think ln -s /usr/src/linux/.config .signature)

_______________________________________________
Kernel-janitors mailing list
[email protected]
http://lists.osdl.org/mailman/listinfo/kernel-janitors