Patch: Defer bus_config_intr() until bus_alloc_resource()..

[John Baldwin]

On Tuesday 01 June 2004 04:25 pm, Nate Lawson wrote:
> On Tue, 1 Jun 2004, John Baldwin wrote:
> > I need the patch below in order to turn on bus_config_intr() when using
> > the I/O APICs.  The original problem is that the _CRS of link devices is
> > configured which is the PIC IRQ and thus screws up intpins when using the
> > APIC.  It basically takes bus_config_intr() out of the resource parsing
> > code and does the config when an IRQ is allocated via
> > bus_alloc_resource() for normal devices, and when a PCI IRQ is routed for
> > PCI link devices.
>
> I appreciate what you're trying to do but I don't like this approach.
> Deferring half the parsing to alloc time and moving it from
> acpi_resource.c results in a lot of unnecessary duplication and layering
> violation.  The real issue you're trying to work around is that you want
> to defer the actual config_intr until you're sure which intr you're going
> to use.

Well, arguably it exposes an improper layering violation when 
bus_config_intr() was added.  bus_config_intr() was probably added in the 
place that it is now simply because it was easier to do so.  That doesn't 
mean it's in the correct place.  I don't really consider having an 
ACPI-specific routine understand ACPI-specific resources.  One possibility 
though might be to add a wrapper function to acpi_resource.c that does the 
bus_config_intr() on a passed-in pointer to an ACPI resource.  That might 
reduce at least some of the duplication.

> Some suggestions...  Make polarity and trigger real resource types
> (sys/i386/include/resource.h) and do a bus_set_resource of them in the
> resource parsing code.  Then in the alloc code do a bus_get_resource for
> them and then call BUS_CONFIG_INTR.  Additionally, instead of doing the
> deferred BUS_CONFIG_INTR in the alloc code, it should actually be done in
> the MD code for bus_setup_intr().  This seems cleaner since allocating an
> irq resource shouldn't poke the hw until bus_setup_intr().

*sigh*  Trigger and polarity are not resources, they are a property of the IRQ 
resource perhaps.  Unfortunately, our rman(9) interface doesn't support 
type-specific resource attributes and I'm really not up for chainsawing 
rman(9) enough to get that into place.  Getting this bug fixed is holding up 
a lot of other work.

-- 
John Baldwin <jhb at FreeBSD.org>  <><  http://www.FreeBSD.org/~jhb/
"Power Users Use the Power to Serve"  =  http://www.FreeBSD.org