[U-Boot] [PATCH V4 09/11] ARM/PPC: add a common way to access registers

[Stefano Babic <sbabic@denx.de>]

Wolfgang Denk wrote:
> Dear Stefano,
> 
Hi Wolfgang,

> could you _please_ provide sufficient "References:" and "In-reply-to:"
> headers with your messages, so threading is working? You are using
> git-send-email so this is really simple - just enter the respective
> message ID when it asks
> 
> 	Message-ID to be used as In-Reply-To for the first email? 

Sorry for that.

> In my first comment I wrote:
> 
> | Please document these new macros.  Please be explicit about the
> | behaviour of these macros on systems with different endianess.
> 
> This is still missing.
> 
> We need an entry in the README or even better some new
> "doc/README.io_accessors" or similar, which lists the new macros and
> their intended use and interface.
> 
> This file should also document the behaviour of these macros on big
> endian and little endian machines.

I understand what you mean and I come now to a slightly different
solution as what you have proposed. Really my patches drop the endianess
(that is explicitely set in the actual fsl_esdhc driver) and generate
the confusion you reported. Probably, even if I add a better
documentation, it remains unclear which is the endianess used.

Another possibility will be to explicitely set the endianess with a
CONFIG_ when it is needed.
I found that in u-boot there is already a solution like this (for USB):

	CONFIG_EHCI_DESC_BIG_ENDIAN

My proposal will be:

In the accessors (io.h), I will add all requested macros for both
endianess (clrbits_le32,clrbits_be32, setbits_be32,...) for the arm
architecture as you proposed. It seems there is no need to change the
accessors for powerpc. I have to drop the "implicite" endianess I set
(for example, when I set clrsetbits32 as big endian for powerpc and
little endian for arm), source of confusion.

Probably I do not need in this case to write any additional
documentation because the endianess is explicitely set and IMHO is self
explained.

I will add something like CONFIG_FSL_ESDHC_BIG_ENDIAN or
CONFIG_FSL_ESDHC_LITTLE_ENDIAN in the driver I changed. In the related
header file for the driver, I can set driver specific macros that point
to the correct accessors, depending on the new CONFIG_ switch.
Then it should be clear which is the endianess used and it will be not
related to the processor architecture. What do you think about this ?

Best regards,
Stefano Babic

-- 
=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office at denx.de
=====================================================================