On 22-04-08 01:10, Bjorn Helgaas wrote:
Yes, right. I reproduced this on an ACPI system where the BIOS leaves
a couple devices disabled.
Ah, good, not just isapnp then. I was starting to feel lonely here, sitting
atop my pile of ISA crap...
Getting things working also needs setting pnp_res->index (to nport, nmem,
nirq, ndma in pnp_assign_resources) so that the isapnp_set_resources which
follows sets to the correct hardware index, but at that point position in
the list and the index are mixing together in unhealthy ways -- in the
pnp_assign_foo helpers, pnp_get_resource(.., idx) just get the "idx-th"
resource of the correct type in the list but it seems it really should be
getting the resource of the correct type with its ->index set to "idx".
I don't mind setting pnp_res->index in the generic pnp_assign_* code.
We have to do that already in pnp_set_current_resources() (the /sys
interface), and I don't see a good way around it.
In pnp_assign_resources(), we currently assume that all independent
options appear before any dependent ones because we compute nport,
nmem, etc by iterating through the independent options first. Then
we use those nport, nmem, etc values as the "index" (CSR index for
ISAPNP, nth resource type in the template for PNPBIOS and PNPACPI).
I don't know whether this assumption is in the spec, but at least
we've assumed it for a long time.
Did this just address my position/index worry above?
It seems you designed the list to be basically in any order, judging by
things such as pnp_new_resource which'll happily reuse resources of the
correct type at any position in the list. Yet, pnp_assign_foo() and friends
retrieve resources (through pnp_get_resource) by position in the list and
not by the index. I'm not overly sure of failure scenarios but isn't this
mixing up position and index in a bad way?
I'm trying to figure out the cases where pnp_assign_resources()
has to pay attention to pre-existing configuration. It looks like
the common case is that we'll start with an empty resource list, and
we can just find non-conflicting values and use pnp_add_foo_resource().
Yes...
But I'm concerned about all the IORESOURCE_AUTO stuff. Seems like
we should only get to pnp_assign_foo() with !IORESOURCE_AUTO if
(a) we've used /sys to set some but not all resources, or (b) the
BIOS described fewer things in _CRS than in _PRS (which seems like
a BIOS bug). But I'm not comfortable with this yet.
Sounds right to me. Note that the /sys stuff is also not a corner case
situation either, as it's the way to force at least ISAPnP hardware to
manual settings.
(do note that pnp_assign_foo are the only callers of pnp_check_foo and they
could be either merged together or at least not communicate via "idx" but
simply by passing the res/pnp_res).
Yes, I'd like to do that. But I think I'd better wait or I'll never
get anything finished :-)
Well, the idea here was that getting rid of one "idx" here so that things
communicate directly removes at least one possible ordering artifact...
Also note -- manually set resources are skipped in pnp_assign_resources, yet
they also definitely need their index initialized for use by
isapnp_set_resources.
Yes. "Manually set resources" includes ones from /sys and also the
resources we discover from active devices. We should already be setting
those in the /sys and ISAPNP "read resources" paths.
Hmm, yes, that sounds true...
Rene.
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
