Subject: Re: [PATCH, RFC] linkstation: implement standby

From: [email-masked] (Tony Breeds)
Date: Mon, 27 Aug 2007 10:44:28 +1000

On Sun, Aug 26, 2007 at 02:03:49AM +0200, Guennadi Liakhovetski wrote:
> Implement suspend/resume for "mpc10x" compatible fsl host PCI controllers, 
> use it for linkstation standby.

Hi Guennadi

<snip>

> +static int __init ls_pm_init(void)
> +{
> +	if (!machine_is(linkstation))
> +		return 0;

This should probably be -ENODEV.

<snip>

> +int mpc10x_suspend(suspend_state_t state)

You only seem to call this from ls_pm_enter() where you don't check the
return value, perhaps this should be void ?

> +{
> +	struct pci_dev *bridge;
> +	u16 pmcr1;
> +
> +	bridge = pci_find_slot(0, 0);
> +	if (!bridge)
> +		return -ENODEV;

I think you want pci_get_bus_and_slot() here instead of pci_find_slot(),
in fact I think it'd be cleaner, if you located the bridge in
ls_pm_enter() and passed it in.

> +int mpc10x_resume(suspend_state_t state)
> +{
> +	struct pci_dev *bridge;
> +	u16 pmcr1;
> +
> +	bridge = pci_find_slot(0, 0);
> +	if (!bridge)
> +		return -ENODEV;

Same comments ass mpc10x_suspend();

Yours Tony

  linux.conf.au        http://linux.conf.au/ || http://lca2008.linux.org.au/
  Jan 28 - Feb 02 2008 The Australian Linux Technical Conference!