Rudolf just found a bug in the sb700 code:
u32 dword; .. dword = pci_read_config8(dev, 0x64); dword |= 1 << 10; pci_write_config8(dev, 0x64, dword);
And I'm ranting now, because a pci_set8() macro/function could have found this bug at compile time, and because I don't like these constructs. (Compiler warnings would also have indicated a problem. They're currently disabled for this code.)
Carl-Daniel is working on a patch, meanwhile consider this:
#define pci_set8(dev, reg, val) do { \ if ((a) & ~0xff) can_only_set_low_8_bits(); \ pci_set8_nocheck((dev), (reg), (val)); \ } while(0)
u8 pci_set8_nocheck(device_t dev, u32 reg, u8 val) { u8 tmp = pci_read_config8(dev, reg); tmp |= val; pci_write_config8(dev, reg, tmp); return tmp; }
$ gcc -g -o a a.c /tmp/cchpQWSr.o: In function `main': /tmp/a.c:11: undefined reference to `can_only_set_low_8_bits' collect2: ld returned 1 exit status
Now, this is ugly, and warnings is the only right way, but I still very much think that one pci_set8() or pci_clear8() call is way better than code to read+mod+write. The reason is that it's how I think about these operations; "set bits xy in reg foo." Read+mod+write is one lower level of abstraction, and not relevant in the context of setting the bits. Ie. I don't want to write/see *how* bits get set, I just want to write/see *that* bits get set.
I completely understand that everyone else may not have the same mental model of these operations, but I hope many enough do.. (Yes, we've discussed before and it seemed not to be the case.)
Carl-Daniel mentioned that there may be a risk for confusion between pci_set8() and pci_write_config8(), and this was also noted before. Isn't it really reasonable to expect that people can actually keep track of how those two functions are different?
I understand that my taste is more terse than most, and if consensus is that _set8 _clear8 etc. names suck, then I'd be happy with other fun names too, as long as read+mod+write blocks can be replaced with single lines of code.
Like the CAR thing I think this really helps write- and readability, and even thinkability, for our code. The latter helps make further abstraction easier, which allows more refactoring. Bad goal?
//Peter