coreboot-gerrit September 2024

coreboot-gerrit@coreboot.org

1 participants
3342 discussions

[M] Change in coreboot[main]: soc/intel: Refactor ITSS macros
by Subrata Banik (Code Review) 04 Sep '24

04 Sep '24

Subrata Banik has submitted this change. ( https://review.coreboot.org/c/coreboot/+/84183?usp=email ) Change subject: soc/intel: Refactor ITSS macros ...................................................................... soc/intel: Refactor ITSS macros This patch refactors ITSS related SoC specific macros by consolidating them into a common itss.h file. This improves code maintainability and reduces redundancy as each SoC previously defined the same macros. Specific changes include: - Move SoC specific ITSS macros into intelblocks/itss.h. - SoC code now includes intelblocks/itss.h instead of the SoC-local soc/itss.h. - Drop soc/itss.h from static ASL files. - Delete soc/itss.h from all SoC locals except Apollo Lake and Sky Lake. TEST=Able to build and boot google/hatch, google/xol and google/karis. Change-Id: I6461dc93b0d21bec5429075bc26435bae3754d74 Signed-off-by: Subrata Banik <subratabanik(a)google.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/84183 Reviewed-by: Jayvik Desai <jayvik(a)google.com> Reviewed-by: Dinesh Gehlot <digehlot(a)google.com> Reviewed-by: Shuo Liu <shuo.liu(a)intel.com> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Eric Lai <ericllai(a)google.com> Reviewed-by: Kapil Porwal <kapilporwal(a)google.com> Reviewed-by: Sean Rhodes <sean(a)starlabs.systems> --- M src/soc/intel/alderlake/acpi/southbridge.asl M src/soc/intel/alderlake/chip.c D src/soc/intel/alderlake/include/soc/itss.h M src/soc/intel/apollolake/include/soc/itss.h D src/soc/intel/cannonlake/include/soc/itss.h M src/soc/intel/common/block/include/intelblocks/itss.h M src/soc/intel/common/block/itss/itss.c M src/soc/intel/elkhartlake/acpi/southbridge.asl M src/soc/intel/elkhartlake/chip.c D src/soc/intel/elkhartlake/include/soc/itss.h M src/soc/intel/jasperlake/acpi/southbridge.asl M src/soc/intel/jasperlake/chip.c D src/soc/intel/jasperlake/include/soc/itss.h M src/soc/intel/meteorlake/acpi/southbridge.asl M src/soc/intel/meteorlake/chip.c D src/soc/intel/meteorlake/include/soc/itss.h M src/soc/intel/skylake/acpi/pch.asl M src/soc/intel/skylake/chip.c M src/soc/intel/skylake/include/soc/itss.h M src/soc/intel/tigerlake/acpi/southbridge.asl M src/soc/intel/tigerlake/chip.c D src/soc/intel/tigerlake/include/soc/itss.h M src/soc/intel/xeon_sp/gnr/soc_acpi.c D src/soc/intel/xeon_sp/include/soc/itss.h 24 files changed, 9 insertions(+), 92 deletions(-) Approvals: Eric Lai: Looks good to me, but someone else must approve build bot (Jenkins): Verified Sean Rhodes: Looks good to me, but someone else must approve Shuo Liu: Looks good to me, but someone else must approve Jayvik Desai: Looks good to me, approved Dinesh Gehlot: Looks good to me, approved Kapil Porwal: Looks good to me, approved diff --git a/src/soc/intel/alderlake/acpi/southbridge.asl b/src/soc/intel/alderlake/acpi/southbridge.asl index af641db..651d26c 100644 --- a/src/soc/intel/alderlake/acpi/southbridge.asl +++ b/src/soc/intel/alderlake/acpi/southbridge.asl @@ -2,7 +2,6 @@ #include <intelblocks/itss.h> #include <intelblocks/pcr.h> -#include <soc/itss.h> #include <soc/pcr_ids.h> /* PCR access */ diff --git a/src/soc/intel/alderlake/chip.c b/src/soc/intel/alderlake/chip.c index 8979ae0f6..23a05de 100644 --- a/src/soc/intel/alderlake/chip.c +++ b/src/soc/intel/alderlake/chip.c @@ -15,7 +15,6 @@ #include <intelblocks/xdci.h> #include <soc/hsphy.h> #include <soc/intel/common/vbt.h> -#include <soc/itss.h> #include <soc/p2sb.h> #include <soc/pci_devs.h> #include <soc/pcie.h> diff --git a/src/soc/intel/alderlake/include/soc/itss.h b/src/soc/intel/alderlake/include/soc/itss.h deleted file mode 100644 index 0ea8ff1..0000000 --- a/src/soc/intel/alderlake/include/soc/itss.h +++ /dev/null @@ -1,10 +0,0 @@ -/* SPDX-License-Identifier: GPL-2.0-only */ - -#ifndef SOC_INTEL_ADL_ITSS_H -#define SOC_INTEL_ADL_ITSS_H - -#define ITSS_MAX_IRQ 119 -#define IRQS_PER_IPC 32 -#define NUM_IPC_REGS ((ITSS_MAX_IRQ + IRQS_PER_IPC - 1)/IRQS_PER_IPC) - -#endif /* SOC_INTEL_ADL_ITSS_H */ diff --git a/src/soc/intel/apollolake/include/soc/itss.h b/src/soc/intel/apollolake/include/soc/itss.h index b88dfd8..03ba785 100644 --- a/src/soc/intel/apollolake/include/soc/itss.h +++ b/src/soc/intel/apollolake/include/soc/itss.h @@ -3,11 +3,9 @@ #ifndef _SOC_APOLLOLAKE_ITSS_H_ #define _SOC_APOLLOLAKE_ITSS_H_ +#include <intelblocks/itss.h> + #define GPIO_IRQ_START 50 #define GPIO_IRQ_END ITSS_MAX_IRQ -#define ITSS_MAX_IRQ 119 -#define IRQS_PER_IPC 32 -#define NUM_IPC_REGS ((ITSS_MAX_IRQ + IRQS_PER_IPC - 1)/IRQS_PER_IPC) - #endif /* _SOC_APOLLOLAKE_ITSS_H_ */ diff --git a/src/soc/intel/cannonlake/include/soc/itss.h b/src/soc/intel/cannonlake/include/soc/itss.h deleted file mode 100644 index 902b8e2..0000000 --- a/src/soc/intel/cannonlake/include/soc/itss.h +++ /dev/null @@ -1,10 +0,0 @@ -/* SPDX-License-Identifier: GPL-2.0-only */ - -#ifndef SOC_INTEL_CNL_ITSS_H -#define SOC_INTEL_CNL_ITSS_H - -#define ITSS_MAX_IRQ 119 -#define IRQS_PER_IPC 32 -#define NUM_IPC_REGS ((ITSS_MAX_IRQ + IRQS_PER_IPC - 1)/IRQS_PER_IPC) - -#endif /* SOC_INTEL_CNL_ITSS_H */ diff --git a/src/soc/intel/common/block/include/intelblocks/itss.h b/src/soc/intel/common/block/include/intelblocks/itss.h index 4d26b25..84ba7b4 100644 --- a/src/soc/intel/common/block/include/intelblocks/itss.h +++ b/src/soc/intel/common/block/include/intelblocks/itss.h @@ -3,6 +3,10 @@ #ifndef SOC_INTEL_COMMON_BLOCK_ITSS_H #define SOC_INTEL_COMMON_BLOCK_ITSS_H +#define ITSS_MAX_IRQ 119 +#define IRQS_PER_IPC 32 +#define NUM_IPC_REGS ((ITSS_MAX_IRQ + IRQS_PER_IPC - 1)/IRQS_PER_IPC) + /* PIRQA Routing Control Register */ #define PCR_ITSS_PIRQA_ROUT 0x3100 /* PIRQB Routing Control Register */ diff --git a/src/soc/intel/common/block/itss/itss.c b/src/soc/intel/common/block/itss/itss.c index 79ed5d0..50e4c38 100644 --- a/src/soc/intel/common/block/itss/itss.c +++ b/src/soc/intel/common/block/itss/itss.c @@ -5,7 +5,6 @@ #include <console/console.h> #include <intelblocks/itss.h> #include <intelblocks/pcr.h> -#include <soc/itss.h> #include <soc/pcr_ids.h> #include <southbridge/intel/common/acpi_pirq_gen.h> diff --git a/src/soc/intel/elkhartlake/acpi/southbridge.asl b/src/soc/intel/elkhartlake/acpi/southbridge.asl index 8b0748d..deb626d 100644 --- a/src/soc/intel/elkhartlake/acpi/southbridge.asl +++ b/src/soc/intel/elkhartlake/acpi/southbridge.asl @@ -2,7 +2,6 @@ #include <intelblocks/itss.h> #include <intelblocks/pcr.h> -#include <soc/itss.h> #include <soc/pcr_ids.h> /* PCI IRQ assignment */ diff --git a/src/soc/intel/elkhartlake/chip.c b/src/soc/intel/elkhartlake/chip.c index f9e4e0e..138d625 100644 --- a/src/soc/intel/elkhartlake/chip.c +++ b/src/soc/intel/elkhartlake/chip.c @@ -12,7 +12,6 @@ #include <intelblocks/systemagent.h> #include <intelblocks/xdci.h> #include <soc/intel/common/vbt.h> -#include <soc/itss.h> #include <soc/pci_devs.h> #include <soc/pcie.h> #include <soc/ramstage.h> diff --git a/src/soc/intel/elkhartlake/include/soc/itss.h b/src/soc/intel/elkhartlake/include/soc/itss.h deleted file mode 100644 index 90a4af2..0000000 --- a/src/soc/intel/elkhartlake/include/soc/itss.h +++ /dev/null @@ -1,10 +0,0 @@ -/* SPDX-License-Identifier: GPL-2.0-only */ - -#ifndef SOC_INTEL_EHL_ITSS_H -#define SOC_INTEL_EHL_ITSS_H - -#define ITSS_MAX_IRQ 119 -#define IRQS_PER_IPC 32 -#define NUM_IPC_REGS ((ITSS_MAX_IRQ + IRQS_PER_IPC - 1)/IRQS_PER_IPC) - -#endif /* SOC_INTEL_EHL_ITSS_H */ diff --git a/src/soc/intel/jasperlake/acpi/southbridge.asl b/src/soc/intel/jasperlake/acpi/southbridge.asl index a463304..63b2dfa 100644 --- a/src/soc/intel/jasperlake/acpi/southbridge.asl +++ b/src/soc/intel/jasperlake/acpi/southbridge.asl @@ -2,7 +2,6 @@ #include <intelblocks/itss.h> #include <intelblocks/pcr.h> -#include <soc/itss.h> #include <soc/pcr_ids.h> /* PCI IRQ assignment */ diff --git a/src/soc/intel/jasperlake/chip.c b/src/soc/intel/jasperlake/chip.c index 184e6ed..6b4c74c 100644 --- a/src/soc/intel/jasperlake/chip.c +++ b/src/soc/intel/jasperlake/chip.c @@ -12,7 +12,6 @@ #include <intelblocks/systemagent.h> #include <intelblocks/xdci.h> #include <soc/intel/common/vbt.h> -#include <soc/itss.h> #include <soc/pci_devs.h> #include <soc/pcie.h> #include <soc/ramstage.h> diff --git a/src/soc/intel/jasperlake/include/soc/itss.h b/src/soc/intel/jasperlake/include/soc/itss.h deleted file mode 100644 index ba1a1c2..0000000 --- a/src/soc/intel/jasperlake/include/soc/itss.h +++ /dev/null @@ -1,10 +0,0 @@ -/* SPDX-License-Identifier: GPL-2.0-only */ - -#ifndef SOC_INTEL_JSL_ITSS_H -#define SOC_INTEL_JSL_ITSS_H - -#define ITSS_MAX_IRQ 119 -#define IRQS_PER_IPC 32 -#define NUM_IPC_REGS ((ITSS_MAX_IRQ + IRQS_PER_IPC - 1)/IRQS_PER_IPC) - -#endif /* SOC_INTEL_JSL_ITSS_H */ diff --git a/src/soc/intel/meteorlake/acpi/southbridge.asl b/src/soc/intel/meteorlake/acpi/southbridge.asl index e89b656..5e8042e 100644 --- a/src/soc/intel/meteorlake/acpi/southbridge.asl +++ b/src/soc/intel/meteorlake/acpi/southbridge.asl @@ -2,7 +2,6 @@ #include <intelblocks/itss.h> #include <intelblocks/pcr.h> -#include <soc/itss.h> #include <soc/pcr_ids.h> /* SoC PCR access */ diff --git a/src/soc/intel/meteorlake/chip.c b/src/soc/intel/meteorlake/chip.c index 51e89dc..84b9235b2 100644 --- a/src/soc/intel/meteorlake/chip.c +++ b/src/soc/intel/meteorlake/chip.c @@ -18,7 +18,6 @@ #include <soc/intel/common/reset.h> #include <soc/intel/common/vbt.h> #include <soc/iomap.h> -#include <soc/itss.h> #include <soc/p2sb.h> #include <soc/pci_devs.h> #include <soc/pcie.h> diff --git a/src/soc/intel/meteorlake/include/soc/itss.h b/src/soc/intel/meteorlake/include/soc/itss.h deleted file mode 100644 index cee8245..0000000 --- a/src/soc/intel/meteorlake/include/soc/itss.h +++ /dev/null @@ -1,10 +0,0 @@ -/* SPDX-License-Identifier: GPL-2.0-only */ - -#ifndef SOC_INTEL_MTL_ITSS_H -#define SOC_INTEL_MTL_ITSS_H - -#define ITSS_MAX_IRQ 119 -#define IRQS_PER_IPC 32 -#define NUM_IPC_REGS ((ITSS_MAX_IRQ + IRQS_PER_IPC - 1)/IRQS_PER_IPC) - -#endif /* SOC_INTEL_MTL_ITSS_H */ diff --git a/src/soc/intel/skylake/acpi/pch.asl b/src/soc/intel/skylake/acpi/pch.asl index a2ab35c..8326633 100644 --- a/src/soc/intel/skylake/acpi/pch.asl +++ b/src/soc/intel/skylake/acpi/pch.asl @@ -4,7 +4,6 @@ #include <intelblocks/pcr.h> #include <soc/iomap.h> #include <soc/irq.h> -#include <soc/itss.h> #include <soc/gpe.h> #include <soc/pcr_ids.h> diff --git a/src/soc/intel/skylake/chip.c b/src/soc/intel/skylake/chip.c index 050a272..d1cc6f9 100644 --- a/src/soc/intel/skylake/chip.c +++ b/src/soc/intel/skylake/chip.c @@ -22,8 +22,8 @@ #include <soc/intel/common/vbt.h> #include <soc/interrupt.h> #include <soc/iomap.h> -#include <soc/irq.h> #include <soc/itss.h> +#include <soc/irq.h> #include <soc/pci_devs.h> #include <soc/ramstage.h> #include <soc/systemagent.h> diff --git a/src/soc/intel/skylake/include/soc/itss.h b/src/soc/intel/skylake/include/soc/itss.h index 8bb0e12..9501b14 100644 --- a/src/soc/intel/skylake/include/soc/itss.h +++ b/src/soc/intel/skylake/include/soc/itss.h @@ -3,11 +3,9 @@ #ifndef SOC_INTEL_SKL_ITSS_H #define SOC_INTEL_SKL_ITSS_H +#include <intelblocks/itss.h> + #define GPIO_IRQ_START 50 #define GPIO_IRQ_END ITSS_MAX_IRQ -#define ITSS_MAX_IRQ 119 -#define IRQS_PER_IPC 32 -#define NUM_IPC_REGS ((ITSS_MAX_IRQ + IRQS_PER_IPC - 1)/IRQS_PER_IPC) - #endif /* SOC_INTEL_SKL_ITSS_H */ diff --git a/src/soc/intel/tigerlake/acpi/southbridge.asl b/src/soc/intel/tigerlake/acpi/southbridge.asl index c54bc67..23536b3 100644 --- a/src/soc/intel/tigerlake/acpi/southbridge.asl +++ b/src/soc/intel/tigerlake/acpi/southbridge.asl @@ -2,7 +2,6 @@ #include <intelblocks/itss.h> #include <intelblocks/pcr.h> -#include <soc/itss.h> #include <soc/pcr_ids.h> /* PCR access */ diff --git a/src/soc/intel/tigerlake/chip.c b/src/soc/intel/tigerlake/chip.c index c94e727..f90f8ad 100644 --- a/src/soc/intel/tigerlake/chip.c +++ b/src/soc/intel/tigerlake/chip.c @@ -13,7 +13,6 @@ #include <intelblocks/systemagent.h> #include <intelblocks/xdci.h> #include <soc/intel/common/vbt.h> -#include <soc/itss.h> #include <soc/pci_devs.h> #include <soc/ramstage.h> #include <soc/soc_chip.h> diff --git a/src/soc/intel/tigerlake/include/soc/itss.h b/src/soc/intel/tigerlake/include/soc/itss.h deleted file mode 100644 index 0291dc8..0000000 --- a/src/soc/intel/tigerlake/include/soc/itss.h +++ /dev/null @@ -1,10 +0,0 @@ -/* SPDX-License-Identifier: GPL-2.0-only */ - -#ifndef SOC_INTEL_TGL_ITSS_H -#define SOC_INTEL_TGL_ITSS_H - -#define ITSS_MAX_IRQ 119 -#define IRQS_PER_IPC 32 -#define NUM_IPC_REGS ((ITSS_MAX_IRQ + IRQS_PER_IPC - 1)/IRQS_PER_IPC) - -#endif /* SOC_INTEL_TGL_ITSS_H */ diff --git a/src/soc/intel/xeon_sp/gnr/soc_acpi.c b/src/soc/intel/xeon_sp/gnr/soc_acpi.c index c91ed16..54c0cd2 100644 --- a/src/soc/intel/xeon_sp/gnr/soc_acpi.c +++ b/src/soc/intel/xeon_sp/gnr/soc_acpi.c @@ -11,7 +11,6 @@ #include <soc/numa.h> #include <soc/soc_util.h> #include <soc/util.h> -#include <soc/itss.h> #include <soc/pcr_ids.h> int soc_madt_sci_irq_polarity(int sci) diff --git a/src/soc/intel/xeon_sp/include/soc/itss.h b/src/soc/intel/xeon_sp/include/soc/itss.h deleted file mode 100644 index 3be9b8b..0000000 --- a/src/soc/intel/xeon_sp/include/soc/itss.h +++ /dev/null @@ -1,10 +0,0 @@ -/* SPDX-License-Identifier: GPL-2.0-only */ - -#ifndef SOC_INTEL_XEON_SP_ITSS_H -#define SOC_INTEL_XEON_SP_ITSS_H - -#define ITSS_MAX_IRQ 119 -#define IRQS_PER_IPC 32 -#define NUM_IPC_REGS ((ITSS_MAX_IRQ + IRQS_PER_IPC - 1)/IRQS_PER_IPC) - -#endif /* SOC_INTEL_XEON_SP_ITSS_H */ -- To view, visit https://review.coreboot.org/c/coreboot/+/84183?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: I6461dc93b0d21bec5429075bc26435bae3754d74 Gerrit-Change-Number: 84183 Gerrit-PatchSet: 3 Gerrit-Owner: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Christian Walter <christian.walter(a)9elements.com> Gerrit-Reviewer: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Reviewer: Eran Mitrani <mitrani(a)google.com> Gerrit-Reviewer: Eric Lai <ericllai(a)google.com> Gerrit-Reviewer: Jakub Czapiga <czapiga(a)google.com> Gerrit-Reviewer: Jayvik Desai <jayvik(a)google.com> Gerrit-Reviewer: Johnny Lin <Johnny_Lin(a)wiwynn.com> Gerrit-Reviewer: Jonathan Zhang <jon.zhixiong.zhang(a)gmail.com> Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: Rishika Raj <rishikaraj(a)google.com> Gerrit-Reviewer: Sean Rhodes <sean(a)starlabs.systems> Gerrit-Reviewer: Shuo Liu <shuo.liu(a)intel.com> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Tarun <tstuli(a)gmail.com> Gerrit-Reviewer: Tim Chu <Tim.Chu(a)quantatw.com> Gerrit-Reviewer: Werner Zeh <werner.zeh(a)siemens.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>

1 0

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

04 Sep '24

Attention is currently required from: Julius Werner, Jérémy Compostella, Karthik Ramasubramanian, Martin Roth, Pratikkumar V Prajapati, Saurabh Mishra, Subrata Banik. Ashish Kumar 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 18: Code-Review+2 -- 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: 18 Gerrit-Owner: Saurabh Mishra <mishra.saurabh(a)intel.com> Gerrit-Reviewer: Ashish Kumar Mishra <ashish.k.mishra(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: 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: 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 12:47:29 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes

1 0

[M] Change in coreboot[main]: [WIP] src/console: Add read-only flash console
by J. Neuschäfer (Code Review) 04 Sep '24

04 Sep '24

J. Neuschäfer has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/84214?usp=email ) Change subject: [WIP] src/console: Add read-only flash console ...................................................................... [WIP] src/console: Add read-only flash console Change-Id: Ie2f7a48e0cc6f7e91129bea9f15a5b7de3bbdfab Signed-off-by: J. Neuschäfer <j.ne(a)posteo.net> --- M src/console/Kconfig M src/console/console.c M src/drivers/spi/Makefile.mk A src/drivers/spi/ro_flashconsole.c A src/include/console/ro_flash.h 5 files changed, 66 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/14/84214/1 diff --git a/src/console/Kconfig b/src/console/Kconfig index db71204..c90c083 100644 --- a/src/console/Kconfig +++ b/src/console/Kconfig @@ -300,6 +300,22 @@ value (128K or 0x20000 bytes) is large enough to accommodate even the BIOS_SPEW level. +config CONSOLE_RO_FLASH + bool "Read-only flash console" + default n + help + Read-only flash console. (TODO) + +if CONSOLE_RO_FLASH + +config CONSOLE_RO_FLASH_OFFSET + hex "Base offset into flash" + +config CONSOLE_RO_FLASH_SHIFT + int "Bit shift" + +endif # CONSOLE_RO_FLASH + config CONSOLE_I2C_SMBUS bool "SMBus console output" depends on SOUTHBRIDGE_INTEL_COMMON_SMBUS diff --git a/src/console/console.c b/src/console/console.c index bb9d374..cec3c43 100644 --- a/src/console/console.c +++ b/src/console/console.c @@ -12,6 +12,7 @@ #include <console/system76_ec.h> #include <console/uart.h> #include <console/usb.h> +#include <console/ro_flash.h> #include <types.h> /* Note: when adding a new console, make sure you update the definition of @@ -27,6 +28,7 @@ __usbdebug_init(); __spiconsole_init(); __flashconsole_init(); + __ro_flashconsole_init(); __system76_ec_init(); __i2c_smbus_console_init(); __simnow_console_init(); @@ -47,6 +49,7 @@ __ne2k_tx_byte(byte); __usb_tx_byte(byte); __spiconsole_tx_byte(byte); + __ro_flashconsole_tx_byte(byte); __system76_ec_tx_byte(byte); __i2c_smbus_console_tx_byte(byte); __simnow_console_tx_byte(byte); diff --git a/src/drivers/spi/Makefile.mk b/src/drivers/spi/Makefile.mk index 97c3d63..6724cd1 100644 --- a/src/drivers/spi/Makefile.mk +++ b/src/drivers/spi/Makefile.mk @@ -20,6 +20,7 @@ $(1)-$(CONFIG_SPI_SDCARD) += spi_sdcard.c $(1)-$(CONFIG_BOOT_DEVICE_SPI_FLASH_RW_NOMMAP$(2)) += boot_device_rw_nommap.c $(1)-$(CONFIG_CONSOLE_SPI_FLASH) += flashconsole.c +$(1)-$(CONFIG_CONSOLE_RO_FLASH) += ro_flashconsole.c $(1)-$(CONFIG_SPI_FLASH_ADESTO) += adesto.c $(1)-$(CONFIG_SPI_FLASH_AMIC) += amic.c $(1)-$(CONFIG_SPI_FLASH_ATMEL) += atmel.c diff --git a/src/drivers/spi/ro_flashconsole.c b/src/drivers/spi/ro_flashconsole.c new file mode 100644 index 0000000..14a12d0 --- /dev/null +++ b/src/drivers/spi/ro_flashconsole.c @@ -0,0 +1,18 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ + +#include <console/ro_flash.h> +#include <commonlib/region.h> +#include <boot_device.h> + +void ro_flashconsole_init(void) +{ +} + +void ro_flashconsole_tx_byte(unsigned char c) +{ + const struct region_device *rdev = boot_device_ro(); + uint32_t offset = CONFIG_CONSOLE_RO_FLASH_OFFSET + (c << CONFIG_CONSOLE_RO_FLASH_SHIFT); + + char buf[1]; + rdev_readat(rdev, buf, offset, 1); +} diff --git a/src/include/console/ro_flash.h b/src/include/console/ro_flash.h new file mode 100644 index 0000000..c5858b5 --- /dev/null +++ b/src/include/console/ro_flash.h @@ -0,0 +1,28 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ + +#ifndef CONSOLE_RO_FLASH_H +#define CONSOLE_RO_FLASH_H + +#include <stdint.h> + +void ro_flashconsole_init(void); +void ro_flashconsole_tx_byte(unsigned char data); + +#define __RO_FLASHCONSOLE_ENABLE__ CONFIG(CONSOLE_RO_FLASH) + +#if __RO_FLASHCONSOLE_ENABLE__ +static inline void __ro_flashconsole_init(void) +{ + ro_flashconsole_init(); +} + +static inline void __ro_flashconsole_tx_byte(u8 data) +{ + ro_flashconsole_tx_byte(data); +} +#else +static inline void __ro_flashconsole_init(void) {} +static inline void __ro_flashconsole_tx_byte(u8 data) {} +#endif + +#endif /* CONSOLE_RO_FLASH_H */ -- To view, visit https://review.coreboot.org/c/coreboot/+/84214?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: newchange Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: Ie2f7a48e0cc6f7e91129bea9f15a5b7de3bbdfab Gerrit-Change-Number: 84214 Gerrit-PatchSet: 1 Gerrit-Owner: J. Neuschäfer <j.ne(a)posteo.net>

1 0

[XS] Change in coreboot[main]: [WIP]mb/google/brya: Work around a GCC LTO bug
by Arthur Heymans (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. Hello Dinesh Gehlot, Eran Mitrani, Jakub Czapiga, Jayvik Desai, Kapil Porwal, Nick Vaccaro, Rishika Raj, Subrata Banik, Tarun, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/84205?usp=email to look at the new patch set (#4). The following approvals got outdated and were removed: Verified+1 by build bot (Jenkins) Change subject: [WIP]mb/google/brya: Work around a GCC LTO bug ...................................................................... [WIP]mb/google/brya: Work around a GCC LTO bug Work around a GCC LTO bug. GCC with LTO hits a bug on code meant for real DIMMS. Increase the SPD buffer to counter that garbage collected buffer overflow. Let's see if CI needs this more broadly? Change-Id: Ib3d4ed8032bb06b6d08fbc2dc4b697df88745243 Signed-off-by: Arthur Heymans <arthur(a)aheymans.xyz> --- M src/mainboard/google/brya/Kconfig 1 file changed, 5 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/05/84205/4 -- 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: newpatchset Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: Ib3d4ed8032bb06b6d08fbc2dc4b697df88745243 Gerrit-Change-Number: 84205 Gerrit-PatchSet: 4 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>

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/a2b3040c_aa59112b?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 ? > > > > > > No that's not the issue. CONFIG_DIMM_MAX is 4 which is how large the array is for containing SPD pointers. LPDDRx has 16 bit per channel, so the code thinks there are 8 channels in total. CONFIG_DIMM_PER_CHANNEL is set to 2. The common code loops over channels and dimms per channel so it's overflowing. What is the appropriate fix? do you think. > > > > > > https://qa.coreboot.org/job/coreboot-gerrit/263746/testReport/junit/(root)/… is the compilation error with LTO which can detect the buffer overflow. > > > > will this work ? > > ``` > > git diff > > diff --git a/src/soc/intel/common/block/memory/meminit.c b/src/soc/intel/common/block/memory/meminit.c > > index f583213254..cf12aff803 100644 > > --- a/src/soc/intel/common/block/memory/meminit.c > > +++ b/src/soc/intel/common/block/memory/meminit.c > > @@ -189,12 +189,13 @@ void mem_populate_channel_data(FSPM_UPD *memupd, const struct soc_mem_cfg *soc_m > > struct mem_channel_data *data) > > { > > size_t spd_md_len = 0, spd_dimm_len = 0; > > - bool have_dimms; > > + bool have_dimms = 0; > > > > memset(data, 0, sizeof(*data)); > > > > read_spd_md(soc_mem_cfg, spd_info, half_populated, data, &spd_md_len); > > - have_dimms = read_spd_dimm(memupd, soc_mem_cfg, spd_info, half_populated, data, > > + if ((info->topo & MEM_TOPO_DIMM_MODULE) > > + have_dimms = read_spd_dimm(memupd, soc_mem_cfg, spd_info, half_populated, data, > > &spd_dimm_len); > > > > if (data->ch_population_flags == NO_CHANNEL_POPULATED) > > ``` > > I tried this. And looking more at it seems that this in fact a compiler bug. I'll play around with some options. It looks like even without lto this unused code would be in the binary. Maybe it's reasonable to move the memory type to Kconfig to optimize the code? the only problem is often we build a board like Intel RVP with unified AP FW that support both DDR5 and LPDDR5x. With introduction of Kconfig, we won't be able to keep unified AP FW to boot same HW with different memory configuration. If we need to block this code from getting complied then we can set `has_spd=false` and set to `true` depending upon `(info->topo & MEM_TOPO_DIMM_MODULE)`. That way, we don't see compilation error and at runtime `read_spd_dimm` API will be only called for DDRx DIMM devices along. WDYT? -- 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 12:32:34 +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 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/alderlake/meminit.c: https://review.coreboot.org/c/coreboot/+/84205/comment/a9f92388_f4cfbef3?us… : PS3, Line 239: = {}; > Maybe use a ```memset``` instead of ```{}``` for default value initialization ? > > - I don't see anywhere else in the code initializing structs default like this. > - since you have now kept the func ```mem_populate_channel_data``` under a condition ( the function was defaulting this variable to zero ), a memset here might be better IMO. > > this is just a suggestion, ignore it if you think it will break the logic. The code generated would be the same. It does look like {0} is ideomatic and not {}. -- 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 12:20:14 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Jayvik Desai <jayvik(a)google.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, 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/8d14cb18_e4bae43b?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 ? > > > > No that's not the issue. CONFIG_DIMM_MAX is 4 which is how large the array is for containing SPD pointers. LPDDRx has 16 bit per channel, so the code thinks there are 8 channels in total. CONFIG_DIMM_PER_CHANNEL is set to 2. The common code loops over channels and dimms per channel so it's overflowing. What is the appropriate fix? do you think. > > > > https://qa.coreboot.org/job/coreboot-gerrit/263746/testReport/junit/(root)/… is the compilation error with LTO which can detect the buffer overflow. > > will this work ? > ``` > git diff > diff --git a/src/soc/intel/common/block/memory/meminit.c b/src/soc/intel/common/block/memory/meminit.c > index f583213254..cf12aff803 100644 > --- a/src/soc/intel/common/block/memory/meminit.c > +++ b/src/soc/intel/common/block/memory/meminit.c > @@ -189,12 +189,13 @@ void mem_populate_channel_data(FSPM_UPD *memupd, const struct soc_mem_cfg *soc_m > struct mem_channel_data *data) > { > size_t spd_md_len = 0, spd_dimm_len = 0; > - bool have_dimms; > + bool have_dimms = 0; > > memset(data, 0, sizeof(*data)); > > read_spd_md(soc_mem_cfg, spd_info, half_populated, data, &spd_md_len); > - have_dimms = read_spd_dimm(memupd, soc_mem_cfg, spd_info, half_populated, data, > + if ((info->topo & MEM_TOPO_DIMM_MODULE) > + have_dimms = read_spd_dimm(memupd, soc_mem_cfg, spd_info, half_populated, data, > &spd_dimm_len); > > if (data->ch_population_flags == NO_CHANNEL_POPULATED) > ``` I tried this. And looking more at it seems that this in fact a compiler bug. I'll play around with some options. It looks like even without lto this unused code would be in the binary. Maybe it's reasonable to move the memory type to Kconfig to optimize the code? -- 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: 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 11:08:46 +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]: mb/google/brox/var/lotso: remove unused cam enable_gpio
by Karthik Ramasubramanian (Code Review) 04 Sep '24

04 Sep '24

Attention is currently required from: Jian Tong, Kun Liu, Shelley Chen. Karthik Ramasubramanian has posted comments on this change by Jian Tong. ( https://review.coreboot.org/c/coreboot/+/84198?usp=email ) Change subject: mb/google/brox/var/lotso: remove unused cam enable_gpio ...................................................................... Patch Set 3: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/84198?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: Id8f30597ef9bceb9bdd4a3267266f1d189aa6fd8 Gerrit-Change-Number: 84198 Gerrit-PatchSet: 3 Gerrit-Owner: Jian Tong <tongjian(a)huaqin.corp-partner.google.com> Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Reviewer: Kun Liu <liukun11(a)huaqin.corp-partner.google.com> Gerrit-Reviewer: Shelley Chen <shchen(a)google.com> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Jinfang Mao <maojinfang(a)huaqin.corp-partner.google.com> Gerrit-Attention: Shelley Chen <shchen(a)google.com> Gerrit-Attention: Jian Tong <tongjian(a)huaqin.corp-partner.google.com> Gerrit-Attention: Kun Liu <liukun11(a)huaqin.corp-partner.google.com> Gerrit-Comment-Date: Wed, 04 Sep 2024 10:38:16 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes

1 0

[S] Change in coreboot[main]: mb/google/nissa/var/yavilla: Add 1.2V enable pin in VCM
by Subrata Banik (Code Review) 04 Sep '24

04 Sep '24

Attention is currently required from: Chen Wisley, Dinesh Gehlot, Eric Lai, Jayvik Desai, Kapil Porwal, Nick Vaccaro, Rishika Raj, Wisley Chen. Subrata Banik has posted comments on this change by Chen Wisley. ( https://review.coreboot.org/c/coreboot/+/84211?usp=email ) Change subject: mb/google/nissa/var/yavilla: Add 1.2V enable pin in VCM ...................................................................... Patch Set 1: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/84211?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: I495b2e266ee3d24ed3334bb9c173b3993d095e8e Gerrit-Change-Number: 84211 Gerrit-PatchSet: 1 Gerrit-Owner: Chen Wisley <wisley.chen%quantatw.com(a)gtempaccount.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: Wisley Chen <wisley.chen(a)quanta.corp-partner.google.com> 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: Chen Wisley <wisley.chen%quantatw.com(a)gtempaccount.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: Wisley Chen <wisley.chen(a)quanta.corp-partner.google.com> Gerrit-Comment-Date: Wed, 04 Sep 2024 09:47:42 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes

1 0

[XL] Change in coreboot[main]: mainboard/hardkernel: Add ODROID H4+ initial support
by Michał Żygowski (Code Review) 04 Sep '24

04 Sep '24

Michał Żygowski has posted comments on this change by Michał Żygowski. ( https://review.coreboot.org/c/coreboot/+/84213?usp=email ) Change subject: mainboard/hardkernel: Add ODROID H4+ initial support ...................................................................... Patch Set 1: (1 comment) Patchset: PS1: We are excited to see more community interest in the Odroid-H4 board, and would like to share a snapshot of our draft work, which has been in progress on the public Dasharo repo since 8th Aug: https://github.com/Dasharo/coreboot/pull/545 -- To view, visit https://review.coreboot.org/c/coreboot/+/84213?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: Iee066cb0da27e3bf7ce52fc199396185912893aa Gerrit-Change-Number: 84213 Gerrit-PatchSet: 1 Gerrit-Owner: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-Comment-Date: Wed, 04 Sep 2024 09:45:51 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No

1 0

Jump to page:

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit September 2024