[U-Boot] [PATCH V4 09/11] ARM/PPC: add a common way to access registers
Stefano Babic
sbabic at denx.de
Fri Jan 29 12:28:21 CET 2010
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
=====================================================================
More information about the U-Boot
mailing list