coreboot-gerrit May 2021

coreboot-gerrit@coreboot.org

1 participants
6048 discussions

Change in coreboot[master]: *x86: fix x2apic mode boot issue
by Wonkyu Kim (Code Review) 05 May '21

05 May '21

Attention is currently required from: Lance Zhao, Shaunak Saha, Nico Huber, Furquan Shaikh, Matt DeVillier, Ravishankar Sarawadi, Srinidhi N Kaushik, Angel Pons, Subrata Banik. Wonkyu Kim has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/52696 ) Change subject: *x86: fix x2apic mode boot issue ...................................................................... Patch Set 3: (1 comment) Patchset: PS2: > fixes booting issues on google/kahlee introduced by CB:51723, but also has the same commit title whi […] Done -- To view, visit https://review.coreboot.org/c/coreboot/+/52696 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ia28f60a077182c3753f6ba9fbdd141f951d39b37 Gerrit-Change-Number: 52696 Gerrit-PatchSet: 3 Gerrit-Owner: Wonkyu Kim <wonkyu.kim(a)intel.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Lance Zhao Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Ravishankar Sarawadi <ravishankar.sarawadi(a)intel.com> Gerrit-Reviewer: Shaunak Saha <shaunak.saha(a)intel.com> Gerrit-Reviewer: Srinidhi N Kaushik <srinidhi.n.kaushik(a)intel.com> Gerrit-Reviewer: Srinidhi N Kaushik <srinidhi.n.kaushik(a)intel.corp-partner.google.com> Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Reviewer: Wonkyu Kim <wonkyu.kim(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Lance Zhao Gerrit-Attention: Shaunak Saha <shaunak.saha(a)intel.com> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Furquan Shaikh <furquan(a)google.com> Gerrit-Attention: Matt DeVillier <matt.devillier(a)gmail.com> Gerrit-Attention: Ravishankar Sarawadi <ravishankar.sarawadi(a)intel.com> Gerrit-Attention: Srinidhi N Kaushik <srinidhi.n.kaushik(a)intel.corp-partner.google.com> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Attention: Srinidhi N Kaushik <srinidhi.n.kaushik(a)intel.com> Gerrit-Comment-Date: Wed, 05 May 2021 22:43:38 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Matt DeVillier <matt.devillier(a)gmail.com> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: *x86: fix x2apic mode boot issue
by Wonkyu Kim (Code Review) 05 May '21

05 May '21

Attention is currently required from: Lance Zhao, Shaunak Saha, Nico Huber, Furquan Shaikh, Wonkyu Kim, Ravishankar Sarawadi, Srinidhi N Kaushik, Angel Pons, Subrata Banik. Hello Lance Zhao, build bot (Jenkins), Shaunak Saha, Nico Huber, Furquan Shaikh, Matt DeVillier, Ravishankar Sarawadi, Srinidhi N Kaushik, Angel Pons, Subrata Banik, Srinidhi N Kaushik, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/52696 to look at the new patch set (#3). Change subject: *x86: fix x2apic mode boot issue ...................................................................... *x86: fix x2apic mode boot issue Fix booting issues on google/kahlee introduced by CB:51723. Update use inital apic id in smm_stub.S to support xapic mode error. Check more bits(LAPIC_BASE_MSR BIT10 and BIT11) for x2apic mode. TEST=Boot to OS and check apicid, debug log for CPUIDs cpuid_ebx(1), cpuid_ext(0xb, 0), cpuid_edx(0xb) etc Signed-off-by: Wonkyu Kim <wonkyu.kim(a)intel.com> Change-Id: Ia28f60a077182c3753f6ba9fbdd141f951d39b37 --- M src/cpu/x86/smm/smm_stub.S M src/include/cpu/x86/lapic.h M src/include/cpu/x86/lapic_def.h 3 files changed, 22 insertions(+), 17 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/96/52696/3 -- To view, visit https://review.coreboot.org/c/coreboot/+/52696 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ia28f60a077182c3753f6ba9fbdd141f951d39b37 Gerrit-Change-Number: 52696 Gerrit-PatchSet: 3 Gerrit-Owner: Wonkyu Kim <wonkyu.kim(a)intel.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Lance Zhao Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Ravishankar Sarawadi <ravishankar.sarawadi(a)intel.com> Gerrit-Reviewer: Shaunak Saha <shaunak.saha(a)intel.com> Gerrit-Reviewer: Srinidhi N Kaushik <srinidhi.n.kaushik(a)intel.com> Gerrit-Reviewer: Srinidhi N Kaushik <srinidhi.n.kaushik(a)intel.corp-partner.google.com> Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Reviewer: Wonkyu Kim <wonkyu.kim(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Lance Zhao Gerrit-Attention: Shaunak Saha <shaunak.saha(a)intel.com> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Furquan Shaikh <furquan(a)google.com> Gerrit-Attention: Wonkyu Kim <wonkyu.kim(a)intel.com> Gerrit-Attention: Ravishankar Sarawadi <ravishankar.sarawadi(a)intel.com> Gerrit-Attention: Srinidhi N Kaushik <srinidhi.n.kaushik(a)intel.corp-partner.google.com> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Attention: Srinidhi N Kaushik <srinidhi.n.kaushik(a)intel.com> Gerrit-MessageType: newpatchset

1 0

Change in coreboot[master]: mb/google/dedede: Create pirika variant
by Felix Held (Code Review) 05 May '21

05 May '21

Felix Held has submitted this change. ( https://review.coreboot.org/c/coreboot/+/52626 ) Change subject: mb/google/dedede: Create pirika variant ...................................................................... mb/google/dedede: Create pirika variant Create the pirika variant of the waddledee reference board by copying the template files to a new directory named for the variant. (Auto-Generated by create_coreboot_variant.sh version 4.5.0). BUG=b:184157747 BRANCH=None TEST=util/abuild/abuild -p none -t google/dedede -x -a make sure the build includes GOOGLE_PIRIKA Signed-off-by: kirk_wang <kirk_wang(a)pegatron.corp-partner.google.com> Change-Id: I57bf33deeadacc88800f9ce1d3d54385ba56c798 Reviewed-on: https://review.coreboot.org/c/coreboot/+/52626 Reviewed-by: Paul Fagerburg <pfagerburg(a)chromium.org> Reviewed-by: Karthik Ramasubramanian <kramasub(a)google.com> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> --- M src/mainboard/google/dedede/Kconfig M src/mainboard/google/dedede/Kconfig.name A src/mainboard/google/dedede/variants/pirika/include/variant/ec.h A src/mainboard/google/dedede/variants/pirika/include/variant/gpio.h A src/mainboard/google/dedede/variants/pirika/memory/Makefile.inc A src/mainboard/google/dedede/variants/pirika/memory/dram_id.generated.txt A src/mainboard/google/dedede/variants/pirika/memory/mem_parts_used.txt A src/mainboard/google/dedede/variants/pirika/overridetree.cb 8 files changed, 82 insertions(+), 0 deletions(-) Approvals: build bot (Jenkins): Verified Karthik Ramasubramanian: Looks good to me, approved Paul Fagerburg: Looks good to me, but someone else must approve Kirk Wang: Looks good to me, but someone else must approve diff --git a/src/mainboard/google/dedede/Kconfig b/src/mainboard/google/dedede/Kconfig index 3ec377e..e237b80 100644 --- a/src/mainboard/google/dedede/Kconfig +++ b/src/mainboard/google/dedede/Kconfig @@ -109,6 +109,7 @@ default "Kracko" if BOARD_GOOGLE_KRACKO default "Blipper" if BOARD_GOOGLE_BLIPPER default "Cret" if BOARD_GOOGLE_CRET + default "Pirika" if BOARD_GOOGLE_PIRIKA config MAX_CPUS int @@ -144,6 +145,7 @@ default "kracko" if BOARD_GOOGLE_KRACKO default "blipper" if BOARD_GOOGLE_BLIPPER default "cret" if BOARD_GOOGLE_CRET + default "pirika" if BOARD_GOOGLE_PIRIKA endif #BOARD_GOOGLE_BASEBOARD_DEDEDE diff --git a/src/mainboard/google/dedede/Kconfig.name b/src/mainboard/google/dedede/Kconfig.name index 9a960e8..e0ead8c 100644 --- a/src/mainboard/google/dedede/Kconfig.name +++ b/src/mainboard/google/dedede/Kconfig.name @@ -133,3 +133,8 @@ select BASEBOARD_DEDEDE_LAPTOP select DRIVERS_GENERIC_MAX98357A select DRIVERS_I2C_DA7219 + +config BOARD_GOOGLE_PIRIKA + bool "-> Pirika" + select BOARD_GOOGLE_BASEBOARD_DEDEDE_CR50 + select BASEBOARD_DEDEDE_LAPTOP diff --git a/src/mainboard/google/dedede/variants/pirika/include/variant/ec.h b/src/mainboard/google/dedede/variants/pirika/include/variant/ec.h new file mode 100644 index 0000000..08870e0 --- /dev/null +++ b/src/mainboard/google/dedede/variants/pirika/include/variant/ec.h @@ -0,0 +1,8 @@ +/* SPDX-License-Identifier: GPL-2.0-or-later */ + +#ifndef MAINBOARD_EC_H +#define MAINBOARD_EC_H + +#include <baseboard/ec.h> + +#endif diff --git a/src/mainboard/google/dedede/variants/pirika/include/variant/gpio.h b/src/mainboard/google/dedede/variants/pirika/include/variant/gpio.h new file mode 100644 index 0000000..9078664 --- /dev/null +++ b/src/mainboard/google/dedede/variants/pirika/include/variant/gpio.h @@ -0,0 +1,8 @@ +/* SPDX-License-Identifier: GPL-2.0-or-later */ + +#ifndef MAINBOARD_GPIO_H +#define MAINBOARD_GPIO_H + +#include <baseboard/gpio.h> + +#endif /* MAINBOARD_GPIO_H */ diff --git a/src/mainboard/google/dedede/variants/pirika/memory/Makefile.inc b/src/mainboard/google/dedede/variants/pirika/memory/Makefile.inc new file mode 100644 index 0000000..b0ca222 --- /dev/null +++ b/src/mainboard/google/dedede/variants/pirika/memory/Makefile.inc @@ -0,0 +1,5 @@ +## SPDX-License-Identifier: GPL-2.0-or-later +## This is an auto-generated file. Do not edit!! +## Add memory parts in mem_parts_used.txt and run spd_tools to regenerate. + +SPD_SOURCES = placeholder.spd.hex diff --git a/src/mainboard/google/dedede/variants/pirika/memory/dram_id.generated.txt b/src/mainboard/google/dedede/variants/pirika/memory/dram_id.generated.txt new file mode 100644 index 0000000..fa24790 --- /dev/null +++ b/src/mainboard/google/dedede/variants/pirika/memory/dram_id.generated.txt @@ -0,0 +1 @@ +DRAM Part Name ID to assign diff --git a/src/mainboard/google/dedede/variants/pirika/memory/mem_parts_used.txt b/src/mainboard/google/dedede/variants/pirika/memory/mem_parts_used.txt new file mode 100644 index 0000000..e4258b5 --- /dev/null +++ b/src/mainboard/google/dedede/variants/pirika/memory/mem_parts_used.txt @@ -0,0 +1,11 @@ +# This is a CSV file containing a list of memory parts used by this variant. +# One part per line with an optional fixed ID in column 2. +# Only include a fixed ID if it is required for legacy reasons! +# Generated IDs are dependent on the order of parts in this file, +# so new parts must always be added at the end of the file! +# +# Generate an updated Makefile.inc and dram_id.generated.txt by running the +# gen_part_id tool from util/spd_tools/{ddr4,lp4x}. +# See util/spd_tools/{ddr4,lp4x}/README.md for more details and instructions. + +# Part Name diff --git a/src/mainboard/google/dedede/variants/pirika/overridetree.cb b/src/mainboard/google/dedede/variants/pirika/overridetree.cb new file mode 100644 index 0000000..404024b --- /dev/null +++ b/src/mainboard/google/dedede/variants/pirika/overridetree.cb @@ -0,0 +1,42 @@ +chip soc/intel/jasperlake + + # Intel Common SoC Config + #+-------------------+---------------------------+ + #| Field | Value | + #+-------------------+---------------------------+ + #| GSPI0 | cr50 TPM. Early init is | + #| | required to set up a BAR | + #| | for TPM communication | + #| | before memory is up | + #| I2C0 | Trackpad | + #| I2C1 | Digitizer | + #| I2C2 | Touchscreen | + #| I2C3 | Camera | + #| I2C4 | Audio | + #+-------------------+---------------------------+ + register "common_soc_config" = "{ + .gspi[0] = { + .speed_mhz = 1, + .early_init = 1, + }, + .i2c[0] = { + .speed = I2C_SPEED_FAST, + }, + .i2c[1] = { + .speed = I2C_SPEED_FAST, + }, + .i2c[2] = { + .speed = I2C_SPEED_FAST, + }, + .i2c[3] = { + .speed = I2C_SPEED_FAST, + }, + .i2c[4] = { + .speed = I2C_SPEED_FAST, + }, + }" + + device domain 0 on + device pci 15.0 on end + end +end -- To view, visit https://review.coreboot.org/c/coreboot/+/52626 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I57bf33deeadacc88800f9ce1d3d54385ba56c798 Gerrit-Change-Number: 52626 Gerrit-PatchSet: 3 Gerrit-Owner: Kirk Wang Gerrit-Reviewer: Alex1 Kao <alex1_kao(a)pegatron.corp-partner.google.com> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Henry Sun <henrysun(a)google.com> Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Reviewer: Kirk Wang <kirk_wang(a)pegatron.corp-partner.google.com> Gerrit-Reviewer: Paul Fagerburg <pfagerburg(a)chromium.org> Gerrit-Reviewer: Paul2 Huang <paul2_huang(a)pegatron.corp-partner.google.com> Gerrit-Reviewer: Shou-Chieh Hsu <shouchieh(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Ariel Chang <ariel_chang(a)pegatron.corp-partner.google.com> Gerrit-CC: Hao Chou <hao_chou(a)pegatron.corp-partner.google.com> Gerrit-CC: Wayne3 Wang <wayne3_wang(a)pegatron.corp-partner.google.com> Gerrit-MessageType: merged

1 0

Change in coreboot[master]: soc/intel/alderlake: Add GPIO definition for CPU PCIe vGPIO
by Felix Held (Code Review) 05 May '21

05 May '21

Felix Held has submitted this change. ( https://review.coreboot.org/c/coreboot/+/52783 ) Change subject: soc/intel/alderlake: Add GPIO definition for CPU PCIe vGPIO ...................................................................... soc/intel/alderlake: Add GPIO definition for CPU PCIe vGPIO Adding GPIO definition for community 3 which is CPU reserved GPIO used by CPU side PCIe root ports. We did not have this definition since FSP used to program this GPIOs. Now, instead of FSP, coreboot programs CPU PCIe GPIOs for CLKSRC and lanes to put GPIOs in native mode. Thus adding definition of this virtual GPIOs in this CL. BUG=None BRANCH=None TEST=Check if correct registers are being programmed Change-Id: I481ea7e3ba948bf6d37b97d08c675a18ee68125d Signed-off-by: Maulik V Vaghela <maulik.v.vaghela(a)intel.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/52783 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Tim Wawrzynczak <twawrzynczak(a)chromium.org> --- M src/soc/intel/alderlake/gpio.c M src/soc/intel/alderlake/include/soc/gpio_defs.h M src/soc/intel/alderlake/include/soc/gpio_soc_defs.h 3 files changed, 237 insertions(+), 100 deletions(-) Approvals: build bot (Jenkins): Verified Tim Wawrzynczak: Looks good to me, approved diff --git a/src/soc/intel/alderlake/gpio.c b/src/soc/intel/alderlake/gpio.c index 1f96179..ef6f6e4 100644 --- a/src/soc/intel/alderlake/gpio.c +++ b/src/soc/intel/alderlake/gpio.c @@ -50,6 +50,10 @@ INTEL_GPP(GPD0, GPD0, GPD_DRAM_RESETB), /* GPD */ }; +/* This community is not visible to the OS */ +static const struct pad_group adl_community3_groups[] = { + INTEL_GPP(GPP_CPU_RSVD_1, GPP_CPU_RSVD_1, GPP_vGPIO_PCIE_83), /* vGPIO */ +}; static const struct pad_group adl_community4_groups[] = { INTEL_GPP_BASE(GPP_C0, GPP_C0, GPP_C23, 256), /* GPP_C */ INTEL_GPP_BASE(GPP_C0, GPP_F0, GPP_F_CLK_LOOPBK, 288), /* GPP_F */ @@ -120,6 +124,25 @@ .groups = adl_community2_groups, .num_groups = ARRAY_SIZE(adl_community2_groups), }, + [COMM_3] = { /* vGPIO */ + .port = PID_GPIOCOM3, + .first_pad = GPIO_COM3_START, + .last_pad = GPIO_COM3_END, + .num_gpi_regs = NUM_GPIO_COM3_GPI_REGS, + .pad_cfg_base = PAD_CFG_BASE, + .host_own_reg_0 = HOSTSW_OWN_REG_0, + .gpi_int_sts_reg_0 = GPI_INT_STS_0, + .gpi_int_en_reg_0 = GPI_INT_EN_0, + .gpi_smi_sts_reg_0 = GPI_SMI_STS_0, + .gpi_smi_en_reg_0 = GPI_SMI_EN_0, + .max_pads_per_group = GPIO_MAX_NUM_PER_GROUP, + .name = "GPP_VGPIO", + .acpi_path = "\\_SB.PCI0.GPIO", + .reset_map = rst_map, + .num_reset_vals = ARRAY_SIZE(rst_map), + .groups = adl_community3_groups, + .num_groups = ARRAY_SIZE(adl_community3_groups), + }, [COMM_4] = { /* GPP F, C, HVMOS, E */ .port = PID_GPIOCOM4, .first_pad = GPIO_COM4_START, @@ -183,4 +206,4 @@ }; *num = ARRAY_SIZE(routes); return routes; -} +}; diff --git a/src/soc/intel/alderlake/include/soc/gpio_defs.h b/src/soc/intel/alderlake/include/soc/gpio_defs.h index 68b886a..b329f3c 100644 --- a/src/soc/intel/alderlake/include/soc/gpio_defs.h +++ b/src/soc/intel/alderlake/include/soc/gpio_defs.h @@ -16,6 +16,7 @@ #define NUM_GPIO_COM0_GPI_REGS NUM_GPIO_COMx_GPI_REGS(NUM_GPIO_COM0_PADS) #define NUM_GPIO_COM1_GPI_REGS NUM_GPIO_COMx_GPI_REGS(NUM_GPIO_COM1_PADS) #define NUM_GPIO_COM2_GPI_REGS NUM_GPIO_COMx_GPI_REGS(NUM_GPIO_COM2_PADS) +#define NUM_GPIO_COM3_GPI_REGS NUM_GPIO_COMx_GPI_REGS(NUM_GPIO_COM3_PADS) #define NUM_GPIO_COM4_GPI_REGS NUM_GPIO_COMx_GPI_REGS(NUM_GPIO_COM4_PADS) #define NUM_GPIO_COM5_GPI_REGS NUM_GPIO_COMx_GPI_REGS(NUM_GPIO_COM5_PADS) diff --git a/src/soc/intel/alderlake/include/soc/gpio_soc_defs.h b/src/soc/intel/alderlake/include/soc/gpio_soc_defs.h index 5035718..b1f2341 100644 --- a/src/soc/intel/alderlake/include/soc/gpio_soc_defs.h +++ b/src/soc/intel/alderlake/include/soc/gpio_soc_defs.h @@ -27,9 +27,9 @@ #define COMM_0 0 #define COMM_1 1 #define COMM_2 2 -/* GPIO community 3 is not exposed to be used and hence is skipped. */ -#define COMM_4 3 -#define COMM_5 4 +#define COMM_3 3 +#define COMM_4 4 +#define COMM_5 5 /* * GPIOs are ordered monotonically increasing to match ACPI/OS driver. */ @@ -196,117 +196,230 @@ #define GPIO_COM2_END GPD_DRAM_RESETB #define NUM_GPIO_COM2_PADS (GPIO_COM2_END - GPIO_COM2_START + 1) +/* PCIE VGPIO group */ +#define GPP_CPU_RSVD_1 137 +#define GPP_CPU_RSVD_2 138 +#define GPP_CPU_RSVD_3 139 +#define GPP_CPU_RSVD_4 140 +#define GPP_CPU_RSVD_5 141 +#define GPP_CPU_RSVD_6 142 +#define GPP_CPU_RSVD_7 143 +#define GPP_CPU_RSVD_8 144 +#define GPP_CPU_RSVD_9 145 +#define GPP_CPU_RSVD_10 146 +#define GPP_CPU_RSVD_11 147 +#define GPP_CPU_RSVD_12 148 +#define GPP_CPU_RSVD_13 149 +#define GPP_CPU_RSVD_14 150 +#define GPP_CPU_RSVD_15 151 +#define GPP_vGPIO_PCIE_0 152 +#define GPP_vGPIO_PCIE_1 153 +#define GPP_vGPIO_PCIE_2 154 +#define GPP_vGPIO_PCIE_3 155 +#define GPP_vGPIO_PCIE_4 156 +#define GPP_vGPIO_PCIE_5 157 +#define GPP_vGPIO_PCIE_6 158 +#define GPP_vGPIO_PCIE_7 159 +#define GPP_vGPIO_PCIE_8 160 +#define GPP_vGPIO_PCIE_9 161 +#define GPP_vGPIO_PCIE_10 162 +#define GPP_vGPIO_PCIE_11 163 +#define GPP_vGPIO_PCIE_12 164 +#define GPP_vGPIO_PCIE_13 165 +#define GPP_vGPIO_PCIE_14 166 +#define GPP_vGPIO_PCIE_15 167 +#define GPP_vGPIO_PCIE_16 168 +#define GPP_vGPIO_PCIE_17 169 +#define GPP_vGPIO_PCIE_18 170 +#define GPP_vGPIO_PCIE_19 171 +#define GPP_vGPIO_PCIE_20 172 +#define GPP_vGPIO_PCIE_21 173 +#define GPP_vGPIO_PCIE_22 174 +#define GPP_vGPIO_PCIE_23 175 +#define GPP_vGPIO_PCIE_24 176 +#define GPP_vGPIO_PCIE_25 177 +#define GPP_vGPIO_PCIE_26 178 +#define GPP_vGPIO_PCIE_27 179 +#define GPP_vGPIO_PCIE_28 180 +#define GPP_vGPIO_PCIE_29 181 +#define GPP_vGPIO_PCIE_30 182 +#define GPP_vGPIO_PCIE_31 183 +#define GPP_vGPIO_PCIE_32 184 +#define GPP_vGPIO_PCIE_33 185 +#define GPP_vGPIO_PCIE_34 186 +#define GPP_vGPIO_PCIE_35 187 +#define GPP_vGPIO_PCIE_36 188 +#define GPP_vGPIO_PCIE_37 189 +#define GPP_vGPIO_PCIE_38 190 +#define GPP_vGPIO_PCIE_39 191 +#define GPP_vGPIO_PCIE_40 192 +#define GPP_vGPIO_PCIE_41 193 +#define GPP_vGPIO_PCIE_42 194 +#define GPP_vGPIO_PCIE_43 195 +#define GPP_vGPIO_PCIE_44 196 +#define GPP_vGPIO_PCIE_45 197 +#define GPP_vGPIO_PCIE_46 198 +#define GPP_vGPIO_PCIE_47 199 +#define GPP_vGPIO_PCIE_48 200 +#define GPP_vGPIO_PCIE_49 201 +#define GPP_vGPIO_PCIE_50 202 +#define GPP_vGPIO_PCIE_51 203 +#define GPP_vGPIO_PCIE_52 204 +#define GPP_vGPIO_PCIE_53 205 +#define GPP_vGPIO_PCIE_54 206 +#define GPP_vGPIO_PCIE_55 207 +#define GPP_vGPIO_PCIE_56 208 +#define GPP_vGPIO_PCIE_57 209 +#define GPP_vGPIO_PCIE_58 210 +#define GPP_vGPIO_PCIE_59 211 +#define GPP_vGPIO_PCIE_60 212 +#define GPP_vGPIO_PCIE_61 213 +#define GPP_vGPIO_PCIE_62 214 +#define GPP_vGPIO_PCIE_63 215 +#define GPP_vGPIO_PCIE_64 216 +#define GPP_vGPIO_PCIE_65 217 +#define GPP_vGPIO_PCIE_66 218 +#define GPP_vGPIO_PCIE_67 219 +#define GPP_vGPIO_PCIE_68 220 +#define GPP_vGPIO_PCIE_69 221 +#define GPP_vGPIO_PCIE_70 222 +#define GPP_vGPIO_PCIE_71 223 +#define GPP_vGPIO_PCIE_72 224 +#define GPP_vGPIO_PCIE_73 225 +#define GPP_vGPIO_PCIE_74 226 +#define GPP_vGPIO_PCIE_75 227 +#define GPP_vGPIO_PCIE_76 228 +#define GPP_vGPIO_PCIE_77 229 +#define GPP_vGPIO_PCIE_78 230 +#define GPP_vGPIO_PCIE_79 231 +#define GPP_CPU_RSVD_16 232 +#define GPP_CPU_RSVD_17 233 +#define GPP_CPU_RSVD_18 234 +#define GPP_CPU_RSVD_19 235 +#define GPP_CPU_RSVD_20 236 +#define GPP_CPU_RSVD_21 237 +#define GPP_CPU_RSVD_22 238 +#define GPP_CPU_RSVD_23 239 +#define GPP_vGPIO_PCIE_80 240 +#define GPP_vGPIO_PCIE_81 241 +#define GPP_vGPIO_PCIE_82 242 +#define GPP_vGPIO_PCIE_83 243 + +#define GPIO_COM3_START GPP_CPU_RSVD_1 +#define GPIO_COM3_END GPP_vGPIO_PCIE_83 +#define NUM_GPIO_COM3_PADS (GPIO_COM3_END - GPIO_COM3_START + 1) + /* Group C */ -#define GPP_C0 137 -#define GPP_C1 138 -#define GPP_C2 139 -#define GPP_C3 140 -#define GPP_C4 141 -#define GPP_C5 142 -#define GPP_C6 143 -#define GPP_C7 144 -#define GPP_C8 145 -#define GPP_C9 146 -#define GPP_C10 147 -#define GPP_C11 148 -#define GPP_C12 149 -#define GPP_C13 150 -#define GPP_C14 151 -#define GPP_C15 152 -#define GPP_C16 153 -#define GPP_C17 154 -#define GPP_C18 155 -#define GPP_C19 156 -#define GPP_C20 157 -#define GPP_C21 158 -#define GPP_C22 159 -#define GPP_C23 160 +#define GPP_C0 244 +#define GPP_C1 245 +#define GPP_C2 246 +#define GPP_C3 247 +#define GPP_C4 248 +#define GPP_C5 249 +#define GPP_C6 250 +#define GPP_C7 251 +#define GPP_C8 252 +#define GPP_C9 253 +#define GPP_C10 254 +#define GPP_C11 255 +#define GPP_C12 256 +#define GPP_C13 257 +#define GPP_C14 258 +#define GPP_C15 259 +#define GPP_C16 260 +#define GPP_C17 261 +#define GPP_C18 262 +#define GPP_C19 263 +#define GPP_C20 264 +#define GPP_C21 265 +#define GPP_C22 266 +#define GPP_C23 267 /* Group F */ -#define GPP_F0 161 -#define GPP_F1 162 -#define GPP_F2 163 -#define GPP_F3 164 -#define GPP_F4 165 -#define GPP_F5 166 -#define GPP_F6 167 -#define GPP_F7 168 -#define GPP_F8 169 -#define GPP_F9 170 -#define GPP_F10 171 -#define GPP_F11 172 -#define GPP_F12 173 -#define GPP_F13 174 -#define GPP_F14 175 -#define GPP_F15 176 -#define GPP_F16 177 -#define GPP_F17 178 -#define GPP_F18 179 -#define GPP_F19 180 -#define GPP_F20 181 -#define GPP_F21 182 -#define GPP_F22 183 -#define GPP_F23 184 -#define GPP_F_CLK_LOOPBK 185 +#define GPP_F0 268 +#define GPP_F1 269 +#define GPP_F2 270 +#define GPP_F3 271 +#define GPP_F4 272 +#define GPP_F5 273 +#define GPP_F6 274 +#define GPP_F7 275 +#define GPP_F8 276 +#define GPP_F9 277 +#define GPP_F10 278 +#define GPP_F11 279 +#define GPP_F12 280 +#define GPP_F13 281 +#define GPP_F14 282 +#define GPP_F15 283 +#define GPP_F16 284 +#define GPP_F17 285 +#define GPP_F18 286 +#define GPP_F19 287 +#define GPP_F20 288 +#define GPP_F21 289 +#define GPP_F22 290 +#define GPP_F23 291 +#define GPP_F_CLK_LOOPBK 292 /* Group HVMOS */ -#define GPP_L_BKLTEN 186 -#define GPP_L_BKLTCTL 187 -#define GPP_L_VDDEN 188 -#define GPP_SYS_PWROK 189 -#define GPP_SYS_RESETB 190 -#define GPP_MLK_RSTB 191 +#define GPP_L_BKLTEN 293 +#define GPP_L_BKLTCTL 294 +#define GPP_L_VDDEN 295 +#define GPP_SYS_PWROK 296 +#define GPP_SYS_RESETB 297 +#define GPP_MLK_RSTB 298 /* Group E */ -#define GPP_E0 192 -#define GPP_E1 193 -#define GPP_E2 194 -#define GPP_E3 195 -#define GPP_E4 196 -#define GPP_E5 197 -#define GPP_E6 198 -#define GPP_E7 199 -#define GPP_E8 200 -#define GPP_E9 201 -#define GPP_E10 202 -#define GPP_E11 203 -#define GPP_E12 204 -#define GPP_E13 205 -#define GPP_E14 206 -#define GPP_E15 207 -#define GPP_E16 208 -#define GPP_E17 209 -#define GPP_E18 210 -#define GPP_E19 211 -#define GPP_E20 212 -#define GPP_E21 213 -#define GPP_E22 214 -#define GPP_E23 215 -#define GPP_E_CLK_LOOPBK 216 +#define GPP_E0 299 +#define GPP_E1 300 +#define GPP_E2 301 +#define GPP_E3 302 +#define GPP_E4 303 +#define GPP_E5 304 +#define GPP_E6 305 +#define GPP_E7 306 +#define GPP_E8 307 +#define GPP_E9 308 +#define GPP_E10 309 +#define GPP_E11 310 +#define GPP_E12 311 +#define GPP_E13 312 +#define GPP_E14 313 +#define GPP_E15 314 +#define GPP_E16 315 +#define GPP_E17 316 +#define GPP_E18 317 +#define GPP_E19 318 +#define GPP_E20 319 +#define GPP_E21 320 +#define GPP_E22 321 +#define GPP_E23 322 +#define GPP_E_CLK_LOOPBK 323 #define GPIO_COM4_START GPP_C0 #define GPIO_COM4_END GPP_E_CLK_LOOPBK #define NUM_GPIO_COM4_PADS (GPIO_COM4_END - GPIO_COM4_START + 1) /* Group R */ -#define GPP_R0 217 -#define GPP_R1 218 -#define GPP_R2 219 -#define GPP_R3 220 -#define GPP_R4 221 -#define GPP_R5 222 -#define GPP_R6 223 -#define GPP_R7 224 +#define GPP_R0 324 +#define GPP_R1 325 +#define GPP_R2 326 +#define GPP_R3 327 +#define GPP_R4 328 +#define GPP_R5 329 +#define GPP_R6 330 +#define GPP_R7 331 /* Group SPI0 */ -#define GPP_SPI0_IO_2 225 -#define GPP_SPI0_IO_3 226 -#define GPP_SPI0_MOSI_IO_0 227 -#define GPP_SPI0_MOSI_IO_1 228 -#define GPP_SPI0_TPM_CSB 229 -#define GPP_SPI0_FLASH_0_CSB 230 -#define GPP_SPI0_FLASH_1_CSB 231 -#define GPP_SPI0_CLK 232 +#define GPP_SPI0_IO_2 332 +#define GPP_SPI0_IO_3 333 +#define GPP_SPI0_MOSI_IO_0 334 +#define GPP_SPI0_MOSI_IO_1 335 +#define GPP_SPI0_TPM_CSB 336 +#define GPP_SPI0_FLASH_0_CSB 337 +#define GPP_SPI0_FLASH_1_CSB 338 +#define GPP_SPI0_CLK 339 #define GPIO_COM5_START GPP_R0 #define GPIO_COM5_END GPP_SPI0_CLK -- To view, visit https://review.coreboot.org/c/coreboot/+/52783 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I481ea7e3ba948bf6d37b97d08c675a18ee68125d Gerrit-Change-Number: 52783 Gerrit-PatchSet: 7 Gerrit-Owner: Maulik V Vaghela <maulik.v.vaghela(a)intel.com> Gerrit-Reviewer: Bernardo Perez Priego <bernardo.perez.priego(a)intel.com> Gerrit-Reviewer: Bora Guvendik <bora.guvendik(a)intel.com> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Meera Ravindranath <meera.ravindranath(a)intel.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Ronak Kanabar <ronak.kanabar(a)intel.com> Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Patrick Georgi <pgeorgi(a)google.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-MessageType: merged

1 0

Change in coreboot[master]: soc/intel/common/block: Add definition for NAF_VWE bit for PAD_CFG0 reg
by Felix Held (Code Review) 05 May '21

05 May '21

Felix Held has submitted this change. ( https://review.coreboot.org/c/coreboot/+/52782 ) Change subject: soc/intel/common/block: Add definition for NAF_VWE bit for PAD_CFG0 reg ...................................................................... soc/intel/common/block: Add definition for NAF_VWE bit for PAD_CFG0 reg Earlier we did not have definition for BIT27 for PAD_CFG0 register, we will use this BIT to enable "virtual wire messaging for native function" If this bit is enabled, whenever change is detected on the pad, virtual wire message is generated and sent to destination set by native function. This bit must be set while enabling CPU PCIe root port programming for ADL and thus defining a new macro to set native pad function along with NAF_VWE bit to make GPIO programming easier from coreboot. BUG=None BRANCH=None TEST=Code compilation works fine and if we use this macro to program GPIO, proper bit is getting set in PAD_CFG register Change-Id: I732e68b413eb01b8ae1a4927836762c8875b73d2 Signed-off-by: Maulik V Vaghela <maulik.v.vaghela(a)intel.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/52782 Reviewed-by: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Reviewed-by: Subrata Banik <subrata.banik(a)intel.com> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> --- M src/soc/intel/common/block/include/intelblocks/gpio_defs.h 1 file changed, 8 insertions(+), 0 deletions(-) Approvals: build bot (Jenkins): Verified Subrata Banik: Looks good to me, but someone else must approve Tim Wawrzynczak: Looks good to me, approved diff --git a/src/soc/intel/common/block/include/intelblocks/gpio_defs.h b/src/soc/intel/common/block/include/intelblocks/gpio_defs.h index 2d6f62a..e5a536d 100644 --- a/src/soc/intel/common/block/include/intelblocks/gpio_defs.h +++ b/src/soc/intel/common/block/include/intelblocks/gpio_defs.h @@ -35,6 +35,7 @@ #define PAD_CFG0_TRIG_EDGE_SINGLE (1 << 25) /* controlled by RX_INVERT*/ #define PAD_CFG0_TRIG_OFF (2 << 25) #define PAD_CFG0_TRIG_EDGE_BOTH (3 << 25) +#define PAD_CFG0_NAFVWE_ENABLE (1 << 27) #define PAD_CFG0_RXRAW1_MASK (1 << 28) #define PAD_CFG0_RXPADSTSEL_MASK (1 << 29) #define PAD_CFG0_RESET_MASK (3 << 30) @@ -215,6 +216,13 @@ PAD_RESET(rst) | PAD_FUNC(func), PAD_PULL(pull) | \ PAD_IOSSTATE(iosstate) | PAD_IOSTERM(iosterm)) +/* Native function configuration with "native function virtual wire + messaging enable (NAFVWE_ENABLE)" */ +#define PAD_CFG_NF_VWEN(pad, pull, rst, func) \ + _PAD_CFG_STRUCT(pad, \ + PAD_RESET(rst) | PAD_FUNC(func) | PAD_CFG0_NAFVWE_ENABLE,\ + PAD_PULL(pull) | PAD_IOSSTATE(TxLASTRxE)) + /* General purpose output, no pullup/down. */ #define PAD_CFG_GPO(pad, val, rst) \ _PAD_CFG_STRUCT(pad, \ -- To view, visit https://review.coreboot.org/c/coreboot/+/52782 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I732e68b413eb01b8ae1a4927836762c8875b73d2 Gerrit-Change-Number: 52782 Gerrit-PatchSet: 5 Gerrit-Owner: Maulik V Vaghela <maulik.v.vaghela(a)intel.com> Gerrit-Reviewer: Bernardo Perez Priego <bernardo.perez.priego(a)intel.com> Gerrit-Reviewer: Bora Guvendik <bora.guvendik(a)intel.com> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Meera Ravindranath <meera.ravindranath(a)intel.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Ronak Kanabar <ronak.kanabar(a)intel.com> Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Patrick Georgi <pgeorgi(a)google.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-MessageType: merged

1 0

Change in coreboot[master]: mb/google/guybrush: Configure Pen Detect device
by Felix Held (Code Review) 05 May '21

05 May '21

Felix Held has submitted this change. ( https://review.coreboot.org/c/coreboot/+/52906 ) Change subject: mb/google/guybrush: Configure Pen Detect device ...................................................................... mb/google/guybrush: Configure Pen Detect device Pen Detect GPIO is exported through GPIO keys driver to the kernel so that stylus tools is popped on pen eject event. Hence enable the GPIO keys driver and configure the devicetree. BUG=b:186011392 TEST=Build and boot to OS in guybrush. Ensure that PRP0001 device is added to the ACPI SSDT table. Ensure that the Pen Eject events are detected. Event: time 1620159356.243180, type 5 (EV_SW), code 15 (SW_PEN_INSERTED), value 1 Event: time 1620159356.243180, -------------- SYN_REPORT ------------ Event: time 1620159356.735316, type 5 (EV_SW), code 15 (SW_PEN_INSERTED), value 0 Ensure that when the device is suspended, it wake on Pen Eject event and does not wake on Pen Insert event. Change-Id: I4d2aa29c0f1839c563b40734527a687a5618ba5c Signed-off-by: Karthikeyan Ramasubramanian <kramasub(a)google.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/52906 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Raul Rangel <rrangel(a)chromium.org> Reviewed-by: Furquan Shaikh <furquan(a)google.com> --- M src/mainboard/google/guybrush/Kconfig M src/mainboard/google/guybrush/variants/guybrush/overridetree.cb 2 files changed, 16 insertions(+), 0 deletions(-) Approvals: build bot (Jenkins): Verified Furquan Shaikh: Looks good to me, approved Raul Rangel: Looks good to me, approved diff --git a/src/mainboard/google/guybrush/Kconfig b/src/mainboard/google/guybrush/Kconfig index 51ac55d..8c23cf0 100644 --- a/src/mainboard/google/guybrush/Kconfig +++ b/src/mainboard/google/guybrush/Kconfig @@ -12,6 +12,7 @@ select DISABLE_KEYBOARD_RESET_PIN select DISABLE_SPI_FLASH_ROM_SHARING select DRIVERS_AMD_I2S_MACHINE_DEV + select DRIVERS_GENERIC_GPIO_KEYS select DRIVERS_I2C_GENERIC select DRIVERS_I2C_HID select DRIVERS_UART_ACPI diff --git a/src/mainboard/google/guybrush/variants/guybrush/overridetree.cb b/src/mainboard/google/guybrush/variants/guybrush/overridetree.cb index 53e821e..416d058 100644 --- a/src/mainboard/google/guybrush/variants/guybrush/overridetree.cb +++ b/src/mainboard/google/guybrush/variants/guybrush/overridetree.cb @@ -84,6 +84,21 @@ end end # I2C0 + device ref i2c_1 on + chip drivers/generic/gpio_keys + register "name" = ""PENH"" + register "gpio" = "ACPI_GPIO_INPUT_ACTIVE_LOW(GPIO_4)" + register "key.dev_name" = ""EJCT"" + register "key.wakeup_event_action" = "EV_ACT_DEASSERTED" + register "key.linux_code" = "SW_PEN_INSERTED" + register "key.linux_input_type" = "EV_SW" + register "key.label" = ""pen_eject"" + register "key.debounce_interval" = "100" + register "key.wakeup_route" = "WAKEUP_ROUTE_GPIO_IRQ" + device generic 0 on end + end + end # I2C1 + device ref i2c_2 on chip drivers/i2c/generic register "hid" = ""10EC5682"" -- To view, visit https://review.coreboot.org/c/coreboot/+/52906 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I4d2aa29c0f1839c563b40734527a687a5618ba5c Gerrit-Change-Number: 52906 Gerrit-PatchSet: 2 Gerrit-Owner: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Mathew King <mathewk(a)chromium.org> Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: merged

1 0

Change in coreboot[master]: soc/intel/common: Add new IRQ module
by Tim Wawrzynczak (Code Review) 05 May '21

05 May '21

1 0

Change in coreboot[master]: soc/intel/common/block/irq: Add support for intel_write_pci0_PRT
by Tim Wawrzynczak (Code Review) 05 May '21

05 May '21

Attention is currently required from: Nico Huber, Furquan Shaikh, Patrick Rudolph. Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/51159 ) Change subject: soc/intel/common/block/irq: Add support for intel_write_pci0_PRT ...................................................................... Patch Set 11: (1 comment) File src/soc/intel/common/block/irq/irq.c: https://review.coreboot.org/c/coreboot/+/51159/comment/187aafcd_461ac9a0 PS7, Line 378: /* Map INTA->PIRQ_A, INTB->PIRQ_B, INTC->PIRQ_C, INTD->PIRQ_D */ > for posterity: Sorry I misunderstood what you were getting at, we talked on IRC, I realized the erro […] Ack -- To view, visit https://review.coreboot.org/c/coreboot/+/51159 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I21a4835890ca03bff83ed0e8791441b3af54cb62 Gerrit-Change-Number: 51159 Gerrit-PatchSet: 11 Gerrit-Owner: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)google.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Furquan Shaikh <furquan(a)google.com> Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Comment-Date: Wed, 05 May 2021 22:35:32 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Comment-In-Reply-To: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: soc/intel/common: Add new IRQ module
by build bot (Jenkins) (Code Review) 05 May '21

05 May '21

Attention is currently required from: Nico Huber, Furquan Shaikh. build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/49408 ) Change subject: soc/intel/common: Add new IRQ module ...................................................................... Patch Set 27: (1 comment) File src/soc/intel/common/block/irq/irq.c: Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-118549): https://review.coreboot.org/c/coreboot/+/49408/comment/aa81a355_a8564c1e PS27, Line 209: printk(BIOS_ERR, "ERROR: fn %u no free or shared pins?\n", i); line over 96 characters -- To view, visit https://review.coreboot.org/c/coreboot/+/49408 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I0c22a08ce589fa80d0bb1e637422304a3af2045c Gerrit-Change-Number: 49408 Gerrit-PatchSet: 27 Gerrit-Owner: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: Aamir Bohra <aamir.bohra(a)intel.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Duncan Laurie Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)google.com> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Srinidhi N Kaushik <srinidhi.n.kaushik(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Nico Huber <nico.h(a)gmx.de> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Furquan Shaikh <furquan(a)google.com> Gerrit-Comment-Date: Wed, 05 May 2021 22:35:15 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in coreboot[master]: soc/intel/common: Add new IRQ module
by Tim Wawrzynczak (Code Review) 05 May '21

05 May '21

Attention is currently required from: Nico Huber, Furquan Shaikh. Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/49408 ) Change subject: soc/intel/common: Add new IRQ module ...................................................................... Patch Set 27: (19 comments) Commit Message: https://review.coreboot.org/c/coreboot/+/49408/comment/3d693774_48d6ec70 PS26, Line 25: meet the FSPs rules > nit: I think most(all but 1?) rules are what the BWG requires. Done. File src/soc/intel/common/block/include/intelblocks/irq.h: https://review.coreboot.org/c/coreboot/+/49408/comment/9f5dadad_e36d8b9a PS26, Line 14: irq_constraint_type > Can a single device have multiple constraints? e.g. […] Right now no, I have not come across a need for that (glanced at a few generations) https://review.coreboot.org/c/coreboot/+/49408/comment/36b83d03_c4b4a3c3 PS26, Line 26: IDENTITY > Haven't read rest of the CL yet, but it is not very clear from the comment above how `IDENTITY` is d […] Sure, I'll use my words instead of the BGW 😊 https://review.coreboot.org/c/coreboot/+/49408/comment/38cd9c9b_d2910afe PS26, Line 30: pci_devfn_t > Shouldn't this be `unsigned int`? Done https://review.coreboot.org/c/coreboot/+/49408/comment/ba99fe9b_ec485255 PS26, Line 32: pci_pin > Is this set only in case of `FIXED_INT_PIN`? Yes, I will rename it to make that more clear. https://review.coreboot.org/c/coreboot/+/49408/comment/d5daab60_ec47e163 PS26, Line 40: pci_devfn_t > `unsigned int`? Done File src/soc/intel/common/block/irq/irq.c: https://review.coreboot.org/c/coreboot/+/49408/comment/8e3ed9a2_53d115ec PS26, Line 13: MIN_IRQ > Should this be `MIN_SHARED_IRQ` so that it is consistent with other _SHARED_IRQ macros below? Done https://review.coreboot.org/c/coreboot/+/49408/comment/3014711b_864588b3 PS26, Line 17: #define TOTAL_IRQ (MAX_IRQ - MIN_IRQ + 1) : #define MAX_IRQ_ENTRIES 120 > These macros are unused. […] From an older patchset, removed. https://review.coreboot.org/c/coreboot/+/49408/comment/a77e37e3_77340e50 PS26, Line 21: MAX_PINS > Is this the same as PCI_INT_MAX? Yes, with PCI_INT_MAX now defined, will use that 😊 https://review.coreboot.org/c/coreboot/+/49408/comment/06f30125_b37a8f4b PS26, Line 82: > > Shouldn't this be ==? MAX_IRQ is 119 here... next patchset redefines it to 120 and then I think the logic here is a little more clear https://review.coreboot.org/c/coreboot/+/49408/comment/b4a97460_5dfe54cf PS26, Line 91: used > Instead of determining the pin state here and in `find_shareable_pin`, do you think if it would make […] Nice use of a better data structure to make this cleaner! My original choice was obviously the fn_pin_map and the pin_irq_map (linked by the pins) but I have a good feeling about this change 👍 https://review.coreboot.org/c/coreboot/+/49408/comment/02e42020_4ccc2754 PS26, Line 136: unsigned int share_count[TOTAL_SHARED_IRQ] = {0}; > IIUC, one of the reasons why you are recalculating the share_count every time is because we haven't yet added entries for the current slot. That's correct. > But, if we don't rely on `entries[]` to determine this and instead add a step in `assign_irqs()` to `increment_shared_count()` then share_count will always have the up-to-date information. SGTM https://review.coreboot.org/c/coreboot/+/49408/comment/15f75b26_3f49bdec PS26, Line 175: fn_pin_map > I am trying to understand what state `fn_pin_map[]` would be in. […] Let me see if I can get rid of the EMPTY_FN ...I think it's a relic from a previous patchset (a sentinel value that would stick out) that I didn't see effectively get removed 😊 also yes there was an assumption about the order there... will fix. https://review.coreboot.org/c/coreboot/+/49408/comment/57916951_007669aa PS26, Line 198: constraints[i].type == FIXED_INT_PIN || constraints[i].type == IDENTITY > I wonder if the constraints type should be a bit-field so that multiple can be set e.g. `FIXED_INT_PIN` and `IDENTITY`. `FIXED_INT_PIN` indicates that a fixed pin is allocated for the slot-function. `IDENTITY` indicates that the pin is mapped 1:1 to PIRQ #. Then, we don't need to make the assumption that IDENTITY implies fixed pin allocation. Interesting thought; IDENTITY is currently only applicable for PCIe RPs > Also, do we need to handle cases like a `FIXED_INT_PIN` also requires a `UNIQUE_IRQ`? I haven't come across one yet, but who knows what may come in the future... let me see how that looks. > This needs some more thought because I understand that having multiple bits set at the same time also requires more paths to handle in `assign_pins()`. Probably, will require bits in the same order as we will process them and use something like `fls()` to properly handle it. Indeed, there is a "precedence" that must be handled correctly between different constraint types to make sure the logic works out, so it would complicate things a little bit. https://review.coreboot.org/c/coreboot/+/49408/comment/f0c70c5a_85734eea PS26, Line 200: return false; > printk(BIOS_ERR, ""); to indicate that the SoC constraint is incorrect. Done https://review.coreboot.org/c/coreboot/+/49408/comment/feec364d_843cbdb0 PS26, Line 202: i > Same comment as above. This will have to be PCI_FUNC(... […] Done https://review.coreboot.org/c/coreboot/+/49408/comment/e4856b1c_d10a3f96 PS26, Line 208: (pin - PCI_INT_A) > PIN2IDX? Done https://review.coreboot.org/c/coreboot/+/49408/comment/ff925b4a_366d20e4 PS26, Line 214: fn_pin_map[0] = PCI_INT_A; > Should there be a check to ensure that fn_pin_map[0] is not already assigned to something other than […] I guess then there would be a conflict between a FIXED_INT_PIN (or IDENTITY) and a single-function device requirement... I'll add the check for now. https://review.coreboot.org/c/coreboot/+/49408/comment/cb2b936a_51f9a781 PS26, Line 226: if (constraints[i].type == UNIQUE_IRQ) { > If you invert this check, additional tab on rest of the lines can be avoided. […] Done -- To view, visit https://review.coreboot.org/c/coreboot/+/49408 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I0c22a08ce589fa80d0bb1e637422304a3af2045c Gerrit-Change-Number: 49408 Gerrit-PatchSet: 27 Gerrit-Owner: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: Aamir Bohra <aamir.bohra(a)intel.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Duncan Laurie Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)google.com> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Srinidhi N Kaushik <srinidhi.n.kaushik(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Nico Huber <nico.h(a)gmx.de> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Furquan Shaikh <furquan(a)google.com> Gerrit-Comment-Date: Wed, 05 May 2021 22:35:11 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Furquan Shaikh <furquan(a)google.com> Gerrit-MessageType: comment

1 0

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit May 2021