linux-kernel - RE: [PATCH V3] PM / devfreq: tie suspend/resume to runtime-pm

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-prev] [day] [month] [year] [list]

Date:	Mon, 06 May 2013 20:42:14 +0900
From:	"myungjoo.ham" <myungjoo.ham@...sung.com>
To:	"'Rafael J. Wysocki'" <rjw@...k.pl>,
	'Rajagopal Venkat' <rajagopal.venkat@...aro.org>
Cc:	'Kevin Hilman' <khilman@...aro.org>,
	'Alan Stern' <stern@...land.harvard.edu>, patches@...aro.org,
	linaro-kernel@...ts.linaro.org, linux-pm@...r.kernel.org,
	linux-kernel@...r.kernel.org
Subject: RE: [PATCH V3] PM / devfreq: tie suspend/resume to runtime-pm

From: Rafael J. Wysocki [mailto:rjw@...k.pl] 
> On Wednesday, April 17, 2013 06:58:21 PM Rajagopal Venkat wrote:
> > Devfreq core runtime suspend/resume of a device is explicitly handled 
> > by devfreq driver using devfreq_suspend_device() and
> > devfreq_resume_device() apis typically called from runtime 
> > suspend/resume callbacks. This patch aims to take away this from 
> > devfreq drivers and handle it from runtime-pm core. So that devfreq 
> > core runtime suspend/resume of a device is automatically done with 
> > runtime pm suspend/resume. The devfreq drivers shouldn't be concerned 
> > on when to suspend/resume the devfreq.
> 
> I agree, but perhaps there's a better way to achieve that than fumbling in the PM core?
> 
> Did you consider using a PM domain for that?

As genpd_add_device seems to allow a device to register multiple domains, it seems fine. We need to ensure that there is only one device for the devfreq domain though.

pm_domain seems to be an overkill; however, the excessive overhead seems to be there only for register/unregister and that seems acceptable.

> 
> > This patch is targeted to handle devfreq core load monitoring runtime 
> > suspend/resume only. Not the actual hardware itself.
> > All the resources like clocks and regulators must still be handled by 
> > device driver using runtime-pm. The sequence of devfreq and device 
> > runtime suspend/resume is,
> > 
> > pm_runtime_suspend(dev) will first suspend device devfreq (if 
> > available) before device is suspended to ensure devfreq load 
> > monitoring is stopped and no device resources like clocks are accessed 
> > while device suspend is in progress.
> > 
> > pm_runtime_resume(dev) will resume device devfreq(if available) after 
> > device is resumed to ensure device resources like clocks are ready for 
> > use.
> > 
> > As devfreq runtime suspend/resume is done automatically from runtime 
> > core, this patch removes the existing devfreq_suspend_device() and
> > devfreq_resume_device() apis.
> > 
> > Signed-off-by: Rajagopal Venkat <rajagopal.venkat@...aro.org>
> 
> I'm having a problem with this patch, because it's adding overhead into
> rpm_suspend() and rpm_resume() for all devices, even though many of them may not use devfreq.  Worse yet, there are systems in which devfreq will never be used at all.
> 
> Thanks,
> Rafael

I thought about having the polling loop to check if the device is running or not before getting usage statistics. But we still need something to notify resume.


> 
> 
> --
> I speak only for myself.
> Rafael J. Wysocki, Intel Open Source Technology Center.
>

Cheers,
MyungJoo


--
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/