On Friday 30 March 2001 11:18 pm, Chris Gilbert wrote:
> On Friday 30 March 2001 10:32 am, Mike Pumford wrote:
> > > On Thursday 29 March 2001  9:19 pm, Mike Pumford wrote:
> > > > > Actually the patch needs updating before it can be applied to
> > > > > current. I have done the mods necessary and the updated patch is
> > > > > attached.
> > > >
> > > > Actually it wasn't but here it is.
> > >
> > > Ok I'll give it a try this weekend, check my etherm still works, and
> > > submit it (unless there's any objections)
> > >
> > > Is there any reason for the:
> > > #ifdef EH500
> > > that are sprinkled through the code?  Is that code untested?  Perhaps
> > > we should start a list of the #ifdef's we've got sprinkled through the
> > > code?
> >
> > Yes that is non-working code that hopefully will support the Etherlan
> > 500 16bit podules. I would just remove that before commiting the code.
> > I normally remove it before generating the patches but I forgot this
> > time. I can cook a new patch this evening without it.
>
> Don't mind, I'll aim to get to it tommorrow at some point.

Patch has been submitted, seems to still work on etherM, if someone can 
confirm it working on the other cards I'll close the PR.

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))?

Cheers,
Chris