Am 21.07.2012 16:02 schrieb Michael Karcher:
Am Dienstag, den 17.07.2012, 09:17 +0200 schrieb Carl-Daniel Hailfinger:
Second patch in the series. Error checking has not changed at all, and I intend to fix that in a second spin of this patch. Right now I just want to check if we're on the same page.
Actually, I plan to commit the error checking change separately because Niklas Söderlund already sent a patch doing that.
Signed-off-by: Carl-Daniel Hailfinger c-d.hailfinger.devel.2006@gmx.net
-struct pci_dev *pcidev_dev = NULL;
Yeah!
if ((addr = pcidev_readbar(dev, bar)) != 0) {
curaddr = addr;pcidev_dev = dev;
found_dev = dev; found++; }I wonder what to do about this: While the goal of my suggestion was to decouple BAR access from the PCI device scan, it is an integral part of the scan loop, probably to avoid disabled chips (e.g. for onboard components that are not used).
Intel dual port NICs are represented by two PCI devices, but only one of them has an active BAR for our purposes. It would be unfair of us to have users guess the right PCI device.
The nice property of the code as-is is that you can be sure reading the "primary" BAR will not fail.
As we sometimes need two BARs, having one valid BAR does not mean the device is necessarily usable for us, so this check is only half of what we need. As already discussed on IRC, passing a set of BARs into this function is not really the direction we want to head to, so client code needs to be prepared to find unusable BARs anyway. Still, we like autoskip. Several ideas come to my mind
- Use the PCI command word for autoskip
How would that work?
- Implement a set-of-BAR (bitmask, array) anyway
Depending on the PCI header type, BARs can be at completely different locations. Still, set-of-BAR sounds like a good idea.
- Probe that all BARs (except ROM perhaps) that are not unused contain
sensible values
How do we determine which values are sensible?
- Hand in a "is_usable" callback (a standard callback for testing a
single BAR could be provided)
Do you really trust new driver authors to get that right?
All of these approaches of course complicate pcidev_init, but it seems like the only choices we have is:
- do the BAR check the right way (TM)
- lose the autoskip of disabled PCI devices
Or we handle this in a completely different way...not sure how, but maybe there really is some magic third way.
Regards, Carl-Daniel