From: Tarek Dakhran <t.dakhran@samsung.com>
To: Nicolas Pitre <nicolas.pitre@linaro.org>,
Dave Martin <Dave.Martin@arm.com>
Cc: Mark Rutland <Mark.Rutland@arm.com>,
Daniel Lezcano <daniel.lezcano@linaro.org>,
Heiko Stuebner <heiko@sntech.de>,
"linux-doc@vger.kernel.org" <linux-doc@vger.kernel.org>,
"tomasz.figa@gmail.com" <tomasz.figa@gmail.com>,
Naour Romain <romain.naour@openwide.fr>,
Kukjin Kim <kgene.kim@samsung.com>,
Russell King <linux@arm.linux.org.uk>,
Ian Campbell <ijc+devicetree@hellion.org.uk>,
"devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
Pawel Moll <Pawel.Moll@arm.com>,
Stephen Warren <swarren@wwwdotorg.org>,
"rob.herring@calxeda.com" <rob.herring@calxeda.com>,
"linux-samsung-soc@vger.kernel.org"
<linux-samsung-soc@vger.kernel.org>,
Vyacheslav Tyrtov <v.tyrtov@samsung.com>,
Ben Dooks <ben-linux@fluff.org>,
Mike Turquette <mturquette@linaro.org>,
Thomas Gleixner <tglx@linutronix.de>,
"linux-arm-kernel@lists.infradead.org"
<linux-arm-kernel@lists.infradead.org>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
Rob Landley <rob@landley.net>
Subject: Re: [PATCH v3 3/4] ARM: EXYNOS: add Exynos Dual Cluster Support
Date: Mon, 11 Nov 2013 19:45:30 +0400 [thread overview]
Message-ID: <5280FB9A.7030500@samsung.com> (raw)
In-Reply-To: <52808E09.9060902@samsung.com>
Hi,
On 11.11.2013 11:58, Tarek Dakhran wrote:
>
>> On Thu, Nov 07, 2013 at 11:51:49AM -0500, Nicolas Pitre wrote:
>> Samsung people: could you give us more info on the behavior of the power
>> controller please?
>>
>>
>> Nicolas
>>
> This is how the power controller works on exynos5410. For example for
> CORE0.
>
> ARM_CORE0_STATUS register indicates the power state of Core 0 part of
> processor core.
> 0x3 indicates that power to Core 0 is turned-on. 0x0 indicates that
> power to Core 0 is turned-off.
> All other values indicate that the power On/Off sequence of Core 0 in
> progress.
>
> To turn Off the power of Core 0 power domain:
>
> 1. Set the LOCAL_POWER_CFG field of ARM_CORE0_CONFIGURATION register
> to 0x3.
> 2. After PMU detects a change in the LOCAL_POWER_CFG field, it waits
> for the execution of WFI.
> 3. After Core 0 executes the WFI instruction, PMU starts the
> power-down sequence.
> 4. The Status field of ARM_CORE0_STATUS register indicates the
> completion of the sequence.
>
> That's why in the v1 of this patch exynos_core_power_control function
> was implemented as:
>
> static int exynos_core_power_control(unsigned int cpu, unsigned int
> cluster, int enable)
> {
> unsigned long timeout = jiffies + msecs_to_jiffies(10);
> unsigned int offset = cluster * MAX_CPUS_PER_CLUSTER + cpu;
> int value = enable ? S5P_CORE_LOCAL_PWR_EN : 0;
>
> if ((__raw_readl(EXYNOS5410_CORE_STATUS(offset)) & 0x3) == value)
> return 0;
>
> __raw_writel(value, EXYNOS5410_CORE_CONFIGURATION(offset));
> do {
> if ((__raw_readl(EXYNOS5410_CORE_STATUS(offset)) & 0x3)
> == value)
> return 0;
> } while (time_before(jiffies, timeout));
>
> return -EDEADLK;
> }
>
> But, as i mentioned, this is no good using while here.
> Now thinking about the problem.
>
> Thank you,
> Tarek Dakhran
What do you think about this way of solving the problem with races?
Add new edcs_power_controller_wait function.
static void edcs_power_controller_wait(unsigned int cpu, unsigned int
cluster){
unsigned long timeout = jiffies + msecs_to_jiffies(10);
unsigned int offset = cluster * EDCS_CPUS_PER_CLUSTER + cpu;
void __iomem *status_reg = EDCS_CORE_STATUS(offset);
/* wait till core power controller finish the work */
do {
if ((readl_relaxed(status_reg) & 3) ==
edcs_use_count[cpu][cluster] ? 3 : 0)
return;
} while (time_before(jiffies, timeout));
/* Should never get here */
BUG();
}
Use it in:
static void exynos_core_power_up(unsigned int cpu, unsigned int cluster)
{
exynos_core_power_control(cpu, cluster, true);
edcs_power_controller_wait(cpu, cluster);
}
and in:
static void exynos_power_down(void)
{
bool last_man = false, skip_wfi = false;
unsigned int mpidr = read_cpuid_mpidr();
unsigned int cpu = MPIDR_AFFINITY_LEVEL(mpidr, 0);
unsigned int cluster = MPIDR_AFFINITY_LEVEL(mpidr, 1);
pr_debug("%s: CORE%d on CLUSTER %d\n", __func__, cpu, cluster);
BUG_ON(cpu >= EDCS_CPUS_PER_CLUSTER || cluster >= EDCS_CLUSTERS);
__mcpm_cpu_going_down(cpu, cluster);
arch_spin_lock(&edcs_lock);
BUG_ON(__mcpm_cluster_state(cluster) != CLUSTER_UP);
edcs_use_count[cpu][cluster]--;
if (edcs_use_count[cpu][cluster] == 0) {
exynos_core_power_down(cpu, cluster);
--core_count[cluster];
if (core_count[cluster] == 0)
last_man = true;
[snip]
__mcpm_cpu_down(cpu, cluster);
if (!skip_wfi){
wfi();
}
edcs_power_controller_wait(cpu, cluster);
}
Comments appreciated. Thanks.
Best regards,
Tarek Dakhran.
next prev parent reply other threads:[~2013-11-11 15:45 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <20131108184036.GD2602@localhost.localdomain>
[not found] ` <alpine.LFD.2.03.1311081356300.15571@syhkavp.arg>
2013-11-11 7:58 ` [PATCH v3 3/4] ARM: EXYNOS: add Exynos Dual Cluster Support Tarek Dakhran
2013-11-11 15:45 ` Tarek Dakhran [this message]
2013-11-07 8:12 [PATCH v3 0/4] Exynos 5410 Dual cluster support Vyacheslav Tyrtov
2013-11-07 8:12 ` [PATCH v3 3/4] ARM: EXYNOS: add Exynos Dual Cluster Support Vyacheslav Tyrtov
[not found] ` <20131107130141.GA3129@localhost.localdomain>
2013-11-11 8:13 ` Tarek Dakhran
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=5280FB9A.7030500@samsung.com \
--to=t.dakhran@samsung.com \
--cc=Dave.Martin@arm.com \
--cc=Mark.Rutland@arm.com \
--cc=Pawel.Moll@arm.com \
--cc=ben-linux@fluff.org \
--cc=daniel.lezcano@linaro.org \
--cc=devicetree@vger.kernel.org \
--cc=heiko@sntech.de \
--cc=ijc+devicetree@hellion.org.uk \
--cc=kgene.kim@samsung.com \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-doc@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-samsung-soc@vger.kernel.org \
--cc=linux@arm.linux.org.uk \
--cc=mturquette@linaro.org \
--cc=nicolas.pitre@linaro.org \
--cc=rob.herring@calxeda.com \
--cc=rob@landley.net \
--cc=romain.naour@openwide.fr \
--cc=swarren@wwwdotorg.org \
--cc=tglx@linutronix.de \
--cc=tomasz.figa@gmail.com \
--cc=v.tyrtov@samsung.com \
/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;
as well as URLs for NNTP newsgroup(s).