Change in coreboot[master]: mb/google/endeavour: chrontel: fix interrupt and compat string

List overview All Threads
Download

newer

older

Change in coreboot[master]:...

Change in coreboot[master]: [WIP]...

Jeff Chase (Code Review)

8 May 2020 8 May '20

9:54 p.m.

Jeff Chase has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/41185 )

Change subject: mb/google/endeavour: chrontel: fix interrupt and compat string ......................................................................

mb/google/endeavour: chrontel: fix interrupt and compat string

BUG=b:146576073 TEST=cec-ctl with ch7322 driver

Change-Id: I737d951db135c53deb0f3cb956f0d0f275082251 Signed-off-by: Jeff Chase jnchase@google.com --- M src/mainboard/google/fizz/variants/endeavour/gpio.c M src/mainboard/google/fizz/variants/endeavour/overridetree.cb 2 files changed, 4 insertions(+), 4 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/85/41185/1

diff --git a/src/mainboard/google/fizz/variants/endeavour/gpio.c b/src/mainboard/google/fizz/variants/endeavour/gpio.c index c28f95c..f337777 100644 --- a/src/mainboard/google/fizz/variants/endeavour/gpio.c +++ b/src/mainboard/google/fizz/variants/endeavour/gpio.c @@ -28,9 +28,9 @@ /* SUSACK# */ PAD_CFG_NC(GPP_A15), /* TP150 */ /* SD_1P8_SEL */ PAD_CFG_NF(GPP_A16, NONE, DEEP, NF1), /* SD_PWR_EN# */ PAD_CFG_NF(GPP_A17, NONE, DEEP, NF1), -/* ISH_GP0 */ PAD_CFG_GPI_APIC(GPP_A18, NONE, DEEP), /* 7322_INTO */ +/* ISH_GP0 */ PAD_CFG_GPI_INT(GPP_A18, NONE, PLTRST, EDGE), /* 7322_INTO */ /* ISH_GP1 */ PAD_CFG_GPO_GPIO_DRIVER(GPP_A19, 1, DEEP, NONE), /* 7322_OE */ -/* ISH_GP2 */ PAD_CFG_GPI_APIC(GPP_A20, NONE, DEEP), /* 7322_INTO */ +/* ISH_GP2 */ PAD_CFG_GPI_INT(GPP_A20, NONE, PLTRST, EDGE), /* 7322_INTO */ /* ISH_GP3 */ PAD_CFG_GPO_GPIO_DRIVER(GPP_A21, 1, DEEP, NONE), /* 7322_OE */ /* ISH_GP4 */ PAD_CFG_NC(GPP_A22), /* ISH_GP5 */ PAD_CFG_NC(GPP_A23), diff --git a/src/mainboard/google/fizz/variants/endeavour/overridetree.cb b/src/mainboard/google/fizz/variants/endeavour/overridetree.cb index 861e1b1..1d83793 100644 --- a/src/mainboard/google/fizz/variants/endeavour/overridetree.cb +++ b/src/mainboard/google/fizz/variants/endeavour/overridetree.cb @@ -137,7 +137,7 @@ register "hid" = "ACPI_DT_NAMESPACE_HID" register "desc" = ""Chrontel 7322"" register "uid" = "1" - register "compat_string" = ""chrontel,7322"" + register "compat_string" = ""chrontel,ch7322"" register "irq_gpio" = "ACPI_GPIO_IRQ_EDGE_HIGH(GPP_A18)" device i2c 75 on end end @@ -145,7 +145,7 @@ register "hid" = "ACPI_DT_NAMESPACE_HID" register "desc" = ""Chrontel 7322"" register "uid" = "2" - register "compat_string" = ""chrontel,7322"" + register "compat_string" = ""chrontel,ch7322"" register "irq_gpio" = "ACPI_GPIO_IRQ_EDGE_HIGH(GPP_A20)" device i2c 76 on end end

Attachments:

attachment.htm (text/html — 4.9 KB)

Show replies by date

Furquan Shaikh (Code Review)

10 May 10 May

11:20 p.m.

Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41185 )

Change subject: mb/google/endeavour: chrontel: fix interrupt and compat string ......................................................................

Patch Set 1:

(2 comments)

https://review.coreboot.org/c/coreboot/+/41185/1/src/mainboard/google/fizz/v... File src/mainboard/google/fizz/variants/endeavour/gpio.c:

https://review.coreboot.org/c/coreboot/+/41185/1/src/mainboard/google/fizz/v... PS1, Line 31: PAD_CFG_GPI_APIC What was the issue with this configuration?

https://review.coreboot.org/c/coreboot/+/41185/1/src/mainboard/google/fizz/v... PS1, Line 31: EDGE We typically apply LEVEL triggering at the pad to pass the input signal as is to the interrupt controller and the ACPI/device tree entry is filled with the actual trigger for the interrupt.

-- To view, visit https://review.coreboot.org/c/coreboot/+/41185 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I737d951db135c53deb0f3cb956f0d0f275082251 Gerrit-Change-Number: 41185 Gerrit-PatchSet: 1 Gerrit-Owner: Jeff Chase jnchase@google.com Gerrit-Reviewer: Furquan Shaikh furquan@google.com Gerrit-Reviewer: Shelley Chen shchen@google.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-Comment-Date: Mon, 11 May 2020 06:20:12 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

Paul Menzel (Code Review)

11 May 11 May

3:49 a.m.

Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41185 )

Change subject: mb/google/endeavour: chrontel: fix interrupt and compat string ......................................................................

Patch Set 1:

(2 comments)

https://review.coreboot.org/c/coreboot/+/41185/1//COMMIT_MSG Commit Message:

https://review.coreboot.org/c/coreboot/+/41185/1//COMMIT_MSG@8 PS1, Line 8: Please summarize the problem, and explain the fix.

https://review.coreboot.org/c/coreboot/+/41185/1//COMMIT_MSG@10 PS1, Line 10: TEST=cec-ctl with ch7322 driver Please elaborate.

-- To view, visit https://review.coreboot.org/c/coreboot/+/41185 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I737d951db135c53deb0f3cb956f0d0f275082251 Gerrit-Change-Number: 41185 Gerrit-PatchSet: 1 Gerrit-Owner: Jeff Chase jnchase@google.com Gerrit-Reviewer: Furquan Shaikh furquan@google.com Gerrit-Reviewer: Shelley Chen shchen@google.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Comment-Date: Mon, 11 May 2020 10:49:03 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

Jeff Chase (Code Review)

3:11 p.m.

Hello Shelley Chen, build bot (Jenkins), Furquan Shaikh,

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

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

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

Change subject: mb/google/endeavour: chrontel: fix interrupt and compat string ......................................................................

mb/google/endeavour: chrontel: fix interrupt and compat string

The devicetree declares the chrontel interrupt as GpioInt so the GPIO needs to be configured as such instead of routing directly to APIC.

Also update the compatible string to conform to kernel standards.

BUG=b:146576073 TEST=install ch7322 driver; send commands using cec-ctl and verify that the interrupt handler is called.

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/85/41185/2

-- To view, visit https://review.coreboot.org/c/coreboot/+/41185 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I737d951db135c53deb0f3cb956f0d0f275082251 Gerrit-Change-Number: 41185 Gerrit-PatchSet: 2 Gerrit-Owner: Jeff Chase jnchase@google.com Gerrit-Reviewer: Furquan Shaikh furquan@google.com Gerrit-Reviewer: Shelley Chen shchen@google.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-MessageType: newpatchset

Jeff Chase (Code Review)

3:15 p.m.

Jeff Chase has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41185 )

Change subject: mb/google/endeavour: chrontel: fix interrupt and compat string ......................................................................

Patch Set 2:

(4 comments)

https://review.coreboot.org/c/coreboot/+/41185/1//COMMIT_MSG Commit Message:

https://review.coreboot.org/c/coreboot/+/41185/1//COMMIT_MSG@8 PS1, Line 8:

...

Please summarize the problem, and explain the fix.

Done

https://review.coreboot.org/c/coreboot/+/41185/1//COMMIT_MSG@10 PS1, Line 10: TEST=cec-ctl with ch7322 driver

...

Please elaborate.

Done

https://review.coreboot.org/c/coreboot/+/41185/1/src/mainboard/google/fizz/v... File src/mainboard/google/fizz/variants/endeavour/gpio.c:

https://review.coreboot.org/c/coreboot/+/41185/1/src/mainboard/google/fizz/v... PS1, Line 31: PAD_CFG_GPI_APIC

...

What was the issue with this configuration?

The interrupt for this device was declared as GpioInt in the ACPI table but it was configured here to route to APIC directly instead of enabling the interrupt through the GPIO controller (as I understand it). The interrupt handler in the driver was not firing.

https://review.coreboot.org/c/coreboot/+/41185/1/src/mainboard/google/fizz/v... PS1, Line 31: EDGE

...

We typically apply LEVEL triggering at the pad to pass the input signal as is to the interrupt contr […]

Done

-- To view, visit https://review.coreboot.org/c/coreboot/+/41185 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I737d951db135c53deb0f3cb956f0d0f275082251 Gerrit-Change-Number: 41185 Gerrit-PatchSet: 2 Gerrit-Owner: Jeff Chase jnchase@google.com Gerrit-Reviewer: Furquan Shaikh furquan@google.com Gerrit-Reviewer: Shelley Chen shchen@google.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Comment-Date: Mon, 11 May 2020 22:15:18 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Furquan Shaikh furquan@google.com Comment-In-Reply-To: Paul Menzel paulepanter@users.sourceforge.net Gerrit-MessageType: comment

Jeff Chase (Code Review)

20 May 20 May

8:19 a.m.

Jeff Chase has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41185 )

Change subject: mb/google/endeavour: chrontel: fix interrupt and compat string ......................................................................

Patch Set 2:

This change is ready for review.

-- To view, visit https://review.coreboot.org/c/coreboot/+/41185 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I737d951db135c53deb0f3cb956f0d0f275082251 Gerrit-Change-Number: 41185 Gerrit-PatchSet: 2 Gerrit-Owner: Jeff Chase jnchase@google.com Gerrit-Reviewer: Furquan Shaikh furquan@google.com Gerrit-Reviewer: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Reviewer: Shelley Chen shchen@google.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-Comment-Date: Wed, 20 May 2020 15:19:58 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment

Jeff Chase (Code Review)

8:21 a.m.

Jeff Chase has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41185 )

Change subject: mb/google/endeavour: chrontel: fix interrupt and compat string ......................................................................

Patch Set 2:

(1 comment)

https://review.coreboot.org/c/coreboot/+/41185/1/src/mainboard/google/fizz/v... File src/mainboard/google/fizz/variants/endeavour/gpio.c:

https://review.coreboot.org/c/coreboot/+/41185/1/src/mainboard/google/fizz/v... PS1, Line 31: PAD_CFG_GPI_APIC

...

The interrupt for this device was declared as GpioInt in the ACPI table but it was configured here t […]

Done

-- To view, visit https://review.coreboot.org/c/coreboot/+/41185 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I737d951db135c53deb0f3cb956f0d0f275082251 Gerrit-Change-Number: 41185 Gerrit-PatchSet: 2 Gerrit-Owner: Jeff Chase jnchase@google.com Gerrit-Reviewer: Furquan Shaikh furquan@google.com Gerrit-Reviewer: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Reviewer: Shelley Chen shchen@google.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-Comment-Date: Wed, 20 May 2020 15:21:59 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Furquan Shaikh furquan@google.com Comment-In-Reply-To: Jeff Chase jnchase@google.com Gerrit-MessageType: comment

Jeff Chase (Code Review)

8:22 a.m.

Jeff Chase has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41185 )

Change subject: mb/google/endeavour: chrontel: fix interrupt and compat string ......................................................................

Patch Set 2:

Could someone help me submit this change? Thank you.

-- To view, visit https://review.coreboot.org/c/coreboot/+/41185 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I737d951db135c53deb0f3cb956f0d0f275082251 Gerrit-Change-Number: 41185 Gerrit-PatchSet: 2 Gerrit-Owner: Jeff Chase jnchase@google.com Gerrit-Reviewer: Furquan Shaikh furquan@google.com Gerrit-Reviewer: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Reviewer: Shelley Chen shchen@google.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-Comment-Date: Wed, 20 May 2020 15:22:20 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment

Shelley Chen (Code Review)

10:19 a.m.

Shelley Chen has submitted this change. ( https://review.coreboot.org/c/coreboot/+/41185 )

Change subject: mb/google/endeavour: chrontel: fix interrupt and compat string ......................................................................

mb/google/endeavour: chrontel: fix interrupt and compat string

The devicetree declares the chrontel interrupt as GpioInt so the GPIO needs to be configured as such instead of routing directly to APIC.

Also update the compatible string to conform to kernel standards.

BUG=b:146576073 TEST=install ch7322 driver; send commands using cec-ctl and verify that the interrupt handler is called.

Change-Id: I737d951db135c53deb0f3cb956f0d0f275082251 Signed-off-by: Jeff Chase jnchase@google.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/41185 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Furquan Shaikh furquan@google.com Reviewed-by: Paul Menzel paulepanter@users.sourceforge.net --- M src/mainboard/google/fizz/variants/endeavour/gpio.c M src/mainboard/google/fizz/variants/endeavour/overridetree.cb 2 files changed, 4 insertions(+), 4 deletions(-)

Approvals: build bot (Jenkins): Verified Paul Menzel: Looks good to me, but someone else must approve Furquan Shaikh: Looks good to me, approved

diff --git a/src/mainboard/google/fizz/variants/endeavour/gpio.c b/src/mainboard/google/fizz/variants/endeavour/gpio.c index 0915c3e..432a180 100644 --- a/src/mainboard/google/fizz/variants/endeavour/gpio.c +++ b/src/mainboard/google/fizz/variants/endeavour/gpio.c @@ -27,9 +27,9 @@ /* SUSACK# */ PAD_CFG_NC(GPP_A15), /* TP150 */ /* SD_1P8_SEL */ PAD_CFG_NF(GPP_A16, NONE, DEEP, NF1), /* SD_PWR_EN# */ PAD_CFG_NF(GPP_A17, NONE, DEEP, NF1), -/* ISH_GP0 */ PAD_CFG_GPI_APIC(GPP_A18, NONE, DEEP), /* 7322_INTO */ +/* ISH_GP0 */ PAD_CFG_GPI_INT(GPP_A18, NONE, PLTRST, LEVEL), /* 7322_INTO */ /* ISH_GP1 */ PAD_CFG_GPO_GPIO_DRIVER(GPP_A19, 1, DEEP, NONE), /* 7322_OE */ -/* ISH_GP2 */ PAD_CFG_GPI_APIC(GPP_A20, NONE, DEEP), /* 7322_INTO */ +/* ISH_GP2 */ PAD_CFG_GPI_INT(GPP_A20, NONE, PLTRST, LEVEL), /* 7322_INTO */ /* ISH_GP3 */ PAD_CFG_GPO_GPIO_DRIVER(GPP_A21, 1, DEEP, NONE), /* 7322_OE */ /* ISH_GP4 */ PAD_CFG_NC(GPP_A22), /* ISH_GP5 */ PAD_CFG_NC(GPP_A23), diff --git a/src/mainboard/google/fizz/variants/endeavour/overridetree.cb b/src/mainboard/google/fizz/variants/endeavour/overridetree.cb index 861e1b1..1d83793 100644 --- a/src/mainboard/google/fizz/variants/endeavour/overridetree.cb +++ b/src/mainboard/google/fizz/variants/endeavour/overridetree.cb @@ -137,7 +137,7 @@ register "hid" = "ACPI_DT_NAMESPACE_HID" register "desc" = ""Chrontel 7322"" register "uid" = "1" - register "compat_string" = ""chrontel,7322"" + register "compat_string" = ""chrontel,ch7322"" register "irq_gpio" = "ACPI_GPIO_IRQ_EDGE_HIGH(GPP_A18)" device i2c 75 on end end @@ -145,7 +145,7 @@ register "hid" = "ACPI_DT_NAMESPACE_HID" register "desc" = ""Chrontel 7322"" register "uid" = "2" - register "compat_string" = ""chrontel,7322"" + register "compat_string" = ""chrontel,ch7322"" register "irq_gpio" = "ACPI_GPIO_IRQ_EDGE_HIGH(GPP_A20)" device i2c 76 on end end

-- To view, visit https://review.coreboot.org/c/coreboot/+/41185 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I737d951db135c53deb0f3cb956f0d0f275082251 Gerrit-Change-Number: 41185 Gerrit-PatchSet: 3 Gerrit-Owner: Jeff Chase jnchase@google.com Gerrit-Reviewer: Furquan Shaikh furquan@google.com Gerrit-Reviewer: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Reviewer: Shelley Chen shchen@google.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-MessageType: merged

9elements QA (Code Review)

10:36 a.m.

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

Change subject: mb/google/endeavour: chrontel: fix interrupt and compat string ......................................................................

Patch Set 3:

Automatic boot test returned (PASS/FAIL/TOTAL): 4/0/4 Emulation targets: "QEMU x86 q35/ich9" using payload TianoCore : SUCCESS : https://lava.9esec.io/r/3703 "QEMU x86 q35/ich9" using payload SeaBIOS : SUCCESS : https://lava.9esec.io/r/3702 "QEMU x86 i440fx/piix4" using payload SeaBIOS : SUCCESS : https://lava.9esec.io/r/3701 "QEMU AArch64" using payload LinuxBoot_u-root_kexec : SUCCESS : https://lava.9esec.io/r/3700

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

-- To view, visit https://review.coreboot.org/c/coreboot/+/41185 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I737d951db135c53deb0f3cb956f0d0f275082251 Gerrit-Change-Number: 41185 Gerrit-PatchSet: 3 Gerrit-Owner: Jeff Chase jnchase@google.com Gerrit-Reviewer: Furquan Shaikh furquan@google.com Gerrit-Reviewer: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Reviewer: Shelley Chen shchen@google.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: 9elements QA hardwaretestrobot@gmail.com Gerrit-Comment-Date: Wed, 20 May 2020 17:36:58 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment

1642

days inactive

1653

days old

coreboot-gerrit@coreboot.org

9 comments

5 participants

tags (0)

participants (5)

9elements QA (Code Review)
Furquan Shaikh (Code Review)
Jeff Chase (Code Review)
Paul Menzel (Code Review)
Shelley Chen (Code Review)