Re: [Zaurus-devel] [RFC PATCH v2 1/3] PM / Core: suspend_again callback for device PM.
Hi! too heavy (in fact it's much lighter weight than resuming all devices that your approach doesn't prevent from happening, so for example on my desktop/notebook machines I woulnd't mind at all if user space were thawed after all of the devices had been resumed). Well, it would be behavior change for the user. I told the zaurus to go s2ram, I do not expect it to wake up after 5 minutes because it needed to check battery status. I'd have to modify my userland to retry suspend again and again and again... And that's exactly what should be done. Have a user space process controlling that, because avoiding to thaw user space doesn't buy us almost anything. That makes Zaurus implement different user-kernel interface than PC class machine, because of hardware quirk. Now, I know that it's probably easier to modify the kernel than to write a user space tool for that, test it and so on, but easier is not necessarily better. It is easier, allows us to keep same user-kernel interface on PC and Zaurus, and is compatible with 2.6.38. Heck, I'm used to typing echo mem /sys/power/state. I should not have to learn different interface just because Zaurus does not have proper hardware charger. I'm not sure if we need to cover hibernation. Do you know any machine that needs this for hibernation case? Yes, any machine that needs it while suspended. What's the difference, after all? The only difference is that there's an image on permanent storage in the hibernation case. You still can overheat a battery when charging it while hibernated, right? No, you can not; not on Zaurus. It can not really power down; it is always sleeping. s2ram is sleep in operating system, hibernation or poweroff is sleep in bootloader. Bootloader takes care of battery in that case. To conclude, I'm not sure about the approach. In particular, I'm not sure if the benefit is worth the effort and the resulting complications (ie. the possibility of having to deal with wakeup signals not requested by user space) seem to be a bit too far reaching. We already have platform-specific hacks to do exactly this at least on Zaurus. Moving it to common code means that hacks are not duplicated.. Well, good to know they are there, but I'm still not sure what to do about that. At the moment I feel like having too little information to really decide, so perhaps please point me to the code you're talking about for starters. Ok, see the spitz_should_wakeup() function in arch/arm/mach-pxa/* and should_wakeup() usage. Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html ___ Zaurus-devel mailing list Zaurus-devel@lists.linuxtogo.org http://lists.linuxtogo.org/cgi-bin/mailman/listinfo/zaurus-devel
Re: [Zaurus-devel] [RFC PATCH v2 1/3] PM / Core: suspend_again callback for device PM.
On Tuesday, April 26, 2011, Pavel Machek wrote: Hi! too heavy (in fact it's much lighter weight than resuming all devices that your approach doesn't prevent from happening, so for example on my desktop/notebook machines I woulnd't mind at all if user space were thawed after all of the devices had been resumed). Well, it would be behavior change for the user. I told the zaurus to go s2ram, I do not expect it to wake up after 5 minutes because it needed to check battery status. I'd have to modify my userland to retry suspend again and again and again... And that's exactly what should be done. Have a user space process controlling that, because avoiding to thaw user space doesn't buy us almost anything. That makes Zaurus implement different user-kernel interface than PC class machine, because of hardware quirk. Let me say that again: If Zaurus needs to resume everything except for user space periodically to monitor the battery charger, I'm not sure if our suspend interface is the right one for it in the first place. It seriously looks like a workaround for the lack of appropriately implemented runtime PM, just like the Android's opportunistic suspend. Now, I know that it's probably easier to modify the kernel than to write a user space tool for that, test it and so on, but easier is not necessarily better. It is easier, allows us to keep same user-kernel interface on PC and Zaurus, and is compatible with 2.6.38. Heck, I'm used to typing echo mem /sys/power/state. I should not have to learn different interface just because Zaurus does not have proper hardware charger. No, this interface should not be used on Zaurus at all. It's not mean for that and while you can hack it to kind of work, it still is hacking rather than designing things. I'm not sure if we need to cover hibernation. Do you know any machine that needs this for hibernation case? Yes, any machine that needs it while suspended. What's the difference, after all? The only difference is that there's an image on permanent storage in the hibernation case. You still can overheat a battery when charging it while hibernated, right? No, you can not; not on Zaurus. It can not really power down; it is always sleeping. s2ram is sleep in operating system, Which is not the meaning of S2RAM I use. hibernation or poweroff is sleep in bootloader. Bootloader takes care of battery in that case. So the difference is that we let someone else worry. Cool. :-) To conclude, I'm not sure about the approach. In particular, I'm not sure if the benefit is worth the effort and the resulting complications (ie. the possibility of having to deal with wakeup signals not requested by user space) seem to be a bit too far reaching. We already have platform-specific hacks to do exactly this at least on Zaurus. Moving it to common code means that hacks are not duplicated.. Well, good to know they are there, but I'm still not sure what to do about that. At the moment I feel like having too little information to really decide, so perhaps please point me to the code you're talking about for starters. Ok, see the spitz_should_wakeup() function in arch/arm/mach-pxa/* and should_wakeup() usage. OK, I will. Thanks, Rafael ___ Zaurus-devel mailing list Zaurus-devel@lists.linuxtogo.org http://lists.linuxtogo.org/cgi-bin/mailman/listinfo/zaurus-devel
Re: [Zaurus-devel] [RFC PATCH v2 1/3] PM / Core: suspend_again callback for device PM.
Hi! And that's exactly what should be done. Have a user space process controlling that, because avoiding to thaw user space doesn't buy us almost anything. That makes Zaurus implement different user-kernel interface than PC class machine, because of hardware quirk. Let me say that again: If Zaurus needs to resume everything except for user space periodically to monitor the battery charger, I'm not sure if our suspend interface is the right one for it in the first place. Well, Zaurus does not need to resume everything. SPI+charger code is enough, and that would be preffered. No need to touch disk etc. It seriously looks like a workaround for the lack of appropriately implemented runtime PM, just like the Android's opportunistic suspend. No, not really. I'm running standard Debian; I do not want/need anything like opportunistic suspend. Now, I know that it's probably easier to modify the kernel than to write a user space tool for that, test it and so on, but easier is not necessarily better. It is easier, allows us to keep same user-kernel interface on PC and Zaurus, and is compatible with 2.6.38. Heck, I'm used to typing echo mem /sys/power/state. I should not have to learn different interface just because Zaurus does not have proper hardware charger. No, this interface should not be used on Zaurus at all. It's not mean for that and while you can hack it to kind of work, it still is hacking rather than designing things. Why not? I want the Zaurus to sleep. Why should I have to know how its charging unit works? Why should I use different interface than on PC? I want it to suspend, put it into backpack and move... Bootloader takes care of battery in that case. So the difference is that we let someone else worry. Cool. :-) Yes. Ok, see the spitz_should_wakeup() function in arch/arm/mach-pxa/* and should_wakeup() usage. OK, I will. Thanks. Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html ___ Zaurus-devel mailing list Zaurus-devel@lists.linuxtogo.org http://lists.linuxtogo.org/cgi-bin/mailman/listinfo/zaurus-devel
Re: [Zaurus-devel] [RFC PATCH v2 1/3] PM / Core: suspend_again callback for device PM.
On Tuesday, April 26, 2011, Rafael J. Wysocki wrote: On Tuesday, April 26, 2011, Pavel Machek wrote: ... Ok, see the spitz_should_wakeup() function in arch/arm/mach-pxa/* and should_wakeup() usage. OK, I will. Well, as far as I can tell, on Zaurus this is all done within the platform -enter() callback, so it doesn't carry out the device resume, while the proposed $subject patch does. For this reason, I'm not even sure if the proposed approach is really suitable for Zaurus (the resuming and suspending of all devices every once a while will cost quite some enery I guess). Still, in the unlikely case it is suitable, here's my opinion. I don't think syscore_ops is the right place to put the new suspend again callback for reasons stated previously. I also don't think dev_pm_ops is the right place to put such a callback, because it will only be necessary on very few platforms and there's no reason why every driver, subsystem or power domain in the kernel should care. Moreover, the usage cases appear to be suspend-specific. For the above reasons, the only place the suspend again callback may be put into is struct platform_suspend_ops. Then, suspend_devices_and_enter() may be modified so that it loops between dpm_suspend_start() and dpm_suspend_end() until that callback returns false and there are no wakeup events (as indicated by pm_wakeup_pending(); but please note that this would require suspend_enter() to indicate that pm_wakeup_pending() returned true and events_check_enabled should not be reset until we're sure that we _really_ want to resume). Alternatively, if that's sufficient, suspend_enter() may be called in a loop until the suspend again callback returns false and there are no wakeup events (in the above sense). The callback can be called repeat and return bool as far as I'm concerned, but I'm not insisting on that. MyungJoo, would that be suitable to you? Rafael ___ Zaurus-devel mailing list Zaurus-devel@lists.linuxtogo.org http://lists.linuxtogo.org/cgi-bin/mailman/listinfo/zaurus-devel
