coreboot-gerrit November 2020

coreboot-gerrit@coreboot.org

1 participants
3243 discussions

Change in coreboot[master]: soc/intel/common/acpi: rename LPID to PEPD
by Michael Niewöhner (Code Review) 03 Nov '20

03 Nov '20

Hello build bot (Jenkins), Nico Huber, Furquan Shaikh, Matt DeVillier, Tim Wawrzynczak, Subrata Banik, Patrick Rudolph, Karthikeyan Ramasubramanian, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/46470 to look at the new patch set (#4). Change subject: soc/intel/common/acpi: rename LPID to PEPD ...................................................................... soc/intel/common/acpi: rename LPID to PEPD Rename LPID to PEPD for consistency. PEPD means "Power Engine Plug-In Device" and is the name, Intel and vendors usually use, so let's comply. Change-Id: I1caa009a3946b1c55da8afbae058cafe98940c6d Signed-off-by: Michael Niewöhner <foss(a)mniewoehner.de> --- M src/mainboard/google/drallion/variants/drallion/include/variant/acpi/mainboard.asl M src/mainboard/google/sarien/variants/arcada/include/variant/acpi/mainboard.asl M src/mainboard/google/sarien/variants/sarien/include/variant/acpi/mainboard.asl M src/mainboard/google/volteer/mainboard.asl M src/soc/intel/common/acpi/lpit.asl 5 files changed, 20 insertions(+), 20 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/70/46470/4 -- 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: Angel Pons <th3fanbus(a)gmail.com> Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-MessageType: newpatchset

1 0

Change in coreboot[master]: driver/usb/acpi: Add power resources for devices on USB ports
by Furquan Shaikh (Code Review) 03 Nov '20

03 Nov '20

1 0

Change in coreboot[master]: driver/usb/acpi: Add power resources for devices on USB ports
by Furquan Shaikh (Code Review) 03 Nov '20

03 Nov '20

Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46713 ) Change subject: driver/usb/acpi: Add power resources for devices on USB ports ...................................................................... Patch Set 2: (1 comment) > Patch Set 2: > > (1 comment) > > Hmm, this makes me want to attack the power resource problem again... > With the devtree alias patches in, expressing power resource dependencies is even cleaner, e.g.,: > > ``` > chip drivers/generic/power_resource/ > register "on" = "{{GPIO_L(GPP_A1), 50}, > {GPIO_L(GPIO_B2), 1},}" > register "off" = "{{GPIO_L(GPP_B2), 1}, > {GPIO(GPP_A1), 10},}" > # or even the current interface in generic/2c with the stop_gpio, reset_gpio, etc. > # but I kind of like this explicit sequencing you get here. > device generic 0 alias i2c1_power_res on end > end > chip drivers/i2c/generic > use i2c1_power_res as power_resource; > device i2c 1f on end > end > chip drivers/spi/generic > use i2c1_power_res as power_resource; > device spi 0 on end > end > ``` > > Each "resource" in the 'on' and 'off' list (i.e., GPIOs) can emit enable/disable methods that use reference counting to keep track of when it's safe to actually assert/deassert the pins. > > WDYT? If you don't want to tackle that right now, factoring out the power resource fields into a common `struct power_resource_config` or similar would be better than copy-pasting these fields around into different drivers. We might want to eventually do something like you recommended i.e. having a separate power resource device especially if multiple devices have to share power resource. Last week you mentioned that this might probably be a use case for the camera device. One thing that we will probably have to think about some more is -- if a driver wants to expose the GPIOs in _CRS in addition to generating the power resource, then it will require both on/off as well as reset/enable GPIOs to be provided by mainboard in the devicetree entry. It might be helpful to write these thoughts in a doc/bug so that we can capture all scenarios. About adding a power_resource structure - I think that can be a quick way forward right now especially if Karthik is looking to unblock the mainboard CLs. However, if you plan to refactor and add a new power_resource driver, do you want to wait until then to decide what the structure should really look like? https://review.coreboot.org/c/coreboot/+/46713/2/src/drivers/usb/acpi/chip.h File src/drivers/usb/acpi/chip.h: https://review.coreboot.org/c/coreboot/+/46713/2/src/drivers/usb/acpi/chip.… PS2, Line 47: /* Disable reset and enable GPIO export in _CRS */ : bool disable_gpio_export_in_crs; I think we should skip this. This was added in I2C driver for a very specific use case and it is being copied over without real usage. I think we should not expose the GPIOs in CRS if they are being used specifically for PowerResource. Exposing them in _CRS has resulted in problem in some platforms where the kernel driver tries to reset the device again after ACPI has done the required work. I will raise a bug to clean this up for I2C too. For this driver, let's not add this unless there is an actual use case. -- To view, visit https://review.coreboot.org/c/coreboot/+/46713 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Icc1aebfb9e3e646a7f608f0cd391079fd30dd1c0 Gerrit-Change-Number: 46713 Gerrit-PatchSet: 2 Gerrit-Owner: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Tue, 03 Nov 2020 01:00:42 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in coreboot[master]: soc/intel/block/acpi: add code for CPPC entries generation
by Michael Niewöhner (Code Review) 03 Nov '20

03 Nov '20

Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45535 ) Change subject: soc/intel/block/acpi: add code for CPPC entries generation ...................................................................... Patch Set 32: (1 comment) https://review.coreboot.org/c/coreboot/+/45535/31/src/soc/intel/common/bloc… File src/soc/intel/common/block/acpi/acpi.c: https://review.coreboot.org/c/coreboot/+/45535/31/src/soc/intel/common/bloc… PS31, Line 6: #include <bootstate.h> > arch/cpu. […] Done -- To view, visit https://review.coreboot.org/c/coreboot/+/45535 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I1fcc2d0d7c6b6f35f8dd011f55dab8469be99d47 Gerrit-Change-Number: 45535 Gerrit-PatchSet: 32 Gerrit-Owner: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-Reviewer: Elyes HAOUAS Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)gmail.com> Gerrit-Reviewer: Matt Delco <delco(a)chromium.org> 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: Furquan Shaikh <furquan(a)google.com> Gerrit-Comment-Date: Tue, 03 Nov 2020 00:57:42 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: soc/intel/block/acpi: add code for CPPC entries generation
by Michael Niewöhner (Code Review) 03 Nov '20

03 Nov '20

Hello Elyes HAOUAS, build bot (Jenkins), Matt Delco, Nico Huber, Matt DeVillier, Tim Wawrzynczak, Paul Menzel, Subrata Banik, Patrick Rudolph, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/45535 to look at the new patch set (#32). Change subject: soc/intel/block/acpi: add code for CPPC entries generation ...................................................................... soc/intel/block/acpi: add code for CPPC entries generation Copy the code for CPPC entries generation, needed for Intel SpeedShift, from SKL to common ACPI code. This way all SoCs using the common code get the CPPC entries added. SKL is going to use common ACPI code, too, in the future, so this code duplication will vanish soon. Test: dumped SSDT from Clevo L140CU and checked decompiled version Change-Id: I1fcc2d0d7c6b6f35f8dd011f55dab8469be99d47 Signed-off-by: Michael Niewöhner <foss(a)mniewoehner.de> --- M src/soc/intel/common/block/acpi/acpi.c 1 file changed, 22 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/35/45535/32 -- To view, visit https://review.coreboot.org/c/coreboot/+/45535 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I1fcc2d0d7c6b6f35f8dd011f55dab8469be99d47 Gerrit-Change-Number: 45535 Gerrit-PatchSet: 32 Gerrit-Owner: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-Reviewer: Elyes HAOUAS Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)gmail.com> Gerrit-Reviewer: Matt Delco <delco(a)chromium.org> 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: Furquan Shaikh <furquan(a)google.com> Gerrit-MessageType: newpatchset

1 0

Change in coreboot[master]: soc/intel/common/acpi: fix Windows crash on S0ix-enabled boards
by Michael Niewöhner (Code Review) 03 Nov '20

03 Nov '20

Michael Niewöhner 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 1: (1 comment) 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 > All Windows versions? I will test more and add the tested versions -- 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: 1 Gerrit-Owner: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.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-Comment-Date: Mon, 02 Nov 2020 23:49:52 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: mb/google/dedede/var/drawcia: Probe and enable DPTF configuration
by Karthik Ramasubramanian (Code Review) 03 Nov '20

03 Nov '20

Karthik Ramasubramanian has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47010 ) Change subject: mb/google/dedede/var/drawcia: Probe and enable DPTF configuration ...................................................................... Patch Set 4: (4 comments) https://review.coreboot.org/c/coreboot/+/47010/1//COMMIT_MSG Commit Message: https://review.coreboot.org/c/coreboot/+/47010/1//COMMIT_MSG@11 PS1, Line 11: board version > May be the commit message is not entirely accurate. […] Done https://review.coreboot.org/c/coreboot/+/47010/1/src/mainboard/google/deded… File src/mainboard/google/dedede/variants/drawcia/variant.c: https://review.coreboot.org/c/coreboot/+/47010/1/src/mainboard/google/deded… PS1, Line 27: DPTF_PASSIVE > Yes based on the available data. Done https://review.coreboot.org/c/coreboot/+/47010/1/src/mainboard/google/deded… PS1, Line 33: board_ver == 1 > I like that idea. We should be able to probe based on the clamshell or convertible FW_CONFIG bit. […] Done https://review.coreboot.org/c/coreboot/+/47010/1/src/mainboard/google/deded… PS1, Line 33: if (!google_chromeec_get_board_version(&board_ver) && board_ver == 1) : memcpy(&cfg->policies.passive[DPTF_CHARGER], &charger_policy, : sizeof(charger_policy)); > With the suggestion from Furquan, I think I may not need this approach. […] Ack -- To view, visit https://review.coreboot.org/c/coreboot/+/47010 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ibf166a2e36fa5775e2dea7c1adcae843cc143d32 Gerrit-Change-Number: 47010 Gerrit-PatchSet: 4 Gerrit-Owner: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Reviewer: Evan Green <evgreen(a)chromium.org> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Sumeet R Pawnikar <sumeet.r.pawnikar(a)intel.com> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Mon, 02 Nov 2020 23:44:35 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Furquan Shaikh <furquan(a)google.com> Comment-In-Reply-To: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Comment-In-Reply-To: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: mb/google/dedede/var/drawcia: Probe and enable DPTF configuration
by Karthik Ramasubramanian (Code Review) 03 Nov '20

03 Nov '20

Hello build bot (Jenkins), Furquan Shaikh, Evan Green, Tim Wawrzynczak, Sumeet R Pawnikar, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/47010 to look at the new patch set (#4). Change subject: mb/google/dedede/var/drawcia: Probe and enable DPTF configuration ...................................................................... mb/google/dedede/var/drawcia: Probe and enable DPTF configuration Different form factors require different DPTF configuration from performance standpoint. Probe for the form factor and choose the DPTF configuration accordingly. BUG=b:170229672 TEST=Build and boot to OS in Drawlat & Drawcia. Ensure that the DPTF configuration is enabled based on the device form factor. Change-Id: Ibf166a2e36fa5775e2dea7c1adcae843cc143d32 Signed-off-by: Karthikeyan Ramasubramanian <kramasub(a)google.com> --- M src/mainboard/google/dedede/variants/drawcia/overridetree.cb 1 file changed, 57 insertions(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/10/47010/4 -- To view, visit https://review.coreboot.org/c/coreboot/+/47010 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ibf166a2e36fa5775e2dea7c1adcae843cc143d32 Gerrit-Change-Number: 47010 Gerrit-PatchSet: 4 Gerrit-Owner: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Reviewer: Evan Green <evgreen(a)chromium.org> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Sumeet R Pawnikar <sumeet.r.pawnikar(a)intel.com> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: newpatchset

1 0

Change in coreboot[master]: console: Allow VPD to disable an otherwise enabled coreboot console
by Julius Werner (Code Review) 03 Nov '20

03 Nov '20

Julius Werner 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: Code-Review-1 (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. The patch makes an unconditional call with very complicated dependencies very early in the boot process. It may logically not change behavior but it pulls in a lot of code into stages that otherwise wouldn't link it, adds boot time impact to every pre-RAM stage (VPD is not meant to be accessed pre-RAM and not made to be efficient there), and adds a big risk of hanging the firmware before it can output anything on the console if there is any issue in that large chain of dependencies. This issue has already been discussed in several places recently (e.g. CB:45408, CB:45405, CB:45774). This patch has basically the same issues and I still think we shouldn't do something like this at all, because VPD is just a system that isn't well-suited for this super early use. But if enough people think we do need something like this, it should absolutely be behind a Kconfig so it can be disabled to the point of not even linking the extra code, and it should be default-off. I think Nico's attempt of tying it into the get_option() API and having a default-off Kconfig to allow options to be read from VPD (CB:45774) was the best approach yet if we really want to go down that route. Trying to expand/fix the option table API seems cleaner than adding yet another one-off control somewhere else. -- 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: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-CC: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Comment-Date: Mon, 02 Nov 2020 23:23:12 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in coreboot[master]: soc/intel/common/acpi: rename LPID to PEPD
by Michael Niewöhner (Code Review) 03 Nov '20

03 Nov '20

Michael Niewöhner 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 3: > Patch Set 3: > > > Patch Set 3: > > > > What does PEPD mean? > > PEP Device. PEPD is the name Intel refcode and vendors (at least all I have seen) use Ah, well, PEP is "Power Engine Plug-in" -- 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: 3 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: Angel Pons <th3fanbus(a)gmail.com> Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Comment-Date: Mon, 02 Nov 2020 23:14:35 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Jump to page:

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit November 2020