Change in coreboot[master]: mb/google/volteer/variants/volteer: route GPP_F14 via APIC

List overview All Threads
Download

newer

older

Change in coreboot[master]:...

Alex Levin (Code Review)

31 Aug 2020 31 Aug '20

11:26 p.m.

Alex Levin has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/44948 )

Change subject: mb/google/volteer/variants/volteer: route GPP_F14 via APIC ......................................................................

mb/google/volteer/variants/volteer: route GPP_F14 via APIC

GPP_F14 should be configured to be routed via APIC and not SCI.

BUG=b:162722965 TEST=verified on a volteer

Change-Id: Ie262ceeaea1c07bcc99e1545f5eb99e0d0dee905 Signed-off-by: Alex Levin levinale@google.com --- M src/mainboard/google/volteer/variants/volteer/gpio.c 1 file changed, 1 insertion(+), 1 deletion(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/48/44948/1

diff --git a/src/mainboard/google/volteer/variants/volteer/gpio.c b/src/mainboard/google/volteer/variants/volteer/gpio.c index fd355a8..2d0d5e0 100644 --- a/src/mainboard/google/volteer/variants/volteer/gpio.c +++ b/src/mainboard/google/volteer/variants/volteer/gpio.c @@ -125,7 +125,7 @@ /* F13 : GSXDOUT ==> WiFi_DISABLE_L */ PAD_CFG_GPO(GPP_F13, 1, DEEP), /* F14 : GSXDIN ==> SAR0_INT_L */ - PAD_CFG_GPI_SCI_LOW(GPP_F14, NONE, PLTRST, EDGE_SINGLE), + PAD_CFG_GPI_APIC(GPP_F14, NONE, PLTRST, LEVEL, INVERT), /* F15 : GSXSRESET# ==> RCAM_RST_L */ PAD_CFG_GPO(GPP_F15, 1, DEEP), /* F16 : GSXCLK ==> WWAN_DPR_SAR_ODL */

Attachments:

attachment.htm (text/html — 2.7 KB)

Show replies by date

Aaron Durbin (Code Review)

31 Aug 31 Aug

11:37 p.m.

Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44948 )

Change subject: mb/google/volteer/variants/volteer: route GPP_F14 via APIC ......................................................................

Patch Set 1:

(1 comment)

https://review.coreboot.org/c/coreboot/+/44948/1/src/mainboard/google/voltee... File src/mainboard/google/volteer/variants/volteer/gpio.c:

https://review.coreboot.org/c/coreboot/+/44948/1/src/mainboard/google/voltee... PS1, Line 128: PAD_CFG_GPI_APIC(GPP_F14, NONE, PLTRST, LEVEL, INVERT), Why the inversion? What code is referencing this IRQ?

-- To view, visit https://review.coreboot.org/c/coreboot/+/44948 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ie262ceeaea1c07bcc99e1545f5eb99e0d0dee905 Gerrit-Change-Number: 44948 Gerrit-PatchSet: 1 Gerrit-Owner: Alex Levin levinale@google.com Gerrit-Reviewer: Aaron Durbin adurbin@chromium.org Gerrit-Reviewer: Nick Vaccaro nvaccaro@google.com Gerrit-Reviewer: Tim Wawrzynczak twawrzynczak@google.com Gerrit-Comment-Date: Mon, 31 Aug 2020 21:37:29 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

Alex Levin (Code Review)

11:40 p.m.

Alex Levin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44948 )

Change subject: mb/google/volteer/variants/volteer: route GPP_F14 via APIC ......................................................................

Patch Set 1:

(1 comment)

https://review.coreboot.org/c/coreboot/+/44948/1/src/mainboard/google/voltee... File src/mainboard/google/volteer/variants/volteer/gpio.c:

https://review.coreboot.org/c/coreboot/+/44948/1/src/mainboard/google/voltee... PS1, Line 128: PAD_CFG_GPI_APIC(GPP_F14, NONE, PLTRST, LEVEL, INVERT),

...

Why the inversion? What code is referencing this IRQ?

sx9310 is referencing this. I thought that inversion would we the way to mark this as being active low. from your question I realize I might be wrong 😊

-- To view, visit https://review.coreboot.org/c/coreboot/+/44948 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ie262ceeaea1c07bcc99e1545f5eb99e0d0dee905 Gerrit-Change-Number: 44948 Gerrit-PatchSet: 1 Gerrit-Owner: Alex Levin levinale@google.com Gerrit-Reviewer: Aaron Durbin adurbin@chromium.org Gerrit-Reviewer: Nick Vaccaro nvaccaro@google.com Gerrit-Reviewer: Tim Wawrzynczak twawrzynczak@google.com Gerrit-Comment-Date: Mon, 31 Aug 2020 21:40:47 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Aaron Durbin adurbin@chromium.org Gerrit-MessageType: comment

Aaron Durbin (Code Review)

11:46 p.m.

Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44948 )

Change subject: mb/google/volteer/variants/volteer: route GPP_F14 via APIC ......................................................................

Patch Set 1:

(1 comment)

https://review.coreboot.org/c/coreboot/+/44948/1/src/mainboard/google/voltee... File src/mainboard/google/volteer/variants/volteer/gpio.c:

https://review.coreboot.org/c/coreboot/+/44948/1/src/mainboard/google/voltee... PS1, Line 128: PAD_CFG_GPI_APIC(GPP_F14, NONE, PLTRST, LEVEL, INVERT),

...

sx9310 is referencing this. […]

If there apic redirection entry is active low there's no need to invert because the inversion for the event will be filtered in the apic. It really depends on that.

I see in src/drivers/i2c/sx9310/sx9310.c:

...

-------if (config->irq_gpio.pin_count) ------->-------acpi_device_write_gpio(&config->irq_gpio); -------else ------->-------acpi_device_write_interrupt(&config->irq);

and in the devictree files:

$ git grep -A 3 -i sx9310 -- src/mainboard/google/volteer/ | grep irq | awk '{ print $NF }' | uniq -c 8 "ACPI_IRQ_LEVEL_LOW(GPP_F14_IRQ)"

So you not invert this and pass it through as level in the pad config.

-- To view, visit https://review.coreboot.org/c/coreboot/+/44948 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ie262ceeaea1c07bcc99e1545f5eb99e0d0dee905 Gerrit-Change-Number: 44948 Gerrit-PatchSet: 1 Gerrit-Owner: Alex Levin levinale@google.com Gerrit-Reviewer: Aaron Durbin adurbin@chromium.org Gerrit-Reviewer: Nick Vaccaro nvaccaro@google.com Gerrit-Reviewer: Tim Wawrzynczak twawrzynczak@google.com Gerrit-Comment-Date: Mon, 31 Aug 2020 21:46:00 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Alex Levin levinale@google.com Comment-In-Reply-To: Aaron Durbin adurbin@chromium.org Gerrit-MessageType: comment

Alex Levin (Code Review)

1 Sep 1 Sep

12:03 a.m.

Alex Levin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44948 )

Change subject: mb/google/volteer/variants/volteer: route GPP_F14 via APIC ......................................................................

Patch Set 1:

(1 comment)

https://review.coreboot.org/c/coreboot/+/44948/1/src/mainboard/google/voltee... File src/mainboard/google/volteer/variants/volteer/gpio.c:

https://review.coreboot.org/c/coreboot/+/44948/1/src/mainboard/google/voltee... PS1, Line 128: PAD_CFG_GPI_APIC(GPP_F14, NONE, PLTRST, LEVEL, INVERT),

...

If there apic redirection entry is active low there's no need to invert because the inversion for th […]

thanks for the explanation.

-- To view, visit https://review.coreboot.org/c/coreboot/+/44948 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ie262ceeaea1c07bcc99e1545f5eb99e0d0dee905 Gerrit-Change-Number: 44948 Gerrit-PatchSet: 1 Gerrit-Owner: Alex Levin levinale@google.com Gerrit-Reviewer: Aaron Durbin adurbin@chromium.org Gerrit-Reviewer: Nick Vaccaro nvaccaro@google.com Gerrit-Reviewer: Tim Wawrzynczak twawrzynczak@google.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-Comment-Date: Mon, 31 Aug 2020 22:03:20 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Alex Levin levinale@google.com Comment-In-Reply-To: Aaron Durbin adurbin@chromium.org Gerrit-MessageType: comment

Alex Levin (Code Review)

12:03 a.m.

Hello build bot (Jenkins), Nick Vaccaro, Tim Wawrzynczak, Aaron Durbin,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/44948

to look at the new patch set (#2).

Change subject: mb/google/volteer/variants/volteer: route GPP_F14 via APIC ......................................................................

mb/google/volteer/variants/volteer: route GPP_F14 via APIC

GPP_F14 should be configured to be routed via APIC and not SCI.

BUG=b:162722965 TEST=verified on a volteer

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/48/44948/2

Alex Levin (Code Review)

12:08 a.m.

Hello build bot (Jenkins), Nick Vaccaro, Tim Wawrzynczak, Aaron Durbin,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/44948

to look at the new patch set (#3).

Change subject: mb/google/volteer/variants/volteer: route GPP_F14 via APIC ......................................................................

mb/google/volteer/variants/volteer: route GPP_F14 via APIC

GPP_F14 should be configured to be routed via APIC and not SCI.

BUG=b:162528549 TEST=verified on a volteer

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/48/44948/3

Aaron Durbin (Code Review)

12:29 a.m.

Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44948 )

Change subject: mb/google/volteer/variants/volteer: route GPP_F14 via APIC ......................................................................

Patch Set 3:

Same patch pushed as original.

-- To view, visit https://review.coreboot.org/c/coreboot/+/44948 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ie262ceeaea1c07bcc99e1545f5eb99e0d0dee905 Gerrit-Change-Number: 44948 Gerrit-PatchSet: 3 Gerrit-Owner: Alex Levin levinale@google.com Gerrit-Reviewer: Aaron Durbin adurbin@chromium.org Gerrit-Reviewer: Nick Vaccaro nvaccaro@google.com Gerrit-Reviewer: Tim Wawrzynczak twawrzynczak@google.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-Comment-Date: Mon, 31 Aug 2020 22:29:42 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment

Alex Levin (Code Review)

12:31 a.m.

Hello build bot (Jenkins), Nick Vaccaro, Tim Wawrzynczak, Aaron Durbin,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/44948

to look at the new patch set (#4).

Change subject: mb/google/volteer/variants/volteer: route GPP_F14 via APIC ......................................................................

mb/google/volteer/variants/volteer: route GPP_F14 via APIC

GPP_F14 should be configured to be routed via APIC and not SCI.

BUG=b:162528549 TEST=verified on a volteer

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/48/44948/4

Aaron Durbin (Code Review)

12:35 a.m.

Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44948 )

Change subject: mb/google/volteer/variants/volteer: route GPP_F14 via APIC ......................................................................

Patch Set 4: Code-Review+2

-- To view, visit https://review.coreboot.org/c/coreboot/+/44948 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ie262ceeaea1c07bcc99e1545f5eb99e0d0dee905 Gerrit-Change-Number: 44948 Gerrit-PatchSet: 4 Gerrit-Owner: Alex Levin levinale@google.com Gerrit-Reviewer: Aaron Durbin adurbin@chromium.org Gerrit-Reviewer: Nick Vaccaro nvaccaro@google.com Gerrit-Reviewer: Tim Wawrzynczak twawrzynczak@google.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-Comment-Date: Mon, 31 Aug 2020 22:35:16 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

Tim Wawrzynczak (Code Review)

12:37 a.m.

Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44948 )

Change subject: mb/google/volteer/variants/volteer: route GPP_F14 via APIC ......................................................................

Patch Set 4: Code-Review+2

-- To view, visit https://review.coreboot.org/c/coreboot/+/44948 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ie262ceeaea1c07bcc99e1545f5eb99e0d0dee905 Gerrit-Change-Number: 44948 Gerrit-PatchSet: 4 Gerrit-Owner: Alex Levin levinale@google.com Gerrit-Reviewer: Aaron Durbin adurbin@chromium.org Gerrit-Reviewer: Nick Vaccaro nvaccaro@google.com Gerrit-Reviewer: Tim Wawrzynczak twawrzynczak@chromium.org Gerrit-Reviewer: Tim Wawrzynczak twawrzynczak@google.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-Comment-Date: Mon, 31 Aug 2020 22:37:50 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

Tim Wawrzynczak (Code Review)

3 Sep 3 Sep

9:48 p.m.

Tim Wawrzynczak has submitted this change. ( https://review.coreboot.org/c/coreboot/+/44948 )

Change subject: mb/google/volteer/variants/volteer: route GPP_F14 via APIC ......................................................................

mb/google/volteer/variants/volteer: route GPP_F14 via APIC

GPP_F14 should be configured to be routed via APIC and not SCI.

BUG=b:162528549 TEST=verified on a volteer

Change-Id: Ie262ceeaea1c07bcc99e1545f5eb99e0d0dee905 Signed-off-by: Alex Levin levinale@google.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/44948 Reviewed-by: Aaron Durbin adurbin@chromium.org Reviewed-by: Tim Wawrzynczak twawrzynczak@chromium.org Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M src/mainboard/google/volteer/variants/volteer/gpio.c 1 file changed, 1 insertion(+), 1 deletion(-)

Approvals: build bot (Jenkins): Verified Aaron Durbin: Looks good to me, approved Tim Wawrzynczak: Looks good to me, approved

diff --git a/src/mainboard/google/volteer/variants/volteer/gpio.c b/src/mainboard/google/volteer/variants/volteer/gpio.c index fd355a8..bd84585 100644 --- a/src/mainboard/google/volteer/variants/volteer/gpio.c +++ b/src/mainboard/google/volteer/variants/volteer/gpio.c @@ -125,7 +125,7 @@ /* F13 : GSXDOUT ==> WiFi_DISABLE_L */ PAD_CFG_GPO(GPP_F13, 1, DEEP), /* F14 : GSXDIN ==> SAR0_INT_L */ - PAD_CFG_GPI_SCI_LOW(GPP_F14, NONE, PLTRST, EDGE_SINGLE), + PAD_CFG_GPI_APIC(GPP_F14, NONE, PLTRST, LEVEL, NONE), /* F15 : GSXSRESET# ==> RCAM_RST_L */ PAD_CFG_GPO(GPP_F15, 1, DEEP), /* F16 : GSXCLK ==> WWAN_DPR_SAR_ODL */

-- To view, visit https://review.coreboot.org/c/coreboot/+/44948 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ie262ceeaea1c07bcc99e1545f5eb99e0d0dee905 Gerrit-Change-Number: 44948 Gerrit-PatchSet: 5 Gerrit-Owner: Alex Levin levinale@google.com Gerrit-Reviewer: Aaron Durbin adurbin@chromium.org Gerrit-Reviewer: Nick Vaccaro nvaccaro@google.com Gerrit-Reviewer: Tim Wawrzynczak twawrzynczak@chromium.org Gerrit-Reviewer: Tim Wawrzynczak twawrzynczak@google.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-MessageType: merged

9elements QA (Code Review)

10:38 p.m.

9elements QA has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44948 )

Change subject: mb/google/volteer/variants/volteer: route GPP_F14 via APIC ......................................................................

Patch Set 5:

Automatic boot test returned (PASS/FAIL/TOTAL): 7/1/8 "QEMU x86 q35/ich9" (x86_32) using payload TianoCore : SUCCESS : https://lava.9esec.io/r/17737 "QEMU x86 q35/ich9" (x86_32) using payload SeaBIOS : SUCCESS : https://lava.9esec.io/r/17736 "QEMU x86 i440fx/piix4" (x86_64) using payload SeaBIOS : FAIL : https://lava.9esec.io/r/17735 "QEMU x86 i440fx/piix4" (x86_32) using payload SeaBIOS : SUCCESS : https://lava.9esec.io/r/17734 "QEMU AArch64" using payload LinuxBoot_u-root_kexec : SUCCESS : https://lava.9esec.io/r/17733 "HP Z220 SFF Workstation" (x86_32) using payload LinuxBoot_BusyBox_kexec : SUCCESS : https://lava.9esec.io/r/17740 "HP Compaq 8200 Elite SFF PC" (x86_32) using payload TianoCore : SUCCESS : https://lava.9esec.io/r/17739 "HP Compaq 8200 Elite SFF PC" (x86_32) using payload SeaBIOS : SUCCESS : https://lava.9esec.io/r/17738

Please note: This test is under development and might not be accurate at all!

-- To view, visit https://review.coreboot.org/c/coreboot/+/44948 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ie262ceeaea1c07bcc99e1545f5eb99e0d0dee905 Gerrit-Change-Number: 44948 Gerrit-PatchSet: 5 Gerrit-Owner: Alex Levin levinale@google.com Gerrit-Reviewer: Aaron Durbin adurbin@chromium.org Gerrit-Reviewer: Nick Vaccaro nvaccaro@google.com Gerrit-Reviewer: Tim Wawrzynczak twawrzynczak@chromium.org Gerrit-Reviewer: Tim Wawrzynczak twawrzynczak@google.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: 9elements QA hardwaretestrobot@gmail.com Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Comment-Date: Thu, 03 Sep 2020 20:38:41 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment

1542

days inactive

1545

days old

coreboot-gerrit@coreboot.org

12 comments

4 participants

tags (0)

participants (4)

9elements QA (Code Review)
Aaron Durbin (Code Review)
Alex Levin (Code Review)
Tim Wawrzynczak (Code Review)