Date: Wed, 2 Jun 2004 17:30:56 -0700 (PDT) From: Nate Lawson <nate@root.org> To: "M. Warner Losh" <imp@bsdimp.com> Cc: freebsd-acpi@freebsd.org Subject: Re: Patch: Defer bus_config_intr() until bus_alloc_resource().. Message-ID: <20040602171730.Q38563@root.org> In-Reply-To: <20040602.180819.122454942.imp@bsdimp.com> References: <20040601.163316.88476133.imp@bsdimp.com> <200406021358.17521.jhb@FreeBSD.org> <20040602.180819.122454942.imp@bsdimp.com>
next in thread | previous in thread | raw e-mail | index | archive | help
On Wed, 2 Jun 2004, M. Warner Losh wrote:
> In message: <20040602160338.T38138@root.org>
> Nate Lawson <nate@root.org> writes:
> : 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.
>
> What sort of resources have modes (apart from the polarity of
> interrupts)?
Rman has other types (like meter) and those units may have some
differentiating factor. Memory may be tuned to be uncacheable,
write-through, or other modes. For instance, instead of setting MTRRs,
being able to set a parameter for a given resource might be a more general
way to do this kind of thing. I don't have a good idea about the future
of this, just that per-resource attributes (in some form) are the right
way to capture the polarity/trigger and may be generally useful in the
future.
> What does the 'flags' mean? Who is allowed to set it?
It's a per-resource attribute, interpreted by anyone who knows more about
the specific resource (i.e. the bus). Anyone is allowed to set it that
can call the API, just like every other bus function.
> It would likely be better to have an API that allows mutliple parties
> to set it.
Sure, that's the model of having a separate "set attribute" function.
> The PCI bus code sets the IRQ number or requests the
> higher level nodes in the device tree to route an interrupt for it,
> which it then sets. The ordering is not quite right to allow for the
> flags to be set in the initial bus_set_resource.
Ok, then this should be separate.
> : > 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.
>
> Not necessarily true. It may be better to deal with this in an ad-hoc
> way now to make progress and deal with the resource model short
> comings in the future. Especially given how close we are to the 5.x
> branch point. It might make good sense to have an expedient fix as
> well as a long term plan.
I agree this is shaping up to be a short/long term kind of thing. Let me
think a little about the last version of jhb's patch, see if I can improve
it, and get back to you within a day. One problem with his approach is it
splits acpi resource parsing due to a lower-level API deficiency and that
makes maintaining the resource code harder. In fact, my local rman work
has shown me that we need to be improving things before they split
further.
> The bigger problem is that interrupts are an afterthough in our
> resource allocation mechanism. They just barely fit into the scheme
> we have now, and not very well. Two step allocation/activation is
> different than other resources (since you have to give an ISR rather
> than just calling bus_activiate_resource). Forcing all resources to
> behave like interrupts so we can use the common underlying mechanism
> might not be the right thing either.
Yes, this is a really big architectural issue. I was hoping we could come
up with a general hack for passing private information from parse time to
consumers downstream. You're right that we should work on this for the
6.x timeframe. I also found I needed multi-pass probe/attach for the rman
work but recreated it locally within the acpi probe.
I'm still holding out for a low-impact way of passing info from parse time
to alloc time. Perhaps I can use an ACPI ivar. I'll get back to you.
-Nate
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20040602171730.Q38563>