Frank Wu has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/45565 )
Change subject: mb/google/zork/vilboz: Enable P sensor ......................................................................
mb/google/zork/vilboz: Enable P sensor
BUG=b:161759253 BRANCH=firmware-zork-13434.B TEST=emerge-zork coreboot chromeos-bootimage
Signed-off-by: Frank Wu frank_wu@compal.corp-partner.google.com Change-Id: I6294ce291365443dd1c4550ba75cb7f33481b889 --- M src/mainboard/google/zork/variants/vilboz/gpio.c M src/mainboard/google/zork/variants/vilboz/overridetree.cb 2 files changed, 9 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/65/45565/1
diff --git a/src/mainboard/google/zork/variants/vilboz/gpio.c b/src/mainboard/google/zork/variants/vilboz/gpio.c index 12b303a..d4bcfa5 100644 --- a/src/mainboard/google/zork/variants/vilboz/gpio.c +++ b/src/mainboard/google/zork/variants/vilboz/gpio.c @@ -10,6 +10,8 @@ static const struct soc_amd_gpio bid_1_gpio_set_stage_ram[] = { /* TP */ PAD_NC(GPIO_32), + /* P sensor INT */ + PAD_GPI(GPIO_40, PULL_NONE), /* EN_DEV_BEEP_L */ PAD_GPO(GPIO_89, HIGH), /* USI_RESET */ diff --git a/src/mainboard/google/zork/variants/vilboz/overridetree.cb b/src/mainboard/google/zork/variants/vilboz/overridetree.cb index d415de5..932805f 100644 --- a/src/mainboard/google/zork/variants/vilboz/overridetree.cb +++ b/src/mainboard/google/zork/variants/vilboz/overridetree.cb @@ -136,5 +136,12 @@ register "hid_desc_reg_offset" = "0x20" device i2c 2c on end end + chip drivers/i2c/generic + register "hid" = ""STH9324"" + register "name" = ""SEMTECH SX9324"" + register "desc" = ""SAR Proximity Sensor"" + register "irq_gpio" = "ACPI_GPIO_IRQ_EDGE_LOW(GPIO_40)" + device i2c 28 on end + end end end # chip soc/amd/picasso
Frank Wu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45565 )
Change subject: mb/google/zork/vilboz: Enable P sensor ......................................................................
Patch Set 1:
Hi Google,
Would you help review the CL or give us some suggestion? Thank you very much.
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45565 )
Change subject: mb/google/zork/vilboz: Enable P sensor ......................................................................
Patch Set 1:
(2 comments)
https://review.coreboot.org/c/coreboot/+/45565/1/src/mainboard/google/zork/v... File src/mainboard/google/zork/variants/vilboz/gpio.c:
https://review.coreboot.org/c/coreboot/+/45565/1/src/mainboard/google/zork/v... PS1, Line 14: PAD_GPI Shouldn't this be PAD_INT?
https://review.coreboot.org/c/coreboot/+/45565/1/src/mainboard/google/zork/v... File src/mainboard/google/zork/variants/vilboz/overridetree.cb:
https://review.coreboot.org/c/coreboot/+/45565/1/src/mainboard/google/zork/v... PS1, Line 140: STH9324 What kernel driver is used for this? I don't see this HID in kernel v5.4
Hello build bot (Jenkins), Furquan Shaikh, Martin Roth, Tim Wawrzynczak, Aaron Durbin, EricR Lai,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/45565
to look at the new patch set (#2).
Change subject: mb/google/zork/vilboz: Enable SAR proximity sensor STH9324 ......................................................................
mb/google/zork/vilboz: Enable SAR proximity sensor STH9324
BUG=b:161759253 BRANCH=firmware-zork-13434.B TEST=emerge-zork coreboot chromeos-bootimage
Signed-off-by: Frank Wu frank_wu@compal.corp-partner.google.com Change-Id: I6294ce291365443dd1c4550ba75cb7f33481b889 --- M src/mainboard/google/zork/variants/vilboz/gpio.c M src/mainboard/google/zork/variants/vilboz/overridetree.cb 2 files changed, 9 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/65/45565/2
Frank Wu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45565 )
Change subject: mb/google/zork/vilboz: Enable SAR proximity sensor STH9324 ......................................................................
Patch Set 2:
(2 comments)
https://review.coreboot.org/c/coreboot/+/45565/1/src/mainboard/google/zork/v... File src/mainboard/google/zork/variants/vilboz/gpio.c:
https://review.coreboot.org/c/coreboot/+/45565/1/src/mainboard/google/zork/v... PS1, Line 14: PAD_GPI
Shouldn't this be PAD_INT?
Done
https://review.coreboot.org/c/coreboot/+/45565/1/src/mainboard/google/zork/v... File src/mainboard/google/zork/variants/vilboz/overridetree.cb:
https://review.coreboot.org/c/coreboot/+/45565/1/src/mainboard/google/zork/v... PS1, Line 140: STH9324
What kernel driver is used for this? I don't see this HID in kernel v5. […]
According to https://partnerissuetracker.corp.google.com/issues/161759253#comment8 , there is no device driver for STH9324 in kernel v5.4 now. The kernel driver should be cherry-picked/upstreamed to kernel v5.4 later.
Frank Wu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45565 )
Change subject: mb/google/zork/vilboz: Enable SAR proximity sensor STH9324 ......................................................................
Patch Set 2:
Hi Google,
Would you help review the CL or give us some suggestion? Thank you.
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45565 )
Change subject: mb/google/zork/vilboz: Enable SAR proximity sensor STH9324 ......................................................................
Patch Set 2:
(2 comments)
https://review.coreboot.org/c/coreboot/+/45565/2/src/mainboard/google/zork/v... File src/mainboard/google/zork/variants/vilboz/gpio.c:
https://review.coreboot.org/c/coreboot/+/45565/2/src/mainboard/google/zork/v... PS2, Line 10: bid_1_gpio_set_stage_ram Is P-sensor interrupt used only on board with version 1 or all future board versions as well? If it is used on all future board versions too, then you will have to add another table that will configure this interrupt line for future board versions and will be returned on line 39.
https://review.coreboot.org/c/coreboot/+/45565/1/src/mainboard/google/zork/v... File src/mainboard/google/zork/variants/vilboz/overridetree.cb:
https://review.coreboot.org/c/coreboot/+/45565/1/src/mainboard/google/zork/v... PS1, Line 140: STH9324
According to https://partnerissuetracker.corp.google.com/issues/161759253#comment8 , […]
I think it would be good to get the driver in before landing this change. So that the ACPI information is correctly supplied here as per the driver expectations.
Frank Wu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45565 )
Change subject: mb/google/zork/vilboz: Enable SAR proximity sensor STH9324 ......................................................................
Patch Set 2:
(2 comments)
https://review.coreboot.org/c/coreboot/+/45565/2/src/mainboard/google/zork/v... File src/mainboard/google/zork/variants/vilboz/gpio.c:
https://review.coreboot.org/c/coreboot/+/45565/2/src/mainboard/google/zork/v... PS2, Line 10: bid_1_gpio_set_stage_ram
Is P-sensor interrupt used only on board with version 1 or all future board versions as well? If it […]
I will discuss that with our hardware team and update result later.
https://review.coreboot.org/c/coreboot/+/45565/1/src/mainboard/google/zork/v... File src/mainboard/google/zork/variants/vilboz/overridetree.cb:
https://review.coreboot.org/c/coreboot/+/45565/1/src/mainboard/google/zork/v... PS1, Line 140: STH9324
I think it would be good to get the driver in before landing this change. […]
I appreciate the comment.
Here is the CL for kernel driver. https://chromium-review.googlesource.com/c/chromiumos/third_party/kernel/+/2... We are going to verify the p snesor function is workable or not. I will keep you posted. Thank you.
EricR Lai has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45565 )
Change subject: mb/google/zork/vilboz: Enable SAR proximity sensor STH9324 ......................................................................
Patch Set 2: Code-Review+2
Frank Wu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45565 )
Change subject: mb/google/zork/vilboz: Enable SAR proximity sensor STH9324 ......................................................................
Patch Set 2:
Here is the result for reference. https://partnerissuetracker.corp.google.com/issues/161759253#comment22
Would you help review? Thank you.
EricR Lai has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45565 )
Change subject: mb/google/zork/vilboz: Enable SAR proximity sensor STH9324 ......................................................................
Patch Set 2: Code-Review+1
Oh, still need to confirm with HW for the gpio.
Hello build bot (Jenkins), Furquan Shaikh, Martin Roth, Tim Wawrzynczak, Aaron Durbin, EricR Lai,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/45565
to look at the new patch set (#3).
Change subject: mb/google/zork/vilboz: Enable SAR proximity sensor STH9324 ......................................................................
mb/google/zork/vilboz: Enable SAR proximity sensor STH9324
BUG=b:161759253 BRANCH=firmware-zork-13434.B TEST=emerge-zork coreboot chromeos-bootimage
Signed-off-by: Frank Wu frank_wu@compal.corp-partner.google.com Change-Id: I6294ce291365443dd1c4550ba75cb7f33481b889 --- M src/mainboard/google/zork/variants/vilboz/gpio.c M src/mainboard/google/zork/variants/vilboz/overridetree.cb 2 files changed, 14 insertions(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/65/45565/3
Frank Wu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45565 )
Change subject: mb/google/zork/vilboz: Enable SAR proximity sensor STH9324 ......................................................................
Patch Set 3:
(1 comment)
Would you help review the CL based on result https://partnerissuetracker.corp.google.com/issues/161759253#comment23 ?
Thank you.
https://review.coreboot.org/c/coreboot/+/45565/2/src/mainboard/google/zork/v... File src/mainboard/google/zork/variants/vilboz/gpio.c:
https://review.coreboot.org/c/coreboot/+/45565/2/src/mainboard/google/zork/v... PS2, Line 10: bid_1_gpio_set_stage_ram
I will discuss that with our hardware team and update result later.
The schematic about p sensor will not be modified for board version 2+. Therefore, the gpio is configured in vilboz_gpio_set_stage_ram. I will create another issue to track and update the rest gpios between board version 1 and 2.
Frank Wu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45565 )
Change subject: mb/google/zork/vilboz: Enable SAR proximity sensor STH9324 ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/c/coreboot/+/45565/1/src/mainboard/google/zork/v... File src/mainboard/google/zork/variants/vilboz/overridetree.cb:
https://review.coreboot.org/c/coreboot/+/45565/1/src/mainboard/google/zork/v... PS1, Line 140: STH9324
I appreciate the comment. […]
The result: https://partnerissuetracker.corp.google.com/issues/161759253#comment23 and https://partnerissuetracker.corp.google.com/issues/161759253#comment22
Frank Wu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45565 )
Change subject: mb/google/zork/vilboz: Enable SAR proximity sensor STH9324 ......................................................................
Patch Set 3:
Hi Google,
Would you help review the CL or give us some suggestion? Thank you.
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45565 )
Change subject: mb/google/zork/vilboz: Enable SAR proximity sensor STH9324 ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/c/coreboot/+/45565/3//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/45565/3//COMMIT_MSG@11 PS3, Line 11: TEST=emerge-zork coreboot chromeos-bootimage Any logs in coreboot or Linux, that show that it’s enabled? Please paste those.
Hello build bot (Jenkins), Furquan Shaikh, Martin Roth, Tim Wawrzynczak, Aaron Durbin, EricR Lai,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/45565
to look at the new patch set (#4).
Change subject: mb/google/zork/vilboz: Enable SAR proximity sensor STH9324 ......................................................................
mb/google/zork/vilboz: Enable SAR proximity sensor STH9324
BUG=b:161759253 BRANCH=firmware-zork-13434.B TEST=emerge-zork coreboot chromeos-bootimage firmware log: _SB.I2C2.SEMTECH SX9324: SAR Proximity Sensor at I2C: 02:28 kernel log: INFO kernel: [ 11.238644] sx932x i2c-STH9324:00: initial compensation success
Signed-off-by: Frank Wu frank_wu@compal.corp-partner.google.com Change-Id: I6294ce291365443dd1c4550ba75cb7f33481b889 --- M src/mainboard/google/zork/variants/vilboz/gpio.c M src/mainboard/google/zork/variants/vilboz/overridetree.cb 2 files changed, 14 insertions(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/65/45565/4
Frank Wu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45565 )
Change subject: mb/google/zork/vilboz: Enable SAR proximity sensor STH9324 ......................................................................
Patch Set 4:
(1 comment)
https://review.coreboot.org/c/coreboot/+/45565/3//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/45565/3//COMMIT_MSG@11 PS3, Line 11: TEST=emerge-zork coreboot chromeos-bootimage
Any logs in coreboot or Linux, that show that it’s enabled? Please paste those.
The logs from firmware and kernel are updated to commit message for reference.
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45565 )
Change subject: mb/google/zork/vilboz: Enable SAR proximity sensor STH9324 ......................................................................
Patch Set 4: Code-Review+1
Frank Wu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45565 )
Change subject: mb/google/zork/vilboz: Enable SAR proximity sensor STH9324 ......................................................................
Patch Set 4:
Hi Google,
Would you help to review the CL? Then we can have CPFE FW image to verify it. Thank you.
Kangheui Won has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45565 )
Change subject: mb/google/zork/vilboz: Enable SAR proximity sensor STH9324 ......................................................................
Patch Set 4: Code-Review+1
Sam McNally has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45565 )
Change subject: mb/google/zork/vilboz: Enable SAR proximity sensor STH9324 ......................................................................
Patch Set 4: Code-Review+2
EricR Lai has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45565 )
Change subject: mb/google/zork/vilboz: Enable SAR proximity sensor STH9324 ......................................................................
Patch Set 4: Code-Review+2
Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/45565 )
Change subject: mb/google/zork/vilboz: Enable SAR proximity sensor STH9324 ......................................................................
mb/google/zork/vilboz: Enable SAR proximity sensor STH9324
BUG=b:161759253 BRANCH=firmware-zork-13434.B TEST=emerge-zork coreboot chromeos-bootimage firmware log: _SB.I2C2.SEMTECH SX9324: SAR Proximity Sensor at I2C: 02:28 kernel log: INFO kernel: [ 11.238644] sx932x i2c-STH9324:00: initial compensation success
Signed-off-by: Frank Wu frank_wu@compal.corp-partner.google.com Change-Id: I6294ce291365443dd1c4550ba75cb7f33481b889 Reviewed-on: https://review.coreboot.org/c/coreboot/+/45565 Reviewed-by: Paul Menzel paulepanter@users.sourceforge.net Reviewed-by: Kangheui Won khwon@chromium.org Reviewed-by: Sam McNally sammc@google.com Reviewed-by: EricR Lai ericr_lai@compal.corp-partner.google.com Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M src/mainboard/google/zork/variants/vilboz/gpio.c M src/mainboard/google/zork/variants/vilboz/overridetree.cb 2 files changed, 14 insertions(+), 2 deletions(-)
Approvals: build bot (Jenkins): Verified Paul Menzel: Looks good to me, but someone else must approve EricR Lai: Looks good to me, approved Kangheui Won: Looks good to me, but someone else must approve Sam McNally: Looks good to me, approved
diff --git a/src/mainboard/google/zork/variants/vilboz/gpio.c b/src/mainboard/google/zork/variants/vilboz/gpio.c index 91ed61a..6fba0e2 100644 --- a/src/mainboard/google/zork/variants/vilboz/gpio.c +++ b/src/mainboard/google/zork/variants/vilboz/gpio.c @@ -16,6 +16,11 @@ PAD_GPO(GPIO_140, HIGH), };
+static const struct soc_amd_gpio vilboz_gpio_set_stage_ram[] = { + /* P sensor INT */ + PAD_INT(GPIO_40, PULL_NONE, LEVEL_LOW, STATUS_DELIVERY), +}; + const struct soc_amd_gpio *variant_override_gpio_table(size_t *size) { uint32_t board_version; @@ -33,6 +38,6 @@ return bid_1_gpio_set_stage_ram; }
- *size = 0; - return NULL; + *size = ARRAY_SIZE(vilboz_gpio_set_stage_ram); + return vilboz_gpio_set_stage_ram; } diff --git a/src/mainboard/google/zork/variants/vilboz/overridetree.cb b/src/mainboard/google/zork/variants/vilboz/overridetree.cb index a93beb5..a61c027 100644 --- a/src/mainboard/google/zork/variants/vilboz/overridetree.cb +++ b/src/mainboard/google/zork/variants/vilboz/overridetree.cb @@ -142,5 +142,12 @@ register "hid_desc_reg_offset" = "0x20" device i2c 2c on end end + chip drivers/i2c/generic + register "hid" = ""STH9324"" + register "name" = ""SEMTECH SX9324"" + register "desc" = ""SAR Proximity Sensor"" + register "irq_gpio" = "ACPI_GPIO_IRQ_LEVEL_LOW(GPIO_40)" + device i2c 28 on end + end end end # chip soc/amd/picasso