Hi Trent, I think there are now enough ideas in this thread that they're starting to get confusing. 1) Function vs. device removal 2) User interface 3) Existing fakephp bugs For (1), do you need function level hotplug? Or will you be able to get away with device level? The answer to (2) will naturally flow from your answer to (1). A few responses for you, along with some of my thoughts... * Trent Piepho <xyzzy@xxxxxxxxxxxxx>: > It looks like you weren't opposed to reverting the original > patch. I think that should be done for 2.6.27 and 2.6.28, to > restore the existing interface. Unfortunately, it won't be that simple. The reason is, the PCI slot logic expects a bus/slot tuple. It doesn't know anything about functions. In the context of the original goal (physical slots), this design decision made sense. So we can't do a simple revert of fe99740c because: - fakephp registers with PCI hotplug core for device xxxx:yy.0 - fakephp tries to register device xxxx:yy.1 - PCI hotplug core returns -EBUSY because the _device_ is already claimed I played around with this today and encountered the above problem. Now, if _device_ level hotplug is sufficient for you, then fixing (2) and (3) within the context of fakephp is pretty easy. The interface would have the limitation of only displaying function 0 per given device. [root@canola slots]# pwd /sys/bus/pci/slots # my test patch applied to fakephp to get domain:bus:slot.fn in sysfs [root@canola slots]# ls 0000:00:01.0 0000:0f:00.0 0000:4e:00.0 0000:50:01.0 0000:c2:00.0 0000:00:02.0 0000:10:00.0 0000:4f:00.0 0000:51:00.0 0000:00:04.0 0000:48:02.0 0000:50:00.0 0000:89:00.0 We can then work on fixing the fakephp bugs that you've found. Something tells me that you're interested in _function_ level hotplug though. Can you confirm this? > What should be done is to create a better interface for > disabling PCI devices/functions/bridges and re-scanning. Then > create a compatibility system that will allow software the used > fakephp to continue to work. Now fakephp can change to be more > like a real hotplug driver. At some time in future, the > compatibility layer, which will have been listed in the feature > removal schedule, can be removed. I took a _very_ brief look at the compatibility driver you wrote. It _is_ a lot simpler than fakephp. ;) You've convinced me that the 'fake%d' names are pretty useless. I was thinking of submitting my test patch that shows the domain:bus:slot.fn output above. If I do that, then fakephp will have issues with the legacy_fakephp driver you wrote because we will have name collisions in sysfs. So the question is, do we keep the existing sucky fakephp mess that I created and go with your new legacy driver as the way forward? Or do we try and fix up fakephp? Again, I think the answer to this question is whether you need function or device level hotplug. If the former, then we go with your new legacy driver and the fakephp interface has to remain sucky. If the latter, then we can keep fakephp and just get it usable again. > I also noticed this in drivers/pci/probe.c:pci_scan_device() > > list_for_each_entry(slot, &bus->slots, list) > if(PCI_SLOT(devfn) == slot->number) > dev->slot = slot; > > This appears to assume that there will only one slot with a given number > on the same bus. But if fakephp and real hotplug driver are both loaded, > this won't be the case anymore, will it? Only one hotplug driver can claim a device at a time, so the above won't be an issue. > I think a much more useful interface would be a "scan" file > that will just trigger a rescan of everything. Even the users > who know what ID to scan would probably rather not be bothered > to be forced to specify. Yes, I think a generic /sys/bus/pci/scan is a good idea. > Maybe each PCI device could get a 'rescan' attribute that > triggers a rescan of that device for new functions and > recursively rescans any subordinate busses if it's a bridge? > That might be useful too. Also a good idea. Thanks. /ac -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
