Attention is currently required from: Eric Lai, Kapil Porwal, Pranava Y N, Subrata Banik.
Jérémy Compostella has posted comments on this change by Subrata Banik. ( https://review.coreboot.org/c/coreboot/+/84236?usp=email )
Change subject: soc/intel/cmn/block/cpu: Simplify calculation of non-eviction ways
......................................................................
Patch Set 3:
(1 comment)
File src/soc/intel/common/block/cpu/car/cache_as_ram.S:
https://review.coreboot.org/c/coreboot/+/84236/comment/c5320d51_72295a3f?us… :
PS1, Line 519: add %edx, %eax
> > I find this a bit suspicous, let's take an example: with `CONFIG_DCACHE_RAM_SIZE` at 0x200000 and […]
Seems better, did you run a debugger to verify that the registers through this logic ?
Now we 3 instructions instead of 3 with an extra label. I had a version like this originally that I discarded as I preferred to not introduce a new label.
I let you decide if you want to proceed with this. I don't see any sinificant improvement with this new version but I am not opposed to it.
--
To view, visit https://review.coreboot.org/c/coreboot/+/84236?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: I7cf5ff19ec440d049edc3bf52c660dea96b1f08a
Gerrit-Change-Number: 84236
Gerrit-PatchSet: 3
Gerrit-Owner: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: Eric Lai <ericllai(a)google.com>
Gerrit-Reviewer: Jérémy Compostella <jeremy.compostella(a)intel.com>
Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Reviewer: Pranava Y N <pranavayn(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: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Attention: Pranava Y N <pranavayn(a)google.com>
Gerrit-Comment-Date: Mon, 09 Sep 2024 16:48:58 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com>
Comment-In-Reply-To: Jérémy Compostella <jeremy.compostella(a)intel.com>
Attention is currently required from: Anil Kumar K, Bora Guvendik, Jamie Ryu, Jeremy Compostella, Wonkyu Kim.
Jérémy Compostella has posted comments on this change by Wonkyu Kim. ( https://review.coreboot.org/c/coreboot/+/84229?usp=email )
Change subject: libpayload: add more condition to check valid PCI device id
......................................................................
Patch Set 3:
(3 comments)
File payloads/libpayload/drivers/usb/usbinit.c:
https://review.coreboot.org/c/coreboot/+/84229/comment/7ee00b07_0c17a665?us… :
PS3, Line 119: int bus, int dev, int fun
Why can't it take a `pcidev_t` ?
https://review.coreboot.org/c/coreboot/+/84229/comment/1fe36e4c_3578883a?us… :
PS3, Line 130: (did == 0x0000) || (did == 0xffff)) {
It should fit in one line.
https://review.coreboot.org/c/coreboot/+/84229/comment/d5f3689c_a8929ee3?us… :
PS3, Line 147: if ( is_valid_pci_dev(bus, dev, 0) == false)
The implementation is other parts of the lib is as follow:
```
if (val == 0xffffffff || val == 0x00000000 ||
val == 0x0000ffff || val == 0xffff0000)
```
Could we use this instead ? Could we even make a shared function for it ?
--
To view, visit https://review.coreboot.org/c/coreboot/+/84229?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: I772c4199c7a6c13a25590a36f1bfee17c1a44daf
Gerrit-Change-Number: 84229
Gerrit-PatchSet: 3
Gerrit-Owner: Wonkyu Kim <wonkyu.kim(a)intel.com>
Gerrit-Reviewer: Anil Kumar K <anil.kumar.k(a)intel.com>
Gerrit-Reviewer: Bora Guvendik <bora.guvendik(a)intel.com>
Gerrit-Reviewer: Jamie Ryu <jamie.m.ryu(a)intel.com>
Gerrit-Reviewer: Jeremy Compostella <jeremy.compostella(a)gmail.com>
Gerrit-Reviewer: Jérémy Compostella <jeremy.compostella(a)intel.com>
Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Bora Guvendik <bora.guvendik(a)intel.com>
Gerrit-Attention: Anil Kumar K <anil.kumar.k(a)intel.com>
Gerrit-Attention: Jamie Ryu <jamie.m.ryu(a)intel.com>
Gerrit-Attention: Wonkyu Kim <wonkyu.kim(a)intel.com>
Gerrit-Attention: Jeremy Compostella <jeremy.compostella(a)gmail.com>
Gerrit-Comment-Date: Mon, 09 Sep 2024 16:42:23 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Attention is currently required from: Anil Kumar K, Bora Guvendik, Jamie Ryu, Jeremy Compostella, Jérémy Compostella, Wonkyu Kim.
Paul Menzel has posted comments on this change by Wonkyu Kim. ( https://review.coreboot.org/c/coreboot/+/84210?usp=email )
Change subject: src/intel/cmm/block: Update sa_get_tseg_size function
......................................................................
Patch Set 2:
(4 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/84210/comment/05768fe2_a952f9ae?us… :
PS2, Line 7: Update sa_get_tseg_size function
Please be more specific.
https://review.coreboot.org/c/coreboot/+/84210/comment/fc27611e_159f2c0e?us… :
PS2, Line 10: nagative
negative
https://review.coreboot.org/c/coreboot/+/84210/comment/c943e5fc_2d509aae?us… :
PS2, Line 10: api
API
File src/soc/intel/common/block/systemagent/systemagent_early.c:
https://review.coreboot.org/c/coreboot/+/84210/comment/67313a9d_7c36fcbd?us… :
PS2, Line 155: sa_get_gsm_base() - sa_get_tseg_base()
Would an assert be useful?
--
To view, visit https://review.coreboot.org/c/coreboot/+/84210?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: Ic3d5175add1f7dc6e2c9f1c38133b36ffc59e789
Gerrit-Change-Number: 84210
Gerrit-PatchSet: 2
Gerrit-Owner: Wonkyu Kim <wonkyu.kim(a)intel.com>
Gerrit-Reviewer: Anil Kumar K <anil.kumar.k(a)intel.com>
Gerrit-Reviewer: Bora Guvendik <bora.guvendik(a)intel.com>
Gerrit-Reviewer: Jamie Ryu <jamie.m.ryu(a)intel.com>
Gerrit-Reviewer: Jeremy Compostella <jeremy.compostella(a)gmail.com>
Gerrit-Reviewer: Jérémy Compostella <jeremy.compostella(a)intel.com>
Gerrit-Reviewer: Pratikkumar V Prajapati <pratikkumar.v.prajapati(a)intel.com>
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: Bora Guvendik <bora.guvendik(a)intel.com>
Gerrit-Attention: Anil Kumar K <anil.kumar.k(a)intel.com>
Gerrit-Attention: Jamie Ryu <jamie.m.ryu(a)intel.com>
Gerrit-Attention: Wonkyu Kim <wonkyu.kim(a)intel.com>
Gerrit-Attention: Jeremy Compostella <jeremy.compostella(a)gmail.com>
Gerrit-Attention: Jérémy Compostella <jeremy.compostella(a)intel.com>
Gerrit-Comment-Date: Mon, 09 Sep 2024 16:32:58 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Attention is currently required from: Sean Rhodes.
Paul Menzel has posted comments on this change by Sean Rhodes. ( https://review.coreboot.org/c/coreboot/+/83879?usp=email )
Change subject: ec/starlabs/merlin: Add Intel Virtual Button driver
......................................................................
Patch Set 3:
(8 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/83879/comment/984f8baa_d02a7cfd?us… :
PS3, Line 15: Ubuntu
Which version?
https://review.coreboot.org/c/coreboot/+/83879/comment/7c552914_0bdbf459?us… :
PS3, Line 16: show
show*n*
https://review.coreboot.org/c/coreboot/+/83879/comment/bfa0c2e1_3339c64a?us… :
PS3, Line 15: Tested on `starlite_adl` with Ubuntu, by checking the
: virtual keyboard is show when the tablet is undocked
: and hidden when docked.
Can it be tested by looking at some logs?
https://review.coreboot.org/c/coreboot/+/83879/comment/ab2f2b4b_fc6c5aff?us… :
PS3, Line 9: Add Intel Virtual Button driver which is used to report
: to the OS whether a tablet is docked or undocked.
:
: This is currently only used on `mb/starlite_adl` so the
: GPIO is hardcoded to GPP_F15 for now.
:
: Tested on `starlite_adl` with Ubuntu, by checking the
: virtual keyboard is show when the tablet is undocked
: and hidden when docked.
Please try to use 72 characters per line.
File src/ec/starlabs/merlin/acpi/battery.asl:
PS3:
This is from a different commit, isn’t it?
File src/ec/starlabs/merlin/acpi/dock.asl:
PS3:
There is already:
1. `src/mainboard/purism/librem_jsl/acpi/vbtn.asl`
2. `src/ec/google/wilco/acpi/vbtn.asl`
Should it be at least named like this? Any possibilities to generalize the driver?
File src/ec/starlabs/merlin/acpi/ec.asl:
https://review.coreboot.org/c/coreboot/+/83879/comment/2ea24e94_aef2f756?us… :
PS3, Line 53: #if CONFIG(SOC_INTEL_COMMON)
: \LIDS = 0x03
: #endif
Please comment in the commit message why this is done, or add a comment?
File src/ec/starlabs/merlin/acpi/hid.asl:
https://review.coreboot.org/c/coreboot/+/83879/comment/d14cea88_9f8bc1b8?us… :
PS3, Line 352: Case (0x08)
: {
: Return (\_SB.PCI0.LPCB.EC.VBTN.VGBS())
: }
Please mention this in the commit message?
--
To view, visit https://review.coreboot.org/c/coreboot/+/83879?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: I574a1b2d3907b2341a0dfdc412151d574ba4848e
Gerrit-Change-Number: 83879
Gerrit-PatchSet: 3
Gerrit-Owner: Sean Rhodes <sean(a)starlabs.systems>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Sean Rhodes <sean(a)starlabs.systems>
Gerrit-Comment-Date: Mon, 09 Sep 2024 16:30:46 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Attention is currently required from: Anil Kumar K, Bora Guvendik, Felix Held, Hannah Williams, Jamie Ryu, Subrata Banik.
Cliff Huang has posted comments on this change by Cliff Huang. ( https://review.coreboot.org/c/coreboot/+/84104?usp=email )
Change subject: soc/intel/common/block/pmc: Add GPE1 functions
......................................................................
Patch Set 11:
(1 comment)
File src/soc/intel/common/block/pmc/pmclib.c:
https://review.coreboot.org/c/coreboot/+/84104/comment/dbab8a04_66523be5?us… :
PS11, Line 68: /* SoC overrides for GPE1 when SOC_INTEL_COMMON_BLOCK_ACPI_HAVE_GPE1 is enabled */
: __weak const char *const *soc_std_gpe1_sts_array(int idx, size_t *a)
: {
: return NULL;
: }
:
: /* disable the corresponding GPE1 bits based on standard GPE0 bits */
: __weak void soc_pmc_disable_std_gpe1(uint32_t gpe0_mask)
: {
: }
:
: /* enable the corresponding GPE1 bits based on standard GPE0 bits */
: __weak void soc_pmc_enable_std_gpe1(uint32_t gpe0_mask)
: {
: }
> when a soc selects SOC_INTEL_COMMON_BLOCK_ACPI_HAVE_GPE1, it has to implement those 3 functions in t […]
These __weak functions are meant for SOCs with no SOC_INTEL_COMMON_BLOCK_ACPI_HAVE_GPE1 being being set. Thyy are still needed when we compile since we don't use SOC_INTEL_COMMON_BLOCK_ACPI_HAVE_GPE1 as compilation to exclude these calls.
--
To view, visit https://review.coreboot.org/c/coreboot/+/84104?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: I7ac1fbe6d45cbe0c86c3f72911900d92a186168d
Gerrit-Change-Number: 84104
Gerrit-PatchSet: 11
Gerrit-Owner: Cliff Huang <cliff.huang(a)intel.com>
Gerrit-Reviewer: Anil Kumar K <anil.kumar.k(a)intel.com>
Gerrit-Reviewer: Bora Guvendik <bora.guvendik(a)intel.com>
Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-CC: Hannah Williams <hannah.williams(a)intel.com>
Gerrit-CC: Jamie Ryu <jamie.m.ryu(a)intel.com>
Gerrit-Attention: Bora Guvendik <bora.guvendik(a)intel.com>
Gerrit-Attention: Hannah Williams <hannah.williams(a)intel.com>
Gerrit-Attention: Anil Kumar K <anil.kumar.k(a)intel.com>
Gerrit-Attention: Jamie Ryu <jamie.m.ryu(a)intel.com>
Gerrit-Attention: Subrata Banik <subratabanik(a)google.com>
Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Comment-Date: Mon, 09 Sep 2024 16:24:46 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com>
Comment-In-Reply-To: Felix Held <felix-coreboot(a)felixheld.de>
Attention is currently required from: Anil Kumar K, Bora Guvendik, Felix Held, Hannah Williams, Jamie Ryu, Subrata Banik.
Cliff Huang has posted comments on this change by Cliff Huang. ( https://review.coreboot.org/c/coreboot/+/84104?usp=email )
Change subject: soc/intel/common/block/pmc: Add GPE1 functions
......................................................................
Patch Set 11:
(1 comment)
File src/soc/intel/common/block/include/intelblocks/pmclib.h:
https://review.coreboot.org/c/coreboot/+/84104/comment/762b1e16_f57edbf2?us… :
PS11, Line 241: gpe0_mask
> > shouldn't this be gpe1_mask? same below […]
it is gpe0_mask indeed, as all GPE1 events are finer granularity of some GPE0 events, which included TCSS/non-TCSS PME_B0, hot plug, PCIe events. For instance, if GPE0 PME_EN is disabled, the [internal device]_PME_EN bits will also needs to be disabled. Same thing apply to hot plug and PCIe events. There is no additional types of event added in GPE1. The purpose of GPE1 events is for performance matter. Instead of waking up all device and finding out which one is the event source. GPE1 events is checked directly.
--
To view, visit https://review.coreboot.org/c/coreboot/+/84104?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: I7ac1fbe6d45cbe0c86c3f72911900d92a186168d
Gerrit-Change-Number: 84104
Gerrit-PatchSet: 11
Gerrit-Owner: Cliff Huang <cliff.huang(a)intel.com>
Gerrit-Reviewer: Anil Kumar K <anil.kumar.k(a)intel.com>
Gerrit-Reviewer: Bora Guvendik <bora.guvendik(a)intel.com>
Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-CC: Hannah Williams <hannah.williams(a)intel.com>
Gerrit-CC: Jamie Ryu <jamie.m.ryu(a)intel.com>
Gerrit-Attention: Bora Guvendik <bora.guvendik(a)intel.com>
Gerrit-Attention: Hannah Williams <hannah.williams(a)intel.com>
Gerrit-Attention: Anil Kumar K <anil.kumar.k(a)intel.com>
Gerrit-Attention: Jamie Ryu <jamie.m.ryu(a)intel.com>
Gerrit-Attention: Subrata Banik <subratabanik(a)google.com>
Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Comment-Date: Mon, 09 Sep 2024 16:21:08 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com>
Comment-In-Reply-To: Felix Held <felix-coreboot(a)felixheld.de>
Attention is currently required from: Anil Kumar K, Bora Guvendik, Cliff Huang, Jamie Ryu, Lance Zhao, Tim Wawrzynczak, Wonkyu Kim.
Jérémy Compostella has posted comments on this change by Wonkyu Kim. ( https://review.coreboot.org/c/coreboot/+/84230?usp=email )
Change subject: make same cpu pyhsical address
......................................................................
Patch Set 2:
(1 comment)
Patchset:
PS2:
If I remember well, CPU Physical Address size and SoC physic Address size can be different. Like on MTL CPU Phisical Address size is 46 but only 42 can be used at the SoC level. This is the reason we introduced this difference in the coreboot code.
--
To view, visit https://review.coreboot.org/c/coreboot/+/84230?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: I06c485511fa7f72a445c8aed56d8a470b4772092
Gerrit-Change-Number: 84230
Gerrit-PatchSet: 2
Gerrit-Owner: Wonkyu Kim <wonkyu.kim(a)intel.com>
Gerrit-Reviewer: Anil Kumar K <anil.kumar.k(a)intel.com>
Gerrit-Reviewer: Bora Guvendik <bora.guvendik(a)intel.com>
Gerrit-Reviewer: Cliff Huang <cliff.huang(a)intel.com>
Gerrit-Reviewer: Jamie Ryu <jamie.m.ryu(a)intel.com>
Gerrit-Reviewer: Jérémy Compostella <jeremy.compostella(a)intel.com>
Gerrit-Reviewer: Lance Zhao <lance.zhao(a)gmail.com>
Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: Tim Wawrzynczak <inforichland(a)gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Bora Guvendik <bora.guvendik(a)intel.com>
Gerrit-Attention: Lance Zhao <lance.zhao(a)gmail.com>
Gerrit-Attention: Anil Kumar K <anil.kumar.k(a)intel.com>
Gerrit-Attention: Cliff Huang <cliff.huang(a)intel.com>
Gerrit-Attention: Jamie Ryu <jamie.m.ryu(a)intel.com>
Gerrit-Attention: Wonkyu Kim <wonkyu.kim(a)intel.com>
Gerrit-Attention: Tim Wawrzynczak <inforichland(a)gmail.com>
Gerrit-Comment-Date: Mon, 09 Sep 2024 16:13:26 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Elyes Haouas has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/84277?usp=email )
Change subject: edid.h: Increase EDID_ASCII_STRING_LENGTH to 14
......................................................................
edid.h: Increase EDID_ASCII_STRING_LENGTH to 14
This fixes following errors (using GCC-15):
CC+STRIP src/drivers/mipi/panel-KD_KD101NE3_40TI.c
CC+STRIP src/drivers/mipi/panel-LCE_LMFBX101117480.c
src/drivers/mipi/panel-LCE_LMFBX101117480.c:7:33: error: initializer-string for array of 'char' is too long [-Werror=unterminated-string-initialization]
7 | .ascii_string = "LMFBX101117480",
| ^~~~~~~~~~~~~~~~
Change-Id: I48196ba7f7cd8520dbf7f62385b7d83012ad617f
Signed-off-by: Elyes Haouas <ehaouas(a)noos.fr>
---
M src/include/edid.h
1 file changed, 1 insertion(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/77/84277/1
diff --git a/src/include/edid.h b/src/include/edid.h
index b2d6aa2..a21609e 100644
--- a/src/include/edid.h
+++ b/src/include/edid.h
@@ -44,7 +44,7 @@
* unsigned int. We can move more into this struct as needed.
*/
-#define EDID_ASCII_STRING_LENGTH 13
+#define EDID_ASCII_STRING_LENGTH 14
struct edid {
/* These next three things used to all be called bpp.
--
To view, visit https://review.coreboot.org/c/coreboot/+/84277?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: I48196ba7f7cd8520dbf7f62385b7d83012ad617f
Gerrit-Change-Number: 84277
Gerrit-PatchSet: 1
Gerrit-Owner: Elyes Haouas <ehaouas(a)noos.fr>
Attention is currently required from: Anil Kumar K, Bora Guvendik, Cliff Huang, Hannah Williams, Jamie Ryu, Subrata Banik.
Felix Held has posted comments on this change by Cliff Huang. ( https://review.coreboot.org/c/coreboot/+/84104?usp=email )
Change subject: soc/intel/common/block/pmc: Add GPE1 functions
......................................................................
Patch Set 11:
(2 comments)
File src/soc/intel/common/block/pmc/pmclib.c:
PS11:
> > are the gpe1 registers for the same event as the gpe0 register with the same number? the code usin […]
so i'd guess that it's not correct to use the identical mask pmc_enable_std_gpe and pmc_disable_std_gpe to enable/disable the GPE0 and GPE1
https://review.coreboot.org/c/coreboot/+/84104/comment/1b37ef77_afbebd8b?us… :
PS11, Line 68: /* SoC overrides for GPE1 when SOC_INTEL_COMMON_BLOCK_ACPI_HAVE_GPE1 is enabled */
: __weak const char *const *soc_std_gpe1_sts_array(int idx, size_t *a)
: {
: return NULL;
: }
:
: /* disable the corresponding GPE1 bits based on standard GPE0 bits */
: __weak void soc_pmc_disable_std_gpe1(uint32_t gpe0_mask)
: {
: }
:
: /* enable the corresponding GPE1 bits based on standard GPE0 bits */
: __weak void soc_pmc_enable_std_gpe1(uint32_t gpe0_mask)
: {
: }
> > are those weak functions needed? i'd assume that if a soc selects SOC_INTEL_COMMON_BLOCK_ACPI_HAVE […]
when a soc selects SOC_INTEL_COMMON_BLOCK_ACPI_HAVE_GPE1, it has to implement those 3 functions in the soc code; when a soc doesn't select SOC_INTEL_COMMON_BLOCK_ACPI_HAVE_GPE1, all calls of those functions get optimized out, so i don't think that there's the need to have a weak implementation of those
--
To view, visit https://review.coreboot.org/c/coreboot/+/84104?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: I7ac1fbe6d45cbe0c86c3f72911900d92a186168d
Gerrit-Change-Number: 84104
Gerrit-PatchSet: 11
Gerrit-Owner: Cliff Huang <cliff.huang(a)intel.com>
Gerrit-Reviewer: Anil Kumar K <anil.kumar.k(a)intel.com>
Gerrit-Reviewer: Bora Guvendik <bora.guvendik(a)intel.com>
Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-CC: Hannah Williams <hannah.williams(a)intel.com>
Gerrit-CC: Jamie Ryu <jamie.m.ryu(a)intel.com>
Gerrit-Attention: Bora Guvendik <bora.guvendik(a)intel.com>
Gerrit-Attention: Hannah Williams <hannah.williams(a)intel.com>
Gerrit-Attention: Anil Kumar K <anil.kumar.k(a)intel.com>
Gerrit-Attention: Cliff Huang <cliff.huang(a)intel.com>
Gerrit-Attention: Jamie Ryu <jamie.m.ryu(a)intel.com>
Gerrit-Attention: Subrata Banik <subratabanik(a)google.com>
Gerrit-Comment-Date: Mon, 09 Sep 2024 15:46:27 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com>
Comment-In-Reply-To: Felix Held <felix-coreboot(a)felixheld.de>