Attention is currently required from: Matt Chen, Poornima Tom, Poornima Tom.
Paul Menzel has posted comments on this change by Poornima Tom. ( https://review.coreboot.org/c/coreboot/+/84060?usp=email )
Change subject: [skolas]: Add reference to wifi_sar_0.hex file
......................................................................
Patch Set 5:
(1 comment)
Commit Message:
https://review.coreboot.org/c/coreboot/+/84060/comment/c8c66e12_19bd0e0c?us… :
PS5, Line 7: [skolas]: Add reference to wifi_sar_0.hex file
Please go through `git log --oneline` to see commonly used prefixes.
--
To view, visit https://review.coreboot.org/c/coreboot/+/84060?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: Ic3bb2088f970e829f43a49e0e3497049eccf2b6d
Gerrit-Change-Number: 84060
Gerrit-PatchSet: 5
Gerrit-Owner: Poornima Tom <poornima.tom(a)intel.com>
Gerrit-Reviewer: Matt Chen <matt.chen(a)intel.corp-partner.google.com>
Gerrit-Reviewer: Poornima Tom <poornima.tom(a)intel.corp-partner.google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Poornima Tom <poornima.tom(a)intel.com>
Gerrit-Attention: Poornima Tom <poornima.tom(a)intel.corp-partner.google.com>
Gerrit-Attention: Matt Chen <matt.chen(a)intel.corp-partner.google.com>
Gerrit-Comment-Date: Mon, 26 Aug 2024 10:00:35 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Attention is currently required from: Jakub Czapiga, Nico Huber.
Paul Menzel has posted comments on this change by Nico Huber. ( https://review.coreboot.org/c/coreboot/+/84088?usp=email )
Change subject: b64_decode-test: Properly terminate strings before comparison
......................................................................
Patch Set 1: Code-Review+1
(2 comments)
Patchset:
PS1:
Thank you for analyzing this.
Commit Message:
https://review.coreboot.org/c/coreboot/+/84088/comment/7d8d0bab_1859ea94?us… :
PS1, Line 12:
Add test output failure before your change?
--
To view, visit https://review.coreboot.org/c/coreboot/+/84088?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: Id1bd2c3ff06bc1d4e5aa21ddd0f1d5802540999d
Gerrit-Change-Number: 84088
Gerrit-PatchSet: 1
Gerrit-Owner: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Jakub Czapiga <czapiga(a)google.com>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Nicholas Sudsgaard <devel+coreboot(a)nsudsgaard.com>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Jakub Czapiga <czapiga(a)google.com>
Gerrit-Comment-Date: Mon, 26 Aug 2024 09:59:53 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Attention is currently required from: KunYi Chen, Martin L Roth, Nicholas Chin.
Nicholas Sudsgaard has posted comments on this change by KunYi Chen. ( https://review.coreboot.org/c/coreboot/+/83719?usp=email )
Change subject: mb/lattepanda: Add support for LattePanda Mu
......................................................................
Patch Set 8:
(2 comments)
File src/mainboard/lattepanda/mu/devicetree.cb:
PS8:
Missing SPDX license.
https://review.coreboot.org/c/coreboot/+/83719/comment/d81f121e_9d7b6c77?us… :
PS8, Line 9: register "usb2_ports[0]" = "USB2_PORT_MID(OC_SKIP)" # USB3/2 Type A
: register "usb2_ports[1]" = "USB2_PORT_MID(OC_SKIP)" # USB3/2 Type A
: register "usb2_ports[2]" = "USB2_PORT_MID(OC_SKIP)" # USB2 Type A
: register "usb2_ports[3]" = "USB2_PORT_EMPTY"
: register "usb2_ports[4]" = "USB2_PORT_MID(OC_SKIP)" # USB2 Type A
: register "usb2_ports[5]" = "USB2_PORT_EMPTY"
: register "usb2_ports[6]" = "USB2_PORT_MID(OC_SKIP)" # M.2 E 2230
:
: register "usb3_ports[0]" = "USB3_PORT_EMPTY"
: register "usb3_ports[1]" = "USB3_PORT_EMPTY"
: register "usb3_ports[2]" = "USB3_PORT_DEFAULT(OC_SKIP)" # USB3/2 Type A
: register "usb3_ports[3]" = "USB3_PORT_DEFAULT(OC_SKIP)" # USB3/2 Type A
:
Use this format, and place it under `xhci`.
```
register "usb2_ports" = "{
[0] = USB2_PORT_MID(OC_SKIP), // comment
...
}"
```
--
To view, visit https://review.coreboot.org/c/coreboot/+/83719?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: I79696bdd837a221860b32f54629212c3346dca66
Gerrit-Change-Number: 83719
Gerrit-PatchSet: 8
Gerrit-Owner: KunYi Chen
Gerrit-Reviewer: Nicholas Chin <nic.c3.14(a)gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Martin L Roth <gaumless(a)gmail.com>
Gerrit-CC: Nicholas Sudsgaard <devel+coreboot(a)nsudsgaard.com>
Gerrit-Attention: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Attention: KunYi Chen
Gerrit-Attention: Nicholas Chin <nic.c3.14(a)gmail.com>
Gerrit-Comment-Date: Mon, 26 Aug 2024 09:47:01 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Attention is currently required from: Arthur Heymans, Dinesh Gehlot, Jayvik Desai, Kapil Porwal, Nick Vaccaro, Rishika Raj.
Subrata Banik has posted comments on this change by Subrata Banik. ( https://review.coreboot.org/c/coreboot/+/84080?usp=email )
Change subject: soc/intel/adl: Prevent unconditional legacy COM ports initialization
......................................................................
Patch Set 1:
(1 comment)
Commit Message:
https://review.coreboot.org/c/coreboot/+/84080/comment/20b56b48_acf93f32?us… :
PS1, Line 14: These COM ports are being activated unconditionally, which is
: undesirable for the Intel Alder Lake platform and causes traffic over
: the IO bus.
> > I don't understand this. What code is generating those writes?
>
> sorry if things are not cleared. As we are not using Legacy UART for ChromeOS device but looking at the Alder Lake code, we found that Legacy COMs are default enabled and we have observed some activity across those ports. Hence, pushed this CL to ensure legacy COMs are not getting enabled when the desired kconfig are not being selected
After booting to Kernel, we are seeing some traffic. For sure that is not any FW code.
--
To view, visit https://review.coreboot.org/c/coreboot/+/84080?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: I7a6e38bd151f823d37c07ee89a800489122cc209
Gerrit-Change-Number: 84080
Gerrit-PatchSet: 1
Gerrit-Owner: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: Dinesh Gehlot <digehlot(a)google.com>
Gerrit-Reviewer: Eric Lai <ericllai(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: Rishika Raj <rishikaraj(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Attention: Jayvik Desai <jayvik(a)google.com>
Gerrit-Attention: Rishika Raj <rishikaraj(a)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-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Comment-Date: Mon, 26 Aug 2024 09:18:38 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com>
Comment-In-Reply-To: Arthur Heymans <arthur(a)aheymans.xyz>
Attention is currently required from: Jakub Czapiga, Nico Huber.
Nicholas Sudsgaard has posted comments on this change by Nico Huber. ( https://review.coreboot.org/c/coreboot/+/84088?usp=email )
Change subject: b64_decode-test: Properly terminate strings before comparison
......................................................................
Patch Set 1:
(1 comment)
Patchset:
PS1:
> Right now this function is used only in the ChromeOS VPD code. […]
Acknowledged
--
To view, visit https://review.coreboot.org/c/coreboot/+/84088?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: Id1bd2c3ff06bc1d4e5aa21ddd0f1d5802540999d
Gerrit-Change-Number: 84088
Gerrit-PatchSet: 1
Gerrit-Owner: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Jakub Czapiga <czapiga(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Nicholas Sudsgaard <devel+coreboot(a)nsudsgaard.com>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Jakub Czapiga <czapiga(a)google.com>
Gerrit-Comment-Date: Mon, 26 Aug 2024 09:15:23 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Jakub Czapiga <czapiga(a)google.com>
Comment-In-Reply-To: Nicholas Sudsgaard <devel+coreboot(a)nsudsgaard.com>
Attention is currently required from: Nicholas Sudsgaard, Nico Huber.
Jakub Czapiga has posted comments on this change by Nico Huber. ( https://review.coreboot.org/c/coreboot/+/84088?usp=email )
Change subject: b64_decode-test: Properly terminate strings before comparison
......................................................................
Patch Set 1:
(1 comment)
Patchset:
PS1:
> Is there a reason why we can't add the null terminator in the `b64_decode()` function?
Right now this function is used only in the ChromeOS VPD code. It decodes protobuf Base64 fields, so it cannot append NULL terminator as it would return incorrect data.
--
To view, visit https://review.coreboot.org/c/coreboot/+/84088?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: Id1bd2c3ff06bc1d4e5aa21ddd0f1d5802540999d
Gerrit-Change-Number: 84088
Gerrit-PatchSet: 1
Gerrit-Owner: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Jakub Czapiga <czapiga(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Nicholas Sudsgaard <devel+coreboot(a)nsudsgaard.com>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Nicholas Sudsgaard <devel+coreboot(a)nsudsgaard.com>
Gerrit-Comment-Date: Mon, 26 Aug 2024 09:04:11 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Nicholas Sudsgaard <devel+coreboot(a)nsudsgaard.com>
Attention is currently required from: Arthur Heymans, Dinesh Gehlot, Jayvik Desai, Kapil Porwal, Nick Vaccaro, Rishika Raj.
Subrata Banik has posted comments on this change by Subrata Banik. ( https://review.coreboot.org/c/coreboot/+/84080?usp=email )
Change subject: soc/intel/adl: Prevent unconditional legacy COM ports initialization
......................................................................
Patch Set 1:
(2 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/84080/comment/25f2bdca_6ab169b0?us… :
PS1, Line 14: These COM ports are being activated unconditionally, which is
: undesirable for the Intel Alder Lake platform and causes traffic over
: the IO bus.
> I don't understand this. What code is generating those writes?
sorry if things are not cleared. As we are not using Legacy UART for ChromeOS device but looking at the Alder Lake code, we found that Legacy COMs are default enabled and we have observed some activity across those ports. Hence, pushed this CL to ensure legacy COMs are not getting enabled when the desired kconfig are not being selected
File src/soc/intel/alderlake/bootblock/pch.c:
https://review.coreboot.org/c/coreboot/+/84080/comment/a31a9292_6667e5ad?us… :
PS1, Line 101: LPC_IOE_LPT_EN | LPC_IOE_FDD_EN |
: LPC_IOE_LGE_200 | LPC_IOE_HGE_208 |
> Do you use a printer on the parallel port, a floppy disk drive, and a gaming port?
I believe this code still requires some additional cleaning, but I am not going to touch it because I am unsure of who the consumer of this code is. We have encountered an issue in which legacy COMs are enabled by default, which should only occur when DRIVERS_UART_8250IO is selected (not POR for CrOS). As a result, I simply removed those lines of code with the assumption that when someone selects DRIVERS_UART_8250IO, legacy COMs will be enabled.
--
To view, visit https://review.coreboot.org/c/coreboot/+/84080?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: I7a6e38bd151f823d37c07ee89a800489122cc209
Gerrit-Change-Number: 84080
Gerrit-PatchSet: 1
Gerrit-Owner: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: Dinesh Gehlot <digehlot(a)google.com>
Gerrit-Reviewer: Eric Lai <ericllai(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: Rishika Raj <rishikaraj(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Attention: Jayvik Desai <jayvik(a)google.com>
Gerrit-Attention: Rishika Raj <rishikaraj(a)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-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Comment-Date: Mon, 26 Aug 2024 08:49:44 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Arthur Heymans <arthur(a)aheymans.xyz>
Attention is currently required from: Dinesh Gehlot, Jayvik Desai, Kapil Porwal, Nick Vaccaro, Rishika Raj, Subrata Banik.
Arthur Heymans has posted comments on this change by Subrata Banik. ( https://review.coreboot.org/c/coreboot/+/84080?usp=email )
The change is no longer submittable: All-Comments-Resolved is unsatisfied now.
Change subject: soc/intel/adl: Prevent unconditional legacy COM ports initialization
......................................................................
Patch Set 1:
(2 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/84080/comment/1d26dff9_d89dcc3e?us… :
PS1, Line 14: These COM ports are being activated unconditionally, which is
: undesirable for the Intel Alder Lake platform and causes traffic over
: the IO bus.
I don't understand this. What code is generating those writes?
File src/soc/intel/alderlake/bootblock/pch.c:
https://review.coreboot.org/c/coreboot/+/84080/comment/775d9955_92f35183?us… :
PS1, Line 101: LPC_IOE_LPT_EN | LPC_IOE_FDD_EN |
: LPC_IOE_LGE_200 | LPC_IOE_HGE_208 |
Do you use a printer on the parallel port, a floppy disk drive, and a gaming port?
--
To view, visit https://review.coreboot.org/c/coreboot/+/84080?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: I7a6e38bd151f823d37c07ee89a800489122cc209
Gerrit-Change-Number: 84080
Gerrit-PatchSet: 1
Gerrit-Owner: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: Dinesh Gehlot <digehlot(a)google.com>
Gerrit-Reviewer: Eric Lai <ericllai(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: Rishika Raj <rishikaraj(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Attention: Jayvik Desai <jayvik(a)google.com>
Gerrit-Attention: Rishika Raj <rishikaraj(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: Nick Vaccaro <nvaccaro(a)chromium.org>
Gerrit-Comment-Date: Mon, 26 Aug 2024 08:30:11 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No