Attention is currently required from: Amanda Hwang, Dinesh Gehlot, Eric Lai, Kapil Porwal, Nick Vaccaro, Subrata Banik.
Hello Dinesh Gehlot, Eric Lai, Ian Feng, Kapil Porwal, Nick Vaccaro, Subrata Banik, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/83324?usp=email
to look at the new patch set (#4).
Change subject: mb/google/brya: Select Intel PDC to PMC CONFIGURATION for orisa
......................................................................
mb/google/brya: Select Intel PDC to PMC CONFIGURATION for orisa
Orisa uses PDC<->PMC direct connection for USBC mux configuration.
Select SOC_INTEL_TCSS_USE_PDC_PMC_USBC_MUX_CONFIGURATION to enable it.
BUG=b:345070027
TEST=emerge-nissa coreboot chromeos-bootimage
Change-Id: I3f740bedc8ff667d15f077fa57d201ab0d42ebf8
Signed-off-by: Amanda Huang <amanda_hwang(a)compal.corp-partner.google.com>
---
M src/mainboard/google/brya/Kconfig
1 file changed, 2 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/24/83324/4
--
To view, visit https://review.coreboot.org/c/coreboot/+/83324?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: I3f740bedc8ff667d15f077fa57d201ab0d42ebf8
Gerrit-Change-Number: 83324
Gerrit-PatchSet: 4
Gerrit-Owner: Amanda Hwang <amanda_hwang(a)compal.corp-partner.google.com>
Gerrit-Reviewer: Dinesh Gehlot <digehlot(a)google.com>
Gerrit-Reviewer: Eric Lai <ericllai(a)google.com>
Gerrit-Reviewer: Ian Feng <ian_feng(a)compal.corp-partner.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: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Eric Lai <ericllai(a)google.com>
Gerrit-Attention: Subrata Banik <subratabanik(a)google.com>
Gerrit-Attention: Dinesh Gehlot <digehlot(a)google.com>
Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Attention: Nick Vaccaro <nvaccaro(a)chromium.org>
Gerrit-Attention: Amanda Hwang <amanda_hwang(a)compal.corp-partner.google.com>
Attention is currently required from: Jérémy Compostella.
Hello Jérémy Compostella,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/83372?usp=email
to look at the new patch set (#2).
Change subject: drivers/pc80/pc/spkmodem.c: Use beep() instead of own implementation
......................................................................
drivers/pc80/pc/spkmodem.c: Use beep() instead of own implementation
Commit 23b79483554f (pc80/i8254: Add speaker beep function) added a
driver to play beeps using the 8254 PIT chip and the PC speaker, which
is essentially the same as the make_tone() function in spkmodem.c. Drop
the duplicate functionality provided by make_tone() in favour of the
common beep() function, which also has a more intuitive signature.
Converting the make_tone() arguments to beep() arguments is not trivial,
as they are different in meaning and requires some background knowledge
of PIT programming, explained briefly here.
The `duration` argument to make_tone() is somewhat misleading, as it is
not used as a direct number of units of time, but rather the number of
PIT counter reloads to wait for. The frequency of the reloads is related
to the `freq_count` argument, which is the value the counter is reloaded
to when it counts down to 0. Since the counter decrements at 1193181 Hz,
dividing this frequency by the desired frequency of the tone gives the
counter reload value, which was used to calculate the freq_count
argument for the calls to make_tone().
However, since the PIT output toggles at every reload, each full cycle
occurs at half the desired frequency. To compensate for this, mode 3
(square wave generator) of the PIT chip actually decrements the counter
by 2 each time so that the counter reloads twice as often. Thus, each
reload occurs every 1 / (frequency * 2) seconds.
Since `duration` counts reloads, multiplying it by the time between
reloads gives the time in seconds, and thus the following formula was
used to convert `duration` into a millisecond value to pass as the
duration_msec argument for beep():
duration_msec = 1000 * duration/(2*frequency)
Change-Id: If95b518c991a0d700489566e5609344ca2cbbc7a
Signed-off-by: Nicholas Chin <nic.c3.14(a)gmail.com>
---
M src/drivers/pc80/pc/spkmodem.c
1 file changed, 6 insertions(+), 83 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/72/83372/2
--
To view, visit https://review.coreboot.org/c/coreboot/+/83372?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: If95b518c991a0d700489566e5609344ca2cbbc7a
Gerrit-Change-Number: 83372
Gerrit-PatchSet: 2
Gerrit-Owner: Nicholas Chin <nic.c3.14(a)gmail.com>
Gerrit-Reviewer: Jérémy Compostella <jeremy.compostella(a)intel.com>
Gerrit-CC: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Jérémy Compostella <jeremy.compostella(a)intel.com>
Nicholas Chin has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/83372?usp=email )
Change subject: drivers/pc80/pc/spkmodem.c: Use beep() instead of own implementation
......................................................................
drivers/pc80/pc/spkmodem.c: Use beep() instead of own implementation
Commit 23b79483554f (pc80/i8254: Add speaker beep function) added a
driver to play beeps using the 8254 PIT chip and the PC speaker, which
is essentially the same as the make_tone() function in spkmodem.c. Drop
the duplicate functionality provided by make_tone() in favour of the
common beep() function, which also has a more intuitive signature.
Converting the make_tone() arguments to beep() arguments is not trivial,
as they are different in meaning and requires some background knowledge
of PIT programming, explained briefly here.
The `duration` argument to make_tone() is somewhat misleading, as it is
not used as a direct number of units of time, but rather the number of
PIT counter reloads to wait for. The frequency of the reloads is related
to the `freq_count` argument, which is the value the counter is reloaded
to when it counts down to 0. Since the counter decrements at 1193181 Hz,
dividing this frequency by the desired frequency of the tone gives the
counter reload value, which was used to calculate the freq_count
argument for the calls to make_tone().
However, since the PIT output toggles at every reload, each full cycle
occurs at half the desired frequency. To compensate for this, mode 3
(square wave generator) of the PIT chip actually decrements the counter
by 2 each time so that the counter reloads twice as often. Thus, each
reload occurs every 1 / (frequency * 2) seconds.
Since `duration` counts reloads, multiplying it by the time between
reloads gives the time in seconds, and thus the following formula was
used to convert `duration` into a millisecond value to pass as the
duration_msec argument for beep():
duration_msec = 1000 * duration/(2*frequency)
Change-Id: If95b518c991a0d700489566e5609344ca2cbbc7a
Signed-off-by: Nicholas Chin <nic.c3.14(a)gmail.com>
---
M src/drivers/pc80/pc/spkmodem.c
1 file changed, 5 insertions(+), 81 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/72/83372/1
diff --git a/src/drivers/pc80/pc/spkmodem.c b/src/drivers/pc80/pc/spkmodem.c
index 9e457f7..5ef0dd1 100644
--- a/src/drivers/pc80/pc/spkmodem.c
+++ b/src/drivers/pc80/pc/spkmodem.c
@@ -2,95 +2,19 @@
#include <arch/io.h>
#include <console/spkmodem.h>
-
-#define SPEAKER_PIT_FREQUENCY 0x1234dd
-
-enum {
- PIT_COUNTER_0 = 0x40,
- PIT_COUNTER_1 = 0x41,
- PIT_COUNTER_2 = 0x42,
- PIT_CTRL = 0x43,
- PIT_SPEAKER_PORT = 0x61,
-};
-
-enum {
- PIT_SPK_TMR2 = 0x01,
- PIT_SPK_DATA = 0x02,
- PIT_SPK_TMR2_LATCH = 0x20
-};
-
-enum {
- PIT_CTRL_SELECT_MASK = 0xc0,
- PIT_CTRL_SELECT_0 = 0x00,
- PIT_CTRL_SELECT_1 = 0x40,
- PIT_CTRL_SELECT_2 = 0x80,
-
- PIT_CTRL_READLOAD_MASK = 0x30,
- PIT_CTRL_COUNTER_LATCH = 0x00,
- PIT_CTRL_READLOAD_LSB = 0x10,
- PIT_CTRL_READLOAD_MSB = 0x20,
- PIT_CTRL_READLOAD_WORD = 0x30,
-
- PIT_CTRL_MODE_MASK = 0x0e,
- PIT_CTRL_INTR_ON_TERM = 0x00,
- PIT_CTRL_PROGR_ONE_SHOT = 0x02,
-
- PIT_CTRL_RATE_GEN = 0x04,
-
- PIT_CTRL_SQUAREWAVE_GEN = 0x06,
- PIT_CTRL_SOFTSTROBE = 0x08,
-
- PIT_CTRL_HARDSTROBE = 0x0a,
-
- PIT_CTRL_COUNT_MASK = 0x01,
- PIT_CTRL_COUNT_BINARY = 0x00,
- PIT_CTRL_COUNT_BCD = 0x01
-};
-
-static void
-make_tone(uint16_t freq_count, unsigned int duration)
-{
- outb(PIT_CTRL_SELECT_2
- | PIT_CTRL_READLOAD_WORD
- | PIT_CTRL_SQUAREWAVE_GEN
- | PIT_CTRL_COUNT_BINARY, PIT_CTRL);
-
- outb(freq_count & 0xff, PIT_COUNTER_2);
-
- outb((freq_count >> 8) & 0xff, PIT_COUNTER_2);
-
- outb(inb(PIT_SPEAKER_PORT)
- | PIT_SPK_TMR2 | PIT_SPK_DATA,
- PIT_SPEAKER_PORT);
-
- for (; duration; duration--) {
- unsigned short counter, previous_counter = 0xffff;
-
- while (1) {
- counter = inb(PIT_COUNTER_2);
- counter |= ((uint16_t)inb(PIT_COUNTER_2)) << 8;
- if (counter > previous_counter) {
- previous_counter = counter;
- break;
- }
- previous_counter = counter;
- }
- }
-}
+#include <pc80/i8254.h>
void spkmodem_tx_byte(unsigned char c)
{
int i;
-
- make_tone(SPEAKER_PIT_FREQUENCY / 200, 4);
+ beep(200, 10);
for (i = 7; i >= 0; i--) {
if ((c >> i) & 1)
- make_tone(SPEAKER_PIT_FREQUENCY / 2000, 20);
+ beep(2000, 5);
else
- make_tone(SPEAKER_PIT_FREQUENCY / 4000, 40);
- make_tone(SPEAKER_PIT_FREQUENCY / 1000, 10);
+ beep(4000, 5);
+ beep(1000, 5);
}
- make_tone(SPEAKER_PIT_FREQUENCY / 200, 0);
}
void spkmodem_init(void)
--
To view, visit https://review.coreboot.org/c/coreboot/+/83372?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: newchange
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: If95b518c991a0d700489566e5609344ca2cbbc7a
Gerrit-Change-Number: 83372
Gerrit-PatchSet: 1
Gerrit-Owner: Nicholas Chin <nic.c3.14(a)gmail.com>
Attention is currently required from: Jérémy Compostella, Paul Menzel, Shuo Liu, yuchi.chen(a)intel.com.
Felix Singer has posted comments on this change by yuchi.chen(a)intel.com. ( https://review.coreboot.org/c/coreboot/+/83192?usp=email )
Change subject: vc/intel/fsp/fsp2_0/snowridge: Add FSP headers for Snow Ridge SoC
......................................................................
Patch Set 4:
(2 comments)
File src/vendorcode/intel/fsp/fsp2_0/snowridge/FspmUpd.h:
https://review.coreboot.org/c/coreboot/+/83192/comment/824d4d3c_f3bd17fd?us… :
PS4, Line 29: This file is automatically generated. Please do NOT modify !!!
> Please add to the commit message, how it was generated.
It's the same procedure for all FSP headers. Nothing new.
https://review.coreboot.org/c/coreboot/+/83192/comment/b9e46537_8b712d60?us… :
PS4, Line 100: This structure holds the DLL configuration
: register values that will be programmed by RC.
: Those policies should be used by platform if default values
: provided by RC are not sufficient to provide stable operation
: at all supported speed modes. RC will blindly set the DLL values
: as provided in this structure.
> The FSP headers are not own by my team, I'm asking my colleagues for help now, please wait for some […]
It's not a big issue. It can be updated with another update of the headers. I'm marking it as resolved.
--
To view, visit https://review.coreboot.org/c/coreboot/+/83192?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: I333b137c1dc08a3c06bdd3f7a78ca44a5dd043cc
Gerrit-Change-Number: 83192
Gerrit-PatchSet: 4
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-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: yuchi.chen(a)intel.com
Gerrit-Attention: Jérémy Compostella <jeremy.compostella(a)intel.com>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Shuo Liu <shuo.liu(a)intel.com>
Gerrit-Comment-Date: Mon, 08 Jul 2024 00:11:12 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: yuchi.chen(a)intel.com
Comment-In-Reply-To: Paul Menzel <paulepanter(a)mailbox.org>
Attention is currently required from: Jérémy Compostella, Paul Menzel, Shuo Liu.
yuchi.chen(a)intel.com has posted comments on this change by yuchi.chen(a)intel.com. ( https://review.coreboot.org/c/coreboot/+/83192?usp=email )
Change subject: vc/intel/fsp/fsp2_0/snowridge: Add FSP headers for Snow Ridge SoC
......................................................................
Patch Set 4:
(1 comment)
File src/vendorcode/intel/fsp/fsp2_0/snowridge/FspmUpd.h:
https://review.coreboot.org/c/coreboot/+/83192/comment/6e3402be_3482d4e2?us… :
PS4, Line 100: This structure holds the DLL configuration
: register values that will be programmed by RC.
: Those policies should be used by platform if default values
: provided by RC are not sufficient to provide stable operation
: at all supported speed modes. RC will blindly set the DLL values
: as provided in this structure.
> Line breaks make it harder to read.
The FSP headers are not own by my team, I'm asking my colleagues for help now, please wait for some time.
--
To view, visit https://review.coreboot.org/c/coreboot/+/83192?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: I333b137c1dc08a3c06bdd3f7a78ca44a5dd043cc
Gerrit-Change-Number: 83192
Gerrit-PatchSet: 4
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: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Jérémy Compostella <jeremy.compostella(a)intel.com>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Shuo Liu <shuo.liu(a)intel.com>
Gerrit-Comment-Date: Mon, 08 Jul 2024 00:00:10 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Paul Menzel <paulepanter(a)mailbox.org>
Attention is currently required from: Felix Singer, Martin L Roth, Nico Huber.
Hello Elyes Haouas, Martin L Roth, Nico Huber, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/83347?usp=email
to look at the new patch set (#6).
Change subject: xcompile: Apply -Wextra with temporary exceptions to GCC
......................................................................
xcompile: Apply -Wextra with temporary exceptions to GCC
In order to detect more issues in our code, make GCC more picky by
enabling -Wextra. Disable a couple of warnings turned on by -Wextra
temporarily in order to keep everything compiling and working for now.
The warnings may be enabled step by step later.
Since xcompiles applies to coreboot and libpayload, add Wextra here
instead of the top-level Makefile.mk.
Change-Id: I60915cb66581dc2c9b6807335fd0e214b45e76d6
Signed-off-by: Felix Singer <felixsinger(a)posteo.net>
---
M util/xcompile/xcompile
1 file changed, 33 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/47/83347/6
--
To view, visit https://review.coreboot.org/c/coreboot/+/83347?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: I60915cb66581dc2c9b6807335fd0e214b45e76d6
Gerrit-Change-Number: 83347
Gerrit-PatchSet: 6
Gerrit-Owner: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-Reviewer: Elyes Haouas <ehaouas(a)noos.fr>
Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Martin L Roth <gaumless(a)gmail.com>
Attention is currently required from: Elyes Haouas, Martin L Roth.
Felix Singer has posted comments on this change by Elyes Haouas. ( https://review.coreboot.org/c/coreboot/+/83364?usp=email )
Change subject: [test] Enable Wparentheses for LLVM
......................................................................
Patch Set 1:
(1 comment)
File util/xcompile/xcompile:
https://review.coreboot.org/c/coreboot/+/83364/comment/2e8b24bb_023548c9?us… :
PS1, Line 265: CLANG_CFLAGS_${TARCH}+=-Qunused-arguments -m${TWIDTH}
: # tone down clang compiler warnings
: CLANG_CFLAGS_${TARCH}+=-Wno-unused-variable -Wno-unused-function -Wno-tautological-compare
: CLANG_CFLAGS_${TARCH}+=-Wno-shift-overflow -Wno-address-of-packed-member -Wno-initializer-overrides
: CL
> While on it, could you please move each option into their own line please?
Please create a separate patch.
--
To view, visit https://review.coreboot.org/c/coreboot/+/83364?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: I346c078988f3afe3487aea02bb91c86fb47c0cb7
Gerrit-Change-Number: 83364
Gerrit-PatchSet: 1
Gerrit-Owner: Elyes Haouas <ehaouas(a)noos.fr>
Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-Attention: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Attention: Elyes Haouas <ehaouas(a)noos.fr>
Gerrit-Comment-Date: Sun, 07 Jul 2024 11:52:21 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>