coreboot-gerrit November 2022

coreboot-gerrit@coreboot.org

1 participants
7286 discussions

[M] Change in coreboot[master]: mb/google/zork: Use detect vs probed flag for touchscreens
by Angel Pons (Code Review) 12 Nov '22

12 Nov '22

Attention is currently required from: Jason Nien, Matt DeVillier, Martin Roth. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/69457 ) Change subject: mb/google/zork: Use detect vs probed flag for touchscreens ...................................................................... Patch Set 2: Code-Review+1 (1 comment) Commit Message: https://review.coreboot.org/c/coreboot/+/69457/comment/a0a24ae1_3686ef82 PS2, Line 22: CB:67779 Please reference using the commit hash and summary in parentheses -- To view, visit https://review.coreboot.org/c/coreboot/+/69457 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Idfe899bd535507c56f0825c6538246441b3b0827 Gerrit-Change-Number: 69457 Gerrit-PatchSet: 2 Gerrit-Owner: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Jason Nien <jason.nien(a)amd.corp-partner.google.com> Gerrit-Reviewer: Martin Roth <martin.roth(a)amd.corp-partner.google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Jason Nien <jason.nien(a)amd.corp-partner.google.com> Gerrit-Attention: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com> Gerrit-Attention: Martin Roth <martin.roth(a)amd.corp-partner.google.com> Gerrit-Comment-Date: Fri, 11 Nov 2022 23:59:24 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[M] Change in coreboot[master]: mb/starlabs/*: Reserve the century byte in CMOS
by Angel Pons (Code Review) 12 Nov '22

12 Nov '22

Attention is currently required from: Sean Rhodes, Arthur Heymans. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/69323 ) Change subject: mb/starlabs/*: Reserve the century byte in CMOS ...................................................................... Patch Set 8: Code-Review+1 (1 comment) Commit Message: https://review.coreboot.org/c/coreboot/+/69323/comment/b4a3855e_8552189a PS8, Line 7: mb/starlabs/*: Reserve the century byte in CMOS Why is the console loglevel moved? It fits where it is -- To view, visit https://review.coreboot.org/c/coreboot/+/69323 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I0237772fb2f705065c833dc721184295c75884ea Gerrit-Change-Number: 69323 Gerrit-PatchSet: 8 Gerrit-Owner: Sean Rhodes <sean(a)starlabs.systems> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Sean Rhodes <sean(a)starlabs.systems> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Comment-Date: Fri, 11 Nov 2022 23:56:49 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[S] Change in coreboot[master]: mb/google/zork: Implement touchscreen power sequencing
by Angel Pons (Code Review) 12 Nov '22

12 Nov '22

Attention is currently required from: Jason Nien, Matt DeVillier, Martin Roth. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/69456 ) Change subject: mb/google/zork: Implement touchscreen power sequencing ...................................................................... Patch Set 2: Code-Review+1 (1 comment) Commit Message: https://review.coreboot.org/c/coreboot/+/69456/comment/41e828be_3b1aa286 PS2, Line 15: CB:67778 Please reference using the commit hash and summary in parentheses -- To view, visit https://review.coreboot.org/c/coreboot/+/69456 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ifdd75cd96e7b6880085a3f47214b92948a56aa2e Gerrit-Change-Number: 69456 Gerrit-PatchSet: 2 Gerrit-Owner: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Jason Nien <jason.nien(a)amd.corp-partner.google.com> Gerrit-Reviewer: Martin Roth <martin.roth(a)amd.corp-partner.google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Jason Nien <jason.nien(a)amd.corp-partner.google.com> Gerrit-Attention: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com> Gerrit-Attention: Martin Roth <martin.roth(a)amd.corp-partner.google.com> Gerrit-Comment-Date: Fri, 11 Nov 2022 23:54:45 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[S] Change in coreboot[master]: mg/google/zork: Add functionality to set GPIOs in romstage
by Angel Pons (Code Review) 12 Nov '22

12 Nov '22

Attention is currently required from: Jason Nien, Matt DeVillier, Martin Roth. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/69482 ) Change subject: mg/google/zork: Add functionality to set GPIOs in romstage ...................................................................... Patch Set 1: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/69482 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: If0f626dbc7e601c2f49759e49a0baf027bf25f96 Gerrit-Change-Number: 69482 Gerrit-PatchSet: 1 Gerrit-Owner: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Jason Nien <jason.nien(a)amd.corp-partner.google.com> Gerrit-Reviewer: Martin Roth <martin.roth(a)amd.corp-partner.google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Jason Nien <jason.nien(a)amd.corp-partner.google.com> Gerrit-Attention: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com> Gerrit-Attention: Martin Roth <martin.roth(a)amd.corp-partner.google.com> Gerrit-Comment-Date: Fri, 11 Nov 2022 23:52:50 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[S] Change in coreboot[master]: mb/google/zork: rename baseboard GPIO table getter for clarity
by Angel Pons (Code Review) 12 Nov '22

12 Nov '22

Attention is currently required from: Jason Nien, Matt DeVillier. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/69453 ) Change subject: mb/google/zork: rename baseboard GPIO table getter for clarity ...................................................................... Patch Set 1: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/69453 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Idd8ea3446ab7940b21265a3ed8080ba4029c4ff7 Gerrit-Change-Number: 69453 Gerrit-PatchSet: 1 Gerrit-Owner: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Jason Nien <jason.nien(a)amd.corp-partner.google.com> Gerrit-Reviewer: Martin Roth <martin.roth(a)amd.corp-partner.google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Jason Nien <jason.nien(a)amd.corp-partner.google.com> Gerrit-Attention: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com> Gerrit-Comment-Date: Fri, 11 Nov 2022 23:48:45 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[S] Change in coreboot[master]: soc/amd/picasso: add mb_pre_fspm() definition and weak implementation
by Angel Pons (Code Review) 12 Nov '22

12 Nov '22

Attention is currently required from: Jason Glenesk, Matt DeVillier, Fred Reitberger, Felix Held. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/69452 ) Change subject: soc/amd/picasso: add mb_pre_fspm() definition and weak implementation ...................................................................... Patch Set 1: (1 comment) Commit Message: https://review.coreboot.org/c/coreboot/+/69452/comment/5da082b0_26ebb50c PS1, Line 10: ; will be moved in a : subsequent commit Will it, though? CB:69454 is abandoned -- To view, visit https://review.coreboot.org/c/coreboot/+/69452 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ia422aaa9e80355f9a9f8f850368441e5c8ff6598 Gerrit-Change-Number: 69452 Gerrit-PatchSet: 1 Gerrit-Owner: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Fred Reitberger <reitbergerfred(a)gmail.com> Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)amd.corp-partner.google.com> Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Reviewer: Martin Roth <martin.roth(a)amd.corp-partner.google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Attention: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com> Gerrit-Attention: Fred Reitberger <reitbergerfred(a)gmail.com> Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Comment-Date: Fri, 11 Nov 2022 23:47:18 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[S] Change in coreboot[master]: soc/amd/picasso: add mb_pre_fspm() definition and weak implementation
by Angel Pons (Code Review) 12 Nov '22

12 Nov '22

Attention is currently required from: Jason Glenesk, Matt DeVillier, Fred Reitberger, Felix Held. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/69452 ) Change subject: soc/amd/picasso: add mb_pre_fspm() definition and weak implementation ...................................................................... Patch Set 1: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/69452 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ia422aaa9e80355f9a9f8f850368441e5c8ff6598 Gerrit-Change-Number: 69452 Gerrit-PatchSet: 1 Gerrit-Owner: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Fred Reitberger <reitbergerfred(a)gmail.com> Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)amd.corp-partner.google.com> Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Reviewer: Martin Roth <martin.roth(a)amd.corp-partner.google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Attention: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com> Gerrit-Attention: Fred Reitberger <reitbergerfred(a)gmail.com> Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Comment-Date: Fri, 11 Nov 2022 23:46:50 +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) 12 Nov '22

12 Nov '22

Attention is currently required from: Maximilian Brune, David Milosevic. 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 6: (10 comments) File src/mainboard/prodrive/atlas/emi.h: https://review.coreboot.org/c/coreboot/+/68137/comment/3b9fd389_6d0c7656 PS6, Line 33: typedef enum { > avoid typedef as per coreboot coding style Also, use lowercase for enum type names https://review.coreboot.org/c/coreboot/+/68137/comment/b4d55588_bf9d0f7d PS6, Line 36: EMI_INSTANCE_1 ///< EMI instance 1 (EMI1) > the comment is unnecessary. […] +1 https://review.coreboot.org/c/coreboot/+/68137/comment/988044f5_8832e1d9 PS6, Line 76: const u16 addr, > do not use tabs for this kind of indentation. […] We use tabs for indentation of such things in other files anyway. Tabs are 8 characters wide in coreboot. But we don't align things this way. Instead, we align to the opening parentheses: ``` int emi_read_region(const EMI_INSTANCE instance, const EMI_REGION region, const EMI_ACCESS access, const u16 addr, u8 *buff); ``` File src/mainboard/prodrive/atlas/emi.c: https://review.coreboot.org/c/coreboot/+/68137/comment/f905035e_a41201c5 PS6, Line 41: int emi_read_region(const EMI_INSTANCE instance, const EMI_REGION region, > as far as I can see this function is only used by "emi_read_region_blocK". […] It might be a good idea to keep the logic to select an address: ``` u16 emi_select_region(const EMI_INSTANCE instance, const EMI_REGION region, const EMI_ACCESS access, const u16 addr) { 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); return base; } ``` Then do the access with the desired bit width outside the function. https://review.coreboot.org/c/coreboot/+/68137/comment/729cd573_4538d058 PS6, Line 139: inline void emi_write_register(const EMI_INSTANCE instance, const u8 offs, const u8 value) > function can be removed, since it's not used anywhere (only clutters up code). Also, explicitly declaring things inline is generally not a good idea in coreboot. File src/mainboard/prodrive/atlas/ld_config.c: PS6: Please use `device/pnp_ops.h` File src/mainboard/prodrive/atlas/mainboard.c: https://review.coreboot.org/c/coreboot/+/68137/comment/c0e0c615_ac800a3f PS6, Line 14: static const char* get_smbios_part_number(void); > "foo* bar" should be "foo *bar" Please fix. https://review.coreboot.org/c/coreboot/+/68137/comment/eeb96274_861b7417 PS6, Line 41: static const char* get_smbios_part_number(void) > "foo* bar" should be "foo *bar" Please fix. File src/mainboard/prodrive/atlas/smbios.c: https://review.coreboot.org/c/coreboot/+/68137/comment/435cc610_39182108 PS6, Line 8: const char* smbios_mainboard_serial_number(void) > "foo* bar" should be "foo *bar" Please fix. File src/mainboard/prodrive/atlas/vpd.c: https://review.coreboot.org/c/coreboot/+/68137/comment/0bc7606c_fee279c3 PS6, Line 36: case vpd(part_number): return PN_LENGTH; > as mentioned above, this macro makes it very hard to read (especially considering it has the same na […] Or if the macro were defined immediately before the function, and using an uppercase name. -- 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: 6 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: David Milosevic <David.Milosevic(a)9elements.com> Gerrit-Comment-Date: Fri, 11 Nov 2022 23:42:35 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Maximilian Brune <maximilian.brune(a)9elements.com> Gerrit-MessageType: comment

1 0

[S] Change in coreboot[master]: include/cper.h: Add CPER Memory Error Section definitions
by Angel Pons (Code Review) 12 Nov '22

12 Nov '22

Attention is currently required from: Johnny Lin, Shuming Chu (Shuming), Tim Chu. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/69198 ) Change subject: include/cper.h: Add CPER Memory Error Section definitions ...................................................................... Patch Set 3: Code-Review+1 (1 comment) File src/include/cper.h: https://review.coreboot.org/c/coreboot/+/69198/comment/c8f570d0_e900210c PS3, Line 400: enum cper_err_code mem_err_type; Will this work properly? Not sure if the size of enum types is well-defined. -- To view, visit https://review.coreboot.org/c/coreboot/+/69198 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I0a165350a16a4cbe4033a3e7c43fa23a5b27c44b Gerrit-Change-Number: 69198 Gerrit-PatchSet: 3 Gerrit-Owner: Shuming Chu (Shuming) <s1218944(a)gmail.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Johnny Lin <Johnny_Lin(a)wiwynn.com> Gerrit-Reviewer: Jonathan Zhang <jonzhang(a)fb.com> Gerrit-Reviewer: Tim Chu <Tim.Chu(a)quantatw.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Johnny Lin <Johnny_Lin(a)wiwynn.com> Gerrit-Attention: Shuming Chu (Shuming) <s1218944(a)gmail.com> Gerrit-Attention: Tim Chu <Tim.Chu(a)quantatw.com> Gerrit-Comment-Date: Fri, 11 Nov 2022 23:22:29 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[S] Change in coreboot[master]: superio/fintek/f71808a/f71808a_hwm.c: Fix writing to hardware monitor...
by Angel Pons (Code Review) 12 Nov '22

12 Nov '22

Attention is currently required from: Ravi Mistry, Felix Held. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/69260 ) Change subject: superio/fintek/f71808a/f71808a_hwm.c: Fix writing to hardware monitor registers ...................................................................... Patch Set 2: Code-Review+1 -- To view, visit https://review.coreboot.org/c/coreboot/+/69260 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I13e91e9900cfd151d143af4530d18ece2dbbe647 Gerrit-Change-Number: 69260 Gerrit-PatchSet: 2 Gerrit-Owner: Ravi Mistry Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Ravi Mistry Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Ravi Mistry Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Comment-Date: Fri, 11 Nov 2022 23:17:16 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit November 2022