Change in coreboot[master]: mb/google/kohaku: Set reset hold time for I2C digitizer device

List overview All Threads
Download

newer

older

Change in coreboot[master]:...

Seunghwan Kim (Code Review)

23 Dec 2019 23 Dec '19

10:53 a.m.

Seunghwan Kim has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/37911 )

Change subject: mb/google/kohaku: Set reset hold time for I2C digitizer device ......................................................................

mb/google/kohaku: Set reset hold time for I2C digitizer device

According to device's spec, we should hold reset assertion for 100ms in its power on sequence. This change passes the hold time amount to driver for it.

BUG=b:129159369 BRANCH=firmware-hatch-12672.B TEST=Checked the reset signal timing in digitizer device's power on sequence

Signed-off-by: Seunghwan Kim sh_.kim@samsung.com Change-Id: Iccb33c13c9a390a2c971325c74c0c4ad4b08618e --- M src/mainboard/google/hatch/variants/kohaku/overridetree.cb 1 file changed, 4 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/11/37911/1

diff --git a/src/mainboard/google/hatch/variants/kohaku/overridetree.cb b/src/mainboard/google/hatch/variants/kohaku/overridetree.cb index cd5ce0e..70d664c 100644 --- a/src/mainboard/google/hatch/variants/kohaku/overridetree.cb +++ b/src/mainboard/google/hatch/variants/kohaku/overridetree.cb @@ -208,6 +208,10 @@ register "generic.reset_gpio" = "ACPI_GPIO_OUTPUT_ACTIVE_LOW(GPP_A19)" register "generic.reset_delay_ms" = "1" register "generic.has_power_resource" = "1" + register "generic.property_count" = "1" + register "generic.property_list[0].type" = "ACPI_DP_TYPE_INTEGER" + register "generic.property_list[0].name" = ""post-power-on-delay-ms"" + register "generic.property_list[0].integer" = "100" register "hid_desc_reg_offset" = "0x1" device i2c 0x09 on end end

Attachments:

attachment.htm (text/html — 3.3 KB)

Show replies by date

Furquan Shaikh (Code Review)

23 Dec 23 Dec

11:10 a.m.

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

Change subject: mb/google/kohaku: Set reset hold time for I2C digitizer device ......................................................................

Patch Set 1:

(1 comment)

https://review.coreboot.org/c/coreboot/+/37911/1/src/mainboard/google/hatch/... File src/mainboard/google/hatch/variants/kohaku/overridetree.cb:

https://review.coreboot.org/c/coreboot/+/37911/1/src/mainboard/google/hatch/... PS1, Line 213: post-power-on-delay-ms Just curious: Is this better/different than setting "reset_delay_ms" to 100 above in line 209 above?

-- To view, visit https://review.coreboot.org/c/coreboot/+/37911 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Iccb33c13c9a390a2c971325c74c0c4ad4b08618e Gerrit-Change-Number: 37911 Gerrit-PatchSet: 1 Gerrit-Owner: Seunghwan Kim sh_.kim@samsung.com Gerrit-Reviewer: Seunghwan Kim sh_.kim@samsung.com Gerrit-Reviewer: Tim Wawrzynczak twawrzynczak@google.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Furquan Shaikh furquan@google.com Gerrit-Comment-Date: Mon, 23 Dec 2019 10:10:23 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

Tim Wawrzynczak (Code Review)

8:53 p.m.

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

Change subject: mb/google/kohaku: Set reset hold time for I2C digitizer device ......................................................................

Patch Set 1:

(1 comment)

https://review.coreboot.org/c/coreboot/+/37911/1/src/mainboard/google/hatch/... File src/mainboard/google/hatch/variants/kohaku/overridetree.cb:

https://review.coreboot.org/c/coreboot/+/37911/1/src/mainboard/google/hatch/... PS1, Line 213: post-power-on-delay-ms

...

Just curious: Is this better/different than setting "reset_delay_ms" to 100 above in line 209 above?

This looks like it will do the same thing as reset_delay, except not allow coordination with enable GPIOs, etc. Leaving it this way means that the ACPI _ON method will do the reset/enable GPIO sequence in common ACPI code, then when it gets to this driver, it will reset it again, without any regards for the enable GPIO sequencing.

-- To view, visit https://review.coreboot.org/c/coreboot/+/37911 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Iccb33c13c9a390a2c971325c74c0c4ad4b08618e Gerrit-Change-Number: 37911 Gerrit-PatchSet: 1 Gerrit-Owner: Seunghwan Kim sh_.kim@samsung.com Gerrit-Reviewer: Seunghwan Kim sh_.kim@samsung.com Gerrit-Reviewer: Tim Wawrzynczak twawrzynczak@google.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Furquan Shaikh furquan@google.com Gerrit-CC: Tim Wawrzynczak twawrzynczak@chromium.org Gerrit-Comment-Date: Mon, 23 Dec 2019 19:53:45 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Furquan Shaikh furquan@google.com Gerrit-MessageType: comment

Furquan Shaikh (Code Review)

24 Dec 24 Dec

5:08 a.m.

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

Change subject: mb/google/kohaku: Set reset hold time for I2C digitizer device ......................................................................

Patch Set 1:

(1 comment)

https://review.coreboot.org/c/coreboot/+/37911/1/src/mainboard/google/hatch/... File src/mainboard/google/hatch/variants/kohaku/overridetree.cb:

https://review.coreboot.org/c/coreboot/+/37911/1/src/mainboard/google/hatch/... PS1, Line 213: post-power-on-delay-ms

...

This looks like it will do the same thing as reset_delay, except not allow coordination with enable […]

So, is the intent of this change to push the handling of reset delay to kernel driver and not rely on ACPI to do it?

-- To view, visit https://review.coreboot.org/c/coreboot/+/37911 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Iccb33c13c9a390a2c971325c74c0c4ad4b08618e Gerrit-Change-Number: 37911 Gerrit-PatchSet: 1 Gerrit-Owner: shkim sh_.kim@samsung.com Gerrit-Reviewer: Tim Wawrzynczak twawrzynczak@google.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-Reviewer: shkim sh_.kim@samsung.com Gerrit-CC: Furquan Shaikh furquan@google.com Gerrit-CC: Tim Wawrzynczak twawrzynczak@chromium.org Gerrit-Comment-Date: Tue, 24 Dec 2019 04:08:51 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Tim Wawrzynczak twawrzynczak@chromium.org Comment-In-Reply-To: Furquan Shaikh furquan@google.com Gerrit-MessageType: comment

shkim (Code Review)

5:47 a.m.

shkim has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37911 )

Change subject: mb/google/kohaku: Set reset hold time for I2C digitizer device ......................................................................

Patch Set 1:

(1 comment)

https://review.coreboot.org/c/coreboot/+/37911/1/src/mainboard/google/hatch/... File src/mainboard/google/hatch/variants/kohaku/overridetree.cb:

https://review.coreboot.org/c/coreboot/+/37911/1/src/mainboard/google/hatch/... PS1, Line 213: post-power-on-delay-ms

...

So, is the intent of this change to push the handling of reset delay to kernel driver and not rely o […]

Wacom guys advised to put an 100ms to hold reset_gpio as low for the issue b:129159369, and I expected this change to put the delay before reset_gpio to high in driver. But actually it's doing same thing with "reset_delay_ms" as Tim commented. BTW we saw either of this change and "reset_delay_ms = 100" improves the driver binding issue in b:129159369, so we need further check for this issue.

-- To view, visit https://review.coreboot.org/c/coreboot/+/37911 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Iccb33c13c9a390a2c971325c74c0c4ad4b08618e Gerrit-Change-Number: 37911 Gerrit-PatchSet: 1 Gerrit-Owner: shkim sh_.kim@samsung.com Gerrit-Reviewer: Tim Wawrzynczak twawrzynczak@google.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-Reviewer: shkim sh_.kim@samsung.com Gerrit-CC: Furquan Shaikh furquan@google.com Gerrit-CC: Tim Wawrzynczak twawrzynczak@chromium.org Gerrit-Comment-Date: Tue, 24 Dec 2019 04:47:56 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Tim Wawrzynczak twawrzynczak@chromium.org Comment-In-Reply-To: Furquan Shaikh furquan@google.com Gerrit-MessageType: comment

shkim (Code Review)

5:48 a.m.

shkim has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37911 )

Change subject: mb/google/kohaku: Set reset hold time for I2C digitizer device ......................................................................

Patch Set 1: Code-Review-1

-- To view, visit https://review.coreboot.org/c/coreboot/+/37911 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Iccb33c13c9a390a2c971325c74c0c4ad4b08618e Gerrit-Change-Number: 37911 Gerrit-PatchSet: 1 Gerrit-Owner: shkim sh_.kim@samsung.com Gerrit-Reviewer: Tim Wawrzynczak twawrzynczak@google.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-Reviewer: shkim sh_.kim@samsung.com Gerrit-CC: Furquan Shaikh furquan@google.com Gerrit-CC: Tim Wawrzynczak twawrzynczak@chromium.org Gerrit-Comment-Date: Tue, 24 Dec 2019 04:48:07 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

shkim (Code Review)

26 Dec 26 Dec

5:59 a.m.

Hello Tim Wawrzynczak, build bot (Jenkins),

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

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

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

Change subject: mb/google/kohaku: Set reset hold time for I2C digitizer device ......................................................................

mb/google/kohaku: Set reset hold time for I2C digitizer device

According to device's spec, we should hold reset assertion for 100ms in its power on sequence. This change passes the hold time amount to driver for it.

BUG=b:129159369 BRANCH=firmware-hatch-12672.B TEST=Checked the reset signal timing in digitizer device's power on sequence

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/11/37911/2

shkim (Code Review)

6:20 a.m.

New subject: Change in coreboot[master]: mb/google/kohaku: Update reset timing for digitizer device

Hello Tim Wawrzynczak, build bot (Jenkins),

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

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

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

Change subject: mb/google/kohaku: Update reset timing for digitizer device ......................................................................

mb/google/kohaku: Update reset timing for digitizer device

This change puts delays for digitizer device's power on sequence.

- According to device's spec, we should hold reset assertion for 100ms in its power on sequence. 100ms of "enable_delay_ms" is for it. - We found the driver binding failure issue could be cleared with 100ms of "reset_delay_ms". It seems the IC need some time before communication after de-assertion of reset.

BUG=b:129159369 BRANCH=firmware-hatch-12672.B TEST=Verified the reset timing and driver bound successfully.

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/11/37911/3

shkim (Code Review)

6:21 a.m.

New subject: Change in coreboot[master]: mb/google/kohaku: Update reset timing for digitizer device

shkim has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37911 )

Change subject: mb/google/kohaku: Update reset timing for digitizer device ......................................................................

Patch Set 3:

(1 comment)

...

Patch Set 1:

(1 comment)

https://review.coreboot.org/c/coreboot/+/37911/1/src/mainboard/google/hatch/... File src/mainboard/google/hatch/variants/kohaku/overridetree.cb:

https://review.coreboot.org/c/coreboot/+/37911/1/src/mainboard/google/hatch/... PS1, Line 213: post-power-on-delay-ms

...

Wacom guys advised to put an 100ms to hold reset_gpio as low for the issue b:129159369, and I expect […]

Please review the new patch set.

-- To view, visit https://review.coreboot.org/c/coreboot/+/37911 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Iccb33c13c9a390a2c971325c74c0c4ad4b08618e Gerrit-Change-Number: 37911 Gerrit-PatchSet: 3 Gerrit-Owner: shkim sh_.kim@samsung.com Gerrit-Reviewer: Tim Wawrzynczak twawrzynczak@google.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-Reviewer: shkim sh_.kim@samsung.com Gerrit-CC: Furquan Shaikh furquan@google.com Gerrit-CC: Tim Wawrzynczak twawrzynczak@chromium.org Gerrit-Comment-Date: Thu, 26 Dec 2019 05:21:45 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: shkim sh_.kim@samsung.com Comment-In-Reply-To: Tim Wawrzynczak twawrzynczak@chromium.org Comment-In-Reply-To: Furquan Shaikh furquan@google.com Gerrit-MessageType: comment

Furquan Shaikh (Code Review)

6:38 a.m.

New subject: Change in coreboot[master]: mb/google/kohaku: Update reset timing for digitizer device

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

Change subject: mb/google/kohaku: Update reset timing for digitizer device ......................................................................

Patch Set 3:

(1 comment)

https://review.coreboot.org/c/coreboot/+/37911/3//COMMIT_MSG Commit Message:

https://review.coreboot.org/c/coreboot/+/37911/3//COMMIT_MSG@14 PS3, Line 14: It seems the IC need some time before communication : after de-assertion of reset. Was this verified with the device vendor? Is this captured in the power sequencing requirement?

-- To view, visit https://review.coreboot.org/c/coreboot/+/37911 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Iccb33c13c9a390a2c971325c74c0c4ad4b08618e Gerrit-Change-Number: 37911 Gerrit-PatchSet: 3 Gerrit-Owner: shkim sh_.kim@samsung.com Gerrit-Reviewer: Tim Wawrzynczak twawrzynczak@google.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-Reviewer: shkim sh_.kim@samsung.com Gerrit-CC: Furquan Shaikh furquan@google.com Gerrit-CC: Tim Wawrzynczak twawrzynczak@chromium.org Gerrit-Comment-Date: Thu, 26 Dec 2019 05:38:49 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

shkim (Code Review)

6:45 a.m.

New subject: Change in coreboot[master]: mb/google/kohaku: Update reset timing for digitizer device

shkim has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37911 )

Change subject: mb/google/kohaku: Update reset timing for digitizer device ......................................................................

Patch Set 3:

(1 comment)

https://review.coreboot.org/c/coreboot/+/37911/3//COMMIT_MSG Commit Message:

https://review.coreboot.org/c/coreboot/+/37911/3//COMMIT_MSG@14 PS3, Line 14: It seems the IC need some time before communication : after de-assertion of reset.

...

Was this verified with the device vendor? Is this captured in the power sequencing requirement?

Just guessing from our experiment. Without reset_delay, driver get failed to bind always. We may need to get any information from device vendor with this result.

-- To view, visit https://review.coreboot.org/c/coreboot/+/37911 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Iccb33c13c9a390a2c971325c74c0c4ad4b08618e Gerrit-Change-Number: 37911 Gerrit-PatchSet: 3 Gerrit-Owner: shkim sh_.kim@samsung.com Gerrit-Reviewer: Tim Wawrzynczak twawrzynczak@google.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-Reviewer: shkim sh_.kim@samsung.com Gerrit-CC: Furquan Shaikh furquan@google.com Gerrit-CC: Tim Wawrzynczak twawrzynczak@chromium.org Gerrit-Comment-Date: Thu, 26 Dec 2019 05:45:45 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Furquan Shaikh furquan@google.com Gerrit-MessageType: comment

Tim Wawrzynczak (Code Review)

9:51 p.m.

New subject: Change in coreboot[master]: mb/google/kohaku: Update reset timing for digitizer device

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

Change subject: mb/google/kohaku: Update reset timing for digitizer device ......................................................................

Patch Set 3:

(1 comment)

Hm the datasheet for that part leaves a lot to the imagination....

https://review.coreboot.org/c/coreboot/+/37911/3//COMMIT_MSG Commit Message:

https://review.coreboot.org/c/coreboot/+/37911/3//COMMIT_MSG@11 PS3, Line 11: we should hold reset assertion for 100ms : in its power on sequence In the latest datasheet I could find, I don't see the RESET signal mentioned in the power sequencing section anywhere...

-- To view, visit https://review.coreboot.org/c/coreboot/+/37911 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Iccb33c13c9a390a2c971325c74c0c4ad4b08618e Gerrit-Change-Number: 37911 Gerrit-PatchSet: 3 Gerrit-Owner: shkim sh_.kim@samsung.com Gerrit-Reviewer: Tim Wawrzynczak twawrzynczak@google.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-Reviewer: shkim sh_.kim@samsung.com Gerrit-CC: Furquan Shaikh furquan@google.com Gerrit-CC: Tim Wawrzynczak twawrzynczak@chromium.org Gerrit-Comment-Date: Thu, 26 Dec 2019 20:51:34 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

SH Kim (Code Review)

27 Dec 27 Dec

2:13 a.m.

New subject: Change in coreboot[master]: mb/google/kohaku: Update reset_delay_ms for digitizer device

SH Kim has uploaded a new patch set (#4) to the change originally created by shkim. ( https://review.coreboot.org/c/coreboot/+/37911 )

Change subject: mb/google/kohaku: Update reset_delay_ms for digitizer device ......................................................................

mb/google/kohaku: Update reset_delay_ms for digitizer device

We found the driver binding failure issue could be cleared with 100ms of "reset_delay_ms". Needs further check with device vendor, anyway it seems the IC need some time before communication after de-assertion of reset.

BUG=b:129159369 BRANCH=firmware-hatch-12672.B TEST=Verified driver bound successfully.

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/11/37911/4

SH Kim (Code Review)

2:14 a.m.

New subject: Change in coreboot[master]: mb/google/kohaku: Update reset_delay_ms for digitizer device

SH Kim has uploaded a new patch set (#5) to the change originally created by shkim. ( https://review.coreboot.org/c/coreboot/+/37911 )

Change subject: mb/google/kohaku: Update reset_delay_ms for digitizer device ......................................................................

mb/google/kohaku: Update reset_delay_ms for digitizer device

BUG=b:129159369 BRANCH=firmware-hatch-12672.B TEST=Verified driver bound successfully.

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/11/37911/5

SH Kim (Code Review)

2:14 a.m.

New subject: Change in coreboot[master]: mb/google/kohaku: Update reset_delay_ms for digitizer device

SH Kim has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37911 )

Change subject: mb/google/kohaku: Update reset_delay_ms for digitizer device ......................................................................

Patch Set 5:

(1 comment)

https://review.coreboot.org/c/coreboot/+/37911/3//COMMIT_MSG Commit Message:

https://review.coreboot.org/c/coreboot/+/37911/3//COMMIT_MSG@11 PS3, Line 11: we should hold reset assertion for 100ms : in its power on sequence

...

In the latest datasheet I could find, I don't see the RESET signal mentioned in the power sequencing […]

We might have miss-communicated with device vendor. As we checked with them again, we don't need this delay. Thanks for the pointing!

-- To view, visit https://review.coreboot.org/c/coreboot/+/37911 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Iccb33c13c9a390a2c971325c74c0c4ad4b08618e Gerrit-Change-Number: 37911 Gerrit-PatchSet: 5 Gerrit-Owner: shkim sh_.kim@samsung.com Gerrit-Reviewer: Tim Wawrzynczak twawrzynczak@google.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-Reviewer: shkim sh_.kim@samsung.com Gerrit-CC: Furquan Shaikh furquan@google.com Gerrit-CC: SH Kim sh_.kim@samsung.corp-partner.google.com Gerrit-CC: Tim Wawrzynczak twawrzynczak@chromium.org Gerrit-Comment-Date: Fri, 27 Dec 2019 01:14:15 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Tim Wawrzynczak twawrzynczak@chromium.org Gerrit-MessageType: comment

Furquan Shaikh (Code Review)

3:31 a.m.

New subject: Change in coreboot[master]: mb/google/kohaku: Update reset_delay_ms for digitizer device

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

Change subject: mb/google/kohaku: Update reset_delay_ms for digitizer device ......................................................................

Patch Set 5: Code-Review+2

-- To view, visit https://review.coreboot.org/c/coreboot/+/37911 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Iccb33c13c9a390a2c971325c74c0c4ad4b08618e Gerrit-Change-Number: 37911 Gerrit-PatchSet: 5 Gerrit-Owner: shkim sh_.kim@samsung.com Gerrit-Reviewer: Furquan Shaikh furquan@google.com Gerrit-Reviewer: Tim Wawrzynczak twawrzynczak@google.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-Reviewer: shkim sh_.kim@samsung.com Gerrit-CC: SH Kim sh_.kim@samsung.corp-partner.google.com Gerrit-CC: Tim Wawrzynczak twawrzynczak@chromium.org Gerrit-Comment-Date: Fri, 27 Dec 2019 02:31:48 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

SH Kim (Code Review)

3:40 a.m.

New subject: Change in coreboot[master]: mb/google/kohaku: Update reset_delay_ms for digitizer device

SH Kim has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37911 )

Change subject: mb/google/kohaku: Update reset_delay_ms for digitizer device ......................................................................

Patch Set 5:

(2 comments)

https://review.coreboot.org/c/coreboot/+/37911/3//COMMIT_MSG Commit Message:

https://review.coreboot.org/c/coreboot/+/37911/3//COMMIT_MSG@14 PS3, Line 14: It seems the IC need some time before communication : after de-assertion of reset.

...

Just guessing from our experiment. Without reset_delay, driver get failed to bind always. […]

Done

https://review.coreboot.org/c/coreboot/+/37911/1/src/mainboard/google/hatch/... File src/mainboard/google/hatch/variants/kohaku/overridetree.cb:

https://review.coreboot.org/c/coreboot/+/37911/1/src/mainboard/google/hatch/... PS1, Line 213: post-power-on-delay-ms

...

Please review the new patch set.

Done

-- To view, visit https://review.coreboot.org/c/coreboot/+/37911 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Iccb33c13c9a390a2c971325c74c0c4ad4b08618e Gerrit-Change-Number: 37911 Gerrit-PatchSet: 5 Gerrit-Owner: shkim sh_.kim@samsung.com Gerrit-Reviewer: Furquan Shaikh furquan@google.com Gerrit-Reviewer: Tim Wawrzynczak twawrzynczak@google.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-Reviewer: shkim sh_.kim@samsung.com Gerrit-CC: SH Kim sh_.kim@samsung.corp-partner.google.com Gerrit-CC: Tim Wawrzynczak twawrzynczak@chromium.org Gerrit-Comment-Date: Fri, 27 Dec 2019 02:40:25 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: shkim sh_.kim@samsung.com Comment-In-Reply-To: Tim Wawrzynczak twawrzynczak@chromium.org Comment-In-Reply-To: Furquan Shaikh furquan@google.com Gerrit-MessageType: comment

Patrick Georgi (Code Review)

10:03 a.m.

New subject: Change in coreboot[master]: mb/google/kohaku: Update reset_delay_ms for digitizer device

Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/37911 )

Change subject: mb/google/kohaku: Update reset_delay_ms for digitizer device ......................................................................

mb/google/kohaku: Update reset_delay_ms for digitizer device

BUG=b:129159369 BRANCH=firmware-hatch-12672.B TEST=Verified driver bound successfully.

Signed-off-by: Seunghwan Kim sh_.kim@samsung.com Change-Id: Iccb33c13c9a390a2c971325c74c0c4ad4b08618e Reviewed-on: https://review.coreboot.org/c/coreboot/+/37911 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Furquan Shaikh furquan@google.com --- M src/mainboard/google/hatch/variants/kohaku/overridetree.cb 1 file changed, 1 insertion(+), 1 deletion(-)

Approvals: build bot (Jenkins): Verified Furquan Shaikh: Looks good to me, approved

diff --git a/src/mainboard/google/hatch/variants/kohaku/overridetree.cb b/src/mainboard/google/hatch/variants/kohaku/overridetree.cb index 158731b..ce87469 100644 --- a/src/mainboard/google/hatch/variants/kohaku/overridetree.cb +++ b/src/mainboard/google/hatch/variants/kohaku/overridetree.cb @@ -206,7 +206,7 @@ register "generic.irq" = "ACPI_IRQ_LEVEL_LOW(GPP_C7_IRQ)" register "generic.enable_gpio" = "ACPI_GPIO_OUTPUT_ACTIVE_HIGH(GPP_C15)" register "generic.reset_gpio" = "ACPI_GPIO_OUTPUT_ACTIVE_LOW(GPP_A19)" - register "generic.reset_delay_ms" = "1" + register "generic.reset_delay_ms" = "100" register "generic.has_power_resource" = "1" register "hid_desc_reg_offset" = "0x1" device i2c 0x09 on end

-- To view, visit https://review.coreboot.org/c/coreboot/+/37911 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Iccb33c13c9a390a2c971325c74c0c4ad4b08618e Gerrit-Change-Number: 37911 Gerrit-PatchSet: 6 Gerrit-Owner: shkim sh_.kim@samsung.com Gerrit-Reviewer: Furquan Shaikh furquan@google.com Gerrit-Reviewer: Patrick Georgi pgeorgi@google.com Gerrit-Reviewer: Tim Wawrzynczak twawrzynczak@google.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-Reviewer: shkim sh_.kim@samsung.com Gerrit-CC: SH Kim sh_.kim@samsung.corp-partner.google.com Gerrit-CC: Tim Wawrzynczak twawrzynczak@chromium.org Gerrit-MessageType: merged

1852

days inactive

1856

days old

coreboot-gerrit@coreboot.org

17 comments

6 participants

tags (0)

participants (6)

Furquan Shaikh (Code Review)
Patrick Georgi (Code Review)
Seunghwan Kim (Code Review)
SH Kim (Code Review)
shkim (Code Review)
Tim Wawrzynczak (Code Review)