Attention is currently required from: Kapil Porwal, Paul Menzel, Pranava Y N, Subrata Banik.
Cliff Huang has posted comments on this change by Cliff Huang. ( https://review.coreboot.org/c/coreboot/+/84297?usp=email )
Change subject: soc/intel/ptl: Add GPE1 defines
......................................................................
Patch Set 9:
(1 comment)
File src/soc/intel/pantherlake/include/soc/pm.h:
https://review.coreboot.org/c/coreboot/+/84297/comment/4b26953b_92415e34?us… :
PS9, Line 133: #if CONFIG(SOC_INTEL_COMMON_BLOCK_ACPI_HAVE_GPE1)
> Sure. Let me a new CL for this and remove this CPP.
Subrata, we will hit the GPE1_* marco redefined build error if CONFIG_SOC_INTEL_COMMON_BLOCK_ACPI_HAVE_GPE1 is not set in PTL. I recall I brought up this issue.
--
To view, visit https://review.coreboot.org/c/coreboot/+/84297?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: Iebf6f6d02b37cc9702e4ee07c1ec0017b6628836
Gerrit-Change-Number: 84297
Gerrit-PatchSet: 9
Gerrit-Owner: Cliff Huang <cliff.huang(a)intel.com>
Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Reviewer: Pranava Y N <pranavayn(a)google.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: Subrata Banik <subratabanik(a)google.com>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Attention: Pranava Y N <pranavayn(a)google.com>
Gerrit-Comment-Date: Mon, 16 Sep 2024 18:38:56 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Cliff Huang <cliff.huang(a)intel.com>
Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com>
Attention is currently required from: Cliff Huang.
Subrata Banik has posted comments on this change by Cliff Huang. ( https://review.coreboot.org/c/coreboot/+/84392?usp=email )
The change is no longer submittable: All-Comments-Resolved is unsatisfied now.
Change subject: soc/intel/common/block/acpi: Fix GPE1 blocks to ACPI FADT table
......................................................................
Patch Set 1:
(2 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/84392/comment/419ad8db_10817664?us… :
PS1, Line 12: 362310295
b:362310295
https://review.coreboot.org/c/coreboot/+/84392/comment/cd24942f_75cb65ce?us… :
PS1, Line 13: , boot to
boot google/fatcat or intel/ptlrvp to CrOS
--
To view, visit https://review.coreboot.org/c/coreboot/+/84392?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: Idd8115044faff3161ea6bd1cae6c0fe8aa0ff8d7
Gerrit-Change-Number: 84392
Gerrit-PatchSet: 1
Gerrit-Owner: Cliff Huang <cliff.huang(a)intel.com>
Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Cliff Huang <cliff.huang(a)intel.com>
Gerrit-Comment-Date: Mon, 16 Sep 2024 18:17:56 +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 21:
(1 comment)
File src/soc/intel/common/block/include/intelblocks/pmclib.h:
https://review.coreboot.org/c/coreboot/+/84104/comment/cd8caa28_40122182?us… :
PS11, Line 241: gpe0_mask
> > > @subratabanik@google.com I do not see use case for pmc_enable_std_gpe and pmc_disable_std_gpe. […]
sure. on my list.
--
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: 21
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-CC: Paul Menzel <paulepanter(a)mailbox.org>
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, 16 Sep 2024 18:17:06 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Hannah Williams <hannah.williams(a)intel.com>
Comment-In-Reply-To: Cliff Huang <cliff.huang(a)intel.com>
Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com>
Comment-In-Reply-To: Felix Held <felix-coreboot(a)felixheld.de>
Cliff Huang has posted comments on this change by Cliff Huang. ( https://review.coreboot.org/c/coreboot/+/84103?usp=email )
Change subject: soc/intel/common/block/acpi: Add GPE1 blocks to ACPI FADT table
......................................................................
Patch Set 15:
(1 comment)
File src/soc/intel/common/block/acpi/acpi.c:
https://review.coreboot.org/c/coreboot/+/84103/comment/d6ec895d_c82a1d30?us… :
PS15, Line 110: fadt->gpe1_blk = GPE1_STS(0) ? (pmbase + GPE1_STS(0)) : 0;
: if (fadt->gpe1_blk) {
> i find this in patchset 7 much easier to read and understand compared to patchset 15, but since this […]
Felix,
I pushed this CL:
https://review.coreboot.org/c/coreboot/+/84392
--
To view, visit https://review.coreboot.org/c/coreboot/+/84103?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: Ia6928c35b86f4a2243d58597b17b2a3a5f54271e
Gerrit-Change-Number: 84103
Gerrit-PatchSet: 15
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: 9elements QA <hardwaretestrobot(a)gmail.com>
Gerrit-CC: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-CC: Hannah Williams <hannah.williams(a)intel.com>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Comment-Date: Mon, 16 Sep 2024 18:14:48 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Cliff Huang <cliff.huang(a)intel.com>
Comment-In-Reply-To: Felix Held <felix-coreboot(a)felixheld.de>
Attention is currently required from: Kapil Porwal, Paul Menzel, Pranava Y N, Subrata Banik.
Cliff Huang has posted comments on this change by Cliff Huang. ( https://review.coreboot.org/c/coreboot/+/84297?usp=email )
Change subject: soc/intel/ptl: Add GPE1 defines
......................................................................
Patch Set 9:
(1 comment)
File src/soc/intel/pantherlake/include/soc/pm.h:
https://review.coreboot.org/c/coreboot/+/84297/comment/5d3842a9_b7cea1c8?us… :
PS9, Line 133: #if CONFIG(SOC_INTEL_COMMON_BLOCK_ACPI_HAVE_GPE1)
> > Subrata, if these 3 defines are here, we won't be using the defines in . […]
Sure. Let me a new CL for this and remove this CPP.
--
To view, visit https://review.coreboot.org/c/coreboot/+/84297?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: Iebf6f6d02b37cc9702e4ee07c1ec0017b6628836
Gerrit-Change-Number: 84297
Gerrit-PatchSet: 9
Gerrit-Owner: Cliff Huang <cliff.huang(a)intel.com>
Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Reviewer: Pranava Y N <pranavayn(a)google.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: Subrata Banik <subratabanik(a)google.com>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Attention: Pranava Y N <pranavayn(a)google.com>
Gerrit-Comment-Date: Mon, 16 Sep 2024 16:58:28 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Cliff Huang <cliff.huang(a)intel.com>
Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com>
Attention is currently required from: Cliff Huang, Kapil Porwal, Paul Menzel, Pranava Y N.
Subrata Banik has posted comments on this change by Cliff Huang. ( https://review.coreboot.org/c/coreboot/+/84297?usp=email )
Change subject: soc/intel/ptl: Add GPE1 defines
......................................................................
Patch Set 9:
(1 comment)
File src/soc/intel/pantherlake/include/soc/pm.h:
https://review.coreboot.org/c/coreboot/+/84297/comment/6ef7b961_3ce2fc5d?us… :
PS9, Line 133: #if CONFIG(SOC_INTEL_COMMON_BLOCK_ACPI_HAVE_GPE1)
> Subrata, if these 3 defines are here, we won't be using the defines in ./soc/intel/common/block/include/intelblocks/pmclib.h
>
> #if !CONFIG(SOC_INTEL_COMMON_BLOCK_ACPI_HAVE_GPE1)
> #define GPE1_STS(x) (0x0 + ((x) * 4))
> #define GPE1_EN(x) (0x0 + ((x) * 4))
> #define GPE1_REG_MAX 0
> #endif
>
> and ending up we will add GEP1 blocks in FADT.
You won't be using the code in FADT if you follow the suggetion shared by Felix (https://review.coreboot.org/c/coreboot/+/84103/comment/f0a82554_c1c0c50b/)
ideally GPE1_STATUS can be non-zero for PTL as this is SoC feature but a platform can decide (mainboard) if wish to use SOC_INTEL_COMMON_BLOCK_ACPI_HAVE_GPE1 or not. In that way
```
fadt->gpe1_blk = 0;
if (CONFIG(SOC_INTEL_COMMON_BLOCK_ACPI_HAVE_GPE1)) {
fadt->gpe1_blk = pmbase + GPE1_STS(0);
fadt->gpe1_blk_len = 2 * GPE1_REG_MAX * sizeof(uint32_t);
/*
* NOTE: gpe1 is after gpe0, which has _STS and _EN register sets.
* gpe1_base is the starting bit offset for GPE1.
*/
fadt->gpe1_base = fadt->gpe0_blk_len / 2 * 8;
}
```
--
To view, visit https://review.coreboot.org/c/coreboot/+/84297?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: Iebf6f6d02b37cc9702e4ee07c1ec0017b6628836
Gerrit-Change-Number: 84297
Gerrit-PatchSet: 9
Gerrit-Owner: Cliff Huang <cliff.huang(a)intel.com>
Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Reviewer: Pranava Y N <pranavayn(a)google.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: Cliff Huang <cliff.huang(a)intel.com>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Attention: Pranava Y N <pranavayn(a)google.com>
Gerrit-Comment-Date: Mon, 16 Sep 2024 16:45:33 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Cliff Huang <cliff.huang(a)intel.com>
Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com>
Attention is currently required from: Maximilian Brune.
Ana Carolina Cabral has posted comments on this change by Maximilian Brune. ( https://review.coreboot.org/c/coreboot/+/84375?usp=email )
Change subject: soc/amd/glinda: Update pci int defs
......................................................................
Patch Set 1:
(1 comment)
File src/soc/amd/glinda/fch.c:
https://review.coreboot.org/c/coreboot/+/84375/comment/9255bfd2_dda7ad5d?us… :
PS1, Line 65: { PIRQ_UART2, "UART2" },
PIRQ_GSCI and PIRQ_GSMI missing from amd_pci_int_defs.h
--
To view, visit https://review.coreboot.org/c/coreboot/+/84375?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: I843e5e2b01301eb02cb5be347e122cffbe76d80d
Gerrit-Change-Number: 84375
Gerrit-PatchSet: 1
Gerrit-Owner: Maximilian Brune <maximilian.brune(a)9elements.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Ana Carolina Cabral
Gerrit-Attention: Maximilian Brune <maximilian.brune(a)9elements.com>
Gerrit-Comment-Date: Mon, 16 Sep 2024 16:41:55 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Attention is currently required from: Kapil Porwal, Paul Menzel, Pranava Y N, Subrata Banik.
Cliff Huang has posted comments on this change by Cliff Huang. ( https://review.coreboot.org/c/coreboot/+/84297?usp=email )
Change subject: soc/intel/ptl: Add GPE1 defines
......................................................................
Patch Set 9:
(3 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/84297/comment/d1b5d4d2_353fdb17?us… :
PS9, Line 12: STD
> standard?
STD GPE have been used for Intel's GPE bits in this case. I can add more here.
https://review.coreboot.org/c/coreboot/+/84297/comment/d8fc487f_b2bcf7dd?us… :
PS9, Line 12: NOTE: All GEP1 bits are STD GPE bits.
> Shouldn’t this then go into some common directory?
GPE1 implementation is SOC specific.
File src/soc/intel/pantherlake/include/soc/pm.h:
https://review.coreboot.org/c/coreboot/+/84297/comment/071e11d8_5fdafe04?us… :
PS9, Line 133: #if CONFIG(SOC_INTEL_COMMON_BLOCK_ACPI_HAVE_GPE1)
> > Subrata, we are still switching in between using GPE0 and GPE1 for further validation until we ver […]
Subrata, if these 3 defines are here, we won't be using the defines in ./soc/intel/common/block/include/intelblocks/pmclib.h
#if !CONFIG(SOC_INTEL_COMMON_BLOCK_ACPI_HAVE_GPE1)
#define GPE1_STS(x) (0x0 + ((x) * 4))
#define GPE1_EN(x) (0x0 + ((x) * 4))
#define GPE1_REG_MAX 0
#endif
and ending up we will add GEP1 blocks in FADT.
--
To view, visit https://review.coreboot.org/c/coreboot/+/84297?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: Iebf6f6d02b37cc9702e4ee07c1ec0017b6628836
Gerrit-Change-Number: 84297
Gerrit-PatchSet: 9
Gerrit-Owner: Cliff Huang <cliff.huang(a)intel.com>
Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Reviewer: Pranava Y N <pranavayn(a)google.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: Subrata Banik <subratabanik(a)google.com>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Attention: Pranava Y N <pranavayn(a)google.com>
Gerrit-Comment-Date: Mon, 16 Sep 2024 16:40:10 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Cliff Huang <cliff.huang(a)intel.com>
Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com>
Comment-In-Reply-To: Paul Menzel <paulepanter(a)mailbox.org>