On Thu, 2011-08-04 at 12:01 +0530, viresh kumar wrote:

On 08/04/2011 11:06 AM, Koul, Vinod wrote:

quoted

On Thu, 2011-08-04 at 10:49 +0530, viresh kumar wrote:

quoted

On 08/03/2011 06:09 PM, Russell King - ARM Linux wrote:

quoted

On Mon, Aug 01, 2011 at 03:07:18PM +0530, Viresh Kumar wrote:

@@ -405,6 +406,7 @@ pl08x_get_phy_channel(struct pl08x_driver_data *pl08x,
 		return NULL;
 	}
 
+	pm_runtime_get_sync(&pl08x->adev->dev);

this should be ideally one of the first things you would do not last.
get_sync will ensure your .runtime_resume callback is called before it
returns (if its suspended)

quoted

 	return ch;
 }

Until this point we are not touching the registers at all. And they will
accessed after this point only.

But from maintainability POV it should be at the start.

quoted

quoted

+	pm_runtime_put(&adev->dev);

_put is probe looks suspect, why do you need this here 

To save power.

quoted

as you are already setting the status as active, this _put will decrement your
usage count and possibly call your runtime_suspend

We set status as active, as amba/bus layer has enabled it before calling
probe and it doesn't put it.

As DMA will not be used until get_phy_channel() is called, so we can save
some energy here too. So i put it here.

-- 
~Vinod