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>
Attention is currently required from: Cliff Huang, Kapil Porwal, 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/1d7d3fb5_8363be47?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 verify all GPE1 bits are working at this time. here, we will still need this flag here, and also in ASL files and code for SSDT.
i don't agree that you need a CPP here. what you are trying to protect here in a header file by adding CPP guard ? these are registers definition for PTL and where we are selecting `SOC_INTEL_COMMON_BLOCK_ACPI_HAVE_GPE1` or not, these GPE1 definitions are here to stay. Hence, you can drop the line #133 and line #137. Rest looks good to me
--
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: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Attention: Pranava Y N <pranavayn(a)google.com>
Gerrit-Comment-Date: Mon, 16 Sep 2024 16:26: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: Kapil Porwal, 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/e4a93bcb_3e1f18a6?us… :
PS9, Line 133: #if CONFIG(SOC_INTEL_COMMON_BLOCK_ACPI_HAVE_GPE1)
> we can drop this CPP. […]
Subrata, we are still switching in between using GPE0 and GPE1 for further validation until we verify all GPE1 bits are working at this time. here, we will still need this flag here, and also in ASL files and code for SSDT.
--
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: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Attention: Pranava Y N <pranavayn(a)google.com>
Gerrit-Comment-Date: Mon, 16 Sep 2024 16:20:14 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com>
Attention is currently required from: Maximilian Brune.
Felix Held has posted comments on this change by Maximilian Brune. ( https://review.coreboot.org/c/coreboot/+/84382?usp=email )
Change subject: soc/amd/glinda: Update I2C for glinda
......................................................................
Patch Set 1:
(1 comment)
File src/soc/amd/glinda/include/soc/smi.h:
https://review.coreboot.org/c/coreboot/+/84382/comment/52d50e5b_0df155cc?us… :
PS1, Line 88: #define SMITYPE_USB_PD_I2C4 40
would be good to mark those as reserved to be consistent with the rest of the file
--
To view, visit https://review.coreboot.org/c/coreboot/+/84382?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: I676e76aa2309d9ab82d63b48a2dec3c100241131
Gerrit-Change-Number: 84382
Gerrit-PatchSet: 1
Gerrit-Owner: Maximilian Brune <maximilian.brune(a)9elements.com>
Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Maximilian Brune <maximilian.brune(a)9elements.com>
Gerrit-Comment-Date: Mon, 16 Sep 2024 16:16:30 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Attention is currently required from: Maximilian Brune.
Felix Held has posted comments on this change by Maximilian Brune. ( https://review.coreboot.org/c/coreboot/+/84379?usp=email )
Change subject: Update UPD for system configuration, Fast and Slow PPT to set 45W TDP
......................................................................
Patch Set 2: Code-Review-1
(1 comment)
Patchset:
PS2:
this shouldn't be overwritten by a hard-coded value, but specified in the devicetree
--
To view, visit https://review.coreboot.org/c/coreboot/+/84379?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: Ib73c3a1270c7b1d490fd14c4e60f7aa9be93429b
Gerrit-Change-Number: 84379
Gerrit-PatchSet: 2
Gerrit-Owner: Maximilian Brune <maximilian.brune(a)9elements.com>
Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Maximilian Brune <maximilian.brune(a)9elements.com>
Gerrit-Comment-Date: Mon, 16 Sep 2024 16:14:42 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Attention is currently required from: Maximilian Brune.
Felix Held has posted comments on this change by Maximilian Brune. ( https://review.coreboot.org/c/coreboot/+/84381?usp=email )
Change subject: soc/amd/glinda: Update SCI mapping
......................................................................
Patch Set 1:
(1 comment)
File src/soc/amd/glinda/include/soc/pci_devs.h:
https://review.coreboot.org/c/coreboot/+/84381/comment/1c31edf1_bdb80c84?us… :
PS1, Line 68: #define XHCI0_DEV 0x0
: #define XHCI0_FUNC 0
: #define XHCI0_DEVFN PCI_DEVFN(XHCI0_DEV, XHCI0_FUNC)
this one should be moved below the correct internal bus C
--
To view, visit https://review.coreboot.org/c/coreboot/+/84381?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: I5eaed888109b89c25bcf0ba91abefa7c36c1851b
Gerrit-Change-Number: 84381
Gerrit-PatchSet: 1
Gerrit-Owner: Maximilian Brune <maximilian.brune(a)9elements.com>
Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Maximilian Brune <maximilian.brune(a)9elements.com>
Gerrit-Comment-Date: Mon, 16 Sep 2024 16:11:58 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Attention is currently required from: Felix Held, Fred Reitberger, Jason Glenesk.
Matt DeVillier has posted comments on this change by Felix Held. ( https://review.coreboot.org/c/coreboot/+/84355?usp=email )
Change subject: soc/amd/common/psp/psb: add missing newline in debug message
......................................................................
Patch Set 1: Code-Review+2
--
To view, visit https://review.coreboot.org/c/coreboot/+/84355?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: I794899fa55b510e6f39dadc1a831b86389ab31ca
Gerrit-Change-Number: 84355
Gerrit-PatchSet: 1
Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Fred Reitberger <reitbergerfred(a)gmail.com>
Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com>
Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com>
Gerrit-Attention: Fred Reitberger <reitbergerfred(a)gmail.com>
Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Comment-Date: Mon, 16 Sep 2024 15:58:17 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes