coreboot-gerrit September 2024

coreboot-gerrit@coreboot.org

1 participants
3342 discussions

[S] Change in coreboot[main]: soc/intel/{adl,mtl}: Don't set up SPD on LPDDRx
by Subrata Banik (Code Review) 04 Sep '24

04 Sep '24

Attention is currently required from: Arthur Heymans, Dinesh Gehlot, Eran Mitrani, Jakub Czapiga, Jayvik Desai, Kapil Porwal, Nick Vaccaro, Rishika Raj, Tarun. Subrata Banik has posted comments on this change by Arthur Heymans. ( https://review.coreboot.org/c/coreboot/+/84205?usp=email ) Change subject: soc/intel/{adl,mtl}: Don't set up SPD on LPDDRx ...................................................................... Patch Set 3: (1 comment) File src/soc/intel/meteorlake/meminit.c: https://review.coreboot.org/c/coreboot/+/84205/comment/4657f917_1adde10c?us… : PS2, Line 209: has_spd = true; > > > > Can you help me understand the purpose of `has_spd`? Does it mean the mainboard has an SPD chip and we'll read the hex file using SMBUS? > > > > > > would need_spd be a better name? LPDDRx DRAM technology does not use SPD and the code for SPD in soc/intel/common is doing doing undefined behavior like overflowing an array. > > > > Yes, that name sounds better. I'd like to suggest adding a comment that explains the purpose of this variable. People often confuse the SPD chip with the SPD hex data file. The code should make it clear that the SPD chip isn't required to read the SPD hex file (need_spd). > > No actually it's about SPD data and this code is wrong. The soc/intel/common code is simply wrong for LPDDRx. The loop over channels and dimms per channel is overflowing. There are no DIMMs in LPDDRx which is what need to be addressed. okay, so when you set `has_spd` or `need_spd`, are you saying that we will read SPD data over SMBUS and for lpddrx, we shall pass SPD hex file ? -- To view, visit https://review.coreboot.org/c/coreboot/+/84205?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: Ib3d4ed8032bb06b6d08fbc2dc4b697df88745243 Gerrit-Change-Number: 84205 Gerrit-PatchSet: 3 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Reviewer: Eran Mitrani <mitrani(a)google.com> Gerrit-Reviewer: Jakub Czapiga <czapiga(a)google.com> Gerrit-Reviewer: Jayvik Desai <jayvik(a)google.com> Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Reviewer: Rishika Raj <rishikaraj(a)google.com> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Tarun <tstuli(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Jayvik Desai <jayvik(a)google.com> Gerrit-Attention: Rishika Raj <rishikaraj(a)google.com> Gerrit-Attention: Eran Mitrani <mitrani(a)google.com> Gerrit-Attention: Jakub Czapiga <czapiga(a)google.com> Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Attention: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Attention: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Tarun <tstuli(a)gmail.com> Gerrit-Comment-Date: Wed, 04 Sep 2024 06:50:56 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com> Comment-In-Reply-To: Arthur Heymans <arthur(a)aheymans.xyz>

1 0

[XS] Change in coreboot[main]: src/include: Introduce a new BIT_FLAG_32(x) macro
by Saurabh Mishra (Code Review) 04 Sep '24

04 Sep '24

Attention is currently required from: Ashish Kumar Mishra, Julius Werner, Jérémy Compostella, Karthik Ramasubramanian, Martin Roth, Pratikkumar V Prajapati, Subrata Banik. Saurabh Mishra has posted comments on this change by Saurabh Mishra. ( https://review.coreboot.org/c/coreboot/+/84142?usp=email ) Change subject: src/include: Introduce a new BIT_FLAG_32(x) macro ...................................................................... Patch Set 17: (3 comments) Commit Message: https://review.coreboot.org/c/coreboot/+/84142/comment/f623aeb9_5631856d?us… : PS17, Line 11: 1ul is 64 bits wide > Is it true even for 32 bit systems? On most 32-bit systems, unsigned long (1ul) is 32 bits wide. But nn most 64-bit systems (using the LP64 model), unsigned long (1ul) is 64 bits wide. File src/include/types.h: https://review.coreboot.org/c/coreboot/+/84142/comment/90b1dcc1_ef43d899?us… : PS17, Line 26: defined > used Done https://review.coreboot.org/c/coreboot/+/84142/comment/9f39c467_dec5ce24?us… : PS17, Line 27: * used a different name. > s/use/used/ Acknowledged -- To view, visit https://review.coreboot.org/c/coreboot/+/84142?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: I70be1ccba59d25af2ba85a2014232072abf2f87d Gerrit-Change-Number: 84142 Gerrit-PatchSet: 17 Gerrit-Owner: Saurabh Mishra <mishra.saurabh(a)intel.com> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> 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-CC: Anil Kumar K <anil.kumar.k(a)intel.com> Gerrit-CC: Ashish Kumar Mishra <ashish.k.mishra(a)intel.com> Gerrit-CC: Balaji Manigandan <balaji.manigandan(a)intel.com> Gerrit-CC: Bora Guvendik <bora.guvendik(a)intel.com> Gerrit-CC: Hannah Williams <hannah.williams(a)intel.com> Gerrit-CC: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-CC: Krishna P Bhat D <krishna.p.bhat.d(a)intel.com> Gerrit-CC: Martin Roth <martin.roth(a)amd.corp-partner.google.com> Gerrit-CC: Pratikkumar V Prajapati <pratikkumar.v.prajapati(a)intel.com> Gerrit-CC: Ronak Kanabar <ronak.kanabar(a)intel.com> Gerrit-CC: Sanju Jose Thottan <sanjujose.thottan(a)intel.com> Gerrit-CC: Saurabh Mishra <mishra.saurabh(a)intel.corp-partner.google.com> Gerrit-CC: Wonkyu Kim <wonkyu.kim(a)intel.com> Gerrit-Attention: Ashish Kumar Mishra <ashish.k.mishra(a)intel.com> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Attention: Julius Werner <jwerner(a)chromium.org> Gerrit-Attention: Martin Roth <martin.roth(a)amd.corp-partner.google.com> Gerrit-Attention: Pratikkumar V Prajapati <pratikkumar.v.prajapati(a)intel.com> Gerrit-Attention: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Comment-Date: Wed, 04 Sep 2024 06:48:20 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Jérémy Compostella <jeremy.compostella(a)intel.com> Comment-In-Reply-To: Karthik Ramasubramanian <kramasub(a)google.com>

1 0

[XS] Change in coreboot[main]: src/include: Introduce a new BIT_FLAG_32(x) macro
by Saurabh Mishra (Code Review) 04 Sep '24

04 Sep '24

Attention is currently required from: Ashish Kumar Mishra, Julius Werner, Martin Roth, Pratikkumar V Prajapati, Saurabh Mishra, Subrata Banik. Hello Julius Werner, Jérémy Compostella, Subrata Banik, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/84142?usp=email to look at the new patch set (#18). The following approvals got outdated and were removed: Verified+1 by build bot (Jenkins) Change subject: src/include: Introduce a new BIT_FLAG_32(x) macro ...................................................................... src/include: Introduce a new BIT_FLAG_32(x) macro Introduces the BIT_FLAG_32(x) macro to create a 32-bit mask with the designated bit set. This ensures compatibility with the 32-bit 'GEN_PMCON_A' register on 64-bit systems, where 1ul is 64 bits wide and could potentially cause an overflow when shifted beyond 31 bits. Change-Id: I70be1ccba59d25af2ba85a2014232072abf2f87d Signed-off-by: Saurabh Mishra <mishra.saurabh(a)intel.com> --- M src/include/types.h 1 file changed, 9 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/42/84142/18 -- To view, visit https://review.coreboot.org/c/coreboot/+/84142?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: I70be1ccba59d25af2ba85a2014232072abf2f87d Gerrit-Change-Number: 84142 Gerrit-PatchSet: 18 Gerrit-Owner: Saurabh Mishra <mishra.saurabh(a)intel.com> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> 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-CC: Anil Kumar K <anil.kumar.k(a)intel.com> Gerrit-CC: Ashish Kumar Mishra <ashish.k.mishra(a)intel.com> Gerrit-CC: Balaji Manigandan <balaji.manigandan(a)intel.com> Gerrit-CC: Bora Guvendik <bora.guvendik(a)intel.com> Gerrit-CC: Hannah Williams <hannah.williams(a)intel.com> Gerrit-CC: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-CC: Krishna P Bhat D <krishna.p.bhat.d(a)intel.com> Gerrit-CC: Martin Roth <martin.roth(a)amd.corp-partner.google.com> Gerrit-CC: Pratikkumar V Prajapati <pratikkumar.v.prajapati(a)intel.com> Gerrit-CC: Ronak Kanabar <ronak.kanabar(a)intel.com> Gerrit-CC: Sanju Jose Thottan <sanjujose.thottan(a)intel.com> Gerrit-CC: Saurabh Mishra <mishra.saurabh(a)intel.corp-partner.google.com> Gerrit-CC: Wonkyu Kim <wonkyu.kim(a)intel.com> Gerrit-Attention: Saurabh Mishra <mishra.saurabh(a)intel.com> Gerrit-Attention: Ashish Kumar Mishra <ashish.k.mishra(a)intel.com> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Julius Werner <jwerner(a)chromium.org> Gerrit-Attention: Martin Roth <martin.roth(a)amd.corp-partner.google.com> Gerrit-Attention: Pratikkumar V Prajapati <pratikkumar.v.prajapati(a)intel.com>

1 0

[S] Change in coreboot[main]: soc/intel/{adl,mtl}: Don't set up SPD on LPDDRx
by Arthur Heymans (Code Review) 04 Sep '24

04 Sep '24

Attention is currently required from: Dinesh Gehlot, Eran Mitrani, Jakub Czapiga, Jayvik Desai, Kapil Porwal, Nick Vaccaro, Rishika Raj, Subrata Banik, Tarun. Arthur Heymans has posted comments on this change by Arthur Heymans. ( https://review.coreboot.org/c/coreboot/+/84205?usp=email ) Change subject: soc/intel/{adl,mtl}: Don't set up SPD on LPDDRx ...................................................................... Patch Set 3: (1 comment) File src/soc/intel/meteorlake/meminit.c: https://review.coreboot.org/c/coreboot/+/84205/comment/7a4d84d1_cf1abaea?us… : PS2, Line 209: has_spd = true; > > > Can you help me understand the purpose of `has_spd`? Does it mean the mainboard has an SPD chip and we'll read the hex file using SMBUS? > > > > would need_spd be a better name? LPDDRx DRAM technology does not use SPD and the code for SPD in soc/intel/common is doing doing undefined behavior like overflowing an array. > > Yes, that name sounds better. I'd like to suggest adding a comment that explains the purpose of this variable. People often confuse the SPD chip with the SPD hex data file. The code should make it clear that the SPD chip isn't required to read the SPD hex file (need_spd). No actually it's about SPD data and this code is wrong. The soc/intel/common code is simply wrong for LPDDRx. The loop over channels and dimms per channel is overflowing. There are no DIMMs in LPDDRx which is what need to be addressed. -- To view, visit https://review.coreboot.org/c/coreboot/+/84205?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: Ib3d4ed8032bb06b6d08fbc2dc4b697df88745243 Gerrit-Change-Number: 84205 Gerrit-PatchSet: 3 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Reviewer: Eran Mitrani <mitrani(a)google.com> Gerrit-Reviewer: Jakub Czapiga <czapiga(a)google.com> Gerrit-Reviewer: Jayvik Desai <jayvik(a)google.com> Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Reviewer: Rishika Raj <rishikaraj(a)google.com> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Tarun <tstuli(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Jayvik Desai <jayvik(a)google.com> Gerrit-Attention: Rishika Raj <rishikaraj(a)google.com> Gerrit-Attention: Eran Mitrani <mitrani(a)google.com> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Jakub Czapiga <czapiga(a)google.com> Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Attention: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Attention: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Attention: Tarun <tstuli(a)gmail.com> Gerrit-Comment-Date: Wed, 04 Sep 2024 06:44:17 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com> Comment-In-Reply-To: Arthur Heymans <arthur(a)aheymans.xyz>

1 0

[S] Change in coreboot[main]: soc/intel/{adl,mtl}: Don't set up SPD on LPDDRx
by Subrata Banik (Code Review) 04 Sep '24

04 Sep '24

Attention is currently required from: Arthur Heymans, Dinesh Gehlot, Eran Mitrani, Jakub Czapiga, Jayvik Desai, Kapil Porwal, Nick Vaccaro, Rishika Raj, Tarun. Subrata Banik has posted comments on this change by Arthur Heymans. ( https://review.coreboot.org/c/coreboot/+/84205?usp=email ) Change subject: soc/intel/{adl,mtl}: Don't set up SPD on LPDDRx ...................................................................... Patch Set 3: (1 comment) File src/soc/intel/meteorlake/meminit.c: https://review.coreboot.org/c/coreboot/+/84205/comment/9379bb02_7a29c0cd?us… : PS2, Line 209: has_spd = true; > > Can you help me understand the purpose of `has_spd`? Does it mean the mainboard has an SPD chip and we'll read the hex file using SMBUS? > > would need_spd be a better name? LPDDRx DRAM technology does not use SPD and the code for SPD in soc/intel/common is doing doing undefined behavior like overflowing an array. Yes, that name sounds better. I'd like to suggest adding a comment that explains the purpose of this variable. People often confuse the SPD chip with the SPD hex data file. The code should make it clear that the SPD chip isn't required to read the SPD hex file (need_spd). -- To view, visit https://review.coreboot.org/c/coreboot/+/84205?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: Ib3d4ed8032bb06b6d08fbc2dc4b697df88745243 Gerrit-Change-Number: 84205 Gerrit-PatchSet: 3 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Reviewer: Eran Mitrani <mitrani(a)google.com> Gerrit-Reviewer: Jakub Czapiga <czapiga(a)google.com> Gerrit-Reviewer: Jayvik Desai <jayvik(a)google.com> Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Reviewer: Rishika Raj <rishikaraj(a)google.com> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Tarun <tstuli(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Jayvik Desai <jayvik(a)google.com> Gerrit-Attention: Rishika Raj <rishikaraj(a)google.com> Gerrit-Attention: Eran Mitrani <mitrani(a)google.com> Gerrit-Attention: Jakub Czapiga <czapiga(a)google.com> Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Attention: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Attention: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Tarun <tstuli(a)gmail.com> Gerrit-Comment-Date: Wed, 04 Sep 2024 06:39:38 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com> Comment-In-Reply-To: Arthur Heymans <arthur(a)aheymans.xyz>

1 0

[XS] Change in coreboot[main]: FIXME remove asserts for mt8186 code
by Yu-Ping Wu (Code Review) 04 Sep '24

04 Sep '24

Attention is currently required from: Arthur Heymans, Hung-Te Lin, Yidi Lin. Yu-Ping Wu has posted comments on this change by Arthur Heymans. ( https://review.coreboot.org/c/coreboot/+/84208?usp=email ) Change subject: FIXME remove asserts for mt8186 code ...................................................................... Patch Set 3: Code-Review+2 (2 comments) Commit Message: https://review.coreboot.org/c/coreboot/+/84208/comment/260d9531_7cbc42a3?us… : PS2, Line 7: remove > > Doesn't this patch "add" assertions? […] Right, sorry. I must have looked at the diff in the reverse way. https://review.coreboot.org/c/coreboot/+/84208/comment/183a7cbf_38463ba9?us… : PS2, Line 7: FIXME > > Why this tag? […] Got it. Could you please paste the error messages here? -- To view, visit https://review.coreboot.org/c/coreboot/+/84208?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: I6185e87a374f8722dba545d6bbce1c3a8de53e7e Gerrit-Change-Number: 84208 Gerrit-PatchSet: 3 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Hung-Te Lin <hungte(a)chromium.org> 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-Attention: Hung-Te Lin <hungte(a)chromium.org> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Yidi Lin <yidilin(a)google.com> Gerrit-Comment-Date: Wed, 04 Sep 2024 06:37:48 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: Arthur Heymans <arthur(a)aheymans.xyz> Comment-In-Reply-To: Yu-Ping Wu <yupingso(a)google.com>

1 0

[S] Change in coreboot[main]: soc/intel/common/systemagent: Add Kconfig item HAVE_TSEG_LIMIT_REGISTER
by Name of user not set (Code Review) 04 Sep '24

04 Sep '24

Attention is currently required from: 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/+/84200?usp=email ) Change subject: soc/intel/common/systemagent: Add Kconfig item HAVE_TSEG_LIMIT_REGISTER ...................................................................... Patch Set 2: (3 comments) Commit Message: https://review.coreboot.org/c/coreboot/+/84200/comment/e1612075_63f973e8?us… : PS1, Line 9: Systemagent assumes GSM region is next to TSEG region, but some SoC may > System agent Done https://review.coreboot.org/c/coreboot/+/84200/comment/f6bb815a_e072a246?us… : PS1, Line 11: On such platforms, TSEG region is limited by the TSEG limit register, > defined Done https://review.coreboot.org/c/coreboot/+/84200/comment/c935c23a_49d4076b?us… : PS1, Line 12: and the default offset for that is (TSEG + 4). > I guess you are meaning the register offset of TSEG limit is TSEG base + 4, right? Yes -- To view, visit https://review.coreboot.org/c/coreboot/+/84200?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: I6cb4fbecc1dbafc770d3809a75d05917a141a9af Gerrit-Change-Number: 84200 Gerrit-PatchSet: 2 Gerrit-Owner: yuchi.chen(a)intel.com Gerrit-Reviewer: Shuo Liu <shuo.liu(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Shuo Liu <shuo.liu(a)intel.com> Gerrit-Comment-Date: Wed, 04 Sep 2024 06:36:21 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Shuo Liu <shuo.liu(a)intel.com>

1 0

[M] Change in coreboot[main]: soc/mediatek/mt8196: Add unmask eint event for bootblock
by Yu-Ping Wu (Code Review) 04 Sep '24

04 Sep '24

Attention is currently required from: Hung-Te Lin, Jarried Lin, Yidi Lin. Yu-Ping Wu has posted comments on this change by Jarried Lin. ( https://review.coreboot.org/c/coreboot/+/84025?usp=email ) Change subject: soc/mediatek/mt8196: Add unmask eint event for bootblock ...................................................................... Patch Set 14: (1 comment) File src/soc/mediatek/mt8196/eint_event.c: https://review.coreboot.org/c/coreboot/+/84025/comment/9dfb139c_3e145a80?us… : PS14, Line 16: static void eint_clear_event_mask(uintptr_t base, unsigned int port_num) > This function looks quite similar to the common `unmask_eint_event_mask`. […] More precisely: ``` // common/eint_event.c #define MASK_CLEAR_OFFSET 0x880 #define MASK_CLEAR_VALUE 0xFFFFFFFF void eint_clear_event_mask(size_t port_num) { int i; struct eint_event_reg *reg = (void *)(EINT_BASE + MASK_CLEAR_OFFSET); if (port_num == 0) port_num = ARRAY_SIZE(reg->eint_event_mask_clr); assert(port_num <= ARRAY_SIZE(reg->eint_event_mask_clr)); for (i = 0; i < port_num; i++) write32(&reg->eint_event_mask_clr[i], MASK_CLEAR_VALUE); } void unmask_eint_event_mask(void) { eint_clear_event_mask(0); } // mt8196/eint_event.c void unmask_eint_event_mask(void) { for (...) eint_clear_event_mask(); } ``` Does that make sense? -- To view, visit https://review.coreboot.org/c/coreboot/+/84025?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: I4bf3820a89172186b8f51591f8760787affbb7a3 Gerrit-Change-Number: 84025 Gerrit-PatchSet: 14 Gerrit-Owner: Jarried Lin <jarried.lin(a)mediatek.com> Gerrit-Reviewer: Hung-Te Lin <hungte(a)chromium.org> 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-Attention: Hung-Te Lin <hungte(a)chromium.org> Gerrit-Attention: Jarried Lin <jarried.lin(a)mediatek.com> Gerrit-Attention: Yidi Lin <yidilin(a)google.com> Gerrit-Comment-Date: Wed, 04 Sep 2024 06:35:16 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Yu-Ping Wu <yupingso(a)google.com>

1 0

[M] Change in coreboot[main]: include/spd_bin.h: Add SPD IO layer
by Name of user not set (Code Review) 04 Sep '24

04 Sep '24

Attention is currently required from: yuchi.chen(a)intel.com. Hello Shuo Liu, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/84201?usp=email to look at the new patch set (#3). Change subject: include/spd_bin.h: Add SPD IO layer ...................................................................... include/spd_bin.h: Add SPD IO layer By default, PCH SMBus code will be called to retrieve SPD data. This patch add a SPD IO layer so that SoC could implement its specific SPD IO layer functions such as using Integrated Memory Controller to get SPD data. Change-Id: I656298aeda409fca3c85266b5b8727fac9bfc917 Signed-off-by: Yuchi Chen <yuchi.chen(a)intel.com> --- M src/include/spd_bin.h M src/soc/intel/common/block/smbus/Makefile.mk M src/soc/intel/common/block/smbus/smbuslib.c A src/soc/intel/common/block/smbus/spd_access.c 4 files changed, 40 insertions(+), 15 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/01/84201/3 -- To view, visit https://review.coreboot.org/c/coreboot/+/84201?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: I656298aeda409fca3c85266b5b8727fac9bfc917 Gerrit-Change-Number: 84201 Gerrit-PatchSet: 3 Gerrit-Owner: yuchi.chen(a)intel.com Gerrit-Reviewer: Shuo Liu <shuo.liu(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: yuchi.chen(a)intel.com

1 0

[S] Change in coreboot[main]: soc/intel/common/systemagent: Fixup systemagent address
by Name of user not set (Code Review) 04 Sep '24

04 Sep '24

Attention is currently required from: Arthur Heymans, Felix Held, Felix Singer, Jérémy Compostella, yuchi.chen(a)intel.com. Hello Jérémy Compostella, Shuo Liu, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/83318?usp=email to look at the new patch set (#17). Change subject: soc/intel/common/systemagent: Fixup systemagent address ...................................................................... soc/intel/common/systemagent: Fixup systemagent address System agent in Intel common block (1) assumes TOLUD and TOUUD registers hold the max available address plus 1, but on some SoC like Snow Ridge, it holds the max available address; (2) aligns TOLUD, TOUUD and TSEG registers to 1 MiB default, but some SoC may have different alignments. This patch add a new function soc_systemagent_fixup_address() to handle this special cases. Change-Id: If32c2a6524c9d55ce7f9c3dd203bcf85cab76c2c Signed-off-by: Yuchi Chen <yuchi.chen(a)intel.com> --- M src/soc/intel/common/block/include/intelblocks/systemagent.h M src/soc/intel/common/block/systemagent/Kconfig M src/soc/intel/common/block/systemagent/systemagent.c M src/soc/intel/common/block/systemagent/systemagent_early.c 4 files changed, 38 insertions(+), 6 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/18/83318/17 -- To view, visit https://review.coreboot.org/c/coreboot/+/83318?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: If32c2a6524c9d55ce7f9c3dd203bcf85cab76c2c Gerrit-Change-Number: 83318 Gerrit-PatchSet: 17 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: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-CC: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-CC: Felix Singer <service+coreboot-gerrit(a)felixsinger.de> Gerrit-CC: Vasiliy Khoruzhick <vasilykh(a)arista.com> Gerrit-Attention: Felix Singer <service+coreboot-gerrit(a)felixsinger.de> Gerrit-Attention: yuchi.chen(a)intel.com Gerrit-Attention: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de>

1 0

Jump to page:

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit September 2024