public inbox for linux-pm@vger.kernel.org
 help / color / mirror / Atom feed
From: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
To: Stephen Boyd <sboyd@codeaurora.org>
Cc: Mike Turquette <mturquette@linaro.org>,
	Viresh Kumar <viresh.kumar@linaro.org>,
	"Rafael J. Wysocki" <rjw@rjwysocki.net>,
	Jason Cooper <jason@lakedaemon.net>, Andrew Lunn <andrew@lunn.ch>,
	Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>,
	Gregory Clement <gregory.clement@free-electrons.com>,
	Lior Amsalem <alior@marvell.com>,
	Tawfik Bayouk <tawfik@marvell.com>,
	linux-pm@vger.kernel.org, Nadav Haklai <nadavh@marvell.com>,
	Ezequiel Garcia <ezequiel.garcia@free-electrons.com>,
	linux-arm-kernel@lists.infradead.org,
	Boris Brezillon <boris.brezillon@free-electrons.com>
Subject: Re: [PATCHv2 1/8] clk: add an APPLY_RATE_CHANGE notifier event during clk_set_rate()
Date: Tue, 8 Jul 2014 22:25:16 +0200	[thread overview]
Message-ID: <20140708222516.6b0c0d66@free-electrons.com> (raw)
In-Reply-To: <53BC522E.8080302@codeaurora.org>

Dear Stephen Boyd,

On Tue, 08 Jul 2014 13:18:54 -0700, Stephen Boyd wrote:

> >>> In order to solve this problem, we propose to add an APPLY_RATE_CHANGE
> >>> notifier event, which gets called right after ->set_rate(), but before
> >>> ->recalc_rate(), and therefore regardless of whether there was an
> >>> actualy frequency change or not.
> >> Is there any reason why we can't call the pmsu code (part #3) directly
> >> from the cpu clock driver? It seems like if we just called the
> >> .set_rate() op we wouldn't actually have changed the clock's rate. That
> >> doesn't seem very intuitive and it really makes the code flow hard to
> >> follow.
> > Right, but what solution would you propose to achieve that? These days,
> > a direct call from drivers/ code to arch/arm/mach-<foo>/ code is
> > frowned upon, no? (The code handling the PMSU is in
> > arch/arm/mach-mvebu/pmsu.c).
> 
> I don't see a problem with having an include file in include/linux/ for
> this, maybe others do though.

I'm fine with that as well, but I believe one of the policy was to
avoid having too much drivers/ stuff call into arch/arm/ stuff.

> If it actually is a problem then perhaps
> moving the pmsu.c code into drivers/ is the right solution.

Yes, but where would it belong? The PMSU hardware block is tightly
linked to SMP initialization (which means it should be up and running
very early), CPU hotplug, cpuidle, cpufreq and suspend/resume. It means
that it interacts with a lot of different subsystems.

Maybe the solution of adding a direct call from
drivers/clk/mvebu/clk-cpu.c to arch/arm/mach-mvebu/pmsu.c is the
easiest and most explicit solution.

Thanks,

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

  reply	other threads:[~2014-07-08 20:25 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-07-07 14:51 [PATCHv2 0/8] cpufreq support for Marvell Armada XP Thomas Petazzoni
2014-07-07 14:51 ` [PATCHv2 1/8] clk: add an APPLY_RATE_CHANGE notifier event during clk_set_rate() Thomas Petazzoni
2014-07-07 23:44   ` Stephen Boyd
2014-07-08  7:15     ` Thomas Petazzoni
2014-07-08 20:18       ` Stephen Boyd
2014-07-08 20:25         ` Thomas Petazzoni [this message]
2014-07-07 14:51 ` [PATCHv2 2/8] clk: mvebu: extend clk-cpu for dynamic frequency scaling Thomas Petazzoni
2014-07-07 14:51 ` [PATCHv2 3/8] ARM: mvebu: ensure CPU clocks are enabled Thomas Petazzoni
2014-07-07 14:51 ` [PATCHv2 4/8] ARM: mvebu: extend PMSU code to support dynamic frequency scaling Thomas Petazzoni
2014-07-08 13:05   ` Ezequiel Garcia
2014-07-07 14:51 ` [PATCHv2 5/8] ARM: mvebu: update Armada XP DT for " Thomas Petazzoni
2014-07-07 14:51 ` [PATCHv2 6/8] ARM: mvebu: allow enabling of cpufreq on Armada XP Thomas Petazzoni
2014-07-07 14:51 ` [PATCHv2 7/8] ARM: mvebu: update mvebu_v7_defconfig with cpufreq support Thomas Petazzoni
2014-07-07 14:51 ` [PATCHv2 8/8] ARM: configs: add cpufreq-generic in multi_v7_defconfig Thomas Petazzoni

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20140708222516.6b0c0d66@free-electrons.com \
    --to=thomas.petazzoni@free-electrons.com \
    --cc=alior@marvell.com \
    --cc=andrew@lunn.ch \
    --cc=boris.brezillon@free-electrons.com \
    --cc=ezequiel.garcia@free-electrons.com \
    --cc=gregory.clement@free-electrons.com \
    --cc=jason@lakedaemon.net \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-pm@vger.kernel.org \
    --cc=mturquette@linaro.org \
    --cc=nadavh@marvell.com \
    --cc=rjw@rjwysocki.net \
    --cc=sboyd@codeaurora.org \
    --cc=sebastian.hesselbarth@gmail.com \
    --cc=tawfik@marvell.com \
    --cc=viresh.kumar@linaro.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox