* [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; 9+ 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] 9+ 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; 9+ 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] 9+ 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; 9+ 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] 9+ 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; 9+ 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] 9+ 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; 9+ 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] 9+ 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; 9+ 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] 9+ 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; 9+ 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] 9+ 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; 9+ 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] 9+ 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; 9+ 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] 9+ messages in thread
end of thread, other threads:[~2013-10-08 18:26 UTC | newest] Thread overview: 9+ 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
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.