In message <200012201454.JAA27875@dragon.tools.gtei.net>, jchacon@genuity.net w
rites:
>There's 2 ways to really look at this:
>
>1. wssattach() looks at already assigned irq's and doesn't try to attach if
>   it's irq is taken. (Ok, yuck..violates some abstraction and gets very bad
>   very quick).
>
>2. Have isa_intr_establish return NULL on conflict cases and all drivers that
>   use it start checking return values. (Not horrible, but would require some
>   fairly wide code changes).

Either of these would certainly be better than the current state 
(no ISA wss).

(1) doesn't necessarily violate the abstraction. You could
add an isa_intr_query() function to isa_machdep.c to allow
checking this, and assert that the interface is:
  i.  Optionally call isa_intr_query() to confirm there is no
      interrupt handler. This is necessary if you want isa_intr_establish()
      to not panic() the machine in the event of a conflict.
  ii. Call isa_intr_establish() to set up the IRQ
This would not be an abstraction violation. It would be not-so-beautiful,
but it has the advantage that it only requires modifying the code where
you care about it (e.g. any device you might otherwise comment out of
GENERIC).

(2) Seems quite reasonable, other than requiring some code changes.
It seems that use of isa_intr_establish() is mostly confined to
/sys/dev/isa{,pnp} and /sys/arch/*/isa (plus eisa_intr_establish()
calls it) and there aren't too many of them. Note that some routines
already check to see if the return value of isa_intr_establish() is
NULL. E.g. uha_isa.c:

 169          sc->sc_ih = isa_intr_establish(ic, upd.sc_irq, IST_EDGE, IPL_BIO
 169  ,
 170              u14_intr, sc);
 171          if (sc->sc_ih == NULL) {
 172                  printf("%s: couldn't establish interrupt\n",
 173                      sc->sc_dev.dv_xname);
 174                  return;
 175          }
 176  


Just think, this would be a great chance to write up a section 9
manpage ;-)

Of course, isa_intr_establish() is an MI interface and this should get
some minor discussion on tech-kern first. Do other *_intr_establish()
routines have a better way of dealing with this?


Going back to your previous messages, in
<200012201448.JAA27804@dragon.tools.gtei.net> you write:

| Basically once you get into isa_intr_establish there's no way to gracefully
| exit back with an error in today's scheme. Probably not the best overall
| working model since this comes up in a number of cases.
| 
| I'm not saying comment out anything that might come up in this scenario, but
| currently I find the idea of a sound card not config'ing vs. a panic on first
| install to be a reasonable tradeoff. 

I don't think it's terribly reasonable if it results in us supporting
less hardware than we used to in the default install under some
circumstances, and if there's an alternate way to make everything play
nice together. It seems like both conditions are true here.

| No, this is just the standard "we had to put something in the irq section so
| 10 was chosen and that's conflicting".

I was under the impression that IRQ 10 was the default for this device
in most installations. Interesting. Certainly probing the IRQ would
be good then (cf. other discussion of same).

| >No, what's to document is for the people who ran GENERIC kernels under 1.5
| >for whom sound always worked out of the box, but for whom it will not
| >work under 1.6 because of your change. That's the CHANGES entry.
| 
| Actaully it would only not work in the legacy isa case. I didn't touch any
| other wss entries in the file so the isapnp one would still work.

"legacy ISA" users deserve notification that things that worked by
default under 1.5 will break under 1.6, as much as any other user.

Incidently, I note you failed to update GENERIC_LAPTOP -- that's probably
a mistake?


| >True (and unfortunate). However, removing devices that have been enabled
| >in GENERIC for a long time is somewhat different from devices that have
| >never been enabled by default. I don't know about ess, but the aria
| >is commented out because the probe give sfall hists (i.e. it is buggy).
| 
| Actually checking, there are 35 cases of isa devices (now) commented
| out in GENERIC for one reason or another. Note, that *none* of these
| have any comments in the config file stating why

a) This is untrue:

# ISA audio devices
# the "aria" probe might give false hits
#aria0  at isa? port 0x290 irq 10               # Aria
#ess0   at isa? port 0x220 irq 5 drq 1 drq2 5   # ESS 18XX

b) Past failures to document why some things are broken is no
excuse for failing to document it in the future...

| and a quick perusal and some man pages also shows no additional
| documentation. Documentation for configuring a kernel should include
| warnings about double assigning irq's, etc but otherwise I'm not
| seeing any current model to work from.

As I mentioned previously, I was under the impression there was some
particular conflict you were seeing with wss(4) that wasn't possible
with most other ISA devices (hence the presumed justification for
special treatment). You seem justified in omitting a change to wss(4),
but it also seems important to cover the isa(4) conflict case, as well
as the CHANGES update.

| Actually let me look into that. If I can get the wss to just auto
| config itself that'll be simpler.

I'm sure we all agree ;-)

--jhawk