
Hello Wolfgang,
Dear Marek,
In message 201402172222.38911.marex@denx.de you wrote:
+#define PHY_CMD1_READ BIT(6) +#define PHY_CMD1_WRITE BIT(5) +#define PHY_CMD1_PHYADDR BIT(0)
+#define PHY_CMD2_PEND BIT(7) +#define PHY_CMD2_READY BIT(6)
...
As mentioned in patch # 1, I object against the use of these obfuscating BIT() macros. Please do not use these; use plain readable code, that leaves no ambiguities to the reader.
Just to chime in real quick, Linux uses these 'BIT()' macros, but I personally have no hard feelings about them either way.
Yes, certain developers have been using this style before. This does not make it any better. Fact is, that I have no way to tell what the code means. BIT(0) can be expected to have any of the following meanings: 0x01, 0x80, 0x8000, 0x80000000, ... So I always have to look up the macro definition first, before I can unerstand it. And then I have to compute in my head what the number actually means.
Compare: BIT(6) or 0x40 - what is easier to write, to read, and to understand?
(1 << 6) is easier for me to read honestly, because then I can quickly crosscheck it with the datasheet.
You dump a register - either with the BDI, or with some printf(). You get 0x27051956. Is BIT(17) set? Is bit 0x00020000 set? Which of these questions is easier to answer (even when you are sure that this is on a system where bit no. 0 is the LSB)?
You have a point with the endianness here.
Best regards, Marek Vasut