* [PATCH 0/2] additional vexpress-tc2 big endian fixes
@ 2013-10-08 4:37 Victor Kamensky
2013-10-08 4:37 ` [PATCH 1/2] ARM: mcpm: fix big endian issue in mcpm startup code Victor Kamensky
` (2 more replies)
0 siblings, 3 replies; 10+ messages in thread
From: Victor Kamensky @ 2013-10-08 4:37 UTC (permalink / raw)
To: linux-arm-kernel
Hi Ben,
Here are couple more big endian related fixes. We run into these
with vexpress TC2 board, when CONFIG_MCPM and CONFIG_ARM_CCI configs
are enabled. Big endian image fails to boot. With fixes BE TC2
boots and it sees all 5 cores (2xA15, 3xA7).
These were tested with vexpress TC2 on latest linux-linaro
branch (include BE patch series) and it was tested on very
recent rmk/fixes branch along with BE series on top of it.
Thanks,
Victor
Victor Kamensky (2):
ARM: mcpm: fix big endian issue in mcpm startup code
ARM: cci: driver need big endian fixes in asm code
arch/arm/common/mcpm_head.S | 2 ++
drivers/bus/arm-cci.c | 6 ++++++
2 files changed, 8 insertions(+)
--
1.8.1.4
^ permalink raw reply [flat|nested] 10+ messages in thread* [PATCH 1/2] ARM: mcpm: fix big endian issue in mcpm startup code 2013-10-08 4:37 [PATCH 0/2] additional vexpress-tc2 big endian fixes Victor Kamensky @ 2013-10-08 4:37 ` Victor Kamensky 2013-10-08 4:37 ` [PATCH 2/2] ARM: cci: driver need big endian fixes in asm code Victor Kamensky 2013-10-08 7:27 ` [PATCH 0/2] additional vexpress-tc2 big endian fixes Ben Dooks 2 siblings, 0 replies; 10+ messages in thread From: Victor Kamensky @ 2013-10-08 4:37 UTC (permalink / raw) To: linux-arm-kernel In big endian mode mcpm_entry_point is first function that called on secondaries CPU. First it should switch CPU into big endian code. Signed-off-by: Victor Kamensky <victor.kamensky@linaro.org> --- arch/arm/common/mcpm_head.S | 2 ++ 1 file changed, 2 insertions(+) diff --git a/arch/arm/common/mcpm_head.S b/arch/arm/common/mcpm_head.S index 39c96df..4f88f5e 100644 --- a/arch/arm/common/mcpm_head.S +++ b/arch/arm/common/mcpm_head.S @@ -15,6 +15,7 @@ #include <linux/linkage.h> #include <asm/mcpm.h> +#include <asm/assembler.h> #include "vlock.h" @@ -47,6 +48,7 @@ ENTRY(mcpm_entry_point) + ARM_BE8(setend be) THUMB( adr r12, BSYM(1f) ) THUMB( bx r12 ) THUMB( .thumb ) -- 1.8.1.4 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 2/2] ARM: cci: driver need big endian fixes in asm code 2013-10-08 4:37 [PATCH 0/2] additional vexpress-tc2 big endian fixes Victor Kamensky 2013-10-08 4:37 ` [PATCH 1/2] ARM: mcpm: fix big endian issue in mcpm startup code Victor Kamensky @ 2013-10-08 4:37 ` Victor Kamensky 2013-10-08 17:51 ` Dave Martin 2013-10-08 7:27 ` [PATCH 0/2] additional vexpress-tc2 big endian fixes Ben Dooks 2 siblings, 1 reply; 10+ messages in thread From: Victor Kamensky @ 2013-10-08 4:37 UTC (permalink / raw) To: linux-arm-kernel cci_enable_port_for_self written in asm and it works with h/w registers that are in little endian format. When run in big endian mode it needs byte swap before/after it writes/reads to/from such registers Signed-off-by: Victor Kamensky <victor.kamensky@linaro.org> --- drivers/bus/arm-cci.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/drivers/bus/arm-cci.c b/drivers/bus/arm-cci.c index 2009266..6db173e 100644 --- a/drivers/bus/arm-cci.c +++ b/drivers/bus/arm-cci.c @@ -281,6 +281,9 @@ asmlinkage void __naked cci_enable_port_for_self(void) /* Enable the CCI port */ " ldr r0, [r0, %[offsetof_port_phys]] \n" " mov r3, #"__stringify(CCI_ENABLE_REQ)" \n" +#ifdef __ARMEB__ +" rev r3, r3 \n" +#endif /* __ARMEB__ */ " str r3, [r0, #"__stringify(CCI_PORT_CTRL)"] \n" /* poll the status reg for completion */ @@ -288,6 +291,9 @@ asmlinkage void __naked cci_enable_port_for_self(void) " ldr r0, [r1] \n" " ldr r0, [r0, r1] @ cci_ctrl_base \n" "4: ldr r1, [r0, #"__stringify(CCI_CTRL_STATUS)"] \n" +#ifdef __ARMEB__ +" rev r1, r1 \n" +#endif /* __ARMEB__ */ " tst r1, #1 \n" " bne 4b \n" -- 1.8.1.4 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 2/2] ARM: cci: driver need big endian fixes in asm code 2013-10-08 4:37 ` [PATCH 2/2] ARM: cci: driver need big endian fixes in asm code Victor Kamensky @ 2013-10-08 17:51 ` Dave Martin 2013-10-08 17:57 ` Nicolas Pitre 0 siblings, 1 reply; 10+ messages in thread From: Dave Martin @ 2013-10-08 17:51 UTC (permalink / raw) To: linux-arm-kernel On Mon, Oct 07, 2013 at 09:37:20PM -0700, Victor Kamensky wrote: > cci_enable_port_for_self written in asm and it works with h/w > registers that are in little endian format. When run in big > endian mode it needs byte swap before/after it writes/reads > to/from such registers Lorenzo should comment on this if he's not already seen it. > > Signed-off-by: Victor Kamensky <victor.kamensky@linaro.org> > --- > drivers/bus/arm-cci.c | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/drivers/bus/arm-cci.c b/drivers/bus/arm-cci.c > index 2009266..6db173e 100644 > --- a/drivers/bus/arm-cci.c > +++ b/drivers/bus/arm-cci.c > @@ -281,6 +281,9 @@ asmlinkage void __naked cci_enable_port_for_self(void) > /* Enable the CCI port */ > " ldr r0, [r0, %[offsetof_port_phys]] \n" > " mov r3, #"__stringify(CCI_ENABLE_REQ)" \n" > +#ifdef __ARMEB__ > +" rev r3, r3 \n" > +#endif /* __ARMEB__ */ Hmmm, this is a bit ugly, but we can't easily use ARM_BE8() here. That would be more easily solved if this was a separate .S file... Maybe it's not worth it. Cheers ---Dave > " str r3, [r0, #"__stringify(CCI_PORT_CTRL)"] \n" > > /* poll the status reg for completion */ > @@ -288,6 +291,9 @@ asmlinkage void __naked cci_enable_port_for_self(void) > " ldr r0, [r1] \n" > " ldr r0, [r0, r1] @ cci_ctrl_base \n" > "4: ldr r1, [r0, #"__stringify(CCI_CTRL_STATUS)"] \n" > +#ifdef __ARMEB__ > +" rev r1, r1 \n" > +#endif /* __ARMEB__ */ > " tst r1, #1 \n" > " bne 4b \n" > > -- > 1.8.1.4 > > > _______________________________________________ > linaro-kernel mailing list > linaro-kernel at lists.linaro.org > http://lists.linaro.org/mailman/listinfo/linaro-kernel ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 2/2] ARM: cci: driver need big endian fixes in asm code 2013-10-08 17:51 ` Dave Martin @ 2013-10-08 17:57 ` Nicolas Pitre 2013-10-08 18:03 ` Dave Martin 2013-10-08 18:14 ` Victor Kamensky 0 siblings, 2 replies; 10+ messages in thread From: Nicolas Pitre @ 2013-10-08 17:57 UTC (permalink / raw) To: linux-arm-kernel On Tue, 8 Oct 2013, Dave Martin wrote: > On Mon, Oct 07, 2013 at 09:37:20PM -0700, Victor Kamensky wrote: > > cci_enable_port_for_self written in asm and it works with h/w > > registers that are in little endian format. When run in big > > endian mode it needs byte swap before/after it writes/reads > > to/from such registers > > Lorenzo should comment on this if he's not already seen it. > > > > > Signed-off-by: Victor Kamensky <victor.kamensky@linaro.org> > > --- > > drivers/bus/arm-cci.c | 6 ++++++ > > 1 file changed, 6 insertions(+) > > > > diff --git a/drivers/bus/arm-cci.c b/drivers/bus/arm-cci.c > > index 2009266..6db173e 100644 > > --- a/drivers/bus/arm-cci.c > > +++ b/drivers/bus/arm-cci.c > > @@ -281,6 +281,9 @@ asmlinkage void __naked cci_enable_port_for_self(void) > > /* Enable the CCI port */ > > " ldr r0, [r0, %[offsetof_port_phys]] \n" > > " mov r3, #"__stringify(CCI_ENABLE_REQ)" \n" > > +#ifdef __ARMEB__ > > +" rev r3, r3 \n" > > +#endif /* __ARMEB__ */ > > Hmmm, this is a bit ugly, but we can't easily use ARM_BE8() here. In any case, it would be more efficient to swap the CCI_ENABLE_REQ constant at compile time rather than doing it at run time with an extra instruction. Nicolas ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 2/2] ARM: cci: driver need big endian fixes in asm code 2013-10-08 17:57 ` Nicolas Pitre @ 2013-10-08 18:03 ` Dave Martin 2013-10-08 18:14 ` Victor Kamensky 1 sibling, 0 replies; 10+ messages in thread From: Dave Martin @ 2013-10-08 18:03 UTC (permalink / raw) To: linux-arm-kernel On Tue, Oct 08, 2013 at 01:57:48PM -0400, Nicolas Pitre wrote: > On Tue, 8 Oct 2013, Dave Martin wrote: > > > On Mon, Oct 07, 2013 at 09:37:20PM -0700, Victor Kamensky wrote: > > > cci_enable_port_for_self written in asm and it works with h/w > > > registers that are in little endian format. When run in big > > > endian mode it needs byte swap before/after it writes/reads > > > to/from such registers > > > > Lorenzo should comment on this if he's not already seen it. > > > > > > > > Signed-off-by: Victor Kamensky <victor.kamensky@linaro.org> > > > --- > > > drivers/bus/arm-cci.c | 6 ++++++ > > > 1 file changed, 6 insertions(+) > > > > > > diff --git a/drivers/bus/arm-cci.c b/drivers/bus/arm-cci.c > > > index 2009266..6db173e 100644 > > > --- a/drivers/bus/arm-cci.c > > > +++ b/drivers/bus/arm-cci.c > > > @@ -281,6 +281,9 @@ asmlinkage void __naked cci_enable_port_for_self(void) > > > /* Enable the CCI port */ > > > " ldr r0, [r0, %[offsetof_port_phys]] \n" > > > " mov r3, #"__stringify(CCI_ENABLE_REQ)" \n" > > > +#ifdef __ARMEB__ > > > +" rev r3, r3 \n" > > > +#endif /* __ARMEB__ */ > > > > Hmmm, this is a bit ugly, but we can't easily use ARM_BE8() here. > > In any case, it would be more efficient to swap the CCI_ENABLE_REQ > constant at compile time rather than doing it at run time with an extra > instruction. Indeed, but we can't stringify that. A different way of wedging it into the assembler would be needed... We might need to use ldr= too, which doesn't pay nice with inline asm. Cheers ---Dave ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 2/2] ARM: cci: driver need big endian fixes in asm code 2013-10-08 17:57 ` Nicolas Pitre 2013-10-08 18:03 ` Dave Martin @ 2013-10-08 18:14 ` Victor Kamensky 2013-10-08 18:26 ` Nicolas Pitre 1 sibling, 1 reply; 10+ messages in thread From: Victor Kamensky @ 2013-10-08 18:14 UTC (permalink / raw) To: linux-arm-kernel Hi Nicolas, On 8 October 2013 10:57, Nicolas Pitre <nicolas.pitre@linaro.org> wrote: > On Tue, 8 Oct 2013, Dave Martin wrote: > >> On Mon, Oct 07, 2013 at 09:37:20PM -0700, Victor Kamensky wrote: >> > cci_enable_port_for_self written in asm and it works with h/w >> > registers that are in little endian format. When run in big >> > endian mode it needs byte swap before/after it writes/reads >> > to/from such registers >> >> Lorenzo should comment on this if he's not already seen it. >> >> > >> > Signed-off-by: Victor Kamensky <victor.kamensky@linaro.org> >> > --- >> > drivers/bus/arm-cci.c | 6 ++++++ >> > 1 file changed, 6 insertions(+) >> > >> > diff --git a/drivers/bus/arm-cci.c b/drivers/bus/arm-cci.c >> > index 2009266..6db173e 100644 >> > --- a/drivers/bus/arm-cci.c >> > +++ b/drivers/bus/arm-cci.c >> > @@ -281,6 +281,9 @@ asmlinkage void __naked cci_enable_port_for_self(void) >> > /* Enable the CCI port */ >> > " ldr r0, [r0, %[offsetof_port_phys]] \n" >> > " mov r3, #"__stringify(CCI_ENABLE_REQ)" \n" >> > +#ifdef __ARMEB__ >> > +" rev r3, r3 \n" >> > +#endif /* __ARMEB__ */ >> >> Hmmm, this is a bit ugly, but we can't easily use ARM_BE8() here. > > In any case, it would be more efficient to swap the CCI_ENABLE_REQ > constant at compile time rather than doing it at run time with an extra > instruction. I could try to do this but it would require introducing another constant, because CCI_ENABLE_REQ is used in another place in writel_relaxed call which will do byteswap inside. Is such run-time optimization really worth it compared to code readability. Please let me know. I am OK either way. Personally I thought that run-time byteswap would be more readable. Also I am concerned how can I compile time byteswap CCI_ENABLE_REQ in context of its being used with __stringify ... Wondering it should be stringfied into real number not long expression Thanks, Victor > > Nicolas ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 2/2] ARM: cci: driver need big endian fixes in asm code 2013-10-08 18:14 ` Victor Kamensky @ 2013-10-08 18:26 ` Nicolas Pitre 0 siblings, 0 replies; 10+ messages in thread From: Nicolas Pitre @ 2013-10-08 18:26 UTC (permalink / raw) To: linux-arm-kernel On Tue, 8 Oct 2013, Victor Kamensky wrote: > Hi Nicolas, > > On 8 October 2013 10:57, Nicolas Pitre <nicolas.pitre@linaro.org> wrote: > > On Tue, 8 Oct 2013, Dave Martin wrote: > > > >> On Mon, Oct 07, 2013 at 09:37:20PM -0700, Victor Kamensky wrote: > >> > cci_enable_port_for_self written in asm and it works with h/w > >> > registers that are in little endian format. When run in big > >> > endian mode it needs byte swap before/after it writes/reads > >> > to/from such registers > >> > >> Lorenzo should comment on this if he's not already seen it. > >> > >> > > >> > Signed-off-by: Victor Kamensky <victor.kamensky@linaro.org> > >> > --- > >> > drivers/bus/arm-cci.c | 6 ++++++ > >> > 1 file changed, 6 insertions(+) > >> > > >> > diff --git a/drivers/bus/arm-cci.c b/drivers/bus/arm-cci.c > >> > index 2009266..6db173e 100644 > >> > --- a/drivers/bus/arm-cci.c > >> > +++ b/drivers/bus/arm-cci.c > >> > @@ -281,6 +281,9 @@ asmlinkage void __naked cci_enable_port_for_self(void) > >> > /* Enable the CCI port */ > >> > " ldr r0, [r0, %[offsetof_port_phys]] \n" > >> > " mov r3, #"__stringify(CCI_ENABLE_REQ)" \n" > >> > +#ifdef __ARMEB__ > >> > +" rev r3, r3 \n" > >> > +#endif /* __ARMEB__ */ > >> > >> Hmmm, this is a bit ugly, but we can't easily use ARM_BE8() here. > > > > In any case, it would be more efficient to swap the CCI_ENABLE_REQ > > constant at compile time rather than doing it at run time with an extra > > instruction. > > I could try to do this but it would require introducing another constant, > because CCI_ENABLE_REQ is used in another place in writel_relaxed > call which will do byteswap inside. Is such run-time optimization really > worth it compared to code readability. Please let me know. > I am OK either way. Personally I thought that run-time byteswap would > be more readable. You're probably right. > Also I am concerned how can I compile time byteswap CCI_ENABLE_REQ > in context of its being used with __stringify ... Wondering it should be > stringfied into real number not long expression The assembler can accept a subset of the simple C operands so it is possible that this could just work. Otherwise it is best to go with a runtime swap. Nicolas ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 0/2] additional vexpress-tc2 big endian fixes 2013-10-08 4:37 [PATCH 0/2] additional vexpress-tc2 big endian fixes Victor Kamensky 2013-10-08 4:37 ` [PATCH 1/2] ARM: mcpm: fix big endian issue in mcpm startup code Victor Kamensky 2013-10-08 4:37 ` [PATCH 2/2] ARM: cci: driver need big endian fixes in asm code Victor Kamensky @ 2013-10-08 7:27 ` Ben Dooks 2 siblings, 0 replies; 10+ messages in thread From: Ben Dooks @ 2013-10-08 7:27 UTC (permalink / raw) To: linux-arm-kernel On 08/10/13 06:37, Victor Kamensky wrote: > Hi Ben, > > Here are couple more big endian related fixes. We run into these > with vexpress TC2 board, when CONFIG_MCPM and CONFIG_ARM_CCI configs > are enabled. Big endian image fails to boot. With fixes BE TC2 > boots and it sees all 5 cores (2xA15, 3xA7). > > These were tested with vexpress TC2 on latest linux-linaro > branch (include BE patch series) and it was tested on very > recent rmk/fixes branch along with BE series on top of it. > > Thanks, > Victor > > Victor Kamensky (2): > ARM: mcpm: fix big endian issue in mcpm startup code > ARM: cci: driver need big endian fixes in asm code > > arch/arm/common/mcpm_head.S | 2 ++ > drivers/bus/arm-cci.c | 6 ++++++ > 2 files changed, 8 insertions(+) Thanks, anyone any objections if these go into my 312-rc4 branch? -- Ben Dooks http://www.codethink.co.uk/ Senior Engineer Codethink - Providing Genius ^ permalink raw reply [flat|nested] 10+ messages in thread
[parent not found: <20131008175553.GB3866@localhost.localdomain>]
* [PATCH 1/2] ARM: mcpm: fix big endian issue in mcpm startup code [not found] <20131008175553.GB3866@localhost.localdomain> @ 2013-10-08 18:28 ` Nicolas Pitre 0 siblings, 0 replies; 10+ messages in thread From: Nicolas Pitre @ 2013-10-08 18:28 UTC (permalink / raw) To: linux-arm-kernel On Tue, 8 Oct 2013, Dave Martin wrote: > On Mon, Oct 07, 2013 at 09:37:19PM -0700, Victor Kamensky wrote: > > In big endian mode mcpm_entry_point is first function > > that called on secondaries CPU. First it should switch > > CPU into big endian code. > > > > Signed-off-by: Victor Kamensky <victor.kamensky@linaro.org> > > Providing Nico's also OK with it, I don't see a problem with this. > > Minor cosmetic nit: please line up the ) after be with the others. > Not the end of the world, though. > > Reviewed-by: Dave Martin <Dave.Martin@arm.com> No problem here. Acked-by: Nicolas Pitre <nico@linaro.org> > > Cheers > ---Dave > > > --- > > arch/arm/common/mcpm_head.S | 2 ++ > > 1 file changed, 2 insertions(+) > > > > diff --git a/arch/arm/common/mcpm_head.S b/arch/arm/common/mcpm_head.S > > index 39c96df..4f88f5e 100644 > > --- a/arch/arm/common/mcpm_head.S > > +++ b/arch/arm/common/mcpm_head.S > > @@ -15,6 +15,7 @@ > > > > #include <linux/linkage.h> > > #include <asm/mcpm.h> > > +#include <asm/assembler.h> > > > > #include "vlock.h" > > > > @@ -47,6 +48,7 @@ > > > > ENTRY(mcpm_entry_point) > > > > + ARM_BE8(setend be) > > THUMB( adr r12, BSYM(1f) ) > > THUMB( bx r12 ) > > THUMB( .thumb ) > > -- > > 1.8.1.4 > > > > > > _______________________________________________ > > linaro-kernel mailing list > > linaro-kernel at lists.linaro.org > > http://lists.linaro.org/mailman/listinfo/linaro-kernel > ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2013-10-08 18:28 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-10-08 4:37 [PATCH 0/2] additional vexpress-tc2 big endian fixes Victor Kamensky
2013-10-08 4:37 ` [PATCH 1/2] ARM: mcpm: fix big endian issue in mcpm startup code Victor Kamensky
2013-10-08 4:37 ` [PATCH 2/2] ARM: cci: driver need big endian fixes in asm code Victor Kamensky
2013-10-08 17:51 ` Dave Martin
2013-10-08 17:57 ` Nicolas Pitre
2013-10-08 18:03 ` Dave Martin
2013-10-08 18:14 ` Victor Kamensky
2013-10-08 18:26 ` Nicolas Pitre
2013-10-08 7:27 ` [PATCH 0/2] additional vexpress-tc2 big endian fixes Ben Dooks
[not found] <20131008175553.GB3866@localhost.localdomain>
2013-10-08 18:28 ` [PATCH 1/2] ARM: mcpm: fix big endian issue in mcpm startup code Nicolas Pitre
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).