coreboot-gerrit November 2022

coreboot-gerrit@coreboot.org

1 participants
7286 discussions

[L] Change in coreboot[master]: [WIP] mb/prodrive/atlas: Populate smbios table with VPD from ECs EMI
by Arthur Heymans (Code Review) 17 Nov '22

17 Nov '22

Attention is currently required from: Maximilian Brune, Angel Pons, Lean Sheng Tan, David Milosevic. Arthur Heymans 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 11: (3 comments) File src/mainboard/prodrive/atlas/mainboard.c: https://review.coreboot.org/c/coreboot/+/68137/comment/34fccca4_fa798936 PS11, Line 42: const char prefix[5] = "P/N: "; : static u8 pn[sizeof(prefix) + PN_LENGTH + 1] = { '\x00' }; : : strncpy((char *) pn, prefix, sizeof(prefix)) Do you need a strncpy? I think you can statically set pn[] with a prefix. Btw if you set this to PAGESIZE + prefix, you might simplify the read_region function as it's easier to deal with 4 byte aligned data. Then just add NULL termination here at sizeof(prefix) + PN_LENGTH. File src/mainboard/prodrive/atlas/vpd.h: https://review.coreboot.org/c/coreboot/+/68137/comment/67e608fd_d8cccf5a PS11, Line 13: __packed __packed is a NOOP needed here. File src/mainboard/prodrive/atlas/vpd.c: https://review.coreboot.org/c/coreboot/+/68137/comment/8616899c_49f2cc14 PS11, Line 36: PN_LENGTH Do you need this in the read_region code? reading 4byte aligned data and dealing with it later is probably easier. -- 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: 11 Gerrit-Owner: David Milosevic <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-CC: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-CC: Maximilian Brune <maximilian.brune(a)9elements.com> Gerrit-CC: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Attention: Maximilian Brune <maximilian.brune(a)9elements.com> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Attention: David Milosevic <David.Milosevic(a)9elements.com> Gerrit-Comment-Date: Thu, 17 Nov 2022 16:31:14 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[L] Change in coreboot[master]: [WIP] mb/prodrive/atlas: Populate smbios table with VPD from ECs EMI
by Arthur Heymans (Code Review) 17 Nov '22

17 Nov '22

Attention is currently required from: Maximilian Brune, Angel Pons, Lean Sheng Tan, David Milosevic. Arthur Heymans 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 11: (3 comments) File src/mainboard/prodrive/atlas/emi.c: https://review.coreboot.org/c/coreboot/+/68137/comment/93ff04fd_c3cec9c6 PS11, Line 59: u8 bytes[4] = { 0 }; use u32. https://review.coreboot.org/c/coreboot/+/68137/comment/3c671c14_f1cc4aba PS11, Line 71: buff[idx++] = bytes[i]; Use memcpy https://review.coreboot.org/c/coreboot/+/68137/comment/c9277ce1_746e5b72 PS11, Line 68: if (idx == n) : return; : : buff[idx++] Can you put those conditions in the for loop? That makes it easier to read. -- 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: 11 Gerrit-Owner: David Milosevic <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-CC: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-CC: Maximilian Brune <maximilian.brune(a)9elements.com> Gerrit-CC: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Attention: Maximilian Brune <maximilian.brune(a)9elements.com> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Attention: David Milosevic <David.Milosevic(a)9elements.com> Gerrit-Comment-Date: Thu, 17 Nov 2022 16:06:23 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[S] Change in coreboot[master]: mb/google/nissa/var/craask: Disable gpio export in crs for G2 touchsc...
by David Wu (Code Review) 17 Nov '22

17 Nov '22

Attention is currently required from: Tarun Tuli, Kangheui Won, Reka Norman, Tyler Wang, Nick Vaccaro. David Wu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/69749 ) Change subject: mb/google/nissa/var/craask: Disable gpio export in crs for G2 touchscreen ...................................................................... Patch Set 2: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/69749 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I21a47adde48555098d041b94d483cad308bdb717 Gerrit-Change-Number: 69749 Gerrit-PatchSet: 2 Gerrit-Owner: Tyler Wang <tyler.wang(a)quanta.corp-partner.google.com> Gerrit-Reviewer: David Wu <david_wu(a)quanta.corp-partner.google.com> Gerrit-Reviewer: Eric Lai <eric_lai(a)quanta.corp-partner.google.com> Gerrit-Reviewer: Kangheui Won <khwon(a)chromium.org> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)google.com> Gerrit-Reviewer: Reka Norman <rekanorman(a)chromium.org> Gerrit-Reviewer: Tarun Tuli <taruntuli(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Cathy Chen <cathy_chen(a)quanta.corp-partner.google.com> Gerrit-CC: Ren Kuo <ren.kuo(a)quanta.corp-partner.google.com> Gerrit-Attention: Tarun Tuli <taruntuli(a)google.com> Gerrit-Attention: Kangheui Won <khwon(a)chromium.org> Gerrit-Attention: Reka Norman <rekanorman(a)chromium.org> Gerrit-Attention: Tyler Wang <tyler.wang(a)quanta.corp-partner.google.com> Gerrit-Attention: Nick Vaccaro <nvaccaro(a)google.com> Gerrit-Comment-Date: Thu, 17 Nov 2022 16:00:09 +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 David Milosevic (Code Review) 17 Nov '22

17 Nov '22

Attention is currently required from: Maximilian Brune, Angel Pons, Lean Sheng Tan. Hello build bot (Jenkins), Christian Walter, Angel Pons, Lean Sheng Tan, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/68137 to look at the new patch set (#11). 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: David Milosevic <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, 377 insertions(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/37/68137/11 -- 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: 11 Gerrit-Owner: David Milosevic <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-CC: Maximilian Brune <maximilian.brune(a)9elements.com> Gerrit-CC: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Attention: Maximilian Brune <maximilian.brune(a)9elements.com> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-MessageType: newpatchset

1 0

[S] Change in coreboot[master]: mb/google/nissa/var/craask: Disable gpio export in crs for G2 touchsc...
by Eric Lai (Code Review) 17 Nov '22

17 Nov '22

Attention is currently required from: Tarun Tuli, Kangheui Won, Reka Norman, Tyler Wang, Nick Vaccaro. Eric Lai has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/69749 ) Change subject: mb/google/nissa/var/craask: Disable gpio export in crs for G2 touchscreen ...................................................................... Patch Set 2: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/69749 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I21a47adde48555098d041b94d483cad308bdb717 Gerrit-Change-Number: 69749 Gerrit-PatchSet: 2 Gerrit-Owner: Tyler Wang <tyler.wang(a)quanta.corp-partner.google.com> Gerrit-Reviewer: Eric Lai <eric_lai(a)quanta.corp-partner.google.com> Gerrit-Reviewer: Kangheui Won <khwon(a)chromium.org> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)google.com> Gerrit-Reviewer: Reka Norman <rekanorman(a)chromium.org> Gerrit-Reviewer: Tarun Tuli <taruntuli(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Cathy Chen <cathy_chen(a)quanta.corp-partner.google.com> Gerrit-CC: David Wu <david_wu(a)quanta.corp-partner.google.com> Gerrit-CC: Ren Kuo <ren.kuo(a)quanta.corp-partner.google.com> Gerrit-Attention: Tarun Tuli <taruntuli(a)google.com> Gerrit-Attention: Kangheui Won <khwon(a)chromium.org> Gerrit-Attention: Reka Norman <rekanorman(a)chromium.org> Gerrit-Attention: Tyler Wang <tyler.wang(a)quanta.corp-partner.google.com> Gerrit-Attention: Nick Vaccaro <nvaccaro(a)google.com> Gerrit-Comment-Date: Thu, 17 Nov 2022 15:37:24 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[S] Change in coreboot[master]: soc/intel/broadwell: Fix out() parameter order
by Arthur Heymans (Code Review) 17 Nov '22

17 Nov '22

Attention is currently required from: Kyösti Mälkki. Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/69703 ) Change subject: soc/intel/broadwell: Fix out() parameter order ...................................................................... Patch Set 1: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/69703 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I0897acddd00bad89a5fd784f82380ed0d0d2c06e Gerrit-Change-Number: 69703 Gerrit-PatchSet: 1 Gerrit-Owner: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Elyes Haouas <ehaouas(a)noos.fr> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Comment-Date: Thu, 17 Nov 2022 15:23:43 +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 David Milosevic (Code Review) 17 Nov '22

17 Nov '22

Attention is currently required from: Maximilian Brune, Angel Pons, Lean Sheng Tan. David Milosevic 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 10: (15 comments) File src/mainboard/prodrive/atlas/emi.h: https://review.coreboot.org/c/coreboot/+/68137/comment/c8391426_579d8195 PS6, Line 33: typedef enum { > Also, use lowercase for enum type names Done https://review.coreboot.org/c/coreboot/+/68137/comment/527e58c0_fda3d9a0 PS6, Line 36: EMI_INSTANCE_1 ///< EMI instance 1 (EMI1) > +1 Done https://review.coreboot.org/c/coreboot/+/68137/comment/6cdcc049_2bb90a1a PS6, Line 40: typedef enum { > I totally understand why someone might not like the usage of typedefs. […] Done https://review.coreboot.org/c/coreboot/+/68137/comment/819992cb_a97026ee PS6, Line 47: typedef enum { > See previous reply. Done https://review.coreboot.org/c/coreboot/+/68137/comment/40c69742_de4ec2d4 PS6, Line 76: const u16 addr, > We use tabs for indentation of such things in other files anyway. […] Done File src/mainboard/prodrive/atlas/emi.c: https://review.coreboot.org/c/coreboot/+/68137/comment/a5b20d51_f662985a PS6, Line 41: int emi_read_region(const EMI_INSTANCE instance, const EMI_REGION region, > It might be a good idea to keep the logic to select an address: […] Done https://review.coreboot.org/c/coreboot/+/68137/comment/2ff5e1b8_c6caf51b PS6, Line 83: { > personally I think it makes sense to keep the write function, because: […] Done https://review.coreboot.org/c/coreboot/+/68137/comment/c0fe72cc_fe89812a PS6, Line 139: inline void emi_write_register(const EMI_INSTANCE instance, const u8 offs, const u8 value) > same as above. Done https://review.coreboot.org/c/coreboot/+/68137/comment/1046704f_2b697f43 PS6, Line 144: inline u8 emi_read_register(const EMI_INSTANCE instance, const u8 offs) > see previous replies Done File src/mainboard/prodrive/atlas/mainboard.c: https://review.coreboot.org/c/coreboot/+/68137/comment/16e89680_c691be1b PS6, Line 14: static const char* get_smbios_part_number(void); > > "foo* bar" should be "foo *bar" […] Done https://review.coreboot.org/c/coreboot/+/68137/comment/e54c1748_217a03e6 PS6, Line 41: static const char* get_smbios_part_number(void) > > "foo* bar" should be "foo *bar" […] Done https://review.coreboot.org/c/coreboot/+/68137/comment/cb4e0f8a_b5e7b8ba PS6, Line 46: strncpy((char *) pn, prefix, sizeof(prefix)); > Regarding Done File src/mainboard/prodrive/atlas/smbios.c: https://review.coreboot.org/c/coreboot/+/68137/comment/4cd07f97_a31960dc PS6, Line 8: const char* smbios_mainboard_serial_number(void) > > "foo* bar" should be "foo *bar" […] Done File src/mainboard/prodrive/atlas/vpd.h: https://review.coreboot.org/c/coreboot/+/68137/comment/0e45094e_65823958 PS6, Line 26: #define vpd(x) offsetof(vpd_section_t, x) > this macro only makes the rest of the code far less obvious. Done File src/mainboard/prodrive/atlas/vpd.c: https://review.coreboot.org/c/coreboot/+/68137/comment/0294c8c1_47d102a0 PS6, Line 36: case vpd(part_number): return PN_LENGTH; > Or if the macro were defined immediately before the function, and using an uppercase name. Done -- 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: 10 Gerrit-Owner: David Milosevic <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-CC: Maximilian Brune <maximilian.brune(a)9elements.com> Gerrit-CC: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Attention: Maximilian Brune <maximilian.brune(a)9elements.com> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Comment-Date: Thu, 17 Nov 2022 14:53:56 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Comment-In-Reply-To: Maximilian Brune <maximilian.brune(a)9elements.com> Comment-In-Reply-To: Lean Sheng Tan <sheng.tan(a)9elements.com> Comment-In-Reply-To: David Milosevic <David.Milosevic(a)9elements.com> Gerrit-MessageType: comment

1 0

[L] Change in coreboot[master]: [WIP] mb/prodrive/atlas: Populate smbios table with VPD from ECs EMI
by David Milosevic (Code Review) 17 Nov '22

17 Nov '22

Attention is currently required from: Maximilian Brune, Angel Pons, David Milosevic. Hello build bot (Jenkins), Christian Walter, Angel Pons, Lean Sheng Tan, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/68137 to look at the new patch set (#10). 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: David Milosevic <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, 378 insertions(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/37/68137/10 -- 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: 10 Gerrit-Owner: David Milosevic <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-CC: Maximilian Brune <maximilian.brune(a)9elements.com> Gerrit-CC: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Attention: Maximilian Brune <maximilian.brune(a)9elements.com> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: David Milosevic <David.Milosevic(a)9elements.com> Gerrit-MessageType: newpatchset

1 0

[M] Change in coreboot[master]: build: List all Kconfigs in CBFS `config` file, compress it
by Jakub Czapiga (Code Review) 17 Nov '22

17 Nov '22

Attention is currently required from: Vadim Bendebury, Martin L Roth, Patrick Georgi, Julius Werner, Yu-Ping Wu. Jakub Czapiga has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/69710 ) Change subject: build: List all Kconfigs in CBFS `config` file, compress it ...................................................................... Patch Set 2: Code-Review+1 (1 comment) File Makefile.inc: https://review.coreboot.org/c/coreboot/+/69710/comment/5ea22eca_e8c9d512 PS1, Line 334: \ > Is this `\` intended? Backslash here tells shell not to use any aliases. However it should not be needed as Make invokes recipes in non-interactive `sh`, so AFAIK aliases should not be available anyway, so this backslash has no function here. -- To view, visit https://review.coreboot.org/c/coreboot/+/69710 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I5259ec6f932cdc5780b8843f46dd476da9d19728 Gerrit-Change-Number: 69710 Gerrit-PatchSet: 2 Gerrit-Owner: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Jakub Czapiga <jacz(a)semihalf.com> Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com> Gerrit-Reviewer: Patrick Georgi <patrick(a)coreboot.org> Gerrit-Reviewer: Yu-Ping Wu <yupingso(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Vadim Bendebury <vbendeb(a)chromium.org> Gerrit-Attention: Vadim Bendebury <vbendeb(a)chromium.org> Gerrit-Attention: Martin L Roth <gaumless(a)gmail.com> Gerrit-Attention: Patrick Georgi <patrick(a)coreboot.org> Gerrit-Attention: Julius Werner <jwerner(a)chromium.org> Gerrit-Attention: Yu-Ping Wu <yupingso(a)google.com> Gerrit-Comment-Date: Thu, 17 Nov 2022 14:34:14 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: Yu-Ping Wu <yupingso(a)google.com> Gerrit-MessageType: comment

1 0

[L] Change in coreboot[master]: [WIP] mb/prodrive/atlas: Populate smbios table with VPD from ECs EMI
by David Milosevic (Code Review) 17 Nov '22

17 Nov '22

Attention is currently required from: Maximilian Brune, Angel Pons, David Milosevic. Hello build bot (Jenkins), Christian Walter, Angel Pons, Lean Sheng Tan, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/68137 to look at the new patch set (#9). 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: David Milosevic <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, 378 insertions(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/37/68137/9 -- 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: 9 Gerrit-Owner: David Milosevic <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-CC: Maximilian Brune <maximilian.brune(a)9elements.com> Gerrit-CC: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Attention: Maximilian Brune <maximilian.brune(a)9elements.com> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: David Milosevic <David.Milosevic(a)9elements.com> Gerrit-MessageType: newpatchset

1 0

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit November 2022