[L] Change in coreboot[master]: [WIP] mb/prodrive/atlas: Populate smbios table with VPD from ECs EMI
Attention is currently required from: Christian Walter, Angel Pons, Lean Sheng Tan.
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/68137 )
Change subject: [WIP] mb/prodrive/atlas: Populate smbios table with VPD from ECs EMI ......................................................................
Patch Set 1:
(65 comments)
Commit Message:
Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-159709): https://review.coreboot.org/c/coreboot/+/68137/comment/f5556675_853cc8b3 PS1, Line 18: smbios.c: populates smbios type 1 with the serial-number by using vpd.h Possible unwrapped commit description (prefer a maximum 72 chars per line)
Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-159709): https://review.coreboot.org/c/coreboot/+/68137/comment/233025ee_7580626a PS1, Line 21: devicetree.cb: enables address range 0xc00-0xcff for the EMI runtime registers Possible unwrapped commit description (prefer a maximum 72 chars per line)
Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-159709): https://review.coreboot.org/c/coreboot/+/68137/comment/c5159a1e_855be86d PS1, Line 22: Makefile.inc: adds emi.c, vpd.c, smbios.c, ld_config.c to the ramstage compilation Possible unwrapped commit description (prefer a maximum 72 chars per line)
Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-159709): https://review.coreboot.org/c/coreboot/+/68137/comment/e09bf477_875e65be PS1, Line 24: TODO: ld_config.* is redundant. We can replace this with pnp_device.h or pnp_ops.h Possible unwrapped commit description (prefer a maximum 72 chars per line)
File src/mainboard/prodrive/atlas/emi.h:
Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-159709): https://review.coreboot.org/c/coreboot/+/68137/comment/215922ea_3f004b7b PS1, Line 29: * the embedded controller (MEC152x). trailing whitespace
Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-159709): https://review.coreboot.org/c/coreboot/+/68137/comment/4b67172f_08bc1b6a PS1, Line 49: EMI_ACCESS_8BIT = 0b00, ///< 8-bit read/write access Avoid gcc v4.3+ binary constant extension: <0b00>
Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-159709): https://review.coreboot.org/c/coreboot/+/68137/comment/c91ad91e_0ae44a9a PS1, Line 50: EMI_ACCESS_16BIT = 0b01, ///< 16-bit read/write access Avoid gcc v4.3+ binary constant extension: <0b01>
Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-159709): https://review.coreboot.org/c/coreboot/+/68137/comment/54650168_ae8622aa PS1, Line 51: EMI_ACCESS_32BIT = 0b10, ///< 32-bit read/write access Avoid gcc v4.3+ binary constant extension: <0b10>
Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-159709): https://review.coreboot.org/c/coreboot/+/68137/comment/2e7a7e3f_67651a92 PS1, Line 52: EMI_ACCESS_32BIT_AUTO_INC = 0b11 ///< 32-bit read/write access (auto increment) Avoid gcc v4.3+ binary constant extension: <0b11>
Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-159709): https://review.coreboot.org/c/coreboot/+/68137/comment/edadb27d_b98c8915 PS1, Line 74: extern int emi_read_region(const EMI_INSTANCE instance, const EMI_REGION region, const EMI_ACCESS access, const u16 addr, u8 *buff); line length of 132 exceeds 96 columns
Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-159709): https://review.coreboot.org/c/coreboot/+/68137/comment/3fbcaab7_a9b72ebc PS1, Line 85: extern int emi_write_region(const EMI_INSTANCE instance, const EMI_REGION region, const EMI_ACCESS access, const u16 addr, u8 *bytes); line length of 134 exceeds 96 columns
Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-159709): https://review.coreboot.org/c/coreboot/+/68137/comment/bdff0bcd_518cedeb PS1, Line 95: extern void emi_read_region_block(const EMI_INSTANCE instance, const EMI_REGION region, const u16 addr, const u16 n, u8 *buff); line length of 127 exceeds 96 columns
File src/mainboard/prodrive/atlas/emi.c:
Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-159709): https://review.coreboot.org/c/coreboot/+/68137/comment/2e6db9f8_c5c867df PS1, Line 15: #define EMI_1_BASE EMI_0_BASE + APPLICATION_ID + 0x1 Macros with complex values should be enclosed in parentheses
Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-159709): https://review.coreboot.org/c/coreboot/+/68137/comment/aba183ae_c3fe0d1f PS1, Line 27: void emi_init(const EMI_INSTANCE instance) { open brace '{' following function definitions go on the next line
Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-159709): https://review.coreboot.org/c/coreboot/+/68137/comment/c73d6900_08c97d34 PS1, Line 36: ld_config_write(ESPI_IO_COMPONENT, index + 0, 0b00000001); // valid bit Avoid gcc v4.3+ binary constant extension: <0b00000001>
Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-159709): https://review.coreboot.org/c/coreboot/+/68137/comment/23c7eab1_dfb579d0 PS1, Line 41: int emi_read_region(const EMI_INSTANCE instance, const EMI_REGION region, const EMI_ACCESS access, const u16 addr, u8 *buff) { line length of 126 exceeds 96 columns
Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-159709): https://review.coreboot.org/c/coreboot/+/68137/comment/1cf88e18_f3cf7043 PS1, Line 41: int emi_read_region(const EMI_INSTANCE instance, const EMI_REGION region, const EMI_ACCESS access, const u16 addr, u8 *buff) { open brace '{' following function definitions go on the next line
Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-159709): https://review.coreboot.org/c/coreboot/+/68137/comment/709e8c62_fb026049 PS1, Line 48: switch(access) { switch and case should be at the same indent
Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-159709): https://review.coreboot.org/c/coreboot/+/68137/comment/683cc72a_d5aa6adb PS1, Line 48: switch(access) { space required before the open parenthesis '('
Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-159709): https://review.coreboot.org/c/coreboot/+/68137/comment/4ba104f2_2cb7451c PS1, Line 52: *((u8*) buff) = inb(base + EC_DATA_BYTE_0); "(foo*)" should be "(foo *)"
Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-159709): https://review.coreboot.org/c/coreboot/+/68137/comment/04c15778_66fa0259 PS1, Line 58: *((u16*) buff) = inw(base + EC_DATA_BYTE_0); "(foo*)" should be "(foo *)"
Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-159709): https://review.coreboot.org/c/coreboot/+/68137/comment/3f4dd07e_2fa1aff1 PS1, Line 65: *((u32*) buff) = inl(base + EC_DATA_BYTE_0); "(foo*)" should be "(foo *)"
Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-159709): https://review.coreboot.org/c/coreboot/+/68137/comment/4606efc3_199e53a8 PS1, Line 69: default: printk(BIOS_NOTICE, "emi: invalid access.\n"); return EMI_FAILURE; trailing statements should be on next line
Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-159709): https://review.coreboot.org/c/coreboot/+/68137/comment/e4dddd9a_84bf1056 PS1, Line 75: int emi_write_region(const EMI_INSTANCE instance, const EMI_REGION region, const EMI_ACCESS access, const u16 addr, u8 *bytes) { line length of 128 exceeds 96 columns
Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-159709): https://review.coreboot.org/c/coreboot/+/68137/comment/51614cc6_c5175457 PS1, Line 75: int emi_write_region(const EMI_INSTANCE instance, const EMI_REGION region, const EMI_ACCESS access, const u16 addr, u8 *bytes) { open brace '{' following function definitions go on the next line
Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-159709): https://review.coreboot.org/c/coreboot/+/68137/comment/e40443ab_a3640f86 PS1, Line 82: switch(access) { switch and case should be at the same indent
Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-159709): https://review.coreboot.org/c/coreboot/+/68137/comment/572f46c2_ecd6ead4 PS1, Line 82: switch(access) { space required before the open parenthesis '('
Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-159709): https://review.coreboot.org/c/coreboot/+/68137/comment/4801a6fb_e62effdf PS1, Line 86: outb(*((u8*) bytes), base + EC_DATA_BYTE_0); "(foo*)" should be "(foo *)"
Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-159709): https://review.coreboot.org/c/coreboot/+/68137/comment/1418fdef_5b9c8ff9 PS1, Line 92: outw(*((u16*) bytes), base + EC_DATA_BYTE_0); "(foo*)" should be "(foo *)"
Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-159709): https://review.coreboot.org/c/coreboot/+/68137/comment/32e32476_9d23fdc2 PS1, Line 99: outl(*((u32*) bytes), base + EC_DATA_BYTE_0); "(foo*)" should be "(foo *)"
Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-159709): https://review.coreboot.org/c/coreboot/+/68137/comment/53eeae22_992d0a95 PS1, Line 103: default: printk(BIOS_NOTICE, "emi: invalid access.\n"); return EMI_FAILURE; trailing statements should be on next line
Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-159709): https://review.coreboot.org/c/coreboot/+/68137/comment/ac0c464e_68aec387 PS1, Line 109: void emi_read_region_block(const EMI_INSTANCE instance, const EMI_REGION region, const u16 addr, const u16 n, u8 *buff) { line length of 121 exceeds 96 columns
Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-159709): https://review.coreboot.org/c/coreboot/+/68137/comment/87ecb6e3_1c3b27bb PS1, Line 109: void emi_read_region_block(const EMI_INSTANCE instance, const EMI_REGION region, const u16 addr, const u16 n, u8 *buff) { open brace '{' following function definitions go on the next line
Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-159709): https://review.coreboot.org/c/coreboot/+/68137/comment/8aa5cb4a_2ca3cd5b PS1, Line 113: for(;;) { space required before the open parenthesis '('
Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-159709): https://review.coreboot.org/c/coreboot/+/68137/comment/df3a6188_7d3349d8 PS1, Line 118: for(u8 i = 0; i < 4; i++) { space required before the open parenthesis '('
Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-159709): https://review.coreboot.org/c/coreboot/+/68137/comment/360ca655_e3aafa74 PS1, Line 120: if(idx == n) space required before the open parenthesis '('
Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-159709): https://review.coreboot.org/c/coreboot/+/68137/comment/48d63c5e_c2f9f14f PS1, Line 128: inline void emi_write_register(const EMI_INSTANCE instance, const u8 offs, const u8 value) { open brace '{' following function definitions go on the next line
Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-159709): https://review.coreboot.org/c/coreboot/+/68137/comment/fb4abfa4_80ac0847 PS1, Line 133: inline u8 emi_read_register(const EMI_INSTANCE instance, const u8 offs) { open brace '{' following function definitions go on the next line
Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-159709): https://review.coreboot.org/c/coreboot/+/68137/comment/49b77e3b_69aaac76 PS1, Line 140: static inline u16 emi_base(const EMI_INSTANCE instance) { open brace '{' following function definitions go on the next line
Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-159709): https://review.coreboot.org/c/coreboot/+/68137/comment/ebad3f10_92f62f45 PS1, Line 145: static inline u8 emi_index(const EMI_INSTANCE instance) { open brace '{' following function definitions go on the next line
File src/mainboard/prodrive/atlas/ld_config.h:
Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-159709): https://review.coreboot.org/c/coreboot/+/68137/comment/8f0097e4_1b583290 PS1, Line 7: * This file is redundant and should only be used temporarily within trailing whitespace
File src/mainboard/prodrive/atlas/ld_config.c:
Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-159709): https://review.coreboot.org/c/coreboot/+/68137/comment/072f1a4e_f241f5e5 PS1, Line 15: * This file is redundant and should only be used temporarily within trailing whitespace
Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-159709): https://review.coreboot.org/c/coreboot/+/68137/comment/791f72ea_d6f45bc6 PS1, Line 20: void ld_config_enter(void) { open brace '{' following function definitions go on the next line
Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-159709): https://review.coreboot.org/c/coreboot/+/68137/comment/5434bbb3_e97c6ceb PS1, Line 25: void ld_config_leave(void) { open brace '{' following function definitions go on the next line
Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-159709): https://review.coreboot.org/c/coreboot/+/68137/comment/0658f14c_caac2d18 PS1, Line 30: void ld_config_write(const u8 ldn, const u8 index, const u8 value) { open brace '{' following function definitions go on the next line
Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-159709): https://review.coreboot.org/c/coreboot/+/68137/comment/e06e5b3a_4fe636d9 PS1, Line 38: u8 ld_config_read(const u8 ldn, const u8 index) { open brace '{' following function definitions go on the next line
Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-159709): https://review.coreboot.org/c/coreboot/+/68137/comment/97ac7991_82b108f8 PS1, Line 45: } adding a line without newline at end of file
File src/mainboard/prodrive/atlas/mainboard.c:
Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-159709): https://review.coreboot.org/c/coreboot/+/68137/comment/971551f1_af28e297 PS1, Line 14: static const char* get_smbios_part_number(void); "foo* bar" should be "foo *bar"
Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-159709): https://review.coreboot.org/c/coreboot/+/68137/comment/a355cb38_9e983698 PS1, Line 23: static void mainboard_enable(struct device *dev) { open brace '{' following function definitions go on the next line
Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-159709): https://review.coreboot.org/c/coreboot/+/68137/comment/4e21880d_d668187a PS1, Line 36: static void get_smbios_strings(struct device *dev, struct smbios_type11 *t) { open brace '{' following function definitions go on the next line
Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-159709): https://review.coreboot.org/c/coreboot/+/68137/comment/07c3e3e3_eb43365c PS1, Line 41: static const char* get_smbios_part_number(void) { "foo* bar" should be "foo *bar"
Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-159709): https://review.coreboot.org/c/coreboot/+/68137/comment/30ed79e6_260ca019 PS1, Line 41: static const char* get_smbios_part_number(void) { open brace '{' following function definitions go on the next line
Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-159709): https://review.coreboot.org/c/coreboot/+/68137/comment/8491d2b3_2eb57260 PS1, Line 46: strncpy((char*) pn, prefix, sizeof(prefix)); "(foo*)" should be "(foo *)"
Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-159709): https://review.coreboot.org/c/coreboot/+/68137/comment/1e4b3197_ecee02e0 PS1, Line 49: return (char*) pn; "(foo*)" should be "(foo *)"
File src/mainboard/prodrive/atlas/smbios.c:
Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-159709): https://review.coreboot.org/c/coreboot/+/68137/comment/6e6537ae_23e381a5 PS1, Line 8: const char* smbios_mainboard_serial_number(void) { "foo* bar" should be "foo *bar"
Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-159709): https://review.coreboot.org/c/coreboot/+/68137/comment/5ceecabd_457c6c55 PS1, Line 8: const char* smbios_mainboard_serial_number(void) { open brace '{' following function definitions go on the next line
Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-159709): https://review.coreboot.org/c/coreboot/+/68137/comment/10824954_e982a92e PS1, Line 13: return (char*) sn; "(foo*)" should be "(foo *)"
File src/mainboard/prodrive/atlas/vpd.h:
Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-159709): https://review.coreboot.org/c/coreboot/+/68137/comment/249b94c4_dd0de84e PS1, Line 12: typedef struct __attribute__((__packed__)) vpd_section { Prefer __packed over __attribute__((__packed__))
Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-159709): https://review.coreboot.org/c/coreboot/+/68137/comment/89c0548a_93f7629d PS1, Line 14: char part_number [PAGESIZE]; space prohibited before open square bracket '['
Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-159709): https://review.coreboot.org/c/coreboot/+/68137/comment/aa35b899_3dbd8779 PS1, Line 34: #endif adding a line without newline at end of file
File src/mainboard/prodrive/atlas/vpd.c:
Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-159709): https://review.coreboot.org/c/coreboot/+/68137/comment/cbd4064b_333f5d79 PS1, Line 18: void vpd_get(const u16 vpd, u8 *buff) { open brace '{' following function definitions go on the next line
Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-159709): https://review.coreboot.org/c/coreboot/+/68137/comment/e722c294_e984e8b4 PS1, Line 20: if(emi_initialized == false) { space required before the open parenthesis '('
Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-159709): https://review.coreboot.org/c/coreboot/+/68137/comment/4d771bcc_9c075d70 PS1, Line 32: static u16 vpd_length(const u16 vpd) { open brace '{' following function definitions go on the next line
Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-159709): https://review.coreboot.org/c/coreboot/+/68137/comment/7279e453_2f031d88 PS1, Line 34: switch(vpd) { switch and case should be at the same indent
Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-159709): https://review.coreboot.org/c/coreboot/+/68137/comment/1d4d5b83_05947cc4 PS1, Line 34: switch(vpd) { space required before the open parenthesis '('
-
build bot (Jenkins) (Code Review)