On Tue, 7 Dec 1999, Richard Henderson wrote:
>(1) What in the world is the difference between shutdown/disable
>end/enable supposed to be in hw_interrupt_type? There appear to
>be zero instances of such a difference in the existing code base.
There's no difference in the alpha (infact both functions does the same
thing). The difference is that you may have to configure an irq controller
to work and to shutdown when you won't use it anymore. So shutdown and
startup can take a long time while the disable/enable must be very fast. I
agree it looks more like power management stuff ;). CPUs that implements
cpu_idle with a `while (!current->need_resched);' or that have only one
irq controller probably will never need such a feature ;). Anyway the
current code is generic enough and we don't lose anything in performances
as I just said shutdown/startup are not in fast-paths.
>(4)
>+ if (irqflags & SA_SHIRQ) {
>+ if (!dev_id)
>+ printk("Bad boy: %s (at 0x%x) called us without \
>+ a dev_id\n", devname, (&irq)[-1]);
>
>Careful, that should be %p and __builtin_return_address.
>Obviously directly copied from x86, but it's technically
>wrong there too.
Agreed obviously ;).
>(5) I don't think you can get rid of PROBE_MASK, though it's kind
>of hard to see from just the patch. Does your replacement code
>have some mechanism to prevent certain IRQs from being probed?
Yes.
>Similarly,
>
>+ for (i = NR_IRQS-1; i > 0; i--)
>+ if (!irq_desc[i].action)
>+ irq_desc[i].handler->startup(i);
>
>should probably be iterating over ACTUAL_NR_IRQS instead.
No, that's the _whole_ point.
The rewrite gets rid of the old ugly stuff completly. All the PROBE_MASK
stuff gets delete by design. I'll probably get rid of ACTUAL_NR_IRQS too,
now I changed the irq.c i386 code to use ACTUAL_NR_IRQS where necessary to
be more strict and to more easily catch bugs but I plan to remove it and
to go with NR_IRQS soon. It's the irq controller init code that must take
care to implement the hw_handler for real irqs, all the rest in irq.c
should _not_ know anything about the details of the underlying hardware.
At compile time the ->startup will be a noop. _Only_ if the irq controller
initialization will register the hw_controller handler in the
irq_desc[irq].handler, _then_ the ->startup will do something and you'll
have a chance to get such an irq detected in the irq probe stage.
And it will do so _only_ for irqs that have no action registered yet.
BTW, the trick to take the timer irq always disabled during the irq probe
was extremely ugly (it was associated with the other ugliness of having
the irqaction of the timer interrupt set to NULL by magic):
/* The normal mask includes all the IRQs except timer IRQ 0. */
#define _PROBE_MASK(nr_irqs) \
(((nr_irqs > 63) ? ~0UL : ((1UL << (nr_irqs & 63)) - 1)) & ~1UL)
The rewrite I did cleanup all such stuff and shares the plain i386 code
with the advantage that I don't risk to insert bugs (I just catched the
last one some month ago in the i386 port and I don't want to reinvent the
wheel ;), and it has the other advantage that if somebody will fix a new
i386 bug, we'll get such bugfix for free on alpha too. So I am heavily
reducing the differences between the two ports in the most critical part
of the irq handling (SMP raceland). Also the controller drivers like
kernel/i8259.c could be shared theorically (actually it's not possible
because i386 is very arch-dirty there and basically puts all the very i386
stuff there (irq entry vectors and the like) but such sutff could be moved
elsewhere in the future, and right now I had to copy it from i386/kernel
to alpha/kernel and remove the top of the file).
Andrea
