Attention is currently required from: 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:
(1 comment)
Commit Message:
https://review.coreboot.org/c/coreboot/+/76159/comment/d8d359a9_5cf88ff5 :
PS7, Line 7: to add support
> If you'd drop `to add support `, it would fit the line without […]
Done
--
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: 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: Fri, 30 Jun 2023 22:49:32 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de>
Gerrit-MessageType: comment
Attention is currently required from: 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:
(4 comments)
File src/mainboard/google/link/acpi/mainboard.asl:
https://review.coreboot.org/c/coreboot/+/76159/comment/21cdeb4d_1ffd4cdb :
PS7, Line 49: ){
> Missing space before `{`. Same below.
Done
https://review.coreboot.org/c/coreboot/+/76159/comment/cabb8cd3_8443b9d4 :
PS7, Line 51: Method (_STA, 0, NotSerialized) // _STA: Status
: {
: Return (0x0F)
: }
> This seems superfluous, 0xf should be the default w/o _STA. Same below.
_STA added here for clarity, since the peripheral devices have it too (for consistency with google/samus)
https://review.coreboot.org/c/coreboot/+/76159/comment/59b50462_e6f16888 :
PS7, Line 56: MMIO
> I don't see this being used, I guess that would be the Windows driver? […]
creating a _DSM with a UUID / functions seems overkill, since MMIO here is a replacement for what would otherwise be _CRS. As mentioned earlier, _CRS can't be used because the memory overlaps the PCIe device for the IGPU
https://review.coreboot.org/c/coreboot/+/76159/comment/6b37a0d8_19665bc0 :
PS7, Line 82: ISTP
> code was based off Samus, where that is present
Done
--
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: 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: Fri, 30 Jun 2023 22:49:07 +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: Martin Roth, Matt DeVillier, Nico Huber, Stefan Reinauer.
Hello Martin Roth, Matt DeVillier, Stefan Reinauer, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/76159?usp=email
to look at the new patch set (#8).
The following approvals got outdated and were removed:
Verified+1 by build bot (Jenkins)
Change subject: mb/google/link: rework TP/TS ACPI for new Windows I2C driver
......................................................................
mb/google/link: rework TP/TS ACPI for new Windows I2C driver
This is a brand new I2C driver that is designed specifically for the
Pixel 2013 chromebook (LINK). The GMBus interface on the IGPU is an
i2c-compatible interface, but AFAIK only Link has touch devices
attached in this way.
On Windows, the PCIe device for the IGP is owned by the Intel
proprietary driver, hence a separate ACPI device has to be added for
the I2C driver arbitrator to attach to. The MMIO method is used instead
of _CRS so that Windows does not try to assign ownership of the
resource to our device (even though we're using the MMIO registers at
the same time as the IGP driver).
Even though in theory 2 drivers accessing the same MMIO may cause
problems, in testing, there has been no issues with
sleep/wake/hibernate, updating/installing/uninstalling the IGP driver,
or changing display resolutions with the i2c driver attached.
The arbitrator is necessary as well, since even though there are
multiple i2c buses, the MMIO registers are shared. Hence a shared lock
is required for i2c access across the buses.
The original Sleep Button devices are preserved for Linux due to the
completely custom and non-standard implementation of the Windows driver
in order to work around the non-standard nature of Link's hardware.
Change-Id: If7ee05d15bc17d335cf8c1a8e80bea62800de475
Signed-off-by: CoolStar <coolstarorganization(a)gmail.com>
---
M src/mainboard/google/link/acpi/mainboard.asl
M src/mainboard/google/link/dsdt.asl
M src/mainboard/google/link/onboard.h
3 files changed, 114 insertions(+), 7 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/59/76159/8
--
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: 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-MessageType: newpatchset
Attention is currently required from: Martin L Roth, Raul Rangel, Rob Barnes.
Karthik Ramasubramanian has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/76188?usp=email )
Change subject: util/apcb: Add apcb edit tool for phoenix
......................................................................
Patch Set 2:
(3 comments)
File util/apcb/apcb_v3a_edit.py:
https://review.coreboot.org/c/coreboot/+/76188/comment/d18e6746_b6e402fb :
PS2, Line 18: spd_block_header = namedtuple(
: 'spd_block_header', 'Type, Length, Key, Reserved')
Interesting that there is no block ID in the header. I thought there was block ID. This will help to ensure that we are injecting the block at the right spot.
https://review.coreboot.org/c/coreboot/+/76188/comment/2d3a590b_0f31c9bf :
PS2, Line 96: if b in ZERO_BLOCKS: continue
The code asserts that it is zero block in our SPD. Should it also assert that it is zero block in APCB's SPD too or does APCB not contain zero blocks?
https://review.coreboot.org/c/coreboot/+/76188/comment/e4293951_27caf40b :
PS2, Line 120: assert len(apcb) == apcb
Just curious whether we need to assert every time a block is injected. I see that you are asserting the final APCB length after injecting all SPDs in line 130.
--
To view, visit https://review.coreboot.org/c/coreboot/+/76188?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: Ifb50287de77138170714a702ab87d56427aacfef
Gerrit-Change-Number: 76188
Gerrit-PatchSet: 2
Gerrit-Owner: Rob Barnes <robbarnes(a)google.com>
Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com>
Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Raul Rangel <rrangel(a)chromium.org>
Gerrit-Attention: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Attention: Rob Barnes <robbarnes(a)google.com>
Gerrit-Comment-Date: Fri, 30 Jun 2023 22:19:07 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Andrey Petrov, Angel Pons, Arthur Heymans, Arthur Heymans, Felix Held, Fred Reitberger, Jason Glenesk, Jeff Daly, Lance Zhao, Mariusz Szafrański, Marshall Dawson, Matt DeVillier, Raul Rangel, Sean Rhodes, Sumeet R Pawnikar, Suresh Bellampalli, Tim Wawrzynczak, Tim Wawrzynczak, Vanessa Eusebio.
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/64303?usp=email )
Change subject: acpi/gnvs.c: Drop unused pointer to the cbmem console
......................................................................
Patch Set 2:
(1 comment)
Patchset:
PS1:
> Status? If still active can you rebase on top of https://review.coreboot. […]
Done
--
To view, visit https://review.coreboot.org/c/coreboot/+/64303?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: I7e2018dbccead15fcd84e34df8207120d3a0c57c
Gerrit-Change-Number: 64303
Gerrit-PatchSet: 2
Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Andrey Petrov <andrey.petrov(a)gmail.com>
Gerrit-Reviewer: Angel Pons <th3fanbus(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: Jeff Daly <jeffd(a)silicom-usa.com>
Gerrit-Reviewer: Kyösti Mälkki <kyosti.malkki(a)gmail.com>
Gerrit-Reviewer: Lance Zhao <lance.zhao(a)gmail.com>
Gerrit-Reviewer: Mariusz Szafrański <mariuszx(a)akumat.pl>
Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd(a)gmail.com>
Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com>
Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org>
Gerrit-Reviewer: Sean Rhodes <sean(a)starlabs.systems>
Gerrit-Reviewer: Sumeet R Pawnikar <sumeet.r.pawnikar(a)intel.com>
Gerrit-Reviewer: Suresh Bellampalli <suresh.bellampalli(a)intel.com>
Gerrit-Reviewer: Tim Wawrzynczak <inforichland(a)gmail.com>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)google.com>
Gerrit-Reviewer: Vanessa Eusebio <vanessa.f.eusebio(a)intel.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Arthur Heymans <arthur.heymans(a)9elements.com>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Arthur Heymans <arthur.heymans(a)9elements.com>
Gerrit-Attention: Raul Rangel <rrangel(a)chromium.org>
Gerrit-Attention: Jeff Daly <jeffd(a)silicom-usa.com>
Gerrit-Attention: Mariusz Szafrański <mariuszx(a)akumat.pl>
Gerrit-Attention: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com>
Gerrit-Attention: Sumeet R Pawnikar <sumeet.r.pawnikar(a)intel.com>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)google.com>
Gerrit-Attention: Andrey Petrov <andrey.petrov(a)gmail.com>
Gerrit-Attention: Lance Zhao <lance.zhao(a)gmail.com>
Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com>
Gerrit-Attention: Sean Rhodes <sean(a)starlabs.systems>
Gerrit-Attention: Marshall Dawson <marshalldawson3rd(a)gmail.com>
Gerrit-Attention: Tim Wawrzynczak <inforichland(a)gmail.com>
Gerrit-Attention: Suresh Bellampalli <suresh.bellampalli(a)intel.com>
Gerrit-Attention: Vanessa Eusebio <vanessa.f.eusebio(a)intel.com>
Gerrit-Attention: Fred Reitberger <reitbergerfred(a)gmail.com>
Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Comment-Date: Fri, 30 Jun 2023 22:13:02 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Jon Murphy <jpmurphy(a)google.com>
Gerrit-MessageType: comment
Attention is currently required from: Martin L Roth, Raul Rangel, Rob Barnes.
Karthik Ramasubramanian has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/76188?usp=email )
Change subject: util/apcb: Add apcb edit tool for phoenix
......................................................................
Patch Set 2:
(1 comment)
File util/apcb/apcb_v3a_edit.py:
https://review.coreboot.org/c/coreboot/+/76188/comment/ed53e5aa_7a0e98c1 :
PS2, Line 45: 16
APCB_CHECKSUM_OFFSET instead of magic 16 and APCB_CHECKSUM_OFFSET + 1 for magic 17
--
To view, visit https://review.coreboot.org/c/coreboot/+/76188?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: Ifb50287de77138170714a702ab87d56427aacfef
Gerrit-Change-Number: 76188
Gerrit-PatchSet: 2
Gerrit-Owner: Rob Barnes <robbarnes(a)google.com>
Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com>
Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Raul Rangel <rrangel(a)chromium.org>
Gerrit-Attention: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Attention: Rob Barnes <robbarnes(a)google.com>
Gerrit-Comment-Date: Fri, 30 Jun 2023 21:49:54 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment