Re: [PATCH] ioc3-eth.c: add missing pci_enable_device()

[Ralf Baechle]

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@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/