On Fri, 20 May 2011, Dave Martin wrote: [ ... ] >> I've simply done there what the "setmode" macro from >> <asm/assembler.h> is doing, have chosen not to include that file >> because it overrides "push" on a global scale for no good reason and >> that sort of thing makes me cringe. > > Actually, I guess you should be using that header, from a general > standardisation point of view. In particular, it would be nicer to > use setmode than to copy it. > > The setmode macro itself could be improved to use cps for Thumb-2, > but that would be a separate (and low-priority) patch. > > > Re the push/pop macros, I'm not sure of the history for those. arch/arm/lib are the only users. > > In the kernel, it seems customary to write stmfd sp!, / ldmfd sp!, > instead of push / pop, so you could always use those mnemonics instead. > They're equivalent. The "customary" seems largely a consequence of having the #define in <asm/assembler.h> as you get a compile error if you use push/pop as instruction. I've made the change to "setmode" and stmfd/ldmfd, ok. [ ... ] > I think I agree. Few drivers use FIQ, and if there are drivers > which use FIQ but don't implement suspend/resume hooks, that sounds > like a driver bug. > > Assuming nobody disagrees with that conclusion, it would be a good > idea to stick a comment somewhere explaining that policy. <speak up or remain silent forever> ;-) Since the change which moved get/set_fiq_regs() to pure assembly has just gone in, would a simple comment update in the header file along those lines be sufficient ? I.e. state "do not assume persistence of these registers over power mgmt transitions - if that's what you require, implement suspend / resume hooks" ? > >> >> See also Russell's mails about that, for previous attempts a year >> and half a year ago to get "something for ARM hibernation" in: >> >> https://lists.linux-foundation.org/pipermail/linux-pm/2010-July/027571.html >> https://lists.linux-foundation.org/pipermail/linux-pm/2010-December/029665.html > > Relying on drivers to save/restore the FIQ state if necessary seems > to correspond to what Russell is saying in his second mail. Agreed, and largely why I haven't bothered touching FIQ. > >> >> The kernel doesn't do IRQ/FIQ/ABT/UND save / restore for >> suspend-to-ram either. CPU hotplug support (re)initializes those. I >> believe we're fine. > > OK, just wanted to make sure I understood that right. By and large, "if suspend-to-ram works, suspend-to-disk should too" seems a good idea; if the former doesn't (need to) save/restore such state even though it's not off-mode-persistent, then the latter doesn't need either. That said, I've seen (out-of-tree) SoC-specific *suspend() code that simply blotted everything out. Seems the most trivial way to go, but not necessarily the best. > > I'll leave it to others to comment on the actual SoC-specific routines, > but thanks for the illustration. To make this clear, I'm not personally considering these parts optimal as the attached patch is doing them. That's why preferrably, only the "enabler" code should go in first. I do wonder, though, whether the machine maintainers are willing to make the change to these low-level suspend parts that'd split chip state save/restore from the actual power transition. That'd allow to turn this from a "backdoor" into a proper use of the interface. Russell has made the change to pass the context buffer as argument, but not split the power transition off; things are getting there, eventually :) > > Cheers > ---Dave Thanks again, FrankH. _______________________________________________ linux-pm mailing list linux-pm@xxxxxxxxxxxxxxxxxxxxxxxxxx https://lists.linux-foundation.org/mailman/listinfo/linux-pm
