Attention is currently required from: Jérémy Compostella, yuchi.chen(a)intel.com.
Shuo Liu has posted comments on this change by yuchi.chen(a)intel.com. ( https://review.coreboot.org/c/coreboot/+/83316?usp=email )
Change subject: soc/intel/common/block/gpio/gpio.c: Improve GPIO debug infos
......................................................................
Patch Set 7: Code-Review+1
(2 comments)
File src/soc/intel/common/block/gpio/gpio.c:
https://review.coreboot.org/c/coreboot/+/83316/comment/633f735d_62529920?us… :
PS6, Line 196: printk(BIOS_DEBUG, "GPE_EN[0x%02x, %02zu]: Reg: 0x%x, Value = 0x%08x\n",
> zu is used to print unsigned size_t. […]
Done
https://review.coreboot.org/c/coreboot/+/83316/comment/30989beb_0ac5797f?us… :
PS6, Line 399: pcr_read32(comm->port, PAD_CFG_OFFSET(config_offset, i))); /* updated value */
> I moved the printk function after pcr_write so that it can print the value after writting.
Done
--
To view, visit https://review.coreboot.org/c/coreboot/+/83316?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I8820956f6db91c7bcc26b46a4361da3dfa8f77b5
Gerrit-Change-Number: 83316
Gerrit-PatchSet: 7
Gerrit-Owner: yuchi.chen(a)intel.com
Gerrit-Reviewer: Jérémy Compostella <jeremy.compostella(a)intel.com>
Gerrit-Reviewer: Shuo Liu <shuo.liu(a)intel.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: yuchi.chen(a)intel.com
Gerrit-Attention: Jérémy Compostella <jeremy.compostella(a)intel.com>
Gerrit-Comment-Date: Mon, 22 Jul 2024 01:19:24 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Comment-In-Reply-To: yuchi.chen(a)intel.com
Comment-In-Reply-To: Shuo Liu <shuo.liu(a)intel.com>
Attention is currently required from: Felix Singer, Jérémy Compostella, yuchi.chen(a)intel.com.
Shuo Liu has posted comments on this change by yuchi.chen(a)intel.com. ( https://review.coreboot.org/c/coreboot/+/83315?usp=email )
Change subject: soc/intel/common/intelblocks/gpio.h: Allow specifying the pad ownership
......................................................................
Patch Set 7:
(1 comment)
File src/soc/intel/common/block/include/intelblocks/gpio.h:
https://review.coreboot.org/c/coreboot/+/83315/comment/0ae7893d_2d4c71e9?us… :
PS7, Line 124: uint16_t pad_own_reg_0; /* offset to Pad Ownership Reg 0 */
it would be great if adding some comments for pad_own_reg_0 and host_own_reg_0 here.
--
To view, visit https://review.coreboot.org/c/coreboot/+/83315?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I30a934fd00a7a42cb156341da1954e4e4b1231d8
Gerrit-Change-Number: 83315
Gerrit-PatchSet: 7
Gerrit-Owner: yuchi.chen(a)intel.com
Gerrit-Reviewer: Jérémy Compostella <jeremy.compostella(a)intel.com>
Gerrit-Reviewer: Shuo Liu <shuo.liu(a)intel.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-Attention: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-Attention: yuchi.chen(a)intel.com
Gerrit-Attention: Jérémy Compostella <jeremy.compostella(a)intel.com>
Gerrit-Comment-Date: Mon, 22 Jul 2024 01:14:26 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Attention is currently required from: Felix Singer, Jérémy Compostella, yuchi.chen(a)intel.com.
Shuo Liu has posted comments on this change by yuchi.chen(a)intel.com. ( https://review.coreboot.org/c/coreboot/+/83315?usp=email )
Change subject: soc/intel/common/intelblocks/gpio.h: Allow specifying the pad ownership
......................................................................
Patch Set 7: Code-Review+1
(1 comment)
File src/soc/intel/common/block/include/intelblocks/gpio.h:
https://review.coreboot.org/c/coreboot/+/83315/comment/eed147f3_4b0765d2?us… :
PS6, Line 125: uint16_t host_own_reg_0; /* offset to Host Ownership Reg 0 */
> The pad_own_reg_0 is used to indicate whether this pad is owned by host software or Intel Management […]
Done
--
To view, visit https://review.coreboot.org/c/coreboot/+/83315?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I30a934fd00a7a42cb156341da1954e4e4b1231d8
Gerrit-Change-Number: 83315
Gerrit-PatchSet: 7
Gerrit-Owner: yuchi.chen(a)intel.com
Gerrit-Reviewer: Jérémy Compostella <jeremy.compostella(a)intel.com>
Gerrit-Reviewer: Shuo Liu <shuo.liu(a)intel.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-Attention: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-Attention: yuchi.chen(a)intel.com
Gerrit-Attention: Jérémy Compostella <jeremy.compostella(a)intel.com>
Gerrit-Comment-Date: Mon, 22 Jul 2024 01:13:36 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Comment-In-Reply-To: yuchi.chen(a)intel.com
Comment-In-Reply-To: Shuo Liu <shuo.liu(a)intel.com>
Attention is currently required from: Dinesh Gehlot, Eric Lai, Kapil Porwal, Nick Vaccaro, Paul Menzel, Subrata Banik, Sumeet R Pawnikar, YH Lin, YH Lin.
SH Kim has posted comments on this change by SH Kim. ( https://review.coreboot.org/c/coreboot/+/83479?usp=email )
Change subject: mb/google/brya/var/xol: Limit power limits for low/no battery case
......................................................................
Patch Set 7:
(1 comment)
Commit Message:
https://review.coreboot.org/c/coreboot/+/83479/comment/b8f2d7ba_7c8b9861?us… :
PS7, Line 17: BUG=b:353395811
> Done.
Can we get this change merged if we don't have more comment?
--
To view, visit https://review.coreboot.org/c/coreboot/+/83479?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: Ic19119042ffdcc15c72764d8c27bcdce9f229438
Gerrit-Change-Number: 83479
Gerrit-PatchSet: 7
Gerrit-Owner: SH Kim <sh_.kim(a)samsung.corp-partner.google.com>
Gerrit-Reviewer: Dinesh Gehlot <digehlot(a)google.com>
Gerrit-Reviewer: Eric Lai <ericllai(a)google.com>
Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)chromium.org>
Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: Sumeet R Pawnikar <sumeet.r.pawnikar(a)intel.com>
Gerrit-Reviewer: YH Lin <yueherngl(a)chromium.org>
Gerrit-Reviewer: YH Lin <yueherngl(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Eric Lai <ericllai(a)google.com>
Gerrit-Attention: YH Lin <yueherngl(a)google.com>
Gerrit-Attention: Subrata Banik <subratabanik(a)google.com>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Attention: Dinesh Gehlot <digehlot(a)google.com>
Gerrit-Attention: Sumeet R Pawnikar <sumeet.r.pawnikar(a)intel.com>
Gerrit-Attention: Nick Vaccaro <nvaccaro(a)chromium.org>
Gerrit-Attention: YH Lin <yueherngl(a)chromium.org>
Gerrit-Comment-Date: Mon, 22 Jul 2024 00:44:33 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: YH Lin <yueherngl(a)google.com>
Comment-In-Reply-To: Sumeet R Pawnikar <sumeet.r.pawnikar(a)intel.com>
Attention is currently required from: Dinesh Gehlot, Edward Doan, Eric Lai, Jamie Chen, Kapil Porwal, Nick Vaccaro, Nick Vaccaro, Subrata Banik, YH Lin.
SH Kim has posted comments on this change by SH Kim. ( https://review.coreboot.org/c/coreboot/+/83346?usp=email )
Change subject: mb/google/brya/var/xol: Change touchpad I2C interrupt type to GPIO_INT
......................................................................
Patch Set 5:
(3 comments)
File src/mainboard/google/brya/variants/xol/gpio.c:
https://review.coreboot.org/c/coreboot/+/83346/comment/24fe8ec9_b64c1b86?us… :
PS1, Line 123: PAD_CFG_GPI_INT_LOCK(GPP_F14, NONE, LEVEL, LOCK_CONFIG),
> I'm not sure where that clip came from, but the GPP_F14 gpio is not locked in mainboard/google/brya/ […]
LOCK_CONFIG for GPP_F14 has been removed from baseboard/brya due to this: https://partnerissuetracker.corp.google.com/issues/346917118#comment16
And I removed LOCK_CONFIG from this CL after that comment.
File src/mainboard/google/brya/variants/xol/gpio.c:
https://review.coreboot.org/c/coreboot/+/83346/comment/f77008a8_76217c13?us… :
PS5, Line 128: LEVEL
> Isn't this signal active-low?
There is no LEVEL_LOW or INVERT option for PAD_CFG_GPI_INT macro. I think OS driver can set it's invert property by refering to the ACPI interrupt property from "ACPI_GPIO_IRQ_LEVEL_LOW_WAKE(GPP_F14)" in overridetree.cb.
File src/mainboard/google/brya/variants/xol/overridetree.cb:
https://review.coreboot.org/c/coreboot/+/83346/comment/4c1a047f_828b9e2c?us… :
PS1, Line 347: register "generic.detect" = "1"
> Won't this enable the interrupt? (IIRC, the touchpad is powered directly off of a power rail, so no […]
Just had some test for the scenario you mentioned.
- Keep making touchpad input during booting with 5 times of "reboot" command and 5 times of ec reset
- With/without this change, I couldn't find such issue on my test.
From the test result, touchpad interrupt before kernel could be another story with the issue we are focussing on this change.
--
To view, visit https://review.coreboot.org/c/coreboot/+/83346?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: Ie1b59355a694e5a42367a20e03f6c5f93225e79c
Gerrit-Change-Number: 83346
Gerrit-PatchSet: 5
Gerrit-Owner: SH Kim <sh_.kim(a)samsung.corp-partner.google.com>
Gerrit-Reviewer: Dinesh Gehlot <digehlot(a)google.com>
Gerrit-Reviewer: Edward Doan <edoan(a)chromium.org>
Gerrit-Reviewer: Eric Lai <ericllai(a)google.com>
Gerrit-Reviewer: Jamie Chen <jamie.chen(a)intel.corp-partner.google.com>
Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)chromium.org>
Gerrit-Reviewer: SH Kim <sh_.kim(a)samsung.corp-partner.google.com>
Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: YH Lin <yueherngl(a)chromium.org>
Gerrit-Reviewer: YH Lin <yueherngl(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Nick Vaccaro <nvaccaro(a)google.com>
Gerrit-Attention: Edward Doan <edoan(a)chromium.org>
Gerrit-Attention: Eric Lai <ericllai(a)google.com>
Gerrit-Attention: Jamie Chen <jamie.chen(a)intel.corp-partner.google.com>
Gerrit-Attention: Subrata Banik <subratabanik(a)google.com>
Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Attention: Dinesh Gehlot <digehlot(a)google.com>
Gerrit-Attention: Nick Vaccaro <nvaccaro(a)chromium.org>
Gerrit-Attention: Nick Vaccaro <nvaccaro(a)google.com>
Gerrit-Attention: YH Lin <yueherngl(a)chromium.org>
Gerrit-Comment-Date: Mon, 22 Jul 2024 00:41:23 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com>
Comment-In-Reply-To: SH Kim <sh_.kim(a)samsung.corp-partner.google.com>
Comment-In-Reply-To: Nick Vaccaro <nvaccaro(a)google.com>
Attention is currently required from: Arthur Heymans, Julius Werner.
Hello Arthur Heymans, Julius Werner, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/83457?usp=email
to look at the new patch set (#6).
Change subject: commonlib/device_tree.c: Add read reg property helper
......................................................................
commonlib/device_tree.c: Add read reg property helper
Add a helper function to read the reg property from an unflattened
device tree.
It also factors out the common code into a new function called
`read_reg_prop`.
Signed-off-by: Maximilian Brune <maximilian.brune(a)9elements.com>
Change-Id: I7846eb8af390d709b0757262025cb819e9988699
---
M src/commonlib/device_tree.c
M src/commonlib/include/commonlib/device_tree.h
2 files changed, 73 insertions(+), 26 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/57/83457/6
--
To view, visit https://review.coreboot.org/c/coreboot/+/83457?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: newpatchset
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I7846eb8af390d709b0757262025cb819e9988699
Gerrit-Change-Number: 83457
Gerrit-PatchSet: 6
Gerrit-Owner: Maximilian Brune <maximilian.brune(a)9elements.com>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Lean Sheng Tan <sheng.tan(a)9elements.com>
Gerrit-Attention: Julius Werner <jwerner(a)chromium.org>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Attention is currently required from: Arthur Heymans, Julius Werner.
Hello Arthur Heymans, Julius Werner, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/83457?usp=email
to look at the new patch set (#5).
Change subject: commonlib/device_tree.c: Add read reg property helper
......................................................................
commonlib/device_tree.c: Add read reg property helper
Add a helper function to read the reg property from an unflattened
device tree.
It also factors out the common code into a new function called
`read_reg_prop`.
Signed-off-by: Maximilian Brune <maximilian.brune(a)9elements.com>
Change-Id: I7846eb8af390d709b0757262025cb819e9988699
---
M src/commonlib/device_tree.c
M src/commonlib/include/commonlib/device_tree.h
2 files changed, 73 insertions(+), 26 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/57/83457/5
--
To view, visit https://review.coreboot.org/c/coreboot/+/83457?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: newpatchset
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I7846eb8af390d709b0757262025cb819e9988699
Gerrit-Change-Number: 83457
Gerrit-PatchSet: 5
Gerrit-Owner: Maximilian Brune <maximilian.brune(a)9elements.com>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Lean Sheng Tan <sheng.tan(a)9elements.com>
Gerrit-Attention: Julius Werner <jwerner(a)chromium.org>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Attention is currently required from: Arthur Heymans, Julius Werner.
Maximilian Brune has posted comments on this change by Maximilian Brune. ( https://review.coreboot.org/c/coreboot/+/83457?usp=email )
Change subject: commonlib/device_tree.c: Add read reg property helper
......................................................................
Patch Set 4:
(1 comment)
File src/commonlib/device_tree.c:
https://review.coreboot.org/c/coreboot/+/83457/comment/49c1c3e9_9f01dc58?us… :
PS3, Line 1013: size_t count = prop->prop.size / (4 * addr_cells + 4 * size_cells);
> Can we factor out everything starting here into a common function that's used by both (dt and fdt)?
like this?
--
To view, visit https://review.coreboot.org/c/coreboot/+/83457?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I7846eb8af390d709b0757262025cb819e9988699
Gerrit-Change-Number: 83457
Gerrit-PatchSet: 4
Gerrit-Owner: Maximilian Brune <maximilian.brune(a)9elements.com>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Lean Sheng Tan <sheng.tan(a)9elements.com>
Gerrit-Attention: Julius Werner <jwerner(a)chromium.org>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Comment-Date: Sun, 21 Jul 2024 21:56:43 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Julius Werner <jwerner(a)chromium.org>
Attention is currently required from: Arthur Heymans, Maximilian Brune.
Hello Arthur Heymans, Julius Werner, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/83457?usp=email
to look at the new patch set (#4).
The following approvals got outdated and were removed:
Verified+1 by build bot (Jenkins)
Change subject: commonlib/device_tree.c: Add read reg property helper
......................................................................
commonlib/device_tree.c: Add read reg property helper
Add a helper function to read the reg property from an unflattened
device tree.
It also factors out the common code into a new function called
`read_reg_prop`.
Signed-off-by: Maximilian Brune <maximilian.brune(a)9elements.com>
Change-Id: I7846eb8af390d709b0757262025cb819e9988699
---
M src/commonlib/device_tree.c
M src/commonlib/include/commonlib/device_tree.h
2 files changed, 73 insertions(+), 26 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/57/83457/4
--
To view, visit https://review.coreboot.org/c/coreboot/+/83457?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: newpatchset
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I7846eb8af390d709b0757262025cb819e9988699
Gerrit-Change-Number: 83457
Gerrit-PatchSet: 4
Gerrit-Owner: Maximilian Brune <maximilian.brune(a)9elements.com>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Lean Sheng Tan <sheng.tan(a)9elements.com>
Gerrit-Attention: Maximilian Brune <maximilian.brune(a)9elements.com>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>