ULL fixes for qlogicfc

PATCH In review [email protected] 4 participants 9 messages Sep 29, 2003 SCSI

lore

Share & tools

lore

[email protected]

author

2003-09-29 17:07 UTC #0 lore raw

PATCH 1/1 ULL fixes for qlogicfc

drivers/scsi/qlogicfc.c +2 -2

diff -urpN --exclude-from=/home/davej/.exclude bk-linus/drivers/scsi/qlogicfc.c linux-2.5/drivers/scsi/qlogicfc.c
@@ -718,8 +718,8 @@ int isp2x00_detect(Scsi_Host_Template * 
 				continue;
 
 			/* Try to configure DMA attributes. */
-			if (pci_set_dma_mask(pdev, (u64) 0xffffffffffffffff) &&
-			    pci_set_dma_mask(pdev, (u64) 0xffffffff))
+			if (pci_set_dma_mask(pdev, 0xffffffffffffffffULL) &&
+			    pci_set_dma_mask(pdev, 0xffffffffULL))
 					continue;
 
 		        host = scsi_register(tmpt, sizeof(struct isp2x00_hostdata));

Jeff Garzik

2003-09-29 17:26 UTC #1 lore raw

drivers/scsi/qlogicfc.c int isp2x00_detect(Scsi_Host_Template * · line 723

Quoted 12 lines · On Mon, Sep 29, 2003 at 06:04:34PM +0100, [email protected] wrote:

diff -urpN --exclude-from=/home/davej/.exclude bk-linus/drivers/scsi/qlogicfc.c linux-2.5/drivers/scsi/qlogicfc.c

drivers/scsi/qlogicfc.c +2 -2

@@ -718,8 +718,8 @@ int isp2x00_detect(Scsi_Host_Template * 
 				continue;
 
 			/* Try to configure DMA attributes. */
-			if (pci_set_dma_mask(pdev, (u64) 0xffffffffffffffff) &&
-			    pci_set_dma_mask(pdev, (u64) 0xffffffff))
+			if (pci_set_dma_mask(pdev, 0xffffffffffffffffULL) &&
+			    pci_set_dma_mask(pdev, 0xffffffffULL))
 					continue;

Looks great.

I wonder if you are motivated to create similar pci_set_dma_mask() cleanups for other drivers? ;-) Several other drivers need this same cleanup, too.

Jeff

Dave Jones

author

2003-09-29 17:29 UTC #2 lore raw

Quoted 16 lines · Jeff Garzik

drivers/scsi/qlogicfc.c int isp2x00_detect(Scsi_Host_Template * · line 723
Quoted 12 lines · On Mon, Sep 29, 2003 at 06:04:34PM +0100, [email protected] wrote:
diff -urpN --exclude-from=/home/davej/.exclude bk-linus/drivers/scsi/qlogicfc.c linux-2.5/drivers/scsi/qlogicfc.c
drivers/scsi/qlogicfc.c +2 -2
@@ -718,8 +718,8 @@ int isp2x00_detect(Scsi_Host_Template * 
 				continue;
 
 			/* Try to configure DMA attributes. */
-			if (pci_set_dma_mask(pdev, (u64) 0xffffffffffffffff) &&
-			    pci_set_dma_mask(pdev, (u64) 0xffffffff))
+			if (pci_set_dma_mask(pdev, 0xffffffffffffffffULL) &&
+			    pci_set_dma_mask(pdev, 0xffffffffULL))
 					continue;
Looks great.

Quoted 2 lines

I wonder if you are motivated to create similar pci_set_dma_mask() cleanups for other drivers? ;-)

Yeah, I'm on it. I have some patches already, will do another bombing run later tonight.

Dave

Signature · 1 line

 Dave Jones     http://www.codemonkey.org.uk

H. Peter Anvin

2003-09-29 20:25 UTC #3 lore raw

In reply to Jeff Garzik

Followup to:  <[email protected]>
By author:    Jeff Garzik <[email protected]>
In newsgroup: linux.dev.kernel

Quoted 17 lines · On Mon, Sep 29, 2003 at 06:04:34PM +0100, [email protected] wrote:

drivers/scsi/qlogicfc.c int isp2x00_detect(Scsi_Host_Template * · line 723
Quoted 12 lines
diff -urpN --exclude-from=/home/davej/.exclude bk-linus/drivers/scsi/qlogicfc.c linux-2.5/drivers/scsi/qlogicfc.c
drivers/scsi/qlogicfc.c +2 -2
@@ -718,8 +718,8 @@ int isp2x00_detect(Scsi_Host_Template * 
 				continue;
 
 			/* Try to configure DMA attributes. */
-			if (pci_set_dma_mask(pdev, (u64) 0xffffffffffffffff) &&
-			    pci_set_dma_mask(pdev, (u64) 0xffffffff))
+			if (pci_set_dma_mask(pdev, 0xffffffffffffffffULL) &&
+			    pci_set_dma_mask(pdev, 0xffffffffULL))
 					continue;
Looks great.

Quoted 4 lines

I wonder if you are motivated to create similar pci_set_dma_mask() cleanups for other drivers? ;-) Several other drivers need this same cleanup, too.

Dumb question: why marking these explicitly as ULL instead of letting the compiler do its usual promotion?

(By all means, remove the explicit cast -- the function call should do this by itself.)

0xffffffff is unsigned int and will be promoted to 0x00000000ffffffffULL by the function call. 0xffffffffffffffff is UL on 64-bit platforms and ULL on 32-bit platforms and will be promoted to 0xffffffffffffffffULL by the function call.

-hpa

Signature · 4 lines

<[email protected]> at work, <[email protected]> in private!
If you send me mail in HTML format I will assume it's spam.
"Unix gives you enough rope to shoot yourself in the foot."
Architectures needed: ia64 m68k mips64 ppc ppc64 s390 s390x sh v850 x86-64

Arjan van de Ven

2003-09-29 20:33 UTC #4 lore raw

Quoted 20 lines · H. Peter Anvin

Followup to:  <[email protected]>
By author:    Jeff Garzik <[email protected]>
In newsgroup: linux.dev.kernel
Quoted 17 lines · On Mon, Sep 29, 2003 at 06:04:34PM +0100, [email protected] wrote:
drivers/scsi/qlogicfc.c int isp2x00_detect(Scsi_Host_Template * · line 723
Quoted 12 lines
diff -urpN --exclude-from=/home/davej/.exclude bk-linus/drivers/scsi/qlogicfc.c linux-2.5/drivers/scsi/qlogicfc.c
drivers/scsi/qlogicfc.c +2 -2
@@ -718,8 +718,8 @@ int isp2x00_detect(Scsi_Host_Template * 
 				continue;
 
 			/* Try to configure DMA attributes. */
-			if (pci_set_dma_mask(pdev, (u64) 0xffffffffffffffff) &&
-			    pci_set_dma_mask(pdev, (u64) 0xffffffff))
+			if (pci_set_dma_mask(pdev, 0xffffffffffffffffULL) &&
+			    pci_set_dma_mask(pdev, 0xffffffffULL))
 					continue;
Looks great.

Quoted 7 lines

Quoted 4 lines
I wonder if you are motivated to create similar pci_set_dma_mask() cleanups for other drivers? ;-) Several other drivers need this same cleanup, too.
Dumb question: why marking these explicitly as ULL instead of letting the compiler do its usual promotion?

even dumbe question, why don't we provide ONE #define PCI_DMA_MASK_64BIT that does it right....
and use that in all needed places
(of course we need a _32BIT one too)

Jeff Garzik

2003-09-29 21:13 UTC #5 lore raw

Quoted 3 lines · Arjan van de Ven

even dumbe question, why don't we provide ONE #define PCI_DMA_MASK_64BIT that does it right.... and use that in all needed places (of course we need a _32BIT one too)

It's been suggested before, and I have no objection.

Wanna cook up the patch? ;-)

Jeff

Jeff Garzik

2003-09-29 21:11 UTC #6 lore raw

In reply to H. Peter Anvin

Quoted 1 line · H. Peter Anvin

0xffffffff is unsigned int and will be promoted to

0xffffffff without a prefix is signed.

H. Peter Anvin

2003-09-29 21:24 UTC #7 lore raw

Quoted 5 lines · Jeff Garzik

Quoted 1 line · H. Peter Anvin
0xffffffff is unsigned int and will be promoted to
0xffffffff without a prefix is signed.

No, it's not.

ISO/IEC 9899:1999(E) §6.4.4.1, page 55f:

5 The type of an integer constant is the first of the corresponding list in which its value can be represented.

Suffix Decimal Constant Octal or Hexadecimal
Constant

none int int
long int unsigned int
long long int long int
unsigned long int long long int unsigned long long int

... so 0x7fffffff is signed int, but 0xffffffff is unsigned int on an I32-model system (all Linux systems are I32-model.)

-hpa

Jeff Garzik

2003-09-29 21:37 UTC #8 lore raw

Quoted 5 lines · H. Peter Anvin

Quoted 1 line · Jeff Garzik
0xffffffff without a prefix is signed.
No, it's not.

[...]

Quoted 2 lines

... so 0x7fffffff is signed int, but 0xffffffff is unsigned int on an I32-model system (all Linux systems are I32-model.)

I was looking at C99 standard as I typed that :) But I thought it was referring to raw storage size, and that things changed on some 64-bit platforms.

So, I stand corrected.

Jeff