Re: [PATCH] ARM: tegra20: Store CPU "resettable" status in IRAM
11.03.2015 13:29, Thierry Reding пишет:
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
Fixes: 7232398abc6a (ARM: tegra: Convert PMC to a driver)
Cc: # 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(-)
Sorry for the long delay, but I've applied this now to the Tegra tree.
Thierry
Thanks, now we can move forward.
--
Dmitry
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] ARM: tegra20: Store CPU "resettable" status in IRAM
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
> Fixes: 7232398abc6a (ARM: tegra: Convert PMC to a driver)
> Cc: # 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(-)
Sorry for the long delay, but I've applied this now to the Tegra tree.
Thierry
pgplRTu0EO7qf.pgp
Description: PGP signature
Re: [PATCH] ARM: tegra20: Store CPU resettable status in IRAM
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 dig...@gmail.com Fixes: 7232398abc6a (ARM: tegra: Convert PMC to a driver) Cc: sta...@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(-) Sorry for the long delay, but I've applied this now to the Tegra tree. Thierry pgplRTu0EO7qf.pgp Description: PGP signature
Re: [PATCH] ARM: tegra20: Store CPU resettable status in IRAM
11.03.2015 13:29, Thierry Reding пишет: 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 dig...@gmail.com Fixes: 7232398abc6a (ARM: tegra: Convert PMC to a driver) Cc: sta...@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(-) Sorry for the long delay, but I've applied this now to the Tegra tree. Thierry Thanks, now we can move forward. -- Dmitry -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] ARM: tegra20: Store CPU "resettable" status in IRAM
19.01.2015 21:26, Dmitry Osipenko пишет: 19.01.2015 21:00, Dmitry Osipenko пишет: 19.01.2015 20:45, Stephen Warren пишет: On 01/19/2015 10:41 AM, Dmitry Osipenko wrote: 19.01.2015 20:26, Stephen Warren пишет: 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. I don't see how it can conflict with AVP code. First KB of IRAM is reserved for reset handler. Am I missing something? From reset.h: /* The first 1K of IRAM is permanently reserved for the CPU reset handler */ I believe "CPU" in that context means AVP CPU. Still, I may not be correct, and to be honest it's likely not too well defined even if that comment seems clear-cut. Hmm... Suddenly I recalled that LP2 was always disabled in downstream kernel. I remember that I tried it once (couple years ago) and it didn't work, however I presume it was just broken. Now I don't feel good with it. Can't generic RAM be used for "resettable" status? Or it will be too slow?... CPU1 always come up after CPU0, so RAM is already init'ed. Given that CPU0 can't be halted with running CPU1, I suppose CPU1 can't be booted first, right? Anyway it's not the case for linux. Correcting myself: Well, it's meaningless in case if LP2 cpuidle can't co-exist with AVP firmware. Isn't possible verify it? -- Dmitry -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] ARM: tegra20: Store CPU resettable status in IRAM
19.01.2015 21:26, Dmitry Osipenko пишет: 19.01.2015 21:00, Dmitry Osipenko пишет: 19.01.2015 20:45, Stephen Warren пишет: On 01/19/2015 10:41 AM, Dmitry Osipenko wrote: 19.01.2015 20:26, Stephen Warren пишет: 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. I don't see how it can conflict with AVP code. First KB of IRAM is reserved for reset handler. Am I missing something? From reset.h: /* The first 1K of IRAM is permanently reserved for the CPU reset handler */ I believe CPU in that context means AVP CPU. Still, I may not be correct, and to be honest it's likely not too well defined even if that comment seems clear-cut. Hmm... Suddenly I recalled that LP2 was always disabled in downstream kernel. I remember that I tried it once (couple years ago) and it didn't work, however I presume it was just broken. Now I don't feel good with it. Can't generic RAM be used for resettable status? Or it will be too slow?... CPU1 always come up after CPU0, so RAM is already init'ed. Given that CPU0 can't be halted with running CPU1, I suppose CPU1 can't be booted first, right? Anyway it's not the case for linux. Correcting myself: Well, it's meaningless in case if LP2 cpuidle can't co-exist with AVP firmware. Isn't possible verify it? -- Dmitry -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] ARM: tegra20: Store CPU "resettable" status in IRAM
On Mon, Jan 19, 2015 at 11:12 PM, 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
>> Fixes: 7232398abc6a (ARM: tegra: Convert PMC to a driver)
>> Cc: # 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?
None from me ; but I am not qualified to give an valuable opinion on
this part of the code.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] ARM: tegra20: Store CPU "resettable" status in IRAM
19.01.2015 21:00, Dmitry Osipenko пишет: 19.01.2015 20:45, Stephen Warren пишет: On 01/19/2015 10:41 AM, Dmitry Osipenko wrote: 19.01.2015 20:26, Stephen Warren пишет: 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. I don't see how it can conflict with AVP code. First KB of IRAM is reserved for reset handler. Am I missing something? From reset.h: /* The first 1K of IRAM is permanently reserved for the CPU reset handler */ I believe "CPU" in that context means AVP CPU. Still, I may not be correct, and to be honest it's likely not too well defined even if that comment seems clear-cut. Hmm... Suddenly I recalled that LP2 was always disabled in downstream kernel. I remember that I tried it once (couple years ago) and it didn't work, however I presume it was just broken. Now I don't feel good with it. Can't generic RAM be used for "resettable" status? Or it will be too slow?... CPU1 always come up after CPU0, so RAM is already init'ed. Given that CPU0 can't be halted with running CPU1, I suppose CPU1 can't be booted first, right? Anyway it's not the case for linux. -- Dmitry -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] ARM: tegra20: Store CPU "resettable" status in IRAM
19.01.2015 20:45, Stephen Warren пишет: On 01/19/2015 10:41 AM, Dmitry Osipenko wrote: 19.01.2015 20:26, Stephen Warren пишет: 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. I don't see how it can conflict with AVP code. First KB of IRAM is reserved for reset handler. Am I missing something? From reset.h: /* The first 1K of IRAM is permanently reserved for the CPU reset handler */ I believe "CPU" in that context means AVP CPU. Still, I may not be correct, and to be honest it's likely not too well defined even if that comment seems clear-cut. Hmm... Suddenly I recalled that LP2 was always disabled in downstream kernel. I remember that I tried it once (couple years ago) and it didn't work, however I presume it was just broken. Now I don't feel good with it. -- Dmitry -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] ARM: tegra20: Store CPU "resettable" status in IRAM
On 01/19/2015 10:41 AM, Dmitry Osipenko wrote: 19.01.2015 20:26, Stephen Warren пишет: 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. I don't see how it can conflict with AVP code. First KB of IRAM is reserved for reset handler. Am I missing something? From reset.h: /* The first 1K of IRAM is permanently reserved for the CPU reset handler */ I believe "CPU" in that context means AVP CPU. Still, I may not be correct, and to be honest it's likely not too well defined even if that comment seems clear-cut. __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. I also wasn't happy with "underscory" :) And yes, I left it for consistency. Please feel free to rename it, if needed. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] ARM: tegra20: Store CPU "resettable" status in IRAM
19.01.2015 20:26, Stephen Warren пишет: 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. I don't see how it can conflict with AVP code. First KB of IRAM is reserved for reset handler. Am I missing something? From reset.h: /* The first 1K of IRAM is permanently reserved for the CPU reset handler */ __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. I also wasn't happy with "underscory" :) And yes, I left it for consistency. Please feel free to rename it, if needed. -- Dmitry -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] ARM: tegra20: Store CPU "resettable" status in IRAM
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
Fixes: 7232398abc6a (ARM: tegra: Convert PMC to a driver)
Cc: # 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.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] ARM: tegra20: Store CPU "resettable" status in IRAM
19.01.2015 17:12, Thierry Reding пишет:
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
Fixes: 7232398abc6a (ARM: tegra: Convert PMC to a driver)
Cc: # 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?
Thierry
Btw, I'm preparing cleanup for cpuidle and hotplug code. So this asm "ugliness"
should go away.
--
Dmitry
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] ARM: tegra20: Store CPU "resettable" status in IRAM
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
> Fixes: 7232398abc6a (ARM: tegra: Convert PMC to a driver)
> Cc: # 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?
Thierry
pgpydf6UjGXyE.pgp
Description: PGP signature
Re: [PATCH] ARM: tegra20: Store CPU resettable status in IRAM
On Mon, Jan 19, 2015 at 11:12 PM, Thierry Reding thierry.red...@gmail.com 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 dig...@gmail.com Fixes: 7232398abc6a (ARM: tegra: Convert PMC to a driver) Cc: sta...@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? None from me ; but I am not qualified to give an valuable opinion on this part of the code. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] ARM: tegra20: Store CPU resettable status in IRAM
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 dig...@gmail.com Fixes: 7232398abc6a (ARM: tegra: Convert PMC to a driver) Cc: sta...@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? Thierry pgpydf6UjGXyE.pgp Description: PGP signature
Re: [PATCH] ARM: tegra20: Store CPU resettable status in IRAM
19.01.2015 17:12, Thierry Reding пишет: 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 dig...@gmail.com Fixes: 7232398abc6a (ARM: tegra: Convert PMC to a driver) Cc: sta...@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? Thierry Btw, I'm preparing cleanup for cpuidle and hotplug code. So this asm ugliness should go away. -- Dmitry -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] ARM: tegra20: Store CPU resettable status in IRAM
19.01.2015 20:26, Stephen Warren пишет: 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. I don't see how it can conflict with AVP code. First KB of IRAM is reserved for reset handler. Am I missing something? From reset.h: /* The first 1K of IRAM is permanently reserved for the CPU reset handler */ __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. I also wasn't happy with underscory :) And yes, I left it for consistency. Please feel free to rename it, if needed. -- Dmitry -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] ARM: tegra20: Store CPU resettable status in IRAM
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 dig...@gmail.com Fixes: 7232398abc6a (ARM: tegra: Convert PMC to a driver) Cc: sta...@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. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] ARM: tegra20: Store CPU resettable status in IRAM
On 01/19/2015 10:41 AM, Dmitry Osipenko wrote: 19.01.2015 20:26, Stephen Warren пишет: 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. I don't see how it can conflict with AVP code. First KB of IRAM is reserved for reset handler. Am I missing something? From reset.h: /* The first 1K of IRAM is permanently reserved for the CPU reset handler */ I believe CPU in that context means AVP CPU. Still, I may not be correct, and to be honest it's likely not too well defined even if that comment seems clear-cut. __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. I also wasn't happy with underscory :) And yes, I left it for consistency. Please feel free to rename it, if needed. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] ARM: tegra20: Store CPU resettable status in IRAM
19.01.2015 20:45, Stephen Warren пишет: On 01/19/2015 10:41 AM, Dmitry Osipenko wrote: 19.01.2015 20:26, Stephen Warren пишет: 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. I don't see how it can conflict with AVP code. First KB of IRAM is reserved for reset handler. Am I missing something? From reset.h: /* The first 1K of IRAM is permanently reserved for the CPU reset handler */ I believe CPU in that context means AVP CPU. Still, I may not be correct, and to be honest it's likely not too well defined even if that comment seems clear-cut. Hmm... Suddenly I recalled that LP2 was always disabled in downstream kernel. I remember that I tried it once (couple years ago) and it didn't work, however I presume it was just broken. Now I don't feel good with it. -- Dmitry -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] ARM: tegra20: Store CPU resettable status in IRAM
19.01.2015 21:00, Dmitry Osipenko пишет: 19.01.2015 20:45, Stephen Warren пишет: On 01/19/2015 10:41 AM, Dmitry Osipenko wrote: 19.01.2015 20:26, Stephen Warren пишет: 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. I don't see how it can conflict with AVP code. First KB of IRAM is reserved for reset handler. Am I missing something? From reset.h: /* The first 1K of IRAM is permanently reserved for the CPU reset handler */ I believe CPU in that context means AVP CPU. Still, I may not be correct, and to be honest it's likely not too well defined even if that comment seems clear-cut. Hmm... Suddenly I recalled that LP2 was always disabled in downstream kernel. I remember that I tried it once (couple years ago) and it didn't work, however I presume it was just broken. Now I don't feel good with it. Can't generic RAM be used for resettable status? Or it will be too slow?... CPU1 always come up after CPU0, so RAM is already init'ed. Given that CPU0 can't be halted with running CPU1, I suppose CPU1 can't be booted first, right? Anyway it's not the case for linux. -- Dmitry -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] ARM: tegra20: Store CPU "resettable" status in IRAM
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
Fixes: 7232398abc6a (ARM: tegra: Convert PMC to a driver)
Cc: # 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(-)
diff --git a/arch/arm/mach-tegra/cpuidle-tegra20.c
b/arch/arm/mach-tegra/cpuidle-tegra20.c
index b30bf5c..f209e9c 100644
--- a/arch/arm/mach-tegra/cpuidle-tegra20.c
+++ b/arch/arm/mach-tegra/cpuidle-tegra20.c
@@ -35,6 +35,7 @@
#include "iomap.h"
#include "irq.h"
#include "pm.h"
+#include "reset.h"
#include "sleep.h"
#ifdef CONFIG_PM_SLEEP
@@ -72,15 +73,13 @@ static struct cpuidle_driver tegra_idle_driver = {
#ifdef CONFIG_PM_SLEEP
#ifdef CONFIG_SMP
-static void __iomem *pmc = IO_ADDRESS(TEGRA_PMC_BASE);
-
static int tegra20_reset_sleeping_cpu_1(void)
{
int ret = 0;
tegra_pen_lock();
- if (readl(pmc + PMC_SCRATCH41) == CPU_RESETTABLE)
+ if (readb(tegra20_cpu1_resettable_status) == CPU_RESETTABLE)
tegra20_cpu_shutdown(1);
else
ret = -EINVAL;
diff --git a/arch/arm/mach-tegra/reset-handler.S
b/arch/arm/mach-tegra/reset-handler.S
index 7b2baab..60e9add 100644
--- a/arch/arm/mach-tegra/reset-handler.S
+++ b/arch/arm/mach-tegra/reset-handler.S
@@ -168,10 +168,10 @@ after_errata:
cmp r6, #TEGRA20
bne 1f
/* If not CPU0, don't let CPU0 reset CPU1 now that CPU1 is coming up. */
- mov32 r5, TEGRA_PMC_BASE
- mov r0, #0
+ mov32 r5, TEGRA_IRAM_BASE + TEGRA_IRAM_RESET_HANDLER_OFFSET
+ mov r0, #CPU_NOT_RESETTABLE
cmp r10, #0
- strne r0, [r5, #PMC_SCRATCH41]
+ strneb r0, [r5, #__tegra20_cpu1_resettable_status_offset]
1:
#endif
@@ -280,6 +280,10 @@ __tegra_cpu_reset_handler_data:
.rept TEGRA_RESET_DATA_SIZE
.long 0
.endr
+ .globl __tegra20_cpu1_resettable_status_offset
+ .equ__tegra20_cpu1_resettable_status_offset, \
+ . - __tegra_cpu_reset_handler_start
+ .byte 0
.align L1_CACHE_SHIFT
ENTRY(__tegra_cpu_reset_handler_end)
diff --git a/arch/arm/mach-tegra/reset.h b/arch/arm/mach-tegra/reset.h
index 76a9343..29c3dec 100644
--- a/arch/arm/mach-tegra/reset.h
+++ b/arch/arm/mach-tegra/reset.h
@@ -35,6 +35,7 @@ extern unsigned long
__tegra_cpu_reset_handler_data[TEGRA_RESET_DATA_SIZE];
void __tegra_cpu_reset_handler_start(void);
void __tegra_cpu_reset_handler(void);
+void __tegra20_cpu1_resettable_status_offset(void);
void __tegra_cpu_reset_handler_end(void);
void tegra_secondary_startup(void);
@@ -47,6 +48,9 @@ void tegra_secondary_startup(void);
(IO_ADDRESS(TEGRA_IRAM_BASE + TEGRA_IRAM_RESET_HANDLER_OFFSET + \
((u32)&__tegra_cpu_reset_handler_data[TEGRA_RESET_MASK_LP2] - \
(u32)__tegra_cpu_reset_handler_start)))
+#define tegra20_cpu1_resettable_status \
+ (IO_ADDRESS(TEGRA_IRAM_BASE + TEGRA_IRAM_RESET_HANDLER_OFFSET + \
+(u32)__tegra20_cpu1_resettable_status_offset))
#endif
#define tegra_cpu_reset_handler_offset \
diff --git a/arch/arm/mach-tegra/sleep-tegra20.S
b/arch/arm/mach-tegra/sleep-tegra20.S
index be4bc5f..e6b684e 100644
--- a/arch/arm/mach-tegra/sleep-tegra20.S
+++ b/arch/arm/mach-tegra/sleep-tegra20.S
@@ -97,9 +97,10 @@ ENDPROC(tegra20_hotplug_shutdown)
ENTRY(tegra20_cpu_shutdown)
cmp r0, #0
reteq lr @ must not be called for CPU 0
- mov32 r1, TEGRA_PMC_VIRT + PMC_SCRATCH41
+ mov32 r1, TEGRA_IRAM_RESET_BASE_VIRT
+ ldr r2, =__tegra20_cpu1_resettable_status_offset
mov r12, #CPU_RESETTABLE
- str r12, [r1]
+ strbr12, [r1, r2]
cpu_to_halt_reg r1, r0
ldr r3, =TEGRA_FLOW_CTRL_VIRT
@@ -182,38 +183,41 @@ ENDPROC(tegra_pen_unlock)
/*
* tegra20_cpu_clear_resettable(void)
*
- * Called to clear the "resettable soon" flag in PMC_SCRATCH41 when
+ * Called to clear the "resettable soon" flag in IRAM variable when
* it is expected that the secondary CPU will be idle soon.
*/
[PATCH] ARM: tegra20: Store CPU resettable status in IRAM
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 dig...@gmail.com
Fixes: 7232398abc6a (ARM: tegra: Convert PMC to a driver)
Cc: sta...@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(-)
diff --git a/arch/arm/mach-tegra/cpuidle-tegra20.c
b/arch/arm/mach-tegra/cpuidle-tegra20.c
index b30bf5c..f209e9c 100644
--- a/arch/arm/mach-tegra/cpuidle-tegra20.c
+++ b/arch/arm/mach-tegra/cpuidle-tegra20.c
@@ -35,6 +35,7 @@
#include iomap.h
#include irq.h
#include pm.h
+#include reset.h
#include sleep.h
#ifdef CONFIG_PM_SLEEP
@@ -72,15 +73,13 @@ static struct cpuidle_driver tegra_idle_driver = {
#ifdef CONFIG_PM_SLEEP
#ifdef CONFIG_SMP
-static void __iomem *pmc = IO_ADDRESS(TEGRA_PMC_BASE);
-
static int tegra20_reset_sleeping_cpu_1(void)
{
int ret = 0;
tegra_pen_lock();
- if (readl(pmc + PMC_SCRATCH41) == CPU_RESETTABLE)
+ if (readb(tegra20_cpu1_resettable_status) == CPU_RESETTABLE)
tegra20_cpu_shutdown(1);
else
ret = -EINVAL;
diff --git a/arch/arm/mach-tegra/reset-handler.S
b/arch/arm/mach-tegra/reset-handler.S
index 7b2baab..60e9add 100644
--- a/arch/arm/mach-tegra/reset-handler.S
+++ b/arch/arm/mach-tegra/reset-handler.S
@@ -168,10 +168,10 @@ after_errata:
cmp r6, #TEGRA20
bne 1f
/* If not CPU0, don't let CPU0 reset CPU1 now that CPU1 is coming up. */
- mov32 r5, TEGRA_PMC_BASE
- mov r0, #0
+ mov32 r5, TEGRA_IRAM_BASE + TEGRA_IRAM_RESET_HANDLER_OFFSET
+ mov r0, #CPU_NOT_RESETTABLE
cmp r10, #0
- strne r0, [r5, #PMC_SCRATCH41]
+ strneb r0, [r5, #__tegra20_cpu1_resettable_status_offset]
1:
#endif
@@ -280,6 +280,10 @@ __tegra_cpu_reset_handler_data:
.rept TEGRA_RESET_DATA_SIZE
.long 0
.endr
+ .globl __tegra20_cpu1_resettable_status_offset
+ .equ__tegra20_cpu1_resettable_status_offset, \
+ . - __tegra_cpu_reset_handler_start
+ .byte 0
.align L1_CACHE_SHIFT
ENTRY(__tegra_cpu_reset_handler_end)
diff --git a/arch/arm/mach-tegra/reset.h b/arch/arm/mach-tegra/reset.h
index 76a9343..29c3dec 100644
--- a/arch/arm/mach-tegra/reset.h
+++ b/arch/arm/mach-tegra/reset.h
@@ -35,6 +35,7 @@ extern unsigned long
__tegra_cpu_reset_handler_data[TEGRA_RESET_DATA_SIZE];
void __tegra_cpu_reset_handler_start(void);
void __tegra_cpu_reset_handler(void);
+void __tegra20_cpu1_resettable_status_offset(void);
void __tegra_cpu_reset_handler_end(void);
void tegra_secondary_startup(void);
@@ -47,6 +48,9 @@ void tegra_secondary_startup(void);
(IO_ADDRESS(TEGRA_IRAM_BASE + TEGRA_IRAM_RESET_HANDLER_OFFSET + \
((u32)__tegra_cpu_reset_handler_data[TEGRA_RESET_MASK_LP2] - \
(u32)__tegra_cpu_reset_handler_start)))
+#define tegra20_cpu1_resettable_status \
+ (IO_ADDRESS(TEGRA_IRAM_BASE + TEGRA_IRAM_RESET_HANDLER_OFFSET + \
+(u32)__tegra20_cpu1_resettable_status_offset))
#endif
#define tegra_cpu_reset_handler_offset \
diff --git a/arch/arm/mach-tegra/sleep-tegra20.S
b/arch/arm/mach-tegra/sleep-tegra20.S
index be4bc5f..e6b684e 100644
--- a/arch/arm/mach-tegra/sleep-tegra20.S
+++ b/arch/arm/mach-tegra/sleep-tegra20.S
@@ -97,9 +97,10 @@ ENDPROC(tegra20_hotplug_shutdown)
ENTRY(tegra20_cpu_shutdown)
cmp r0, #0
reteq lr @ must not be called for CPU 0
- mov32 r1, TEGRA_PMC_VIRT + PMC_SCRATCH41
+ mov32 r1, TEGRA_IRAM_RESET_BASE_VIRT
+ ldr r2, =__tegra20_cpu1_resettable_status_offset
mov r12, #CPU_RESETTABLE
- str r12, [r1]
+ strbr12, [r1, r2]
cpu_to_halt_reg r1, r0
ldr r3, =TEGRA_FLOW_CTRL_VIRT
@@ -182,38 +183,41 @@ ENDPROC(tegra_pen_unlock)
/*
* tegra20_cpu_clear_resettable(void)
*
- * Called to clear the resettable soon flag in PMC_SCRATCH41 when
+ * Called to clear the resettable soon flag in IRAM variable when
* it is expected that the secondary CPU will be
