2001-04-12 19:32:33

by Steve Modica

[permalink] [raw]
Subject: Proposal for a new PCI function call

Hi All,

We found recently that the acenic driver for the 3com gigabit ethernet card does
not enable 64 bit DMAs. (this is done by setting the appropriate mask in
pci_dev->dma_mask).

Jes suggested that the appropriate way to fix this would be to create a function
like pci_enable_dma64 and then have the driver call that, rather than directly
setting this value (a small handful of drivers do this now).

I think the function idea would let us do some sanity checking to make sure
drivers weren't setting this to 64bit on non-64 bit busses and stuff.

Anyhow, so long as no one has any heartburn with this, we'll try to put
something together and submit it. I'm not subscribed to the list *yet* (need to
get procmail setup), so please leave me in the CC line.

Thanks!
Steve

--
Steve Modica
Manager - Networking Drivers Group
"Give a man a fish, and he will eat for a day, hit him with a fish and
he leaves you alone" - me


2001-04-12 19:42:59

by Jeff Garzik

[permalink] [raw]
Subject: Re: Proposal for a new PCI function call

Steve Modica wrote:
>
> Hi All,
>
> We found recently that the acenic driver for the 3com gigabit ethernet card does
> not enable 64 bit DMAs. (this is done by setting the appropriate mask in
> pci_dev->dma_mask).
>
> Jes suggested that the appropriate way to fix this would be to create a function
> like pci_enable_dma64 and then have the driver call that, rather than directly
> setting this value (a small handful of drivers do this now).
>
> I think the function idea would let us do some sanity checking to make sure
> drivers weren't setting this to 64bit on non-64 bit busses and stuff.

pci_set_dma_mask. Modify that to do the additional checks you need.

Nobody should be setting dma_mask directly anymore, it should be done
through this function.

Jeff


--
Jeff Garzik | Sam: "Mind if I drive?"
Building 1024 | Max: "Not if you don't mind me clawing at the dash
MandrakeSoft | and shrieking like a cheerleader."

2001-04-13 00:30:20

by Jes Sorensen

[permalink] [raw]
Subject: Re: Proposal for a new PCI function call

>>>>> "Jeff" == Jeff Garzik <[email protected]> writes:

>> I think the function idea would let us do some sanity checking to
>> make sure drivers weren't setting this to 64bit on non-64 bit
>> busses and stuff.

Jeff> pci_set_dma_mask. Modify that to do the additional checks you
Jeff> need.

Jeff> Nobody should be setting dma_mask directly anymore, it should be
Jeff> done through this function.

Hmmm, I was wondering if could come up with a pretty way to do this on
32 bit boxes that wants to enable highmem DMA. Right now
pci_set_dma_mask() wants a dma_addr_t which means you have to do
#ifdef CONFIG_HIGHMEM <blah> #else <bleh> #endif.

Introducing a new function that takes bit flags as arguments might be
better?

Jes

2001-04-13 00:39:12

by Alan

[permalink] [raw]
Subject: Re: Proposal for a new PCI function call

> Hmmm, I was wondering if could come up with a pretty way to do this on
> 32 bit boxes that wants to enable highmem DMA. Right now
> pci_set_dma_mask() wants a dma_addr_t which means you have to do
> #ifdef CONFIG_HIGHMEM <blah> #else <bleh> #endif.
>
> Introducing a new function that takes bit flags as arguments might be
> better?

pci_set_dma_mask_bits() ? So you could do

pci_set_dma_mask_bits(pdev, 64);

We want everything to go through pci_set_dma_mask... type functions either way
so that we can and the mask with upstream bridges when we hit address range
limits in some peoples hardware

Alan

2001-04-14 01:50:39

by Jeff Garzik

[permalink] [raw]
Subject: Re: Proposal for a new PCI function call

Jes Sorensen wrote:
> >>>>> "Jeff" == Jeff Garzik <[email protected]> writes:
> >> I think the function idea would let us do some sanity checking to
> >> make sure drivers weren't setting this to 64bit on non-64 bit
> >> busses and stuff.

> Jeff> pci_set_dma_mask. Modify that to do the additional checks you
> Jeff> need.

> Jeff> Nobody should be setting dma_mask directly anymore, it should be
> Jeff> done through this function.

> Hmmm, I was wondering if could come up with a pretty way to do this on
> 32 bit boxes that wants to enable highmem DMA. Right now
> pci_set_dma_mask() wants a dma_addr_t which means you have to do
> #ifdef CONFIG_HIGHMEM <blah> #else <bleh> #endif.

It seems to me that not doing #ifdef CONFIG_HIGHMEM right now is a
bug... I think it's the megaraid driver that wants to set dma_addr_t to
a 64-bit mask.


Alan Cox wrote:
> pci_set_dma_mask_bits() ? So you could do
>
> pci_set_dma_mask_bits(pdev, 64);

As they say, "six of one, 'half-dozen of the other." I don't have a
preference...

Jeff


--
Jeff Garzik | Sam: "Mind if I drive?"
Building 1024 | Max: "Not if you don't mind me clawing at the dash
MandrakeSoft | and shrieking like a cheerleader."

2001-04-16 14:22:48

by Venkatesh Ramamurthy

[permalink] [raw]
Subject: Re: Proposal for a new PCI function call

> It seems to me that not doing #ifdef CONFIG_HIGHMEM right now is a
> bug... I think it's the megaraid driver that wants to set dma_addr_t to
> a 64-bit mask.

MegaRAID driver:
Only if the flag __LP64__ is defined, a 64 bit mask is set , otherwise only
a 32 bit mask is used instead. However check for CONFIG_HIGHMEM needs to be
done.




2001-04-19 02:27:11

by Jes Sorensen

[permalink] [raw]
Subject: Re: Proposal for a new PCI function call

>>>>> "Jeff" == Jeff Garzik <[email protected]> writes:

Jeff> Jes Sorensen wrote:
>> Hmmm, I was wondering if could come up with a pretty way to do this
>> on 32 bit boxes that wants to enable highmem DMA. Right now
>> pci_set_dma_mask() wants a dma_addr_t which means you have to do
>> #ifdef CONFIG_HIGHMEM <blah> #else <bleh> #endif.

Jeff> It seems to me that not doing #ifdef CONFIG_HIGHMEM right now is
Jeff> a bug... I think it's the megaraid driver that wants to set
Jeff> dma_addr_t to a 64-bit mask.

There's a bunch of them doing it - it's butt-ugly though.

Jes

2001-04-19 02:27:32

by Jes Sorensen

[permalink] [raw]
Subject: Re: Proposal for a new PCI function call

>>>>> "Alan" == Alan Cox <[email protected]> writes:

>> Introducing a new function that takes bit flags as arguments might
>> be better?

Alan> pci_set_dma_mask_bits() ? So you could do

Alan> pci_set_dma_mask_bits(pdev, 64);

Alan> We want everything to go through pci_set_dma_mask... type
Alan> functions either way so that we can and the mask with upstream
Alan> bridges when we hit address range limits in some peoples
Alan> hardware

Looks good to me

Jes