| Message ID | 1433272378-8470-2-git-send-email-sboyd@codeaurora.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, Jun 02, 2015 at 12:12:57PM -0700, Stephen Boyd wrote: > Some platforms always enter the kernel in the ARM state even if > the kernel is compiled for THUMB2. Add a small wrapper on top of > cpu_resume() that switches into THUMB2 state. > > This fixes a problem reported by Kevin Hilman on next-20150601 > where the ifc6410 fails to boot a THUMB2 kernel because the > platform's firmware always enters the kernel in ARM mode from > deep idle states. I think this bit of the commit message isn't quite correct: this will only resolve the boot problem with a T2 kernel when the failing platform is converted to use the cpu_resume_arm() entry point. Apart from that, the patch looks good to me, thanks.
On 06/03, Russell King - ARM Linux wrote: > On Tue, Jun 02, 2015 at 12:12:57PM -0700, Stephen Boyd wrote: > > Some platforms always enter the kernel in the ARM state even if > > the kernel is compiled for THUMB2. Add a small wrapper on top of > > cpu_resume() that switches into THUMB2 state. > > > > This fixes a problem reported by Kevin Hilman on next-20150601 > > where the ifc6410 fails to boot a THUMB2 kernel because the > > platform's firmware always enters the kernel in ARM mode from > > deep idle states. > > I think this bit of the commit message isn't quite correct: this will > only resolve the boot problem with a T2 kernel when the failing platform > is converted to use the cpu_resume_arm() entry point. > > Apart from that, the patch looks good to me, thanks. Sure, it could say "This provides the functionality to fix a problem reported by..."
Stephen Boyd <sboyd@codeaurora.org> writes: > On 06/03, Russell King - ARM Linux wrote: >> On Tue, Jun 02, 2015 at 12:12:57PM -0700, Stephen Boyd wrote: >> > Some platforms always enter the kernel in the ARM state even if >> > the kernel is compiled for THUMB2. Add a small wrapper on top of >> > cpu_resume() that switches into THUMB2 state. >> > >> > This fixes a problem reported by Kevin Hilman on next-20150601 >> > where the ifc6410 fails to boot a THUMB2 kernel because the >> > platform's firmware always enters the kernel in ARM mode from >> > deep idle states. >> >> I think this bit of the commit message isn't quite correct: this will >> only resolve the boot problem with a T2 kernel when the failing platform >> is converted to use the cpu_resume_arm() entry point. >> >> Apart from that, the patch looks good to me, thanks. > > Sure, it could say "This provides the functionality to fix a problem > reported by..." Stephen, are you planning to submit this through Russell's patch tracker? or... Russell, should we take this through arm-soc along with PATCH 2/2 which depends on this one? Kevin
On 06/08/2015 02:33 PM, Kevin Hilman wrote: > Stephen Boyd <sboyd@codeaurora.org> writes: > >> On 06/03, Russell King - ARM Linux wrote: >>> On Tue, Jun 02, 2015 at 12:12:57PM -0700, Stephen Boyd wrote: >>>> Some platforms always enter the kernel in the ARM state even if >>>> the kernel is compiled for THUMB2. Add a small wrapper on top of >>>> cpu_resume() that switches into THUMB2 state. >>>> >>>> This fixes a problem reported by Kevin Hilman on next-20150601 >>>> where the ifc6410 fails to boot a THUMB2 kernel because the >>>> platform's firmware always enters the kernel in ARM mode from >>>> deep idle states. >>> I think this bit of the commit message isn't quite correct: this will >>> only resolve the boot problem with a T2 kernel when the failing platform >>> is converted to use the cpu_resume_arm() entry point. >>> >>> Apart from that, the patch looks good to me, thanks. >> Sure, it could say "This provides the functionality to fix a problem >> reported by..." > Stephen, are you planning to submit this through Russell's patch > tracker? or... > > Russell, should we take this through arm-soc along with PATCH 2/2 which > depends on this one? > I'm happy to see it be applied by arm-soc maintainers directly given that Russell said "the patch looks good to me". The commit text needs a slight reword here though, so I can resubmit the patches to arm-soc if you like.
Stephen Boyd <sboyd@codeaurora.org> writes: > On 06/08/2015 02:33 PM, Kevin Hilman wrote: >> Stephen Boyd <sboyd@codeaurora.org> writes: >> >>> On 06/03, Russell King - ARM Linux wrote: >>>> On Tue, Jun 02, 2015 at 12:12:57PM -0700, Stephen Boyd wrote: >>>>> Some platforms always enter the kernel in the ARM state even if >>>>> the kernel is compiled for THUMB2. Add a small wrapper on top of >>>>> cpu_resume() that switches into THUMB2 state. >>>>> >>>>> This fixes a problem reported by Kevin Hilman on next-20150601 >>>>> where the ifc6410 fails to boot a THUMB2 kernel because the >>>>> platform's firmware always enters the kernel in ARM mode from >>>>> deep idle states. >>>> I think this bit of the commit message isn't quite correct: this will >>>> only resolve the boot problem with a T2 kernel when the failing platform >>>> is converted to use the cpu_resume_arm() entry point. >>>> >>>> Apart from that, the patch looks good to me, thanks. >>> Sure, it could say "This provides the functionality to fix a problem >>> reported by..." >> Stephen, are you planning to submit this through Russell's patch >> tracker? or... >> >> Russell, should we take this through arm-soc along with PATCH 2/2 which >> depends on this one? >> > > I'm happy to see it be applied by arm-soc maintainers directly given > that Russell said "the patch looks good to me". The commit text needs a > slight reword here though, so I can resubmit the patches to arm-soc if > you like. Go ahead and respin/resubmit, and I can get them ready, but I'll wait for an official word or acked-by from Russell before taking them both through arm-soc. Kevin
On Mon, Jun 08, 2015 at 03:03:04PM -0700, Kevin Hilman wrote: > Stephen Boyd <sboyd@codeaurora.org> writes: > > I'm happy to see it be applied by arm-soc maintainers directly given > > that Russell said "the patch looks good to me". The commit text needs a > > slight reword here though, so I can resubmit the patches to arm-soc if > > you like. > > Go ahead and respin/resubmit, and I can get them ready, but I'll wait > for an official word or acked-by from Russell before taking them both > through arm-soc. I've suggested to Kevin that, as I have another patch outstanding for arm-soc, that I take this one into that same branch and send Kevin a pull for both patches. (Esp. as I don't like sending single-patch pull requests. Having two patches solves my dilema!) Stephen, if you can arrange for the patch to end up in the patch system please... Thanks.
On 06/09/2015 08:29 AM, Russell King - ARM Linux wrote: > > Stephen, if you can arrange for the patch to end up in the patch system > please... > Sure no problem. The patch is 8389/1 in the tracker. I also fixed the commit text.
Hello, On Tue, Jun 02, 2015 at 12:12:57PM -0700, Stephen Boyd wrote: > Some platforms always enter the kernel in the ARM state even if > the kernel is compiled for THUMB2. Add a small wrapper on top of > cpu_resume() that switches into THUMB2 state. > > This fixes a problem reported by Kevin Hilman on next-20150601 > where the ifc6410 fails to boot a THUMB2 kernel because the > platform's firmware always enters the kernel in ARM mode from > deep idle states. > > Reported-by: Kevin Hilman <khilman@linaro.org> > Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org> > Cc: Lina Iyer <lina.iyer@linaro.org> > Signed-off-by: Stephen Boyd <sboyd@codeaurora.org> > diff --git a/arch/arm/kernel/sleep.S b/arch/arm/kernel/sleep.S > index 76bb3128e135..f37593567ef5 100644 > --- a/arch/arm/kernel/sleep.S > +++ b/arch/arm/kernel/sleep.S > @@ -118,6 +118,12 @@ ENDPROC(cpu_resume_after_mmu) > > .text > .align > + .arm > +ENTRY(cpu_resume_arm) > + THUMB( badr r9, 1f ) @ Kernel is entered in ARM. > + THUMB( bx r9 ) @ If this is a Thumb-2 kernel, > + THUMB( .thumb ) @ switch to Thumb now. > + THUMB(1: ) > ENTRY(cpu_resume) > ARM_BE8(setend be) @ ensure we are in BE mode > #ifdef CONFIG_ARM_VIRT_EXT this patch is in next as 51ac91b7f6b11b0da55ac93885ee7b864865bcb1 and breaks efm32_defconfig. The exact error message is: AS arch/arm/kernel/sleep.o arch/arm/kernel/sleep.S: Assembler messages: arch/arm/kernel/sleep.S:121: Error: selected processor does not support ARM opcodes arch/arm/kernel/sleep.S:123: Error: bad instruction `badr r9,1f' arch/arm/kernel/sleep.S:124: Error: attempt to use an ARM instruction on a Thumb-only processor -- `bx r9' scripts/Makefile.build:294: recipe for target 'arch/arm/kernel/sleep.o' failed make[3]: *** [arch/arm/kernel/sleep.o] Error 1 Best regards Uwe
On Mon, Jun 15, 2015 at 08:33:25AM +0200, Uwe Kleine-König wrote: > Hello, > > + .arm > > +ENTRY(cpu_resume_arm) > > + THUMB( badr r9, 1f ) @ Kernel is entered in ARM. > > + THUMB( bx r9 ) @ If this is a Thumb-2 kernel, > > + THUMB( .thumb ) @ switch to Thumb now. > > + THUMB(1: ) > > ENTRY(cpu_resume) > > ARM_BE8(setend be) @ ensure we are in BE mode > > #ifdef CONFIG_ARM_VIRT_EXT > this patch is in next as 51ac91b7f6b11b0da55ac93885ee7b864865bcb1 and > breaks efm32_defconfig. The exact error message is: > > AS arch/arm/kernel/sleep.o > arch/arm/kernel/sleep.S: Assembler messages: > arch/arm/kernel/sleep.S:121: Error: selected processor does not support ARM opcodes > arch/arm/kernel/sleep.S:123: Error: bad instruction `badr r9,1f' > arch/arm/kernel/sleep.S:124: Error: attempt to use an ARM instruction on a Thumb-only processor -- `bx r9' > scripts/Makefile.build:294: recipe for target 'arch/arm/kernel/sleep.o' failed > make[3]: *** [arch/arm/kernel/sleep.o] Error 1 Don't get me wrong, the build testing which goes on is really great, but there's now a problem with all this. There needs to be coordination between the people doing the build tests to ensure that we don't "tire out" those who read the emails with different groups of people reporting the same problem days after it was first discovered, and even worse, days after it's been fixed. The worst thing to do is to report build regressions on Monday for a tree which was created on Thursday - by the time Monday comes around, projects such as the 0-day builder have had plenty of time to find them. Remember, the linux-next tree which is published on Friday (Austrailian time) is a result of git trees snapshotted around midnight on Thursday. So, if you're going to build an old linux-next tree, before you report any problems, please check whether other build systems have already reported them. If you've gone to the trouble of tracking down the commit which caused it, and therefore the likely git tree, check whether a fix has already been merged. Or maybe wait until the post-weekend linux-next is published... (The problem you're referring to was fixed and pushed out on Friday - which seems to be a regular thing that happens with problems you've reported on Mondays...)
Hello Russell, On Mon, Jun 15, 2015 at 12:01:36PM +0100, Russell King - ARM Linux wrote: > On Mon, Jun 15, 2015 at 08:33:25AM +0200, Uwe Kleine-König wrote: > > Hello, > > > + .arm > > > +ENTRY(cpu_resume_arm) > > > + THUMB( badr r9, 1f ) @ Kernel is entered in ARM. > > > + THUMB( bx r9 ) @ If this is a Thumb-2 kernel, > > > + THUMB( .thumb ) @ switch to Thumb now. > > > + THUMB(1: ) > > > ENTRY(cpu_resume) > > > ARM_BE8(setend be) @ ensure we are in BE mode > > > #ifdef CONFIG_ARM_VIRT_EXT > > this patch is in next as 51ac91b7f6b11b0da55ac93885ee7b864865bcb1 and > > breaks efm32_defconfig. The exact error message is: > > > > AS arch/arm/kernel/sleep.o > > arch/arm/kernel/sleep.S: Assembler messages: > > arch/arm/kernel/sleep.S:121: Error: selected processor does not support ARM opcodes > > arch/arm/kernel/sleep.S:123: Error: bad instruction `badr r9,1f' > > arch/arm/kernel/sleep.S:124: Error: attempt to use an ARM instruction on a Thumb-only processor -- `bx r9' > > scripts/Makefile.build:294: recipe for target 'arch/arm/kernel/sleep.o' failed > > make[3]: *** [arch/arm/kernel/sleep.o] Error 1 > > Don't get me wrong, the build testing which goes on is really great, but > there's now a problem with all this. > > There needs to be coordination between the people doing the build tests > to ensure that we don't "tire out" those who read the emails with different > groups of people reporting the same problem days after it was first > discovered, and even worse, days after it's been fixed. > > The worst thing to do is to report build regressions on Monday for a tree > which was created on Thursday - by the time Monday comes around, projects > such as the 0-day builder have had plenty of time to find them. > > Remember, the linux-next tree which is published on Friday (Austrailian > time) is a result of git trees snapshotted around midnight on Thursday. > > So, if you're going to build an old linux-next tree, before you report any > problems, please check whether other build systems have already reported > them. If you've gone to the trouble of tracking down the commit which > caused it, and therefore the likely git tree, check whether a fix has > already been merged. Or maybe wait until the post-weekend linux-next is > published... > > (The problem you're referring to was fixed and pushed out on Friday - > which seems to be a regular thing that happens with problems you've > reported on Mondays...) I consulted Google (asking for the commit id) and checked the linux-arm-kernel list for reports which usually works well enough. But I understand your complaint, last time I introduced a build regression I got two automatic reports and three times the same fix into my mailbox. So I will try to be more careful in the future. Best regards Uwe
diff --git a/arch/arm/include/asm/suspend.h b/arch/arm/include/asm/suspend.h index cd20029bcd94..6c7182f32cef 100644 --- a/arch/arm/include/asm/suspend.h +++ b/arch/arm/include/asm/suspend.h @@ -7,6 +7,7 @@ struct sleep_save_sp { }; extern void cpu_resume(void); +extern void cpu_resume_arm(void); extern int cpu_suspend(unsigned long, int (*)(unsigned long)); #endif diff --git a/arch/arm/kernel/sleep.S b/arch/arm/kernel/sleep.S index 76bb3128e135..f37593567ef5 100644 --- a/arch/arm/kernel/sleep.S +++ b/arch/arm/kernel/sleep.S @@ -118,6 +118,12 @@ ENDPROC(cpu_resume_after_mmu) .text .align + .arm +ENTRY(cpu_resume_arm) + THUMB( badr r9, 1f ) @ Kernel is entered in ARM. + THUMB( bx r9 ) @ If this is a Thumb-2 kernel, + THUMB( .thumb ) @ switch to Thumb now. + THUMB(1: ) ENTRY(cpu_resume) ARM_BE8(setend be) @ ensure we are in BE mode #ifdef CONFIG_ARM_VIRT_EXT @@ -149,6 +155,7 @@ THUMB( ldmia r0!, {r1, r2, r3} ) THUMB( mov sp, r2 ) THUMB( bx r3 ) ENDPROC(cpu_resume) +ENDPROC(cpu_resume_arm) .align 2 _sleep_save_sp:
Some platforms always enter the kernel in the ARM state even if the kernel is compiled for THUMB2. Add a small wrapper on top of cpu_resume() that switches into THUMB2 state. This fixes a problem reported by Kevin Hilman on next-20150601 where the ifc6410 fails to boot a THUMB2 kernel because the platform's firmware always enters the kernel in ARM mode from deep idle states. Reported-by: Kevin Hilman <khilman@linaro.org> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org> Cc: Lina Iyer <lina.iyer@linaro.org> Signed-off-by: Stephen Boyd <sboyd@codeaurora.org> --- arch/arm/include/asm/suspend.h | 1 + arch/arm/kernel/sleep.S | 7 +++++++ 2 files changed, 8 insertions(+)