Andi,
Thanks for the feedback.
One contentious issue you didn't mention was ordering of
when to call pci_request_region() and pci_enable_device().
I've been thinking about it and I think the API design is just
broken right now. We shouldn't need pci_request_resource() at all.
If pci_enable_device() is expected to allocate requested resource
types (MMIO vs IO Port) when BIOS does not, then it should also
reserve resources when BIOS or some other code did the allocation.
For "normal" PCI BARs, something like:
pci_enable_device(pdev,resource_type_mask_to_enable);
dev->base = ioremap_nocache(pdev, pci_resource(xxx));
should be sufficient. Or am I missing something?
I'm sure MMIO/IO Port resources NOT described by PCI config space can
be described by generic request_resource() (note the lack of "pci_").
On Sat, Oct 21, 2006 at 02:55:29PM +0200, Andi Kleen wrote:
> On Saturday 21 October 2006 09:14, Grant Grundler wrote:
>
> I think you should add a note somewhere that the driver
> should be prepared to initialize the device from any
> state it put it in before. That is important for kexec
> which tries to reinit the drivers after a crash.
Yes, I agree - also so the driver can run under different
types of BIOS (different architectures).
I'll think about it more.
> And add a reference to kerneldoc somewhere too? There
> is some PCI documentation in there (although it probably
> could need some overall editing)
I need more specific guidance on this.
> > 3.2 Set the DMA mask size
> > ~~~~~~~~~~~~~~~~~~~~~~~~~
> > While all drivers should explictly indicate the DMA capability
>
> I think it's ok to rely on the defaults if 0xffffffff is suitable
I want the driver writer to think about this. I hope it
gets them to verify the device DMA capabilities and if it's
something other than 32-bit, get motivated to make use
of those features.
E.g. lots of SATA/IDE devices are 64-bit DMA capable but it
looks like generic code is leaving them at 32-bit.
Not good if the box is > 4GB (and many are now).
> And perhaps one sentence what a consistent allocation is.
Hrm...ok. I'm not enthusiastic about this though.
I've read your other comments and no issues with them.
thanks again!
grant
-
To unsubscribe from this list: send the line "unsubscribe linux-arch" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
