Attention is currently required from: Dinesh Gehlot, Edward Doan, Eric Lai, Kapil Porwal, Nick Vaccaro, SH Kim, Subrata Banik, YH Lin.
Jamie Chen 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 3: Code-Review+1
(1 comment)
Patchset:
PS3:
> > Please refer to the bug for the detail. Add Jamie@intel for any further question.
> >
> > - We can refer to this comment for why we have IOAPIC configuration has this problem.: https://partnerissuetracker.corp.google.com/issues/350609957#comment22. I just assume GPI_INT has differt mechanism to handle its interrupt status in SoC or GPIO driver, Intel can help for any comment about it.
>
>
> if SoC is using same IRQ number for its internal device then changing this PIN to GPI IOAPIC makes lot more sense to me but looks like other parts are happily using same PIN for IOAPIC hence, I'm suspecting some abnormality with the device firmware.
>
> I don't know the timing difference between IOAPIC interrupt vs GPI IOAPIC interrupt.
The timing difference may be caused by the two types of interrupts, which are managed by different drivers, and the paths of the interrupts are different.
IOAPIC:
- GPIO==>IRQ127 ==> APIC==>CPU==>ISR registered by GPIO driver ==> GPIO driver dispatches event in GPI_IS ==> hook callback registered by i2c-hid driver
GPI IOAPIC:
- GPIO==>IRQ100 ==> APIC==>CPU==>ISR registered by i2c-hid driver
>
> >
> > - We had discussion with device vendor in the bug, they said cannot change interrupt signal timing in device firmware.: https://partnerissuetracker.corp.google.com/issues/350609957#comment27
>
> if device firmware acknowledge there is some limitation hence, we should be good to support this CL.
>
> Do you know if the device fw is meeting Intel's minimum IRQ width (150uS) requirement for ADL
From touchpad vendor, "IRQ width of Touchpad is depend on data size of touchpad. I think Device FW meet ADL requrement because most of touchpad data is over 4Byte."
>
> >
> > - We had 2 options for this problem, one was a patch in kernel side and another one was this patch. And we choose this change for Xol.
> >
> > - Actually we didn't try Edge Single interrupt configuration but I think it may cause DUT to miss interrupts from device on continuous user input on touchpad.
--
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: 3
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: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Edward Doan <edoan(a)chromium.org>
Gerrit-Attention: Eric Lai <ericllai(a)google.com>
Gerrit-Attention: Subrata Banik <subratabanik(a)google.com>
Gerrit-Attention: SH Kim <sh_.kim(a)samsung.corp-partner.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: YH Lin <yueherngl(a)chromium.org>
Gerrit-Comment-Date: Thu, 18 Jul 2024 11:58:16 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com>
Comment-In-Reply-To: SH Kim <sh_.kim(a)samsung.corp-partner.google.com>
Attention is currently required from: Kapil Porwal, Nick Vaccaro, Subrata Banik.
Eric Lai has posted comments on this change by Subrata Banik. ( https://review.coreboot.org/c/coreboot/+/83543?usp=email )
Change subject: mb/google/brya: Enable SKIP_RAM_ID_STRAPS for TRULO variant
......................................................................
Patch Set 1: Code-Review+2
--
To view, visit https://review.coreboot.org/c/coreboot/+/83543?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: I1acb4680a143611c55f4fa6e032fde38c62af054
Gerrit-Change-Number: 83543
Gerrit-PatchSet: 1
Gerrit-Owner: Subrata Banik <subratabanik(a)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: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Subrata Banik <subratabanik(a)google.com>
Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Attention: Nick Vaccaro <nvaccaro(a)chromium.org>
Gerrit-Comment-Date: Thu, 18 Jul 2024 11:46:04 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Attention is currently required from: Michał Kopeć, Michał Żygowski, Paul Menzel, Sergii Dmytruk.
Krystian Hebel has posted comments on this change by Krystian Hebel. ( https://review.coreboot.org/c/coreboot/+/82555?usp=email )
Change subject: mb/qemu-{i440fx,q35}/rom_media.c: add code for writable flash
......................................................................
Patch Set 8:
(2 comments)
File src/mainboard/emulation/qemu-i440fx/rom_media.c:
https://review.coreboot.org/c/coreboot/+/82555/comment/a9d82572_2fe6f905?us… :
PS3, Line 31: to write to the log
> Flash based event log only, the one enabled with CONFIG_ELOG. […]
Done
File src/mainboard/emulation/qemu-i440fx/rom_media.c:
https://review.coreboot.org/c/coreboot/+/82555/comment/94db5778_46cae2ec?us… :
PS4, Line 158: flash_ops
> `mdev_readat` is used for all 3 options, it is copied in line 131 from `mem_rdev_rw_ops`.
Done
--
To view, visit https://review.coreboot.org/c/coreboot/+/82555?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: I3ab9f22c6165064a769881d4be5eab13a0a2f519
Gerrit-Change-Number: 82555
Gerrit-PatchSet: 8
Gerrit-Owner: Krystian Hebel <krystian.hebel(a)3mdeb.com>
Gerrit-Reviewer: Michał Kopeć <michal.kopec(a)3mdeb.com>
Gerrit-Reviewer: Michał Żygowski <michal.zygowski(a)3mdeb.com>
Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com>
Gerrit-Reviewer: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Michał Żygowski <michal.zygowski(a)3mdeb.com>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Michał Kopeć <michal.kopec(a)3mdeb.com>
Gerrit-Attention: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com>
Gerrit-Comment-Date: Thu, 18 Jul 2024 11:24:29 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Patrick Rudolph <patrick.rudolph(a)9elements.com>
Comment-In-Reply-To: Paul Menzel <paulepanter(a)mailbox.org>
Comment-In-Reply-To: Krystian Hebel <krystian.hebel(a)3mdeb.com>
Attention is currently required from: Dinesh Gehlot, Kapil Porwal, Nick Vaccaro, Subrata Banik.
Hello Dinesh Gehlot, Kapil Porwal, Nick Vaccaro, Nico Huber, Subrata Banik, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/83356?usp=email
to look at the new patch set (#3).
Change subject: soc/intel/alderlake/tcss: Add definition of IOM_READY bit
......................................................................
soc/intel/alderlake/tcss: Add definition of IOM_READY bit
Add definition of the IOM_READY bit in the IOM_TYPEC_STATUS_1
register. Needed by Protectli VP66XX boards to poll for this bit
for about 2 seconds before FSP Silicon Init to have USB functionality.
ME is supposed to start fetching and executing the TCSS IPs FW right
after DRAM Init Done message, which happens after MRC. For most
platforms the time interval between the end of MemoryInit and start of of SiliconInit is enough for IOM_READY to get set.
TEST=Poll the IOM_READY bit on VP66XX platform and observe the
TCSS XHCI is up in lspci.
Change-Id: If868a77852468ebb73526b1571191cbdeb1804b9
Signed-off-by: Michał Żygowski <michal.zygowski(a)3mdeb.com>
---
M src/soc/intel/alderlake/include/soc/tcss.h
1 file changed, 4 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/56/83356/3
--
To view, visit https://review.coreboot.org/c/coreboot/+/83356?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: If868a77852468ebb73526b1571191cbdeb1804b9
Gerrit-Change-Number: 83356
Gerrit-PatchSet: 3
Gerrit-Owner: Michał Żygowski <michal.zygowski(a)3mdeb.com>
Gerrit-Reviewer: Dinesh Gehlot <digehlot(a)google.com>
Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)chromium.org>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
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>
Attention is currently required from: Dinesh Gehlot, Kapil Porwal, Nick Vaccaro, Subrata Banik.
Michał Żygowski has posted comments on this change by Michał Żygowski. ( https://review.coreboot.org/c/coreboot/+/83356?usp=email )
Change subject: soc/intel/alderlake/tcss: Add definition of IOM_READY bit
......................................................................
Patch Set 2:
(1 comment)
Patchset:
PS1:
Unfortunately I didn't find documentation of this bit in the Core and Uncore specification. I would have to dig in other documents probably.
> What also would be interesting is how long it's supposed to take.
That's a good question. For this platform 2 seconds is the shortest time required to successfully poll this bit (determined with trial and error process). Intel documentation says the BIOS can start polling about 10ms atfer DRAM Init Done message is sent to ME, so ME is supposed to start fetching TCSS IP FW right after MRC. It takes surprisingly long for this platform. Typically IOM is ready when other platforms enter Silicon Init.
> If it's necessary on the resume path too (just out of curiosity).
TBH, I haven't checked how does it look like on S3 resume.
--
To view, visit https://review.coreboot.org/c/coreboot/+/83356?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: If868a77852468ebb73526b1571191cbdeb1804b9
Gerrit-Change-Number: 83356
Gerrit-PatchSet: 2
Gerrit-Owner: Michał Żygowski <michal.zygowski(a)3mdeb.com>
Gerrit-Reviewer: Dinesh Gehlot <digehlot(a)google.com>
Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)chromium.org>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
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-Comment-Date: Thu, 18 Jul 2024 11:17:03 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de>
Comment-In-Reply-To: Michał Żygowski <michal.zygowski(a)3mdeb.com>
Sean Rhodes has posted comments on this change by Sean Rhodes. ( https://review.coreboot.org/c/coreboot/+/83542?usp=email )
Change subject: soc/intel/alderlake: Correctly set I2C SDA and SCL pins
......................................................................
Set Ready For Review
--
To view, visit https://review.coreboot.org/c/coreboot/+/83542?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: I8936012c40351f7b9577187b80dcd3818049cb2b
Gerrit-Change-Number: 83542
Gerrit-PatchSet: 2
Gerrit-Owner: Sean Rhodes <sean(a)starlabs.systems>
Gerrit-CC: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Comment-Date: Thu, 18 Jul 2024 10:44:26 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: No
Attention is currently required from: Dinesh Gehlot, Eran Mitrani, Jakub Czapiga, Julius Werner, Kapil Porwal, Rishika Raj, Tarun.
Hello Dinesh Gehlot, Eran Mitrani, Jakub Czapiga, Julius Werner, Kapil Porwal, Subrata Banik, Tarun, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/83540?usp=email
to look at the new patch set (#5).
Change subject: soc/intel/meteorlake: Increase CAR STACK_SIZE by 31KB to meet coreboot requirements
......................................................................
soc/intel/meteorlake: Increase CAR STACK_SIZE by 31KB to meet coreboot requirements
This change increases the DCACHE_BSP_STACK_SIZE from 512KB + 1KB to
512KB + 32KB, addressing a requirement specified by coreboot where
stack usage is higher than 1KB alone.
BUG=None
TEST=None
Change-Id: Iba3620b3b7c470176330f5e07989cd3f6238713e
Signed-off-by: Rishika Raj <rishikaraj(a)google.com>
---
M src/soc/intel/meteorlake/Kconfig
1 file changed, 2 insertions(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/40/83540/5
--
To view, visit https://review.coreboot.org/c/coreboot/+/83540?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: Iba3620b3b7c470176330f5e07989cd3f6238713e
Gerrit-Change-Number: 83540
Gerrit-PatchSet: 5
Gerrit-Owner: Rishika Raj <rishikaraj(a)google.com>
Gerrit-Reviewer: Dinesh Gehlot <digehlot(a)google.com>
Gerrit-Reviewer: Eran Mitrani <mitrani(a)google.com>
Gerrit-Reviewer: Jakub Czapiga <czapiga(a)google.com>
Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org>
Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: Tarun <tstuli(a)gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Rishika Raj <rishikaraj(a)google.com>
Gerrit-Attention: Eran Mitrani <mitrani(a)google.com>
Gerrit-Attention: Jakub Czapiga <czapiga(a)google.com>
Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Attention: Dinesh Gehlot <digehlot(a)google.com>
Gerrit-Attention: Julius Werner <jwerner(a)chromium.org>
Gerrit-Attention: Tarun <tstuli(a)gmail.com>