From: Stephen Warren <swarren-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>
To: Thierry Reding
<thierry.reding-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>,
Dmitry Osipenko <digetx-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Cc: Alexandre Courbot
<gnurou-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>,
Peter De Schrijver
<pdeschrijver-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>,
linux-tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
stable-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Subject: Re: [PATCH] ARM: tegra20: Store CPU "resettable" status in IRAM
Date: Mon, 19 Jan 2015 10:26:22 -0700 [thread overview]
Message-ID: <54BD3E3E.2040801@wwwdotorg.org> (raw)
In-Reply-To: <20150119141224.GF23778-AwZRO8vwLAwmlAP/+Wk3EA@public.gmane.org>
On 01/19/2015 07:12 AM, Thierry Reding wrote:
> On Thu, Jan 15, 2015 at 01:58:57PM +0300, Dmitry Osipenko wrote:
>> Commit 7232398abc6a ("ARM: tegra: Convert PMC to a driver") changed tegra_resume()
>> location storing from late to early and, as a result, broke suspend on Tegra20.
>> PMC scratch register 41 is used by tegra LP1 resume code for retrieving stored
>> physical memory address of common resume function and in the same time used by
>> tegra20_cpu_shutdown() (shared by Tegra20 cpuidle driver and platform SMP code),
>> which is storing CPU1 "resettable" status. It implies strict order of scratch
>> register usage, otherwise resume function address is lost on Tegra20 after
>> disabling non-boot CPU's on suspend. Fix it by storing "resettable" status in
>> IRAM instead of PMC scratch register.
>>
>> Signed-off-by: Dmitry Osipenko <digetx-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>> Fixes: 7232398abc6a (ARM: tegra: Convert PMC to a driver)
>> Cc: <stable-u79uwXL29TY76Z2rM5mHXA@public.gmane.org> # v3.17+
>> ---
>> arch/arm/mach-tegra/cpuidle-tegra20.c | 5 ++---
>> arch/arm/mach-tegra/reset-handler.S | 10 +++++++---
>> arch/arm/mach-tegra/reset.h | 4 ++++
>> arch/arm/mach-tegra/sleep-tegra20.S | 37 ++++++++++++++++++++---------------
>> arch/arm/mach-tegra/sleep.h | 4 ++++
>> 5 files changed, 38 insertions(+), 22 deletions(-)
>
> I'm leaning towards applying this. Stephen, Alex, Peter: any objections?
Hopefully this works out. I suppose it's unlikely anyone will be running
code on the AVP upstrem, so any potential conflict with AVP's usage of
IRAM isn't likely to occur.
__tegra20_cpu1_resettable_status_offset has a lot of _ at the start.
Should the symbol be named more normally? I guess at least it's
consistent with the existing very "underscory"
__tegra_cpu_reset_handler_start.
WARNING: multiple messages have this Message-ID (diff)
From: Stephen Warren <swarren@wwwdotorg.org>
To: Thierry Reding <thierry.reding@gmail.com>,
Dmitry Osipenko <digetx@gmail.com>
Cc: Alexandre Courbot <gnurou@gmail.com>,
Peter De Schrijver <pdeschrijver@nvidia.com>,
linux-tegra@vger.kernel.org, linux-kernel@vger.kernel.org,
stable@vger.kernel.org
Subject: Re: [PATCH] ARM: tegra20: Store CPU "resettable" status in IRAM
Date: Mon, 19 Jan 2015 10:26:22 -0700 [thread overview]
Message-ID: <54BD3E3E.2040801@wwwdotorg.org> (raw)
In-Reply-To: <20150119141224.GF23778@ulmo.nvidia.com>
On 01/19/2015 07:12 AM, Thierry Reding wrote:
> On Thu, Jan 15, 2015 at 01:58:57PM +0300, Dmitry Osipenko wrote:
>> Commit 7232398abc6a ("ARM: tegra: Convert PMC to a driver") changed tegra_resume()
>> location storing from late to early and, as a result, broke suspend on Tegra20.
>> PMC scratch register 41 is used by tegra LP1 resume code for retrieving stored
>> physical memory address of common resume function and in the same time used by
>> tegra20_cpu_shutdown() (shared by Tegra20 cpuidle driver and platform SMP code),
>> which is storing CPU1 "resettable" status. It implies strict order of scratch
>> register usage, otherwise resume function address is lost on Tegra20 after
>> disabling non-boot CPU's on suspend. Fix it by storing "resettable" status in
>> IRAM instead of PMC scratch register.
>>
>> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
>> Fixes: 7232398abc6a (ARM: tegra: Convert PMC to a driver)
>> Cc: <stable@vger.kernel.org> # v3.17+
>> ---
>> arch/arm/mach-tegra/cpuidle-tegra20.c | 5 ++---
>> arch/arm/mach-tegra/reset-handler.S | 10 +++++++---
>> arch/arm/mach-tegra/reset.h | 4 ++++
>> arch/arm/mach-tegra/sleep-tegra20.S | 37 ++++++++++++++++++++---------------
>> arch/arm/mach-tegra/sleep.h | 4 ++++
>> 5 files changed, 38 insertions(+), 22 deletions(-)
>
> I'm leaning towards applying this. Stephen, Alex, Peter: any objections?
Hopefully this works out. I suppose it's unlikely anyone will be running
code on the AVP upstrem, so any potential conflict with AVP's usage of
IRAM isn't likely to occur.
__tegra20_cpu1_resettable_status_offset has a lot of _ at the start.
Should the symbol be named more normally? I guess at least it's
consistent with the existing very "underscory"
__tegra_cpu_reset_handler_start.
next prev parent reply other threads:[~2015-01-19 17:26 UTC|newest]
Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-01-15 10:58 Two variants of fixing Tegra20 suspend bug Dmitry Osipenko
2015-01-15 10:58 ` [PATCH] ARM: tegra20: Store CPU "resettable" status in IRAM Dmitry Osipenko
[not found] ` <1421319545-23920-2-git-send-email-digetx-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2015-01-19 14:12 ` Thierry Reding
2015-01-19 14:12 ` Thierry Reding
[not found] ` <20150119141224.GF23778-AwZRO8vwLAwmlAP/+Wk3EA@public.gmane.org>
2015-01-19 15:14 ` Dmitry Osipenko
2015-01-19 15:14 ` Dmitry Osipenko
2015-01-19 17:26 ` Stephen Warren [this message]
2015-01-19 17:26 ` Stephen Warren
2015-01-19 17:41 ` Dmitry Osipenko
2015-01-19 17:45 ` Stephen Warren
2015-01-19 18:00 ` Dmitry Osipenko
[not found] ` <54BD4626.70902-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2015-01-19 18:26 ` Dmitry Osipenko
2015-01-19 18:26 ` Dmitry Osipenko
2015-01-20 16:55 ` Dmitry Osipenko
2015-01-20 1:59 ` Alexandre Courbot
2015-01-20 1:59 ` Alexandre Courbot
2015-03-11 10:29 ` Thierry Reding
2015-03-11 10:29 ` Thierry Reding
2015-03-11 13:28 ` Dmitry Osipenko
2015-01-15 10:58 ` [PATCH] ARM: tegra: Store tegra_resume() address " Dmitry Osipenko
2015-01-19 14:01 ` Thierry Reding
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=54BD3E3E.2040801@wwwdotorg.org \
--to=swarren-3lzwwm7+weoh9zmkesr00q@public.gmane.org \
--cc=digetx-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org \
--cc=gnurou-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org \
--cc=linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
--cc=linux-tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
--cc=pdeschrijver-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org \
--cc=stable-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
--cc=thierry.reding-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.