On Fri, May 20, 2011 at 05:24:05PM +0100, Frank Hofmann wrote: > 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 ? Gone in where? I haven't submitted my patch to Russell's patch system yet, but it takes into account earlier discussions and there have been no major disagreements, so I will submit it if it is not superseded. > 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
