coreboot-gerrit April 2022

coreboot-gerrit@coreboot.org

1 participants
5879 discussions

Change in coreboot[master]: drivers/i2c/dw_i2c: Adjust to handle 0-byte transfers
by Matt DeVillier (Code Review) 13 Apr '22

13 Apr '22

Attention is currently required from: Tim Wawrzynczak. Matt DeVillier has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/63561 ) Change subject: drivers/i2c/dw_i2c: Adjust to handle 0-byte transfers ...................................................................... Patch Set 1: (1 comment) File src/drivers/i2c/designware/dw_i2c.c: https://review.coreboot.org/c/coreboot/+/63561/comment/62699e68_854d9260 PS1, Line 377: if (segments->len == 0) > This implies (count == 1) as well right? should it be checked? not necessarily - you still need at least 1 segment even with a zero-byte xfer, to define the slave address. I could check for count == 0 here as well, but did it this way to be consistent with the addition below -- To view, visit https://review.coreboot.org/c/coreboot/+/63561 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I518e849f4c476c264a1464886b1853af66c0b29d Gerrit-Change-Number: 63561 Gerrit-PatchSet: 1 Gerrit-Owner: Matt DeVillier <matt.devillier(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Comment-Date: Wed, 13 Apr 2022 18:39:04 +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]: util/apcb/apcb_v3_edit.py: Edit APCB based on different SPD magic
by Rob Barnes (Code Review) 13 Apr '22

13 Apr '22

Attention is currently required from: Karthik Ramasubramanian. Rob Barnes has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/63598 ) Change subject: util/apcb/apcb_v3_edit.py: Edit APCB based on different SPD magic ...................................................................... Patch Set 1: (1 comment) File util/apcb/apcb_v3_edit.py: https://review.coreboot.org/c/coreboot/+/63598/comment/1a1a464c_a5664194 PS1, Line 51: '--spd_magic', How about adding a --memory_type [lp4 | lp5] argument? Then we can keep the SPD magic numbers defined here. -- To view, visit https://review.coreboot.org/c/coreboot/+/63598 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I8e96c89e4e5ce8e0567a17bf7685b69080fa1708 Gerrit-Change-Number: 63598 Gerrit-PatchSet: 1 Gerrit-Owner: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Reviewer: Jon Murphy <jpmurphy(a)google.com> Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org> Gerrit-Reviewer: Rob Barnes <robbarnes(a)google.com> Gerrit-Reviewer: Robert Zieba <robertzieba(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Comment-Date: Wed, 13 Apr 2022 18:20:34 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in coreboot[master]: drivers/usb/acpi: acpi_power_res_params: Add use_gpio_for_status
by Tim Van Patten (Code Review) 13 Apr '22

13 Apr '22

1 0

Change in coreboot[master]: mb/google/guybrush: Set BT USB to use GPIO for status
by Tim Van Patten (Code Review) 13 Apr '22

13 Apr '22

Attention is currently required from: Raul Rangel. Hello build bot (Jenkins), Raul Rangel, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/63559 to look at the new patch set (#4). Change subject: mb/google/guybrush: Set BT USB to use GPIO for status ...................................................................... mb/google/guybrush: Set BT USB to use GPIO for status Set the BT USB device to use GPIO for the power status. This causes an ACPI `_STA()` function to be generated that returns the power status of the BT USB device, rather than always returning `0x1`. This `_STA()` function can be used during boot to skip enabling the device (and performing the associated sleep) if the device is already powered on. BRANCH=None BUG=b:225022810 TEST=Dump SSDT table for guybrush Signed-off-by: Tim Van Patten <timvp(a)google.com> Change-Id: I72f6b28671efddfbef53f328d904a05f73f39efa --- M src/mainboard/google/guybrush/variants/baseboard/devicetree.cb 1 file changed, 1 insertion(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/59/63559/4 -- To view, visit https://review.coreboot.org/c/coreboot/+/63559 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I72f6b28671efddfbef53f328d904a05f73f39efa Gerrit-Change-Number: 63559 Gerrit-PatchSet: 4 Gerrit-Owner: Tim Van Patten <timvp(a)google.com> Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Raul Rangel <rrangel(a)chromium.org> Gerrit-MessageType: newpatchset

1 0

Change in coreboot[master]: src/acpi/device.c: Early return in _ON if device already enabled
by Tim Van Patten (Code Review) 13 Apr '22

13 Apr '22

Attention is currently required from: Lance Zhao, Raul Rangel, Tim Wawrzynczak. Hello Lance Zhao, Raul Rangel, Tim Wawrzynczak, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/63613 to look at the new patch set (#2). Change subject: src/acpi/device.c: Early return in _ON if device already enabled ...................................................................... src/acpi/device.c: Early return in _ON if device already enabled If the device has enabled `use_gpio_for_status`, then call the `_STA` method in `_ON` to determine if the device is already enabled. If it is already enabled, return early to skip re-enabling the device and performing the associated sleep. This change is necessary since the Linux kernel does not call `_STA` before calling `_ON`. BRANCH=None BUG=b:225022810 TEST=Dump SSDT table for guybrush Signed-off-by: Tim Van Patten <timvp(a)google.com> Change-Id: I13aa41766555953b86eded4c72e3b317fe6db5c8 --- M src/acpi/device.c 1 file changed, 12 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/13/63613/2 -- To view, visit https://review.coreboot.org/c/coreboot/+/63613 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I13aa41766555953b86eded4c72e3b317fe6db5c8 Gerrit-Change-Number: 63613 Gerrit-PatchSet: 2 Gerrit-Owner: Tim Van Patten <timvp(a)google.com> Gerrit-Reviewer: Lance Zhao Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Attention: Lance Zhao Gerrit-Attention: Raul Rangel <rrangel(a)chromium.org> Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-MessageType: newpatchset

1 0

Change in coreboot[master]: drivers/usb/acpi: acpi_power_res_params: Add use_gpio_for_status
by Tim Van Patten (Code Review) 13 Apr '22

13 Apr '22

Attention is currently required from: Lance Zhao, Tim Wawrzynczak, Tim Van Patten. Hello Lance Zhao, build bot (Jenkins), Raul Rangel, Tim Wawrzynczak, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/63558 to look at the new patch set (#4). Change subject: drivers/usb/acpi: acpi_power_res_params: Add use_gpio_for_status ...................................................................... drivers/usb/acpi: acpi_power_res_params: Add use_gpio_for_status Add the member `use_gpio_for_status` to the structure `drivers_usb_acpi_config`, so the `devicetree.cb` can specify it. This field is then used to initialize the corresponding field in the structure `acpi_power_res_params` in `usb_acpi_fill_ssdt_generator()`. The member `acpi_power_res_params::use_gpio_for_status()` is already being used by `acpi_device_add_power_res()` to determine which version of the `_STA()` method to output. BRANCH=None BUG=b:225022810 TEST=Dump SSDT table for guybrush Signed-off-by: Tim Van Patten <timvp(a)google.com> Change-Id: I69eb5f1ad79f3b2980f43dcf4a36585fca198ec9 --- M src/drivers/usb/acpi/chip.h M src/drivers/usb/acpi/usb_acpi.c 2 files changed, 8 insertions(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/58/63558/4 -- To view, visit https://review.coreboot.org/c/coreboot/+/63558 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I69eb5f1ad79f3b2980f43dcf4a36585fca198ec9 Gerrit-Change-Number: 63558 Gerrit-PatchSet: 4 Gerrit-Owner: Tim Van Patten <timvp(a)google.com> Gerrit-Reviewer: Lance Zhao Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Lance Zhao Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Attention: Tim Van Patten <timvp(a)google.com> Gerrit-MessageType: newpatchset

1 0

Change in coreboot[master]: drivers/usb/acpi: acpi_power_res_params: Add use_gpio_for_status
by Raul Rangel (Code Review) 13 Apr '22

13 Apr '22

Attention is currently required from: Lance Zhao, Tim Wawrzynczak, Tim Van Patten. Raul Rangel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/63558 ) Change subject: drivers/usb/acpi: acpi_power_res_params: Add use_gpio_for_status ...................................................................... Patch Set 3: (2 comments) Commit Message: https://review.coreboot.org/c/coreboot/+/63558/comment/46fb7233_0c3f9857 PS3, Line 22: TEST Group BRANCH= BUG= TEST= Above Signed-off-by: Change-Id: File src/drivers/usb/acpi/usb_acpi.c: https://review.coreboot.org/c/coreboot/+/63558/comment/a02d040b_4701364d PS3, Line 98: 0, We should really name all these at some point. -- To view, visit https://review.coreboot.org/c/coreboot/+/63558 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I69eb5f1ad79f3b2980f43dcf4a36585fca198ec9 Gerrit-Change-Number: 63558 Gerrit-PatchSet: 3 Gerrit-Owner: Tim Van Patten <timvp(a)google.com> Gerrit-Reviewer: Lance Zhao Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Lance Zhao Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Attention: Tim Van Patten <timvp(a)google.com> Gerrit-Comment-Date: Wed, 13 Apr 2022 18:08:24 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in coreboot[master]: src/acpi/device.c: Early return in _ON if device already enabled
by Tim Van Patten (Code Review) 13 Apr '22

13 Apr '22

Attention is currently required from: Lance Zhao, Raul Rangel. Tim Van Patten has removed Tim Wawrzynczak from this change. ( https://review.coreboot.org/c/coreboot/+/63613 ) Change subject: src/acpi/device.c: Early return in _ON if device already enabled ...................................................................... Removed reviewer Tim Wawrzynczak. -- To view, visit https://review.coreboot.org/c/coreboot/+/63613 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I13aa41766555953b86eded4c72e3b317fe6db5c8 Gerrit-Change-Number: 63613 Gerrit-PatchSet: 1 Gerrit-Owner: Tim Van Patten <timvp(a)google.com> Gerrit-Reviewer: Lance Zhao Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org> Gerrit-Attention: Lance Zhao Gerrit-Attention: Raul Rangel <rrangel(a)chromium.org> Gerrit-MessageType: deleteReviewer

1 0

Change in coreboot[master]: drivers/usb/acpi: acpi_power_res_params: Add use_gpio_for_status
by Tim Van Patten (Code Review) 13 Apr '22

13 Apr '22

Attention is currently required from: Lance Zhao, Raul Rangel, Tim Wawrzynczak. Tim Van Patten has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/63558 ) Change subject: drivers/usb/acpi: acpi_power_res_params: Add use_gpio_for_status ...................................................................... Patch Set 3: (6 comments) File src/acpi/device.c: PS2: > Split this into its own commit. Done: https://review.coreboot.org/c/coreboot/+/63613 https://review.coreboot.org/c/coreboot/+/63558/comment/202fde45_27743945 PS2, Line 608: bool active > not required Ack https://review.coreboot.org/c/coreboot/+/63558/comment/cbe4b872_57253096 PS2, Line 623: f (active) : acpigen_emit_byte(LNOT_OP); > acpigen_get_tx_gpio() will already return "1" for the active state, regardless of polarity (i.e. […] Ack https://review.coreboot.org/c/coreboot/+/63558/comment/90d25dc5_c06f5bbf PS2, Line 698: ; > How about: […] Done https://review.coreboot.org/c/coreboot/+/63558/comment/93bd5240_caddc888 PS2, Line 703: /* Check if the device is already enabled before enabling it again. : This allows for skipping the sleep if it's also requested. */ > See https://doc.coreboot.org/contributing/coding_style. […] Done https://review.coreboot.org/c/coreboot/+/63558/comment/fae30e19_bfbb92d5 PS2, Line 705: acpigen_write_gpio_status_check > Hrmm I don't think we can just bail since the other GPIOs might be in a different state. Done -- To view, visit https://review.coreboot.org/c/coreboot/+/63558 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I69eb5f1ad79f3b2980f43dcf4a36585fca198ec9 Gerrit-Change-Number: 63558 Gerrit-PatchSet: 3 Gerrit-Owner: Tim Van Patten <timvp(a)google.com> Gerrit-Reviewer: Lance Zhao Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Lance Zhao Gerrit-Attention: Raul Rangel <rrangel(a)chromium.org> Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Comment-Date: Wed, 13 Apr 2022 18:03:56 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Raul Rangel <rrangel(a)chromium.org> Comment-In-Reply-To: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: src/acpi/device.c: Early return in _ON if device already enabled
by Tim Van Patten (Code Review) 13 Apr '22

13 Apr '22

Attention is currently required from: Lance Zhao, Tim Wawrzynczak. Tim Van Patten has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/63613 ) Change subject: src/acpi/device.c: Early return in _ON if device already enabled ...................................................................... src/acpi/device.c: Early return in _ON if device already enabled If the device has enabled `use_gpio_for_status`, then call the `_STA` method in `_ON` to determine if the device is already enabled. If it is already enabled, return early to skip re-enabling the device and performing the associated sleep. This change is necessary since the Linux kernel does not call `_STA` before calling `_ON`. BRANCH=None Signed-off-by: Tim Van Patten <timvp(a)google.com> BUG=b:225022810 TEST=Dump SSDT table for guybrush Change-Id: I13aa41766555953b86eded4c72e3b317fe6db5c8 --- M src/acpi/device.c 1 file changed, 12 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/13/63613/1 diff --git a/src/acpi/device.c b/src/acpi/device.c index 1df179b..b514455 100644 --- a/src/acpi/device.c +++ b/src/acpi/device.c @@ -670,6 +670,18 @@ /* Method (_ON, 0, Serialized) */ acpigen_write_method_serialized("_ON", 0); + /* Call _STA and early return if the device is already enabled, since the Linux + kernel doesn't check the device status before calling _ON. */ + if (params->use_gpio_for_status) { + /* Local0 = _STA () */ + acpigen_write_store(); + acpigen_emit_namestring("_STA"); + acpigen_emit_byte(LOCAL0_OP); + /* If (( Local0 == ACPI_POWER_RESOURCE_STATUS_ON_OP)) */ + acpigen_write_if_lequal_op_op(LOCAL0_OP, ACPI_POWER_RESOURCE_STATUS_ON_OP); + acpigen_write_return_op(ZERO_OP); + acpigen_write_if_end(); + } if (reset_gpio) acpigen_enable_tx_gpio(params->reset_gpio); if (enable_gpio) { -- To view, visit https://review.coreboot.org/c/coreboot/+/63613 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I13aa41766555953b86eded4c72e3b317fe6db5c8 Gerrit-Change-Number: 63613 Gerrit-PatchSet: 1 Gerrit-Owner: Tim Van Patten <timvp(a)google.com> Gerrit-Reviewer: Lance Zhao Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Attention: Lance Zhao Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-MessageType: newchange

1 0

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit April 2022