coreboot-gerrit November 2024

coreboot-gerrit@coreboot.org

1 participants
1920 discussions

[L] Change in coreboot[main]: mb/gigabyte/ga-h77m-d3h: Add Sandy/Ivy Bridge board GA-H77M-D3H
by Fabian Groffen (Code Review) 01 Nov '24

01 Nov '24

1 0

[L] Change in coreboot[main]: mb/gigabyte/ga-h77m-d3h: Add Sandy/Ivy Bridge board GA-H77M-D3H
by Fabian Groffen (Code Review) 01 Nov '24

01 Nov '24

Attention is currently required from: Angel Pons, Felix Singer, Patrick Rudolph. Fabian Groffen has posted comments on this change by Fabian Groffen. ( https://review.coreboot.org/c/coreboot/+/77046?usp=email ) Change subject: mb/gigabyte/ga-h77m-d3h: Add Sandy/Ivy Bridge board GA-H77M-D3H ...................................................................... Patch Set 8: (14 comments) Patchset: PS8: rebased, addressed comments File src/mainboard/gigabyte/ga-h77m-d3h/Kconfig: PS7: > Missing license header Done https://review.coreboot.org/c/coreboot/+/77046/comment/5698282c_812352e3?us… : PS7, Line 21: default 25 > As per schematics, this board doesn't seem to use a DRAM reset gate GPIO at all (the DRAMRST# signal […] Done File src/mainboard/gigabyte/ga-h77m-d3h/Kconfig.name: PS7: > Missing license header Done File src/mainboard/gigabyte/ga-h77m-d3h/devicetree.cb: PS7: > Missing license header Done https://review.coreboot.org/c/coreboot/+/77046/comment/297d8356_7092cc46?us… : PS7, Line 5: subsystemid 0x1458 0x5000 > See this very file, two lines above this one. Done https://review.coreboot.org/c/coreboot/+/77046/comment/9afc9200_122a3274?us… : PS7, Line 26: device ref mei2 off end # Management Engine Interface 2 > I think the chipset devicetree already provides default on/off states for named devices, so you shou […] Done https://review.coreboot.org/c/coreboot/+/77046/comment/cdffa7cb_1fdf11bd?us… : PS7, Line 36: device ref pcie_rp1 on end # PCIe Port #1 : device ref pcie_rp2 off end # PCIe Port #2 : device ref pcie_rp3 off end # PCIe Port #3 : device ref pcie_rp4 off end # PCIe Port #4 : device ref pcie_rp5 on # PCIe Port #5 : device pci 00.0 on # AR8161 GbE : end : end : device ref pcie_rp6 off end # PCIe Port #6 : device ref pcie_rp7 off end # PCIe Port #7 : device ref pcie_rp8 off end # PCIe Port #8 > Based on what I can see in board pictures [1], this is not accurate. […] Done https://review.coreboot.org/c/coreboot/+/77046/comment/93cb29c4_eb601f4e?us… : PS7, Line 50: device ref pci_bridge on end # PCI bridge > The PCI bridge in the southbridge doesn't seem to be used. You should disable it. Done https://review.coreboot.org/c/coreboot/+/77046/comment/e1d501fb_3f826d77?us… : PS7, Line 107: device pci 1f.4 off end > I don't think this device is supposed to exist Done File src/mainboard/gigabyte/ga-h77m-d3h/dsdt.asl: https://review.coreboot.org/c/coreboot/+/77046/comment/acd7c99c_c3af5d05?us… : PS7, Line 10: // OEM revision > Still, it's most likely autoport copy-paste. I'd drop the comment anyway. Done File src/mainboard/gigabyte/ga-h77m-d3h/gma-mainboard.ads: https://review.coreboot.org/c/coreboot/+/77046/comment/162b1eae_2c30fad5?us… : PS7, Line 1: GPL-2.0-only > GPL-2. […] Done File src/mainboard/gigabyte/ga-h77m-d3h/gpio.c: PS7: > Was this file copied as-is from the other board? I hope not, because bad GPIO configuration can brea […] Done File src/mainboard/gigabyte/ga-h77m-d3h/thermal.h: https://review.coreboot.org/c/coreboot/+/77046/comment/3379d9a6_d5fb1713?us… : PS7, Line 9: /* Temperature which OS will throttle CPU */ > We usually don't indent defines. Please remove. Done -- To view, visit https://review.coreboot.org/c/coreboot/+/77046?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: Icb3e74326a0a7aaf770d1917a2a0931feadd7eab Gerrit-Change-Number: 77046 Gerrit-PatchSet: 8 Gerrit-Owner: Fabian Groffen <grobian(a)gentoo.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Felix Singer <service+coreboot-gerrit(a)felixsinger.de> Gerrit-CC: Keith Hui <buurin(a)gmail.com> Gerrit-CC: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Attention: Felix Singer <service+coreboot-gerrit(a)felixsinger.de> Gerrit-Attention: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Fri, 01 Nov 2024 20:14:43 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Felix Singer <service+coreboot-gerrit(a)felixsinger.de> Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Comment-In-Reply-To: Fabian Groffen <grobian(a)gentoo.org>

1 0

[L] Change in coreboot[main]: mb/gigabyte/ga-h77m-d3h: Add Sandy/Ivy Bridge board GA-H77M-D3H
by Fabian Groffen (Code Review) 01 Nov '24

01 Nov '24

Attention is currently required from: Angel Pons, Fabian Groffen, Patrick Rudolph. Hello Angel Pons, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/77046?usp=email to look at the new patch set (#8). The following approvals got outdated and were removed: Code-Review+1 by Angel Pons, Verified+1 by build bot (Jenkins) Change subject: mb/gigabyte/ga-h77m-d3h: Add Sandy/Ivy Bridge board GA-H77M-D3H ...................................................................... mb/gigabyte/ga-h77m-d3h: Add Sandy/Ivy Bridge board GA-H77M-D3H This board is based off ga-b75m-d3h, which uses the same SuperIO chip. It doesn't have the ASMedia SATA3 controller, the H77 chipset comes with 2 SATA3 ports next to the 4 SATA2 ports. Flashing notes: These boards come with dual-BIOS feature. This is set of two unremovable what appears to be identical chips marked M_BIOS and B_BIOS. Flash the B_BIOS chip, and boot the system. Ensure you have a payload and setup ready to boot a Linux system with iomem=relaxed or similar. Immediately use flashrom -p internal to flash the same firmware again. If you skip this step your next boot will show weird exception traces in either coreboot or your payload. Flashing from there via the chip is very difficult (you have to try many times in order to get a booting run), which can all be remedied by doing a flash from internal. I suppose the dual-BIOS feature is somewhat in the way here. Tested with: - CPU Core i7-3770S - RAM single bank 4GB CL11, two banks 4+4GB CL11 - OS Gentoo Linux LiveUSB, KDE desktop (Linux 5.15.72) Working: - GRUB2 payload - Intel ME stripped - Integrated graphics with libgfxinit - (boot from) SATA2, SATA3 ports - Rear and mainboard connector USB ports, supporting boot - Atheros GbE NIC - 2.0 channel audio via lineout jack output - ACPI (power button triggers OS events) - S3 suspend/resume - PWM FAN control, FAN speed readings - Temperature sensor readings Signed-off-by: Fabian Groffen <grobian(a)gentoo.org> Change-Id: Icb3e74326a0a7aaf770d1917a2a0931feadd7eab --- A src/mainboard/gigabyte/ga-h77m-d3h/Kconfig A src/mainboard/gigabyte/ga-h77m-d3h/Kconfig.name A src/mainboard/gigabyte/ga-h77m-d3h/Makefile.mk A src/mainboard/gigabyte/ga-h77m-d3h/acpi/ec.asl A src/mainboard/gigabyte/ga-h77m-d3h/acpi/platform.asl A src/mainboard/gigabyte/ga-h77m-d3h/acpi/superio.asl A src/mainboard/gigabyte/ga-h77m-d3h/board_info.txt A src/mainboard/gigabyte/ga-h77m-d3h/cmos.default A src/mainboard/gigabyte/ga-h77m-d3h/cmos.layout A src/mainboard/gigabyte/ga-h77m-d3h/data.vbt A src/mainboard/gigabyte/ga-h77m-d3h/devicetree.cb A src/mainboard/gigabyte/ga-h77m-d3h/dsdt.asl A src/mainboard/gigabyte/ga-h77m-d3h/early_init.c A src/mainboard/gigabyte/ga-h77m-d3h/gma-mainboard.ads A src/mainboard/gigabyte/ga-h77m-d3h/gpio.c A src/mainboard/gigabyte/ga-h77m-d3h/hda_verb.c A src/mainboard/gigabyte/ga-h77m-d3h/thermal.h 17 files changed, 582 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/46/77046/8 -- To view, visit https://review.coreboot.org/c/coreboot/+/77046?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: newpatchset Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: Icb3e74326a0a7aaf770d1917a2a0931feadd7eab Gerrit-Change-Number: 77046 Gerrit-PatchSet: 8 Gerrit-Owner: Fabian Groffen <grobian(a)gentoo.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Felix Singer <service+coreboot-gerrit(a)felixsinger.de> Gerrit-CC: Keith Hui <buurin(a)gmail.com> Gerrit-CC: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Attention: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Fabian Groffen <grobian(a)gentoo.org>

1 0

[M] Change in coreboot[main]: drivers/wifi: Support Bluetooth Per-Platform Antenna Gain
by Bora Guvendik (Code Review) 01 Nov '24

01 Nov '24

Attention is currently required from: Anil Kumar K, Cliff Huang, Hannah Williams, Jamie Ryu, Jérémy Compostella, Zhixing Ma, dinesh.sharma(a)intel.com. Bora Guvendik has posted comments on this change by Jérémy Compostella. ( https://review.coreboot.org/c/coreboot/+/84941?usp=email ) Change subject: drivers/wifi: Support Bluetooth Per-Platform Antenna Gain ...................................................................... Patch Set 1: (1 comment) File src/drivers/wifi/generic/acpi.c: https://review.coreboot.org/c/coreboot/+/84941/comment/bbe8e9fe_8b439b39?us… : PS1, Line 675: BTPPAG should it be PPAG since acpigen_write_name("PPAG");? -- To view, visit https://review.coreboot.org/c/coreboot/+/84941?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: Iebe95815c944d045f4cf686abcd1874a8a45e210 Gerrit-Change-Number: 84941 Gerrit-PatchSet: 1 Gerrit-Owner: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Reviewer: Anil Kumar K <anil.kumar.k(a)intel.com> Gerrit-Reviewer: Bora Guvendik <bora.guvendik(a)intel.com> Gerrit-Reviewer: Cliff Huang <cliff.huang(a)intel.com> Gerrit-Reviewer: Hannah Williams <hannah.williams(a)intel.com> Gerrit-Reviewer: Jamie Ryu <jamie.m.ryu(a)intel.com> Gerrit-Reviewer: Zhixing Ma <zhixing.ma(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Reviewer: dinesh.sharma(a)intel.com Gerrit-Attention: Zhixing Ma <zhixing.ma(a)intel.com> Gerrit-Attention: Anil Kumar K <anil.kumar.k(a)intel.com> Gerrit-Attention: Hannah Williams <hannah.williams(a)intel.com> Gerrit-Attention: Cliff Huang <cliff.huang(a)intel.com> Gerrit-Attention: Jamie Ryu <jamie.m.ryu(a)intel.com> Gerrit-Attention: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Attention: dinesh.sharma(a)intel.com Gerrit-Comment-Date: Fri, 01 Nov 2024 18:23:23 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No

1 0

[M] Change in coreboot[main]: drivers/wifi: Remove unnecessary data structure copy
by Jérémy Compostella (Code Review) 01 Nov '24

01 Nov '24

Attention is currently required from: Anil Kumar K, Bora Guvendik, Cliff Huang, Hannah Williams, Jamie Ryu, Zhixing Ma, dinesh.sharma(a)intel.com. Jérémy Compostella has posted comments on this change by Jérémy Compostella. ( https://review.coreboot.org/c/coreboot/+/84940?usp=email ) Change subject: drivers/wifi: Remove unnecessary data structure copy ...................................................................... Patch Set 1: (1 comment) File src/drivers/wifi/generic/acpi.c: https://review.coreboot.org/c/coreboot/+/84940/comment/05484bdf_302a79cb?us… : PS1, Line 672: /* > add sar_limits null check? This is a static (private) function and we only call it with sar_limits != NULL -- To view, visit https://review.coreboot.org/c/coreboot/+/84940?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: Iebe95815c944d045f4cf686abcd1874a8a45e207 Gerrit-Change-Number: 84940 Gerrit-PatchSet: 1 Gerrit-Owner: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Reviewer: Anil Kumar K <anil.kumar.k(a)intel.com> Gerrit-Reviewer: Bora Guvendik <bora.guvendik(a)intel.com> Gerrit-Reviewer: Cliff Huang <cliff.huang(a)intel.com> Gerrit-Reviewer: Hannah Williams <hannah.williams(a)intel.com> Gerrit-Reviewer: Jamie Ryu <jamie.m.ryu(a)intel.com> Gerrit-Reviewer: Zhixing Ma <zhixing.ma(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Reviewer: dinesh.sharma(a)intel.com Gerrit-Attention: Bora Guvendik <bora.guvendik(a)intel.com> Gerrit-Attention: Zhixing Ma <zhixing.ma(a)intel.com> Gerrit-Attention: Anil Kumar K <anil.kumar.k(a)intel.com> Gerrit-Attention: Hannah Williams <hannah.williams(a)intel.com> Gerrit-Attention: Cliff Huang <cliff.huang(a)intel.com> Gerrit-Attention: Jamie Ryu <jamie.m.ryu(a)intel.com> Gerrit-Attention: dinesh.sharma(a)intel.com Gerrit-Comment-Date: Fri, 01 Nov 2024 18:10:24 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Bora Guvendik <bora.guvendik(a)intel.com>

1 0

[M] Change in coreboot[main]: drivers/wifi: Remove unnecessary data structure copy
by Bora Guvendik (Code Review) 01 Nov '24

01 Nov '24

Attention is currently required from: Anil Kumar K, Cliff Huang, Hannah Williams, Jamie Ryu, Jérémy Compostella, Zhixing Ma, dinesh.sharma(a)intel.com. Bora Guvendik has posted comments on this change by Jérémy Compostella. ( https://review.coreboot.org/c/coreboot/+/84940?usp=email ) Change subject: drivers/wifi: Remove unnecessary data structure copy ...................................................................... Patch Set 1: (1 comment) File src/drivers/wifi/generic/acpi.c: https://review.coreboot.org/c/coreboot/+/84940/comment/9838b381_d466e38e?us… : PS1, Line 672: /* add sar_limits null check? -- To view, visit https://review.coreboot.org/c/coreboot/+/84940?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: Iebe95815c944d045f4cf686abcd1874a8a45e207 Gerrit-Change-Number: 84940 Gerrit-PatchSet: 1 Gerrit-Owner: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Reviewer: Anil Kumar K <anil.kumar.k(a)intel.com> Gerrit-Reviewer: Bora Guvendik <bora.guvendik(a)intel.com> Gerrit-Reviewer: Cliff Huang <cliff.huang(a)intel.com> Gerrit-Reviewer: Hannah Williams <hannah.williams(a)intel.com> Gerrit-Reviewer: Jamie Ryu <jamie.m.ryu(a)intel.com> Gerrit-Reviewer: Zhixing Ma <zhixing.ma(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Reviewer: dinesh.sharma(a)intel.com Gerrit-Attention: Zhixing Ma <zhixing.ma(a)intel.com> Gerrit-Attention: Anil Kumar K <anil.kumar.k(a)intel.com> Gerrit-Attention: Hannah Williams <hannah.williams(a)intel.com> Gerrit-Attention: Cliff Huang <cliff.huang(a)intel.com> Gerrit-Attention: Jamie Ryu <jamie.m.ryu(a)intel.com> Gerrit-Attention: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Attention: dinesh.sharma(a)intel.com Gerrit-Comment-Date: Fri, 01 Nov 2024 18:05:47 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No

1 0

[S] Change in coreboot[main]: ec/google/chromeec: Add is_battery_present()
by Karthik Ramasubramanian (Code Review) 01 Nov '24

01 Nov '24

Attention is currently required from: Caveh Jalali, Dinesh Gehlot, Eric Herrmann, Forest Mittelberg, Kapil Porwal, Keith Short, Nick Vaccaro, Rishika Raj, Shelly Chang, Sowmya Aralguppe, Subrata Banik. Karthik Ramasubramanian has posted comments on this change by Sowmya Aralguppe. ( https://review.coreboot.org/c/coreboot/+/83735?usp=email ) The change is no longer submittable: All-Comments-Resolved is unsatisfied now. Change subject: ec/google/chromeec: Add is_battery_present() ...................................................................... Patch Set 7: Code-Review+2 (1 comment) Commit Message: https://review.coreboot.org/c/coreboot/+/83735/comment/ed54a920_4f5afd91?us… : PS7, Line 15: Is there an EC dependency. If so, can you please mark so using `Cq-Depend: chromium:xxxx` -- To view, visit https://review.coreboot.org/c/coreboot/+/83735?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: Ie49ed8f6d8b0fa59ec0e7b06efea9cac4d253957 Gerrit-Change-Number: 83735 Gerrit-PatchSet: 7 Gerrit-Owner: Sowmya Aralguppe <sowmya.aralguppe(a)intel.com> Gerrit-Reviewer: Caveh Jalali <caveh(a)chromium.org> Gerrit-Reviewer: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Reviewer: Eric Herrmann <eherrmann(a)google.com> Gerrit-Reviewer: Forest Mittelberg <bmbm(a)google.com> Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Reviewer: Keith Short <keithshort(a)google.com> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Reviewer: Rishika Raj <rishikaraj(a)google.com> Gerrit-Reviewer: Shelly Chang <Shelly_Chang(a)wiwynn.com> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Ashish Kumar Mishra <ashish.k.mishra(a)intel.com> Gerrit-CC: Deepti Deshatty <deepti.deshatty(a)intel.com> Gerrit-CC: Vinay Kumar <vinay.kumar(a)intel.com> Gerrit-Attention: Sowmya Aralguppe <sowmya.aralguppe(a)intel.com> Gerrit-Attention: Rishika Raj <rishikaraj(a)google.com> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Caveh Jalali <caveh(a)chromium.org> Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Attention: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Attention: Eric Herrmann <eherrmann(a)google.com> Gerrit-Attention: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Attention: Keith Short <keithshort(a)google.com> Gerrit-Attention: Shelly Chang <Shelly_Chang(a)wiwynn.com> Gerrit-Attention: Forest Mittelberg <bmbm(a)google.com> Gerrit-Comment-Date: Fri, 01 Nov 2024 18:03:08 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes

1 0

[M] Change in coreboot[main]: mb/google/fizz: Fix USB port defintions
by Matt DeVillier (Code Review) 01 Nov '24

01 Nov '24

Attention is currently required from: Felix Singer. Hello Felix Singer, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/84955?usp=email to look at the new patch set (#2). Change subject: mb/google/fizz: Fix USB port defintions ...................................................................... mb/google/fizz: Fix USB port defintions commit 6c83a71b0a80 ("skl mainboards/dt: Move usb{2,3}_ports settings into XHCI device scope") not only moved the USB port definitions under the XHCI device reference, but also combined multiple register definitions. In doing so, it broke the inheritance from the baseboard, since the variant overridetree registers now replaced the entire usb2_ports/usb3_ports structs, rather than replacing individual array elements therein. This resulted in any USB ports inherited from the baseboard and not overridden by the variant being non-functional as they were not included in the resulting combined devicetree. To fix this, return to overriding individual array elements in the usb2/3_ports structs. TEST=build/boot google/fizz/var/karma. Verify all USB ports present and functional. Verify mainboard/static.c in built shows all ports. Change-Id: I0e80bf4949a857c21d44537eb720a7a8a7db2f80 Signed-off-by: Matt DeVillier <matt.devillier(a)gmail.com> --- M src/mainboard/google/fizz/variants/baseboard/devicetree.cb M src/mainboard/google/fizz/variants/endeavour/overridetree.cb M src/mainboard/google/fizz/variants/karma/overridetree.cb 3 files changed, 34 insertions(+), 44 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/55/84955/2 -- To view, visit https://review.coreboot.org/c/coreboot/+/84955?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: newpatchset Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I0e80bf4949a857c21d44537eb720a7a8a7db2f80 Gerrit-Change-Number: 84955 Gerrit-PatchSet: 2 Gerrit-Owner: Matt DeVillier <matt.devillier(a)gmail.com> Gerrit-Reviewer: Felix Singer <service+coreboot-gerrit(a)felixsinger.de> Gerrit-Attention: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>

1 0

[M] Change in coreboot[main]: mb/google/fizz: Fix USB port defintions
by Matt DeVillier (Code Review) 01 Nov '24

01 Nov '24

Matt DeVillier has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/84955?usp=email ) Change subject: mb/google/fizz: Fix USB port defintions ...................................................................... mb/google/fizz: Fix USB port defintions commit 6c83a71b0a80 ("skl mainboards/dt: Move usb{2,3}_ports settings into XHCI device scope") not only moved the USB port definitions under the XHCI device reference, but also combined multiple register definitions. In doing so, it broke the inheritance from the baseboard, since the variant overridetree registers now replaced the entire usb2_ports/usb3_ports structs, rather than replacing individual array elements therein. This resulted in any USB ports inherited from the baseboard and not overridden by the variant being non-function as they were not included in the resulting combined devicetree. To fix this, return to overriding individual array elements in the usb2/3_ports structs. TEST=build/boot google/fizz/var/karma. Verify all USB ports present and functional. Verify mainboard/static.c in built shows all ports. Change-Id: I0e80bf4949a857c21d44537eb720a7a8a7db2f80 Signed-off-by: Matt DeVillier <matt.devillier(a)gmail.com> --- M src/mainboard/google/fizz/variants/baseboard/devicetree.cb M src/mainboard/google/fizz/variants/endeavour/overridetree.cb M src/mainboard/google/fizz/variants/karma/overridetree.cb 3 files changed, 34 insertions(+), 44 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/55/84955/1 diff --git a/src/mainboard/google/fizz/variants/baseboard/devicetree.cb b/src/mainboard/google/fizz/variants/baseboard/devicetree.cb index 2976deb..25114f0 100644 --- a/src/mainboard/google/fizz/variants/baseboard/devicetree.cb +++ b/src/mainboard/google/fizz/variants/baseboard/devicetree.cb @@ -204,25 +204,22 @@ device ref igpu on end device ref sa_thermal on end device ref south_xhci on - register "usb2_ports" = "{ - [0] = USB2_PORT_LONG(OC0), // Type-C - [1] = USB2_PORT_MID(OC3), // Type-A Rear - [2] = USB2_PORT_MID(OC2), // Type-A Front - [3] = USB2_PORT_MID(OC2), // Type-A Front - [4] = USB2_PORT_MID(OC1), // Type-A Rear - [5] = USB2_PORT_MID(OC1), // Type-A Rear - [6] = USB2_PORT_MID(OC_SKIP), // Bluetooth - [7] = USB2_PORT_MID(OC_SKIP), // Type-A 2.0 / Debug - }" + register "usb2_ports[0]" = "USB2_PORT_LONG(OC0)" # Type-C + register "usb2_ports[1]" = "USB2_PORT_MID(OC3)" # Type-A Rear + register "usb2_ports[2]" = "USB2_PORT_MID(OC2)" # Type-A Front + register "usb2_ports[3]" = "USB2_PORT_MID(OC2)" # Type-A Front + register "usb2_ports[4]" = "USB2_PORT_MID(OC1)" # Type-A Rear + register "usb2_ports[5]" = "USB2_PORT_MID(OC1)" # Type-A Rear + register "usb2_ports[6]" = "USB2_PORT_MID(OC_SKIP)" # Bluetooth + register "usb2_ports[7]" = "USB2_PORT_MID(OC_SKIP)" # Type-A 2.0 / Debug - register "usb3_ports" = "{ - [0] = USB3_PORT_DEFAULT(OC0), // Type-C - [1] = USB3_PORT_DEFAULT(OC3), // Type-A Rear - [2] = USB3_PORT_DEFAULT(OC2), // Type-A Front - [3] = USB3_PORT_DEFAULT(OC2), // Type-A Front - [4] = USB3_PORT_DEFAULT(OC1), // Type-A Rear - [5] = USB3_PORT_DEFAULT(OC1), // Type-A Rear - }" + register "usb3_ports[0]" = "USB3_PORT_DEFAULT(OC0)" # Type-C + register "usb3_ports[1]" = "USB3_PORT_DEFAULT(OC3)" # Type-A Rear + register "usb3_ports[2]" = "USB3_PORT_DEFAULT(OC2)" # Type-A Front + register "usb3_ports[3]" = "USB3_PORT_DEFAULT(OC2)" # Type-A Front + register "usb3_ports[4]" = "USB3_PORT_DEFAULT(OC1)" # Type-A Rear + register "usb3_ports[5]" = "USB3_PORT_DEFAULT(OC1)" # Type-A Rear + chip drivers/usb/acpi register "desc" = ""Root Hub"" register "type" = "UPC_TYPE_HUB" diff --git a/src/mainboard/google/fizz/variants/endeavour/overridetree.cb b/src/mainboard/google/fizz/variants/endeavour/overridetree.cb index 649b5c1..f359bf3 100644 --- a/src/mainboard/google/fizz/variants/endeavour/overridetree.cb +++ b/src/mainboard/google/fizz/variants/endeavour/overridetree.cb @@ -22,23 +22,20 @@ device domain 0 on device ref south_xhci on - register "usb2_ports" = "{ - [0] = USB2_PORT_LONG(OC_SKIP), // Type-C - [1] = USB2_PORT_MID(OC_SKIP), // HDMI - [2] = USB2_PORT_MID(OC2), // Type-A Rear - [3] = USB2_PORT_MID(OC2), // Type-A Rear - [4] = USB2_PORT_MID(OC3), // Type-A Rear - [5] = USB2_PORT_MID(OC_SKIP), // HDMI Audio - [6] = USB2_PORT_MID(OC_SKIP), // Bluetooth - }" + register "usb2_ports[0]" = "USB2_PORT_LONG(OC_SKIP)" # Type-C + register "usb2_ports[1]" = "USB2_PORT_MID(OC_SKIP)" # HDMI + register "usb2_ports[2]" = "USB2_PORT_MID(OC2)" # Type-A Rear + register "usb2_ports[3]" = "USB2_PORT_MID(OC2)" # Type-A Rear + register "usb2_ports[4]" = "USB2_PORT_MID(OC3)" # Type-A Rear + register "usb2_ports[5]" = "USB2_PORT_MID(OC_SKIP)" # HDMI Audio + register "usb2_ports[6]" = "USB2_PORT_MID(OC_SKIP)" # Bluetooth - register "usb3_ports" = "{ - [0] = USB3_PORT_DEFAULT(OC_SKIP), // Type-C - [1] = USB3_PORT_DEFAULT(OC_SKIP), // HDMI - [2] = USB3_PORT_DEFAULT(OC2), // Type-A Rear - [3] = USB3_PORT_DEFAULT(OC2), // Type-A Rear - [4] = USB3_PORT_DEFAULT(OC3), // Type-A Rear - }" + register "usb3_ports[0]" = "USB3_PORT_DEFAULT(OC_SKIP)" # Type-C + register "usb3_ports[1]" = "USB3_PORT_DEFAULT(OC_SKIP)" # HDMI + register "usb3_ports[2]" = "USB3_PORT_DEFAULT(OC2)" # Type-A Rear + register "usb3_ports[3]" = "USB3_PORT_DEFAULT(OC2)" # Type-A Rear + register "usb3_ports[4]" = "USB3_PORT_DEFAULT(OC3)" # Type-A Rear + chip drivers/usb/acpi device usb 0.0 on chip drivers/usb/acpi diff --git a/src/mainboard/google/fizz/variants/karma/overridetree.cb b/src/mainboard/google/fizz/variants/karma/overridetree.cb index 25b39f4..53fef3c 100644 --- a/src/mainboard/google/fizz/variants/karma/overridetree.cb +++ b/src/mainboard/google/fizz/variants/karma/overridetree.cb @@ -15,17 +15,13 @@ #| Camera | 8 | | #| Touchsreen | 10 | | #+----------------+-------+-----------------------------------+ - register "usb2_ports" = "{ - [2] = USB2_PORT_MID(OC2), // Type-A Side - [3] = USB2_PORT_MID(OC_SKIP), // Card reader - [7] = USB2_PORT_MID(OC_SKIP), // Camera - [9] = USB2_PORT_MID(OC_SKIP), // Touchscreen - }" + register "usb2_ports[2]" = "USB2_PORT_MID(OC2)" # Type-A Side + register "usb2_ports[3]" = "USB2_PORT_MID(OC_SKIP)" # Card reader + register "usb2_ports[7]" = "USB2_PORT_MID(OC_SKIP)" # Camera + register "usb2_ports[9]" = "USB2_PORT_MID(OC_SKIP)" # Touchscreen - register "usb3_ports" = "{ - [2] = USB3_PORT_DEFAULT(OC2), // Type-A Side - [3] = USB3_PORT_DEFAULT(OC_SKIP), // Card reader - }" + register "usb3_ports[2]" = "USB3_PORT_DEFAULT(OC2)" # Type-A Side + register "usb3_ports[3]" = "USB3_PORT_DEFAULT(OC_SKIP)" # Card reader chip drivers/usb/acpi device usb 0.0 on -- To view, visit https://review.coreboot.org/c/coreboot/+/84955?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: newchange Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I0e80bf4949a857c21d44537eb720a7a8a7db2f80 Gerrit-Change-Number: 84955 Gerrit-PatchSet: 1 Gerrit-Owner: Matt DeVillier <matt.devillier(a)gmail.com>

1 0

[XS] Change in coreboot[main]: soc/intel/alderlake: Fix uninitialized usb_cfg pointer
by Karthik Ramasubramanian (Code Review) 01 Nov '24

01 Nov '24

Attention is currently required from: Dinesh Gehlot, Jayvik Desai, Kapil Porwal, Nick Vaccaro, Subrata Banik. Karthik Ramasubramanian has posted comments on this change by Karthik Ramasubramanian. ( https://review.coreboot.org/c/coreboot/+/84939?usp=email ) Change subject: soc/intel/alderlake: Fix uninitialized usb_cfg pointer ...................................................................... Patch Set 2: (1 comment) File src/soc/intel/alderlake/fsp_params.c: https://review.coreboot.org/c/coreboot/+/84939/comment/8cc25392_9501a47a?us… : PS2, Line 908: : usb_cfg = port->chip_info; > can we add a NULL check here ? The next line confirms that usb_cfg is not NULL. From the static analyzer's standpoint, we may not even enter the while loop or everything in the loop is skipped based on if block in line 906. So usb_cfg will be NULL and hence needs to be checked specifically in line 934. -- To view, visit https://review.coreboot.org/c/coreboot/+/84939?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I764fed561dfe2a571f3404fe505997edd7aa5ff7 Gerrit-Change-Number: 84939 Gerrit-PatchSet: 2 Gerrit-Owner: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Reviewer: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Reviewer: Jayvik Desai <jayvik(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-Attention: Jayvik Desai <jayvik(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: Fri, 01 Nov 2024 15:46:15 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com>

1 0

Jump to page:

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit November 2024