Attention is currently required from: Jakub Czapiga, Subrata Banik, Tarun Tuli.
Kapil Porwal has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/76212?usp=email )
Change subject: mb/google/rex/var/ovis: Enable LAN1
......................................................................
Patch Set 2:
(1 comment)
File src/mainboard/google/rex/variants/ovis/gpio.c:
https://review.coreboot.org/c/coreboot/+/76212/comment/ecca72e5_351557b3 :
PS2, Line 405: GPP_D02
It is being set to 0 at both the places (bootblock and romstage). Is this intended?
--
To view, visit https://review.coreboot.org/c/coreboot/+/76212?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ifb67cb8e6fc03e3ff14b1b3d8382322fd0b3aeff
Gerrit-Change-Number: 76212
Gerrit-PatchSet: 2
Gerrit-Owner: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: Jakub Czapiga <jacz(a)semihalf.com>
Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Reviewer: Tarun Tuli <taruntuli(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Tarun Tuli <taruntuli(a)google.com>
Gerrit-Attention: Subrata Banik <subratabanik(a)google.com>
Gerrit-Attention: Jakub Czapiga <jacz(a)semihalf.com>
Gerrit-Comment-Date: Sat, 01 Jul 2023 17:43:20 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Felix Singer, Krystian Hebel, Michał Kopeć, Michał Żygowski.
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/69825?usp=email )
Change subject: mb/msi/ms7d25: Configure ASPM and Clock PM based on Kconfig
......................................................................
Patch Set 4: Code-Review+1
(3 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/69825/comment/c4bcedef_329783f4 :
PS3, Line 11: to achieve more deterministic and higher performance of PCIe devices
> Acknowledged
Thank you for the explanation.
Commit Message:
https://review.coreboot.org/c/coreboot/+/69825/comment/3c2bd085_f868fc15 :
PS4, Line 9: Kconfig
If you do another round, maybe mention the two options:
```
config PCIEXP_ASPM
prompt "Enable PCIe ASPM"
bool
default n
help
Detect and enable ASPM (Active State Power Management) on PCIe links.
config PCIEXP_CLK_PM
prompt "Enable PCIe Clock Power Management"
bool
default n
help
Detect and enable Clock Power Management on PCIe.
```
Patchset:
PS4:
Ok, I guess I was confused that Kconfig options were not honored from the beginning. Is that a design problem in coreboot, or was it just unconditionally enabled, because the first devices were “devices with batteries”, so the code was optimized for powersaving?
--
To view, visit https://review.coreboot.org/c/coreboot/+/69825?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I6d9d11016bed89dcfee6909d0d3e3e2e56237a2f
Gerrit-Change-Number: 69825
Gerrit-PatchSet: 4
Gerrit-Owner: Michał Żygowski <michal.zygowski(a)3mdeb.com>
Gerrit-Reviewer: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-Reviewer: Krystian Hebel <krystian.hebel(a)3mdeb.com>
Gerrit-Reviewer: Michał Kopeć <michal.kopec(a)3mdeb.com>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-Attention: Michał Żygowski <michal.zygowski(a)3mdeb.com>
Gerrit-Attention: Michał Kopeć <michal.kopec(a)3mdeb.com>
Gerrit-Attention: Krystian Hebel <krystian.hebel(a)3mdeb.com>
Gerrit-Comment-Date: Sat, 01 Jul 2023 12:18:59 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Comment-In-Reply-To: Michał Żygowski <michal.zygowski(a)3mdeb.com>
Comment-In-Reply-To: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-MessageType: comment
Attention is currently required from: Felix Singer, Krystian Hebel, Michał Kopeć, Michał Żygowski.
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/69826?usp=email )
Change subject: mb/msi/ms7d25: Disable DMI ASPM
......................................................................
Patch Set 4: Code-Review+1
(1 comment)
File src/mainboard/msi/ms7d25/mainboard.c:
https://review.coreboot.org/c/coreboot/+/69826/comment/25a3f0b9_49a59e62 :
PS3, Line 95: params->PchLegacyIoLowLatency = 1;
> Basically it disables clock gating of some blocks in the ITSSPRC register so their latencies are low […]
Done
--
To view, visit https://review.coreboot.org/c/coreboot/+/69826?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I60af9d2ab2913db449059e1e007999fa2f307f5d
Gerrit-Change-Number: 69826
Gerrit-PatchSet: 4
Gerrit-Owner: Michał Żygowski <michal.zygowski(a)3mdeb.com>
Gerrit-Reviewer: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-Reviewer: Krystian Hebel <krystian.hebel(a)3mdeb.com>
Gerrit-Reviewer: Michał Kopeć <michal.kopec(a)3mdeb.com>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-Attention: Michał Żygowski <michal.zygowski(a)3mdeb.com>
Gerrit-Attention: Michał Kopeć <michal.kopec(a)3mdeb.com>
Gerrit-Attention: Krystian Hebel <krystian.hebel(a)3mdeb.com>
Gerrit-Comment-Date: Sat, 01 Jul 2023 12:14:11 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Comment-In-Reply-To: Michał Żygowski <michal.zygowski(a)3mdeb.com>
Comment-In-Reply-To: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-MessageType: comment
Attention is currently required from: Felix Singer, Martin Roth, Matt DeVillier, Nico Huber, Stefan Reinauer.
CoolStar has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/76159?usp=email )
Change subject: mb/google/link: rework TP/TS ACPI for new Windows I2C driver
......................................................................
Patch Set 8:
(2 comments)
File src/mainboard/google/link/acpi/mainboard.asl:
https://review.coreboot.org/c/coreboot/+/76159/comment/fa09c21d_a0f99a85 :
PS7, Line 50: LINK
> Thanks for the link! looks quite generic. […]
the .inf is where the binding is done, yes
https://review.coreboot.org/c/coreboot/+/76159/comment/10e60ff5_24e364bf :
PS7, Line 82: ISTP
> A downstream Linux driver maybe? I couldn't find any reference upstream.
Maybe something from Chrome OS? Didn't see anything for upstream Linux either
--
To view, visit https://review.coreboot.org/c/coreboot/+/76159?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: If7ee05d15bc17d335cf8c1a8e80bea62800de475
Gerrit-Change-Number: 76159
Gerrit-PatchSet: 8
Gerrit-Owner: CoolStar <coolstarorganization(a)gmail.com>
Gerrit-Reviewer: Martin Roth <martinroth(a)google.com>
Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)gmail.com>
Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-CC: Nico Huber <nico.h(a)gmx.de>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-Attention: Martin Roth <martinroth(a)google.com>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Matt DeVillier <matt.devillier(a)gmail.com>
Gerrit-Attention: Stefan Reinauer <stefan.reinauer(a)coreboot.org>
Gerrit-Comment-Date: Sat, 01 Jul 2023 10:40:57 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de>
Comment-In-Reply-To: CoolStar <coolstarorganization(a)gmail.com>
Gerrit-MessageType: comment
Attention is currently required from: CoolStar, Felix Singer, Martin Roth, Matt DeVillier, Stefan Reinauer.
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/76159?usp=email )
Change subject: mb/google/link: rework TP/TS ACPI for new Windows I2C driver
......................................................................
Patch Set 8:
(6 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/76159/comment/fc478fdb_dc16e065 :
PS8, Line 30: The original Sleep Button devices are preserved for Linux due to the
Why can't we do what Samus does in this case?
File src/mainboard/google/link/acpi/mainboard.asl:
https://review.coreboot.org/c/coreboot/+/76159/comment/0238f036_eeee5ce1 :
PS1, Line 14: // Trackpad Wake is GPIO12
: Name(_PRW, Package() { BOARD_TRACKPAD_WAKE_GPIO, 0x03 } )
:
> Okay, well then I didn't say anything 😊
I was about to say: it's moved, not removed. But then realized we end up
with two device nodes for the same device. So why not move everything?
File src/mainboard/google/link/acpi/mainboard.asl:
https://review.coreboot.org/c/coreboot/+/76159/comment/f9f0fb70_004646e5 :
PS7, Line 50: LINK
> I2C bus driver code: https://github.com/coolstar/gmbusi2c […]
Thanks for the link! looks quite generic.
I just remembered, we have `BOOTxxxx` registered. I guess we should use
that? cf. `enum coreboot_acpi_ids` in `src/include/acpi/acpi.h`.
Only for my curiosity: I guess the binding is done in the `.inf` with
```
%GmBusI2C.DeviceDescArb%=GmBusI2C_Device, ACPI\LINK0000
```
correct? Do you know where this is documented?
https://review.coreboot.org/c/coreboot/+/76159/comment/f153cbe5_3ae00a91 :
PS7, Line 51: Method (_STA, 0, NotSerialized) // _STA: Status
: {
: Return (0x0F)
: }
> _STA added here for clarity, since the peripheral devices have it too (for consistency with google/s […]
Well, for Samus it's actual methods with code. Here it's just dead code
constantly returning the default value and (at least for myself) distracting
the reader.
https://review.coreboot.org/c/coreboot/+/76159/comment/0208653a_07e710ea :
PS7, Line 56: MMIO
> creating a _DSM with a UUID / functions seems overkill, since MMIO here is a replacement for what wo […]
It would be the canonical way, though. I would at least add a comment,
that it's a custom OS interface (for the casual reader it would look
like dead code otherwise).
https://review.coreboot.org/c/coreboot/+/76159/comment/c96c1461_37426011 :
PS7, Line 82: ISTP
> Done
A downstream Linux driver maybe? I couldn't find any reference upstream.
--
To view, visit https://review.coreboot.org/c/coreboot/+/76159?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: If7ee05d15bc17d335cf8c1a8e80bea62800de475
Gerrit-Change-Number: 76159
Gerrit-PatchSet: 8
Gerrit-Owner: CoolStar <coolstarorganization(a)gmail.com>
Gerrit-Reviewer: Martin Roth <martinroth(a)google.com>
Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)gmail.com>
Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-CC: Nico Huber <nico.h(a)gmx.de>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-Attention: Martin Roth <martinroth(a)google.com>
Gerrit-Attention: Matt DeVillier <matt.devillier(a)gmail.com>
Gerrit-Attention: Stefan Reinauer <stefan.reinauer(a)coreboot.org>
Gerrit-Attention: CoolStar <coolstarorganization(a)gmail.com>
Gerrit-Comment-Date: Sat, 01 Jul 2023 10:13:24 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de>
Comment-In-Reply-To: Matt DeVillier <matt.devillier(a)gmail.com>
Comment-In-Reply-To: CoolStar <coolstarorganization(a)gmail.com>
Gerrit-MessageType: comment
Attention is currently required from: Jakub Czapiga, Martin L Roth, Nico Huber.
Elyes Haouas has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/70524?usp=email )
Change subject: util/sconfig: Improve usage and long options
......................................................................
Patch Set 5: Code-Review+2
--
To view, visit https://review.coreboot.org/c/coreboot/+/70524?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: If5252de63692c5e43bfbde4d7d93e1d7a84e8dff
Gerrit-Change-Number: 70524
Gerrit-PatchSet: 5
Gerrit-Owner: Jakub Czapiga <jacz(a)semihalf.com>
Gerrit-Reviewer: Elyes Haouas <ehaouas(a)noos.fr>
Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Jan Dabros <jsd(a)semihalf.com>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Attention: Jakub Czapiga <jacz(a)semihalf.com>
Gerrit-Comment-Date: Sat, 01 Jul 2023 09:34:51 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: Felix Singer, Nicholas Chin, Riku Viitanen.
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/74906?usp=email )
Change subject: mb/hp: Add new port for compaq_8300_elite_usdt
......................................................................
Patch Set 10: Code-Review+2
--
To view, visit https://review.coreboot.org/c/coreboot/+/74906?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I2dc31778c2aa1987d5acdf355973a203dd0bb3a3
Gerrit-Change-Number: 74906
Gerrit-PatchSet: 10
Gerrit-Owner: Riku Viitanen <riku.viitanen(a)protonmail.com>
Gerrit-Reviewer: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-Reviewer: Kyösti Mälkki <kyosti.malkki(a)gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Nicholas Chin <nic.c3.14(a)gmail.com>
Gerrit-Attention: Riku Viitanen <riku.viitanen(a)protonmail.com>
Gerrit-Attention: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-Attention: Nicholas Chin <nic.c3.14(a)gmail.com>
Gerrit-Comment-Date: Sat, 01 Jul 2023 07:54:40 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment