Re: [patch 22/53] PNP: factor pnp_init_resource_table() and pnp_clean_resource_table()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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

[Index of Archives]     [Linux IBM ACPI]     [Linux Power Management]     [Linux Kernel]     [Linux Laptop]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Video 4 Linux]     [Device Mapper]     [Linux Resources]

  Powered by Linux