HAOUAS Elyes 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 6:
(7 comments)
Thx
https://review.coreboot.org/c/coreboot/+/45517/6/src/northbridge/intel/commo... File src/northbridge/intel/common/mchbar_ops.h:
https://review.coreboot.org/c/coreboot/+/45517/6/src/northbridge/intel/commo... PS6, Line 8: #if CONFIG(NORTHBRIDGE_INTEL_X4X) : #include <northbridge/intel/x4x/iomap.h> : #elif CONFIG(NORTHBRIDGE_INTEL_PINEVIEW) : #include <northbridge/intel/pineview/memmap.h> : #elif CONFIG(NORTHBRIDGE_INTEL_SANDYBRIDGE) : #include <northbridge/intel/sandybridge/sandybridge.h> : #elif CONFIG(NORTHBRIDGE_INTEL_IRONLAKE) : #include <northbridge/intel/ironlake/ironlake.h> : #elif CONFIG(NORTHBRIDGE_INTEL_HASWELL) : #include <northbridge/intel/haswell/memmap.h> : #endif
yeah... this is something I wanted to avoid. I guess this will have to do in the meantime. […]
ok
https://review.coreboot.org/c/coreboot/+/45517/6/src/northbridge/intel/commo... PS6, Line 20: x
This is unsafe. Please add braces around the macro parameters.
Done
https://review.coreboot.org/c/coreboot/+/45517/6/src/northbridge/intel/commo... PS6, Line 24: int
This should never be negative
Done
https://review.coreboot.org/c/coreboot/+/45517/6/src/northbridge/intel/commo... PS6, Line 27: void mchbar8_and(int offset, u8 value;
Where did the closing brace go?
here we go :)
https://review.coreboot.org/c/coreboot/+/45517/6/src/northbridge/intel/commo... File src/northbridge/intel/common/mchbar_ops.c:
https://review.coreboot.org/c/coreboot/+/45517/6/src/northbridge/intel/commo... PS6, Line 13: void mchbar8_unset_and_set(int offset, u8 unset, u8 set)
Any specific reason to not make these inline? I'd recommend trying both and checking which results i […]
both changes the binary on lenovo/t440p with "BUILD_TIMELESS=1"
https://review.coreboot.org/c/coreboot/+/45517/6/src/northbridge/intel/commo... PS6, Line 40: void mchbar8_and(int offset, u8 value)
For consistency, these should be `mchbarX_unset` functions (or you will have the same problem with t […]
Done
https://review.coreboot.org/c/coreboot/+/45517/6/src/northbridge/intel/commo... PS6, Line 64: void mchbar8_or(int offset, u8 value)
And these should be called mchbarX_set
Done