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 19-04-08 06:43, Rene Herman wrote:

Probably. Just a quick heads up; if you don't beat me to it I'll look at
it in more detail over the weekend but with the current series ISAPnP is
quite decidedly broken. Forget the OOPS for now which seems just a symptom of the same thing as the "too many" warnings". Result of "modprobe snd-cs4236".

I believe it's pnp_assign_resources() which has all the pnp_assign_foo()
helpers fail due to there not being any yet. I guess pnp_assign_mem() and friends should allocate upon not finding one available and not fail?

Don't get why isapnp_get_resources() didn't allocate though. As said, unless you beat me to it I'll look into it more tomorrow or sunday.

isapnp_get_resources didn't allocate simply since it hasn't been called yet; there's nothing to get from an inactive device. It is pnp_assign_resources which constructs the table which isapnp_set_resources then puts to the hardware.

Currently with the list, pnp_assign_resources doesn't construct anything. It finds an empty resource list and then fails all pnp_assign_foo() helpers. I can get things functional by making pnp_assign_foo do a pnp_add_foo_resource upon not finding one yet but that's a hack -- things are just not very right at the moment.

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

Just making it do that then again disagrees with pnpbios/acpi which do not set the index field, nor do I know if you'd want them to or want to keep the index totally isapnp specific.

Anyways, currently things really don't work though, due to

1) pnp_assign_resources needing to construct the list and
2) it needing to match resources by their index.

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

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.

No patches due to the huge series where I don't know where you want to take a different turn to end up right. Also many, many opportunities for more cleanup of drivers/pnp/manager.c in its post-series state but I suppose you want the series bisectably functional first. I'll wait for a next version for now I guess?

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