Hello Wolfgang, > Dear Marek, > > In message <[email protected]> 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 _______________________________________________ U-Boot mailing list [email protected] http://lists.denx.de/mailman/listinfo/u-boot
