Re: [PATCH] power-management elements for 603e/fsl (version 2)

[Johannes Berg <johannes@sipsolutions.net>]

[-- Attachment #1: Type: text/plain, Size: 1426 bytes --]

On Sat, 2007-05-19 at 21:22 +0200, Guennadi Liakhovetski wrote:

> +int fsl_suspend(suspend_state_t state)
> +{
> +	struct pci_dev *bridge;
> +	unsigned long flags;
> +	u16 pmcr1;
> +
> +	bridge = pci_find_slot(0, 0);
> +	if (!bridge)
> +		return -ENODEV;
> +
> +	pci_read_config_word(bridge, 0x70, &pmcr1);
> +	local_irq_save(flags);
> +	/* Apparently, MacOS uses NAP mode for Grackle ??? */

That comment seems out of place :)

> +	pmcr1 &= ~(MPC10X_DOZE | MPC10X_NAP);
> +	pmcr1 |= MPC10X_PM | MPC10X_SLEEP | MPC10X_LP_REF_EN;
> +	pci_write_config_word(bridge, 0x70, pmcr1);
> +	local_irq_restore(flags);
> +
> +	/* Make sure the decrementer won't interrupt us */
> +	asm volatile("mtdec %0" : : "r" (0x7fffffff));
> +	/* Make sure any pending DEC interrupt occurring while we did
> +	 * the above didn't re-enable the DEC */
> +	mb();
> +	asm volatile("mtdec %0" : : "r" (0x7fffffff)); /* 8 seconds */

Are you sure that's 8 seconds? Seems too fast.

Also, the whole irq handling thing in resume/suspend seems bogus. Both
resume and suspend are called with interrupts off, so the
local_irq_{save,restore} calls do nothing. You probably need to use
arch_suspend_{enable,disable}_irqs and do the decrementer magic there
(after all that's why we introduced these.)

Then again, see the other mail I'm about to send in response to a patch
from Scott Wood using these hooks.

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 190 bytes --]