Skip site navigation (1)Skip section navigation (2)
Date:      Thu, 3 Jun 2004 10:54:14 -0700 (PDT)
From:      Nate Lawson <nate@root.org>
To:        John Baldwin <jhb@FreeBSD.org>
Cc:        freebsd-acpi@FreeBSD.org
Subject:   Re: Patch: Defer bus_config_intr() until bus_alloc_resource()..
Message-ID:  <20040603104616.I43856@root.org>
In-Reply-To: <200406030917.36901.jhb@FreeBSD.org>
References:  <20040601141424.I29932@root.org> <200406021358.17521.jhb@FreeBSD.org> <20040602160338.T38138@root.org> <200406030917.36901.jhb@FreeBSD.org>

next in thread | previous in thread | raw e-mail | index | archive | help
On Thu, 3 Jun 2004, John Baldwin wrote:
> 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.

Except we already have the special bus_setup_intr() and bus_config_intr()
methods in addition to bus_alloc_resource() and bus_activate_resource().
This indicates that a more general method is needed (bus_config_resource?)
between alloc/activate to set any special options, properties, etc.  It
could just have a (void *) argument that is interpreted by the bus method
in any way it wants.  This could also allow someone to reconfigure
resources by deactivating them, configuring them, then reactivating them.

> > 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.

With hotplug, we're going to need ways for a separate routine to set
resource attributes before bus_alloc_resource() time.  Plus, you yourself
are an advocate of a multi-pass probing approach that includes separating
resource reservation from allocation.

Regarding your workaround, I've sent you comments in private email and
when we work them out, I'm ok with your patch going in.

-Nate



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20040603104616.I43856>