Messages in this thread |  | | | Date | Wed, 25 Aug 2004 07:49:40 +0200 | | From | Ralf Baechle <> | | Subject | Re: [PATCH] ioc3-eth.c: add missing pci_enable_device() |
| |
On Tue, Aug 24, 2004 at 08:40:38PM -0400, Jeff Garzik wrote:
> I don't see a "signed-off-by" line from Ralf. I noticed you never > bothered to send this patch to me. Did you send it to Ralf either? > > ioc3 is _very_ strange device and not fully compliant to the PCI spec. > > I would appreciate more review and testing before these patches go in, > particularly against net drivers. pci_enable_device() is NOT just a > simple cleanup: > > * each driver may (though unlikely) manage its PCI_COMMAND bits and/or > resources in a special way. IDE driver is an example of where > pci_enable_device() _cannot_ be used. > > * like ioc3, the hardware may be weird > > * you must consider the case of two drivers for the same hardware VERY > carefully. Consider: > > 1) (DRV A) modprobe > 2) (DRV A) pci_enable_device() > 3) (DRV A) starts operation > > 4) (DRV B) modprobe > 5) (DRV B) pci_enable_device() > 6) (DRV B) pci_request_regions() or request_region() fails (since driver > A owns the resources) > 7) (DRV B) pci_disable_device() > > 8) (DRV A) fails miserably, because you just disabled IO/MEM bits from > an _active_ driver. BOOM.
... and that's similar to the hole into which IOC3 is falling. IOC3 is a multi function device - but not in sense of the PCI spec. IOC3 provides a standard ethernet interface with the usual gadgets like PHY interfacing. It also contains a PS/2 keyboard / mouse interface (thanks to which my headless Origin 200 has each 4 keyboard and mouse connectors!), a 486-style backside bus on which a standard PC SuperIO chip providing the 16552s and a RTC chip are hooked up. As things are right now each of these functions is provided by a different driver and there is no central coordination that ensures pci_disable_device() will only be called by the time the last driver has finished it's business. As consequence until we can cleanly handle this is not calling pci_disable_device().
Since the original posting I also found that the original argument about interrupt routing is bogus; in violation of the PCI spec IOC3 supports multiple interrupt pins. On IP27 (Origin, Onyx 2) they're all just wired together re-establishing PCI compliance. That's not the case on Octane which needs special handling outside of what the normal PCI code provides.
Ralf - 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/
|  |