Messages in this thread | | | Date | Mon, 26 May 2014 16:25:56 +0530 | From | "Srivatsa S. Bhat" <> | Subject | Re: [PATCH 3/3][update] PM / sleep: Introduce command line argument for sleep state enumeration |
| |
On 05/26/2014 04:30 PM, Rafael J. Wysocki wrote: > On Monday, May 26, 2014 02:01:14 PM Srivatsa S. Bhat wrote: >> On 05/23/2014 06:33 PM, Rafael J. Wysocki wrote: >>> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com> >>> >>> On some systems the platform doesn't support neither >>> PM_SUSPEND_MEM nor PM_SUSPEND_STANDBY, so PM_SUSPEND_FREEZE is the >>> only available system sleep state. However, some user space frameworks >>> only use the "mem" and (sometimes) "standby" sleep state labels, so >>> the users of those systems need to modify user space in order to be >>> able to use system suspend at all and that is not always possible. >>> >> >> So, is this going to be a temporary solution until all the user-space >> frameworks have been fixed? I certainly hope so, because this clearly looks >> like a bug (or a lack of feature) in user-space to me... in the sense that >> those user-space frameworks don't have support for (i.e., don't know how to >> deal with) freeze-only systems yet. >> >> The more elegant long term solution would have been to teach the kernel to >> export *truly* relative names such as SUSPEND_DEEP, SUSPEND_SHALLOW, >> and SUSPEND_LIGHT or something like that (of course, we can debate on what >> naming would suit best). > > I agree and that's a logical next step, so I have a plan to rename the kernel > symbols corresponding to each state so that they reflect the code more closely > (for example, the current PM_SUSPEND_MEM and PM_SUSPEND_STANDBY depend on the > platform to support them, but that is not clear from the symbol naming, and on > many platforms _MEM doesn't mean "suspend-to-RAM" anyway). >
Ok..
> The strings in the interface are a somewhat different matter, because user > space already depends on them (which is the source of the problem here), so we > may need to keep them or at least respond to them as expected when written to > /sys/power/state. >
Right, I see your point..
> I personally think that we should use "sleep", "snooze" and "pause" to reflect > the "sleep depth". And possibly "hibernate" instead of "disk". >
Yeah, that sounds good.
>> But this patch continues to keep the names 'SUSPEND_MEM' ("mem") etc., which >> still implies that we are doing Suspend-to-RAM, because the name itself betrays >> that info. So IMHO it doesn't really match what the command-line-switch >> 'relative_sleep_states' says. >> >> But I understand that this is a quick hack to make existing user-space work >> with systems that support only the freeze state. However, for the reasons >> mentioned above, I hope that this is a temporary solution and we can remove >> or enhance this once all those user-space frameworks have been fixed. > > Well, it is supposed to be temporary, but I'm not sure if we can count on all > user space to be fixed in any reasonable time frame (think about Android, for > example). >
Hmm, that's sad. But anyway, I just wanted to point out issues with this patch and suggest possible enhancements. But since you seem to have already thought through all of this, I have no objections to this patch.
Thank you!
Regards, Srivatsa S. Bhat
| |