linux-kernel - Re: [PATCH] apds9802als: add runtime PM support

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Date:	Thu, 21 Oct 2010 14:34:18 -0400 (EDT)
From:	Alan Stern <stern@...land.harvard.edu>
To:	Hong Liu <hong.liu@...el.com>
cc:	Alan Cox <alan@...ux.intel.com>,
	Kernel development list <linux-kernel@...r.kernel.org>,
	Linux-pm mailing list <linux-pm@...ts.linux-foundation.org>
Subject: Re: [PATCH] apds9802als: add runtime PM support

On Fri, 15 Oct 2010, Alan Cox wrote:

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

> 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@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/