Date: Tue, 1 Jun 2004 16:38:04 -0400 From: John Baldwin <jhb@FreeBSD.org> To: freebsd-acpi@FreeBSD.org Cc: marcel@FreeBSD.org Subject: Re: Patch: Defer bus_config_intr() until bus_alloc_resource().. Message-ID: <200406011638.04400.jhb@FreeBSD.org> In-Reply-To: <20040601131817.N29571@root.org> References: <200406011531.09077.jhb@FreeBSD.org> <20040601131817.N29571@root.org>
next in thread | previous in thread | raw e-mail | index | archive | help
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@FreeBSD.org> <>< http://www.FreeBSD.org/~jhb/ "Power Users Use the Power to Serve" = http://www.FreeBSD.org
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200406011638.04400.jhb>