coreboot-gerrit August 2024

coreboot-gerrit@coreboot.org

1 participants
3913 discussions

[M] Change in coreboot[main]: soc/intel/common/block/pmc: Add GPE1 functions
by Hannah Williams (Code Review) 28 Aug '24

28 Aug '24

Attention is currently required from: Anil Kumar K, Bora Guvendik, Cliff Huang, Subrata Banik. Hannah Williams 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 3: (1 comment) File src/soc/intel/common/block/pmc/pmclib.c: https://review.coreboot.org/c/coreboot/+/84104/comment/2f8ce13d_23c1ab5c?us… : PS3, Line 393: void pmc_clear_std_gpe1_status Can this be instead pmc_clear_gpe1_status ? Unlike GPE0, you don't need the STD offset to be handled separately here -- 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: 3 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: Hannah Williams <hannah.williams(a)intel.com> Gerrit-Attention: Bora Guvendik <bora.guvendik(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: Subrata Banik <subratabanik(a)google.com> Gerrit-Comment-Date: Wed, 28 Aug 2024 05:44:38 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No

1 0

[S] Change in coreboot[main]: soc/intel/common/block/acpi: Add GPE1 blocks to ACPI FADT table
by Subrata Banik (Code Review) 28 Aug '24

28 Aug '24

Attention is currently required from: Anil Kumar K, Bora Guvendik, Cliff Huang, Hannah Williams. Subrata Banik 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 3: (1 comment) File src/soc/intel/common/block/acpi/acpi.c: https://review.coreboot.org/c/coreboot/+/84103/comment/39abef01_50206d52?us… : PS3, Line 110: SOC_INTEL_COMMON_BLOCK_ACPI_GPE1 suggestion, looks like we don't need a CPP if we follow the below method 1. As ACPI spec, if platform doesn't support `gpe1_blk` then this field should be zero. 2. Define GPE1_STS` for all non-supported SoC as below ``` /* Platform doesn't have support for GPE1, adding dummy entry for common code */ #define GPE1_STS(x) (0x0 + ((x) * 4)) ``` 3. Only supported SoC like PTL would implement something like this ``` #define GPE1_STS(x) (0x10 + ((x) * 4)) ``` 4. update this logic w/o any need for CPP ``` fadt->gpe1_blk = GPE1_STS(0) ? (pmbase + GPE1_STS(0)) : GPE1_STS(0); if (!fadt->gpe1_blk) { 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/+/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: 3 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: Hannah Williams <hannah.williams(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-Comment-Date: Wed, 28 Aug 2024 05:44:11 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No

1 0

[S] Change in coreboot[main]: mb/google/nissa/var/nivviks: Prevent camera LED blinking during boot
by V Sowmya (Code Review) 28 Aug '24

28 Aug '24

Attention is currently required from: Eric Lai, Jayvik Desai, Kapil Porwal, Nick Vaccaro, Rishika Raj, Subrata Banik. V Sowmya has posted comments on this change by V Sowmya. ( https://review.coreboot.org/c/coreboot/+/84019?usp=email ) Change subject: mb/google/nissa/var/nivviks: Prevent camera LED blinking during boot ...................................................................... Patch Set 3: (1 comment) File src/mainboard/google/brya/variants/nivviks/overridetree.cb: https://review.coreboot.org/c/coreboot/+/84019/comment/4e478349_84233aa6?us… : PS3, Line 438: # Controls : register "gpio_panel.gpio[0].gpio_num" = "GPP_D15" # EN_PP2800_WCAM_X > can you please check if we need this ? i guess line#405 does the same as well ? When the CCA is opened, the camera service will configure the VCM and Sensor. The VCM runs before Sensor IC, so we need to add VCM power-on sequence. The VCM & Sensor driver are different, but they are using the same power rail so we also need to include this as part VCM entries. -- To view, visit https://review.coreboot.org/c/coreboot/+/84019?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: I979207d1b6d55f78dea20d3366ef4a833ee9c86d Gerrit-Change-Number: 84019 Gerrit-PatchSet: 3 Gerrit-Owner: V Sowmya <v.sowmya(a)intel.com> Gerrit-Reviewer: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Reviewer: Eric Lai <ericllai(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: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Jayvik Desai <jayvik(a)google.com> Gerrit-Attention: Eric Lai <ericllai(a)google.com> Gerrit-Attention: Rishika Raj <rishikaraj(a)google.com> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Attention: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Comment-Date: Wed, 28 Aug 2024 05:01:56 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com>

1 0

[XS] Change in coreboot[main]: soc/intel/meteorlake: Remove SOC_INTEL_GFX_MBUS_JOIN config
by Paul Menzel (Code Review) 28 Aug '24

28 Aug '24

Attention is currently required from: Eran Mitrani, Jakub Czapiga, Jérémy Compostella, Kapil Porwal, Subrata Banik, Tarun. Paul Menzel has posted comments on this change by Subrata Banik. ( https://review.coreboot.org/c/coreboot/+/81034?usp=email ) Change subject: soc/intel/meteorlake: Remove SOC_INTEL_GFX_MBUS_JOIN config ...................................................................... Patch Set 3: (2 comments) Patchset: PS3: > > What happens, when people use an older Linux kernel version? […] I think, older (non-ChromeOS) installations need to be supported. Commit Message: https://review.coreboot.org/c/coreboot/+/81034/comment/ec8310f2_8985ddf6?us… : PS3, Line 11: kernel driver (https://patchwork.freedesktop.org/series/130480/), > At the commit hash instead? The patches are not in upstream Linux yet (no idea if they have been applied by the subsystem maintainers). -- To view, visit https://review.coreboot.org/c/coreboot/+/81034?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: Ifb0416df53a453ce16815f9fd52ec6b53fade5e2 Gerrit-Change-Number: 81034 Gerrit-PatchSet: 3 Gerrit-Owner: Subrata Banik <subratabanik(a)google.com> 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: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: Paz Zcharya <pazz(a)google.com> Gerrit-Reviewer: Tarun <tstuli(a)gmail.com> Gerrit-Reviewer: YH Lin <yueherngl(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-CC: Matt DeVillier <matt.devillier(a)gmail.com> Gerrit-CC: Nico Huber <nico.h(a)gmx.de> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> 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: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Attention: Tarun <tstuli(a)gmail.com> Gerrit-Comment-Date: Wed, 28 Aug 2024 04:53:48 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com> Comment-In-Reply-To: Paul Menzel <paulepanter(a)mailbox.org>

1 0

[S] Change in coreboot[main]: util/crossgcc: Patch clang to enable linking x86 bare metal targets
by Felix Singer (Code Review) 28 Aug '24

28 Aug '24

Attention is currently required from: Arthur Heymans, Martin L Roth. Felix Singer has posted comments on this change by Arthur Heymans. ( https://review.coreboot.org/c/coreboot/+/84032?usp=email ) Change subject: util/crossgcc: Patch clang to enable linking x86 bare metal targets ...................................................................... Patch Set 8: (1 comment) File util/crossgcc/patches/clang-18.1.6.src_x86_baremetal.patch: PS7: > AFAIK we don't use clang for the toolchain builder. Uh, ccache. It's too early .. -- To view, visit https://review.coreboot.org/c/coreboot/+/84032?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: I943215c8714e392e52ea35667f2bf21e517c4255 Gerrit-Change-Number: 84032 Gerrit-PatchSet: 8 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Felix Singer <service+coreboot-gerrit(a)felixsinger.de> Gerrit-Attention: Martin L Roth <gaumless(a)gmail.com> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Comment-Date: Wed, 28 Aug 2024 04:50:11 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Felix Singer <service+coreboot-gerrit(a)felixsinger.de> Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Comment-In-Reply-To: Arthur Heymans <arthur(a)aheymans.xyz>

1 0

[XS] Change in coreboot[main]: soc/intel/alderlake: Remove SOC_INTEL_GFX_MBUS_JOIN config
by Paul Menzel (Code Review) 28 Aug '24

28 Aug '24

Paul Menzel has posted comments on this change by Subrata Banik. ( https://review.coreboot.org/c/coreboot/+/84030?usp=email ) Change subject: soc/intel/alderlake: Remove SOC_INTEL_GFX_MBUS_JOIN config ...................................................................... Patch Set 2: (1 comment) Patchset: PS2: Hmm, same comments as for Ifb0416df53a453ce16815f9fd52ec6b53fade5e2 [1] apply. coreboot also needs to support older Linux kernel versions in my opinion. [1]: https://review.coreboot.org/c/coreboot/+/81034/ -- To view, visit https://review.coreboot.org/c/coreboot/+/84030?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: I53c08a0e7a40b24db7cc910c5b9adc2376a9bb17 Gerrit-Change-Number: 84030 Gerrit-PatchSet: 2 Gerrit-Owner: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Dinesh Gehlot <digehlot(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: Paz Zcharya <pazz(a)google.com> Gerrit-Reviewer: Rishika Raj <rishikaraj(a)google.com> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: YH Lin <yueherngl(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Matt DeVillier <matt.devillier(a)gmail.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Comment-Date: Wed, 28 Aug 2024 04:50:06 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No

1 0

[S] Change in coreboot[main]: util/crossgcc: Patch clang to enable linking x86 bare metal targets
by Felix Singer (Code Review) 28 Aug '24

28 Aug '24

Attention is currently required from: Arthur Heymans, Martin L Roth. Felix Singer has posted comments on this change by Arthur Heymans. ( https://review.coreboot.org/c/coreboot/+/84032?usp=email ) Change subject: util/crossgcc: Patch clang to enable linking x86 bare metal targets ...................................................................... Patch Set 8: (1 comment) File util/crossgcc/patches/clang-18.1.6.src_x86_baremetal.patch: PS7: > ccache? AFAIK we don't use clang for the toolchain builder. -- To view, visit https://review.coreboot.org/c/coreboot/+/84032?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: I943215c8714e392e52ea35667f2bf21e517c4255 Gerrit-Change-Number: 84032 Gerrit-PatchSet: 8 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Felix Singer <service+coreboot-gerrit(a)felixsinger.de> Gerrit-Attention: Martin L Roth <gaumless(a)gmail.com> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Comment-Date: Wed, 28 Aug 2024 04:48:23 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Felix Singer <service+coreboot-gerrit(a)felixsinger.de> Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Comment-In-Reply-To: Arthur Heymans <arthur(a)aheymans.xyz>

1 0

[XS] Change in coreboot[main]: util/hda-decoder: Protect 'fields' slice if empty string
by Felix Singer (Code Review) 28 Aug '24

28 Aug '24

Attention is currently required from: Maxim. Felix Singer has posted comments on this change by Maxim. ( https://review.coreboot.org/c/coreboot/+/84101?usp=email ) Change subject: util/hda-decoder: Protect 'fields' slice if empty string ...................................................................... Patch Set 2: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/84101?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: I9ae8544e52d82e8d8a82a88a85a2de7d2f295ec1 Gerrit-Change-Number: 84101 Gerrit-PatchSet: 2 Gerrit-Owner: Maxim <max.senia.poliak(a)gmail.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Felix Singer <service+coreboot-gerrit(a)felixsinger.de> Gerrit-Reviewer: Nicholas Sudsgaard <devel+coreboot(a)nsudsgaard.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Maxim <max.senia.poliak(a)gmail.com> Gerrit-Comment-Date: Wed, 28 Aug 2024 04:44:55 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes

1 0

[L] Change in coreboot[main]: mb/lattepanda: Add support for LattePanda Mu
by Felix Singer (Code Review) 28 Aug '24

28 Aug '24

Attention is currently required from: Felix Held, KunYi Chen, Martin L Roth, Nicholas Chin, Paul Menzel. Felix Singer has posted comments on this change by KunYi Chen. ( https://review.coreboot.org/c/coreboot/+/83719?usp=email ) Change subject: mb/lattepanda: Add support for LattePanda Mu ...................................................................... Patch Set 10: (8 comments) File src/mainboard/lattepanda/mu/devicetree.cb: https://review.coreboot.org/c/coreboot/+/83719/comment/36526bed_340092ed?us… : PS10, Line 9: register "emmc_enable_hs400_mode" = "1" Move this setting below the emmc device from the devicetree. https://review.coreboot.org/c/coreboot/+/83719/comment/41c29903_4a9a05ef?us… : PS10, Line 11: register "sata_salp_support" = "0" Settings are initialized with 0, please remove. https://review.coreboot.org/c/coreboot/+/83719/comment/9b2bebef_a251d33c?us… : PS10, Line 38: register "serial_io_gspi_cs_mode" = "{ : [PchSerialIoIndexGSPI0] = 0, : [PchSerialIoIndexGSPI1] = 0, : [PchSerialIoIndexGSPI2] = 0, : [PchSerialIoIndexGSPI3] = 0, : }" : : register "serial_io_gspi_cs_state" = "{ : [PchSerialIoIndexGSPI0] = 0, : [PchSerialIoIndexGSPI1] = 0, : [PchSerialIoIndexGSPI2] = 0, : [PchSerialIoIndexGSPI3] = 0, : }" Settings are initialized with 0, please remove. https://review.coreboot.org/c/coreboot/+/83719/comment/e9ec303a_2dd386ff?us… : PS10, Line 59: register "pch_hda_dsp_enable" = "1" : register "pch_hda_idisp_link_tmode" = "HDA_TMODE_8T" : register "pch_hda_idisp_link_frequency" = "HDA_LINKFREQ_96MHZ" : r Move these settings below the hda device from the devicetree. https://review.coreboot.org/c/coreboot/+/83719/comment/718b1d48_b172e597?us… : PS10, Line 103: [3] = USB2_PORT_EMPTY, USB ports are disabled by default, remove. https://review.coreboot.org/c/coreboot/+/83719/comment/d4d6de7c_e9605a10?us… : PS10, Line 110: [0] = USB3_PORT_EMPTY, : Same here. File src/mainboard/lattepanda/mu/spd/Makefile.mk: https://review.coreboot.org/c/coreboot/+/83719/comment/e5bed233_9748a131?us… : PS10, Line 7: SPD_SOURCES += empty # 0b004 This seems odd. Are these really needed? File src/mainboard/lattepanda/mu/spd/empty.spd.hex: PS10: Why is a zeroed SPD file needed? -- To view, visit https://review.coreboot.org/c/coreboot/+/83719?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: I79696bdd837a221860b32f54629212c3346dca66 Gerrit-Change-Number: 83719 Gerrit-PatchSet: 10 Gerrit-Owner: KunYi Chen <kunyi.chen(a)gmail.com> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Nicholas Chin <nic.c3.14(a)gmail.com> Gerrit-Reviewer: Nicholas Sudsgaard <devel+coreboot(a)nsudsgaard.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Felix Singer <service+coreboot-gerrit(a)felixsinger.de> Gerrit-CC: Martin L Roth <gaumless(a)gmail.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Martin L Roth <gaumless(a)gmail.com> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: KunYi Chen <kunyi.chen(a)gmail.com> Gerrit-Attention: Nicholas Chin <nic.c3.14(a)gmail.com> Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Comment-Date: Wed, 28 Aug 2024 04:44:11 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No

1 0

[L] Change in coreboot[main]: soc/intel/common/block/imc: Add Integrated Memory Controller driver
by Name of user not set (Code Review) 28 Aug '24

28 Aug '24

Attention is currently required from: Felix Singer, Jérémy Compostella, 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/+/83320?usp=email ) Change subject: soc/intel/common/block/imc: Add Integrated Memory Controller driver ...................................................................... Patch Set 13: (2 comments) File src/soc/intel/common/block/smbus/smbuslib.c: https://review.coreboot.org/c/coreboot/+/83320/comment/7564532e_50e821e1?us… : PS3, Line 10: { > Can we not using weak since it would be confusing? […] If not using weak, can I create a new spd.c to hold the default implementation and add it to build tree only if SOC_INTEL_COMMON_BLOCK_IMC is not selected, like this: ifneq ($(CONFIG_SOC_INTEL_COMMON_BLOCK_IMC),y) bootblock-$(CONFIG_SOC_INTEL_COMMON_BLOCK_SCS) += spd.c romstage-$(CONFIG_SOC_INTEL_COMMON_BLOCK_SCS) += spd.c ramstage-$(CONFIG_SOC_INTEL_COMMON_BLOCK_SCS) += spd.c endif https://review.coreboot.org/c/coreboot/+/83320/comment/a4745924_9cf23c65?us… : PS3, Line 96: if (!CONFIG(SOC_INTEL_COMMON_BLOCK_IMC) && blk->addr_map[i] == 0) { > I guess SOC_INTEL_COMMON_BLOCK_IMC will provide SPD data not depending on blk->addr_map[i]? The addr_map[i] is finally used as the address of the slave. In the default implementation it seems 0 is not a valid one but in IMC it's valid. -- To view, visit https://review.coreboot.org/c/coreboot/+/83320?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: I3f47ddeda94d3882852d64c0052f8fb42b6b7ad2 Gerrit-Change-Number: 83320 Gerrit-PatchSet: 13 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: Elyes Haouas <ehaouas(a)noos.fr> 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: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Attention: Shuo Liu <shuo.liu(a)intel.com> Gerrit-Comment-Date: Wed, 28 Aug 2024 04:29:56 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Shuo Liu <shuo.liu(a)intel.com>

1 0

Jump to page:

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit August 2024