Re: [PATCH v3 2/5] powerpc/85xx/86xx: Add suspend/resume support

[Scott Wood <scottwood@freescale.com>]

Anton Vorontsov wrote:
> This patch adds suspend/resume support for MPC8540, MPC8569 and
> MPC8641D-compatible CPUs.
> 
> MPC8540 and MPC8641D-compatible PMCs are trivial: we just write
> the SLP bit into the PM control and status register.
> 
> MPC8569 is a bit trickier, QE turns off during suspend, thus on
> resume we must reload QE microcode and reset QE.
> 
> So far we don't support Deep Sleep mode as found in newer MPC85xx
> CPUs (i.e. MPC8536). It can be relatively easy implemented though,
> and for it we reserve 'mem' suspend type.
> 
> Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>

Acked-by: Scott Wood <scottwood@freescale.com>

>> I'm not sure that we're guaranteed that the sleep has taken effect
>> immediately -- we may need to do a loop waiting for it to clear (on
>> 85xx), probably with some delays to give the bus a chance to become idle.
> 
> Brilliant idea, thanks!
> 
> But it works vice versa: upon write, the code flow stops immediately
> (on 85xx and 86xx),

That's what I've observed as well; the question is whether it's always 
guaranteed to be immediate.

>> The 8610 manual says that that bit should only be set as part of a
>> sequence also involving the use of MSR[POW] (section 23.5.1.4).
> 
> Well, 23.4.1.12 says:
> 
> | Sleep status
> | 0 The device is not attempting to reach sleep mode.
> | 1 The device is attempting to enter sleep mode because
> | POWMGTCSR[SLP] is set, or HID0[SLEEP] and MSR[POW] in
>                          ^^
> | the e600 core are set. The core has halted fetching, snooping
> | to the L1 and L2 caches is disabled, ....
> 
> The same in 85xx specs.
> 
> I can confirm this on real hw, after setting the SLP bit, the 8610
> actually goes into sleep mode, no code flow happens until a wakeup
> event. So playing with MSR[POW] doesn't seem to be necessary. The
> same for 85xx.

OK, looks like section 23.5.1.4 is just bogus.

> This exact snippet could be moved to the "qe" device driver, yes.
> But I didn't bother because fsl_pmc have to request the QE firmware.
> 
> You can't request the firmware in the qe driver's ->suspend()
> routine necause the firmware may be on e.g. NFS filesystem or USB
> stick (implies having QE Ethernet or QE USB fully functional).

Is there any way for software to read out the current firmware from the 
device, or is it write-only?

> We can solve that by implementing ppc_md.suspend_prepare() (must
> be called from userspace context), there we could request the
> firmware. Then QE device driver would reload it in its resume()
> callback. Needless to say that it makes things a bit more
> complicated to follow.
> 
> The current code vanishes without QE anyway. But if you insist,
> I can do the suspend_prepare() thing, although I'd prefer it as
> a cleanup patch for 2.6.33, since it would touch more code,
> specifically I'm concerned about ppc generic files.

I don't insist, it just struck me as odd.

-Scott