On Saturday 31 March 2001  5:50 pm, Ben Harris wrote:
> In article <01033116451801.00335@pinky.paradox.demon.co.uk> you write:
> There's one thing I want to check up on though, I'm not happy with the way
>
> >I've had to do the irq handler attach:
> >The patch wanted to do:
> >! 	npsc->sc_ih.ih_func = dp8390_intr;
> >! 	npsc->sc_ih.ih_arg = dsc;
> >! 	npsc->sc_ih.ih_level = IPL_NET;
> >! 	npsc->sc_ih.ih_name = "if_ne";
> >! 	npsc->sc_ih.ih_maskaddr = npsc->sc_podule->irq_addr;
> >! 	npsc->sc_ih.ih_maskbits = npsc->sc_podule->irq_mask;
> >!
> >! 	if (irq_claim(npsc->sc_podule->interrupt,&(npsc->sc_ih)) < 0) {
> >
> >The code submitted, does:
> >	/* Install an interrupt handler */
> >	evcnt_attach_dynamic(&npsc->sc_intrcnt, EVCNT_TYPE_INTR, NULL,
> >	    self->dv_xname, "intr");
> >	npsc->sc_ih = podulebus_irq_establish(pa->pa_ih, IPL_NET, dp8390_intr,
> >	    dsc, &npsc->sc_intrcnt);
> >	if (npsc->sc_ih == NULL)
> >		panic("%s: Cannot install interrupt handler",
> >		   dsc->sc_dev.dv_xname);
> >+	/* this feels wrong to do this here */
> >+	npsc->sc_ih->ih_maskaddr = npsc->sc_podule->irq_addr;
> >+	npsc->sc_ih->ih_maskbits = npsc->sc_podule->irq_mask;
> >
> >It works, but it just feels wrong to update the maskaddr and maskbits
> >after the handler was established.  What is the correct thing to do?  add
> >extra params to irq_establish for irq_addr and irq_mask (irq_establish is
> >really just a wrapper around intr_claim (which wrappers irq_claim))?
>
> Last time I checked, the IOMD IRQ handler completely ignored those fields,
> which is why podulebus_irq_establish() doesn't bother filling them in.  My
> recommendation would thus be to do the same unless you have a good reason
> to do otherwise (in which case, I want to know about it).

They look to be used in poduleirqhandler, but only if something has gone 
really wrong with the IRQ handling.  This is why I asked, as I noted that 
some drivers don't set them any more. (eg the dev/podbus/if_ie.c doesn't, 
while the arm32 one does)

So it's probably best to take those lines out?

Cheers,
Chris