[PATCH] ULL fixes for qlogicfc

[PATCH] ULL fixes for qlogicfc

[davej]

diff -urpN --exclude-from=/home/davej/.exclude bk-linus/drivers/scsi/qlogicfc.c linux-2.5/drivers/scsi/qlogicfc.c
--- bk-linus/drivers/scsi/qlogicfc.c	2003-09-08 00:47:00.000000000 +0100
+++ linux-2.5/drivers/scsi/qlogicfc.c	2003-09-08 01:30:56.000000000 +0100
@@ -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));

Re: [PATCH] ULL fixes for qlogicfc

[Jeff Garzik]

On Mon, Sep 29, 2003 at 06:04:34PM +0100, davej@redhat.com wrote:
> diff -urpN --exclude-from=/home/davej/.exclude bk-linus/drivers/scsi/qlogicfc.c linux-2.5/drivers/scsi/qlogicfc.c
> --- bk-linus/drivers/scsi/qlogicfc.c	2003-09-08 00:47:00.000000000 +0100
> +++ linux-2.5/drivers/scsi/qlogicfc.c	2003-09-08 01:30:56.000000000 +0100
> @@ -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

Re: [PATCH] ULL fixes for qlogicfc

[Dave Jones]

On Mon, Sep 29, 2003 at 01:23:29PM -0400, Jeff Garzik wrote:
 > On Mon, Sep 29, 2003 at 06:04:34PM +0100, davej@redhat.com wrote:
 > > diff -urpN --exclude-from=/home/davej/.exclude bk-linus/drivers/scsi/qlogicfc.c linux-2.5/drivers/scsi/qlogicfc.c
 > > --- bk-linus/drivers/scsi/qlogicfc.c	2003-09-08 00:47:00.000000000 +0100
 > > +++ linux-2.5/drivers/scsi/qlogicfc.c	2003-09-08 01:30:56.000000000 +0100
 > > @@ -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?  ;-)

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

		Dave

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

Re: [PATCH] ULL fixes for qlogicfc

[H. Peter Anvin]

Followup to:  <20030929172329.GD6526@gtf.org>
By author:    Jeff Garzik <jgarzik@pobox.com>
In newsgroup: linux.dev.kernel
>
> On Mon, Sep 29, 2003 at 06:04:34PM +0100, davej@redhat.com wrote:
> > diff -urpN --exclude-from=/home/davej/.exclude bk-linus/drivers/scsi/qlogicfc.c linux-2.5/drivers/scsi/qlogicfc.c
> > --- bk-linus/drivers/scsi/qlogicfc.c	2003-09-08 00:47:00.000000000 +0100
> > +++ linux-2.5/drivers/scsi/qlogicfc.c	2003-09-08 01:30:56.000000000 +0100
> > @@ -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.
> 

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
-- 
<hpa@transmeta.com> at work, <hpa@zytor.com> 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

Re: [PATCH] ULL fixes for qlogicfc

[Arjan van de Ven]

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

On Mon, 2003-09-29 at 22:25, H. Peter Anvin wrote:
> Followup to:  <20030929172329.GD6526@gtf.org>
> By author:    Jeff Garzik <jgarzik@pobox.com>
> In newsgroup: linux.dev.kernel
> >
> > On Mon, Sep 29, 2003 at 06:04:34PM +0100, davej@redhat.com wrote:
> > > diff -urpN --exclude-from=/home/davej/.exclude bk-linus/drivers/scsi/qlogicfc.c linux-2.5/drivers/scsi/qlogicfc.c
> > > --- bk-linus/drivers/scsi/qlogicfc.c	2003-09-08 00:47:00.000000000 +0100
> > > +++ linux-2.5/drivers/scsi/qlogicfc.c	2003-09-08 01:30:56.000000000 +0100
> > > @@ -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.
> > 
> 
> 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)

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH] ULL fixes for qlogicfc

[Jeff Garzik]

Arjan van de Ven wrote:
> 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

Re: [PATCH] ULL fixes for qlogicfc

[Jeff Garzik]

H. Peter Anvin wrote:
> 0xffffffff is unsigned int and will be promoted to

0xffffffff without a prefix is signed.

Re: [PATCH] ULL fixes for qlogicfc

[H. Peter Anvin]

Jeff Garzik wrote:
> H. Peter Anvin wrote:
> 
>> 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

Re: [PATCH] ULL fixes for qlogicfc

[Jeff Garzik]

H. Peter Anvin wrote:
> Jeff Garzik wrote:

>>0xffffffff without a prefix is signed.

> No, it's not.
[...]
> ... 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