Attention is currently required from: Tim Wawrzynczak, Shon Wang.
Gaggery Tsai has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/63892 )
Change subject: mb/google/brya/var/vell: Remove unused i2c7 settings
......................................................................
Patch Set 1:
(1 comment)
Patchset:
PS1:
@Tim: Do we still need further clarification for this patch?
--
To view, visit https://review.coreboot.org/c/coreboot/+/63892
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ifc1e0085064a13149ebc7e70184d1f40462e0fff
Gerrit-Change-Number: 63892
Gerrit-PatchSet: 1
Gerrit-Owner: Gaggery Tsai <gaggery.tsai(a)intel.com>
Gerrit-Reviewer: Robert Chen <robert.chen(a)quanta.corp-partner.google.com>
Gerrit-Reviewer: Shon Wang <shon.wang(a)quanta.corp-partner.google.com>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Attention: Shon Wang <shon.wang(a)quanta.corp-partner.google.com>
Gerrit-Comment-Date: Wed, 04 May 2022 06:16:51 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Subrata Banik, Tim Wawrzynczak, Arthur Heymans, Nick Vaccaro, Eric Lai.
Werner Zeh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/63492 )
Change subject: soc/intel/fast_spi: Use `need_restore_mtrr()` to clear temp MTRR
......................................................................
Patch Set 2: Code-Review+1
(1 comment)
File src/cpu/x86/mtrr/mtrrlib.c:
https://review.coreboot.org/c/coreboot/+/63492/comment/a57f5b8b_36102d97
PS1, Line 52: /* Need to restore mtrr later using remove_temp_solution. */
: if (ENV_RAMSTAGE)
: need_restore_mtrr();
> > > > > > Ack […]
Is this still an open? Or can this be merged?
--
To view, visit https://review.coreboot.org/c/coreboot/+/63492
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I67fd36080b318ace53f3e34da53d747f9a3aa400
Gerrit-Change-Number: 63492
Gerrit-PatchSet: 2
Gerrit-Owner: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Eric Lai <eric_lai(a)quanta.corp-partner.google.com>
Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)google.com>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: Werner Zeh <werner.zeh(a)siemens.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Subrata Banik <subratabanik(a)google.com>
Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Attention: Nick Vaccaro <nvaccaro(a)google.com>
Gerrit-Attention: Eric Lai <eric_lai(a)quanta.corp-partner.google.com>
Gerrit-Comment-Date: Wed, 04 May 2022 05:57:18 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com>
Comment-In-Reply-To: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-MessageType: comment
Attention is currently required from: Nico Huber, Subrata Banik, Paul Menzel, Arthur Heymans.
Werner Zeh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/63982 )
Change subject: soc/intel/cmn/spi: Add ACPI SSDT extension for fast SPI
......................................................................
Patch Set 2:
(1 comment)
Patchset:
PS2:
> @werner: what you think about porting UART changes into SPI with proper PNP ID based on the PCI devi […]
Using non-registered ACPI-IDs can be a pain (see https://andy-shev.livejournal.com/154513.html?noscroll&utm_medium=endless_s… for an example). So as long as there is no driver model bound to this device in the kernel I would rather try to get an official ID from Intel (which can be challenging). For the time being (especially for this patch) I would rather use the "official" random device ID.
--
To view, visit https://review.coreboot.org/c/coreboot/+/63982
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ifa89cdf41f42d4df5b46f095e22924157d9f3c3f
Gerrit-Change-Number: 63982
Gerrit-PatchSet: 2
Gerrit-Owner: Werner Zeh <werner.zeh(a)siemens.com>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
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-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Subrata Banik <subratabanik(a)google.com>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Comment-Date: Wed, 04 May 2022 05:46:02 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com>
Gerrit-MessageType: comment
Attention is currently required from: Kangheui Won, Reka Norman, Paul Menzel, Tim Wawrzynczak, Nick Vaccaro.
Tyler Wang has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/63893 )
Change subject: mb/google/brya/var/craask: Disable LTE-related GPIOs based on fw_config
......................................................................
Patch Set 8:
(1 comment)
File src/mainboard/google/brya/variants/craask/fw_config.c:
https://review.coreboot.org/c/coreboot/+/63893/comment/2f7ddb3a_14fd783d
PS7, Line 23: if (!fw_config_probe(FW_CONFIG(DB_USB, DB_1C_LTE))) {
: printk(BIOS_DEBUG, "Disable LTE-related GPIO pins.\n");
: gpio_configure_pads(lte_disable_pads, ARRAY_SIZE(lte_disable_pads));
: }
:
> > This approach leavess a gpio interrupt line (GPP_H19) floating for a short time. […]
Hi Reka,
I see, thank you point out the root cause of floating, learn from you.
And also thanks for point out problem of the method I handle fw_config, I will be careful next time.
Would you mind CC me in the CL of fixing nivviks? In that case I will be informed and would update this method in craask.
Thanks!
--
To view, visit https://review.coreboot.org/c/coreboot/+/63893
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I37719cee48370a04534067aa64a3aa77e453948a
Gerrit-Change-Number: 63893
Gerrit-PatchSet: 8
Gerrit-Owner: Tyler Wang <tyler.wang(a)quanta.corp-partner.google.com>
Gerrit-Reviewer: Kangheui Won <khwon(a)chromium.org>
Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)google.com>
Gerrit-Reviewer: Reka Norman <rekanorman(a)chromium.org>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Cathy Chen <cathy_chen(a)quanta.corp-partner.google.com>
Gerrit-CC: David Wu <david_wu(a)quanta.corp-partner.google.com>
Gerrit-CC: Eric Lai <eric_lai(a)quanta.corp-partner.google.com>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-CC: Ren Kuo <ren.kuo(a)quanta.corp-partner.google.com>
Gerrit-Attention: Kangheui Won <khwon(a)chromium.org>
Gerrit-Attention: Reka Norman <rekanorman(a)chromium.org>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Attention: Nick Vaccaro <nvaccaro(a)google.com>
Gerrit-Comment-Date: Wed, 04 May 2022 05:12:25 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Reka Norman <rekanorman(a)chromium.org>
Comment-In-Reply-To: Nick Vaccaro <nvaccaro(a)google.com>
Gerrit-MessageType: comment
Attention is currently required from: Reka Norman, Paul Menzel, Tim Wawrzynczak, Ivy Jian, Julius Werner.
Eric Lai has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/63774 )
Change subject: lib/spd: Demote log about using default DDR4 params to NOTICE
......................................................................
Patch Set 5:
(1 comment)
Patchset:
PS1:
> Sorry, I feel like this is my fault. […]
The stress test just check there no error in FW log. There is interest parts, there is always delay or late release SPD information to the public. Unless someone in the JEDEC can maintain this. Otherwise, we always behind the new memory type release. And this is just trivial information to the user, the real information should still based on SMBios type 17. Thus, I still think this is unimportant log.
--
To view, visit https://review.coreboot.org/c/coreboot/+/63774
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I55f0968b78baaa2fc9a6bbebf6712fb8bfd349f6
Gerrit-Change-Number: 63774
Gerrit-PatchSet: 5
Gerrit-Owner: Eric Lai <eric_lai(a)quanta.corp-partner.google.com>
Gerrit-Reviewer: Ivy Jian <ivy.jian(a)quanta.corp-partner.google.com>
Gerrit-Reviewer: Kangheui Won <khwon(a)chromium.org>
Gerrit-Reviewer: Reka Norman <rekanorman(a)chromium.org>
Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Julius Werner <jwerner(a)chromium.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Reka Norman <rekanorman(a)chromium.org>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Attention: Ivy Jian <ivy.jian(a)quanta.corp-partner.google.com>
Gerrit-Attention: Julius Werner <jwerner(a)chromium.org>
Gerrit-Comment-Date: Wed, 04 May 2022 05:10:25 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Reka Norman <rekanorman(a)chromium.org>
Comment-In-Reply-To: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Comment-In-Reply-To: Julius Werner <jwerner(a)chromium.org>
Comment-In-Reply-To: Eric Lai <eric_lai(a)quanta.corp-partner.google.com>
Gerrit-MessageType: comment
Attention is currently required from: Henry Sun, Super Ni, Teddy Shih, Paul Menzel, Simon Yang, Aseda Aboagye, Ivan Chen, Karthik Ramasubramanian, Felix Held.
Teddy Shih has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/63652 )
Change subject: mb/google/dedede/var/beadrix: Add a Proximity Sensor SX9324 for SAR
......................................................................
Patch Set 9: Code-Review+1
(2 comments)
Patchset:
PS9:
Hi Simon,
Thank you for your advice.
I correct I2C1 and I2C3 refer to https://source.chromium.org/chromiumos/chromiumos/codesearch/+/main:src/thi…
Best Regards,
Teddy
File src/mainboard/google/dedede/variants/beadrix/overridetree.cb:
https://review.coreboot.org/c/coreboot/+/63652/comment/bb3bf51f_2b77989e
PS8, Line 17: Digitizer
> Please correct I2C1/I2C3 either.
Hi Simon,
Thank you for your advice.
I correct I2C1 and I2C3 refer to https://source.chromium.org/chromiumos/chromiumos/codesearch/+/main:src/thi…
--
To view, visit https://review.coreboot.org/c/coreboot/+/63652
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: If172d13aa62503547227adf91f049ea50b948888
Gerrit-Change-Number: 63652
Gerrit-PatchSet: 9
Gerrit-Owner: Teddy Shih <teddyshih(a)ami.corp-partner.google.com>
Gerrit-Reviewer: Aseda Aboagye <aaboagye(a)google.com>
Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Henry Sun <henrysun(a)google.com>
Gerrit-Reviewer: Ivan Chen <yulunchen(a)google.com>
Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Reviewer: Simon Yang <simon1.yang(a)intel.corp-partner.google.com>
Gerrit-Reviewer: Super Ni <super.ni(a)intel.corp-partner.google.com>
Gerrit-Reviewer: Teddy Shih <teddyshih(a)ami.corp-partner.google.com>
Gerrit-Reviewer: Teddy Shih <teddyshihau(a)gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Eddy Lu <eddylu(a)ami.corp-partner.google.com>
Gerrit-CC: Jack Cheng <jack.cheng(a)ecs.corp-partner.google.com>
Gerrit-CC: Raymond Chung <raymondchung(a)ami.corp-partner.google.com>
Gerrit-CC: Simon Yang <simon1.yang(a)intel.com>
Gerrit-Attention: Henry Sun <henrysun(a)google.com>
Gerrit-Attention: Super Ni <super.ni(a)intel.corp-partner.google.com>
Gerrit-Attention: Teddy Shih <teddyshihau(a)gmail.com>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Simon Yang <simon1.yang(a)intel.com>
Gerrit-Attention: Aseda Aboagye <aaboagye(a)google.com>
Gerrit-Attention: Simon Yang <simon1.yang(a)intel.corp-partner.google.com>
Gerrit-Attention: Ivan Chen <yulunchen(a)google.com>
Gerrit-Attention: Karthik Ramasubramanian <kramasub(a)google.com>
Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Comment-Date: Wed, 04 May 2022 04:56:24 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Comment-In-Reply-To: Simon Yang <simon1.yang(a)intel.com>
Gerrit-MessageType: comment
Attention is currently required from: Nico Huber, Paul Menzel, Arthur Heymans, Werner Zeh.
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/63982 )
Change subject: soc/intel/cmn/spi: Add ACPI SSDT extension for fast SPI
......................................................................
Patch Set 2:
(1 comment)
Patchset:
PS2:
@werner: what you think about porting UART changes into SPI with proper PNP ID based on the PCI device type.
https://github.com/coreboot/coreboot/blob/b470361e025a59cbdd4bca1631ce6082f…
I guess your problem statement is valid for any PCI device that gets hidden by FSP UPD policy during boot hence, OS is unable to discover the device unless exposed using ASL.
--
To view, visit https://review.coreboot.org/c/coreboot/+/63982
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ifa89cdf41f42d4df5b46f095e22924157d9f3c3f
Gerrit-Change-Number: 63982
Gerrit-PatchSet: 2
Gerrit-Owner: Werner Zeh <werner.zeh(a)siemens.com>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
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-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Attention: Werner Zeh <werner.zeh(a)siemens.com>
Gerrit-Comment-Date: Wed, 04 May 2022 04:48:48 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Paul Menzel, Tim Wawrzynczak, Ivy Jian, Julius Werner, Eric Lai.
Reka Norman has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/63774 )
Change subject: lib/spd: Demote log about using default DDR4 params to NOTICE
......................................................................
Patch Set 5:
(1 comment)
Patchset:
PS1:
> Done
Sorry, I feel like this is my fault. There's a bug assigned to me to check whether the DDR4 values are still correct for LPDDR5 (http://b/223341399). I just haven't gotten around to it and I've been out for the last two weeks so I didn't see this thread.
I see that CB:63680 already added a case for LPDDR5, treating it the same as DDR4, but I still want to go through and double check this is correct. I'll get on to it soon.
As for this CL, I completely agree with Julius that it's good to have a test to alert us that the memory type is unsupported and force us to go through and check that the values are still correct for the new memory type. I don't have a strong opinion on exactly what the log level should be, e.g. I see your point that people should still be paying attention to warnings. But I think we should make sure it's still caught by the test. I'm not familiar with what this test does. Does it just check there are no ERROR messages in the coreboot log?
--
To view, visit https://review.coreboot.org/c/coreboot/+/63774
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I55f0968b78baaa2fc9a6bbebf6712fb8bfd349f6
Gerrit-Change-Number: 63774
Gerrit-PatchSet: 5
Gerrit-Owner: Eric Lai <eric_lai(a)quanta.corp-partner.google.com>
Gerrit-Reviewer: Ivy Jian <ivy.jian(a)quanta.corp-partner.google.com>
Gerrit-Reviewer: Kangheui Won <khwon(a)chromium.org>
Gerrit-Reviewer: Reka Norman <rekanorman(a)chromium.org>
Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Julius Werner <jwerner(a)chromium.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Attention: Ivy Jian <ivy.jian(a)quanta.corp-partner.google.com>
Gerrit-Attention: Julius Werner <jwerner(a)chromium.org>
Gerrit-Attention: Eric Lai <eric_lai(a)quanta.corp-partner.google.com>
Gerrit-Comment-Date: Wed, 04 May 2022 03:55:45 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Comment-In-Reply-To: Julius Werner <jwerner(a)chromium.org>
Comment-In-Reply-To: Eric Lai <eric_lai(a)quanta.corp-partner.google.com>
Gerrit-MessageType: comment