I guess since my name keeps coming up in this thread I should weigh in.
Dan Malek wrote:
There is more to these patches than just the interrupt change. Why is
the CPM microcode, and so much of commproc.c changed?
As Tom already replied, the CPM microcode stuff was included by mistake. I
wouldn't necessarily recommend submitting that stuff as a separate patch
either. There are also a few other perfectly good bits and pieces of code
that found their way into the patch that aren't relevant: the SPI stuff in
commproc.h, the PCI and PCMCIA stuff in mbx.h, and the Ethernet stuff in
rpxclassic.h.
Dan Malek wrote:
The PCI on MBX does not work well, and this does nothing to improve it.
Does
anyone have a working MBX anymore?
This patch does not fix PCI on the MBX because additional changes are
necessary to the qspan host bridge code. However, it does facilitate making
PCI work on the MBX and other 8xx designs because it allows request_irq() to
work, which all PCI device drivers use to install their interrupt handlers.
I have a working MBX, and with these changes to the interrupt handling and a
few additional changes to the qspan host bridge support, PCI cards seem to
work in it just fine. I have successfully tested a Matrox video card,
several different Ethernet cards, and an MPEG decoder. I also used these
same interrupt handling changes on a custom 860 board with a qspan bridge
that had PCI interrupts pinned out to both SIU IRQs and CPM Port C
interrupts instead of an 8259, and that worked as well.
Dan Malek wrote:
I know Andy is sitting on a ton of 82xx PCI changes that would be nice
to see checked in some day. :-)
I did do some work once on a custom 8260 board with an external PCI host
bridge. I didn't have to do anything to the 8260 interrupt handling other
than rename request_8xxirq to request_irq so that PCI device drivers would
work without modification. Other than that, it was just a matter of
implementing the host bridge support and PCI interrupt routing table
similarly to the way it is done on, say, the 7xx board ports.
Wolfgang Denk wrote:
Isn't this more or less the same idea as presented by Wolfgang Grandegger
more
than half a year ago?
Yes, it is more or less the same idea. There is an important difference
between our approaches, however. Wolfgang's approach was to extend
request_8xxirq() so that it would handle CPM interrupts as well as SIU
interrupts. My approach was to make request_irq() work for SIU interrupts,
CPM interrupts, and any other interrupt controller (like an 8259) you might
have on a particular board. I also implemented it in a way that allows you
to simultaneously continue using request_8xxirq() and cpm_install_handler()
in the same manner as always without the need for any kernel configuration
options. (Tom changed every instance of cpm_install_handler and
request_8xxirq to request_irq in his version of the patch, but that isn't
strictly necessary.) My motivation for doing this in the first place was to
make request_irq() work for all interrupts on 8xx targets so that standard
PCI and PCMCIA device drivers will work without having to put #ifdef's in
each driver to change request_irq to request_8xxirq or cpm_install_handler.
Now as for the argument that this patch is fatally flawed because IRQ
vectors 0 to 15 aren't reserved for 8259 interrupts, I don't find that to be
compelling. PCI cards work just fine on the MBX with no modifications to
the drivers when using IRQ vectors 48 to 63 for the 8259. You just stick
the right IRQ vectors in the PCI interrupt routing table and all PCI device
drivers will happily use them without prejudice. PCMCIA and IDE drivers
also work fine. The only possible issue I see is with legacy ISA drivers
with hardcoded IRQ vectors in the range 0 to 15. I've never had an occasion
to use such a driver on an 8xx target, but if it came down to it I would
prefer to stick a #ifdef in an ISA device driver to hardcode a different IRQ
vector than to stick a #ifdef in every PCI and PCMCIA device driver to make
them call request_8xxirq or cpm_install_handler instead of request_irq.
Andy
** Sent via the linuxppc-embedded mail list. See http://lists.linuxppc.org/
Andy Lowe wrote:
.... My motivation for doing this in the first place was to
make request_irq() work ....
Any 8xx that included a PCI configuration had request_irq() defined.
One of the things Wolfgang did was to map callers of request_irq() to
interrupts that made sense on a particular hardware design. That is
the only change that should have been necessary. None of the other
changes, especially to change all of the other irq requests to call
request_irq() were necessary.
..... but if it came down to it I would
prefer to stick a #ifdef in an ISA device driver to hardcode a different IRQ
vector than to stick a #ifdef in every PCI and PCMCIA device driver to make
them call request_8xxirq or cpm_install_handler instead of request_irq.
The proper solution is to implement a request_irq() function that will
map interrupts properly for the board design, not modify everything to use
that function. You can have a board design that connects the PCI interrupt
lines to GPIOs or external interrupt lines, and in that case request_irq()
would have to know what PCI interrupt numbers are used and would map them
to a CPM interrupt. Along with that comes other special SIU configuration,
so it's uniqueness is quite evident.
This patch works for exactly one board design like the MBX, that uses an 8259
connected in a particular way for PCI interrupts. It isn't a general solution
and may not work properly for the example I mentioned above.
Thanks.
-- Dan
** Sent via the linuxppc-embedded mail list. See http://lists.linuxppc.org/
Dan Malek wrote:
Any 8xx that included a PCI configuration had request_irq() defined.
One of the things Wolfgang did was to map callers of request_irq() to
interrupts that made sense on a particular hardware design. That is
the only change that should have been necessary. None of the other
changes, especially to change all of the other irq requests to call
request_irq() were necessary.
As I pointed out, it isn't necessary to change the existing request_8xxirq
and cpm_install_handler functions to request_irq. You can continue to use
the old favorites if that is your preference, although frankly I don't
understand the attraction of request_8xxirq and cpm_install_handler.
The proper solution is to implement a request_irq() function that will
map interrupts properly for the board design, not modify everything to use
that function.
You have it backwards. Every Linux device driver in the known universe
except a few 8xx-specific drivers already use request_irq to install
interrupt handlers. All the patch does is allow request_irq to work on 8xx
targets the same as on every other architecture supported by Linux.
You can have a board design that connects the PCI interrupt
lines to GPIOs or external interrupt lines, and in that case request_irq()
would have to know what PCI interrupt numbers are used and would map them
to a CPM interrupt. Along with that comes other special SIU
configuration, so it's uniqueness is quite evident.
Not only can you have such a design, but as I pointed out I have already
used this patch on an 8xx design with a qspan bridge that uses a mixture of
both SIU and CPM Port C interrupts for PCI interrupts instead of an 8259.
It fits beautifully into this scheme with absolutely no additional changes
to request_irq. In the PCI interrupt routing table for the board, the PCI
interrupts that are routed to SIU IRQs have IRQ vectors in the range 0 to
15. The PCI interrupts that are routed to CPM interrupts have IRQ vectors
in the range 16 to 47. A PCI device driver will look up its assigned IRQ
vector via the normal method and will use it as the interrupt vector
argument to request_irq. The interrupt handler installation works
regardless of whether the corresponding vector is mapped to the SIU or to
the CPM interrupt controller, and this detail is completely transparent to
the device driver. Interrupt handler installation works properly in all
cases with no modifcations required to any existing PCI device driver.
This patch works for exactly one board design like the MBX, that
uses an 8259 connected in a particular way for PCI interrupts.
This same technique can work for all 8xx boards with any number of external
interrupt controllers cascaded in any fashion you please. It works with no
modifcations whatsoever on every 8xx board that has no external interrupt
controllers.
Andy
** Sent via the linuxppc-embedded mail list. See http://lists.linuxppc.org/