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

[John Baldwin]

On Wednesday 02 June 2004 07:27 pm, Nate Lawson wrote:
> On Wed, 2 Jun 2004, John Baldwin wrote:
> > Ok, summary of PITAs after an hour or so of looking into it:
>
> I appreciate your efforts.  Just to recap, we're trying to work around an
> API design problem with bus_config_intr().

Well, it depends.  See, using the existing API, I can make it work now with a 
small change.  Here's some more food for thought.  Specifically we are 
dealing with properties of an IRQ resource, correct?  Now, the flags in 
struct resource now are provided by the driver as part of the request (align 
to this many bytes, I want it to be prefetchable, etc.).  The properties of 
the actual hardware interrupt line are _not_ provided by the driver, but by 
the firmware.  Now, what's another attribute about an IRQ resource that is 
provided by the firmware/BIOS.  PCI interrupt routing!  Yes, and where do we 
go ask the firmware for that information?  bus_alloc_resource!  So, we have a 
working model now. 
>
> > - For the PIC mode on i386, we use bus_config_intr() after
> > bus_setup_intr() to set the interrupt to level/low.  This can be worked
> > around by deferring handler setup until after acpica_machdep_init() and
> > calling
> >   AcpiEnableSubsystem() twice.
> > - There is no way (currently) to get a pointer to the resource structure
> >   associated with rid X w/o calling bus_alloc_resource().  In fact, there
> > is no resource structure in which to place the IRQ configuration flags
> > until bus_alloc_resource(), thus for the bus_config_intr() that sets the
> > mode for the SCI for PIC mode above there is no resource (once you defer
> > the interrupt setup) and for all of the resources set via
> > bus_set_resource() in acpi_parse_resources() there is no resource to set
> > the flags in, so that info is lost unless we also hack on the resource
> > list items to add flag members and change that API to allow for flags to
> > be passed around.
>
> How about adding bus_{get,set}_resource_flags(dev, int rid, int flags) and
> adding an "int flags" field to struct resource_list_entry?  Or break the
> API now and add flags to the normal bus_{get,set}_resource().  This is
> going to be needed for other resource types in the future that have modes.

Feel free to work on it.  The users with machines that freeze because the APIC 
driver can't support config_intr() yet will just have to wait.  I've got to 
work on putting out preemption fires anyway.  *sigh*

> > I really don't want to spend a lot of time implementing all this. Does
> > somebody else want to do this?  The change I posted earlier is a _lot_
> > simpler and smaller.
>
> Just because bus_config_intr doesn't have a counterpart for storing the
> config doesn't mean the right thing to do is to force every user of it to
> re-parse/fetch the settings they already set.  I understand this is a pain
> but the time to contain the damage is now rather than letting it spread
> more.

No, the only change is to fix ACPI to defer parsing the information.  I'm not 
changing every user of it.  Also, we already have several other drivers (PCI 
bus for example) that do a lot of work in bus_alloc_resource() already.  ACPI 
is actually somewhat unique in that it wants to parse resources ahead of time 
rather than dynamically at bus_alloc_resource() anyways.

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