Re: [PATCH] add dispatch_i8259_irq() to i8259.c

[Jun Sun <jsun@mvista.com>]

On Wed, Dec 18, 2002 at 07:14:20PM +0100, Maciej W. Rozycki wrote:
> On Wed, 18 Dec 2002, Jun Sun wrote:
> 
> > > do_IRQ(poll_8259A_irq(), regs);
> > 
> > I actually don't like the new semantic.  The main drawback is that we can't
> > dispatch a 8259A interrupt from assemably code, which is often needed.
> 
>  You can't do that with your original code either, 

What do you mean?  I could do a standard assembly intr dispatching code like
this;

	move	a0, sp
	jal	dispatch_i8259_irq
	j	ret_from_irq

Please cross-check all current assembly-level irq dispatching calls.  They
are all like this.

> unless you arrange a
> call to your dispatch_i8259_irq() C function.  This can still be done with
> a trivial wrapper like:
> 
> asmlinkage void foo_dispatch_i8259_irq(struct pt_regs *regs)
> {
> 	do_IRQ(poll_8259A_irq(), regs);
> }
>

This is essentially the same as adding dispatch_i8259_irq() to i8259.c file,
which in turn calls an inline function as its function body. 

Unless there is obvious another usage of poll_8259A_irq(), the inline function
might as well be not inlined.
 
> which results in code like you proposed.
>

Yes, that is why I liked my original function call. :-)
 
> > What is wrong with original way of dispatching?  The general interrupt 
> > dispatching flow is that you chase the routing path until you find the final
> > source and do a do_IRQ().  That seems fine with i8259A case here.
> 
>  It does too much and is therefore useful for a single specific case only. 
> I focused on handling the chip only and the resulting function may be used
> however desired, including your specific case.  Not all platforms need to
> want to call do_IRQ() immediately after getting an IRQ number, including
> code already in existence. 

Note those platforms don't read i8259 registers to get irq number either.

There is also a style issue.  Dispatching an interrupt is really part of
hw_interrupt_type structure.  You should implement it in the same file as the
rest functions.  However, if anybody feels it is not optimized enough they are
always free to lump all IRQ dispatching code together in one place, probably even
in assembly code.

I also disagree to have a header file with only one function declaration, but I 
agree there is orthognal issue, mostly a maintenance issue.  So if Ralf is ok with that,
I won't bitching about it.

Jun