On Monday, 2 July 2007 19:24, David Brownell wrote: > On Monday 02 July 2007, Rafael J. Wysocki wrote: > > On Monday, 2 July 2007 07:49, David Brownell wrote: > > > > Also, recall that this was originally PCI-specific. A generalized > > > approach would return a range of states, not a single state that > > > embeds a particular policy that may not be universally appropriate... > > > > Via pointers and the return value equal to 0 on success? > > It could as easily be one pointer: "int minmax[2]". > Then min == minmax[0] and max == minmax[1], and the > ACPI calls could write the caller's data directly. > > But in general, yes -- pointer, not single return value. > > > > > The logic was that PCI devices can all support PCI_D0 and PCI_D3. > > > For non-PCI devices we can't actually know that. So "min" should > > > probably default to ACPI_STATE_D0. If this returns a range, then > > > such issues can stay out of this code. > > > > The ACPI spec says that all devices must support D0 and D3. > > ISTR that it actually says they must "recognize" D3. That's > not the same as actually implementing a software controllable > power-off state ... and not the same as imposing a "use the > biggest numbered D-state" policy at this level. No, it literally says "All devices must support the D0 and D3 states" (that actually is stated in Appendix A, A.3.4 in the 3.0b specification). Still, I think that the question really is what we should return if _SxD or _SxW are not present. > > > > + /* > > > > + * If _PRW says we can wake from the upcoming system state, the _SxD > > > > + * value can wake ... and we'll assume a wakeup-aware driver. If _SxW > > > > + * methods exist (ACPI 3.x), they give the lowest power D-state that > > > > + * can also wake the system. _S0W can be valid. > > > > + */ > > > > + if (acpi_target_sleep_state == ACPI_STATE_S0 || > > > > + (dev->wakeup.state.enabled && These things are the result of the evaluation of _PRW for that device and we need to check them here (or evaluate _PRW ourselves, but that wouldn't make sense, since it's already been evaulated), unless the caller tells us not to bother (we should provide the caller with a means to do it, though). > > > > > > This used device_may_wakeup() before. That ACPI flag is not a > > > direct analogue ... without looking again at the issues, I'd > > > say the right solution is to phase out the ACPI-only flags in > > > new code. > > > > Hm, I'm not sure. This is an ACPI routine after all ... > > It's in the Linux kernel, talking about Linux devices. > Some of them have ACPI support; many don't. (Like the > USB device nodes, and anything else layered on top of > mainboard devices.) I've explained above why these ACPI flags should generally be used here. > > > Maybe just expect the caller to pass the results > > > of device_may_wakeup() in ... or update the signature to accept > > > a "struct device", and fetch the handle from there. (That'd > > > be a better match for most callers, I'd think...) > > > > In that case it would be nicer to pass 'struct acpi_device *' rahter than > > 'struct device *', IMO. > > I'll disagree. Outside of the ACPI code, virtually nothing > has any reason to see an "acpi_device" ... but virtually > everything has reason to see a "device". Even ACPI code > can rely on there being a "device" inside "acpi_device"! Okay, we can use DEVICE_ACPI_HANDLE() in the same way as in your original patch, no problem with that. > This touches on a different problem. The ACPI device tree > is parallel to the "real" tree. In the cases there's a > one-to-one correspondence, there's no confusion; any > acpi_device corresponds to one "real" device, and vice > versa. BUT ... there are cases where the correspondence > isn't one-to-one. Those cases need to be addressed, by > moving towards a one-to-one correspondence. Agreed. > Cases like PCI bridges can be easily dealt with now, given > some resequencing of init logic: use the PNP node instead > of faking out a toplevel /sys/devices/pci0000:00 node. > > But things like PS2 keyboard and mouse nodes are funkier. Yes. :-) Greetings, Rafael -- "Premature optimization is the root of all evil." - Donald Knuth - 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
