lkml.org 
[lkml]   [2013]   [Feb]   [21]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCH 01/35] mfd: ab8500-gpadc: Implemented suspend/resume
From
On 20 February 2013 14:19, Mark Brown
<broonie@opensource.wolfsonmicro.com> wrote:
> On Fri, Feb 15, 2013 at 12:56:32PM +0000, Lee Jones wrote:
>
>> +static int ab8500_gpadc_suspend(struct device *dev)
>> +{
>> + struct ab8500_gpadc *gpadc = dev_get_drvdata(dev);
>> +
>> + mutex_lock(&gpadc->ab8500_gpadc_lock);
>> +
>> + pm_runtime_get_sync(dev);
>> +
>> + regulator_disable(gpadc->regu);
>> + return 0;
>> +}
>
> This doesn't look especially sane... You're doing a runtime get, taking
> the lock without releasing it and disabling the regulator. This is
> *very* odd, both the changelog and the code need to explain what's going
> on and why it's safe in a lot more detail here.

You need to do pm_runtime_get_sync to be able to make sure resources
(which seems to be only the regulator) are safe to switch off. To my
understanding this is a generic way to use for being able to switch
off resources at a device suspend when runtime pm is used in
conjunction.

Regarding the mutex, I can't tell the reason behind it. It seems
strange but not sure.

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

Kind regards
Ulf Hansson


\
 
 \ /
  Last update: 2013-02-22 00:21    [W:0.074 / U:0.172 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site