On Thu, 21 Feb 2008, Rafael J. Wysocki wrote:
>
> > Secondly, the one that people should use ("pci_choose_state()") doesn't
> > actually do what you claim it does. It does all kinds of wrong things, and
> > doesn't even take the target state into account at all. So look again.
>
> Well, if platform_pci_choose_state() is defined, pci_choose_state() returns
> its result and on ACPI systems that points to acpi_pci_choose_state(), so in
> fact it does what I said (apart from the error path).
Did you check closer?
I repeat: acpi_pci_choose_state() (when called from pci_choose_state())
doesn't even look at the target 'state'. It just blindly assumes that you
want the deepest sleep-state you can have.
Which happens to be correct for normal suspend, but means that if you want
to test other states (through '/sys/devices/.../power'), that sounds
broken.
I didn't check any closer, but go check it yourself. The short and sweet:
acpi_pci_choose_state() totally ignores its 'state' argument. Do you
really think that's correct? But yes, "pci_choose_state()' effectively
does that too, apart from PM_EVENT_ON, which is never used.
(But the whole and only point of pci_choose_state() was to do the
PM_EVENT_FREEZE thing differently, which it doesn't do, so I think the
real issue here is that the interface is really rather mis-designed)
I suspect most people who ever really looked and worked on this code had a
specific device in mind, and I'm sure that all of the code individually
always ends up making sense from the standpoint of some specific device
driver. It's just that it never seems to make sense from a bigger issues
standpoint, and often seems senseless from the standpoint of other devices
of other types.
Linus
-
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
