coreboot-gerrit October 2022

coreboot-gerrit@coreboot.org

1 participants
5881 discussions

[S] Change in coreboot[master]: MAINTAINERS: Update maintainers for several Google projects
by Subrata Banik (Code Review) 05 Oct '22

05 Oct '22

Attention is currently required from: Noga Smith, Tarun Tuli. Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/67289 ) Change subject: MAINTAINERS: Update maintainers for several Google projects ...................................................................... Patch Set 3: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/67289 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I973b0abf8a82189df1495e3bcd9bae452a5be827 Gerrit-Change-Number: 67289 Gerrit-PatchSet: 3 Gerrit-Owner: Tarun Tuli <taruntuli(a)google.com> Gerrit-Reviewer: Noga Smith <nogasmith(a)google.com> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Noga Smith <nogasmith(a)google.com> Gerrit-Attention: Tarun Tuli <taruntuli(a)google.com> Gerrit-Comment-Date: Wed, 05 Oct 2022 18:03:01 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[L] Change in coreboot[master]: [WIP] mb/prodrive/atlas: Populate smbios table with VPD from ECs EMI
by Angel Pons (Code Review) 05 Oct '22

05 Oct '22

Attention is currently required from: Christian Walter, Lean Sheng Tan, David.Milosevic(a)9elements.com. Angel Pons 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: (2 comments) Patchset: PS1: Would be nice to make the build bot happy File src/mainboard/prodrive/atlas/emi.c: https://review.coreboot.org/c/coreboot/+/68137/comment/208566cc_a458907c PS1, Line 15: #define EMI_1_BASE EMI_0_BASE + APPLICATION_ID + 0x1 > Macros with complex values should be enclosed in parentheses Otherwise, the expansion can cause operation order problems. #define EMI_1_BASE (EMI_0_BASE + APPLICATION_ID + 0x1) Also, what is APPLICATION_ID? -- To view, visit https://review.coreboot.org/c/coreboot/+/68137 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I47bb4883c43ff344a9bda92c3106dd025533b391 Gerrit-Change-Number: 68137 Gerrit-PatchSet: 1 Gerrit-Owner: David.Milosevic(a)9elements.com Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Christian Walter <christian.walter(a)9elements.com> Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Christian Walter <christian.walter(a)9elements.com> Gerrit-Attention: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Attention: David.Milosevic(a)9elements.com Gerrit-Comment-Date: Wed, 05 Oct 2022 17:04:29 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[L] Change in libgfxinit[master]: common/gma-registers: Update for Tiger Lake
by Nico Huber (Code Review) 05 Oct '22

05 Oct '22

Attention is currently required from: Tim Wawrzynczak, Angel Pons. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/libgfxinit/+/67489 ) Change subject: common/gma-registers: Update for Tiger Lake ...................................................................... Patch Set 1: Verified+1 Code-Review+1 (6 comments) Patchset: PS1: Great work! I left some comments but the numbers seem right so far :) File common/hw-gfx-gma-registers.ads: https://review.coreboot.org/c/libgfxinit/+/67489/comment/c9e38e70_01923099 PS1, Line 1787: GEN9_CHICKEN_DPCR_3 Linux calls it GEN9_CLKGATE_DIS_3, c/p error maybe? https://review.coreboot.org/c/libgfxinit/+/67489/comment/2ba00596_a3088e1c PS1, Line 1987: HIP_INDEX_REG1 => 16#10_10a4# / Register_Width, Given that the indexed registers below are (almost?) grouped by the index they hide behind, can we comment the index above the groups? https://review.coreboot.org/c/libgfxinit/+/67489/comment/69920d9c_ae9a8d06 PS1, Line 2030: COLD Isn't it COLD*ST*? That's what I see in Linux and (not sure if related yet) in the register reference. https://review.coreboot.org/c/libgfxinit/+/67489/comment/8f268db8_bf960064 PS1, Line 2120: PLANE_COLOR_CTL_1_C => 16#07_21cc# / Register_Width, Isn't there a fourth pipe now? https://review.coreboot.org/c/libgfxinit/+/67489/comment/7b420af0_8b911f7a PS1, Line 2134: PORT_TX_DFLEXDPPMS_FIA3 => 16#16_f890# / Register_Width, : PORT_TX_DFLEXDPMLE1_FIA3 => 16#16_f8c0# / Register_Width, These two lines are switched compared to the respective ones above. I assume they were supposed to be sorted by name? (numbers look right) -- To view, visit https://review.coreboot.org/c/libgfxinit/+/67489 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: libgfxinit Gerrit-Branch: master Gerrit-Change-Id: I61331746365a80c9f953e7d75749f138a2e9adac Gerrit-Change-Number: 67489 Gerrit-PatchSet: 1 Gerrit-Owner: Tim Wawrzynczak <inforichland(a)gmail.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Tim Wawrzynczak <inforichland(a)gmail.com> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Wed, 05 Oct 2022 16:34:30 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[L] Change in coreboot[master]: [WIP] mb/prodrive/atlas: Populate smbios table with VPD from ECs EMI
by build bot (Jenkins) (Code Review) 05 Oct '22

05 Oct '22

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 '(' -- To view, visit https://review.coreboot.org/c/coreboot/+/68137 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I47bb4883c43ff344a9bda92c3106dd025533b391 Gerrit-Change-Number: 68137 Gerrit-PatchSet: 1 Gerrit-Owner: David.Milosevic(a)9elements.com Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Christian Walter <christian.walter(a)9elements.com> Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-CC: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Christian Walter <christian.walter(a)9elements.com> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Comment-Date: Wed, 05 Oct 2022 16:22:43 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[S] Change in coreboot[master]: [do not merge] mb/prodrive/atlas: for testing
by build bot (Jenkins) (Code Review) 05 Oct '22

05 Oct '22

Attention is currently required from: Christian Walter, Angel Pons, David.Milosevic(a)9elements.com. build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/67415 ) Change subject: [do not merge] mb/prodrive/atlas: for testing ...................................................................... Patch Set 7: Verified-1 -- To view, visit https://review.coreboot.org/c/coreboot/+/67415 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I37c686aacea130c99e8ab2fe984b150a7ad6f82d Gerrit-Change-Number: 67415 Gerrit-PatchSet: 7 Gerrit-Owner: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Christian Walter <christian.walter(a)9elements.com> Gerrit-Reviewer: Maximilian Brune <maximilian.brune(a)9elements.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Christian Walter <christian.walter(a)9elements.com> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: David.Milosevic(a)9elements.com Gerrit-Comment-Date: Wed, 05 Oct 2022 16:22:05 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[L] Change in coreboot[master]: [WIP] mb/prodrive/atlas: Populate smbios table with VPD from ECs EMI
by Name of user not set (Code Review) 05 Oct '22

05 Oct '22

David.Milosevic(a)9elements.com has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/68137 ) Change subject: [WIP] mb/prodrive/atlas: Populate smbios table with VPD from ECs EMI ...................................................................... [WIP] mb/prodrive/atlas: Populate smbios table with VPD from ECs EMI The embedded controller stores serial-number and part-number within an EEPROM which is unreachable to the Host CPU. Therefore, on the EC side, we set up a shared memory region with the help of EMI and copy the VPDs to it. On the coreboot side, we can read those back and populate the SMBios table type 1 with the serial-number and type 11 with the part-number. emi.*: small interface to access the EMI regions vpd.*: uses the emi interface in order to read the VPDs smbios.c: populates smbios type 1 with the serial-number by using vpd.h mainboard.c: uses vpd.h to add a string to smbios type 11 ld_config.*: offers functionality to configure logical devices devicetree.cb: enables address range 0xc00-0xcff for the EMI runtime registers Makefile.inc: adds emi.c, vpd.c, smbios.c, ld_config.c to the ramstage compilation TODO: ld_config.* is redundant. We can replace this with pnp_device.h or pnp_ops.h Signed-off-by: Milo-D <David.Milosevic(a)9elements.com> Change-Id: I47bb4883c43ff344a9bda92c3106dd025533b391 --- M src/mainboard/prodrive/atlas/Makefile.inc M src/mainboard/prodrive/atlas/devicetree.cb A src/mainboard/prodrive/atlas/emi.c A src/mainboard/prodrive/atlas/emi.h A src/mainboard/prodrive/atlas/ld_config.c A src/mainboard/prodrive/atlas/ld_config.h M src/mainboard/prodrive/atlas/mainboard.c A src/mainboard/prodrive/atlas/smbios.c A src/mainboard/prodrive/atlas/vpd.c A src/mainboard/prodrive/atlas/vpd.h 10 files changed, 503 insertions(+), 2 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/37/68137/1 diff --git a/src/mainboard/prodrive/atlas/Makefile.inc b/src/mainboard/prodrive/atlas/Makefile.inc index 1446e54..1152771 100644 --- a/src/mainboard/prodrive/atlas/Makefile.inc +++ b/src/mainboard/prodrive/atlas/Makefile.inc @@ -7,3 +7,7 @@ ramstage-y += gpio.c ramstage-y += mainboard.c +ramstage-y += ld_config.c +ramstage-y += smbios.c +ramstage-y += emi.c +ramstage-y += vpd.c diff --git a/src/mainboard/prodrive/atlas/devicetree.cb b/src/mainboard/prodrive/atlas/devicetree.cb index 7ca5c88..f15ceee 100644 --- a/src/mainboard/prodrive/atlas/devicetree.cb +++ b/src/mainboard/prodrive/atlas/devicetree.cb @@ -14,6 +14,9 @@ # EC memory map range is 0x900-0x9ff register "gen3_dec" = "0x00fc0901" + # EC EMI-0 range is 0xc00 - 0xcff + register "gen4_dec" = "0x00fc0c01" + # SaGv Configuration register "sagv" = "CONFIG(ATLAS_ENABLE_SAGV) ? SaGv_Enabled : SaGv_Disabled" diff --git a/src/mainboard/prodrive/atlas/emi.c b/src/mainboard/prodrive/atlas/emi.c new file mode 100644 index 0000000..cf03687 --- /dev/null +++ b/src/mainboard/prodrive/atlas/emi.c @@ -0,0 +1,148 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ + +#include <arch/io.h> +#include <console/console.h> + +#include "ld_config.h" +#include "emi.h" + +#define ESPI_IO_COMPONENT 0x0d + +#define EMI_0_CONFIG_INDEX 0x68 +#define EMI_1_CONFIG_INDEX 0x6c + +#define EMI_0_BASE 0xc00 +#define EMI_1_BASE EMI_0_BASE + APPLICATION_ID + 0x1 + +#define encode_address_lsb(addr, access) ((((addr) & 0x00fc) >> 0) | (access)) +#define encode_address_msb(addr, region) ((((addr) & 0x7f00) >> 8) | ((region) << 7)) + +/* --- Forward Declaration of internal Functionality --- */ + +static inline u16 emi_base(const EMI_INSTANCE instance); +static inline u8 emi_index(const EMI_INSTANCE instance); + +/* --- Exposed Functionality --- */ + +void emi_init(const EMI_INSTANCE instance) { + + ld_config_enter(); + + const u16 base = emi_base(instance); + const u8 index = emi_index(instance); + + ld_config_write(ESPI_IO_COMPONENT, index + 2, (base & 0x00ff) >> 0); + ld_config_write(ESPI_IO_COMPONENT, index + 3, (base & 0xff00) >> 8); + ld_config_write(ESPI_IO_COMPONENT, index + 0, 0b00000001); // valid bit + + ld_config_leave(); +} + +int emi_read_region(const EMI_INSTANCE instance, const EMI_REGION region, const EMI_ACCESS access, const u16 addr, u8 *buff) { + + const u16 base = emi_base(instance); + + outb(encode_address_lsb(addr, access), base + EC_ADDRESS_LSB); + outb(encode_address_msb(addr, region), base + EC_ADDRESS_MSB); + + switch(access) { + + case EMI_ACCESS_8BIT: + + *((u8*) buff) = inb(base + EC_DATA_BYTE_0); + + break; + + case EMI_ACCESS_16BIT: + + *((u16*) buff) = inw(base + EC_DATA_BYTE_0); + + break; + + case EMI_ACCESS_32BIT: + case EMI_ACCESS_32BIT_AUTO_INC: + + *((u32*) buff) = inl(base + EC_DATA_BYTE_0); + + break; + + default: printk(BIOS_NOTICE, "emi: invalid access.\n"); return EMI_FAILURE; + } + + return EMI_SUCCESS; +} + +int emi_write_region(const EMI_INSTANCE instance, const EMI_REGION region, const EMI_ACCESS access, const u16 addr, u8 *bytes) { + + const u16 base = emi_base(instance); + + outb(encode_address_lsb(addr, access), base + EC_ADDRESS_LSB); + outb(encode_address_msb(addr, region), base + EC_ADDRESS_MSB); + + switch(access) { + + case EMI_ACCESS_8BIT: + + outb(*((u8*) bytes), base + EC_DATA_BYTE_0); + + break; + + case EMI_ACCESS_16BIT: + + outw(*((u16*) bytes), base + EC_DATA_BYTE_0); + + break; + + case EMI_ACCESS_32BIT: + case EMI_ACCESS_32BIT_AUTO_INC: + + outl(*((u32*) bytes), base + EC_DATA_BYTE_0); + + break; + + default: printk(BIOS_NOTICE, "emi: invalid access.\n"); return EMI_FAILURE; + } + + return EMI_SUCCESS; +} + +void emi_read_region_block(const EMI_INSTANCE instance, const EMI_REGION region, const u16 addr, const u16 n, u8 *buff) { + + u16 idx = 0; + + for(;;) { + + u8 bytes[4] = { 0x0, 0x0, 0x0, 0x0 }; + emi_read_region(instance, region, EMI_ACCESS_32BIT, addr + idx, bytes); + + for(u8 i = 0; i < 4; i++) { + + if(idx == n) + return; + + buff[idx++] = bytes[i]; + } + } +} + +inline void emi_write_register(const EMI_INSTANCE instance, const u8 offs, const u8 value) { + + outb(value, emi_base(instance) + offs); +} + +inline u8 emi_read_register(const EMI_INSTANCE instance, const u8 offs) { + + return inb(emi_base(instance) + offs); +} + +/* --- Internal Functionality --- */ + +static inline u16 emi_base(const EMI_INSTANCE instance) { + + return (instance == EMI_INSTANCE_0) ? EMI_0_BASE : EMI_1_BASE; +} + +static inline u8 emi_index(const EMI_INSTANCE instance) { + + return (instance == EMI_INSTANCE_0) ? EMI_0_CONFIG_INDEX : EMI_1_CONFIG_INDEX; +} diff --git a/src/mainboard/prodrive/atlas/emi.h b/src/mainboard/prodrive/atlas/emi.h new file mode 100644 index 0000000..e01de45 --- /dev/null +++ b/src/mainboard/prodrive/atlas/emi.h @@ -0,0 +1,113 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ + +#ifndef __EMI_H__ +#define __EMI_H__ + +#include <stdint.h> + +#define EMI_SUCCESS 0 +#define EMI_FAILURE 1 + +#define HOST_EC_MBOX 0x00 +#define EC_HOST_MBOX 0x01 +#define EC_ADDRESS_LSB 0x02 +#define EC_ADDRESS_MSB 0x03 +#define EC_DATA_BYTE_0 0x04 +#define EC_DATA_BYTE_1 0x05 +#define EC_DATA_BYTE_2 0x06 +#define EC_DATA_BYTE_3 0x07 +#define INTERRUPT_SOURCE_LSB 0x08 +#define INTERRUPT_SOURCE_MSB 0x09 +#define INTERRUPT_MASK_LSB 0x0a +#define INTERRUPT_MASK_MSB 0x0b +#define APPLICATION_ID 0x0c + +/* + * === Embedded Memory Interface Library === + * + * A small library for accessing EMI regions within + * the embedded controller (MEC152x). + * + * */ + +typedef enum { + + EMI_INSTANCE_0, ///< EMI instance 0 (EMI0) + EMI_INSTANCE_1 ///< EMI instance 1 (EMI1) + +} EMI_INSTANCE; + +typedef enum { + + EMI_REGION_0, ///< EMI region 0 + EMI_REGION_1 ///< EMI region 1 + +} EMI_REGION; + +typedef enum { + + EMI_ACCESS_8BIT = 0b00, ///< 8-bit read/write access + EMI_ACCESS_16BIT = 0b01, ///< 16-bit read/write access + EMI_ACCESS_32BIT = 0b10, ///< 32-bit read/write access + EMI_ACCESS_32BIT_AUTO_INC = 0b11 ///< 32-bit read/write access (auto increment) + +} EMI_ACCESS; + +/* + * emi_init - initialize a specific EMI instance + * @instance: the EMI instance to initialize + * + * [note] it is mandatory to call this function before using + * the rest of the interface. + * */ +extern void emi_init(const EMI_INSTANCE instance); + +/* + * emi_read_region - read from an EMI region at a specific address + * @instance: the desired EMI instance + * @region: the EMI region to read from + * @access: read access type (8-bit read, 16-bit read, ...) + * @addr: the target address (dword aligned) + * @buff: a byte buffer to store contents of the EMI region + * => returns EMI_SUCCESS on success and EMI_FAILURE on failure + * */ +extern int emi_read_region(const EMI_INSTANCE instance, const EMI_REGION region, const EMI_ACCESS access, const u16 addr, u8 *buff); + +/* + * emi_write_region - write to an EMI region at a specific address + * @instance: the desired EMI instance + * @region: the EMI region to write to + * @access: write access type (8-bit write, 16-bit write, ...) + * @addr: the target address (dword aligned) + * @bytes: bytes to write + * => returns EMI_SUCCESS on success and EMI_FAILURE on failure + * */ +extern int emi_write_region(const EMI_INSTANCE instance, const EMI_REGION region, const EMI_ACCESS access, const u16 addr, u8 *bytes); + +/* + * emi_read_region_block - read a sequential block from an EMI region + * @instance: the desired EMI instance + * @region: the EMI region to read from + * @addr: base address for block read (dword aligned) + * @n: the number of bytes to read from the region + * @buff: a byte buffer to store contents of the EMI region + * */ +extern void emi_read_region_block(const EMI_INSTANCE instance, const EMI_REGION region, const u16 addr, const u16 n, u8 *buff); + +/* + * emi_write_register - write to an EMI runtime register + * @instance: the desired EMI instance + * @offs: runtime register offset + * @value: the value to write to the runtime register + * */ +extern void emi_write_register(const EMI_INSTANCE instance, const u8 offs, const u8 value); + +/* + * emi_read_register - read from an EMI runtime register + * @instance: the desired EMI instance + * @offs: runtime register offset + * => returns a byte from a runtime register + * */ +extern u8 emi_read_register(const EMI_INSTANCE instance, const u8 offs); + +#endif diff --git a/src/mainboard/prodrive/atlas/ld_config.c b/src/mainboard/prodrive/atlas/ld_config.c new file mode 100644 index 0000000..c03ada4 --- /dev/null +++ b/src/mainboard/prodrive/atlas/ld_config.c @@ -0,0 +1,45 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ + +#include <arch/io.h> +#include "ld_config.h" + +#define CONFIG_PORT_INDEX 0x2e +#define CONFIG_PORT_DATA 0x2f + +#define CONFIG_ENTRY_KEY 0x55 +#define CONFIG_EXIT_KEY 0xaa + +#define LDN_CONFIG_REGISTER 0x07 + +/* + * This file is redundant and should only be used temporarily within + * the WIP. We can use things like pnp_device.h instead. + * */ + + +void ld_config_enter(void) { + + outb(CONFIG_ENTRY_KEY, CONFIG_PORT_INDEX); +} + +void ld_config_leave(void) { + + outb(CONFIG_EXIT_KEY, CONFIG_PORT_INDEX); +} + +void ld_config_write(const u8 ldn, const u8 index, const u8 value) { + + outb(LDN_CONFIG_REGISTER, CONFIG_PORT_INDEX); + outb(ldn, CONFIG_PORT_DATA); + outb(index, CONFIG_PORT_INDEX); + outb(value, CONFIG_PORT_DATA); +} + +u8 ld_config_read(const u8 ldn, const u8 index) { + + outb(LDN_CONFIG_REGISTER, CONFIG_PORT_INDEX); + outb(ldn, CONFIG_PORT_DATA); + outb(index, CONFIG_PORT_INDEX); + + return inb(CONFIG_PORT_DATA); +} \ No newline at end of file diff --git a/src/mainboard/prodrive/atlas/ld_config.h b/src/mainboard/prodrive/atlas/ld_config.h new file mode 100644 index 0000000..bc2306b --- /dev/null +++ b/src/mainboard/prodrive/atlas/ld_config.h @@ -0,0 +1,37 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ + +#ifndef __LD_CONFIG_H__ +#define __LD_CONFIG_H__ + +/* + * This file is redundant and should only be used temporarily within + * the WIP. We can use things like pnp_device.h instead. + * */ + +/* + * ld_config_enter - enter logical device config mode + * */ +extern void ld_config_enter(void); + +/* + * ld_config_leave - leave logical device config mode + * */ +extern void ld_config_leave(void); + +/* + * ld_config_write - write to a logical device configuration register + * @ldn: logical device number + * @index: index of configuration register + * @value: value to be written to @index + * */ +extern void ld_config_write(const u8 ldn, const u8 index, const u8 value); + +/* + * ld_config_read - read from a logical device configuration register + * @ldn: logical device number + * @index: index of configuration register + * => returns the content of the configuration register + * */ +extern u8 ld_config_read(const u8 ldn, const u8 index); + +#endif diff --git a/src/mainboard/prodrive/atlas/mainboard.c b/src/mainboard/prodrive/atlas/mainboard.c index 036208a..1587d6b 100644 --- a/src/mainboard/prodrive/atlas/mainboard.c +++ b/src/mainboard/prodrive/atlas/mainboard.c @@ -1,15 +1,50 @@ /* SPDX-License-Identifier: GPL-2.0-only */ -#include <device/device.h> #include <stdint.h> +#include <string.h> +#include <smbios.h> +#include <device/device.h> +#include "vpd.h" #include "gpio.h" +/* <--- Forward Declaration of smbios Helpers ---> */ + +static void get_smbios_strings(struct device *dev, struct smbios_type11 *t); +static const char* get_smbios_part_number(void); + +/* <--- Mainboard Chip Operations ---> */ + static void mainboard_init(void *chip_info) { configure_gpio_pads(); } +static void mainboard_enable(struct device *dev) { + + dev->ops->get_smbios_strings = get_smbios_strings; +} + struct chip_operations mainboard_ops = { - .init = mainboard_init, + + .init = mainboard_init, + .enable_dev = mainboard_enable }; + +/* <--- smbios Helpers ---> */ + +static void get_smbios_strings(struct device *dev, struct smbios_type11 *t) { + + t->count = smbios_add_string(t->eos, get_smbios_part_number()); +} + +static const char* get_smbios_part_number(void) { + + const char prefix[5] = "P/N: "; + static u8 pn[sizeof(prefix) + PN_LENGTH + 1] = { '\x00' }; + + strncpy((char*) pn, prefix, sizeof(prefix)); + vpd_get(vpd(part_number), pn + sizeof(prefix)); + + return (char*) pn; +} diff --git a/src/mainboard/prodrive/atlas/smbios.c b/src/mainboard/prodrive/atlas/smbios.c new file mode 100644 index 0000000..8d66d8c --- /dev/null +++ b/src/mainboard/prodrive/atlas/smbios.c @@ -0,0 +1,14 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ + +#include <string.h> +#include <smbios.h> + +#include "vpd.h" + +const char* smbios_mainboard_serial_number(void) { + + static u8 sn[SN_LENGTH + 1] = { '\x00' }; + vpd_get(vpd(serial_number), sn); + + return (char*) sn; +} diff --git a/src/mainboard/prodrive/atlas/vpd.c b/src/mainboard/prodrive/atlas/vpd.c new file mode 100644 index 0000000..01e6219 --- /dev/null +++ b/src/mainboard/prodrive/atlas/vpd.c @@ -0,0 +1,41 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ + +#include <types.h> + +#include "emi.h" +#include "vpd.h" + +/* <--- Globals ---> */ + +static bool emi_initialized = false; + +/* <--- Forward Declaration of Internal Functionality ---> */ + +static u16 vpd_length(const u16 vpd); + +/* <--- Exposed Functionality ---> */ + +void vpd_get(const u16 vpd, u8 *buff) { + + if(emi_initialized == false) { + + emi_init(EMI_INSTANCE_0); + emi_initialized = true; + } + + const u16 len = vpd_length(vpd); + emi_read_region_block(EMI_INSTANCE_0, EMI_REGION_0, vpd, len, buff); +} + +/* <--- Internal Functionality ---> */ + +static u16 vpd_length(const u16 vpd) { + + switch(vpd) { + + case vpd(part_number): return PN_LENGTH; + case vpd(serial_number): return SN_LENGTH; + + default: return 0; + } +} diff --git a/src/mainboard/prodrive/atlas/vpd.h b/src/mainboard/prodrive/atlas/vpd.h new file mode 100644 index 0000000..e0f0bf5 --- /dev/null +++ b/src/mainboard/prodrive/atlas/vpd.h @@ -0,0 +1,34 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ + +#ifndef __VPD_H__ +#define __VPD_H__ + +#include <stddef.h> + +#define PAGESIZE 16 +#define PN_LENGTH 12 ///< part_number, without nullbyte +#define SN_LENGTH 13 ///< serial_number, without nullbyte + +typedef struct __attribute__((__packed__)) vpd_section { + + char part_number [PAGESIZE]; + char serial_number[PAGESIZE]; + +} vpd_section_t; + +/* + * pass this macro as vpd argument, where + * x is a member of vpd_section_t. For example, + * vpd(serial_number) to access the serial number + * within the vpd section. + * */ +#define vpd(x) offsetof(vpd_section_t, x) + +/* + * vpd_get - retrieve a vpd member + * @vpd: target vpd. use vpd(x) macro + * @buff: byte buffer to store the vpd + * */ +extern void vpd_get(const u16 vpd, u8 *buff); + +#endif \ No newline at end of file -- To view, visit https://review.coreboot.org/c/coreboot/+/68137 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I47bb4883c43ff344a9bda92c3106dd025533b391 Gerrit-Change-Number: 68137 Gerrit-PatchSet: 1 Gerrit-Owner: David.Milosevic(a)9elements.com Gerrit-MessageType: newchange

1 0

[XL] Change in coreboot[master]: payloads/cbui: Add new payload CBUI
by build bot (Jenkins) (Code Review) 05 Oct '22

05 Oct '22

Attention is currently required from: Iman Bingi, Martin L Roth, Julius Werner, Patrick Rudolph. build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/23586 ) Change subject: payloads/cbui: Add new payload CBUI ...................................................................... Patch Set 47: (32 comments) File payloads/cbui/NuklearUI/NuklearDatePicker.c: Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-159705): https://review.coreboot.org/c/coreboot/+/23586/comment/8fad1a92_c8d5cb8e PS47, Line 36: case 6: /* fallthrough */ Prefer 'fallthrough;' over fallthrough comment Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-159705): https://review.coreboot.org/c/coreboot/+/23586/comment/e7f62e9b_17eb464d PS47, Line 37: case 9: /* fallthrough */ Prefer 'fallthrough;' over fallthrough comment File payloads/cbui/NuklearUI/NuklearRW.c: Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-159705): https://review.coreboot.org/c/coreboot/+/23586/comment/d65fc6ae_641172d4 PS47, Line 218: static const char *titles[] = { "Hex-View", "Header", "Index/Data" }; static const char * array should probably be static const char * const Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-159705): https://review.coreboot.org/c/coreboot/+/23586/comment/fb2cae7d_08731760 PS47, Line 252: static const char *entries[] = { "NVRAM" }; static const char * array should probably be static const char * const File payloads/cbui/arch/x86/int32.h: Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-159705): https://review.coreboot.org/c/coreboot/+/23586/comment/24613629_3f06d393 PS47, Line 20: typedef struct __attribute__ ((packed)) { Prefer __packed over __attribute__((packed)) File payloads/cbui/arch/x86/int32.nasm: Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-159705): https://review.coreboot.org/c/coreboot/+/23586/comment/6dd41819_36684f28 PS47, Line 38: struc regs16_t 'struc' may be misspelled - perhaps 'struct'? File payloads/cbui/gfx/vbe.c: Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-159705): https://review.coreboot.org/c/coreboot/+/23586/comment/9a95187b_4653f8cb PS47, Line 24: static u8 low_mem[0x1000] __attribute__ ((section(".lowmem"))); Prefer __section(".lowmem") over __attribute__((section(".lowmem"))) File payloads/cbui/lang/de.c: Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-159705): https://review.coreboot.org/c/coreboot/+/23586/comment/42bdb4c1_e9c8bc06 PS47, Line 39: "Kein Framebuffer gefunden. CBUI funktioniert nicht" \ Avoid unnecessary line continuations Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-159705): https://review.coreboot.org/c/coreboot/+/23586/comment/10f3fa6a_b1a841ef PS47, Line 44: "Der coreboot framebuffer hat keine physikalische " \ Avoid unnecessary line continuations Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-159705): https://review.coreboot.org/c/coreboot/+/23586/comment/26b7a76c_0efbb5e7 PS47, Line 47: "Kein Platz auf dem Heap.\nBitte vergroBere den" \ Avoid unnecessary line continuations Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-159705): https://review.coreboot.org/c/coreboot/+/23586/comment/4dfe1c97_b2476035 PS47, Line 50: "Konnte die nuklear Bibliothek nicht intitialisieren.\nBitte" \ Avoid unnecessary line continuations Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-159705): https://review.coreboot.org/c/coreboot/+/23586/comment/b2bc29a0_a9db4617 PS47, Line 52: "stelle sicher, das der Framebuffer entweder das RGBX8 oder " \ Avoid unnecessary line continuations Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-159705): https://review.coreboot.org/c/coreboot/+/23586/comment/403b1970_00d27e05 PS47, Line 65: "Fehler: Keine CMOS Eintrage gefunden. Bitte schalte die " \ Avoid unnecessary line continuations Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-159705): https://review.coreboot.org/c/coreboot/+/23586/comment/25cdee78_ed2378a9 PS47, Line 86: "Fehler: Keine Zeitstempel gefunden. Bitte schalte die " \ Avoid unnecessary line continuations Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-159705): https://review.coreboot.org/c/coreboot/+/23586/comment/44050b21_6eb6fdad PS47, Line 89: "Fehler: Bootlog nicht gefunden. Bitte schalte die Kconfig " \ Avoid unnecessary line continuations File payloads/cbui/lang/en.c: Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-159705): https://review.coreboot.org/c/coreboot/+/23586/comment/496b5f24_3d13f07c PS47, Line 38: "No framebuffer found. CBUI doesn't work in text mode.\n" \ Avoid unnecessary line continuations Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-159705): https://review.coreboot.org/c/coreboot/+/23586/comment/ad9ce094_53828c19 PS47, Line 46: "Failed to init nuklear library.\nMake sure you have enough " \ Avoid unnecessary line continuations Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-159705): https://review.coreboot.org/c/coreboot/+/23586/comment/e456ccc0_6653bf2e PS47, Line 60: "ERROR: No CMOS options found. Please make sure to enable " \ Avoid unnecessary line continuations Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-159705): https://review.coreboot.org/c/coreboot/+/23586/comment/0630f009_946db4a6 PS47, Line 81: "ERROR: No timestamps found. Please make sure to enable " \ Avoid unnecessary line continuations Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-159705): https://review.coreboot.org/c/coreboot/+/23586/comment/9b3c08f7_ca4339de PS47, Line 84: "ERROR: Bootlog not found. Please make sure to enable " \ Avoid unnecessary line continuations File payloads/cbui/modules/bootlog_module.c: Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-159705): https://review.coreboot.org/c/coreboot/+/23586/comment/426dba95_9300ab8a PS47, Line 28: } __attribute__ ((__packed__)); Prefer __packed over __attribute__((__packed__)) Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-159705): https://review.coreboot.org/c/coreboot/+/23586/comment/ca8240e1_86c1d159 PS47, Line 82: } void function return statements are not generally useful File payloads/cbui/modules/cbfs_module.c: Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-159705): https://review.coreboot.org/c/coreboot/+/23586/comment/429ea7bd_60bc359e PS47, Line 50: } __attribute__ ((packed)); Prefer __packed over __attribute__((packed)) Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-159705): https://review.coreboot.org/c/coreboot/+/23586/comment/8a4b663b_aae4a200 PS47, Line 59: } __attribute__ ((packed)); Prefer __packed over __attribute__((packed)) File payloads/cbui/modules/cpuinfo_module.c: Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-159705): https://review.coreboot.org/c/coreboot/+/23586/comment/4fd8898a_26048c70 PS47, Line 35: static const char *intel_cap_edx_flags[] = { static const char * array should probably be static const char * const Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-159705): https://review.coreboot.org/c/coreboot/+/23586/comment/08d4141f_f34ec283 PS47, Line 43: static const char *amd_cap_edx_flags[] = { static const char * array should probably be static const char * const Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-159705): https://review.coreboot.org/c/coreboot/+/23586/comment/43b08504_09a03ef8 PS47, Line 51: static const char *intel_cap_ecx_flags[] = { static const char * array should probably be static const char * const Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-159705): https://review.coreboot.org/c/coreboot/+/23586/comment/54f8287f_8043e805 PS47, Line 59: static const char *amd_cap_ecx_flags[] = { static const char * array should probably be static const char * const Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-159705): https://review.coreboot.org/c/coreboot/+/23586/comment/03e6274b_661655bc PS47, Line 67: static const char *intel_cap_extended_edx_flags[] = { static const char * array should probably be static const char * const Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-159705): https://review.coreboot.org/c/coreboot/+/23586/comment/f2b241cc_da492d32 PS47, Line 75: static const char *intel_cap_extended_ecx_flags[] = { static const char * array should probably be static const char * const Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-159705): https://review.coreboot.org/c/coreboot/+/23586/comment/8e4510b3_3cc62626 PS47, Line 83: static const char *amd_cap_extended_edx_flags[] = { static const char * array should probably be static const char * const Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-159705): https://review.coreboot.org/c/coreboot/+/23586/comment/53d30834_e9e11d28 PS47, Line 92: static const char *amd_cap_extended_ecx_flags[] = { static const char * array should probably be static const char * const -- To view, visit https://review.coreboot.org/c/coreboot/+/23586 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ib9a1a07c1065880aa675380625021750d5cab7d1 Gerrit-Change-Number: 23586 Gerrit-PatchSet: 47 Gerrit-Owner: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Iman Bingi <imanbingy(a)gmail.com> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Attention: Iman Bingi <imanbingy(a)gmail.com> Gerrit-Attention: Martin L Roth <gaumless(a)gmail.com> Gerrit-Attention: Julius Werner <jwerner(a)chromium.org> Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Comment-Date: Wed, 05 Oct 2022 15:56:37 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[XL] Change in coreboot[master]: payloads/cbui: Add new payload CBUI
by Iman Bingi (Code Review) 05 Oct '22

05 Oct '22

Attention is currently required from: Iman Bingi, Martin L Roth, Julius Werner, Patrick Rudolph. Iman Bingi has uploaded a new patch set (#47) to the change originally created by Patrick Rudolph. ( https://review.coreboot.org/c/coreboot/+/23586 ) Change subject: payloads/cbui: Add new payload CBUI ...................................................................... payloads/cbui: Add new payload CBUI Depends on libpayload and nuklear. Features: * Graphical menus with scrolling. * Text rendering engine (atm only bitmap font) * Support for keyboard and mouse * Support for USB and PS/2 devices * Ported coreinfo and nvramcui * Allows to modify NVRAM and RTC * Works as ELF payload * Works as Seabios secondary payload * Basic support for multiple languages * Hacky support for BIOS calls (depends on NASM) * Runs in qemu and on real hardware * Use linker script to allocate low memory Shortcomings: * Doesn't work in VGA text mode * Untested on UEFI * int32 relocates itself to low memory Licenses: * GPLv2 (CBUI + libpayload) * BSD (libpayload) * MIT (nuklear) TODO: * Test on as much platforms as possible * Link int32 into low memory This is Patrick Rudolph's original patch, updated by Ben Adu-Boahen to: * Add Read/Write module * This module allows read/write to any hardware component that is readable/writeable Note: This is work in progress Change-Id: Ib9a1a07c1065880aa675380625021750d5cab7d1 Signed-off-by: Patrick Rudolph <siro(a)das-labor.org> Signed-off-by: Ben Adu-Boahen <imanbingy(a)gmail.com> --- M payloads/Kconfig M payloads/Makefile.inc A payloads/cbui/.gitignore A payloads/cbui/Kconfig A payloads/cbui/Makefile A payloads/cbui/NuklearUI/NuklearCombo.c A payloads/cbui/NuklearUI/NuklearCombo.h A payloads/cbui/NuklearUI/NuklearCommon.h A payloads/cbui/NuklearUI/NuklearDataGrid.c A payloads/cbui/NuklearUI/NuklearDataGrid.h A payloads/cbui/NuklearUI/NuklearDatePicker.c A payloads/cbui/NuklearUI/NuklearDatePicker.h A payloads/cbui/NuklearUI/NuklearFieldHex.c A payloads/cbui/NuklearUI/NuklearFieldHex.h A payloads/cbui/NuklearUI/NuklearFileChooser.c A payloads/cbui/NuklearUI/NuklearFileChooser.h A payloads/cbui/NuklearUI/NuklearGroup.c A payloads/cbui/NuklearUI/NuklearGroup.h A payloads/cbui/NuklearUI/NuklearHex.c A payloads/cbui/NuklearUI/NuklearHex.h A payloads/cbui/NuklearUI/NuklearIntegerRange.c A payloads/cbui/NuklearUI/NuklearIntegerRange.h A payloads/cbui/NuklearUI/NuklearLabel.c A payloads/cbui/NuklearUI/NuklearLabel.h A payloads/cbui/NuklearUI/NuklearObject.c A payloads/cbui/NuklearUI/NuklearObject.h A payloads/cbui/NuklearUI/NuklearPciHeader.c A payloads/cbui/NuklearUI/NuklearPciHeader.h A payloads/cbui/NuklearUI/NuklearRW.c A payloads/cbui/NuklearUI/NuklearRW.h A payloads/cbui/NuklearUI/NuklearRoot.c A payloads/cbui/NuklearUI/NuklearStyle.c A payloads/cbui/NuklearUI/NuklearStyle.h A payloads/cbui/NuklearUI/NuklearTabView.c A payloads/cbui/NuklearUI/NuklearTextView.c A payloads/cbui/NuklearUI/NuklearTextView.h A payloads/cbui/NuklearUI/NuklearTextfield.c A payloads/cbui/NuklearUI/NuklearTextfield.h A payloads/cbui/NuklearUI/NuklearTimePicker.c A payloads/cbui/NuklearUI/NuklearTimePicker.h A payloads/cbui/NuklearUI/NuklearUI.h A payloads/cbui/NuklearUI/NuklearVector.c A payloads/cbui/NuklearUI/NuklearVector.h A payloads/cbui/arch/x86/cpuid.c A payloads/cbui/arch/x86/cpuid.h A payloads/cbui/arch/x86/int32.h A payloads/cbui/arch/x86/int32.ld A payloads/cbui/arch/x86/int32.nasm A payloads/cbui/arch/x86/memcpy.c A payloads/cbui/arch/x86/memcpy.h A payloads/cbui/arch/x86/vga.c A payloads/cbui/arch/x86/vga.h A payloads/cbui/cbui.c A payloads/cbui/cbui.h A payloads/cbui/fsys/usbstorage.c A payloads/cbui/fsys/usbstorage.h A payloads/cbui/gfx/coreboot.c A payloads/cbui/gfx/coreboot.h A payloads/cbui/gfx/gfx.c A payloads/cbui/gfx/gfx.h A payloads/cbui/gfx/splash.c A payloads/cbui/gfx/splash.h A payloads/cbui/gfx/vbe.c A payloads/cbui/gfx/vbe.h A payloads/cbui/lang/de.c A payloads/cbui/lang/en.c A payloads/cbui/lang/lang.c A payloads/cbui/lang/lang.h A payloads/cbui/logo/cbui.png A payloads/cbui/lp.config A payloads/cbui/modules/bootlog_module.c A payloads/cbui/modules/cbfs_module.c A payloads/cbui/modules/cmos_module.c A payloads/cbui/modules/coreboot_module.c A payloads/cbui/modules/cpuinfo_module.c A payloads/cbui/modules/license_module.c A payloads/cbui/modules/modules.c A payloads/cbui/modules/modules.h A payloads/cbui/modules/nvram_module.c A payloads/cbui/modules/pci_module.c A payloads/cbui/modules/reboot_module.c A payloads/cbui/modules/rtc_module.c A payloads/cbui/modules/rw_module.c A payloads/cbui/modules/timestamps_module.c A payloads/cbui/modules/usb_module.c A payloads/libpayload/configs/defconfig-cbui 86 files changed, 12,125 insertions(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/86/23586/47 -- To view, visit https://review.coreboot.org/c/coreboot/+/23586 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ib9a1a07c1065880aa675380625021750d5cab7d1 Gerrit-Change-Number: 23586 Gerrit-PatchSet: 47 Gerrit-Owner: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Iman Bingi <imanbingy(a)gmail.com> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Attention: Iman Bingi <imanbingy(a)gmail.com> Gerrit-Attention: Martin L Roth <gaumless(a)gmail.com> Gerrit-Attention: Julius Werner <jwerner(a)chromium.org> Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org> Gerrit-MessageType: newpatchset

1 0

[S] Change in coreboot[master]: mb/google/skyrim/var/winterhold: Enable DPTC support for Winterhold
by Tim Van Patten (Code Review) 05 Oct '22

05 Oct '22

Attention is currently required from: Raul Rangel, Jason Nien, EricKY Cheng, Isaac Lee, Tim Wawrzynczak, Jon Murphy, Jason Nein, Martin Roth, Eric Peers, Moises Garcia, Karthikeyan Ramasubramanian. Tim Van Patten has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/68076 ) Change subject: mb/google/skyrim/var/winterhold: Enable DPTC support for Winterhold ...................................................................... Patch Set 3: (1 comment) Commit Message: https://review.coreboot.org/c/coreboot/+/68076/comment/b133e254_176d8f1f PS1, Line 13: TEST=emerge-skyrim coreboot > Hi Tim, I have no idea on how to ensure the CLs don't break anything Please test the following on a skyrim device: 1. Boot 2. Log in 3. Reboot 4. 1 suspend/resume That should be enough to verify the device is functional with these changes. -- To view, visit https://review.coreboot.org/c/coreboot/+/68076 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I97c2d3ee29687cd8a9c459e90a45cef05ac4436b Gerrit-Change-Number: 68076 Gerrit-PatchSet: 3 Gerrit-Owner: EricKY Cheng <ericky_cheng(a)compal.corp-partner.google.com> Gerrit-Reviewer: Eric Peers <epeers(a)google.com> Gerrit-Reviewer: EricKY Cheng <ericky_cheng(a)compal.corp-partner.google.com> Gerrit-Reviewer: Isaac Lee <isaaclee(a)google.com> Gerrit-Reviewer: Jason Nein <finaljason(a)gmail.com> Gerrit-Reviewer: Jason Nien <jason.nien(a)amd.corp-partner.google.com> Gerrit-Reviewer: Jon Murphy <jpmurphy(a)google.com> Gerrit-Reviewer: Karthikeyan Ramasubramanian <kramasub(a)chromium.org> Gerrit-Reviewer: Martin Roth <martin.roth(a)amd.corp-partner.google.com> Gerrit-Reviewer: Moises Garcia <moisesgarcia(a)google.com> Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org> Gerrit-Reviewer: Tim Van Patten <timvp(a)google.com> Gerrit-Reviewer: Tim Wawrzynczak <inforichland(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Raul Rangel <rrangel(a)chromium.org> Gerrit-Attention: Jason Nien <jason.nien(a)amd.corp-partner.google.com> Gerrit-Attention: EricKY Cheng <ericky_cheng(a)compal.corp-partner.google.com> Gerrit-Attention: Isaac Lee <isaaclee(a)google.com> Gerrit-Attention: Tim Wawrzynczak <inforichland(a)gmail.com> Gerrit-Attention: Jon Murphy <jpmurphy(a)google.com> Gerrit-Attention: Jason Nein <finaljason(a)gmail.com> Gerrit-Attention: Martin Roth <martin.roth(a)amd.corp-partner.google.com> Gerrit-Attention: Eric Peers <epeers(a)google.com> Gerrit-Attention: Moises Garcia <moisesgarcia(a)google.com> Gerrit-Attention: Karthikeyan Ramasubramanian <kramasub(a)chromium.org> Gerrit-Comment-Date: Wed, 05 Oct 2022 15:28:25 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: EricKY Cheng <ericky_cheng(a)compal.corp-partner.google.com> Comment-In-Reply-To: Tim Van Patten <timvp(a)google.com> Gerrit-MessageType: comment

1 0

[XL] Change in coreboot[master]: payloads/cbui: Add new payload CBUI
by build bot (Jenkins) (Code Review) 05 Oct '22

05 Oct '22

Attention is currently required from: Martin L Roth, Julius Werner, Patrick Rudolph. build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/23586 ) Change subject: payloads/cbui: Add new payload CBUI ...................................................................... Patch Set 46: (36 comments) File payloads/cbui/NuklearUI/NuklearDatePicker.c: Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-159690): https://review.coreboot.org/c/coreboot/+/23586/comment/fd5b5a65_ec50f7fe PS46, Line 36: case 6: /* fallthrough */ Prefer 'fallthrough;' over fallthrough comment Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-159690): https://review.coreboot.org/c/coreboot/+/23586/comment/677b9ea4_e5b18011 PS46, Line 37: case 9: /* fallthrough */ Prefer 'fallthrough;' over fallthrough comment File payloads/cbui/NuklearUI/NuklearRW.c: Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-159690): https://review.coreboot.org/c/coreboot/+/23586/comment/35ceb472_208d3ced PS46, Line 218: static const char *titles[] = { "Hex-View", "Header", "Index/Data" }; static const char * array should probably be static const char * const Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-159690): https://review.coreboot.org/c/coreboot/+/23586/comment/bec22832_dc62149f PS46, Line 252: static const char *entries[] = { "NVRAM" }; static const char * array should probably be static const char * const File payloads/cbui/arch/x86/int32.h: Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-159690): https://review.coreboot.org/c/coreboot/+/23586/comment/95e0606f_c9d3f0a8 PS46, Line 20: typedef struct __attribute__ ((packed)) { Prefer __packed over __attribute__((packed)) File payloads/cbui/arch/x86/int32.nasm: Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-159690): https://review.coreboot.org/c/coreboot/+/23586/comment/f359c056_0d34eca9 PS46, Line 38: struc regs16_t 'struc' may be misspelled - perhaps 'struct'? File payloads/cbui/gfx/vbe.c: Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-159690): https://review.coreboot.org/c/coreboot/+/23586/comment/790f9b5d_298f4652 PS46, Line 24: static u8 low_mem[0x1000] __attribute__ ((section(".lowmem"))); Prefer __section(".lowmem") over __attribute__((section(".lowmem"))) File payloads/cbui/lang/de.c: Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-159690): https://review.coreboot.org/c/coreboot/+/23586/comment/af5aaa83_1f1c6f7b PS46, Line 39: "Kein Framebuffer gefunden. CBUI funktioniert nicht" \ Avoid unnecessary line continuations Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-159690): https://review.coreboot.org/c/coreboot/+/23586/comment/dfec99c4_fdae6ad2 PS46, Line 44: "Der coreboot framebuffer hat keine physikalische " \ Avoid unnecessary line continuations Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-159690): https://review.coreboot.org/c/coreboot/+/23586/comment/958106d3_194c15e8 PS46, Line 47: "Kein Platz auf dem Heap.\nBitte vergroBere den" \ Avoid unnecessary line continuations Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-159690): https://review.coreboot.org/c/coreboot/+/23586/comment/80d30946_ed250099 PS46, Line 50: "Konnte die nuklear Bibliothek nicht intitialisieren.\nBitte" \ Avoid unnecessary line continuations Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-159690): https://review.coreboot.org/c/coreboot/+/23586/comment/8ab7045d_0c72d731 PS46, Line 52: "stelle sicher, das der Framebuffer entweder das RGBX8 oder " \ Avoid unnecessary line continuations Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-159690): https://review.coreboot.org/c/coreboot/+/23586/comment/7f143d9b_872c47d1 PS46, Line 65: "Fehler: Keine CMOS Eintrage gefunden. Bitte schalte die " \ Avoid unnecessary line continuations Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-159690): https://review.coreboot.org/c/coreboot/+/23586/comment/f50ba2f9_dee13920 PS46, Line 86: "Fehler: Keine Zeitstempel gefunden. Bitte schalte die " \ Avoid unnecessary line continuations Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-159690): https://review.coreboot.org/c/coreboot/+/23586/comment/090fb5bf_68db55e4 PS46, Line 89: "Fehler: Bootlog nicht gefunden. Bitte schalte die Kconfig " \ Avoid unnecessary line continuations File payloads/cbui/lang/en.c: Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-159690): https://review.coreboot.org/c/coreboot/+/23586/comment/4b29fcd8_0c9f4e7c PS46, Line 38: "No framebuffer found. CBUI doesn't work in text mode.\n" \ Avoid unnecessary line continuations Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-159690): https://review.coreboot.org/c/coreboot/+/23586/comment/4df1ea69_9e23f3d5 PS46, Line 46: "Failed to init nuklear library.\nMake sure you have enough " \ Avoid unnecessary line continuations Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-159690): https://review.coreboot.org/c/coreboot/+/23586/comment/bc16f6d0_4831bf4d PS46, Line 60: "ERROR: No CMOS options found. Please make sure to enable " \ Avoid unnecessary line continuations Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-159690): https://review.coreboot.org/c/coreboot/+/23586/comment/63b60383_7581b42b PS46, Line 81: "ERROR: No timestamps found. Please make sure to enable " \ Avoid unnecessary line continuations Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-159690): https://review.coreboot.org/c/coreboot/+/23586/comment/0d35676d_1c2f036c PS46, Line 84: "ERROR: Bootlog not found. Please make sure to enable " \ Avoid unnecessary line continuations File payloads/cbui/modules/bootlog_module.c: Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-159690): https://review.coreboot.org/c/coreboot/+/23586/comment/36cd4ae5_89367c80 PS46, Line 28: } __attribute__ ((__packed__)); Prefer __packed over __attribute__((__packed__)) Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-159690): https://review.coreboot.org/c/coreboot/+/23586/comment/75808d87_c1fa4b32 PS46, Line 82: } void function return statements are not generally useful File payloads/cbui/modules/cbfs_module.c: Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-159690): https://review.coreboot.org/c/coreboot/+/23586/comment/dab6344d_da400087 PS46, Line 50: } __attribute__ ((packed)); Prefer __packed over __attribute__((packed)) Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-159690): https://review.coreboot.org/c/coreboot/+/23586/comment/bca863b3_3459a52a PS46, Line 59: } __attribute__ ((packed)); Prefer __packed over __attribute__((packed)) File payloads/cbui/modules/cpuinfo_module.c: Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-159690): https://review.coreboot.org/c/coreboot/+/23586/comment/c63ccd1c_1940a1e9 PS46, Line 35: static const char *intel_cap_edx_flags[] = { static const char * array should probably be static const char * const Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-159690): https://review.coreboot.org/c/coreboot/+/23586/comment/86b53d62_59b57fd3 PS46, Line 43: static const char *amd_cap_edx_flags[] = { static const char * array should probably be static const char * const Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-159690): https://review.coreboot.org/c/coreboot/+/23586/comment/00fe530a_709d07f5 PS46, Line 51: static const char *intel_cap_ecx_flags[] = { static const char * array should probably be static const char * const Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-159690): https://review.coreboot.org/c/coreboot/+/23586/comment/72d2cfc1_373c2dd9 PS46, Line 59: static const char *amd_cap_ecx_flags[] = { static const char * array should probably be static const char * const Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-159690): https://review.coreboot.org/c/coreboot/+/23586/comment/14cb810e_df11390a PS46, Line 67: static const char *intel_cap_extended_edx_flags[] = { static const char * array should probably be static const char * const Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-159690): https://review.coreboot.org/c/coreboot/+/23586/comment/a94006b6_40c19b24 PS46, Line 75: static const char *intel_cap_extended_ecx_flags[] = { static const char * array should probably be static const char * const Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-159690): https://review.coreboot.org/c/coreboot/+/23586/comment/2a088a18_8f9d0450 PS46, Line 83: static const char *amd_cap_extended_edx_flags[] = { static const char * array should probably be static const char * const Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-159690): https://review.coreboot.org/c/coreboot/+/23586/comment/efe073e2_45958dcc PS46, Line 92: static const char *amd_cap_extended_ecx_flags[] = { static const char * array should probably be static const char * const File payloads/cbui/modules/rw_module.c: Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-159690): https://review.coreboot.org/c/coreboot/+/23586/comment/75521242_c8db81c6 PS46, Line 270: static struct rw_pci_index_data* rw_get_pci_dev_id(struct rw_presenter *this) "foo* bar" should be "foo *bar" Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-159690): https://review.coreboot.org/c/coreboot/+/23586/comment/8b12401a_e5075867 PS46, Line 346: int count = sizeof(cell_buffers) / sizeof(cell_buffers[0]); Prefer ARRAY_SIZE(cell_buffers) Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-159690): https://review.coreboot.org/c/coreboot/+/23586/comment/62f0b999_1e31e907 PS46, Line 439: int count = sizeof(cell_buffers) / sizeof(cell_buffers[0]); Prefer ARRAY_SIZE(cell_buffers) Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-159690): https://review.coreboot.org/c/coreboot/+/23586/comment/65c948f2_3f2ab9fd PS46, Line 543: else if (cache->key == KEY_DOWN || cache->key == KEY_RIGHT) { else should follow close brace '}' -- To view, visit https://review.coreboot.org/c/coreboot/+/23586 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ib9a1a07c1065880aa675380625021750d5cab7d1 Gerrit-Change-Number: 23586 Gerrit-PatchSet: 46 Gerrit-Owner: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Iman Bingi <imanbingy(a)gmail.com> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Attention: Martin L Roth <gaumless(a)gmail.com> Gerrit-Attention: Julius Werner <jwerner(a)chromium.org> Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Comment-Date: Wed, 05 Oct 2022 15:25:48 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Jump to page:

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit October 2022