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
Signature · 4 lines
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
Quoted 7 lines · Steve Modica
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).
Quoted 6 lines
Jes suggested that the appropriate way to fix this would be to create a function like
pci_enable_dma64and 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
Signature · 3 lines
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."
Quoted 5 lines
Quoted 5 lines
Quoted 1 line
Quoted 1 line
Quoted 1 line
"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
Quoted 7 lines
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 adma_addr_twhich means you have to do #ifdefCONFIG_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
Quoted 8 lines · Jes Sorensen
Quoted 4 lines
Quoted 4 lines
Quoted 1 line
Quoted 1 line
Quoted 1 line
"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.
Quoted 7 lines
Jeff> Nobody should be setting
dma_maskdirectly 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 adma_addr_twhich means you have to do #ifdefCONFIG_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.
Quoted 3 lines · Alan Cox
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
Signature · 3 lines
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."
Quoted 3 lines
It seems to me that not doing #ifdef
CONFIG_HIGHMEMright now is a bug... I think it's the megaraid driver that wants to setdma_addr_tto 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.
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
"Jeff" == Jeff Garzik <[email protected]> writes:
Quoted 4 lines · Jeff> Jes Sorensen wrote:
Quoted 4 lines
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 adma_addr_twhich means you have to do #ifdefCONFIG_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
Quoted 4 lines
Quoted 4 lines
Quoted 1 line
Quoted 1 line
Quoted 1 line
"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