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? 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)? It's all about writing portable and maintainable code. Best regards, Wolfgang Denk -- DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: [email protected] "I've finally learned what `upward compatible' means. It means we get to keep all our old mistakes." - Dennie van Tassel _______________________________________________ U-Boot mailing list [email protected] http://lists.denx.de/mailman/listinfo/u-boot
