
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