[Kernel-janitors] [PATCH] clean up drivers/scsi/pcmcia/nsp_cs

[Kernel-janitors] [PATCH] clean up drivers/scsi/pcmcia/nsp_cs

[Michael Veeck]

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

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

[-- Attachment #2: minmax_drivers_scsi_pcmcia.patch --]
[-- Type: text/plain, Size: 2498 bytes --]

diff -Naur linux-2.6.5.org/drivers/scsi/pcmcia/nsp_cs.c linux-2.6.5.new/drivers/scsi/pcmcia/nsp_cs.c
--- linux-2.6.5.org/drivers/scsi/pcmcia/nsp_cs.c	2004-04-04 05:38:22.000000000 +0200
+++ linux-2.6.5.new/drivers/scsi/pcmcia/nsp_cs.c	2004-04-07 00:23:21.901487096 +0200
@@ -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
--- linux-2.6.5.org/drivers/scsi/pcmcia/nsp_cs.h	2004-04-04 05:36:57.000000000 +0200
+++ linux-2.6.5.new/drivers/scsi/pcmcia/nsp_cs.h	2004-04-07 00:22:47.050785208 +0200
@@ -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
 

[-- Attachment #3: Type: text/plain, Size: 167 bytes --]

_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
http://lists.osdl.org/mailman/listinfo/kernel-janitors

Re: [Kernel-janitors] [PATCH] clean up drivers/scsi/pcmcia/nsp_cs

[Daniele Bellucci]

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

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]

[-- Attachment #2: Type: text/plain, Size: 167 bytes --]

_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
http://lists.osdl.org/mailman/listinfo/kernel-janitors

Re: [Kernel-janitors] [PATCH] clean up drivers/scsi/pcmcia/nsp_cs

[Michael Veeck]

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


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.
> 
> .. 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?
> 

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.

[-- Attachment #2: Type: text/plain, Size: 167 bytes --]

_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
http://lists.osdl.org/mailman/listinfo/kernel-janitors

Re: [Kernel-janitors] [PATCH] clean up drivers/scsi/pcmcia/nsp_cs

[Randy.Dunlap]

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

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)

[-- Attachment #2: Type: text/plain, Size: 167 bytes --]

_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
http://lists.osdl.org/mailman/listinfo/kernel-janitors