2003-09-29 17:07:27

by Dave Jones

[permalink] [raw]
Subject: [PATCH] ULL fixes for qlogicfc

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));


2003-09-29 17:29:17

by Dave Jones

[permalink] [raw]
Subject: Re: [PATCH] ULL fixes for qlogicfc

On Mon, Sep 29, 2003 at 01:23:29PM -0400, Jeff Garzik wrote:
> 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
> > --- 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

2003-09-29 17:26:32

by Jeff Garzik

[permalink] [raw]
Subject: Re: [PATCH] ULL fixes for qlogicfc

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



2003-09-29 20:25:52

by H. Peter Anvin

[permalink] [raw]
Subject: Re: [PATCH] ULL fixes for qlogicfc

Followup to: <[email protected]>
By author: Jeff Garzik <[email protected]>
In newsgroup: linux.dev.kernel
>
> 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
> > --- 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
--
<[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

2003-09-29 20:33:56

by Arjan van de Ven

[permalink] [raw]
Subject: Re: [PATCH] ULL fixes for qlogicfc

On Mon, 2003-09-29 at 22:25, H. Peter Anvin wrote:
> Followup to: <[email protected]>
> By author: Jeff Garzik <[email protected]>
> In newsgroup: linux.dev.kernel
> >
> > 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
> > > --- 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)


Attachments:
signature.asc (189.00 B)
This is a digitally signed message part

2003-09-29 21:13:23

by Jeff Garzik

[permalink] [raw]
Subject: Re: [PATCH] ULL fixes for qlogicfc

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



2003-09-29 21:11:22

by Jeff Garzik

[permalink] [raw]
Subject: Re: [PATCH] ULL fixes for qlogicfc

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

0xffffffff without a prefix is signed.

2003-09-29 21:24:44

by H. Peter Anvin

[permalink] [raw]
Subject: Re: [PATCH] ULL fixes for qlogicfc

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

2003-09-29 21:37:53

by Jeff Garzik

[permalink] [raw]
Subject: Re: [PATCH] ULL fixes for qlogicfc

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