[PATCH 2/2] ARM: cci: driver need big endian fixes in asm code

[Nicolas Pitre]

On Tue, 8 Oct 2013, Victor Kamensky wrote:

> Hi Nicolas,
> 
> On 8 October 2013 10:57, Nicolas Pitre <nicolas.pitre at 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 at 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