Attention is currently required from: Cliff Huang, Jérémy Compostella, 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 6:
(1 comment)
File src/soc/intel/pantherlake/include/soc/gpe.h:
https://review.coreboot.org/c/coreboot/+/84297/comment/4195fc14_130a301b?us… :
PS4, Line 30: 146
> Subrata,
>
> do you mean to create macros like these?
>
> #define GPE1_START_NUM 0x80
> #define GEP1_BIT(n) (GPE_START_NUM + (n))
>
> #define GPE1_CNVI_BT_PME_B0 GPE1_BIT(18) /* 146 -> _L92 */
> ....
that is much better with a comment to explain what does magic number 0x80 refers at ?
--
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: 6
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-Attention: Cliff Huang <cliff.huang(a)intel.com>
Gerrit-Attention: Jérémy Compostella <jeremy.compostella(a)intel.com>
Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Attention: Pranava Y N <pranavayn(a)google.com>
Gerrit-Comment-Date: Fri, 13 Sep 2024 08:23:46 +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: Jérémy Compostella, 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 6:
(1 comment)
File src/soc/intel/pantherlake/include/soc/gpe.h:
https://review.coreboot.org/c/coreboot/+/84297/comment/293f0097_1a38e472?us… :
PS4, Line 30: 146
> > GPE1[0] starts from 0x80 (i.e. […]
true. Although the GPE1 could be placed starting after 0x80, we don't have much choice but put GPE1 right after GPE0. The maximum GPE event number is 256. GPE0 occupies 128 across all SOCs and PTL GPE1 has three 32-bit blocks, so only another 32-bit left that a SOC can extend in the future. It should be okay to always put it immediately after GPE0. But, detailed comment will be helpful to explain how the numbers are derived from. In addition, there are several unused reserved bits in between GPE1 bits, so they are not incremental numbers. Short comments are placed at the end to indicate its corresponding _L[nn] event method name.
--
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: 6
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-Attention: Subrata Banik <subratabanik(a)google.com>
Gerrit-Attention: Jérémy Compostella <jeremy.compostella(a)intel.com>
Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Attention: Pranava Y N <pranavayn(a)google.com>
Gerrit-Comment-Date: Fri, 13 Sep 2024 08:05:23 +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: Matt DeVillier, Nick Vaccaro.
Paul Menzel has posted comments on this change by Matt DeVillier. ( https://review.coreboot.org/c/coreboot/+/84348?usp=email )
The change is no longer submittable: All-Comments-Resolved is unsatisfied now.
Change subject: mb/google/volteer: Fix USB port definitions
......................................................................
Patch Set 5: Code-Review+1
(2 comments)
Patchset:
PS5:
Is there a generic solution for this problem? Any idea how the fix would look like?
Commit Message:
https://review.coreboot.org/c/coreboot/+/84348/comment/ef5b842c_ca5e28ad?us… :
PS5, Line 16: function
function*al*?
--
To view, visit https://review.coreboot.org/c/coreboot/+/84348?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: I54921fa4ecf594a1ecbcfa7c45e5d745d4a95652
Gerrit-Change-Number: 84348
Gerrit-PatchSet: 5
Gerrit-Owner: Matt DeVillier <matt.devillier(a)gmail.com>
Gerrit-Reviewer: Elyes Haouas <ehaouas(a)noos.fr>
Gerrit-Reviewer: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)chromium.org>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Matt DeVillier <matt.devillier(a)gmail.com>
Gerrit-Attention: Nick Vaccaro <nvaccaro(a)chromium.org>
Gerrit-Comment-Date: Fri, 13 Sep 2024 07:41:14 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Attention is currently required from: Cliff Huang, Jérémy Compostella.
Paul Menzel has posted comments on this change by Cliff Huang. ( https://review.coreboot.org/c/coreboot/+/84349?usp=email )
Change subject: soc/intel/common/block/acpi/exclude DMI fixed memory range if no DMI
......................................................................
Patch Set 3:
(4 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/84349/comment/4ff7d96e_3973e924?us… :
PS3, Line 7: soc/intel/common/block/acpi/exclude DMI fixed memory range if no DMI
I think the colon is missing to separate the prefix from the summary.
https://review.coreboot.org/c/coreboot/+/84349/comment/e716a49e_176c5b8d?us… :
PS3, Line 9: exclude DMI in northbridge.asl if DMI_BASE_SIZE is '0'
Why?
https://review.coreboot.org/c/coreboot/+/84349/comment/0ec34e92_d32f8814?us… :
PS3, Line 9: exclude
Exclude
https://review.coreboot.org/c/coreboot/+/84349/comment/0a7c4210_4b39be59?us… :
PS3, Line 12: Verified on Intel® Simics® Pre Silicon Simulation platform
: for PTL using google/fatcat mainboard.
:
How? Was booting not possible before, and now it is?
--
To view, visit https://review.coreboot.org/c/coreboot/+/84349?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: I971af2eb214b5940fa09d9dc0f9717bb5f0dfb4d
Gerrit-Change-Number: 84349
Gerrit-PatchSet: 3
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-CC: Hannah Williams <hannah.williams(a)intel.com>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Cliff Huang <cliff.huang(a)intel.com>
Gerrit-Attention: Jérémy Compostella <jeremy.compostella(a)intel.com>
Gerrit-Comment-Date: Fri, 13 Sep 2024 07:39:13 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Subrata Banik has submitted this change. ( https://review.coreboot.org/c/coreboot/+/84236?usp=email )
Change subject: soc/intel/cmn/block/cpu: Simplify calculation of non-eviction ways
......................................................................
soc/intel/cmn/block/cpu: Simplify calculation of non-eviction ways
The calculation of non-eviction ways (used for cache-as-ram
configuration) has been simplified by removing conditional move
instructions and directly adding the remainder to the quotient.
This achieves the same ceiling operation but with potentially improved
efficiency (less instructions).
No functional changes are expected.
TEST=Able to build and boot google/rex.
Change-Id: I7cf5ff19ec440d049edc3bf52c660dea96b1f08a
Signed-off-by: Subrata Banik <subratabanik(a)google.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/84236
Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org>
Reviewed-by: Kapil Porwal <kapilporwal(a)google.com>
Reviewed-by: Jérémy Compostella <jeremy.compostella(a)intel.com>
---
M src/soc/intel/common/block/cpu/car/cache_as_ram.S
1 file changed, 5 insertions(+), 5 deletions(-)
Approvals:
Jérémy Compostella: Looks good to me, but someone else must approve
Kapil Porwal: Looks good to me, approved
build bot (Jenkins): Verified
diff --git a/src/soc/intel/common/block/cpu/car/cache_as_ram.S b/src/soc/intel/common/block/cpu/car/cache_as_ram.S
index 90da9e7..c1af882 100644
--- a/src/soc/intel/common/block/cpu/car/cache_as_ram.S
+++ b/src/soc/intel/common/block/cpu/car/cache_as_ram.S
@@ -513,13 +513,13 @@
xor %edx, %edx /* Clear the upper 32-bit of dividend */
div %ecx
/*
- * Increment data_ways by 1 if RW data size (CONFIG_DCACHE_RAM_SIZE) is
+ * Effectively ceiling the result if RW data size (CONFIG_DCACHE_RAM_SIZE) is
* not divisible by way_size (ECX)
*/
- movl $0x01, %ecx
- cmp $0x00, %edx
- cmovne %ecx, %edx
- add %edx, %eax
+ testl %edx, %edx
+ jz skip_increment
+ incl %eax
+skip_increment:
mov %eax, %edx /* back up data_ways in edx */
mov %eax, %ecx
movl $0x01, %eax
--
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: merged
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I7cf5ff19ec440d049edc3bf52c660dea96b1f08a
Gerrit-Change-Number: 84236
Gerrit-PatchSet: 5
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: Nick Vaccaro <nvaccaro(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: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Attention is currently required from: Matt DeVillier, Nick Vaccaro.
Felix Singer has posted comments on this change by Matt DeVillier. ( https://review.coreboot.org/c/coreboot/+/84348?usp=email )
Change subject: mb/google/volteer: Fix USB port definitions
......................................................................
Patch Set 5: Code-Review+2
--
To view, visit https://review.coreboot.org/c/coreboot/+/84348?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: I54921fa4ecf594a1ecbcfa7c45e5d745d4a95652
Gerrit-Change-Number: 84348
Gerrit-PatchSet: 5
Gerrit-Owner: Matt DeVillier <matt.devillier(a)gmail.com>
Gerrit-Reviewer: Elyes Haouas <ehaouas(a)noos.fr>
Gerrit-Reviewer: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Matt DeVillier <matt.devillier(a)gmail.com>
Gerrit-Attention: Nick Vaccaro <nvaccaro(a)chromium.org>
Gerrit-Comment-Date: Fri, 13 Sep 2024 07:02:00 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Attention is currently required from: Hung-Te Lin, Kiwi Liu, Paul Menzel, Yidi Lin, Yu-Ping Wu.
Mengqi Zhang has posted comments on this change by Kiwi Liu. ( https://review.coreboot.org/c/coreboot/+/84298?usp=email )
Change subject: soc/mediatek/common: Correct src clk frq to 400 MHz for eMMMC clk of 400 kHz
......................................................................
Patch Set 15:
(8 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/84298/comment/a8dae5a1_b8a70943?us… :
PS14, Line 7: Reduce eMMC clock frequency to 400kHz
> Correct src clk frq to 400 MHz for eMMMC clk of 400 kHz
Done
https://review.coreboot.org/c/coreboot/+/84298/comment/3c879739_a73e0962?us… :
PS14, Line 10: power-on.
> power-on due to wrong src_hz value.
Done
https://review.coreboot.org/c/coreboot/+/84298/comment/0710f6bd_b1779c00?us… :
PS14, Line 11: When we need to set a clock output frequency, we actually set a
> leave one blank line above.
Done
https://review.coreboot.org/c/coreboot/+/84298/comment/e42662eb_716ec1c8?us… :
PS14, Line 13: he frequency
: division value to 125
> I don't think the origin code will give us the 125 value due to the division rounding error. […]
Done
https://review.coreboot.org/c/coreboot/+/84298/comment/fedb8840_5aeba80a?us… :
PS14, Line 16: 400KHz.
> move to next line.
Done
https://review.coreboot.org/c/coreboot/+/84298/comment/8fc23682_e03b777f?us… :
PS14, Line 16: So we correct source clock frequency to 400MHz for eMMC output clock of 400KHz.
> leave one blank line above.
Done
File src/soc/mediatek/common/msdc.c:
https://review.coreboot.org/c/coreboot/+/84298/comment/cd82c77b_9717deb6?us… :
PS9, Line 432: host->src_hz = 400 * 1000 * 1000;
> According to depthcharge, MT8173's source clock is 200MHz. […]
Done
File src/soc/mediatek/common/msdc.c:
https://review.coreboot.org/c/coreboot/+/84298/comment/9d332d9f_b6ffce93?us… :
PS12, Line 353: msdc_debug("sclk: %d\n", sclk);
> before patch: […]
Done
--
To view, visit https://review.coreboot.org/c/coreboot/+/84298?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: I9c8836b23fb21e9b0bdc80fbe85142ea0fa5e381
Gerrit-Change-Number: 84298
Gerrit-PatchSet: 15
Gerrit-Owner: Kiwi Liu <kiwi.liu(a)mediatek.corp-partner.google.com>
Gerrit-Reviewer: Hung-Te Lin <hungte(a)chromium.org>
Gerrit-Reviewer: Mengqi Zhang <mengqi.zhang(a)mediatek.corp-partner.google.com>
Gerrit-Reviewer: Yidi Lin <yidilin(a)google.com>
Gerrit-Reviewer: Yu-Ping Wu <yupingso(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Hung-Te Lin <hungte(a)chromium.org>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Kiwi Liu <kiwi.liu(a)mediatek.corp-partner.google.com>
Gerrit-Attention: Yu-Ping Wu <yupingso(a)google.com>
Gerrit-Attention: Yidi Lin <yidilin(a)google.com>
Gerrit-Comment-Date: Fri, 13 Sep 2024 06:52:20 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Hung-Te Lin <hungte(a)chromium.org>
Comment-In-Reply-To: Paul Menzel <paulepanter(a)mailbox.org>
Comment-In-Reply-To: Kiwi Liu <kiwi.liu(a)mediatek.corp-partner.google.com>
Comment-In-Reply-To: Mengqi Zhang <mengqi.zhang(a)mediatek.corp-partner.google.com>
Comment-In-Reply-To: Yidi Lin <yidilin(a)google.com>
Comment-In-Reply-To: Yu-Ping Wu <yupingso(a)google.com>