Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46470 )
Change subject: soc/intel/common/acpi: rename LPID to PEPD
......................................................................
Patch Set 4:
(2 comments)
https://review.coreboot.org/c/coreboot/+/46470/4//COMMIT_MSG
Commit Message:
https://review.coreboot.org/c/coreboot/+/46470/4//COMMIT_MSG@7
PS4, Line 7: PEPD
Can we instead rename this to PMCD? PEPD is something very Windows-specific and I see it being used in Intel documents specific to Windows only.
On the other hand, both Microsoft documentation(https://docs.microsoft.com/en-us/windows-hardware/drivers/bri… and Linux kernel(https://github.com/torvalds/linux/blob/master/drivers/acpi/sleep.c#L… refer to this class of devices as "System Power Management Controller".
I think it would be good to use the same in coreboot as well? i.e. PMCD - Power Management Controller Device.
https://review.coreboot.org/c/coreboot/+/46470/4/src/soc/intel/common/acpi/…
File src/soc/intel/common/acpi/lpit.asl:
https://review.coreboot.org/c/coreboot/+/46470/4/src/soc/intel/common/acpi/…
PS4, Line 3: PEPD_DSM_ARG2_ENUM_FUNCTIONS
I think it would be good to have LPI(Low Power Idle) in the name to make it clear that it is specific to Low Power S0 Idle. I think we can drop ARG2 since that is not really helpful.
PMCD_DSM_LPI_ENUM_FUNCTIONS
PMCD_DSM_LPI_GET_DEVICE_CONSTRAINTS
and so on
--
To view, visit https://review.coreboot.org/c/coreboot/+/46470
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I1caa009a3946b1c55da8afbae058cafe98940c6d
Gerrit-Change-Number: 46470
Gerrit-PatchSet: 4
Gerrit-Owner: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com>
Gerrit-Reviewer: Karthikeyan Ramasubramanian <kramasub(a)chromium.org>
Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)gmail.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-Comment-Date: Tue, 03 Nov 2020 21:48:54 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47138 )
Change subject: soc/intel/common/acpi: fix Windows crash on S0ix-enabled boards
......................................................................
Patch Set 3:
(4 comments)
https://review.coreboot.org/c/coreboot/+/47138/1//COMMIT_MSG
Commit Message:
https://review.coreboot.org/c/coreboot/+/47138/1//COMMIT_MSG@9
PS1, Line 9: Windows
> I will test more and add the tested versions
Done
https://review.coreboot.org/c/coreboot/+/47138/1//COMMIT_MSG@22
PS1, Line 22: Windows
> It’d be great if you specified the version, you tested with.
Done
https://review.coreboot.org/c/coreboot/+/47138/3//COMMIT_MSG
Commit Message:
https://review.coreboot.org/c/coreboot/+/47138/3//COMMIT_MSG@7
PS3, Line 7: fix
Rather `prevent` because it's not broken atm.?
https://review.coreboot.org/c/coreboot/+/47138/1/src/soc/intel/common/acpi/…
File src/soc/intel/common/acpi/lpit.asl:
https://review.coreboot.org/c/coreboot/+/47138/1/src/soc/intel/common/acpi/…
PS1, Line 26: debugging
> Also see comment from Tim at line 38 here: https://review.coreboot. […]
That doesn't explain why you want to say that here in a comment. The
truth would be something along "[...] device constraints for low power
states (may be used for debugging [in] Linux)." IMO, too much imple-
mentation specific information. So why not leave it to what the spec
intended?
--
To view, visit https://review.coreboot.org/c/coreboot/+/47138
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Icd08cbcb1dfcb8cbb23f4f4c902bf8c367c8e3ac
Gerrit-Change-Number: 47138
Gerrit-PatchSet: 3
Gerrit-Owner: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Christian Walter <christian.walter(a)9elements.com>
Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com>
Gerrit-Reviewer: Karthikeyan Ramasubramanian <kramasub(a)chromium.org>
Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)gmail.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Patrick Rudolph <patrick.rudolph(a)9elements.com>
Gerrit-Comment-Date: Tue, 03 Nov 2020 21:02:06 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de>
Comment-In-Reply-To: Paul Menzel <paulepanter(a)users.sourceforge.net>
Comment-In-Reply-To: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-MessageType: comment
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45208 )
Change subject: console: Allow VPD to disable an otherwise enabled coreboot console
......................................................................
Patch Set 4:
(1 comment)
https://review.coreboot.org/c/coreboot/+/45208/4//COMMIT_MSG
Commit Message:
https://review.coreboot.org/c/coreboot/+/45208/4//COMMIT_MSG@18
PS4, Line 18: entry is added.
> No, that's not true. […]
This demand to customize binaries pops up rather often lately. I was
wondering if we can find a generic and efficient solution. From the
top of my head: A section per stage reserved for a handful of key+value
pairs and use that as a low-priority backend for get_option() (or a new,
saner API ^^). Ofc, systems where the bootblock is already too big
wouldn't benefit. On the positive side, the data would be implicitly
accessible just like the code (i.e. memory mapped or automatically
loaded into SRAM/CAR/DRAM).
--
To view, visit https://review.coreboot.org/c/coreboot/+/45208
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I4f1f5c45e5ea889176d04e0db438ca2aa7c536ee
Gerrit-Change-Number: 45208
Gerrit-PatchSet: 4
Gerrit-Owner: Patrick Georgi <pgeorgi(a)google.com>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Dossym Nurmukhanov <dossym(a)google.com>
Gerrit-Reviewer: Greg Edelston <gredelston(a)google.com>
Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Nico Huber <nico.h(a)gmx.de>
Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-CC: Stefan Reinauer <stefan.reinauer(a)coreboot.org>
Gerrit-Comment-Date: Tue, 03 Nov 2020 20:21:04 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Julius Werner <jwerner(a)chromium.org>
Gerrit-MessageType: comment
Karthik Ramasubramanian has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44588 )
Change subject: mb/google/dedede/var/boten: Add LTE power on/off sequence
......................................................................
Patch Set 10:
(1 comment)
https://review.coreboot.org/c/coreboot/+/44588/10/src/mainboard/google/dede…
File src/mainboard/google/dedede/variants/boten/gpio.c:
https://review.coreboot.org/c/coreboot/+/44588/10/src/mainboard/google/dede…
PS10, Line 11: 1
> Dear Furquan and Karthik, if set its value equal 0, LTE module can not work, we have do the experime […]
Peichao
1) ACPI methods are going to do the same thing as what these GPIO configurations are doing.
2) GPIOs will toggle to low once during the PO because of CSE FW Sync and then go back to high. This is not expected by the LTE module.
3) Configuring both FCPO and RESET as high here will not help to meet the LTE module's other power sequence requirements.
--
To view, visit https://review.coreboot.org/c/coreboot/+/44588
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ic6d5d21ce5267f147b332a4c9b01a29b3b8ccfb8
Gerrit-Change-Number: 44588
Gerrit-PatchSet: 10
Gerrit-Owner: Karthik Ramasubramanian <kramasub(a)google.com>
Gerrit-Reviewer: Alec Wang <alec.wang(a)lcfc.corp-partner.google.com>
Gerrit-Reviewer: Furquan Shaikh <furquan.m.shaikh(a)gmail.com>
Gerrit-Reviewer: Henry Sun <henrysun(a)google.com>
Gerrit-Reviewer: Marco Chen <marcochen(a)google.com>
Gerrit-Reviewer: Martin Roth <martinroth(a)google.com>
Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com>
Gerrit-Reviewer: Peichao Wang <pwang12(a)lenovo.corp-partner.google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Furquan Shaikh <furquan(a)google.com>
Gerrit-Comment-Date: Tue, 03 Nov 2020 19:16:30 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Furquan Shaikh <furquan(a)google.com>
Comment-In-Reply-To: Peichao Wang <pwang12(a)lenovo.corp-partner.google.com>
Comment-In-Reply-To: Karthik Ramasubramanian <kramasub(a)google.com>
Gerrit-MessageType: comment