On Fri, Jun 15, 2007 at 02:37:56PM +0200, Peter Stuge wrote:
Some even more elegant solution?reg8 &= ~(SECONDARY_IDE_ENABLE); if (conf->ide1_enable) reg8 |= SECONDARY_IDE_ENABLE;
?
Yeah, that would be another option.
As we're probably doing this type of operation quite often, I made a macro out of it (which hopefully works for all situations):
/* If 'cond' is true this macro sets the bit(s) specified by 'bits' in the * 'reg' variable, otherwise it clears those bits. * * Examples: * reg16 = ONOFF(conf->ide0_enable, reg16, (1 << 5)); * reg16 = ONOFF(conf->ide0_enable, reg16, (FOO | BAR)); */ /* TODO: Move into some global header file? */
#define ONOFF(cond,reg,bits) ((cond) ? ((reg) | (bits)) : ((reg) & ~(bits)))
Usage example (from my other patch):
reg8 = pci_read_config8(dev, UDMACTL); reg8 = ONOFF(conf->ide1_drive0_udma33_enable, reg8, SSDE0); reg8 = ONOFF(conf->ide1_drive1_udma33_enable, reg8, SSDE1); pci_write_config8(dev, UDMACTL, reg8);
Shall we create an include/macros.h file with some generic, useful macros? Stuff like the above, and maybe something like:
#define MIN(a,b) ... #define MAX(a,b) ... #define ARRAY_SIZE(a) (sizeof(a) / sizeof((a)[0])) ...
This type of common macros (not too many of course) will quite likely improve the code readability a lot.
Uwe.