Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45517 )
Change subject: nb/intel: Introduce MCHBAR accessor functions ......................................................................
Patch Set 37:
(2 comments)
https://review.coreboot.org/c/coreboot/+/45517/37/src/northbridge/intel/comm... File src/northbridge/intel/common/mchbar_ops.h:
https://review.coreboot.org/c/coreboot/+/45517/37/src/northbridge/intel/comm... PS37, Line 8: (u8 *)DEFAULT_MCHBAR + x) Macro safety, please. Also, should we avoid pointer arithmetics?
#define MCHBAR8(x) (*((volatile u8 *)((void *)(DEFAULT_MCHBAR + (x)))))
https://review.coreboot.org/c/coreboot/+/45517/37/src/northbridge/intel/comm... PS37, Line 27: (u8)
Are these casts already needed to get it through Jenkins? Otherwise, we […]
The casts on `value` should never be needed. Casts on `mask` are probably meant to work around compilers complaining about overflow because of autopromotion when performing bitwise negations.
Also, `0xff`? Did you mean `UINT8_MAX`?