Re: [PATCH] apds9802als: add runtime PM support

[Alan Stern]

On Fri, 15 Oct 2010, Alan Cox wrote:

> From: Hong Liu <hong.liu@xxxxxxxxx>
> 
> 
>     Update the driver for the needed runtime power features. Remove the old
>     user controlled power functions.
> 
>     Signed-off-by: Hong Liu <hong.liu@xxxxxxxxx>
>     Signed-off-by: Alan Cox <alan@xxxxxxxxxxxxxxx>

> diff --git a/drivers/misc/apds9802als.c b/drivers/misc/apds9802als.c
> index fbe4960..15f9436 100644
> --- a/drivers/misc/apds9802als.c
> +++ b/drivers/misc/apds9802als.c

> @@ -265,13 +274,34 @@ static int apds9802als_suspend(struct i2c_client *client, pm_message_t mesg)
>  
>  static int apds9802als_resume(struct i2c_client *client)
>  {
> -	struct als_data *data = i2c_get_clientdata(client);
> +	als_set_default_config(client);
> +
> +	pm_runtime_get(&client->dev);
> +	pm_runtime_put(&client->dev);
> +	return 0;
> +}

This almost certainly does not do what you think.

pm_runtime_get() will increment the device's usage count and queue an 
asynchronous resume request.  However, since the PM workqueue is frozen 
during system sleep transitions, the device will remain suspended.

The pm_runtime_put() will decrement the usage count again, but since
there is already an async resume on the queue it will not queue an
async suspend.  The final result will be that when tasks are unfrozen,
the device will finally be resumed -- long after it should have been.

It looks like what you want to do here is simply call
apds9802als_runtime_resume() directly.  And according to the advice in
Documentation/power/runtime_pm.txt section 6, you should also call

	pm_runtime_disable(dev);
	pm_runtime_set_active(dev);
	pm_runtime_enable(dev);

You probably also do not want the asynchronous calls to 
pm_runtime_get() and pm_runtime_put() in apds9802als_probe().  A more 
common sequence is:

	pm_runtime_set_active(dev);
	pm_runtime_enable(dev);

Alan Stern

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/