coreboot-gerrit May 2022

coreboot-gerrit@coreboot.org

1 participants
6166 discussions

Change in coreboot[master]: intelblocks: Add function to enable GPE
by Maulik V Vaghela (Code Review) 06 May '22

06 May '22

Attention is currently required from: Maulik V Vaghela, Tim Wawrzynczak. Hello build bot (Jenkins), Subrata Banik, Tim Wawrzynczak, Eric Lai, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/64089 to look at the new patch set (#2). Change subject: intelblocks: Add function to enable GPE ...................................................................... intelblocks: Add function to enable GPE coreboot was not programming GPE_EN register bits for GPIO. GPE_EN register programming is required for the GPIO pins which are capable of generating SCI for the system wake. Since coreboot locks GPIO pads during GPIO configuration, it was not possible for OS to program GPE_EN register. This patch add supports to program GPE_EN register before coreboot locks the GPIO registers. Note that coreboot will only program GPE_EN bits for GPIO which are capable of generating SCI. This will help resolve issue where we don't see wake event GPIO in event log. BUG=b:222375516 BRANCH=firmware-brya-14505.B TEST=Compile code for Brya and see GPE_EN bits set from the kernel console Change-Id: I27e525f50c374c2cc9675e77eaa7774683a6e7c2 Signed-off-by: Maulik V Vaghela <maulik.v.vaghela(a)intel.com> --- M src/soc/intel/common/block/gpio/gpio.c 1 file changed, 33 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/89/64089/2 -- To view, visit https://review.coreboot.org/c/coreboot/+/64089 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I27e525f50c374c2cc9675e77eaa7774683a6e7c2 Gerrit-Change-Number: 64089 Gerrit-PatchSet: 2 Gerrit-Owner: Maulik V Vaghela <maulik.v.vaghela(a)intel.com> Gerrit-Reviewer: Eric Lai <eric_lai(a)quanta.corp-partner.google.com> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Maulik V Vaghela <maulik.v.vaghela(a)intel.com> Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-MessageType: newpatchset

1 0

Change in coreboot[master]: soc/inte/*/gpio; Add GPE_EN and GPE_STS register definition
by Maulik V Vaghela (Code Review) 06 May '22

06 May '22

Attention is currently required from: Maulik V Vaghela, Tim Wawrzynczak. Hello build bot (Jenkins), Subrata Banik, Tim Wawrzynczak, Eric Lai, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/64088 to look at the new patch set (#2). Change subject: soc/inte/*/gpio; Add GPE_EN and GPE_STS register definition ...................................................................... soc/inte/*/gpio; Add GPE_EN and GPE_STS register definition coreboot needs to set GPE_EN bit for the GPIOs which are wake capable from s0ix/sleep. Due to GPIO locking mechanism, coreboot/OS will not be able to write GPE_EN register post GPIO has been locked. This patch adds support in SoC code to provide correct offset for GPE_EN and GPE_STS registers to the common code. Plan is to use this offsets to set GPE_EN bits before GPIO locking in coreboot which will be part of subsequent CL. BUG=b:222375516 BRANCH=firmware-brya-14505.B TEST=Check if code compiles for Brya and correct offset values are printed. Change-Id: I6b813b30b8b360f8eccbf539b57387310e380560 Signed-off-by: Maulik V Vaghela <maulik.v.vaghela(a)intel.com> --- M src/soc/intel/alderlake/gpio.c M src/soc/intel/alderlake/include/soc/gpio_defs.h M src/soc/intel/cannonlake/gpio.c M src/soc/intel/cannonlake/include/soc/gpio_defs.h M src/soc/intel/common/block/include/intelblocks/gpio.h M src/soc/intel/tigerlake/gpio.c M src/soc/intel/tigerlake/include/soc/gpio_defs.h 7 files changed, 40 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/88/64088/2 -- To view, visit https://review.coreboot.org/c/coreboot/+/64088 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I6b813b30b8b360f8eccbf539b57387310e380560 Gerrit-Change-Number: 64088 Gerrit-PatchSet: 2 Gerrit-Owner: Maulik V Vaghela <maulik.v.vaghela(a)intel.com> Gerrit-Reviewer: Eric Lai <eric_lai(a)quanta.corp-partner.google.com> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Maulik V Vaghela <maulik.v.vaghela(a)intel.com> Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-MessageType: newpatchset

1 0

Change in coreboot[master]: intelblocks: Add function to enable GPE
by Eric Lai (Code Review) 06 May '22

06 May '22

Attention is currently required from: Maulik V Vaghela, Tim Wawrzynczak. Eric Lai has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/64089 ) Change subject: intelblocks: Add function to enable GPE ...................................................................... Patch Set 1: Code-Review+1 (1 comment) Patchset: PS1: oh, yes. I think this should work. -- To view, visit https://review.coreboot.org/c/coreboot/+/64089 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I27e525f50c374c2cc9675e77eaa7774683a6e7c2 Gerrit-Change-Number: 64089 Gerrit-PatchSet: 1 Gerrit-Owner: Maulik V Vaghela <maulik.v.vaghela(a)intel.com> Gerrit-Reviewer: Eric Lai <eric_lai(a)quanta.corp-partner.google.com> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Maulik V Vaghela <maulik.v.vaghela(a)intel.com> Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Comment-Date: Fri, 06 May 2022 07:05:41 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in coreboot[master]: soc/intel/ehl: Provide function to change PHY-to-MAC IRQ polarity
by Werner Zeh (Code Review) 06 May '22

06 May '22

Attention is currently required from: Paul Menzel, Mario Scheithauer. Werner Zeh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/63888 ) Change subject: soc/intel/ehl: Provide function to change PHY-to-MAC IRQ polarity ...................................................................... Patch Set 5: (12 comments) File src/soc/intel/elkhartlake/chip.h: https://review.coreboot.org/c/coreboot/+/63888/comment/d3b3713d_ce5b6e66 PS5, Line 481: change PHY-to-MAC IRQ polarity set Phy-to-MAC IRQ polarity to rising edge https://review.coreboot.org/c/coreboot/+/63888/comment/0760a6f3_d539fde3 PS5, Line 482: bool PchTsnPhy2MacIrqActiveHigh; : bool PseTsnPhy2MacIrqActiveHigh[MAX_PSE_TSN_PORTS]; How about calling it {Pch,Pse}TsnPhyIRQEdge and using default (0) as falling edge while 1 as rising edge? File src/soc/intel/elkhartlake/include/soc/tsn_gbe.h: https://review.coreboot.org/c/coreboot/+/63888/comment/6cb1060f_01a41022 PS5, Line 11: #define TSN_GMII_TIMEOUT_MS 20 : : #define TSN_MAC_MDIO_ADR 0x200 /* MAC MDIO Address register */ : #define TSN_MAC_MDIO_ADR_MASK 0x03FF7F0E : #define TSN_MAC_PHYAD(pa) (pa << 21) /* Physical Layer Address */ : #define TSN_MAC_REGAD(rda) (rda << 16) /* Register/Device Address */ : #define TSN_MAC_CLK_TRAIL_4 (4 << 12) /* 4 Trailing Clocks */ : #define TSN_MAC_CSR_CLK_DIV_62 (1 << 8) /* 0001: CSR = 100-150 MHz; CSR/62 */ : #define TSN_MAC_OP_CMD_WRITE (1 << 2) /* GMII Operation Command Write */ : #define TSN_MAC_OP_CMD_READ (3 << 2) /* GMII Operation Command Read */ : #define TSN_MAC_GMII_BUSY (1 << 0) /* GMII Busy bit */ : : #define TSN_MAC_MDIO_ADHOC_ADR 0x15 /* MDIO - Adhoc PHY Sublayer Register */ : #define TSN_MAC_MDIO_GCR 0x0 /* Global Configuration Register */ : #define TSN_MAC_PHY2MAC_INTR_POL (1 << 6) /* PHY to MAC Interrupt Polarity bit */ : : #define TSN_MAC_MDIO_DATA 0x204 /* MAC MDIO Data register */ Align the TABs https://review.coreboot.org/c/coreboot/+/63888/comment/03cbe401_69a1dc5a PS5, Line 33: phy_gmii_busy_status maybe better 'phy_gmii_ready()' to indicate that this function waits for the PHY to become ready? File src/soc/intel/elkhartlake/tsn_gbe.c: https://review.coreboot.org/c/coreboot/+/63888/comment/4e964f38_a475bb7d PS5, Line 49: = 0 Not needed since it will be overwritten on line 63. https://review.coreboot.org/c/coreboot/+/63888/comment/0b48fe90_334f1e03 PS5, Line 57: transfer complete transfer to complete https://review.coreboot.org/c/coreboot/+/63888/comment/83d7c50f_d90c3f06 PS5, Line 60: printk(BIOS_ERR, "TSN GMII Busy. MIDO phy_add: 0x%x, Register 0x%x " : "read invalid\n", phy_adr, reg_adr); Can we get this a bit shorter to fit the string at least on a single line? https://review.coreboot.org/c/coreboot/+/63888/comment/04fdd77c_f325ee4c PS5, Line 64: printk(BIOS_DEBUG, "TSN MDIO Read phy_add: 0x%x, reg_add: 0x%x , " : "Value: 0x%x\n", phy_adr, reg_adr, mac_mdio_data); Same here. https://review.coreboot.org/c/coreboot/+/63888/comment/8b2564b9_647134ab PS5, Line 70: void tsn_mdio_write(uint32_t base, uint8_t phy_adr, uint8_t reg_adr, : uint16_t mac_mdio_data) This fits on a single line. https://review.coreboot.org/c/coreboot/+/63888/comment/53d28491_9f11e339 PS5, Line 84: printk(BIOS_ERR, "TSN GMII Busy. MIDO phy_add: 0x%x, Register 0x%x " : "read invalid\n", phy_adr, reg_adr); Same string length comment applies here. https://review.coreboot.org/c/coreboot/+/63888/comment/aff8e982_5a1fed7e PS5, Line 95: uint16_t gcr_reg; Variable definition first. https://review.coreboot.org/c/coreboot/+/63888/comment/ebb68d0f_cfc055e0 PS5, Line 117: if (config->PchTsnPhy2MacIrqActiveHigh) : irq_active_high = true; What about 'irq_active_high = !!config->PchTsnPhy2MacIrqActiveHigh;' ? -- To view, visit https://review.coreboot.org/c/coreboot/+/63888 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ia314014c7cacbeb72629c773c8c0bb5f002a3f54 Gerrit-Change-Number: 63888 Gerrit-PatchSet: 5 Gerrit-Owner: Mario Scheithauer <mario.scheithauer(a)siemens.com> Gerrit-Reviewer: Werner Zeh <werner.zeh(a)siemens.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Mario Scheithauer <mario.scheithauer(a)siemens.com> Gerrit-Comment-Date: Fri, 06 May 2022 07:00:26 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in coreboot[master]: soc/inte/*/gpio; Add GPE_EN and GPE_STS register definition
by Eric Lai (Code Review) 06 May '22

06 May '22

Attention is currently required from: Maulik V Vaghela, Tim Wawrzynczak. Eric Lai has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/64088 ) Change subject: soc/inte/*/gpio; Add GPE_EN and GPE_STS register definition ...................................................................... Patch Set 1: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/64088 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I6b813b30b8b360f8eccbf539b57387310e380560 Gerrit-Change-Number: 64088 Gerrit-PatchSet: 1 Gerrit-Owner: Maulik V Vaghela <maulik.v.vaghela(a)intel.com> Gerrit-Reviewer: Eric Lai <eric_lai(a)quanta.corp-partner.google.com> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Maulik V Vaghela <maulik.v.vaghela(a)intel.com> Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Comment-Date: Fri, 06 May 2022 06:59:52 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in coreboot[master]: intelblocks: Add function to enable GPE
by Subrata Banik (Code Review) 06 May '22

06 May '22

Attention is currently required from: Maulik V Vaghela, Tim Wawrzynczak, Eric Lai. Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/64089 ) Change subject: intelblocks: Add function to enable GPE ...................................................................... Patch Set 1: Code-Review+1 (3 comments) Patchset: PS1: Thanks Maulik for fixing this in proper. File src/soc/intel/common/block/gpio/gpio.c: https://review.coreboot.org/c/coreboot/+/64089/comment/2d7dc8c1_6acfde1a PS1, Line 173: tabs ? https://review.coreboot.org/c/coreboot/+/64089/comment/702382bb_5d17b58c PS1, Line 193: if( if ( -- To view, visit https://review.coreboot.org/c/coreboot/+/64089 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I27e525f50c374c2cc9675e77eaa7774683a6e7c2 Gerrit-Change-Number: 64089 Gerrit-PatchSet: 1 Gerrit-Owner: Maulik V Vaghela <maulik.v.vaghela(a)intel.com> Gerrit-Reviewer: Eric Lai <eric_lai(a)quanta.corp-partner.google.com> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Maulik V Vaghela <maulik.v.vaghela(a)intel.com> Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Attention: Eric Lai <eric_lai(a)quanta.corp-partner.google.com> Gerrit-Comment-Date: Fri, 06 May 2022 06:51:44 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in coreboot[master]: soc/inte/*/gpio; Add GPE_EN and GPE_STS register definition
by Subrata Banik (Code Review) 06 May '22

06 May '22

Attention is currently required from: Maulik V Vaghela, Tim Wawrzynczak, Eric Lai. Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/64088 ) Change subject: soc/inte/*/gpio; Add GPE_EN and GPE_STS register definition ...................................................................... Patch Set 1: Code-Review+2 (1 comment) File src/soc/intel/common/block/include/intelblocks/gpio.h: https://review.coreboot.org/c/coreboot/+/64088/comment/9b12d29f_2dc883cb PS1, Line 129: tab? -- To view, visit https://review.coreboot.org/c/coreboot/+/64088 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I6b813b30b8b360f8eccbf539b57387310e380560 Gerrit-Change-Number: 64088 Gerrit-PatchSet: 1 Gerrit-Owner: Maulik V Vaghela <maulik.v.vaghela(a)intel.com> Gerrit-Reviewer: Eric Lai <eric_lai(a)quanta.corp-partner.google.com> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Attention: Maulik V Vaghela <maulik.v.vaghela(a)intel.com> Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Attention: Eric Lai <eric_lai(a)quanta.corp-partner.google.com> Gerrit-Comment-Date: Fri, 06 May 2022 06:49:50 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in coreboot[master]: mb/siemens/mc_ehl2: Enable TSN GbE driver
by Werner Zeh (Code Review) 06 May '22

06 May '22

Attention is currently required from: Mario Scheithauer. Werner Zeh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/63864 ) Change subject: mb/siemens/mc_ehl2: Enable TSN GbE driver ...................................................................... Patch Set 4: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/63864 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ia052c44feb606f9e1d31d047f2acc67e3226a895 Gerrit-Change-Number: 63864 Gerrit-PatchSet: 4 Gerrit-Owner: Mario Scheithauer <mario.scheithauer(a)siemens.com> Gerrit-Reviewer: Werner Zeh <werner.zeh(a)siemens.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Mario Scheithauer <mario.scheithauer(a)siemens.com> Gerrit-Comment-Date: Fri, 06 May 2022 06:42:30 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in coreboot[master]: soc/intel/elkhartlake: Provide ability to update TSN GbE MAC addresses
by Werner Zeh (Code Review) 06 May '22

06 May '22

Attention is currently required from: Paul Menzel, Mario Scheithauer. Werner Zeh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/63863 ) Change subject: soc/intel/elkhartlake: Provide ability to update TSN GbE MAC addresses ...................................................................... Patch Set 4: Code-Review+1 (1 comment) File src/soc/intel/elkhartlake/tsn_gbe.c: https://review.coreboot.org/c/coreboot/+/63863/comment/b22d9d06_f835fe5a PS4, Line 8: static void program_mac_address(struct device *dev, uint32_t base) Since you pass in the device into this function: Would it make sense to let this function get the needed BAR itself? Would de-couple the code a bit more. -- To view, visit https://review.coreboot.org/c/coreboot/+/63863 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I2303a64cfd09fa02734ca9452d26591af2a76221 Gerrit-Change-Number: 63863 Gerrit-PatchSet: 4 Gerrit-Owner: Mario Scheithauer <mario.scheithauer(a)siemens.com> Gerrit-Reviewer: Werner Zeh <werner.zeh(a)siemens.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Mario Scheithauer <mario.scheithauer(a)siemens.com> Gerrit-Comment-Date: Fri, 06 May 2022 06:42:20 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in coreboot[master]: mb/siemens/mc_ehl2: Set PCH TSN link speed to 1 Gbps in devicetree
by Werner Zeh (Code Review) 06 May '22

06 May '22

Attention is currently required from: Paul Menzel, Mario Scheithauer. Werner Zeh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/64033 ) Change subject: mb/siemens/mc_ehl2: Set PCH TSN link speed to 1 Gbps in devicetree ...................................................................... Patch Set 1: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/64033 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I9f1da971b4de5671d6d38be6dbc50edbbe20d157 Gerrit-Change-Number: 64033 Gerrit-PatchSet: 1 Gerrit-Owner: Mario Scheithauer <mario.scheithauer(a)siemens.com> Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Reviewer: Werner Zeh <werner.zeh(a)siemens.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Mario Scheithauer <mario.scheithauer(a)siemens.com> Gerrit-Comment-Date: Fri, 06 May 2022 06:42:02 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit May 2022