Attention is currently required from: Daniel Peng, Eric Lai, Kapil Porwal, Nick Vaccaro, Shawn Ku.
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80764?usp=email )
The change is no longer submittable: All-Comments-Resolved is unsatisfied now.
Change subject: mb/google/nissa/var/glassway: Initialize gpio table
......................................................................
Patch Set 1:
(5 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/80764/comment/f1f5dcce_e8df0f98 :
PS1, Line 7: gpio
GPIO
https://review.coreboot.org/c/coreboot/+/80764/comment/d633ef8a_e5b39bf4 :
PS1, Line 7: Initialize
Add
https://review.coreboot.org/c/coreboot/+/80764/comment/d465a10d_37f2e390 :
PS1, Line 9: Refer
Refer to
https://review.coreboot.org/c/coreboot/+/80764/comment/b8c29f8a_2fb07dd1 :
PS1, Line 9: devicetree
GPIO
File src/mainboard/google/brya/variants/glassway/gpio.c:
https://review.coreboot.org/c/coreboot/+/80764/comment/e9214378_941380cf :
PS1, Line 9: nivviks
Is that correct?
--
To view, visit https://review.coreboot.org/c/coreboot/+/80764?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: I0de743746160c6eb081cb9a061ac1703b01ba5b4
Gerrit-Change-Number: 80764
Gerrit-PatchSet: 1
Gerrit-Owner: Daniel Peng <daniel_peng(a)pegatron.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: Shawn Ku <shawnku(a)google.com>
Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Frank Chu <frank_chu(a)pegatron.corp-partner.google.com>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Eric Lai <ericllai(a)google.com>
Gerrit-Attention: Shawn Ku <shawnku(a)google.com>
Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Attention: Nick Vaccaro <nvaccaro(a)chromium.org>
Gerrit-Attention: Daniel Peng <daniel_peng(a)pegatron.corp-partner.google.com>
Gerrit-Comment-Date: Mon, 26 Feb 2024 12:29:22 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Daniel Peng, Eric Lai, Kapil Porwal, Nick Vaccaro, Shawn Ku.
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80763?usp=email )
Change subject: mb/google/nissa/var/glassway: Add memory config
......................................................................
Patch Set 2:
(1 comment)
Commit Message:
https://review.coreboot.org/c/coreboot/+/80763/comment/f715b8ea_af9dcfe6 :
PS2, Line 9: Follow glassway schematic to set memory configruation board straps.
You could mention, there are three straps.
--
To view, visit https://review.coreboot.org/c/coreboot/+/80763?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: Id85d590ed55997690e4c279ae83544b78fe72eec
Gerrit-Change-Number: 80763
Gerrit-PatchSet: 2
Gerrit-Owner: Daniel Peng <daniel_peng(a)pegatron.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: Shawn Ku <shawnku(a)google.com>
Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Frank Chu <frank_chu(a)pegatron.corp-partner.google.com>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Eric Lai <ericllai(a)google.com>
Gerrit-Attention: Shawn Ku <shawnku(a)google.com>
Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Attention: Nick Vaccaro <nvaccaro(a)chromium.org>
Gerrit-Attention: Daniel Peng <daniel_peng(a)pegatron.corp-partner.google.com>
Gerrit-Comment-Date: Mon, 26 Feb 2024 12:28:26 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Daniel Peng, Eric Lai, Kapil Porwal, Nick Vaccaro, Shawn Ku.
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80742?usp=email )
The change is no longer submittable: All-Comments-Resolved is unsatisfied now.
Change subject: mb/google/nissa/var/glassway: Initialize devicetree setting
......................................................................
Patch Set 3:
(3 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/80742/comment/15adbfc7_661e1413 :
PS3, Line 7: Initialize devicetree setting
Maybe:
> Add initial override devicetree
https://review.coreboot.org/c/coreboot/+/80742/comment/8b505841_ad10351a :
PS3, Line 10: schematic
Please document the revision.
https://review.coreboot.org/c/coreboot/+/80742/comment/19d04f3f_a6f5ecfb :
PS3, Line 14: TEST=None
Because you do not have a prototype yet?
--
To view, visit https://review.coreboot.org/c/coreboot/+/80742?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: Ibbb10a373bd5fa52a0833b81133517d2a088536b
Gerrit-Change-Number: 80742
Gerrit-PatchSet: 3
Gerrit-Owner: Daniel Peng <daniel_peng(a)pegatron.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: Shawn Ku <shawnku(a)google.com>
Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Frank Chu <frank_chu(a)pegatron.corp-partner.google.com>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Eric Lai <ericllai(a)google.com>
Gerrit-Attention: Shawn Ku <shawnku(a)google.com>
Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Attention: Nick Vaccaro <nvaccaro(a)chromium.org>
Gerrit-Attention: Daniel Peng <daniel_peng(a)pegatron.corp-partner.google.com>
Gerrit-Comment-Date: Mon, 26 Feb 2024 12:27:23 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Kapil Porwal.
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80688?usp=email )
Change subject: soc/intel/alderlake: Add kconfig for Twin Lake
......................................................................
Patch Set 5:
(1 comment)
File src/soc/intel/alderlake/Kconfig:
https://review.coreboot.org/c/coreboot/+/80688/comment/ffce2e99_66b2a0db :
PS5, Line 396: default "src/vendorcode/intel/fsp/fsp2_0/twinlake/" if SOC_INTEL_TWINLAKE && !FSP_USE_REPO
: default "src/vendorcode/intel/fsp/fsp2_0/alderlake_n/" if SOC_INTEL_ALDERLAKE_PCH_N && !FSP_USE_REPO
: default "src/vendorcode/intel/fsp/fsp2_0/raptorlake/" if SOC_INTEL_RAPTORLAKE && !FSP_USE_REPO
Should this be sorted somehow?
--
To view, visit https://review.coreboot.org/c/coreboot/+/80688?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: Ie4c5d137ee54512313344f853e7ca66d1fd25003
Gerrit-Change-Number: 80688
Gerrit-PatchSet: 5
Gerrit-Owner: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)chromium.org>
Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: V Sowmya <v.sowmya(a)intel.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Comment-Date: Mon, 26 Feb 2024 12:26:01 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Angel Pons, Arthur Heymans, Felix Held, Kyösti Mälkki, Michał Żygowski, Nico Huber.
Krystian Hebel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/77338?usp=email )
Change subject: device/pciexp_device.c: Fix setting Max Payload Size
......................................................................
Patch Set 20:
(6 comments)
File src/device/pciexp_device.c:
https://review.coreboot.org/c/coreboot/+/77338/comment/73575b57_1a45e8ce :
PS18, Line 596: unsigned int
> Though, in general, limiting the type also constraints
the compiler in the choices of optimizations it can make.
Yes, but those optimizations are often the cause of "compiler does what
I've told it to do instead of what I wanted" type of errors, especially
if there were arithmetic operations on the right side of the assignment.
That said, this isn't the case here, and as all other calls are using
`unsigned int`, keeping this consistent with existing code seems more
important, so I'm marking this and following as resolved.
https://review.coreboot.org/c/coreboot/+/77338/comment/1a128972_d5fa628a :
PS18, Line 610: unsigned int
> All `pci_find_capability(dev, PCI_CAP_ID_PCIE);` calls were assigning a value to unsigned int (with […]
Acknowledged
https://review.coreboot.org/c/coreboot/+/77338/comment/b2b64d1c_44a5d75d :
PS18, Line 623: unsigned int
> All `pci_find_capability(dev, PCI_CAP_ID_PCIE);` calls were assigning a value to unsigned int (with […]
Acknowledged
https://review.coreboot.org/c/coreboot/+/77338/comment/a9babf67_72424b94 :
PS18, Line 653: pciexp_dev_set_max_payload_size(parent, max_payload);
> Done
Done
https://review.coreboot.org/c/coreboot/+/77338/comment/1704640d_11bc7f4a :
PS18, Line 721: parent's
> From the other comment: […]
I see, I missed that `pci_scan_bus()` in line 758 (so before the loop)
calls `pciexp_scan_bus()` for lower levels _before_ it is propagated up
by `pciexp_tune_dev()`. Now it makes sense.
File src/device/pciexp_device.c:
https://review.coreboot.org/c/coreboot/+/77338/comment/8400eb05_a602a94d :
PS20, Line 603: devctl |= max_payload << 5;
Please either mask it with `PCI_EXP_DEVCTL_PAYLOAD` or add a comment that
this will never overflow to higher bits of `PCI_EXP_DEVCTL` register due
to how callers of this function obtain `max_payload`.
--
To view, visit https://review.coreboot.org/c/coreboot/+/77338?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: I24386dc208363b7d94fea46dec25c231a3968225
Gerrit-Change-Number: 77338
Gerrit-PatchSet: 20
Gerrit-Owner: Michał Żygowski <michal.zygowski(a)3mdeb.com>
Gerrit-Reviewer: Angel Pons <angel.pons(a)9elements.com>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Kyösti Mälkki <kyosti.malkki(a)gmail.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Krystian Hebel <krystian.hebel(a)3mdeb.com>
Gerrit-CC: Patrick Rudolph <patrick.rudolph(a)9elements.com>
Gerrit-CC: Stefan Reinauer <stefan.reinauer(a)coreboot.org>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Michał Żygowski <michal.zygowski(a)3mdeb.com>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Attention: Kyösti Mälkki <kyosti.malkki(a)gmail.com>
Gerrit-Attention: Angel Pons <angel.pons(a)9elements.com>
Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Comment-Date: Mon, 26 Feb 2024 12:13:22 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de>
Comment-In-Reply-To: Michał Żygowski <michal.zygowski(a)3mdeb.com>
Comment-In-Reply-To: Krystian Hebel <krystian.hebel(a)3mdeb.com>
Gerrit-MessageType: comment
Attention is currently required from: Nicholas Chin, Nicholas Sudsgaard, Nico Huber.
Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80695?usp=email )
Change subject: include/device: Merge enums from azalia_device.h and azalia.h
......................................................................
Patch Set 5:
(1 comment)
File src/include/device/azalia_device.h:
https://review.coreboot.org/c/coreboot/+/80695/comment/bcfdc8a8_42bb1eb4 :
PS5, Line 60: AZALIA_LOCATION_UNKNOWN
> I chose "unknown" for consistency with the others. I think you have a point here though. […]
The spec names it "N/A"; maybe `AZALIA_GEOLOC_NA`? The others are named `Unknown` in the spec, so I'd keep them as-is
--
To view, visit https://review.coreboot.org/c/coreboot/+/80695?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: Ie874b083a18963679981a9cd2b25d123890d628e
Gerrit-Change-Number: 80695
Gerrit-PatchSet: 5
Gerrit-Owner: Nicholas Sudsgaard <devel+coreboot(a)nsudsgaard.com>
Gerrit-Reviewer: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-Reviewer: Nicholas Chin <nic.c3.14(a)gmail.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Nicholas Sudsgaard <devel+coreboot(a)nsudsgaard.com>
Gerrit-Attention: Nicholas Chin <nic.c3.14(a)gmail.com>
Gerrit-Comment-Date: Mon, 26 Feb 2024 11:03:48 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de>
Comment-In-Reply-To: Nicholas Sudsgaard <devel+coreboot(a)nsudsgaard.com>
Gerrit-MessageType: comment
Attention is currently required from: Arthur Heymans, Nicholas Sudsgaard.
Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80720?usp=email )
Change subject: mb/clevo/tgl-u: hda_verbs: correct vendor value comments
......................................................................
Patch Set 2:
(3 comments)
Patchset:
PS1:
> Just my 2 cents, maybe the `/* vendor/linux: AZALIA_... */` comments are superfluous. […]
I still like to keep them
PS1:
> Ignore this. I accidentally marked this as resolved.
CB:80727 should go before this change as well; rebase that on top of CB:80740 first?
File src/mainboard/clevo/tgl-u/variants/l140mu/hda_verb.c:
https://review.coreboot.org/c/coreboot/+/80720/comment/f2455ef4_13098312 :
PS1, Line 54: vendor
> linux.
Done
--
To view, visit https://review.coreboot.org/c/coreboot/+/80720?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: I45b1d09d5a11b357ac2a20ef448ea642540cdc99
Gerrit-Change-Number: 80720
Gerrit-PatchSet: 2
Gerrit-Owner: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Nicholas Sudsgaard <devel+coreboot(a)nsudsgaard.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Nicholas Sudsgaard <devel+coreboot(a)nsudsgaard.com>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Comment-Date: Mon, 26 Feb 2024 10:58:18 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Nicholas Sudsgaard <devel+coreboot(a)nsudsgaard.com>
Gerrit-MessageType: comment