coreboot-gerrit April 2024

coreboot-gerrit@coreboot.org

1 participants
3292 discussions

[S] Change in coreboot[main]: lib: Refactor bmp_load_logo() implementation
by Subrata Banik (Code Review) 02 Apr '24

02 Apr '24

Attention is currently required from: Andrey Petrov, Felix Held, Fred Reitberger, Jason Glenesk, Julius Werner, Matt DeVillier, Ronak Kanabar, Sean Rhodes. Hello Andrey Petrov, Felix Held, Fred Reitberger, Jason Glenesk, Matt DeVillier, Ronak Kanabar, Sean Rhodes, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/81618?usp=email to look at the new patch set (#3). Change subject: lib: Refactor bmp_load_logo() implementation ...................................................................... lib: Refactor bmp_load_logo() implementation This refactoring ensures bmp_load_logo() takes logo_size as an argument, returning a valid logo_ptr only if logo_size is non-zero. This prevents potential errors from mismatched size assumption. BUG=b:242829490 TEST=google/rex0 builds successfully. Change-Id: I14bc54670a67980ec93bc366b274832d1f959e50 Signed-off-by: Subrata Banik <subratabanik(a)google.com> --- M src/drivers/intel/fsp1_1/ramstage.c M src/drivers/intel/fsp2_0/fsp_gop_blt.c M src/include/bootsplash.h M src/lib/bmp_logo.c M src/soc/amd/mendocino/fsp_s_params.c M src/soc/intel/apollolake/chip.c M src/soc/intel/cannonlake/fsp_params.c M src/soc/intel/skylake/chip.c 8 files changed, 29 insertions(+), 16 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/18/81618/3 -- To view, visit https://review.coreboot.org/c/coreboot/+/81618?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I14bc54670a67980ec93bc366b274832d1f959e50 Gerrit-Change-Number: 81618 Gerrit-PatchSet: 3 Gerrit-Owner: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Andrey Petrov <andrey.petrov(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)gmail.com> Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com> Gerrit-Reviewer: Ronak Kanabar <ronak.kanabar(a)intel.com> Gerrit-Reviewer: Sean Rhodes <sean(a)starlabs.systems> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Julius Werner <jwerner(a)chromium.org> Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Attention: Sean Rhodes <sean(a)starlabs.systems> Gerrit-Attention: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com> Gerrit-Attention: Julius Werner <jwerner(a)chromium.org> Gerrit-Attention: Ronak Kanabar <ronak.kanabar(a)intel.com> Gerrit-Attention: Fred Reitberger <reitbergerfred(a)gmail.com> Gerrit-Attention: Andrey Petrov <andrey.petrov(a)gmail.com> Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-MessageType: newpatchset

1 0

[M] Change in coreboot[main]: mb/google/brya: Add new baseboard trulo
by Varshit Pandya (Code Review) 02 Apr '24

02 Apr '24

Attention is currently required from: Dinesh Gehlot, Eric Lai, Kapil Porwal, Nick Vaccaro, Subrata Banik. Varshit Pandya has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/81626?usp=email ) Change subject: mb/google/brya: Add new baseboard trulo ...................................................................... Patch Set 2: (6 comments) Commit Message: https://review.coreboot.org/c/coreboot/+/81626/comment/2522e74d_eac3fc4d : PS2, Line 11: would be good if you could add the "TEST" info File src/mainboard/google/brya/Kconfig: https://review.coreboot.org/c/coreboot/+/81626/comment/c812be61_8a8faf30 : PS2, Line 128: config BOARD_GOOGLE_BASEBOARD_TRULO : def_bool n : select BOARD_GOOGLE_BRYA_COMMON : select CHROMEOS_DRAM_PART_NUMBER_IN_CBI if CHROMEOS : select SOC_INTEL_ALDERLAKE_PCH_N : select SYSTEM_TYPE_LAPTOP all the baseboard related config are in alphabetical order, may be move this after the "BOARD_GOOGLE_BASEBOARD_NISSA" ? (start from line 102) File src/mainboard/google/brya/variants/baseboard/trulo/gpio.c: https://review.coreboot.org/c/coreboot/+/81626/comment/62e80ecb_d2bf28bf : PS2, Line 1: empty line after the line containing license information, just like how you did for ec.h file. https://review.coreboot.org/c/coreboot/+/81626/comment/2e728f0b_c0bbd5d1 : PS2, Line 5: #include <types.h> : #include <soc/gpio.h> sort them in alphabetical order ? https://review.coreboot.org/c/coreboot/+/81626/comment/4d0d9940_ceb51a89 : PS2, Line 8: /* Pad configuration in ramstage */ I would put an empty line after include file list ending (line 8 should be an empty line) File src/mainboard/google/brya/variants/baseboard/trulo/include/baseboard/ec.h: https://review.coreboot.org/c/coreboot/+/81626/comment/c45b893b_4a60115e : PS2, Line 6: #include <ec/ec.h> : #include <ec/google/chromeec/ec_commands.h> : #include <baseboard/gpio.h> sort them alphabetically .. -- To view, visit https://review.coreboot.org/c/coreboot/+/81626?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: Iad6230064c6b8359698d37c3e0440614cc7b073d Gerrit-Change-Number: 81626 Gerrit-PatchSet: 2 Gerrit-Owner: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Reviewer: Eric Lai <ericllai(a)google.com> Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Varshit Pandya <pandyavarshit(a)gmail.com> Gerrit-Attention: Eric Lai <ericllai(a)google.com> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Attention: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Comment-Date: Tue, 02 Apr 2024 10:16:32 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[XS] Change in coreboot[main]: drivers/mipi: Fine tune clock for BOE_NV110WUM_L60
by Paul Menzel (Code Review) 02 Apr '24

02 Apr '24

Attention is currently required from: Ruihai Zhou, Yidi Lin. Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/81438?usp=email ) The change is no longer submittable: All-Comments-Resolved is unsatisfied now. Change subject: drivers/mipi: Fine tune clock for BOE_NV110WUM_L60 ...................................................................... Patch Set 6: (1 comment) Commit Message: https://review.coreboot.org/c/coreboot/+/81438/comment/44624ba5_7ff3fef4 : PS4, Line 11: Just keep consistent with the Linux kernel panel driver : configuration. > The Linux patch has not yet been fully upstreamed, so there is no commit yet. […] Please add a URL then. -- To view, visit https://review.coreboot.org/c/coreboot/+/81438?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: Ic44c86f062d4e836f403ee97f2fc6370fff02797 Gerrit-Change-Number: 81438 Gerrit-PatchSet: 6 Gerrit-Owner: Ruihai Zhou <zhouruihai(a)huaqin.corp-partner.google.com> Gerrit-Reviewer: Yidi Lin <yidilin(a)google.com> Gerrit-Reviewer: Yu-Ping Wu <yupingso(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Reviewer: cong yang <yangcong5(a)huaqin.corp-partner.google.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Ruihai Zhou <zhouruihai(a)huaqin.corp-partner.google.com> Gerrit-Attention: Yidi Lin <yidilin(a)google.com> Gerrit-Comment-Date: Tue, 02 Apr 2024 10:06:10 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Paul Menzel <paulepanter(a)mailbox.org> Comment-In-Reply-To: Ruihai Zhou <zhouruihai(a)huaqin.corp-partner.google.com> Gerrit-MessageType: comment

1 0

[M] Change in coreboot[main]: mb/google/brya/var/xol: Reduce power limits according to battery status
by Paul Menzel (Code Review) 02 Apr '24

02 Apr '24

Attention is currently required from: Dinesh Gehlot, Kapil Porwal, Nick Vaccaro, SH Kim, Subrata Banik, Sumeet R Pawnikar. Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/81614?usp=email ) Change subject: mb/google/brya/var/xol: Reduce power limits according to battery status ...................................................................... Patch Set 5: Code-Review+1 -- To view, visit https://review.coreboot.org/c/coreboot/+/81614?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I5d71e9edde0ecbd7aaf316cd754a6ebcff9da77d Gerrit-Change-Number: 81614 Gerrit-PatchSet: 5 Gerrit-Owner: SH Kim <sh_.kim(a)samsung.corp-partner.google.com> Gerrit-Reviewer: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Reviewer: Eric Lai <ericllai(a)google.com> Gerrit-Reviewer: Jamie Chen <jamie.chen(a)intel.com> Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Sumeet R Pawnikar <sumeet.r.pawnikar(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: SH Kim <sh_.kim(a)samsung.corp-partner.google.com> Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Attention: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Attention: Sumeet R Pawnikar <sumeet.r.pawnikar(a)intel.com> Gerrit-Attention: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Comment-Date: Tue, 02 Apr 2024 10:05:36 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[S] Change in coreboot[main]: mb/google/nissa/var/gothrax: Add touchscreen driver for ILI2901A-A210
by Paul Menzel (Code Review) 02 Apr '24

02 Apr '24

Attention is currently required from: Derek Huang, Dinesh Gehlot, Kapil Porwal, Nick Vaccaro, Subrata Banik, Yunlong Jia. Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/81610?usp=email ) Change subject: mb/google/nissa/var/gothrax: Add touchscreen driver for ILI2901A-A210 ...................................................................... Patch Set 2: (1 comment) Commit Message: https://review.coreboot.org/c/coreboot/+/81610/comment/f6a1c822_e54a829c : PS2, Line 13: TEST=emerge-nissa coreboot How is the boot time affected? -- To view, visit https://review.coreboot.org/c/coreboot/+/81610?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I2d26bfd4f415aa128b6256f83bc58987b15a557a Gerrit-Change-Number: 81610 Gerrit-PatchSet: 2 Gerrit-Owner: Yunlong Jia <yunlong.jia(a)ecs.corp-partner.google.com> Gerrit-Reviewer: Derek Huang <derekhuang(a)google.com> Gerrit-Reviewer: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Reviewer: Eric Lai <ericllai(a)google.com> Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Kyle Lin <kylelinck(a)google.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Derek Huang <derekhuang(a)google.com> Gerrit-Attention: Yunlong Jia <yunlong.jia(a)ecs.corp-partner.google.com> Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Attention: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Attention: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Comment-Date: Tue, 02 Apr 2024 10:02:34 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[S] Change in coreboot[main]: mb/google/nissa/var/craaskov: Disable external fivr
by Ian Feng (Code Review) 02 Apr '24

02 Apr '24

Attention is currently required from: Dinesh Gehlot, Dtrain Hsu, Eric Lai, Kapil Porwal, Nick Vaccaro, Paul Menzel, Subrata Banik. Ian Feng has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/81607?usp=email ) Change subject: mb/google/nissa/var/craaskov: Disable external fivr ...................................................................... Patch Set 7: (1 comment) File src/mainboard/google/brya/variants/craaskov/variant.c: https://review.coreboot.org/c/coreboot/+/81607/comment/5cbd5465_8c0c82c3 : PS6, Line 57: External > external […] Done -- To view, visit https://review.coreboot.org/c/coreboot/+/81607?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I9280a86bf78caa10b527a6569ac580dfe1d66f60 Gerrit-Change-Number: 81607 Gerrit-PatchSet: 7 Gerrit-Owner: Ian Feng <ian_feng(a)compal.corp-partner.google.com> Gerrit-Reviewer: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Reviewer: Dtrain Hsu <dtrain_hsu(a)compal.corp-partner.google.com> Gerrit-Reviewer: Eric Lai <ericllai(a)google.com> Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-CC: Van Chen <van_chen(a)compal.corp-partner.google.com> Gerrit-Attention: Eric Lai <ericllai(a)google.com> Gerrit-Attention: Dtrain Hsu <dtrain_hsu(a)compal.corp-partner.google.com> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Attention: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Attention: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Comment-Date: Tue, 02 Apr 2024 09:49:09 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-MessageType: comment

1 0

[M] Change in coreboot[main]: mb/google/rex: Add Intel Touch for controller 1 for Rex
by Paul Menzel (Code Review) 02 Apr '24

02 Apr '24

Attention is currently required from: Cliff Huang, Dinesh Gehlot, Eran Mitrani, Jakub Czapiga, Jingyuan Liang, Kapil Porwal, Subrata Banik, Tarun. Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/81333?usp=email ) Change subject: mb/google/rex: Add Intel Touch for controller 1 for Rex ...................................................................... Patch Set 2: (2 comments) File src/mainboard/google/rex/variants/rex0/overridetree.cb: https://review.coreboot.org/c/coreboot/+/81333/comment/e0b15990_5fc7a5e0 : PS2, Line 33: THC NOTE for P1 and P2:reworked Please add a space after the colon. https://review.coreboot.org/c/coreboot/+/81333/comment/f9353863_a84c87ac : PS2, Line 84: register "thc_configuration" = "THC_CONFIG_DOUBLE_CONTROLLER" This reads not very nice. Can you come up with other value names? -- To view, visit https://review.coreboot.org/c/coreboot/+/81333?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: Id89b5b46d67b90491410d3d08c1a3ae9549b4da5 Gerrit-Change-Number: 81333 Gerrit-PatchSet: 2 Gerrit-Owner: Cliff Huang <cliff.huang(a)intel.com> Gerrit-Reviewer: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Reviewer: Eran Mitrani <mitrani(a)google.com> Gerrit-Reviewer: Jakub Czapiga <czapiga(a)google.com> Gerrit-Reviewer: Jingyuan Liang <jingyliang(a)google.com> Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: Kyoung Il Kim <kyoung.il.kim(a)intel.com> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Tarun <tstuli(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Hannah Williams <hannah.williams(a)intel.com> Gerrit-CC: Henry Barnor <hbarnor(a)chromium.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-CC: Shaunak Saha <shaunak.saha(a)intel.com> Gerrit-Attention: Cliff Huang <cliff.huang(a)intel.com> Gerrit-Attention: Jingyuan Liang <jingyliang(a)google.com> Gerrit-Attention: Eran Mitrani <mitrani(a)google.com> Gerrit-Attention: Jakub Czapiga <czapiga(a)google.com> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Attention: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Attention: Tarun <tstuli(a)gmail.com> Gerrit-Comment-Date: Tue, 02 Apr 2024 09:47:42 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[M] Change in coreboot[main]: soc/intel/meteorlake: Update Touch Controller UDP params
by Paul Menzel (Code Review) 02 Apr '24

02 Apr '24

Attention is currently required from: Arthur Heymans, Cliff Huang, Dinesh Gehlot, Eran Mitrani, Jakub Czapiga, Jingyuan Liang, Kapil Porwal, Subrata Banik, Tarun. Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/81332?usp=email ) Change subject: soc/intel/meteorlake: Update Touch Controller UDP params ...................................................................... Patch Set 2: (3 comments) Commit Message: https://review.coreboot.org/c/coreboot/+/81332/comment/3087c4a4_595fa05b : PS2, Line 9: The UPD, ThcAssignment, and ThcMode, ThcWakeOnTouch, are The UPDs ThcAssignment, ThcMode, and ThcWakeOnTouch are … https://review.coreboot.org/c/coreboot/+/81332/comment/b254989a_47a3bf3c : PS2, Line 11: a A https://review.coreboot.org/c/coreboot/+/81332/comment/6e7fa380_ec5dd3c8 : PS2, Line 11: a weak function drivers_intel_touch_config() is added Why weak? -- To view, visit https://review.coreboot.org/c/coreboot/+/81332?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I19edbcad705e889166d7d340b2caa74ab3bf15a1 Gerrit-Change-Number: 81332 Gerrit-PatchSet: 2 Gerrit-Owner: Cliff Huang <cliff.huang(a)intel.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Reviewer: Eran Mitrani <mitrani(a)google.com> Gerrit-Reviewer: Jakub Czapiga <czapiga(a)google.com> Gerrit-Reviewer: Jingyuan Liang <jingyliang(a)google.com> Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: Kyoung Il Kim <kyoung.il.kim(a)intel.com> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Tarun <tstuli(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Eric Lai <ericllai(a)google.com> Gerrit-CC: Hannah Williams <hannah.williams(a)intel.com> Gerrit-CC: Henry Barnor <hbarnor(a)chromium.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-CC: Shaunak Saha <shaunak.saha(a)intel.com> Gerrit-Attention: Cliff Huang <cliff.huang(a)intel.com> Gerrit-Attention: Jingyuan Liang <jingyliang(a)google.com> Gerrit-Attention: Eran Mitrani <mitrani(a)google.com> Gerrit-Attention: Jakub Czapiga <czapiga(a)google.com> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Attention: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Tarun <tstuli(a)gmail.com> Gerrit-Comment-Date: Tue, 02 Apr 2024 09:45:56 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[L] Change in coreboot[main]: drivers/intel/touch: Add driver for Intel Touch Controller and Devices
by Paul Menzel (Code Review) 02 Apr '24

02 Apr '24

Attention is currently required from: Cliff Huang, Eran Mitrani, Jingyuan Liang. Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/81331?usp=email ) Change subject: drivers/intel/touch: Add driver for Intel Touch Controller and Devices ...................................................................... Patch Set 2: (13 comments) Commit Message: https://review.coreboot.org/c/coreboot/+/81331/comment/184e0e62_7a5ce069 : PS2, Line 9: Support WACOM, ELAN, and generic touch sensor devices. Please write an introduction what that new device it, and what driver is needed for it. https://review.coreboot.org/c/coreboot/+/81331/comment/89bf1ef8_4788c5d9 : PS2, Line 10: This driver generates the following ACPI objects: Please add a blank line between paragraphs. https://review.coreboot.org/c/coreboot/+/81331/comment/0e02b73b_dd8b24bb : PS2, Line 11: _DSM : _CRS : Power resource with _STA, _ON, and _OFF : _RST Please format this as a list. File src/drivers/intel/touch/chip.h: PS2: Mention the datasheet name somewhere in the file? https://review.coreboot.org/c/coreboot/+/81331/comment/a098ed5e_d908f332 : PS2, Line 31: Thc THC? https://review.coreboot.org/c/coreboot/+/81331/comment/c1e7adf1_5ea83c8c : PS2, Line 31: : Please add a space after the colon. https://review.coreboot.org/c/coreboot/+/81331/comment/92dc8e80_6154a2d0 : PS2, Line 31: supporded supported https://review.coreboot.org/c/coreboot/+/81331/comment/d76bf549_880c6a02 : PS2, Line 33: supporded supported https://review.coreboot.org/c/coreboot/+/81331/comment/03e4030c_14ee889b : PS2, Line 33: ; Necessary? https://review.coreboot.org/c/coreboot/+/81331/comment/7e5475d6_29fd9b22 : PS2, Line 29: /* Touch Host Controller Mode : Switch between Intel THC protocol and Industry standard HID Over SPI protocol. : 0x0:Thc IPTS (not supporded at this time) : 0x1:Thc HID SPI : 0x2 Thc HID I2C; (not supporded at this time) : */ I’d use /* * […] */ https://review.coreboot.org/c/coreboot/+/81331/comment/750ea4c8_7d71a0b7 : PS2, Line 41: /* GPIO used to enable device. */ Redundant as the variable name says the same? https://review.coreboot.org/c/coreboot/+/81331/comment/ef8ae9aa_f360d06a : PS2, Line 46: enable_off Why not disable? File src/drivers/intel/touch/elan.h: https://review.coreboot.org/c/coreboot/+/81331/comment/53a68958_4d92ff22 : PS2, Line 9: #define ELAN_RST_SEQ_DLY 50 Is that time? Put the unit into the name? -- To view, visit https://review.coreboot.org/c/coreboot/+/81331?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I52dfcd187c92141cb8fc47f4143b90d243439d4e Gerrit-Change-Number: 81331 Gerrit-PatchSet: 2 Gerrit-Owner: Cliff Huang <cliff.huang(a)intel.com> Gerrit-Reviewer: Eran Mitrani <mitrani(a)google.com> Gerrit-Reviewer: Eric Lai <ericllai(a)google.com> Gerrit-Reviewer: Jingyuan Liang <jingyliang(a)google.com> Gerrit-Reviewer: Kyoung Il Kim <kyoung.il.kim(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Hannah Williams <hannah.williams(a)intel.com> Gerrit-CC: Henry Barnor <hbarnor(a)chromium.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-CC: Shaunak Saha <shaunak.saha(a)intel.com> Gerrit-Attention: Cliff Huang <cliff.huang(a)intel.com> Gerrit-Attention: Jingyuan Liang <jingyliang(a)google.com> Gerrit-Attention: Eran Mitrani <mitrani(a)google.com> Gerrit-Comment-Date: Tue, 02 Apr 2024 09:44:44 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[S] Change in coreboot[main]: lib: Refactor bmp_load_logo() implementation
by Subrata Banik (Code Review) 02 Apr '24

02 Apr '24

Attention is currently required from: Andrey Petrov, Felix Held, Fred Reitberger, Jason Glenesk, Julius Werner, Matt DeVillier, Ronak Kanabar, Sean Rhodes. Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/81618?usp=email ) Change subject: lib: Refactor bmp_load_logo() implementation ...................................................................... Patch Set 2: (2 comments) File src/lib/bmp_logo.c: https://review.coreboot.org/c/coreboot/+/81618/comment/4b386411_e17ec758 : PS1, Line 19: void bmp_load_logo(uintptr_t *logo_ptr, size_t *logo_size) > Why does this function take two out-pointers and have no return value, anyway? I think that's a very […] Acknowledged File src/soc/amd/mendocino/fsp_s_params.c: https://review.coreboot.org/c/coreboot/+/81618/comment/1e6f1be4_eef610b5 : PS1, Line 59: (uintptr_t *) > You can't do these kinds of things. […] Acknowledged -- To view, visit https://review.coreboot.org/c/coreboot/+/81618?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I14bc54670a67980ec93bc366b274832d1f959e50 Gerrit-Change-Number: 81618 Gerrit-PatchSet: 2 Gerrit-Owner: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Andrey Petrov <andrey.petrov(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)gmail.com> Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com> Gerrit-Reviewer: Ronak Kanabar <ronak.kanabar(a)intel.com> Gerrit-Reviewer: Sean Rhodes <sean(a)starlabs.systems> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Julius Werner <jwerner(a)chromium.org> Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Attention: Sean Rhodes <sean(a)starlabs.systems> Gerrit-Attention: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com> Gerrit-Attention: Julius Werner <jwerner(a)chromium.org> Gerrit-Attention: Ronak Kanabar <ronak.kanabar(a)intel.com> Gerrit-Attention: Fred Reitberger <reitbergerfred(a)gmail.com> Gerrit-Attention: Andrey Petrov <andrey.petrov(a)gmail.com> Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Comment-Date: Tue, 02 Apr 2024 09:42:40 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Julius Werner <jwerner(a)chromium.org> Gerrit-MessageType: comment

1 0

Jump to page:

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit April 2024