Messages in this thread | | | Date | Sat, 11 Oct 2003 09:26:08 -0700 | From | David Brownell <> | Subject | Re: USB and DMA on Alpha with 2.6.0-test7 |
| |
Ivan Kokshaysky wrote: > On Fri, Oct 10, 2003 at 12:43:24PM -0700, David Brownell wrote: > >> static inline int >> dma_supported(struct device *dev, u64 mask) >> { >>- BUG_ON(dev->bus != &pci_bus_type); >>- >>- return pci_dma_supported(to_pci_dev(dev), mask); >>+ /* device can dma, using those address bits */ >>+ return dev->dma_mask >>+ && (mask & *dev->dma_mask) == *dev->dma_mask; >> } > > > This doesn't work. You will always return success if mask = ~0ULL.
Only if *dma_mask == ~0ULL too ... meaning that device supports 64 bit DMA addressing, in which case the test should certainly return TRUE since that's what the caller wanted to check.
Not in cases like *dma_mask == 0xffffffff (32 bit DMA only) or 0x00ffffff (ISA dma only) ... if the proposed mask has even just one address bit that is NOT set in the "these bits work" mask, that second test returns FALSE.
(That test is basically what sparc64 does. Most implementations of the dma "mask" checking don't actually use them as masks; so I expected that more-correct check to get second looks!)
> But more important thing is that dma_supported() must not expect > valid *dev->dma_mask, as it's called from dma_set_mask().
It doesn't make sense to me that the dma mask _ever_ be invalid after its "struct device" is initialized (and registered).
The bus setup code must establish initial values for DMA masks, based on bus capabilities: 64bit, normal 32bit, 24bit ISA, etc. So from then on dma_set_mask() will be usable. All USB devices inherit that mask from the Host Controller Driver (HCD) which they happen to be using; usually for 32bit DMA, sometimes 64bit.
Some device drivers can add restrictions, with dma_set_mask(), based on actual device capabilities: certain address bits are always zero, etc. That call is illegal for USB device drivers to use -- only HCD initialization could know enough about that hardware to change the initial value.
Some of the confusion here is surely because historically drivers have been told to call pci_dma_set_mask() directly, to achieve what dma_supported() achieves: testing whether a given class of DMA addresses will work. Which is clearly the wrong model, except for hardware that's less capable than the bus it's running on ... it'd also be broken for a "generic" model, since it can't work for drivers that only work through a HAL (like all USB device drivers).
> That is, your usage of dma_supported() is buggy regardless of > DMA API implementation.
The facts I see don't support that claim.
- Dave
- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |