From: Ralf Baechle <ralf@linux-mips.org>
To: Jeff Garzik <jgarzik@pobox.com>
Cc: bjorn.helgaas@hp.com, Andrew Morton <akpm@osdl.org>,
Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] ioc3-eth.c: add missing pci_enable_device()
Date: Wed, 25 Aug 2004 07:49:40 +0200 [thread overview]
Message-ID: <20040825054940.GA23827@linux-mips.org> (raw)
In-Reply-To: <412BE006.8040606@pobox.com>
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
next prev parent reply other threads:[~2004-08-25 5:50 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <200408242225.i7OMPGLQ029847@hera.kernel.org>
2004-08-25 0:40 ` [PATCH] ioc3-eth.c: add missing pci_enable_device() Jeff Garzik
2004-08-25 5:49 ` Ralf Baechle [this message]
2004-08-25 15:03 ` Bjorn Helgaas
2004-08-25 15:06 ` Christoph Hellwig
2004-08-04 21:38 Bjorn Helgaas
2004-08-19 20:14 ` Ralf Baechle
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20040825054940.GA23827@linux-mips.org \
--to=ralf@linux-mips.org \
--cc=akpm@osdl.org \
--cc=bjorn.helgaas@hp.com \
--cc=jgarzik@pobox.com \
--cc=linux-kernel@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.