Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45517 )
Change subject: nb/intel/common: Introduce common MCHBAR accessor functions ......................................................................
Patch Set 13:
(2 comments)
https://review.coreboot.org/c/coreboot/+/45517/13/src/northbridge/intel/comm... File src/northbridge/intel/common/mchbar_ops.h:
https://review.coreboot.org/c/coreboot/+/45517/13/src/northbridge/intel/comm... PS13, Line 20: #endif We can just move the MCHBAR location to Kconfig?
https://review.coreboot.org/c/coreboot/+/45517/13/src/northbridge/intel/comm... PS13, Line 88: __always_inline void mchbar8_set(unsigned int offset, u8 set)
Ugh, apologies. I just realized that the `mchbarX_set` name is very misleading: […]
Slow down please. With macros instead of functions we'd lose the typing again. Making it impossible for the compiler to detect real overflows. We can use clrsetbitsXX() in the function bodies, ofc. (__versions are not to be used outside the file that defines them, and wouldn't be necessary.)
`clrsetbits` is against our style, IMHO. It should be `clr_set_bits`.