Attention is currently required from: Arthur Heymans, David Hendricks, Jincheng Li, Jonathan Zhang, Lean Sheng Tan, Nicholas Chin, Patrick Rudolph, Paul Menzel, Shuo Liu, TangYiwei, Varshit Pandya.
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/81319?usp=email )
Change subject: mb/intel/avenuecity_crb: Add GNR/SRF-AP 2S server board Avenue City ......................................................................
Patch Set 50:
(3 comments)
File src/drivers/ocp/include/vpd.h:
https://review.coreboot.org/c/coreboot/+/81319/comment/f9ec0d09_9f9b88ed : PS50, Line 6: #include <include/types.h> Huh? This header doesn't need `types.h`
File src/mainboard/intel/avenuecity_crb/config/iio.c:
https://review.coreboot.org/c/coreboot/+/81319/comment/cf189e74_46ff1c2d : PS36, Line 14: _IIO_PORT_CFG_STRUCT_X8(0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, : 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x4B, 0x1), : _IIO_PORT_CFG_STRUCT_DISABLED, : _IIO_PORT_CFG_STRUCT_DISABLED, : _IIO_PORT_CFG_STRUCT_DISABLED, : _IIO_PORT_CFG_STRUCT_X2(0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, : 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x4B, 0x2), : _IIO_PORT_CFG_STRUCT_X2(0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, : 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x4B, 0x3), : _IIO_PORT_CFG_STRUCT_X2(0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, : 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x4B, 0x4), : _IIO_PORT_CFG_STRUCT_X2(0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, : 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x4B, 0x5)
Yes, personally I agree. This is how FSP header defines and we have to follow ... […]
If it's just the macros, we could define some macros to use in coreboot that use left-to-right order. Here's an example (`CB_` means coreboot), suggestions for better names are welcome:
``` #define CB_IIO_BIFURCATE_x8x2x2x2x2 IIO_BIFURCATE_x2x2x2x2x8 ```
File src/mainboard/intel/avenuecity_crb/config/iio.c:
https://review.coreboot.org/c/coreboot/+/81319/comment/99b04f5c_579ed707 : PS50, Line 28: (sizeof(iio_config_table)/sizeof(struct iio_pe_config)) `ARRAY_SIZE(iio_config_table)`